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

Correctly validate leaf certificate trust #173

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

cipherboy
Copy link
Member

HashiCorp Vault v1.14.10 reports fixing the following security vulnerability:

auth/cert: compare public keys of trusted non-CA certificates with
incoming client certificates to prevent trusting certs with the same
serial number but not the same public/private key. [GH-25649]

From this, we can deduce that there is an issue with validating the authenticity of leaf certificates. Luckily there is only one code path so the fix location is obvious.

Note that the issue lies in how the check is performed: a client may present an arbitrary certificate from an arbitrary CA (even a self-signed one if it contains an AuthorityKeyId extension), which will be accepted, assuming it matches the AKID and Serial Number of a present, explicitly trusted leaf certificate.

In particular, while the authenticity of the TLS connection is verified (insofar as the presented certificate matches the TLS channel via valid signature), the authorization check is malformed as it does not correctly tie the channel's connection key to the key contained in the certificate.

This check is sufficient to differentiate certificates from different CAs, say, for the purpose of chain building, but is not strong enough for authentication and authorization.

In the past, several CAs (such as the production grade Dogtag PKI or a manual OpenSSL CA) have defaulted to sequential serial numbers, though, AKIDs would likely still be unique unless key material was reused between different CAs (typically unlikely unless an intermediate CA was re-issued with the same key material but longer expiration).

However, most CAs correspond to the CA/BF's guidelines which require at least 20 bits of entropy, and thus would be less likely to run into this organically, even with reused CA public keys.

See also: https://github.com/hashicorp/vault/releases/tag/v1.14.10
See also: https://cabforum.org/working-groups/server/baseline-requirements/documents/

Resolves: #172

Target Release

Alpha

@cipherboy
Copy link
Member Author

n.b., in my mind, comparing only the public key values, while perhaps sufficient to fix this vulnerability, isn't much better -- general when browsers and other similar tools want to trust a leaf certificate, they explicitly trust just that leaf, and don't trust equivalent certificates (with other Subjects, SANs, or other values). While it would be a violation of CA/BF and IETF RFC 5280 to re-issue a cert with the same serial number, AKID, and public key under the same CA, under a different root with same ICA key material (but "acting" as a different CA), a leaf could be reissued with those same attributes but differing in other aspects.

Thus this aligns us more with browser's trust pages (trust this bad SSL connection one time).

We likely want to add warnings going forward to this as well, suggesting against the use of trusted leaf certificates: users should be using ICAs especially for cert auth. My 2c.

@cipherboy cipherboy force-pushed the fix-cert-auth branch 2 times, most recently from ce29f0b to eb5ac20 Compare March 5, 2024 13:30
@naphelps naphelps self-requested a review March 5, 2024 15:13
HashiCorp Vault v1.14.10 reports fixing the following security
vulnerability:

> auth/cert: compare public keys of trusted non-CA certificates with
> incoming client certificates to prevent trusting certs with the same
> serial number but not the same public/private key. [GH-25649]

From this, we can deduce that there is an issue with validating the
authenticity of leaf certificates. Luckily there is only one code path
so the fix location is obvious.

Note that the issue lies in how the check is performed: a client may
present an arbitrary certificate from an arbitrary CA (even a
self-signed one if it contains an AuthorityKeyId extension), which
will be accepted, assuming it matches the AKID and Serial Number of
a present, explicitly trusted leaf certificate.

In particular, while the authenticity of the TLS connection is verified
(insofar as the presented certificate matches the TLS channel via valid
signature), the authorization check is malformed as it does not
correctly tie the channel's connection key to the key contained in the
certificate.

This check is sufficient to differentiate certificates from different
CAs, say, for the purpose of chain building, but is not strong enough
for authentication and authorization.

In the past, several CAs (such as the production grade Dogtag PKI or
a manual OpenSSL CA) have defaulted to sequential serial numbers,
though, AKIDs would likely still be unique unless key material was
reused between different CAs (typically unlikely unless an intermediate
CA was re-issued with the same key material but longer expiration).

However, most CAs correspond to the CA/BF's guidelines which require
at least 20 bits of entropy, and thus would be less likely to run into
this organically, even with reused CA public keys.

See also: https://github.com/hashicorp/vault/releases/tag/v1.14.10
See also: https://discuss.hashicorp.com/t/hcsec-2024-05-vault-cert-auth-method-did-not-correctly-validate-non-ca-certificates/63382
See also: https://cabforum.org/working-groups/server/baseline-requirements/documents/

Resolves: openbao#172

Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com>
@naphelps naphelps merged commit 3300aa1 into openbao:main Mar 5, 2024
4 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants