-
Notifications
You must be signed in to change notification settings - Fork 367
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
Thinking about changing handshake alert behavior for debug. #606
Comments
While it is true that the DTLS specification recommends that records which are invalid are silently ignored rather than having a fatal alert generated. In practice there is a big difference between doing this during a handshake and after the finish message has been processed. When invalid records are ignored after the handshake has finished, the client will continue and send new records as appropriate without any issues. At this point in time, sending a fatal alert is going to needlessly shutdown the DTLS connection. Not sending a fatal alert during the handshake, specifically for the finish message, means that the client is going to get stuck in the state of re-sending the same message set over and over without being able to advance until such a time as it decides to stop on its own. (As the client never gets a response to the last pair of handshake records - the CSS and Finish record - It will continually time out and resend the same messages which are treated in the same manner by the server.) Sending the fatal alert for a bad record containing the finish message is going to have better network behavior. That said, per the authors of TLS/DTLS the behavior as implemented is not wrong. It is merely IMO sub-optimal for the handshake case. I have not verified this, but I have been told that this will not be an issue for DTLS 1.3 as the handshake will fail in a different manner. |
Note this is the same issue as was raised in #592 |
#592 was assuming that the PSK support in scandium was broken, but it shows, that it was only the wrong secret. FMPOV, for that issue #592 the logging message
should be improved indicating, that a different secret may be the cause of that failure. The other question seems to be, if scandium should offer a configurable possibility to send ALERTs in that case. FMPOV, RFC6347 is NOT clear about that! Until now, only one author answered @jimsch and @sbernard31 questions at all. I would say, it is at least ambig, which parts of RFC6347 should be applied, I don't see, that the RFC specifies a precedence of 4.1.2.7 over 4.2 and so to silently ignore the invalid record is in my opinion a possible implementation, but not the only valid one. For me it seems to be still useful, to extend scandium to send also alert in that case based on a configuration flag. |
One additional reason for returning an error in this case would be to free resources on the server faster. If you return an error here then you can just clean things up instead of waiting until some point in the future. If a client stopped quickly and then sent 100s of attempts to negotiate, you are going to have lots of partially negotiated contexts. I don't know what you are doing to reap old contexts, any answer is wrong for some set of conditions. |
I have been reading RFC 4279 section 2 about unknown psk identities. It would appear that you have chosen not to return an unknown_psk_identity based on #605. In my reading the specification says you can return the alert, or continue and then respond with a decryption_error alert at the finish in order to hide identities. If you ignore the decryption error on the finish then that alert would never be sent. I believe that violates the spirit of the RFC |
For me it seems to be a question of precedence: In sum, I believe, that, if there is no clear spec., making it configurable would do it best. |
@jimsch, here is a discussion about this on the TLS ietf mailing list.
I agree but a client must handle this anyway, as nothing assure that server will not lost DTLS connection later. So client must be able to detect that records sent was ignored and retry a new handshake (or find new credentials)
Yes, it's probably an optimization but not so frequent use case. I mean this happened only for miss-configured devices or credentials rotations.
OK, but we must deal with incomplete handshake anyway.
You talk about optional ACK message ? except that I can see any difference about that ? My general point of view about "sending an alert" is :
So, at short-term I would keep "ignore" behavior and improve logs. If we want to make it configurable, we should get more information from the authors to be sure it's a good idea to send alert. |
In general, DTLS implementations SHOULD silently discard records with Thus it is perfectly permissible to fail with an alert if the implementation desires to. Nor does it say that the same behavior is required for all messages.
Improving the logs does not necessarily do me any good if I am testing a server of somebody in Europe while I sit in the US and they left their system up overnight for me. I would not be able to get to the logs to figure out what is going wrong. |
To be clear, I'm ok sending an alert could be better from a behavior point of view, but I would not sacrifice security for this. So I will try to have more information from IETF. |
FMPOV: |
Currently an unsuccessful handshake because of bad PSK is ignored (server does not answer)
And with #605 unknown psk identity will do the same.
This is the recommended behavior. (referring the TLS and DTLS RFC).
But, it seems there is needs to use scandium as test server to debug clients. So more information could be helpful.
An idea about a way to configure DTLSConnector to change its behavior to send explicit Alert (instead of ignoring record) was proposed.
At first sight, I liked the idea, but I had discussions with teammate and he convinced me this is maybe not a so good idea.
If we expose sandboxes or examples with a default behavior which is no so recommended, we could send a "bad message" to developers, meaning this is the right behavior or the behavior to expect. This could led to new bad DTLS implementation and could affect interoperability.
If the needs is to help developers/debugging maybe, we should made an effort to provide easy/better logs instead.
Currently using :
<logger name="org.eclipse.californium.scandium.DTLSConnector" level="DEBUG"/>
I get this kind of logs.
For UNKNOWN PSK IDENTITY :
For BAD PSK :
Changing log level is not so easy for beginners and maybe we could propose a verbose mode (-v arguments) with a default log configuration which helps to debug.
WDYT ?
The text was updated successfully, but these errors were encountered: