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.

Sign up for GitHub

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

Jump to bottom

Transport parameters are mandatory. Fixes #2528 #2560

Merged
merged 5 commits into from
Apr 30, 2019
Merged

Transport parameters are mandatory. Fixes #2528 #2560

merged 5 commits into from
Apr 30, 2019

Conversation

ekr
Copy link
Collaborator

@ekr ekr commented Mar 27, 2019 •
edited by ianswett
Loading

Fixes #2528

martinthomson
martinthomson approved these changes Mar 27, 2019
View reviewed changes
draft-ietf-quic-tls.md Outdated
@@ -1249,7 +1249,10 @@ quic_transport_parameters extension carries a TransportParameters struct when
the version of QUIC defined in {{QUIC-TRANSPORT}} is used.

The quic_transport_parameters extension is carried in the ClientHello and the
EncryptedExtensions messages during the handshake.
EncryptedExtensions messages during the handshake. Endpoints MUST send the
quic_transport_parameters extension; endpoints which receive ClientHello
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
quic_transport_parameters extension; endpoints which receive ClientHello
quic_transport_parameters extension; endpoints that receive ClientHello

CMOS

draft-ietf-quic-tls.md Outdated
EncryptedExtensions messages during the handshake.
EncryptedExtensions messages during the handshake. Endpoints MUST send the
quic_transport_parameters extension; endpoints which receive ClientHello
or EncryptedExtensions messages without the transport_parameters extension MUST
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
or EncryptedExtensions messages without the transport_parameters extension MUST
or EncryptedExtensions messages without the quic_transport_parameters extension MUST

draft-ietf-quic-tls.md Outdated
EncryptedExtensions messages during the handshake. Endpoints MUST send the
quic_transport_parameters extension; endpoints which receive ClientHello
or EncryptedExtensions messages without the transport_parameters extension MUST
terminate the TLS handshake with a fatal missing_extension alert.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
terminate the TLS handshake with a fatal missing_extension alert.
terminate the TLS handshake with a fatal missing_extension alert (an error of 0x16d).

@martinthomson martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. -tls labels Mar 27, 2019
janaiyengar
janaiyengar reviewed Mar 27, 2019
View reviewed changes
draft-ietf-quic-tls.md
EncryptedExtensions messages during the handshake. Endpoints MUST send the
quic_transport_parameters extension; endpoints that receive ClientHello or
EncryptedExtensions messages without the quic_transport_parameters extension
MUST terminate the TLS handshake with a fatal missing_extension alert (an error
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
MUST terminate the TLS handshake with a fatal missing_extension alert (an error
MUST close the connection with an error of type 0x16d (equivalent to a fatal TLS missing_extension alert, see Section {{tls-errors}}).

Copy link
Contributor

Choose a reason for hiding this comment

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

This will result in "Section Section X.Y" in the final output.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, that comma should be a semicolon; see #2626.

draft-ietf-quic-tls.md Outdated
unsupported_extension alert MUST be sent if this extension is received when the
transport is not QUIC.
unsupported_extension alert (an error of 0x16e) MUST be sent if this extension
is received when the transport is not QUIC.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not just in your text, but while you're here, also replace the text with:
"The connection MUST be closed with an error code of type 0x16e (equivalent to a fatal TLS unsupported_extension alert, see Section {{tls-errors}}) if the transport is not QUIC."

Copy link
Contributor

@MikeBishop MikeBishop Apr 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, I don't think this suggestion is needed. If the transport is not QUIC, then TLS can presumably send its normal alert messages, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @MikeBishop. Specifying the QUIC error code to send if the extension is received when the transport isn't QUIC doesn't make much sense.

MikeBishop
MikeBishop reviewed Apr 16, 2019
View reviewed changes
Copy link
Contributor

@MikeBishop MikeBishop left a comment

Choose a reason for hiding this comment

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

These issues are not with the text as written, but with Jana's suggested changes. 😉

draft-ietf-quic-tls.md
EncryptedExtensions messages during the handshake. Endpoints MUST send the
quic_transport_parameters extension; endpoints that receive ClientHello or
EncryptedExtensions messages without the quic_transport_parameters extension
MUST terminate the TLS handshake with a fatal missing_extension alert (an error
Copy link
Contributor

Choose a reason for hiding this comment

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

This will result in "Section Section X.Y" in the final output.

draft-ietf-quic-tls.md Outdated
unsupported_extension alert MUST be sent if this extension is received when the
transport is not QUIC.
unsupported_extension alert (an error of 0x16e) MUST be sent if this extension
is received when the transport is not QUIC.
Copy link
Contributor

@MikeBishop MikeBishop Apr 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, I don't think this suggestion is needed. If the transport is not QUIC, then TLS can presumably send its normal alert messages, no?

@martinthomson martinthomson merged commit d14cd3f into quicwg:master Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@janaiyengar janaiyengar janaiyengar left review comments

@MikeBishop MikeBishop MikeBishop left review comments

@nharper nharper nharper left review comments

@ianswett ianswett ianswett approved these changes

@martinthomson martinthomson martinthomson approved these changes

Assignees
No one assigned
Labels
-tls design An issue that affects the design of the protocol; resolution requires consensus.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Are transport parameters mandatory?
6 participants
@ekr @martinthomson @nharper @MikeBishop @janaiyengar @ianswett