Errors for bogus tickets. Fixes #1247 #1269
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
As noted in the original issue, we he have unknown_psk_identity, so I think this is unnecessary after all. |
| @@ -4102,6 +4103,13 @@ decrypt_error: | |||
| to correctly verify a signature or validate a Finished message | |||
| or a PSK binder. | |||
|
|
|||
| ticket_invalid: | |||
There was a problem hiding this comment.
"ticket" has a very specific connotation that I think you might want to avoid here. How about psk_rejected? My only reservation there is that it looks like your goal is to signal that the psk identifier is not acceptable to the server.
Comment on lines
+4110
to
+4111
| is being used for session resumption, the server MAY | ||
| ignore the ticket and perform a full handshake instead. |
There was a problem hiding this comment.
Do you really want 2119 language here? This is just how session resumption is supposed to work.
Suggested change
| is being used for session resumption, the server MAY | |
| ignore the ticket and perform a full handshake instead. | |
| is being used for session resumption, the server is expected | |
| to ignore the ticket and perform a full handshake instead. |
| @@ -4836,7 +4846,7 @@ The registries and their allocation policies are below: | |||
| with the values from {{alert-messages-appendix}}. The | |||
| "DTLS-OK" column is marked as "Y" for all such values. | |||
| Values marked as "_RESERVED" have comments | |||
| describing their previous usage. | |||
| describing their previous usage | |||
There was a problem hiding this comment.
Suggested change
| describing their previous usage | |
| describing their previous usage. |
Other list items have a period.
|
Shouldn't we add a security section to that? It feels to me like it may be very easy to mess up the identity checks in a way that provides information to the attacker (Bleichenbacher style). |
|
Do you have some specific text to propose?
…On Mon, Oct 24, 2022 at 4:46 AM Hubert Kario ***@***.***> wrote:
Shouldn't we add a security section to that? It feels to me like it may be
very easy to mess up the identity checks in a way that provides information
to the attacker (Bleichenbacher style).
—
Reply to this email directly, view it on GitHub
<#1269 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIPLIIGGFOVI66OHOOUGCLWEZZJTANCNFSM6AAAAAARLOWL54>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
It's very implementation dependent, how the ticket is constructed. A ticket that's RSA encrypted, AES encrypted, or just a database key will have completely different behaviours and possible attacks, so unfortunately at best I think we can provide only something vague and generic. Rather bad, bad something like this: |
|
Thanks. We have so far remained agnostic about ticket construction and I
think we should do the same here. The introduction of this error would not
change what the client learns, I don't think.
Note that I actually don't plan to land this PR anyway, as there is already
a fine alert.
…On Mon, Oct 24, 2022 at 7:27 AM Hubert Kario ***@***.***> wrote:
It's very implementation dependent, how the ticket is constructed. A
ticket that's RSA encrypted, AES encrypted, or just a database key will
have completely different behaviours and possible attacks, so unfortunately
at best I think we can provide only something vague and generic.
Rather bad, bad something like this:
Note that the decryption and processing of the ticket may provide a side channel about information contained in the ticket or internal state of the server.
Implements should analyse if the particular ticket construction doesn't leak sensitive information when processed from malformed messages or unexpected sources. Using authenticated encryption over all fields of the ticket is one of recommended mitigation strategies.
—
Reply to this email directly, view it on GitHub
<#1269 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIPLIOM3ZTPYMSLBYWX6CLWE2MF7ANCNFSM6AAAAAARLOWL54>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
I wasn't thinking about the presence or even value of the alert being the side channel, I was thinking about timing side-channels. |
|
This really seems out of scope then.
…On Mon, Oct 24, 2022 at 11:31 AM Hubert Kario ***@***.***> wrote:
I wasn't thinking about the presence or even value of the ticket being the
side channel, I was thinking about timing side-channels.
—
Reply to this email directly, view it on GitHub
<#1269 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIPLIJQZ7KXSAVQPVFRVW3WE3IY7ANCNFSM6AAAAAARLOWL54>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.