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

Remove signature_bits on intermediate generate #15478

Merged
merged 2 commits into from May 18, 2022

Conversation

cipherboy
Copy link
Contributor

This extraneous field wasn't respected during intermediate generation
and it isn't clear that it should be. Strictly, this field, if it were
to exist, would control the CSR's internal signature algorithm (certutil
defaults to the sane SHA-256 here). However, there's little value in
changing this as the signing authority can and probably will override
the final certificate's signature bits value, completely ignoring
whatever was in the provided CSR.

Removing this field will now cause warnings for those providing the
parameter (which already wasn't respected), which is the desired
behavior. No breakage should occur as a result of this change.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>


By removing this field, we now get a warning when it is specified:

> vault write pki/intermediate/generate/existing key_ref=default  signature_bits=512
WARNING! The following warnings were returned from Vault:

  * Endpoint ignored these unrecognized parameters: [signature_bits]

Note this removes the path-help; no API docs for this parameter existed (it probably should for root generation though).

@cipherboy cipherboy added bug Used to indicate a potential bug secret/pki labels May 17, 2022
@cipherboy cipherboy added this to the 1.11.0-rc1 milestone May 17, 2022
@cipherboy cipherboy requested a review from a team May 17, 2022 19:10
This extraneous field wasn't respected during intermediate generation
and it isn't clear that it should be. Strictly, this field, if it were
to exist, would control the CSR's internal signature algorithm (certutil
defaults to the sane SHA-256 here). However, there's little value in
changing this as the signing authority can and probably will override
the final certificate's signature bits value, completely ignoring
whatever was in the provided CSR.

Removing this field will now cause warnings for those providing the
parameter (which already wasn't respected), which is the desired
behavior. No breakage should occur as a result of this change.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
@cipherboy cipherboy force-pushed the cipherboy-remove-signature-bits-intermediate-csr branch from 4767233 to 5a57881 Compare May 17, 2022 19:21
@cipherboy cipherboy merged commit 065ace5 into main May 18, 2022
Gabrielopesantos pushed a commit to Gabrielopesantos/vault that referenced this pull request Jun 6, 2022
* Remove signature_bits on intermediate generate

This extraneous field wasn't respected during intermediate generation
and it isn't clear that it should be. Strictly, this field, if it were
to exist, would control the CSR's internal signature algorithm (certutil
defaults to the sane SHA-256 here). However, there's little value in
changing this as the signing authority can and probably will override
the final certificate's signature bits value, completely ignoring
whatever was in the provided CSR.

Removing this field will now cause warnings for those providing the
parameter (which already wasn't respected), which is the desired
behavior. No breakage should occur as a result of this change.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add changelog

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
@cipherboy cipherboy deleted the cipherboy-remove-signature-bits-intermediate-csr branch June 16, 2022 15:35
cipherboy added a commit that referenced this pull request Jun 23, 2022
This endpoint was lacking the signature_bits field like all the other
endpoints. Notably, in #15478, the ability to customize the intermediate
CSR's signature bits was removed without checking for the ability to
customize the final (root-signed) intermediate certificate's value.

This adds in that missing ability, bringing us parity with root
generation and role-based signing.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
cipherboy added a commit that referenced this pull request Jun 23, 2022
* Add signature_bits to sign-intermediate

This endpoint was lacking the signature_bits field like all the other
endpoints. Notably, in #15478, the ability to customize the intermediate
CSR's signature bits was removed without checking for the ability to
customize the final (root-signed) intermediate certificate's value.

This adds in that missing ability, bringing us parity with root
generation and role-based signing.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add signature_bits to sign-verbatim

This endpoint was also lacking the signature_bits field, preventing
other signature hash functions from being utilized here.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add changelog

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug secret/pki
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants