PKI: Support explicit Basic Constraints isCA=False

[cipherboy]

As reported by @mvkonovalov on hashicorp/vault#24817:

Describtion When I try to use /pki_int/sign-verbatim endpoint to sign a new certificate based upon the provided CSR with added basicConstraints=critical,CA:false, I get the following warning in request response: warnings":["specified CSR contained a Basic Constraints extension that was ignored during issuance"]

The issued cert has no any basic constrains.

According to Openssl documentation (see https://www.openssl.org/docs/man1.1.1/man5/x509v3_config.html) for some application it is required to have basicConstraints=critical,CA:false in case of end-entity certificate but vault ignore explicitly provide constrains.

Basic Constraints.
This is a multi valued extension which indicates whether a certificate is a CA certificate. The first (mandatory) name is CA followed by TRUE or FALSE. If CA is TRUE then an optional pathlen name followed by an non-negative value can be included.
For example:
basicConstraints=CA:TRUE
basicConstraints=CA:FALSE
basicConstraints=critical,CA:TRUE, pathlen:0
A CA certificate must include the basicConstraints value with the CA field set to TRUE. An end user certificate must either set CA to FALSE or exclude the extension entirely. Some software may require the inclusion of basicConstraints with CA set to FALSE for end entity certificates.

Perhaps in case of /pki_int/sign-verbatim there should be parameter similar to use_csr_values as for /pki_int/root/sign-intermediate.

To Reproduce Steps to reproduce the behavior:

1. Perform first 2 steps from https://developer.hashicorp.com/vault/tutorials/secrets-management/pki-engine

2. Run `openssl genrsa -out end-entity-pk.pem 4096`

3. Run

openssl req \
    -addext "basicConstraints=critical,CA:false" \
    -new -sha256 -key end-entity-pk.pem \
    -subj "/C=/L=/O=/OU=/CN=foo" \
    -out end-entity-cert.csr

4. Run

cat > payload.json<<EOF
{
    "format":"pem",
    "csr":"$(sed ':a;N;$!ba;s/\n/\\n/g' end-entity-cert.csr)",
    "ttl":"180d"
}
EOF

curl \
    --header "X-Vault-Token: $VAULT_TOKEN" \
    --request POST \
    --data @payload.json \
    $VAULT_ADDR/v1/pki_int/sign-verbatim

Expected behavior Issued certificate contains the following:

X509v3 Basic Constraints critical:
CA:FALSE

---

This likely warrants a new set of parameters on sign-verbatim mirroring what is on the role: basic_constraints_valid_for_non_ca. Since it is already on the role, this should be fairly easy to add and thus a good first issue.

Note that, from memory, I believe use_csr_values=true is already set, though whether or not this should apply to the basic constraints isn't immediately clear.

[DanGhita]

Hi Nathan, I will take this issue, could you assign it to me ?

Thank you,
Dan

[DanGhita]

Hi Alex,

After a few changes in the code, I’m able now to issue a certificate containing the expected extension:

X509v3 Basic Constraints critical:
CA:FALSE

However, I have doubts about the original intention of the initial code: should I just remove the code raising the warning (lines 1050 - 1052 in pki/cert_util.go) , or there are some other (subtle) use-cases where this kind of warning is still necessary ?

[cipherboy]

@DanGhita I'd remove the warning, if the end certificate has the basic constraint marked valid. If there is no basic constraint extension, I'd still warn as it was still ignored. My 2c.

[DanGhita]

@naphelps I have pushed my branch and created my Pull Request.
@cipherboy I didn’t yet test the use-case where the CSR does not contain the basicConstraints field.
Could you confirm that in this case the issued certificate should not contain the basicConstraints extension ?

[DanGhita]

...Oops, the DCO (signed-off) didn’t work, I will try to fix this.

[cipherboy]

@DanGhita said:

@cipherboy I didn’t yet test the use-case where the CSR does not contain the basicConstraints field.
Could you confirm that in this case the issued certificate should not contain the basicConstraints extension ?

When:

1. Role var is asserted, the basic constraint should always be added, regardless of whether or not it is present in the CSR.
2. Role var is not asserted, basic constraint should never be added, regardless of whether it is present in the CSR.
   a. If the CSR contains the basic constraint assertion, it will be warned about and a cert issued.
   b. if the CSR contains no basic constraint assertion, there will be no warning and a cert is issued.

HTH!

[DanGhita]

@cipherboy : Sorry, but I don't understand what you mean by "Role var is asserted" and "Role var is not asserted".
Is this related to the Sign Verbatim option "name" ? (Specifies a role. If set, the following parameters from the role will have effect: ttl, max_ttl, generate_lease, no_store and not_before_duration).

If yes, your option #1 (Role var is asserted) means calling sign-verbatim without a role ? (basically, the initial scope of the current issue ?).

Furthermore, the option #2 corresponds to a call to sign-verbatim with a specified role ?

Many thanks!

[cipherboy]

@DanGhita Sorry, role as in the context of the internal data structure used to process requests. sign-verbatim is (just) a very, very permissive role, accessible only via a privileged endpoint.

The default version of this path did not have it and thus it was missing from the role. But if you add this parameter to the request (set the boolean variable to true == assert it), the role variable also becomes asserted (set to true), and thus behavior 1 applies.

Or if its missing from the request (or set to false explicitly in the request) behavior 2 applies.

Sorry about any confusion!

[DanGhita]

@cipherboy Sorry, in order to avoid any confusion, we should speak on both public API level and low level source code.
Typically, your explanations about "asserted or not asserted role variable" probably refer to the functions pathSignVerbatim (builtin\logical\pki\path_issue_sign.go, line #324) and pathIssueSignCert and their role *roleEntry parameter.

On the other hand, on the API level, your point is to add a new option to Sign Verbatim, something similar to [use_csr_values](https://developer.hashicorp.com/vault/api-docs/secret/pki#use_csr_values).

However, while it is true that sign-verbatim does support some extra parameters (https://developer.hashicorp.com/vault/api-docs/secret/pki#sign-verbatim), it seems to me that the initial intention of the endpoint was to use the provided CSR as it is ("verbatim"). The additional parameters are just to specify the behavior when the CSR does not contain explicit information about them.
For example, concerning the KeyUsage: "Note: previous versions of this document incorrectly called this a constraint; this value is only used as a default when the KeyUsage extension is missing from the CSR."

So maybe the right solution is just to take into account the basicConstraints from the provided CSR, with the following behavior (and no new option in the public API):

1. If the CSR contains the basicConstraints flag, add this extension in the issued certificate
2. Otherwise, don't add the basicConstraints extension in the issued certificate. And this is still a valid certificate.

Please also have a look to my PR .

[cipherboy]

@DanGhita From a high-level, external API perspective then, my original three points stand I believe. Look at the docs for the request parameter here: https://developer.hashicorp.com/vault/api-docs/secret/pki#basic_constraints_valid_for_non_ca for the role-based issuance.

Scenario 1 above, if you call:

$ bao write pki/sign-verbatim basic_constraints_valid_for_non_ca=true csr=@no-basic-constraints-extension.csr
$ bao write pki/sign-verbatim basic_constraints_valid_for_non_ca=true csr=@with-basic-constraints-extension.csr

It should issue a leaf certificate with the extension and with no warning.

Similarly, scenario 2(a) above:

$ bao write pki/sign-verbatim basic_constraints_valid_for_non_ca=false csr=@with-basic-constraints-extension.csr
$ bao write pki/sign-verbatim csr=@with-basic-constraints-extension.csr ## note: defaults to false, equivalent to above call!

this will issue a leaf certificate without the extension, but with a warning saying that the CSR contained an extension we ignored.

For scenario 2(b) above:

$ bao write pki/root/sign-intermediate basic_constraints_valid_for_non_ca=false csr=@no-basic-constraints-extension.csr
$ bao write pki/root/sign-intermediate csr=@no-basic-constraints-extension.csr ## note: defaults to false, equivalent to above call!

again, the leaf will be issued without the extension, and no warning will occur because the CSR did not contain the extension.

AFAIK, this is exactly what your PR is doing. :-) Now I think you just need to write the tests.

CSRs I generated for testing

With extension asserted to false:

-----BEGIN CERTIFICATE REQUEST-----
MIIC1DCCAbwCAQAwbTEUMBIGA1UEAwwLZXhhbXBsZS5jb20xFDASBgNVBAoMC0V4
YW1wbGUgQ28uMRQwEgYDVQQLDAtFbmdpbmVlcmluZzEPMA0GA1UEBwwGQm9zdG9u
MQswCQYDVQQIDAJNQTELMAkGA1UEBhMCVVMwggEiMA0GCSqGSIb3DQEBAQUAA4IB
DwAwggEKAoIBAQC4yAIJiMvcXv9fSs9hSFvrHSAIEWkM15x8tPYn1H0iDMRxo0Sa
sAinrnTtSJshG6oT0w64q+f9K3aahTOBw1S478pKVmg3z5buWvSP7vYeq8egOvxe
rxlB5lXrDiy405cmXPAC1icWKGLj4UfD/FzeayUKuyY4tGQ9XewmMZm7TIVdUbZd
aHq7ttI787Hed/vW96b+tF31G8k+WTENI6vTqDYf3/TlvWOTusD/gCydRSNMARFb
I0FAdfaMIoi5nH7sefXOn2eqZPxV/NGJ9GE7ZEIPofLEbs8iLcJKgyAYZWopRp9R
/17t17b+el5MoXkxZsmGSbPsTojUbYahUCYxAgMBAAGgIjAgBgkqhkiG9w0BCQ4x
EzARMA8GA1UdEwEB/wQFMAMCAQEwDQYJKoZIhvcNAQELBQADggEBAAH3aeLRCx9j
rgE2vWQhNO7ibzaAYnceARYulAXnnlFgDRsW2zW0fENOXFYc47OJeQT9D+Q5xF7+
9UaVNYmSIdjBnL5WEWrmMdXHSjNTlY7AvB5ftCjtvN+R+ryJeTgcmAsgqXb7jxuc
GFhMyO9VWDe3xuwQJIfr8BCZQCCHOGM+6wzKZ5bJgoWQCkvXEtOGU4bFcQ8lwBnc
Ag/daHDvjSKq+jk94dcEgFLssjCfyrQipjQXqHmLYtE837lFdc+N2O+o0J6n+P3y
ijDlzbg5OlhxDcYsOjdim51T4jGvbie9u9WNVSc5eiGC1K8SH6wzOx02WwtPfEO5
cvrUhCYB23I=
-----END CERTIFICATE REQUEST-----

With extension asserted to true:

-----BEGIN CERTIFICATE REQUEST-----
MIIC1zCCAb8CAQAwbTEUMBIGA1UEAwwLZXhhbXBsZS5jb20xFDASBgNVBAoMC0V4
YW1wbGUgQ28uMRQwEgYDVQQLDAtFbmdpbmVlcmluZzEPMA0GA1UEBwwGQm9zdG9u
MQswCQYDVQQIDAJNQTELMAkGA1UEBhMCVVMwggEiMA0GCSqGSIb3DQEBAQUAA4IB
DwAwggEKAoIBAQCue5nckSioptsI7AydbdWjKu1LNNgnvnBDqqADc4sYjIMlEdkY
Zj3bUoA0TStoaPBNtZhp0uypn5IYWSJrjo+lt6sV9kM+r64Qi7keoI2oPoqoC3xv
XFT61aDw8LxT375YmdlZQ6QhqPZEYAYbgAatlgUunTXRZsueIsFX77yEnPzOvG0A
BbedRghIuJfYdlklMPWHSZFqCxDx5xQW8cyDVVvXtqZzAU/xfrQ5995AAkp5sFVy
LqDif/3YOXM6ZYx35BsPT0UO8ND+8MRob0mV1OLMIuMUN1HvmsyIB7MV/jR0yVJf
WkN4RRIV84UQj266mvAFnuCgX2rVGhwjSNvrAgMBAAGgJTAjBgkqhkiG9w0BCQ4x
FjAUMBIGA1UdEwEB/wQIMAYBAf8CAQEwDQYJKoZIhvcNAQELBQADggEBABUzXz4p
UJBDGIKO4QX6MB11fyav9w++FuXxBbHFITjhGB2UiUdR5EnBMO5aB93eYFvt4sf4
1mAaYEHP6LIx5Gb+IoDbxCwm+4g2TE6rAPoE0lLuImKlOYoOBZuO4iAwn42XiYey
duvhYQ3VZ6NLDtp7OaCGXzIPdJ6H+tSUiTOJs1giu/BN67ZRViDnMXol8AbIzZu+
2mQv6+/jCzJO/KovECLsG+ToPQQI+Y3JnYGySZKTlKrWLnV81SCZ0eI8wZFJFbGv
i880VTUpObGsGa+aBfa7qRM3qz25A4Wj+kkUPXLyzhvH1XnNhu0fTfPS63Y1ouku
yRx8fvU/z31TPfY=
-----END CERTIFICATE REQUEST-----

Without extension:

-----BEGIN CERTIFICATE REQUEST-----
MIICsjCCAZoCAQAwbTEUMBIGA1UEAwwLZXhhbXBsZS5jb20xFDASBgNVBAoMC0V4
YW1wbGUgQ28uMRQwEgYDVQQLDAtFbmdpbmVlcmluZzEPMA0GA1UEBwwGQm9zdG9u
MQswCQYDVQQIDAJNQTELMAkGA1UEBhMCVVMwggEiMA0GCSqGSIb3DQEBAQUAA4IB
DwAwggEKAoIBAQDkChvQxLGHA1ren2oT/AWjy3H4BrT1899v7mhldFJAizuogQ5s
tjqaHlDg6+LDEvhIq/L9OVXKy5l3Ws4JAfUzhC0WFiSD1Ws4ZVx+BRA0CoppnDIk
Fi905NEG/+el/Gi0PkpQjUSinCcpnBEps+e+H4fsVlan0EXSTKoiCUqWFOfHbjgo
IysKWWgFVq2v06TRxNQaii9dY13GA8TndaTr27v334vLmTjiso3OUUbkL0dzC6O0
bDhwc6i7D7z7YwQx2GcGxLMEWL+vecvn4ldm82Ta6+d/hkQL3sNF5h1VjhtGS39S
Eb8kfRlHbZngDA0RTjNf9uHydsNN02nCdBERAgMBAAGgADANBgkqhkiG9w0BAQsF
AAOCAQEAyryg17suOsTOdBEq/N0iZAiVUzrs8akVd18eje6BPKSgLDaP0rSqZ8Bt
cqG0txDF2i5Us9GYYnn21tDf0NwLqcF4dYAyxYdA3dKgOtJ2jLPj2BmARUaaOFK2
txIkO1pnvnsxeOjU2sKuxUvi1RwQlQS7o/ZcFmQxRU9MZWeAX4nTwxReH9JA5cWu
/TW5Y89fy21wKjeC9C9nsoIc9+Xzlhw9t5cV9Dv3aGogu2XI2Dqz5YZ5VXzSGuaT
9yL4B8gEG08+smJ5CegV2gOr66jJhLNEXG2CSIBcEMk4awax//FupER1dyxQRS2C
MHPnnpXWoAK2notHBI7tVwzEO5Th3Q==
-----END CERTIFICATE REQUEST-----

Generated via:

openssl req  -addext basicConstraints=critical,CA:TRUE,pathlen:1     -outform pem -newkey rsa:2048 -keyout privkey.pem -new      -subj "${SUBJECT}"  -nodes   -set_serial "${SERIAL}"

&co.

[DanGhita]

@cipherboy Thank you for having taken the time to explain the expected behavior from the public API point of view, I really appreciate, it is much easier to understand the "end-user" expected input/result.

However, I still have some doubts: the first one is related to the openssl command that you have used for generating the CSR.
I was expecting to test with certificates having the CA set to FALSE and not to TRUE : as explained in the documentation, sign-verbatim can sign only leaf certificates (not root, not intermediate). So I would expect to sign certificates with CA=FALSE (but probably, this is not very important for the current issue).

Secondly (and my main concern) is the (expected) behavior that you describe in 2(a):

$ bao write pki/sign-verbatim csr=@with-basic-constraints-extension.csr ## note: defaults to false, equivalent to above call!

for which you are expecting "this will issue a leaf certificate without the extension, but with a warning saying that the CSR contained an extension we ignored".

This is strange, because, if I'm not wrong, this is already the original behavior that Vault always had and that we wanted to fix through the current PR !

I will test your scenarios against a "regular" Vault (without my changes from the current PR) in order to verify which scenario does not fulfill the behavior that you have described in your last post.

[cipherboy]

Hi @DanGhita,

You can test with both TRUE and FALSE and see that OpenBao ignores the value of the IsCA value and always sets it to false on issuance. :-) Command was given for example only, I included both TRUE and FALSE CSRs in my snippet above. Because we know we're issuing a leaf, we overwrite the value on the extension. Perhaps better behavior would be to err out, but we cannot now...

Regarding your new query...

We don't want to change the behavior for existing callers of the API :-) If we were to default basic_constraints_valid_for_non_ca=true (when it is ommitted from the request), this would be a potentially breaking change for existing callers because an unexpected and unnecessarily redundant extension is added where none was previously. Thus the second invocation under 2a must be preserved.

We only want to change this behavior when the caller opts in by specifying the parameter.

The behavior will not pass with upstream presently, because it doesn't implement 1 -- the parameter is unknown by the endpoint. Scenario 2 should pass completely, with a warning about the parameter not existing and thus defaulting to false on the backend system.

HTH,

• A

[DanGhita]

Hi @cipherboy,

I think we are aligned now and I would summarize the goal of the issue/PR as following:

Add a new parameter (basic_constraints_valid_for_non_ca) to the pki/sign-verbatim endpoint in order to specify whether the extension X509v3 Basic Constraints has to be added or not to the issued certificate.
For API backward compatibility, basic_constraints_valid_for_non_ca must default to FALSE.

Note: the parameter basic_constraints_valid_for_non_ca already exists for other PKI endpoints (/pki/roles, for example).

Concerning the current status of the PR: the code evolution seems to be already OK.
TODO: add tests and update documentation for explaining the usage of basic_constraints_valid_for_non_ca in the context of the pki/sign-verbatim endpoint.