-
-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
WIP: Fix keyCertSign checks #7918
base: master
Are you sure you want to change the base?
Conversation
Trying to check CA usage in x509v3_cache_extensions wasn't a good idea. Fixes openssl#1418 This reverts commit f51e5ed.
RFC 5280 says this in 4.2.1.3 "Key Usage": The keyCertSign bit is asserted when the subject public key is used for verifying signatures on public key certificates. If the keyCertSign bit is asserted, then the cA bit in the basic constraints extension (Section 4.2.1.9) MUST also be asserted. check_ca did check these relevant combinations: 1. keyCertSign bit set in keyUsage, no check of basicConstraints. 2. cA bit set in basicConstraints, no check of keyUsage. 3. basicConstraints not present => keyCertSign bit set in keyUsage. Now we change 1 to: 1. keyCertSign bit set in keyUsage => cA bit set in basicConstraints. Fixes openssl#1418
Should this go in 1.1.0 as well? |
HELP NEEDED There is a test_verify failure in Travis (and I suppose Appveyor as well):
Looking more closely at this,
This certainly looks like a self-issued certificate, doesn't it? There's no AKID saying anything else, so the assumption is that it is self-signed as well. |
The problem I had here gets resolved with #7922 |
The above cert is definitely self-issued since its subject and issuer are identical. Yet it turns out that the cert is not self-signed (or at least OpenSSL fails to verify its signature, which needs to be triggered explicitly):
yields
For more information about this cert, see https://tools.ietf.org/html/rfc8410#section-10.2. |
I've just checked: the signature error is thrown at this point in a_verify.c:
because To conclude, the above test cert is self-issued but not self-signed.
even more strange than commented so far in #1418 (comment) . |
Actually, if you look at https://tools.ietf.org/html/rfc8410#section-10.2, you'll note that there's a public key and a private key there as well. I assume that those were used to sign the certificate. Now, ee-ed25519.pem (which is taken directly from that RFC) is constructed a bit weirdly. For all intents and purposes, it's self issued for sure, and as far as RFC 5280 goes, there should be an AKID in it indicating that it's signed by another key. So bottom line, there isn't much information telling us that ee-ed2219.pem isn't a self-signed certificate, save for doing a verification against the included public key... I'm not sure we should do that... |
You know what, I do believe that ee-ed25519.pem is incorrect. From https://tools.ietf.org/html/rfc8410#section-10.2:
Now, the certificate:
Notice anything weird with that SKID? I've always wondered why it was there, for what purpose... but y'know what? I think "they" screwed up, the SKID should really be an AKID with that key id. That would make a lot more sense. |
(also, it must be late at night, but I seem to get a different key ID for that public key than they do... so if someone else could verify that calculation, that would be great) |
you could write to the author and start a discussion. it might be that an errata will have to be filed :) |
@levitte your self-signed certificate looks invalid,
|
@bernd-edlinger, the algorithm X25519 given for the key type is fine because the cert is meant to be for an X25519 key agreement key - see https://tools.ietf.org/html/rfc8410#section-10.2 . @levitte, I confirm your analysis (and need to withdraw one sentence I wrote above concerning the AKID): |
I tried to verify the cert signature with the mentioned public key:
but got:
At least, the encoding of the public key is correct, as confirmed by:
which yields:
Does anyone know/have a signature verifier that can handle ED25519 to verify that the cert is signed using the private key corresponding to |
I get the same result when trying the private key as input for the verification:
yields
|
For SIKD and AKID there is not really right or wrong. As written in RFC 5280 sections 4.2.1.1. and 4.2.1.2, they can be basically anything unique, while the recommendation is to use the SHA-1 hash of the value of the BIT STRING representation of the public key (excluding the tag, length, and number of unused bits). |
the only command I can think of that is able to check the ED25519 signature of an X509 is: |
I still fail to see how the signature can be verified, the certificate has subject=issuer, Everything would instantly make sense, if the key agreement was signed by a CA. |
One needs to be careful here: self-issued does not necessarily imply self-signed! I myself learned this neat distinction just this week. See RFC 5280 section 3.2:
The two different terms are used very deliberately throughout this RFC. In our case here the EE cert happens to have the same subject as the issuer (so is self-issued), and its is signed by the same issuer as the root cert, but only the root cert is self-signed. Note that the same cert signer (same identity) may have multiple key pairs, for potentially different purposes. This is why the presence of the Authority Key Identifier (AKID) is important here to differentiate the signing keys. The cert, as given in https://tools.ietf.org/html/rfc8410#section-10.2 fails to include it. This is the actual reason why the test case
meanwhile (correctly) fails: it should not fail because the cert does not allow for key usage keyCertSign but because the chain to the trusted root cannot be established due to the missing AKID, which misleads the verification procedure in assuming that the cert is self-signed. |
For the given cert, the intended key usage is definitely just key agreement. The private key belonging to the certified public key has not been used to sign this cert (and does not need to be used for signing). This cert happens to be signed by the same issuer, but using a different key: presumably the key certified in the CA cert |
BTW, it looks to me like OpenSSL so far simply does not (fully) support verifying ED25519 signatures. |
I am pretty sure 1.1.1 does support verifying PureEDDSA, because I have implemented
But maybe there is no command which verifies EDDSA |
The ee certificate can't be self-signed because the public key algorihm X25519 does |
So, as a matter of fact we have now a lot of self signed certificates, that are not being used as CA |
@@ -471,8 +463,7 @@ static void x509v3_cache_extensions(X509 *x) | |||
if (!X509_NAME_cmp(X509_get_subject_name(x), X509_get_issuer_name(x))) { | |||
x->ex_flags |= EXFLAG_SI; | |||
/* If SKID matches AKID also indicate self signed */ | |||
if (X509_check_akid(x, x->akid) == X509_V_OK && | |||
!ku_reject(x, KU_KEY_CERT_SIGN)) | |||
if (X509_check_akid(x, x->akid) == X509_V_OK) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about this:
--- a/crypto/x509v3/v3_purp.c
+++ b/crypto/x509v3/v3_purp.c
@@ -463,8 +463,16 @@ static void x509v3_cache_extensions(X509 *x)
if (!X509_NAME_cmp(X509_get_subject_name(x), X509_get_issuer_name(x))) {
x->ex_flags |= EXFLAG_SI;
/* If SKID matches AKID also indicate self signed */
- if (X509_check_akid(x, x->akid) == X509_V_OK)
- x->ex_flags |= EXFLAG_SS;
+ if (X509_check_akid(x, x->akid) == X509_V_OK) {
+ X509_PUBKEY *xpkey = X509_get_X509_PUBKEY(x);
+ if (xpkey != NULL) {
+ EVP_PKEY *pkey = X509_PUBKEY_get0(xpkey);
+ if (pkey != NULL
+ && EVP_PKEY_id(pkey) != EVP_PKEY_X25519
+ && EVP_PKEY_id(pkey) != EVP_PKEY_X448)
+ x->ex_flags |= EXFLAG_SS;
+ }
+ }
}
x->altname = X509_get_ext_d2i(x, NID_subject_alt_name, NULL, NULL);
x->nc = X509_get_ext_d2i(x, NID_name_constraints, &i, NULL);
It can't be self-signed when we are unable to verify the signature using the public key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding references to specific crypto algorithms would add quite some clutter to the verification functions, which are already very hard to follow. If done for these two algorithms, then consequently putting in many more such exceptions etc. would be in order.
The RFC 5280 does not do this, and I'd recommend not to do this in the OpenSSL implementation, either.
Nice.
I've just tried with the latest 3.0.0-dev, for instance like this:
and it works :) @levitte, with this very fresh OpenSSL version one can confirm that (the private key related to) the root cert has been used to sign the weird ee cert:
|
Yes.
RFC 5280 is inconsistent in this respect: it requires keyCertSign for any(!) key used for signing certs, but on the other hand for self-signed EE certs the presence of keyCertSign is not checked according to the algorithm provided in https://tools.ietf.org/html/rfc5280#section-6.1. The most reasonable resolution is to follow the stated algorithm (and not require keyCertSign for such certs).
For good (security) reason, such end-entity certs should indeed have CA=false.
Yes, when they allow keyCertSign this is (as mentioned also in #1418) not conforming with RFC 5280, which (correctly) demands CA=true for all certs having the keyCertSign set. |
So will this be a breaking change, since the current state of affairs forced us to set keyCertSign in the |
When the problem handled in issue #7918 came up for one of my customers some two weeks ago, after finding out what the real reason was behind a rather misleading error: " Yet after some discussion with PKI experts we came to the conclusion that its cleaner - and much safer - not to do it. We use a workaround:
until #7918 is (fully) fixed and the fix has become available in our developments. |
Okay, the good news is, I checked the current PR against our software (an OPC/UA SDK) However, we had detected this issue years before I joined this project. Previously all self-signed certificates were correctly using CA=FALSE, and keyCertSign=FALSE, |
Well, as mentioned certificates with CA=FALSE and keyCertSign set do not comply with RFC 5280. I should have mentioned that we use the |
I am only saying, if a bug like that has been around for four years, you cannot simply |
I had a close look again at that strange ED25519 self-issued example cert with X25519 key agreement public key: https://tools.ietf.org/html/draft-ietf-curdle-pkix-04#section-10.2 This makes |
The incorrect handling of the weird ED25519 example cert is fixed in #10587. |
Are you still interested in having this change included in OpenSSL? If so please rebase the PR to resolve conflicts. Marking as inactive to be closed at the end of 3.4 dev barring further input. |
Trying to check CA usage in x509v3_cache_extensions wasn't a good idea.
However, the keyCertSign check in check_ca wasn't quite right either
RFC 5280 says this in 4.2.1.3 "Key Usage":
check_ca did check these relevant combinations:
Now we change 1 to:
Fixes #1418