Skip to content

Conversation

@DDvO
Copy link
Contributor

@DDvO DDvO commented Oct 20, 2021

When handling #16720, I noticed that the -reqexts option is ignored when the -x509 option is used and the -extensions option is ignored otherwise. This is not only needlessly confusing but also is not in line with the documentation and with the -extensions option of the x509 app.

This PR solves the issue by making the -reqexts option an alias of -extensions.
This also simplifies the code and cleans up the doc accordingly.

Checklist
  • documentation is added or updated

This simplifies code, doc, and use and fixes issue ignoring one or the other.
@DDvO DDvO added triaged: bug The issue/pr is/fixes a bug triaged: documentation The issue/pr deals with documentation (errors) triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly labels Oct 20, 2021
@t8m t8m added the branch: master Applies to master branch label Oct 20, 2021
Copy link
Member

@t8m t8m left a comment

Choose a reason for hiding this comment

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

Approved for master branch.

@t8m t8m added the approval: done This pull request has the required number of approvals label Oct 20, 2021
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Oct 21, 2021
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

openssl-machine pushed a commit that referenced this pull request Oct 22, 2021
This simplifies code, doc, and use.
Fixes issue ignoring one or the other.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #16865)
@DDvO
Copy link
Contributor Author

DDvO commented Oct 22, 2021

Merged - thanks for the swift approval.

@DDvO DDvO closed this Oct 22, 2021
encukou added a commit to encukou/cpython that referenced this pull request 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.
encukou added a commit to python/cpython that referenced this pull request Oct 7, 2024
…GH-125045)

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

`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.
nh2 added a commit to nh2/internal-contstrained-pki that referenced this pull request Oct 25, 2024
`-extensions v3_ca` was supported (and ingnored) by the `openssl req`
of OpenSSL < 3.3 only due to a bug, see:

* openssl/openssl#22966
* openssl/openssl#16865

See how Python fixed it similarly:

* python/cpython@744caa8

In our code, the options that are problematic for `openssl req`
but need to be set for `openssl x509` are set explicitly with
the inline-generated `.ini` files.

It still works with OpenSSL 3.0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch triaged: bug The issue/pr is/fixes a bug triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly triaged: documentation The issue/pr deals with documentation (errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants