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

gh-120762: make_ssl_certs: Don't set extensions for the temporary CSR #125045

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

encukou
Copy link
Member

@encukou encukou commented Oct 7, 2024

openssl req fails with openssl 3.2.2 because the config line

authorityKeyIdentifier = keyid:always,issuer:always

is not supported for certificate signing requests (since the issuing certificate authority is not known).

David von Oheimb, the OpenSSL dev that made the change, commented in: openssl/openssl#22966 (comment) :

This problem did not show up in older OpenSSL versions because of a bug:
the req app ignored the -extensions option unless -x505 is given,
which I fixed in openssl/openssl#16865.

(I assume -x505 is a typo for -x509.)

In our make_cert_key function:

If sign is true:

  • We don't pass -x509 to req, so in this case it should be safe to omit the -extensions argument. (Old OpenSSL ignores it, new OpenSSL fails on it.)
  • The extensions are passed to the ca call later in the function. There they take effect, and authorityKeyIdentifier is valid.

If sign is false, this commit has no effect except rearranging the CLI arguments.

`openssl req` fails with openssl 3.2.2 because the config line

    authorityKeyIdentifier = keyid:always,issuer:always

is not supported for certificate signing requests (since the issuing
certificate authority is not known).

David von Oheimb, the OpenSSL dev that made the change, commented in:
openssl/openssl#22966 (comment) :

> This problem did not show up in older OpenSSL versions because of a bug:
> the `req` app ignored the `-extensions` option unless `-x505` is given,
> which I fixed in openssl/openssl#16865.

(I assume `-x505` is a typo for `-x509`.)

In our `make_cert_key` function:

If `sign` is true:
- We don't pass `-x509` to `req`, so in this case it should be safe to
  omit the `-extensions` argument. (Old OpenSSL ignores it, new OpenSSL
  fails on it.)
- The extensions are passed to the `ca` call later in the function.
  There they take effect, and `authorityKeyIdentifier` is valid.

If `sign` is false, this commit has no effect except rearranging the
CLI arguments.
Copy link
Contributor

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

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

LGTM

@encukou
Copy link
Member Author

encukou commented Oct 7, 2024

Thank you!

@encukou encukou merged commit 744caa8 into python:main Oct 7, 2024
36 checks passed
@encukou encukou deleted the make_ssl_certs-update branch October 7, 2024 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants