From 0d8dbb52e3900fdd096ca1765137958340fb8497 Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Sat, 28 Dec 2019 12:33:12 +0100 Subject: [PATCH] Add X509_self_signed(), extending and improving documenation and tests Reviewed-by: Viktor Dukhovni (Merged from https://github.com/openssl/openssl/pull/10587) --- crypto/cmp/cmp_util.c | 8 +- crypto/x509/x509_vfy.c | 49 ++++++++---- .../man3/ossl_cmp_sk_X509_add1_cert.pod | 8 +- doc/man3/X509_check_issued.pod | 2 +- doc/man3/X509_verify.pod | 14 +++- include/openssl/x509.h | 1 + test/recipes/70-test_verify_extra.t | 1 + test/verify_extra_test.c | 74 +++++++++++++------ util/libcrypto.num | 1 + 9 files changed, 107 insertions(+), 51 deletions(-) diff --git a/crypto/cmp/cmp_util.c b/crypto/cmp/cmp_util.c index 570e14cd24..d1128d7e66 100644 --- a/crypto/cmp/cmp_util.c +++ b/crypto/cmp/cmp_util.c @@ -218,7 +218,7 @@ int ossl_cmp_sk_X509_add1_cert(STACK_OF(X509) *sk, X509 *cert, } int ossl_cmp_sk_X509_add1_certs(STACK_OF(X509) *sk, STACK_OF(X509) *certs, - int no_self_issued, int no_dups, int prepend) + int no_self_signed, int no_dups, int prepend) /* compiler would allow 'const' for the list of certs, yet they are up-ref'ed */ { int i; @@ -230,7 +230,7 @@ int ossl_cmp_sk_X509_add1_certs(STACK_OF(X509) *sk, STACK_OF(X509) *certs, for (i = 0; i < sk_X509_num(certs); i++) { /* certs may be NULL */ X509 *cert = sk_X509_value(certs, i); - if (!no_self_issued || X509_check_issued(cert, cert) != X509_V_OK) { + if (!no_self_signed || X509_self_signed(cert, 0) != 1) { if (!ossl_cmp_sk_X509_add1_cert(sk, cert, no_dups, prepend)) return 0; } @@ -239,7 +239,7 @@ int ossl_cmp_sk_X509_add1_certs(STACK_OF(X509) *sk, STACK_OF(X509) *certs, } int ossl_cmp_X509_STORE_add1_certs(X509_STORE *store, STACK_OF(X509) *certs, - int only_self_issued) + int only_self_signed) { int i; @@ -252,7 +252,7 @@ int ossl_cmp_X509_STORE_add1_certs(X509_STORE *store, STACK_OF(X509) *certs, for (i = 0; i < sk_X509_num(certs); i++) { X509 *cert = sk_X509_value(certs, i); - if (!only_self_issued || X509_check_issued(cert, cert) == X509_V_OK) + if (!only_self_signed || X509_self_signed(cert, 0) == 1) if (!X509_STORE_add_cert(store, cert)) /* ups cert ref counter */ return 0; } diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 122d6f8a3b..1f17c71bc1 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -111,25 +111,43 @@ static int null_callback(int ok, X509_STORE_CTX *e) return ok; } -/* +/*- * Return 1 if given cert is considered self-signed, 0 if not, or -1 on error. - * This does not verify self-signedness but relies on x509v3_cache_extensions() - * matching issuer and subject names (i.e., the cert being self-issued) and any - * present authority key identifier matching the subject key identifier, etc. + * This actually verifies self-signedness only if requested. + * It calls X509v3_cache_extensions() + * to match issuer and subject names (i.e., the cert being self-issued) and any + * present authority key identifier to match the subject key identifier, etc. */ -static int cert_self_signed(X509_STORE_CTX *ctx, X509 *x) +static int x509_self_signed_ex(X509 *cert, int verify_signature, + OPENSSL_CTX *libctx, const char *propq) { - if (!X509v3_cache_extensions(x, ctx->libctx, ctx->propq)) - return -1; + EVP_PKEY *pkey; - if (x->ex_flags & EXFLAG_SS) - return 1; - else + if ((pkey = X509_get0_pubkey(cert)) == NULL) { /* handles cert == NULL */ + X509err(0, X509_R_UNABLE_TO_GET_CERTS_PUBLIC_KEY); + return -1; + } + if (!X509v3_cache_extensions(cert, libctx, propq)) + return -1; + if ((cert->ex_flags & EXFLAG_SS) == 0) return 0; + if (!verify_signature) + return 1; + return X509_verify_ex(cert, pkey, libctx, propq); } -/* Given a certificate try and find an exact match in the store */ +/* wrapper for internal use */ +static int cert_self_signed(X509_STORE_CTX *ctx, X509 *x, int verify_signature) +{ + return x509_self_signed_ex(x, verify_signature, ctx->libctx, ctx->propq); +} + +int X509_self_signed(X509 *cert, int verify_signature) +{ + return x509_self_signed_ex(cert, verify_signature, NULL, NULL); +} +/* Given a certificate try and find an exact match in the store */ static X509 *lookup_cert_match(X509_STORE_CTX *ctx, X509 *x) { STACK_OF(X509) *certs; @@ -365,7 +383,6 @@ static int check_issued(X509_STORE_CTX *ctx, X509 *x, X509 *issuer) } /* Alternative lookup method: look from a STACK stored in other_ctx */ - static int get_issuer_sk(X509 **issuer, X509_STORE_CTX *ctx, X509 *x) { *issuer = find_issuer(ctx, ctx->other_ctx, x); @@ -2954,7 +2971,7 @@ static int build_chain(X509_STORE_CTX *ctx) SSL_DANE *dane = ctx->dane; int num = sk_X509_num(ctx->chain); X509 *cert = sk_X509_value(ctx->chain, num - 1); - int self_signed = cert_self_signed(ctx, cert); + int self_signed; STACK_OF(X509) *sktmp = NULL; unsigned int search; int may_trusted = 0; @@ -2972,7 +2989,7 @@ static int build_chain(X509_STORE_CTX *ctx) return 0; } - self_signed = cert_self_signed(ctx, cert); + self_signed = cert_self_signed(ctx, cert, 0); if (self_signed < 0) { ctx->error = X509_V_ERR_UNSPECIFIED; return 0; @@ -3150,7 +3167,7 @@ static int build_chain(X509_STORE_CTX *ctx) search = 0; continue; } - self_signed = cert_self_signed(ctx, x); + self_signed = cert_self_signed(ctx, x, 0); if (self_signed < 0) { ctx->error = X509_V_ERR_UNSPECIFIED; return 0; @@ -3276,7 +3293,7 @@ static int build_chain(X509_STORE_CTX *ctx) x = xtmp; ++ctx->num_untrusted; - self_signed = cert_self_signed(ctx, xtmp); + self_signed = cert_self_signed(ctx, xtmp, 0); if (self_signed < 0) { sk_X509_free(sktmp); ctx->error = X509_V_ERR_UNSPECIFIED; diff --git a/doc/internal/man3/ossl_cmp_sk_X509_add1_cert.pod b/doc/internal/man3/ossl_cmp_sk_X509_add1_cert.pod index d8f617f55c..289428878e 100644 --- a/doc/internal/man3/ossl_cmp_sk_X509_add1_cert.pod +++ b/doc/internal/man3/ossl_cmp_sk_X509_add1_cert.pod @@ -15,9 +15,9 @@ ossl_cmp_X509_STORE_get1_certs int ossl_cmp_sk_X509_add1_cert(STACK_OF(X509) *sk, X509 *cert, int no_dup, int prepend); int ossl_cmp_sk_X509_add1_certs(STACK_OF(X509) *sk, STACK_OF(X509) *certs, - int no_self_issued, int no_dups, int prepend); + int no_self_signed, int no_dups, int prepend); int ossl_cmp_X509_STORE_add1_certs(X509_STORE *store, STACK_OF(X509) *certs, - int only_self_issued); + int only_self_signed); STACK_OF(X509) *ossl_cmp_X509_STORE_get1_certs(X509_STORE *store); =head1 DESCRIPTION @@ -29,10 +29,10 @@ On success the reference count of the certificate is increased. ossl_cmp_sk_X509_add1_certs() appends or prepends (depending on the I argument) a list of certificates to the given list, -optionally only if not self-issued and optionally only if not already contained. +optionally only if not self-signed and optionally only if not already contained. The reference counts of those certificates appended successfully are increased. -ossl_cmp_X509_STORE_add1_certs() adds all or only self-issued certificates from +ossl_cmp_X509_STORE_add1_certs() adds all or only self-signed certificates from the given stack to given store. The I parameter may be NULL. ossl_cmp_X509_STORE_get1_certs() retrieves a copy of all certificates in the diff --git a/doc/man3/X509_check_issued.pod b/doc/man3/X509_check_issued.pod index cc98541bba..0aedefa459 100644 --- a/doc/man3/X509_check_issued.pod +++ b/doc/man3/X509_check_issued.pod @@ -31,7 +31,7 @@ or some B constant to indicate an error. =head1 SEE ALSO L, L, L, -L +L, L =head1 COPYRIGHT diff --git a/doc/man3/X509_verify.pod b/doc/man3/X509_verify.pod index a1ed4d32fe..e0028473a2 100644 --- a/doc/man3/X509_verify.pod +++ b/doc/man3/X509_verify.pod @@ -2,7 +2,7 @@ =head1 NAME -X509_verify_ex, X509_verify, +X509_verify_ex, X509_verify, X509_self_signed, X509_REQ_verify_ex, X509_REQ_verify, X509_CRL_verify - verify certificate, certificate request, or CRL signature @@ -14,6 +14,7 @@ verify certificate, certificate request, or CRL signature int X509_verify_ex(X509 *x, EVP_PKEY *pkey, OPENSSL_CTX *libctx, const char *propq); int X509_verify(X509 *x, EVP_PKEY *pkey); + int X509_self_signed(X509 *cert, int verify_signature); int X509_REQ_verify_ex(X509_REQ *a, EVP_PKEY *pkey, OPENSSL_CTX *libctx, const char *propq); @@ -31,6 +32,12 @@ no other checks (such as certificate chain validity) are performed. X509_verify() is the same as X509_verify_ex() except that the default library context and property query string are used. +X509_self_signed() checks whether a certificate is self-signed. +For success the issuer and subject names must match, the components of the +authority key identifier (if present) must match the subject key identifier etc. +The signature itself is actually verified only if B is 1, as +for explicitly trusted certificates this verification is not worth the effort. + X509_REQ_verify_ex(), X509_REQ_verify() and X509_CRL_verify() verify the signatures of certificate requests and CRLs, respectively. @@ -42,6 +49,9 @@ return 1 if the signature is valid and 0 if the signature check fails. If the signature could not be checked at all because it was ill-formed or some other error occurred then -1 is returned. +X509_self_signed() returns the same values but also returns 1 +if all respective fields match and B is 0. + =head1 SEE ALSO L, @@ -65,7 +75,7 @@ L The X509_verify(), X509_REQ_verify(), and X509_CRL_verify() functions are available in all versions of OpenSSL. -X509_verify_ex() and X509_REQ_verify_ex() +X509_verify_ex(), X509_REQ_verify_ex(), and X509_self_signed() were added in OpenSSL 3.0. =head1 COPYRIGHT diff --git a/include/openssl/x509.h b/include/openssl/x509.h index b0e33d5286..2212ceeedc 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -345,6 +345,7 @@ const char *X509_verify_cert_error_string(long n); int X509_verify_ex(X509 *a, EVP_PKEY *r, OPENSSL_CTX *libctx, const char *propq); int X509_verify(X509 *a, EVP_PKEY *r); +int X509_self_signed(X509 *cert, int verify_signature); int X509_REQ_verify_ex(X509_REQ *a, EVP_PKEY *r, OPENSSL_CTX *libctx, const char *propq); diff --git a/test/recipes/70-test_verify_extra.t b/test/recipes/70-test_verify_extra.t index b8f4ab4312..6876870bbf 100644 --- a/test/recipes/70-test_verify_extra.t +++ b/test/recipes/70-test_verify_extra.t @@ -14,6 +14,7 @@ setup("test_verify_extra"); plan tests => 1; ok(run(test(["verify_extra_test", + srctop_file("test", "certs", "rootCA.pem"), srctop_file("test", "certs", "roots.pem"), srctop_file("test", "certs", "untrusted.pem"), srctop_file("test", "certs", "bad.pem"), diff --git a/test/verify_extra_test.c b/test/verify_extra_test.c index 6cce626026..99a6361142 100644 --- a/test/verify_extra_test.c +++ b/test/verify_extra_test.c @@ -18,11 +18,24 @@ DEFINE_STACK_OF(X509) +static const char *root_f; static const char *roots_f; static const char *untrusted_f; static const char *bad_f; static const char *req_f; +static X509 *load_cert_from_file(const char *filename) +{ + X509 *cert = NULL; + BIO *bio; + + bio = BIO_new_file(filename, "r"); + if (bio != NULL) + cert = PEM_read_bio_X509(bio, NULL, 0, NULL); + BIO_free(bio); + return cert; +} + static STACK_OF(X509) *load_certs_from_file(const char *filename) { STACK_OF(X509) *certs; @@ -97,7 +110,6 @@ static int test_alt_chains_cert_forgery(void) int i; X509 *x = NULL; STACK_OF(X509) *untrusted = NULL; - BIO *bio = NULL; X509_STORE_CTX *sctx = NULL; X509_STORE *store = NULL; X509_LOOKUP *lookup = NULL; @@ -114,10 +126,7 @@ static int test_alt_chains_cert_forgery(void) untrusted = load_certs_from_file(untrusted_f); - if ((bio = BIO_new_file(bad_f, "r")) == NULL) - goto err; - - if ((x = PEM_read_bio_X509(bio, NULL, 0, NULL)) == NULL) + if ((x = load_cert_from_file(bad_f)) == NULL) goto err; sctx = X509_STORE_CTX_new(); @@ -136,7 +145,6 @@ static int test_alt_chains_cert_forgery(void) err: X509_STORE_CTX_free(sctx); X509_free(x); - BIO_free(bio); sk_X509_pop_free(untrusted, X509_free); X509_STORE_free(store); return ret; @@ -146,14 +154,9 @@ static int test_store_ctx(void) { X509_STORE_CTX *sctx = NULL; X509 *x = NULL; - BIO *bio = NULL; int testresult = 0, ret; - bio = BIO_new_file(bad_f, "r"); - if (bio == NULL) - goto err; - - x = PEM_read_bio_X509(bio, NULL, 0, NULL); + x = load_cert_from_file(bad_f); if (x == NULL) goto err; @@ -175,7 +178,6 @@ static int test_store_ctx(void) err: X509_STORE_CTX_free(sctx); X509_free(x); - BIO_free(bio); return testresult; } @@ -184,16 +186,11 @@ OPT_TEST_DECLARE_USAGE("roots.pem untrusted.pem bad.pem\n") static int test_distinguishing_id(void) { X509 *x = NULL; - BIO *bio = NULL; int ret = 0; ASN1_OCTET_STRING *v = NULL, *v2 = NULL; char *distid = "this is an ID"; - bio = BIO_new_file(bad_f, "r"); - if (bio == NULL) - goto err; - - x = PEM_read_bio_X509(bio, NULL, 0, NULL); + x = load_cert_from_file(bad_f); if (x == NULL) goto err; @@ -217,7 +214,6 @@ static int test_distinguishing_id(void) ret = 1; err: X509_free(x); - BIO_free(bio); return ret; } @@ -261,6 +257,32 @@ static int test_req_distinguishing_id(void) return ret; } +static int test_self_signed(const char *filename, int expected) +{ + X509 *cert; + int ret; + + cert = load_cert_from_file(filename); /* may result in NULL */ + ret = TEST_int_eq(X509_self_signed(cert, 1), expected); + X509_free(cert); + return ret; +} + +static int test_self_signed_good(void) +{ + return test_self_signed(root_f, 1); +} + +static int test_self_signed_bad(void) +{ + return test_self_signed(bad_f, 0); +} + +static int test_self_signed_error(void) +{ + return test_self_signed("nonexistent file name", -1); +} + int setup_tests(void) { if (!test_skip_common_options()) { @@ -268,15 +290,19 @@ int setup_tests(void) return 0; } - if (!TEST_ptr(roots_f = test_get_argument(0)) - || !TEST_ptr(untrusted_f = test_get_argument(1)) - || !TEST_ptr(bad_f = test_get_argument(2)) - || !TEST_ptr(req_f = test_get_argument(3))) + if (!TEST_ptr(root_f = test_get_argument(0)) + || !TEST_ptr(roots_f = test_get_argument(1)) + || !TEST_ptr(untrusted_f = test_get_argument(2)) + || !TEST_ptr(bad_f = test_get_argument(3)) + || !TEST_ptr(req_f = test_get_argument(4))) return 0; ADD_TEST(test_alt_chains_cert_forgery); ADD_TEST(test_store_ctx); ADD_TEST(test_distinguishing_id); ADD_TEST(test_req_distinguishing_id); + ADD_TEST(test_self_signed_good); + ADD_TEST(test_self_signed_bad); + ADD_TEST(test_self_signed_error); return 1; } diff --git a/util/libcrypto.num b/util/libcrypto.num index 22c7cdc709..db033eee9d 100644 --- a/util/libcrypto.num +++ b/util/libcrypto.num @@ -4681,6 +4681,7 @@ ERR_set_error ? 3_0_0 EXIST::FUNCTION: ERR_vset_error ? 3_0_0 EXIST::FUNCTION: X509_get0_authority_issuer ? 3_0_0 EXIST::FUNCTION: X509_get0_authority_serial ? 3_0_0 EXIST::FUNCTION: +X509_self_signed ? 3_0_0 EXIST::FUNCTION: EC_GROUP_new_by_curve_name_ex ? 3_0_0 NOEXIST::FUNCTION:EC EC_KEY_new_ex ? 3_0_0 NOEXIST::FUNCTION:EC EC_KEY_new_by_curve_name_ex ? 3_0_0 NOEXIST::FUNCTION:EC -- 2.34.1