X509: Refactor X509_verify() and X509_REQ_verify() for better streamlining
authorRichard Levitte <levitte@openssl.org>
Fri, 24 Jan 2020 17:04:19 +0000 (18:04 +0100)
committerRichard Levitte <levitte@openssl.org>
Sun, 2 Feb 2020 11:04:00 +0000 (12:04 +0100)
The solution to incorporate the SM2 identity processing was an off
the side hack that more or less duplicated the ASN1_item_verify()
code with just a few lines being different.  We replace this with
a new function ASN1_item_verify_ctx(), which takes an EVP_MD_CTX
pointer instead of an EVP_PKEY pointer, just like its sibling
ASN1_item_sign_ctx().

This allows us to refactor X509_verify() and X509_REQ_verify() to
simply create a local EVP_MD_CTX and an attached EVP_PKEY_CTX,
which gets to hold the SM2 identity, if there is one, and then let
ASN1_item_verify_ctx() to its job.

This will also make it easier to adapt ASN1_item_verify_ctx() for
provider based keys.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/10942)

crypto/asn1/a_verify.c
crypto/x509/x_all.c
include/openssl/x509.h
util/libcrypto.num

index 92f9448..94a11c1 100644 (file)
@@ -88,65 +88,85 @@ int ASN1_verify(i2d_of_void *i2d, X509_ALGOR *a, ASN1_BIT_STRING *signature,
 int ASN1_item_verify(const ASN1_ITEM *it, X509_ALGOR *a,
                      ASN1_BIT_STRING *signature, void *asn, EVP_PKEY *pkey)
 {
-    EVP_MD_CTX *ctx = NULL;
+    int rv = -1;
+    EVP_MD_CTX *ctx = EVP_MD_CTX_new();
+    EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new(pkey, NULL);
+
+    if (ctx == NULL || pctx == NULL) {
+        ASN1err(0, ERR_R_MALLOC_FAILURE);
+        goto err;
+    }
+
+    EVP_MD_CTX_set_pkey_ctx(ctx, pctx);
+
+    rv = ASN1_item_verify_ctx(it, a, signature, asn, ctx);
+
+ err:
+    EVP_PKEY_CTX_free(pctx);
+    EVP_MD_CTX_free(ctx);
+    return rv;
+}
+
+int ASN1_item_verify_ctx(const ASN1_ITEM *it, X509_ALGOR *a,
+                         ASN1_BIT_STRING *signature, void *asn,
+                         EVP_MD_CTX *ctx)
+{
+    EVP_PKEY *pkey;
     unsigned char *buf_in = NULL;
     int ret = -1, inl = 0;
     int mdnid, pknid;
     size_t inll = 0;
 
+    pkey = EVP_PKEY_CTX_get0_pkey(EVP_MD_CTX_pkey_ctx(ctx));
+
     if (pkey == NULL) {
-        ASN1err(ASN1_F_ASN1_ITEM_VERIFY, ERR_R_PASSED_NULL_PARAMETER);
+        ASN1err(0, ERR_R_PASSED_NULL_PARAMETER);
         return -1;
     }
 
     if (signature->type == V_ASN1_BIT_STRING && signature->flags & 0x7) {
-        ASN1err(ASN1_F_ASN1_ITEM_VERIFY, ASN1_R_INVALID_BIT_STRING_BITS_LEFT);
+        ASN1err(0, ASN1_R_INVALID_BIT_STRING_BITS_LEFT);
         return -1;
     }
 
-    ctx = EVP_MD_CTX_new();
-    if (ctx == NULL) {
-        ASN1err(ASN1_F_ASN1_ITEM_VERIFY, ERR_R_MALLOC_FAILURE);
-        goto err;
-    }
-
     /* Convert signature OID into digest and public key OIDs */
     if (!OBJ_find_sigid_algs(OBJ_obj2nid(a->algorithm), &mdnid, &pknid)) {
-        ASN1err(ASN1_F_ASN1_ITEM_VERIFY, ASN1_R_UNKNOWN_SIGNATURE_ALGORITHM);
+        ASN1err(0, ASN1_R_UNKNOWN_SIGNATURE_ALGORITHM);
         goto err;
     }
+
     if (mdnid == NID_undef) {
         if (pkey->ameth == NULL || pkey->ameth->item_verify == NULL) {
-            ASN1err(ASN1_F_ASN1_ITEM_VERIFY,
-                    ASN1_R_UNKNOWN_SIGNATURE_ALGORITHM);
+            ASN1err(0, ASN1_R_UNKNOWN_SIGNATURE_ALGORITHM);
             goto err;
         }
         ret = pkey->ameth->item_verify(ctx, it, asn, a, signature, pkey);
         /*
-         * Return value of 2 means carry on, anything else means we exit
-         * straight away: either a fatal error of the underlying verification
-         * routine handles all verification.
+         * Return values meaning:
+         * <=0: error.
+         *   1: method does everything.
+         *   2: carry on as normal, method has called EVP_DigestVerifyInit()
          */
-        if (ret != 2)
+        if (ret <= 0)
+            ASN1err(0, ERR_R_EVP_LIB);
+        if (ret <= 1)
             goto err;
-        ret = -1;
     } else {
         const EVP_MD *type = EVP_get_digestbynid(mdnid);
 
         if (type == NULL) {
-            ASN1err(ASN1_F_ASN1_ITEM_VERIFY,
-                    ASN1_R_UNKNOWN_MESSAGE_DIGEST_ALGORITHM);
+            ASN1err(0, ASN1_R_UNKNOWN_MESSAGE_DIGEST_ALGORITHM);
             goto err;
         }
 
         /* Check public key OID matches public key type */
         if (EVP_PKEY_type(pknid) != pkey->ameth->pkey_id) {
-            ASN1err(ASN1_F_ASN1_ITEM_VERIFY, ASN1_R_WRONG_PUBLIC_KEY_TYPE);
+            ASN1err(0, ASN1_R_WRONG_PUBLIC_KEY_TYPE);
             goto err;
         }
 
         if (!EVP_DigestVerifyInit(ctx, NULL, type, NULL, pkey)) {
-            ASN1err(ASN1_F_ASN1_ITEM_VERIFY, ERR_R_EVP_LIB);
+            ASN1err(0, ERR_R_EVP_LIB);
             ret = 0;
             goto err;
         }
@@ -154,11 +174,11 @@ int ASN1_item_verify(const ASN1_ITEM *it, X509_ALGOR *a,
 
     inl = ASN1_item_i2d(asn, &buf_in, it);
     if (inl <= 0) {
-        ASN1err(ASN1_F_ASN1_ITEM_VERIFY, ERR_R_INTERNAL_ERROR);
+        ASN1err(0, ERR_R_INTERNAL_ERROR);
         goto err;
     }
     if (buf_in == NULL) {
-        ASN1err(ASN1_F_ASN1_ITEM_VERIFY, ERR_R_MALLOC_FAILURE);
+        ASN1err(0, ERR_R_MALLOC_FAILURE);
         goto err;
     }
     inll = inl;
@@ -166,12 +186,11 @@ int ASN1_item_verify(const ASN1_ITEM *it, X509_ALGOR *a,
     ret = EVP_DigestVerify(ctx, signature->data, (size_t)signature->length,
                            buf_in, inl);
     if (ret <= 0) {
-        ASN1err(ASN1_F_ASN1_ITEM_VERIFY, ERR_R_EVP_LIB);
+        ASN1err(0, ERR_R_EVP_LIB);
         goto err;
     }
     ret = 1;
  err:
     OPENSSL_clear_free(buf_in, inll);
-    EVP_MD_CTX_free(ctx);
     return ret;
 }
index fbdb100..9af26e6 100644 (file)
 #include <openssl/dsa.h>
 #include <openssl/x509v3.h>
 
-#ifndef OPENSSL_NO_SM2
+static void clean_id_ctx(EVP_MD_CTX *ctx)
+{
+    EVP_PKEY_CTX *pctx = EVP_MD_CTX_pkey_ctx(ctx);
 
-# include "crypto/asn1.h"
-# include "crypto/evp.h"
+    EVP_PKEY_CTX_free(pctx);
+    EVP_MD_CTX_free(ctx);
+}
 
-static int common_verify_sm2(void *data, EVP_PKEY *pkey,
-                             int mdnid, int pknid, int req)
+static EVP_MD_CTX *make_id_ctx(EVP_PKEY *r, ASN1_OCTET_STRING *id)
 {
-    X509 *x = NULL;
-    X509_REQ *r = NULL;
     EVP_MD_CTX *ctx = NULL;
-    unsigned char *buf_in = NULL;
-    int ret = -1, inl = 0;
-    size_t inll = 0;
     EVP_PKEY_CTX *pctx = NULL;
-    const EVP_MD *type = EVP_get_digestbynid(mdnid);
-    ASN1_BIT_STRING *signature = NULL;
-    ASN1_OCTET_STRING *sm2_id = NULL;
-    ASN1_VALUE *tbv = NULL;
-
-    if (type == NULL) {
-        X509err(X509_F_COMMON_VERIFY_SM2,
-                ASN1_R_UNKNOWN_MESSAGE_DIGEST_ALGORITHM);
-        goto err;
-    }
-
-    if (pkey == NULL) {
-        X509err(X509_F_COMMON_VERIFY_SM2, ERR_R_PASSED_NULL_PARAMETER);
-        return -1;
-    }
-
-    if (req == 1) {
-        r = (X509_REQ *)data;
-        signature = r->signature;
-        sm2_id = r->sm2_id;
-        tbv = (ASN1_VALUE *)&r->req_info;
-    } else {
-        x = (X509 *)data;
-        signature = &x->signature;
-        sm2_id = x->sm2_id;
-        tbv = (ASN1_VALUE *)&x->cert_info;
-    }
-
-    if (signature->type == V_ASN1_BIT_STRING && signature->flags & 0x7) {
-        X509err(X509_F_COMMON_VERIFY_SM2, ASN1_R_INVALID_BIT_STRING_BITS_LEFT);
-        return -1;
-    }
 
-    ctx = EVP_MD_CTX_new();
-    if (ctx == NULL) {
-        X509err(X509_F_COMMON_VERIFY_SM2, ERR_R_MALLOC_FAILURE);
-        goto err;
+    if ((ctx = EVP_MD_CTX_new()) == NULL
+        || (pctx = EVP_PKEY_CTX_new(r, NULL)) == NULL) {
+        X509err(0, ERR_R_MALLOC_FAILURE);
+        goto error;
     }
 
-    /* Check public key OID matches public key type */
-    if (EVP_PKEY_type(pknid) != pkey->ameth->pkey_id) {
-        X509err(X509_F_COMMON_VERIFY_SM2, ASN1_R_WRONG_PUBLIC_KEY_TYPE);
-        goto err;
+    if (id != NULL) {
+        if (EVP_PKEY_CTX_set1_id(pctx, id->data, id->length) <= 0) {
+            X509err(0, ERR_R_MALLOC_FAILURE);
+            goto error;
+        }
     }
 
-    if (!EVP_PKEY_set_alias_type(pkey, EVP_PKEY_SM2)) {
-        X509err(X509_F_COMMON_VERIFY_SM2, ERR_R_EVP_LIB);
-        ret = 0;
-        goto err;
-    }
-    pctx = EVP_PKEY_CTX_new(pkey, NULL);
-    if (pctx == NULL) {
-        X509err(X509_F_COMMON_VERIFY_SM2, ERR_R_EVP_LIB);
-        ret = 0;
-        goto err;
-    }
-    /* NOTE: we tolerate no actual ID, to provide maximum flexibility */
-    if (sm2_id != NULL
-            && EVP_PKEY_CTX_set1_id(pctx, sm2_id->data, sm2_id->length) != 1) {
-        X509err(X509_F_COMMON_VERIFY_SM2, ERR_R_EVP_LIB);
-        ret = 0;
-        goto err;
-    }
     EVP_MD_CTX_set_pkey_ctx(ctx, pctx);
 
-    if (!EVP_DigestVerifyInit(ctx, NULL, type, NULL, pkey)) {
-        X509err(X509_F_COMMON_VERIFY_SM2, ERR_R_EVP_LIB);
-        ret = 0;
-        goto err;
-    }
-
-    inl = ASN1_item_i2d(tbv, &buf_in,
-                        req == 1 ?
-                        ASN1_ITEM_rptr(X509_REQ_INFO) :
-                        ASN1_ITEM_rptr(X509_CINF));
-    if (inl <= 0) {
-        X509err(X509_F_COMMON_VERIFY_SM2, ERR_R_INTERNAL_ERROR);
-        goto err;
-    }
-    if (buf_in == NULL) {
-        X509err(X509_F_COMMON_VERIFY_SM2, ERR_R_MALLOC_FAILURE);
-        goto err;
-    }
-    inll = inl;
-
-    ret = EVP_DigestVerify(ctx, signature->data,
-                           (size_t)signature->length, buf_in, inl);
-    if (ret <= 0) {
-        X509err(X509_F_COMMON_VERIFY_SM2, ERR_R_EVP_LIB);
-        goto err;
-    }
-    ret = 1;
- err:
-    OPENSSL_clear_free(buf_in, inll);
-    EVP_MD_CTX_free(ctx);
+    return ctx;
+ error:
     EVP_PKEY_CTX_free(pctx);
-    return ret;
-}
-
-static int x509_verify_sm2(X509 *x, EVP_PKEY *pkey, int mdnid, int pknid)
-{
-    return common_verify_sm2(x, pkey, mdnid, pknid, 0);
-}
-
-static int x509_req_verify_sm2(X509_REQ *x, EVP_PKEY *pkey,
-                               int mdnid, int pknid)
-{
-    return common_verify_sm2(x, pkey, mdnid, pknid, 1);
+    EVP_MD_CTX_free(ctx);
+    return NULL;
 }
 
-#endif
-
 int X509_verify(X509 *a, EVP_PKEY *r)
 {
-#ifndef OPENSSL_NO_SM2
-    int mdnid, pknid;
-#endif
+    int rv = 0;
+    EVP_MD_CTX *ctx = NULL;
+    ASN1_OCTET_STRING *id = NULL;
 
     if (X509_ALGOR_cmp(&a->sig_alg, &a->cert_info.signature))
         return 0;
 
 #ifndef OPENSSL_NO_SM2
-    /* Convert signature OID into digest and public key OIDs */
-    if (!OBJ_find_sigid_algs(OBJ_obj2nid(a->sig_alg.algorithm),
-                             &mdnid, &pknid)) {
-        X509err(X509_F_X509_VERIFY, ASN1_R_UNKNOWN_SIGNATURE_ALGORITHM);
-        return 0;
-    }
-
-    if (pknid == NID_sm2)
-        return x509_verify_sm2(a, r, mdnid, pknid);
+    id = a->sm2_id;
 #endif
 
-    return (ASN1_item_verify(ASN1_ITEM_rptr(X509_CINF), &a->sig_alg,
-                             &a->signature, &a->cert_info, r));
+    if ((ctx = make_id_ctx(r, id)) != NULL) {
+        rv = ASN1_item_verify_ctx(ASN1_ITEM_rptr(X509_CINF), &a->sig_alg,
+                                  &a->signature, &a->cert_info, ctx);
+        clean_id_ctx(ctx);
+    }
+    return rv;
 }
 
 int X509_REQ_verify(X509_REQ *a, EVP_PKEY *r)
 {
-#ifndef OPENSSL_NO_SM2
-    int mdnid, pknid;
-
-    /* Convert signature OID into digest and public key OIDs */
-    if (!OBJ_find_sigid_algs(OBJ_obj2nid(a->sig_alg.algorithm),
-                             &mdnid, &pknid)) {
-        X509err(X509_F_X509_REQ_VERIFY, ASN1_R_UNKNOWN_SIGNATURE_ALGORITHM);
-        return 0;
-    }
+    int rv = 0;
+    EVP_MD_CTX *ctx = NULL;
+    ASN1_OCTET_STRING *id = NULL;
 
-    if (pknid == NID_sm2)
-        return x509_req_verify_sm2(a, r, mdnid, pknid);
+#ifndef OPENSSL_NO_SM2
+    id = a->sm2_id;
 #endif
 
-    return (ASN1_item_verify(ASN1_ITEM_rptr(X509_REQ_INFO),
-                             &a->sig_alg, a->signature, &a->req_info, r));
+    if ((ctx = make_id_ctx(r, id)) != NULL) {
+        rv = ASN1_item_verify_ctx(ASN1_ITEM_rptr(X509_REQ_INFO), &a->sig_alg,
+                                  a->signature, &a->req_info, ctx);
+        clean_id_ctx(ctx);
+    }
+    return rv;
 }
 
 int NETSCAPE_SPKI_verify(NETSCAPE_SPKI *a, EVP_PKEY *r)
index 4cd17d2..5e553ef 100644 (file)
@@ -637,6 +637,9 @@ int ASN1_item_digest(const ASN1_ITEM *it, const EVP_MD *type, void *data,
 
 int ASN1_item_verify(const ASN1_ITEM *it, X509_ALGOR *algor1,
                      ASN1_BIT_STRING *signature, void *data, EVP_PKEY *pkey);
+int ASN1_item_verify_ctx(const ASN1_ITEM *it, X509_ALGOR *algor1,
+                         ASN1_BIT_STRING *signature, void *data,
+                         EVP_MD_CTX *ctx);
 
 int ASN1_item_sign(const ASN1_ITEM *it, X509_ALGOR *algor1,
                    X509_ALGOR *algor2, ASN1_BIT_STRING *signature, void *data,
index 8bec344..9dc3f76 100644 (file)
@@ -4917,3 +4917,4 @@ PKCS8_pkey_add1_attr                    ? 3_0_0   EXIST::FUNCTION:
 PKCS8_pkey_add1_attr_by_OBJ             ?      3_0_0   EXIST::FUNCTION:
 EVP_PKEY_private_check                  ?      3_0_0   EXIST::FUNCTION:
 EVP_PKEY_pairwise_check                 ?      3_0_0   EXIST::FUNCTION:
+ASN1_item_verify_ctx                    ?      3_0_0   EXIST::FUNCTION: