openssl / openssl

TLS/SSL and crypto library
https://www.openssl.org
Apache License 2.0
25.76k stars 10.11k forks source link

X509: inconsistent treatment of trust anchors on strict chain checking #20256

Open DDvO opened 1 year ago

DDvO commented 1 year ago

In #12478 I added various certificate well-formedness checks according to requirements in RFC 5280 section 4, which are enabled only if X509_V_FLAG_X509_STRICT is set.

It is important to note here that RFC 6818 section 2 points out that RFC 5280 is not applicable to certain certificates:

Consistent with Section 3.4.61 of X.509 (11/2008) [X.509], we note that use of self-issued certificates and self-signed certificates issued by entities other than CAs are outside the scope of this specification. Thus, for example, a web server or client might generate a self-signed certificate to identify itself. These certificates and how a relying party uses them to authenticate asserted identities are both outside the scope of RFC 5280.

In #12478 I tried to implement this exception in check_chain_extensions() of x509_vfy.c line 524 as directly as possible, namely by the following guard, where i==0 indicates the begin of a chain, i.e., the position of the cert to be verified, which usually is an end-entity cert:

!(i == 0 && (x->ex_flags & EXFLAG_CA) == 0
         && (x->ex_flags & EXFLAG_SI) != 0)

Yet I was advised in https://github.com/openssl/openssl/pull/12478#discussion_r484075271 to change this to

num > 1

which means that the chain (including the trust anchor cert) has at least two elements, i.e., the cert to be verified is not at the same time the trust anchor at the end of the chain, or in other words: the certificate to validate is not directly trusted.

Among others, this has the advantage that it avoids the hardly implementable condition "issued by entities other than CAs". Yet two drawbacks are:

I believe it would be more clean not to perform these strict checks on trust anchors regardless in which chain they are used, i.e., to replace the above condition by

i < num - 1

meaning that it is done for all certs i in the chain except for the final one at position num - 1, representing the trust anchor.

bernd-edlinger commented 1 year ago

I wonder how that condition works with partial chains... Does this mean, that the strict checks are not performed, when only an intermediate CA is installed in my PKI folder, and it will break as soon as the root CA is installed as well?

DDvO commented 1 year ago

I wonder how that condition works with partial chains... Does this mean, that the strict checks are not performed, when only an intermediate CA is installed in my PKI folder, and it will break as soon as the root CA is installed as well?

Good question - right, if that intermediate CA cert is not well-formed and is used as a trust anchor, it would not be strictly checked, while it would get checked if you use a different trust anchor and that intermediate CA cert is not trusted but part of the (remaining) chain.

To me, this behavior ist closest to the intention of RFC 6818 section 2. OTOH, it may be seen as an inconsistency in checking certs.

@vdukhovni what do you think?

nhorman commented 4 months ago

@vdukhovni can you comment here please?

nhorman commented 4 months ago

@vdukhovni ping again

vdukhovni commented 2 months ago

How important are these exclusions to strict checking in practice? Does it make sense for applications that support partial chains or that validate bare EE certificates to also request strict checking?

Or, is it perhaps reasonable (and simplest) to document that strict checks don't try to make ad hoc exceptions, and that applications that want to make exceptions, should avoid requesting strict checks?

The num > 1 and i < num - 1 conditions are, e.g., a somewhat fragile in the case of DANE with 2 1 0 TLSA records that carry the trust anchor in the form of a bare public key, in which case the deepest certificate in the chain is not in fact a trust-anchor, it is merely signed by a bare public key, rather than a public key inside a certificate. And yet, the rules for the EE and intermediate certs should probably be the same as they would be in the case of a TA that was in certificate form.

Basically, I am sceptical that the library can reliably determine whether the context is or isn't outside the boundaries of RFC5280, and perhaps we should enforce strict checking whenever requested without ad hoc exceptions, leaving the decision to avoid strict checks to the application.

Therefore, I'm inclined to read https://datatracker.ietf.org/doc/html/rfc6818#section-2 rather narrowly, as applying at most to just EE-only chains, subject also to there not being a bare-public-key DANE-TA. So, if we're to do anything in the library, it should be num > 1, not i < num - 1, but also taking DANE 2 1 0 into account.

My instinct is to leave this to the application, it is simple, and intuitive.