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

A bunch of editorial cleanup. Short summary: #438

Closed
wants to merge 2 commits into from

Conversation

ekr
Copy link
Collaborator

@ekr ekr commented Apr 13, 2017

  • Remove a bunch of the "can be used" language because we are
    specifying how to use it.

  • Update the 2119 language.

  • Clean up the description of key derivation, also shorten
    it.

  • Misc other stuff.

- Remove a bunch of the "can be used" language because we are
  specifying how to use it.

- Update the 2119 language.

- Clean up the description of key derivation, also shorten
  it.

- Misc other stuff.
selected by TLS. The key length is the AEAD key size. As defined in Section
5.3 of {{!I-D.ietf-tls-tls13}}, the IV length is the larger of 8 or N_MIN (see
Section 4 of {{!RFC5116}}). For any secret S, the corresponding key and
IV are derived as shown below:
Copy link
Member

Choose a reason for hiding this comment

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

There is an above, but not below.

I think that you mean to move the above code block to below this paragraph and remove the "as shown below:" from the previous one.

NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED",
"MAY", and "OPTIONAL" in this document are to be interpreted as
described in BCP 14 {{!RFC2119}}, {{!I-D.leiba-rfc2119-update}} when, and only when, they
appear in all capitals, as shown here.
Copy link
Member

Choose a reason for hiding this comment

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

bleargh

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does this mean you don't want this change, or you are just annoyed that there is a draft on this?

This arrangement means that some TLS messages receive redundant protection from
both the QUIC packet protection and the TLS record protection. These messages
are limited in number; the TLS connection is rarely needed once the handshake
completes.
Copy link
Member

Choose a reason for hiding this comment

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

I'd kinda like to keep something like this. We still double-encrypt NST and people will complain (or get it wrong) if we don't explain it somewhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This point is made in #packet-protection


~~~
info = (HashLen / 256) || (HashLen % 256) || 0x21 ||
"TLS 1.3, QUIC client 1-RTT secret" || 0x00
~~~


Copy link
Member

Choose a reason for hiding this comment

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

Please keep extra space between sections.

@ekr
Copy link
Collaborator Author

ekr commented Apr 18, 2017

@martinthomson The reason that Circle is sad here is an overlong line that should be already fixed in trunk and so should go away on merge.

@martinthomson
Copy link
Member

Merged by hand.

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.

2 participants