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

Rename OCSP extensions #3765

Merged
merged 3 commits into from
Jan 19, 2023
Merged

Rename OCSP extensions #3765

merged 3 commits into from
Jan 19, 2023

Conversation

goatgoose
Copy link
Contributor

@goatgoose goatgoose commented Jan 17, 2023

Resolved issues:

None

Description of changes:

TLS 1.3 allows status_request extensions to be sent in both the client hello, and in the certificate request message. This allows servers to request OCSP stapling from clients:

From RFC 8446:

   A server MAY request that a client present an OCSP response with its
   certificate by sending an empty "status_request" extension in its
   CertificateRequest message.

To support this, a new s2n_extension_type must be created that's sent by the server in certificate request messages, to request an OCSP staple from the client. This extension would normally be called s2n_server_status_request, but this name already exists.

This PR renames the OCSP stapling extensions to make a distinction between a status request and a status response (even though they all have the same IANA value). The following are all of the OCSP extensions, and what the names are changed to in this PR:

  • s2n_client_status_request: This extension is sent by clients to request OCSP stapling from TLS 1.2 and TLS 1.3 servers. This name was not changed.
  • s2n_server_status_request -> s2n_server_status_response: This extension is sent by TLS 1.2 servers in response to clients requesting OCSP stapling, indicating that a certificate status handshake message will be sent that contains an OCSP response.
  • s2n_server_certificate_status -> s2n_tls13_status_response This extension is sent by TLS 1.3 servers in response to clients requesting OCSP stapling, and contains the OCSP response in the extension. This extension will also be sent by clients in response to TLS 1.3 servers that request OCSP stapling, so server was removed from its name.
  • The new extension will be named s2n_server_status_request, which is sent by TLS 1.3 servers to request OCSP stapling from clients.

Call-outs:

I was having trouble thinking of names for these that made sense. Let me know if you think of better ones!

The actual change to add OCSP support for client auth will be made in a separate PR.

Testing:

How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer?

Is this a refactor change? If so, how have you proved that the intended behavior hasn't changed?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@goatgoose goatgoose marked this pull request as ready for review January 18, 2023 00:38
@lrstewart
Copy link
Contributor

These names definitely seem better than what we have, and I can't really think of better ones :)

Will there be a difference between s2n_client_status_request and s2n_server_status_request? Or should it be renamed to just s2n_status_request?

@maddeleine
Copy link
Contributor

These names definitely seem better than what we have, and I can't really think of better ones :)

Will there be a difference between s2n_client_status_request and s2n_server_status_request? Or should it be renamed to just s2n_status_request?

What about these names:
s2n_cert_status_request: Both client and server use this to request OCSP data
s2n_cert_status_response: Server uses this to affirm that the OCSP data will be sent in Certificate Status message (TLS1.2 only)
s2n_cert_status: Contains the actual OCSP data

I don't actually know if you can consolidate these extensions like this, but this is probably how I'd try to group these functionalities.

@goatgoose
Copy link
Contributor Author

goatgoose commented Jan 18, 2023

Thanks for the suggestions!

When a server requests an OCSP staple from a client, the RFC says the extension should be empty. This is different than the extension a client sends to a server, which contains a couple of fields. So, I think there should be a separate client and server extension for the request. So, s2n_client_cert_status_request and s2n_server_cert_status_request maybe.

I updated the PR with the following names:

  • s2n_client_status_request -> s2n_client_cert_status_request
  • s2n_server_status_request -> s2n_cert_status_response
  • s2n_server_certificate_status -> s2n_cert_status
  • The new extension for server OCSP requests will be s2n_server_cert_status_request.

@goatgoose goatgoose enabled auto-merge (squash) January 19, 2023 15:43
@goatgoose goatgoose merged commit 28959c2 into aws:main Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants