[openssl] master update
dev at ddvo.net
dev at ddvo.net
Thu Jul 16 13:49:42 UTC 2020
The branch master has been updated
via 0b670a2101c6cdcc3f2a4ed168f75243fe082a2b (commit)
via 1337a3a998b7dacd55e31c21bb9c647099e63e86 (commit)
from 318565b73374a3821dbd00d1d0e598e957fc45c9 (commit)
- Log -----------------------------------------------------------------
commit 0b670a2101c6cdcc3f2a4ed168f75243fe082a2b
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date: Fri Jul 3 21:19:55 2020 +0200
x509_vfy.c: Improve key usage checks in internal_verify() of cert chains
If a presumably self-signed cert is last in chain we verify its signature
only if X509_V_FLAG_CHECK_SS_SIGNATURE is set. Upon this request we do the
signature verification, but not in case it is a (non-conforming) self-issued
CA certificate with a key usage extension that does not include keyCertSign.
Make clear when we must verify the signature of a certificate
and when we must adhere to key usage restrictions of the 'issuing' cert.
Add some comments for making internal_verify() easier to understand.
Update the documentation of X509_V_FLAG_CHECK_SS_SIGNATURE accordingly.
Reviewed-by: Tomas Mraz <tmraz at fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/12375)
commit 1337a3a998b7dacd55e31c21bb9c647099e63e86
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date: Mon Jul 13 17:13:48 2020 +0200
Constify X509_check_akid and prefer using X509_get0_serialNumber over X509_get_serialNumber
Reviewed-by: Tomas Mraz <tmraz at fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/12375)
-----------------------------------------------------------------------
Summary of changes:
apps/ca.c | 4 +--
apps/x509.c | 2 +-
crypto/cmp/cmp_msg.c | 4 +--
crypto/cms/cms_lib.c | 4 +--
crypto/ess/ess_lib.c | 4 +--
crypto/pkcs7/pk7_doit.c | 2 +-
crypto/pkcs7/pk7_lib.c | 4 +--
crypto/x509/t_x509.c | 3 +-
crypto/x509/v3_akey.c | 2 +-
crypto/x509/v3_purp.c | 6 ++--
crypto/x509/x509_vfy.c | 54 +++++++++++++++++++++++---------
crypto/x509/x_crl.c | 2 +-
doc/man1/openssl.pod | 9 +++---
doc/man3/X509_VERIFY_PARAM_set_flags.pod | 14 +++++----
include/openssl/x509v3.h | 2 +-
15 files changed, 72 insertions(+), 44 deletions(-)
diff --git a/apps/ca.c b/apps/ca.c
index d91b39c91c..d0309ae15c 100644
--- a/apps/ca.c
+++ b/apps/ca.c
@@ -1049,7 +1049,7 @@ end_of_options:
for (i = 0; i < sk_X509_num(cert_sk); i++) {
BIO *Cout = NULL;
X509 *xi = sk_X509_value(cert_sk, i);
- ASN1_INTEGER *serialNumber = X509_get_serialNumber(xi);
+ const ASN1_INTEGER *serialNumber = X509_get0_serialNumber(xi);
const unsigned char *psn = ASN1_STRING_get0_data(serialNumber);
const int snl = ASN1_STRING_length(serialNumber);
const int filen_len = 2 * (snl > 0 ? snl : 1) + sizeof(".pem");
@@ -2113,7 +2113,7 @@ static int do_revoke(X509 *x509, CA_DB *db, REVINFO_TYPE rev_type,
for (i = 0; i < DB_NUMBER; i++)
row[i] = NULL;
row[DB_name] = X509_NAME_oneline(X509_get_subject_name(x509), NULL, 0);
- bn = ASN1_INTEGER_to_BN(X509_get_serialNumber(x509), NULL);
+ bn = ASN1_INTEGER_to_BN(X509_get0_serialNumber(x509), NULL);
if (!bn)
goto end;
if (BN_is_zero(bn))
diff --git a/apps/x509.c b/apps/x509.c
index c64c7d2811..bf168b7863 100644
--- a/apps/x509.c
+++ b/apps/x509.c
@@ -693,7 +693,7 @@ int x509_main(int argc, char **argv)
X509_get_subject_name(x), get_nameopt());
} else if (serial == i) {
BIO_printf(out, "serial=");
- i2a_ASN1_INTEGER(out, X509_get_serialNumber(x));
+ i2a_ASN1_INTEGER(out, X509_get0_serialNumber(x));
BIO_printf(out, "\n");
} else if (next_serial == i) {
ASN1_INTEGER *ser = X509_get_serialNumber(x);
diff --git a/crypto/cmp/cmp_msg.c b/crypto/cmp/cmp_msg.c
index bbc3e9157e..c5a9dbccf8 100644
--- a/crypto/cmp/cmp_msg.c
+++ b/crypto/cmp/cmp_msg.c
@@ -298,7 +298,7 @@ static OSSL_CRMF_MSG *crm_new(OSSL_CMP_CTX *ctx, int bodytype, int rid)
if (bodytype == OSSL_CMP_PKIBODY_KUR) {
OSSL_CRMF_CERTID *cid =
OSSL_CRMF_CERTID_gen(X509_get_issuer_name(refcert),
- X509_get_serialNumber(refcert));
+ X509_get0_serialNumber(refcert));
int ret;
if (cid == NULL)
@@ -469,7 +469,7 @@ OSSL_CMP_MSG *ossl_cmp_rr_new(OSSL_CMP_CTX *ctx)
NULL /* pubkey would be redundant */,
NULL /* subject would be redundant */,
X509_get_issuer_name(ctx->oldCert),
- X509_get_serialNumber(ctx->oldCert)))
+ X509_get0_serialNumber(ctx->oldCert)))
goto err;
/* revocation reason code is optional */
diff --git a/crypto/cms/cms_lib.c b/crypto/cms/cms_lib.c
index 89dfc15081..67f4fbb4ea 100644
--- a/crypto/cms/cms_lib.c
+++ b/crypto/cms/cms_lib.c
@@ -553,7 +553,7 @@ int cms_ias_cert_cmp(CMS_IssuerAndSerialNumber *ias, X509 *cert)
ret = X509_NAME_cmp(ias->issuer, X509_get_issuer_name(cert));
if (ret)
return ret;
- return ASN1_INTEGER_cmp(ias->serialNumber, X509_get_serialNumber(cert));
+ return ASN1_INTEGER_cmp(ias->serialNumber, X509_get0_serialNumber(cert));
}
int cms_keyid_cert_cmp(ASN1_OCTET_STRING *keyid, X509 *cert)
@@ -573,7 +573,7 @@ int cms_set1_ias(CMS_IssuerAndSerialNumber **pias, X509 *cert)
goto err;
if (!X509_NAME_set(&ias->issuer, X509_get_issuer_name(cert)))
goto err;
- if (!ASN1_STRING_copy(ias->serialNumber, X509_get_serialNumber(cert)))
+ if (!ASN1_STRING_copy(ias->serialNumber, X509_get0_serialNumber(cert)))
goto err;
M_ASN1_free_of(*pias, CMS_IssuerAndSerialNumber);
*pias = ias;
diff --git a/crypto/ess/ess_lib.c b/crypto/ess/ess_lib.c
index 3f418235ad..4a7a2632ba 100644
--- a/crypto/ess/ess_lib.c
+++ b/crypto/ess/ess_lib.c
@@ -89,7 +89,7 @@ static ESS_CERT_ID *ESS_CERT_ID_new_init(X509 *cert, int issuer_needed)
name = NULL; /* Ownership is lost. */
ASN1_INTEGER_free(cid->issuer_serial->serial);
if ((cid->issuer_serial->serial =
- ASN1_INTEGER_dup(X509_get_serialNumber(cert))) == NULL)
+ ASN1_INTEGER_dup(X509_get0_serialNumber(cert))) == NULL)
goto err;
return cid;
@@ -183,7 +183,7 @@ static ESS_CERT_ID_V2 *ESS_CERT_ID_V2_new_init(const EVP_MD *hash_alg,
goto err;
name = NULL; /* Ownership is lost. */
ASN1_INTEGER_free(cid->issuer_serial->serial);
- cid->issuer_serial->serial = ASN1_INTEGER_dup(X509_get_serialNumber(cert));
+ cid->issuer_serial->serial = ASN1_INTEGER_dup(X509_get0_serialNumber(cert));
if (cid->issuer_serial->serial == NULL)
goto err;
diff --git a/crypto/pkcs7/pk7_doit.c b/crypto/pkcs7/pk7_doit.c
index 718b6f3899..b815a4a77b 100644
--- a/crypto/pkcs7/pk7_doit.c
+++ b/crypto/pkcs7/pk7_doit.c
@@ -354,7 +354,7 @@ static int pkcs7_cmp_ri(PKCS7_RECIP_INFO *ri, X509 *pcert)
X509_get_issuer_name(pcert));
if (ret)
return ret;
- return ASN1_INTEGER_cmp(X509_get_serialNumber(pcert),
+ return ASN1_INTEGER_cmp(X509_get0_serialNumber(pcert),
ri->issuer_and_serial->serial);
}
diff --git a/crypto/pkcs7/pk7_lib.c b/crypto/pkcs7/pk7_lib.c
index 32e2ffc820..cb8c67b65a 100644
--- a/crypto/pkcs7/pk7_lib.c
+++ b/crypto/pkcs7/pk7_lib.c
@@ -324,7 +324,7 @@ int PKCS7_SIGNER_INFO_set(PKCS7_SIGNER_INFO *p7i, X509 *x509, EVP_PKEY *pkey,
*/
ASN1_INTEGER_free(p7i->issuer_and_serial->serial);
if (!(p7i->issuer_and_serial->serial =
- ASN1_INTEGER_dup(X509_get_serialNumber(x509))))
+ ASN1_INTEGER_dup(X509_get0_serialNumber(x509))))
goto err;
/* lets keep the pkey around for a while */
@@ -477,7 +477,7 @@ int PKCS7_RECIP_INFO_set(PKCS7_RECIP_INFO *p7i, X509 *x509)
ASN1_INTEGER_free(p7i->issuer_and_serial->serial);
if (!(p7i->issuer_and_serial->serial =
- ASN1_INTEGER_dup(X509_get_serialNumber(x509))))
+ ASN1_INTEGER_dup(X509_get0_serialNumber(x509))))
return 0;
pkey = X509_get0_pubkey(x509);
diff --git a/crypto/x509/t_x509.c b/crypto/x509/t_x509.c
index 75d688c50e..199f88857b 100644
--- a/crypto/x509/t_x509.c
+++ b/crypto/x509/t_x509.c
@@ -55,7 +55,6 @@ int X509_print_ex(BIO *bp, X509 *x, unsigned long nmflags,
int ret = 0, i;
char *m = NULL, mlch = ' ';
int nmindent = 0;
- ASN1_INTEGER *bs;
EVP_PKEY *pkey = NULL;
const char *neg;
@@ -84,11 +83,11 @@ int X509_print_ex(BIO *bp, X509 *x, unsigned long nmflags,
}
}
if (!(cflag & X509_FLAG_NO_SERIAL)) {
+ const ASN1_INTEGER *bs = X509_get0_serialNumber(x);
if (BIO_write(bp, " Serial Number:", 22) <= 0)
goto err;
- bs = X509_get_serialNumber(x);
if (bs->length <= (int)sizeof(long)) {
ERR_set_mark();
l = ASN1_INTEGER_get(bs);
diff --git a/crypto/x509/v3_akey.c b/crypto/x509/v3_akey.c
index a40963d9f0..65019a5a12 100644
--- a/crypto/x509/v3_akey.c
+++ b/crypto/x509/v3_akey.c
@@ -132,7 +132,7 @@ static AUTHORITY_KEYID *v2i_AUTHORITY_KEYID(X509V3_EXT_METHOD *method,
if ((issuer && !ikeyid) || (issuer == 2)) {
isname = X509_NAME_dup(X509_get_issuer_name(cert));
- serial = ASN1_INTEGER_dup(X509_get_serialNumber(cert));
+ serial = ASN1_INTEGER_dup(X509_get0_serialNumber(cert));
if (!isname || !serial) {
X509V3err(X509V3_F_V2I_AUTHORITY_KEYID,
X509V3_R_UNABLE_TO_GET_ISSUER_DETAILS);
diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c
index 0fcf53a5ea..4a2b549199 100644
--- a/crypto/x509/v3_purp.c
+++ b/crypto/x509/v3_purp.c
@@ -533,6 +533,7 @@ int X509v3_cache_extensions(X509 *x, OPENSSL_CTX *libctx, const char *propq)
/* .. and the signature alg matches the PUBKEY alg: */
&& check_sig_alg_match(X509_get0_pubkey(x), x) == X509_V_OK)
x->ex_flags |= EXFLAG_SS; /* indicate self-signed */
+ /* This is very related to x509_likely_issued(x, x) == X509_V_OK */
}
/* Handle subject alternative names and various other extensions */
@@ -865,6 +866,7 @@ int x509_likely_issued(X509 *issuer, X509 *subject,
X509_get_issuer_name(subject)) != 0)
return X509_V_ERR_SUBJECT_ISSUER_MISMATCH;
+ /* set issuer->skid and subject->akid */
if (!X509v3_cache_extensions(issuer, libctx, propq)
|| !X509v3_cache_extensions(subject, libctx, propq))
return X509_V_ERR_UNSPECIFIED;
@@ -899,7 +901,7 @@ int X509_check_issued(X509 *issuer, X509 *subject)
return x509_check_issued_int(issuer, subject, NULL, NULL);
}
-int X509_check_akid(X509 *issuer, AUTHORITY_KEYID *akid)
+int X509_check_akid(const X509 *issuer, const AUTHORITY_KEYID *akid)
{
if (akid == NULL)
return X509_V_OK;
@@ -910,7 +912,7 @@ int X509_check_akid(X509 *issuer, AUTHORITY_KEYID *akid)
return X509_V_ERR_AKID_SKID_MISMATCH;
/* Check serial number */
if (akid->serial &&
- ASN1_INTEGER_cmp(X509_get_serialNumber(issuer), akid->serial))
+ ASN1_INTEGER_cmp(X509_get0_serialNumber(issuer), akid->serial))
return X509_V_ERR_AKID_ISSUER_SERIAL_MISMATCH;
/* Check issuer name */
if (akid->issuer) {
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 1f17c71bc1..3bd23d131c 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -1746,6 +1746,7 @@ int x509_check_cert_time(X509_STORE_CTX *ctx, X509 *x, int depth)
return 1;
}
+/* verify the issuer signatures and cert times of ctx->chain */
static int internal_verify(X509_STORE_CTX *ctx)
{
int n = sk_X509_num(ctx->chain) - 1;
@@ -1760,15 +1761,15 @@ static int internal_verify(X509_STORE_CTX *ctx)
if (ctx->bare_ta_signed) {
xs = xi;
xi = NULL;
- goto check_cert;
+ goto check_cert_time;
}
- if (ctx->check_issued(ctx, xi, xi)) /* the last cert appears self-signed */
- xs = xi;
+ if (ctx->check_issued(ctx, xi, xi))
+ xs = xi; /* the typical case: last cert in the chain is self-issued */
else {
if (ctx->param->flags & X509_V_FLAG_PARTIAL_CHAIN) {
xs = xi;
- goto check_cert;
+ goto check_cert_time;
}
if (n <= 0)
return verify_cb_cert(ctx, xi, 0,
@@ -1784,31 +1785,54 @@ static int internal_verify(X509_STORE_CTX *ctx)
*/
while (n >= 0) {
/*
+ * For each iteration of this loop:
+ * n is the subject depth
+ * xs is the subject cert, for which the signature is to be checked
+ * xi is the supposed issuer cert containing the public key to use
+ * Initially xs == xi if the last cert in the chain is self-issued.
+ *
* Skip signature check for self-signed certificates unless explicitly
* asked for because it does not add any security and just wastes time.
- * If the issuer's public key is not available or its key usage does
- * not support issuing the subject cert, report the issuer certificate
- * and its depth (rather than the depth of the subject).
*/
- if (xs != xi || (ctx->param->flags & X509_V_FLAG_CHECK_SS_SIGNATURE)) {
+ if (xs != xi || ((ctx->param->flags & X509_V_FLAG_CHECK_SS_SIGNATURE)
+ && (xi->ex_flags & EXFLAG_SS) != 0)) {
EVP_PKEY *pkey;
- int issuer_depth = n + (xi == xs ? 0 : 1);
- int ret = x509_signing_allowed(xi, xs);
+ /*
+ * If the issuer's public key is not available or its key usage
+ * does not support issuing the subject cert, report the issuer
+ * cert and its depth (rather than n, the depth of the subject).
+ */
+ int issuer_depth = n + (xs == xi ? 0 : 1);
+ /*
+ * According to https://tools.ietf.org/html/rfc5280#section-6.1.4
+ * step (n) we must check any given key usage extension in a CA cert
+ * when preparing the verification of a certificate issued by it.
+ * According to https://tools.ietf.org/html/rfc5280#section-4.2.1.3
+ * we must not verify a certifiate signature if the key usage of the
+ * CA certificate that issued the certificate prohibits signing.
+ * In case the 'issuing' certificate is the last in the chain and is
+ * not a CA certificate but a 'self-issued' end-entity cert (i.e.,
+ * xs == xi && !(xi->ex_flags & EXFLAG_CA)) RFC 5280 does not apply
+ * (see https://tools.ietf.org/html/rfc6818#section-2) and thus
+ * we are free to ignore any key usage restrictions on such certs.
+ */
+ int ret = xs == xi && (xi->ex_flags & EXFLAG_CA) == 0
+ ? X509_V_OK : x509_signing_allowed(xi, xs);
if (ret != X509_V_OK && !verify_cb_cert(ctx, xi, issuer_depth, ret))
return 0;
if ((pkey = X509_get0_pubkey(xi)) == NULL) {
- if (!verify_cb_cert(ctx, xi, issuer_depth,
- X509_V_ERR_UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY))
+ ret = X509_V_ERR_UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY;
+ if (!verify_cb_cert(ctx, xi, issuer_depth, ret))
return 0;
} else if (X509_verify_ex(xs, pkey, ctx->libctx, ctx->propq) <= 0) {
- if (!verify_cb_cert(ctx, xs, n,
- X509_V_ERR_CERT_SIGNATURE_FAILURE))
+ ret = X509_V_ERR_CERT_SIGNATURE_FAILURE;
+ if (!verify_cb_cert(ctx, xs, n, ret))
return 0;
}
}
- check_cert:
+ check_cert_time:
/* Calls verify callback as needed */
if (!x509_check_cert_time(ctx, xs, n))
return 0;
diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
index 0d3e1fedb4..1690dd8963 100644
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c
@@ -370,7 +370,7 @@ int X509_CRL_get0_by_cert(X509_CRL *crl, X509_REVOKED **ret, X509 *x)
{
if (crl->meth->crl_lookup)
return crl->meth->crl_lookup(crl, ret,
- X509_get_serialNumber(x),
+ X509_get0_serialNumber(x),
X509_get_issuer_name(x));
return 0;
}
diff --git a/doc/man1/openssl.pod b/doc/man1/openssl.pod
index dbab509be4..f075e2170b 100644
--- a/doc/man1/openssl.pod
+++ b/doc/man1/openssl.pod
@@ -967,10 +967,11 @@ This certificate may be self-issued or belong to an intermediate CA.
=item B<-check_ss_sig>
-Verify the signature on the last certificate in a chain
-even when it is a self-signed (root CA) certificate.
-By default in this case the check is disabled
-because it does not add any security.
+Verify the signature of
+the last certificate in a chain if the certificate is supposedly self-signed.
+This is prohibited and will result in an error if it is a non-conforming CA
+certificate with key usage restrictions not including the keyCertSign bit.
+This verification is disabled by default because it doesn't add any security.
=item B<-allow_proxy_certs>
diff --git a/doc/man3/X509_VERIFY_PARAM_set_flags.pod b/doc/man3/X509_VERIFY_PARAM_set_flags.pod
index 72da4cb143..4f067c877c 100644
--- a/doc/man3/X509_VERIFY_PARAM_set_flags.pod
+++ b/doc/man3/X509_VERIFY_PARAM_set_flags.pod
@@ -283,13 +283,15 @@ they are enabled.
If B<X509_V_FLAG_USE_DELTAS> is set delta CRLs (if present) are used to
determine certificate status. If not set deltas are ignored.
-B<X509_V_FLAG_CHECK_SS_SIGNATURE> requires verifying the signature of the last
-certificate in a chain even when it is a self-signed (root CA) certificate.
-In this case the check is disabled by default because it does not
+B<X509_V_FLAG_CHECK_SS_SIGNATURE> requests checking the signature of
+the last certificate in a chain if the certificate is supposedly self-signed.
+This is prohibited and will result in an error if it is a non-conforming CA
+certificate with key usage restrictions not including the I<keyCertSign> bit.
+By default this check is disabled because it doesn't
add any additional security but in some cases applications might want to
-check the signature anyway. A side effect of not checking the root CA
-signature is that disabled or unsupported message digests on the root CA
-are not treated as fatal errors.
+check the signature anyway. A side effect of not checking the self-signature
+of such a certificate is that disabled or unsupported message digests used for
+the signature are not treated as fatal errors.
When B<X509_V_FLAG_TRUSTED_FIRST> is set, which is always the case since
OpenSSL 1.1.0, construction of the certificate chain
diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h
index e7d36638b2..6a207f65d1 100644
--- a/include/openssl/x509v3.h
+++ b/include/openssl/x509v3.h
@@ -667,7 +667,7 @@ int X509_check_purpose(X509 *x, int id, int ca);
int X509_supported_extension(X509_EXTENSION *ex);
int X509_PURPOSE_set(int *p, int purpose);
int X509_check_issued(X509 *issuer, X509 *subject);
-int X509_check_akid(X509 *issuer, AUTHORITY_KEYID *akid);
+int X509_check_akid(const X509 *issuer, const AUTHORITY_KEYID *akid);
void X509_set_proxy_flag(X509 *x);
void X509_set_proxy_pathlen(X509 *x, long l);
long X509_get_proxy_pathlen(X509 *x);
More information about the openssl-commits
mailing list