Make X509_set_sm2_id consistent with other setters

This commit makes the X509_set_sm2_id to 'set0' behaviour, which means
the memory management is passed to X509 and user doesn't need to free
the sm2_id parameter later. API name also changes to X509_set0_sm2_id.

Document and test case are also updated.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8626)

diff --git a/apps/verify.c b/apps/verify.c
index 67d3276226c5c77d1cfe702ce9ddd0c615edd9d0..3767972a5e7a0f611e2d57febfd8511c066fa0ab 100644 (file)
--- a/apps/verify.c
+++ b/apps/verify.c
@@ -246,27 +246,37 @@ static int check(X509_STORE *ctx, const char *file,
 
     if (sm2id != NULL) {
 #ifndef OPENSSL_NO_SM2
-        ASN1_OCTET_STRING v;
+        ASN1_OCTET_STRING *v;
 
-        v.data = sm2id;
-        v.length = sm2idlen;
+        v = ASN1_OCTET_STRING_new();
+        if (v == NULL) {
+            BIO_printf(bio_err, "error: SM2 ID allocation failed\n");
+            goto end;
+        }
 
-        X509_set_sm2_id(x, &v);
+        if (!ASN1_OCTET_STRING_set(v, sm2id, sm2idlen)) {
+            BIO_printf(bio_err, "error: setting SM2 ID failed\n");
+            ASN1_OCTET_STRING_free(v);
+            goto end;
+        }
+
+        X509_set0_sm2_id(x, v);
 #endif
     }
 
     csc = X509_STORE_CTX_new();
     if (csc == NULL) {
-        printf("error %s: X.509 store context allocation failed\n",
-               (file == NULL) ? "stdin" : file);
+        BIO_printf(bio_err, "error %s: X.509 store context allocation failed\n",
+                   (file == NULL) ? "stdin" : file);
         goto end;
     }
 
     X509_STORE_set_flags(ctx, vflags);
     if (!X509_STORE_CTX_init(csc, ctx, x, uchain)) {
         X509_STORE_CTX_free(csc);
-        printf("error %s: X.509 store context initialization failed\n",
-               (file == NULL) ? "stdin" : file);
+        BIO_printf(bio_err,
+                   "error %s: X.509 store context initialization failed\n",
+                   (file == NULL) ? "stdin" : file);
         goto end;
     }
     if (tchain != NULL)
@@ -275,28 +285,30 @@ static int check(X509_STORE *ctx, const char *file,
         X509_STORE_CTX_set0_crls(csc, crls);
     i = X509_verify_cert(csc);
     if (i > 0 && X509_STORE_CTX_get_error(csc) == X509_V_OK) {
-        printf("%s: OK\n", (file == NULL) ? "stdin" : file);
+        BIO_printf(bio_out, "%s: OK\n", (file == NULL) ? "stdin" : file);
         ret = 1;
         if (show_chain) {
             int j;
 
             chain = X509_STORE_CTX_get1_chain(csc);
             num_untrusted = X509_STORE_CTX_get_num_untrusted(csc);
-            printf("Chain:\n");
+            BIO_printf(bio_out, "Chain:\n");
             for (j = 0; j < sk_X509_num(chain); j++) {
                 X509 *cert = sk_X509_value(chain, j);
-                printf("depth=%d: ", j);
+                BIO_printf(bio_out, "depth=%d: ", j);
                 X509_NAME_print_ex_fp(stdout,
                                       X509_get_subject_name(cert),
                                       0, get_nameopt());
                 if (j < num_untrusted)
-                    printf(" (untrusted)");
-                printf("\n");
+                    BIO_printf(bio_out, " (untrusted)");
+                BIO_printf(bio_out, "\n");
             }
             sk_X509_pop_free(chain, X509_free);
         }
     } else {
-        printf("error %s: verification failed\n", (file == NULL) ? "stdin" : file);
+        BIO_printf(bio_err,
+                   "error %s: verification failed\n",
+                   (file == NULL) ? "stdin" : file);
     }
     X509_STORE_CTX_free(csc);
 

diff --git a/crypto/include/internal/x509_int.h b/crypto/include/internal/x509_int.h
index 93f923e0d5305218bc308ade30761e21ac921b0e..7c40b159cca680eac177c63ba7f3f23508f1cf52 100644 (file)
--- a/crypto/include/internal/x509_int.h
+++ b/crypto/include/internal/x509_int.h
@@ -184,7 +184,7 @@ struct x509_st {
     CRYPTO_RWLOCK *lock;
     volatile int ex_cached;
 # ifndef OPENSSL_NO_SM2
-    ASN1_OCTET_STRING sm2_id;
+    ASN1_OCTET_STRING *sm2_id;
 # endif
 } /* X509 */ ;
 

diff --git a/crypto/x509/x_all.c b/crypto/x509/x_all.c
index afcf0b7b3d40e5fda6d8a64506ee4d753826f2ed..9c9e8ffad0d51eb17260dcde83ff85347562ab24 100644 (file)
--- a/crypto/x509/x_all.c
+++ b/crypto/x509/x_all.c
@@ -72,7 +72,10 @@ static int x509_verify_sm2(X509 *x, EVP_PKEY *pkey, int mdnid, int pknid)
         ret = 0;
         goto err;
     }
-    if (EVP_PKEY_CTX_set1_id(pctx, x->sm2_id.data, x->sm2_id.length) != 1) {
+    /* NOTE: we tolerate no actual ID, to provide maximum flexibility */
+    if (x->sm2_id != NULL
+            && EVP_PKEY_CTX_set1_id(pctx, x->sm2_id->data,
+                                    x->sm2_id->length) != 1) {
         X509err(X509_F_X509_VERIFY_SM2, ERR_R_EVP_LIB);
         ret = 0;
         goto err;

diff --git a/crypto/x509/x_x509.c b/crypto/x509/x_x509.c
index 901a3e6a925ece6915d596e1013a6c0cdd0e63b9..78e1a7569e521c13e69e720e0b2e3b0853c4892b 100644 (file)
--- a/crypto/x509/x_x509.c
+++ b/crypto/x509/x_x509.c
@@ -72,6 +72,9 @@ static int x509_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
 #ifndef OPENSSL_NO_RFC3779
         ret->rfc3779_addr = NULL;
         ret->rfc3779_asid = NULL;
+#endif
+#ifndef OPENSSL_NO_SM2
+        ret->sm2_id = NULL;
 #endif
         ret->aux = NULL;
         ret->crldp = NULL;
@@ -91,6 +94,9 @@ static int x509_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
 #ifndef OPENSSL_NO_RFC3779
         sk_IPAddressFamily_pop_free(ret->rfc3779_addr, IPAddressFamily_free);
         ASIdentifiers_free(ret->rfc3779_asid);
+#endif
+#ifndef OPENSSL_NO_SM2
+        ASN1_OCTET_STRING_free(ret->sm2_id);
 #endif
         break;
 
@@ -246,13 +252,14 @@ int X509_get_signature_nid(const X509 *x)
 }
 
 #ifndef OPENSSL_NO_SM2
-void X509_set_sm2_id(X509 *x, ASN1_OCTET_STRING *sm2_id)
+void X509_set0_sm2_id(X509 *x, ASN1_OCTET_STRING *sm2_id)
 {
-    x->sm2_id = *sm2_id;
+    ASN1_OCTET_STRING_free(x->sm2_id);
+    x->sm2_id = sm2_id;
 }
 
 ASN1_OCTET_STRING *X509_get0_sm2_id(X509 *x)
 {
-    return &x->sm2_id;
+    return x->sm2_id;
 }
 #endif

diff --git a/doc/man3/X509_get0_sm2_id.pod b/doc/man3/X509_get0_sm2_id.pod
index 84da71e831adca59b49ac0d46ca3eb4d4107fa6f..9698c869a641a83030e1bb37cf9490d3600a5e8d 100644 (file)
--- a/doc/man3/X509_get0_sm2_id.pod
+++ b/doc/man3/X509_get0_sm2_id.pod
@@ -2,20 +2,24 @@
 
 =head1 NAME
 
-X509_get0_sm2_id, X509_set_sm2_id - get or set SM2 ID for certificate operations
+X509_get0_sm2_id, X509_set0_sm2_id - get or set SM2 ID for certificate operations
 
 =head1 SYNOPSIS
 
  #include <openssl/x509.h>
 
  ASN1_OCTET_STRING *X509_get0_sm2_id(X509 *x);
- void X509_set_sm2_id(X509 *x, ASN1_OCTET_STRING *sm2_id);
+ void X509_set0_sm2_id(X509 *x, ASN1_OCTET_STRING *sm2_id);
 
 =head1 DESCRIPTION
 
 X509_get0_sm2_id() gets the ID value of an SM2 certificate B<x> by returning an
 B<ASN1_OCTET_STRING> object which should not be freed by the caller.
-X509_set_sm2_id() sets the B<sm2_id> value to an SM2 certificate B<x>.
+
+X509_set0_sm2_id() sets the B<sm2_id> value to an SM2 certificate B<x>. Calling
+this function transfers the memory management of the value to the X509 object,
+and therefore the value that has been passed in should not be freed by the
+caller after this function has been called.
 
 =head1 NOTES
 
@@ -25,7 +29,7 @@ ability to set and retrieve the SM2 ID value.
 
 =head1 RETURN VALUES
 
-X509_set_sm2_id() does not return a value.
+X509_set0_sm2_id() does not return a value.
 
 =head1 SEE ALSO
 

diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 4de88bd57beb0f8fd7de12a940246c08a497c3fc..7f80da3ccf24ed9b9359652425af86aae4efcb3a 100644 (file)
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -567,7 +567,7 @@ void X509_get0_signature(const ASN1_BIT_STRING **psig,
 int X509_get_signature_nid(const X509 *x);
 
 # ifndef OPENSSL_NO_SM2
-void X509_set_sm2_id(X509 *x, ASN1_OCTET_STRING *sm2_id);
+void X509_set0_sm2_id(X509 *x, ASN1_OCTET_STRING *sm2_id);
 ASN1_OCTET_STRING *X509_get0_sm2_id(X509 *x);
 # endif
 

diff --git a/test/verify_extra_test.c b/test/verify_extra_test.c
index 468de62092a9311f2b89d169156953f004c44da9..f16b3f0b4a1c1c1f87b4bca610e40c2d59ca8873 100644 (file)
--- a/test/verify_extra_test.c
+++ b/test/verify_extra_test.c
@@ -8,6 +8,7 @@
  */
 
 #include <stdio.h>
+#include <string.h>
 #include <openssl/crypto.h>
 #include <openssl/bio.h>
 #include <openssl/x509.h>
@@ -177,6 +178,48 @@ static int test_store_ctx(void)
 
 OPT_TEST_DECLARE_USAGE("roots.pem untrusted.pem bad.pem\n")
 
+#ifndef OPENSSL_NO_SM2
+static int test_sm2_id(void)
+{
+    /* we only need an X509 structure, no matter if it's a real SM2 cert */
+    X509 *x = NULL;
+    BIO *bio = NULL;
+    int ret = 0;
+    ASN1_OCTET_STRING *v = NULL, *v2 = NULL;
+    char *sm2id = "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);
+    if (x == NULL)
+        goto err;
+
+    v = ASN1_OCTET_STRING_new();
+    if (v == NULL)
+        goto err;
+
+    if (!ASN1_OCTET_STRING_set(v, (unsigned char *)sm2id, (int)strlen(sm2id))) {
+        ASN1_OCTET_STRING_free(v);
+        goto err;
+    }
+
+    X509_set0_sm2_id(x, v);
+
+    v2 = X509_get0_sm2_id(x);
+    if (!TEST_ptr(v2)
+            || !TEST_int_eq(ASN1_OCTET_STRING_cmp(v, v2), 0))
+        goto err;
+
+    ret = 1;
+ err:
+    X509_free(x);
+    BIO_free(bio);
+    return ret;
+}
+#endif
+
 int setup_tests(void)
 {
     if (!TEST_ptr(roots_f = test_get_argument(0))
@@ -186,5 +229,8 @@ int setup_tests(void)
 
     ADD_TEST(test_alt_chains_cert_forgery);
     ADD_TEST(test_store_ctx);
+#ifndef OPENSSL_NO_SM2
+    ADD_TEST(test_sm2_id);
+#endif
     return 1;
 }

diff --git a/util/libcrypto.num b/util/libcrypto.num
index d275e579ba4ffb51de54bea4fa667d37846124d2..9569bf43f3f3f73b0c15ba55969e758397ee36b9 100644 (file)
--- a/util/libcrypto.num
+++ b/util/libcrypto.num
@@ -4788,7 +4788,7 @@ OSSL_PARAM_get_utf8_ptr                 4735      3_0_0   EXIST::FUNCTION:
 OSSL_PARAM_set_utf8_ptr                 4736   3_0_0   EXIST::FUNCTION:
 OSSL_PARAM_get_octet_ptr                4737   3_0_0   EXIST::FUNCTION:
 OSSL_PARAM_set_octet_ptr                4738   3_0_0   EXIST::FUNCTION:
-X509_set_sm2_id                         4739   3_0_0   EXIST::FUNCTION:SM2
+X509_set0_sm2_id                        4739   3_0_0   EXIST::FUNCTION:SM2
 X509_get0_sm2_id                        4740   3_0_0   EXIST::FUNCTION:SM2
 EVP_PKEY_get0_engine                    4741   3_0_0   EXIST::FUNCTION:ENGINE
 EVP_MD_upref                            4742   3_0_0   EXIST::FUNCTION: