Add X509_self_signed(), extending and improving documenation and tests

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10587)

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<prepend>
 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<certs> 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<X509_V_ERR*> constant to indicate an error.
 =head1 SEE ALSO
 
 L<X509_verify_cert(3)>, L<X509_verify(3)>, L<X509_check_ca(3)>,
-L<openssl-verify(1)>
+L<openssl-verify(1)>, L<X509_self_signed(3)>
 
 =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<verify_signature> 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<verify_signature> is 0.
+
 =head1 SEE ALSO
 
 L<d2i_X509(3)>,
@@ -65,7 +75,7 @@ L<OPENSSL_CTX(3)>
 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