Skip to content
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

Tls restore compat #1256

Closed
wants to merge 2 commits into from
Closed

Tls restore compat #1256

wants to merge 2 commits into from

Conversation

huitema
Copy link
Contributor

@huitema huitema commented Mar 30, 2018

Removing the gratuitous change of QHKDF-Expand in draft 10, resolving issue #1255

@marten-seemann
Copy link
Contributor

Would this be in draft-11, and would we use that for the remote interop?

Copy link
Member

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of making this sort of change. It seems that it's a product of the idiosyncratic API provided by picotls more than anything else.

Don't take that as an indictment on the goals of the API. I really like the idea that the details of determining keys and IVs are kept contained. I only wish that NSS were anywhere near as principled. We have to break the PKCS#11 boundary to get HKDF to work.

For mine, a more principled AEAD API would take two labels: one for the key and one for the label, and avoid baking in the specific details of the TLS HKDF info structure. A hacky API change could distinguish between a NULL hash and an empty hash.

In summation, I'd prefer not to have to explain to future users of QUIC that this isn't a null terminator on the string, but a product of a design decision made before QUIC even really existed.

@@ -703,12 +703,13 @@ structure:
struct {
uint16 length = Length;
opaque label<6..255> = "QUIC " + Label;
uint8 hashLength = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be explained, or people will ask why the hash is zero length.

@kazuho
Copy link
Member

kazuho commented Apr 4, 2018

@martinthomson

For mine, a more principled AEAD API would take two labels: one for the key and one for the label, and avoid baking in the specific details of the TLS HKDF info structure. A hacky API change could distinguish between a NULL hash and an empty hash.

In summation, I'd prefer not to have to explain to future users of QUIC that this isn't a null terminator on the string, but a product of a design decision made before QUIC even really existed.

Thank you for your comments. While I understand and have sympathy to your arguments, I wonder if there's a strong reason to divert from the structure defined in TLS 1.3 considering the fact that the quic-tls draft depends on TLS 1.3.

@ekr
Copy link
Collaborator

ekr commented Apr 4, 2018

It's not just PicoTLS. This is an issue for Minq as well.

@martinthomson
Copy link
Member

picotls users, does h2o/picotls#138 mean that we're ok with closing this?

@huitema
Copy link
Contributor Author

huitema commented May 15, 2018

I am fine with closing this as "won't fix".

@huitema huitema closed this May 15, 2018
@huitema huitema deleted the tls-restore-compat branch May 15, 2018 23:40
martinthomson added a commit that referenced this pull request May 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants