-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Framing protocol: more expressive error responses possible #228
Comments
Thanks for reaching out and engaging with us. This is an interesting topic and I would say that it is almost already possible to start to use. I looked through our implementation of the protocol in So in general, if I'm not missing something, I would not say that it is to late and that one are free to use it like above. We do not use it like this in our applications at the moment, but most of them so far are quite small that any error simply kills the application, or it aborts the current command and waits for the next (depending on error). Are you using the errors, like you described above, in your applications? In the long run we are looking into if we should use something completely different, like protobuf or similar. Where we can have a config that spans over multiple applications, and that handles the packing of packages for you. But it is a discussion that has barely started. |
This is interesting, because I think the documentation states that it should return
I looked into this because I was looking for a way to respond appropriately to firmware-destined requests, such that the client can discover that TKey isn't in firmware. Unfortunately, the current solution for this error-case is that of not responding. This gives the complication that one has to work with a read-timeout, and consequently "guessing that data will not arrive". Problems that cannot be expressed, AFAICT:
Note: As was stated in another issue already, the App name and version cannot be blindly trusted, and given that I have other ways to validate the claim. The client will still use App name and version to determine if program is expected and if so to determine which API to use.
Yes. I am exploring what the most suitable mechanisms are, hence this issue report. There are other considerations not immediately relevant here. I noticed that the mechanisms already provided as part of the header/protocol are not embraced to their full potential.
Understandable. That would be my preference if I could. This program is a bit bigger, and I'd like the client to be able to adjust if needed.
I realized that. I'm reaching out to make you aware of the possibilities within your chosen direction.
This seems nice. Maybe also very ambitious at the same time. I don't know what the reasonable capacities are. For example, I assume there is a reason that you selected a fairly small So far, I find that I can do quite a bit with the basic mechanisms that are in place now. |
Yes, you are correct that the firmware never will respond with a To answer to the problems you list:
But I do agree with you in the command/response ID, I'm not a fan of having command and response IDs, from my point of view they should be the same and then let the command payload handle the variations if there would be any. I think one of the reasons for Anyhow, all breaking changes to the framing protocol is hard to introduce, but we do appreciate your input. |
I may be misunderstanding something you're talking about but tkey-device-signer does respond to a firmware probe here: https://github.com/tillitis/tkey-device-signer/blob/main/signer/main.c#L371 If you send something with the byte header endpoint set to 2 (that is, firmware), it replies This is also what we rely on in for example tkey-ssh-agent to probe for firmware. If we do get a Oh, and yes, the small sizes and the command/respone on every packet is a legacy from when the communication wasn't very reliable. It has been hard to change the protocol since, of course. It will probably happen, as dehanj mentions, and probably using something like Protobuf, Cap'n Proto, CBOR or something else that is easy to generate code for. If we can fit it within the size requirements. |
Regarding the firmware probe I added some proposed changes to the Dev Handbook: |
No, you aren't misunderstanding. Your counter-examples are perfectly on point. I remember finding a case, first place I looked, that didn't respond. I will try to find it again. |
Okay, so what I meant is: a request with command ID
This just referred to the bug mentioned earlier in this issue, that the response body was not returned if
This is interesting: the way that I know how frame-IDs are used, is to distinguish between multiple (parallel) in-flight requests. So I would only use frame-ID for concurrency (concurrent design), so either: parallel request handling, or to handle a secondary request before the first request can be concluded. For example:
Consequently, from the perspective of a single frame-ID, interaction is strictly linear: request followed by corresponding response. This would allow concurrent interactions without confusing requests/responses of parallel in-flight requests. (Note: this is akin to request-multiplexing in HTTP/2 and QUIC.) Now for me, given that -- at the moment -- I process one request at a time in blocking fashion, I constantly reuse the same frame-ID. No multi-threading, no concurrency. (Yet)
I'll have a look there. My current approach to larger requests, is the following: (i.e. the current experiment)
This still provides space for up to 127 distinct commands (excl. Simple bit-masking is sufficient to read the flag or the command independently. It also means that large payloads can simply be cut up at the frame-length border and transmitted in multiple frames as we simply set the I didn't mention this before, because I do not think this needs to be part of a protocol. This idea operates within the space offered by the protocol. I am elaborating this now, because of your mention of use of the frame-ID for which I seem to have a different purpose in mind. |
First, my mistake. I missed the
That is correct, and is to some extent how we use it in the experiment as well (unless you are referring to multi threaded request handling which it is not). Basically, in the experiment it is using the same command for all request (loading an app), simply filling up the FIFO buffer on the TKey, so the Tkey can run at its "max capacity" and does not have to wait for the next request, it is already in the FIFO buffer.
In this experiment it is still parsing the frames linearly, since that is how the implementation of loading an app in fw is (this was partly an experiment to confirm that the bug in the USB-to-serial converter was fixed, so changing fw was out of scope). But it does not need to be linearly, as long as the application has enough information in the payload (such as offset), then each frame could be completely independent, and hence be parsed in whatever order. The client would keep track of which frame is lost/gets an error for with the frame ID.
We do the same, reuse the same since we don't expect multiple in-flight packets. Anyhow, my point was with the comment from the start that since there is a frame ID, if you don't know the proper response command, replying with a
Nice approach. Looks sound, we might take inspiration in some future application. |
Right, here it seems our approaches differ. You still respond to every frame, while I send all frames with I understand your use of the frame-ID now, because it allows you to send 4 frames and still receive and correlate the response for all of the them. (As you explain; I'm summarizing again.)
That's clear.
Yup, got that. I guess my point is that:
Isn't that meaningful anymore, right? This suggests responding with response-byte and Maybe this isn't something for which follow-up discussion is useful. I pointed out these distinct error cases, as there seems to be some ambiguity and also room for more expressiveness, and you already explained that the protocol is a general indication. I understand your approach with using the frame-IDs for correlating individual requests to responses. That makes sense. As you mention, to some extent the direction is set. Thanks for clarifying. |
You are right, and as you mentioned earlier firmware does behave slightly different than stated. I will see into if an overhaul of the protocol documentation is due, to minimize some ambiguity and make it clearer. Sure, thanks for the discussion. |
I want to put this information out there, maybe it is too late and it is understable if you close the issue. I just want to make sure you are aware that the possibilities exist within the room offered by the framing protocol.
With the fields that are available in the framing protocol, it seems to be possible for more granular, expressive bad/error response messages. These suggestions avoid the need for magical values (all zeroes, all ones, etc.) or patterns, simply because the frame-header can be sufficiently expressive without.
These are the possibilities (cases) I distinguished:
ResponseNotOK
+Command-0
+Length-1
to indicate problem with header: non-existent command, no actual body, soResponseNotOK
can only refer to parts from frame-header: DST, forbidden bits, etc.This allows responding to misdirected firmware/software commands, such that ignoring the message is not necessary. Ignoring the message is problematic because you expect the client-software to draw their own conclusions from that, in addition to having to wait for a time-out.
ResponseNotOK
+<command>
+Length-1
to indicate problem with command: no actual body, so unsupported/unknown/illegal command. One can discuss whether "unknown/unsupported" and "wrong program state for command" should be considered same.This allows you to indicate that the command itself is the problem (at "this" time).
ResponseNotOK
+<command>
+Length-{4,32,128}
to indicate problem with command-body: even if the return body is irrelevant (e.g. all-zeroes), the fact that Length-1 was not chosen can function as indicator that the body rather than the command is the problem. One can even distinguish sub-cases:As a personal opinion, I am not too fond of using a single pool of ID-values for commands and responses. It doesn't seem realistic to consider both ends peers in capabilities/requests. So why not have a command
1
respond with a response1
? (And other IDs if there are more responses, or reusable responses.)An additional problem with separate command and response IDs: if some application does not know the (firmware) command, how to respond if the corresponding response ID needs to be supplied? In that light, with the error-communication as outlined above, any program can sensibly respond to any request.
I have not found a good reason why, possibly in light of error-communication, different response-IDs are necessary. They don't hurt. However, I have not found added benefits.
edit this also should remove the need for juggling with the timeout values, as it is possible to respond with a sensible error response. (See tillitis/tkeyclient#9)
The text was updated successfully, but these errors were encountered: