hashicorp / vault

A tool for secrets management, encryption as a service, and privileged access management
https://www.vaultproject.io/
Other
30.77k stars 4.16k forks source link

`use_csr_serial_number` option for PKI roles #25708

Open devon-mar opened 5 months ago

devon-mar commented 5 months ago

Is your feature request related to a problem? Please describe. Some applications generate a CSR with a serial number in the certificate's subject (not the serial number field). Sometimes it's desirable to ignore this value. The other values that can be taken from the CSR, namely SANs and common_name, have a use_csr_* option to control whether or not they take their values from the CSR. serial_number does not.

Describe the solution you'd like A use_csr_serial_number role parameter which controls whether or not the serial number in the CSR is used.

Describe alternatives you've considered Some applications do not have the ability to remove the serial number from the generated CSR.

/pki/sign-verbatim is not an option since it is not possible to restrict the values of other certificate attributes.

Explain any additional use-cases

Additional context

stevendpclark commented 5 months ago

Hi @devon-mar,

I've been looking a bit at the PR and your use-case, I don't think the use_csr_serial_number is a good fit here. The other usecsr* values, will leverage the value from the CSR but ignore any of the corresponding JSON field. I'd rather not introduce another use_csr_X with a different meaning as we can't start ignoring the serial_number JSON field today while keeping backwards compatibility.

As of today, what controls the subject's serial number field in an issued certificate is either the serial_number JSON data field, the CSR's sibject serial number along with allowed_serial_numbers role value. We always prefer the JSON data field if it contains a value, falling back to the CSR's serial number otherwise we generate one, assuming we passed the validation within allowed_serial_numbers

Could I propose serial_number_source instead as a new role field. This could provide the greatest flexibility in the future. A blank value or default value of json-csr-generate keeps the current behavior, while a value of say generate forces a generation and ignores the json/csr values. We could introduce different combinations later on as well depending on people's needs

Edit: proposal was completely wrong, as I conflated the certificate serial number and the subject serial number, we are talking about the subject serial as devon-mar corrected me below.

devon-mar commented 5 months ago

Hi @stevendpclark

Thanks for taking a look. I'd like to clarify that this issue and PR is only for the serial number in the certificate's subject, not the serial number field. I don't think Vault ever generates a value for the subject serial number, only the serial number field.

For example:

$ vault write -field=certificate pki/sign/test csr=@test.csr serial_number=foo456 ttl=3600s > test.crt
$ openssl x509 -noout -text -in test.crt
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            74:c5:23:18:73:95:e3:2b:ff:f2:88:7a:4f:73:1e:25:2f:79:24:f4
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: CN = Test
        Validity
            Not Before: Mar  4 21:01:51 2024 GMT
            Not After : Mar  4 22:02:21 2024 GMT
        Subject: CN = test, serialNumber = foo456

In the above example, I can only control the serialNumber field in the subject. The certificate serial number field is generated by Vault.

The serial_number field for /pki/sign/:name isn't documented, but I would assume that it is similar to the serial_number field of the other signing endpoints (like /pki/root/sign-intermediate). The documentation for those fields states:

Note that this has no impact on the Certificate's serial number field, which Vault randomly generates.
stevendpclark commented 5 months ago

Hi @devon-mar,

Yup that was a fail on my part, but the fundamental issue exists just my proposal is irrelevant, the use_csr_serial_number you are proposing doesn't behave the same as the others, and we can't really change that now and remain backwards compatible.

You are correct as well that the docs are missing which is another issue...

devon-mar commented 5 months ago

Would a serial_number_source field as suggested work instead then? If empty or json-csr, the existing behaviour of reading from the CSR, but allowing the JSON value to override is kept. If json, then the value is only taken from the JSON and the CSR subject serialNumber is ignored.

And the implementation would look something like:

if (role.SerialNumberSource == "" || role.SerialNumberSource == "json-csr") && ridSerialNumber == "" && csr != nil {
  ridSerialNumber = csr.Subject.SerialNumber
}