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

Nick/lastcall #26

Merged
merged 4 commits into from
Jun 5, 2018
Merged

Nick/lastcall #26

merged 4 commits into from
Jun 5, 2018

Conversation

grittygrease
Copy link
Collaborator

No description provided.

Copy link
Contributor

@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.

We might need to discuss the _cert extension some more.

MUST be unique within the scope of this connection (thus preventing replay
of authenticators). The certificate_request_context SHOULD be chosen to
be echoed in the authenticator message. A certificate_request_context
value MUST not be repeated for multiple authenticator requests generated by
Copy link
Contributor

Choose a reason for hiding this comment

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

MUST NOT

particular, the certificate MUST be valid for the a signature algorithm
indicated by the peer in a "signature_algorithms" extension, as described in
Section 4.2.3 of {{!TLS13}} and Sections 7.4.2 and 7.4.6 of {{!RFC5246}}.
indicated by the peer in a "signature_algorithms_cert" extension, as described in
Copy link
Contributor

Choose a reason for hiding this comment

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

Both signature_algorithms_cert and signature_algorithms apply here.

(Section 4.2.5. of {{!TLS13}}) extensions are used to guide certificate
selection. These extensions are taken from the authenticator request if
selection. These extensions are taken from the authenticator request if
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a risk here that the list be interpreted as definitive. That is, that we can't use other extensions. Maybe in this next sentence include "The extensions, or others that might affect certificate selection, ..." to avoid this misconception.

from one of the signature schemes in the authenticator request. Otherwise, the
signature algorithm used should be chosen from the "signature_algorithms_cert"
extension (in TLS 1.3) or "signature_algorithms" (in TLS 1.2) sent by the peer
in the TLS handshake.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that the signature_algorithms extension is what is relevant here. the _cert variant only really affects the keys on the chain up to, but not including the EE.

@@ -291,7 +298,7 @@ The following sections describes APIs that are considered necessary to implement
The "request" API takes as input:

* certificate_request_context (from 0 to 255 bytes)
* set of extensions to include (this MUST include signature_algorithms)
* set of extensions to include (this MUST include signature_algorithms_cert)
Copy link
Contributor

Choose a reason for hiding this comment

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

See above. The original text was correct, by my understanding at least.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All this is correct. I had a misconception about the two extensions that has since been corrected.

@grittygrease grittygrease merged commit fa27e65 into master Jun 5, 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.

2 participants