-
Notifications
You must be signed in to change notification settings - Fork 158
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
Encrypt content type #51
Conversation
8a3c2a8
to
38f0630
Compare
This pull request has been updated to the current master |
@@ -1010,17 +1010,14 @@ of {{RFC5116}}. The key is either the client_write_key or the server_write_key. | |||
|
|||
%%% Record Layer | |||
struct { | |||
ContentType type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we had agreed to have a dummy type outside the record, so that it doesn't break everyone's parser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the plan was just to do all the changes and see if anything important breaks that needs a kludge to deal with. I think you said this at one of the meetings, if I'm remembering from the video correctly.
Consensus at meeting to merge this PR with updates and with the dummy type byte restored. @ekr has action item to actually measure whether we can strip off the now fixed type, record.version bytes. |
Previous versions of TLS have left the ContentType in the clear without justifying the exposure of this piece of information. This change moves the real ContentType inside the encryption to protect its confidentiality. It leaves the external ContentType, but fixes its value to application_data(23) to appease silly middleboxes. For clarity, this also means redefining the NULL_NULL case to not simply be an pseudo-cipher that implements the identity transformation, but to explicitly state it as sending the TLSPlaintext struct directly over the wire. ekr has agreed to re-write that text.
38f0630
to
797f53a
Compare
I've updated this with the expected changes: dummy external ContentType octet is restored, and fragment.type is after fragment.content instead of the other way around (to make it easier to encrypt without having to shuffle buffers). |
This follows a good suggestion from Dave Garrett about disambiguating field names.
I agree with davegarrett's suggestion about field naming here, and have supplied a change implementing it. |
No description provided.