More more on SM2 error codes and tidy up

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6386)

4 files changed, 211 insertions, 311 deletions

diff --git a/crypto/sm2/sm2_crypt.c b/crypto/sm2/sm2_crypt.c
index 2a0fdceea7..568a423541 100644
--- a/crypto/sm2/sm2_crypt.c
+++ b/crypto/sm2/sm2_crypt.c
@@ -67,17 +67,17 @@ int SM2_plaintext_size(const EC_KEY *key, const EVP_MD *digest, size_t msg_len,
     size_t overhead;
 
     if (md_size < 0) {
-        SM2err(SM2_F_SM2_ENCRYPT, SM2_R_INVALID_DIGEST);
+        SM2err(SM2_F_SM2_PLAINTEXT_SIZE, SM2_R_INVALID_DIGEST);
         return 0;
     }
     if (field_size == 0) {
-        SM2err(SM2_F_SM2_ENCRYPT, SM2_R_INVALID_FIELD);
+        SM2err(SM2_F_SM2_PLAINTEXT_SIZE, SM2_R_INVALID_FIELD);
         return 0;
     }
 
     overhead = 10 + 2 * field_size + (size_t)md_size;
-    if(msg_len <= overhead) {
-        SM2err(SM2_F_SM2_ENCRYPT, SM2_R_INVALID_ENCODING);
+    if (msg_len <= overhead) {
+        SM2err(SM2_F_SM2_PLAINTEXT_SIZE, SM2_R_INVALID_ENCODING);
         return 0;
     }
 
@@ -257,20 +257,16 @@ int SM2_decrypt(const EC_KEY *key,
 {
     int rc = 0;
     int i;
-
     BN_CTX *ctx = NULL;
     const EC_GROUP *group = EC_KEY_get0_group(key);
     EC_POINT *C1 = NULL;
     struct SM2_Ciphertext_st *sm2_ctext = NULL;
     BIGNUM *x2 = NULL;
     BIGNUM *y2 = NULL;
-
     uint8_t *x2y2 = NULL;
     uint8_t *computed_C3 = NULL;
-
     const size_t field_size = EC_field_size(group);
     const int hash_size = EVP_MD_size(digest);
-
     uint8_t *msg_mask = NULL;
     const uint8_t *C2 = NULL;
     const uint8_t *C3 = NULL;
@@ -284,132 +280,94 @@ int SM2_decrypt(const EC_KEY *key,
 
     sm2_ctext = d2i_SM2_Ciphertext(NULL, &ciphertext, ciphertext_len);
 
-    if (sm2_ctext == NULL)
-        {
+    if (sm2_ctext == NULL) {
         SM2err(SM2_F_SM2_DECRYPT, SM2_R_ASN1_ERROR);
         goto done;
-        }
+    }
 
-    if (sm2_ctext->C3->length != hash_size)
-        {
+    if (sm2_ctext->C3->length != hash_size) {
         SM2err(SM2_F_SM2_DECRYPT, SM2_R_INVALID_ENCODING);
         goto done;
-        }
+    }
 
     C2 = sm2_ctext->C2->data;
     C3 = sm2_ctext->C3->data;
     msg_len = sm2_ctext->C2->length;
 
     ctx = BN_CTX_new();
-    if (ctx == NULL)
-        {
+    if (ctx == NULL) {
         SM2err(SM2_F_SM2_DECRYPT, ERR_R_MALLOC_FAILURE);
         goto done;
-        }
+    }
 
     BN_CTX_start(ctx);
     x2 = BN_CTX_get(ctx);
     y2 = BN_CTX_get(ctx);
 
-    if(y2 == NULL)
-        {
+    if (y2 == NULL) {
         SM2err(SM2_F_SM2_DECRYPT, ERR_R_BN_LIB);
         goto done;
-        }
+    }
 
     msg_mask = OPENSSL_zalloc(msg_len);
     x2y2 = OPENSSL_zalloc(2 * field_size);
     computed_C3 = OPENSSL_zalloc(hash_size);
 
-    if(msg_mask == NULL || x2y2 == NULL || computed_C3 == NULL)
-        {
+    if (msg_mask == NULL || x2y2 == NULL || computed_C3 == NULL) {
         SM2err(SM2_F_SM2_DECRYPT, ERR_R_MALLOC_FAILURE);
         goto done;
-        }
+    }
 
     C1 = EC_POINT_new(group);
-    if (C1 == NULL)
-        {
+    if (C1 == NULL) {
         SM2err(SM2_F_SM2_DECRYPT, ERR_R_MALLOC_FAILURE);
         goto done;
-        }
-
-    if (EC_POINT_set_affine_coordinates_GFp
-        (group, C1, sm2_ctext->C1x, sm2_ctext->C1y, ctx) == 0)
-        {
-        SM2err(SM2_F_SM2_DECRYPT, ERR_R_EC_LIB);
-        goto done;
-        }
-
-    if (EC_POINT_mul(group, C1, NULL, C1, EC_KEY_get0_private_key(key), ctx) ==
-        0)
-        {
-        SM2err(SM2_F_SM2_DECRYPT, ERR_R_EC_LIB);
-        goto done;
-        }
+    }
 
-    if (EC_POINT_get_affine_coordinates_GFp(group, C1, x2, y2, ctx) == 0)
-        {
+    if (!EC_POINT_set_affine_coordinates_GFp(group, C1, sm2_ctext->C1x,
+                                            sm2_ctext->C1y, ctx)
+            || !EC_POINT_mul(group, C1, NULL, C1, EC_KEY_get0_private_key(key),
+                             ctx)
+            || !EC_POINT_get_affine_coordinates_GFp(group, C1, x2, y2, ctx)) {
         SM2err(SM2_F_SM2_DECRYPT, ERR_R_EC_LIB);
         goto done;
-        }
-
-    BN_bn2binpad(x2, x2y2, field_size);
-    BN_bn2binpad(y2, x2y2 + field_size, field_size);
+    }
 
-    if (ECDH_KDF_X9_62(msg_mask, msg_len, x2y2, 2 * field_size, NULL, 0, digest)
-        == 0)
+    if (BN_bn2binpad(x2, x2y2, field_size) < 0
+            || BN_bn2binpad(y2, x2y2 + field_size, field_size) < 0
+            || !ECDH_KDF_X9_62(msg_mask, msg_len, x2y2, 2 * field_size, NULL, 0,
+                               digest)) {
+        SM2err(SM2_F_SM2_DECRYPT, ERR_R_INTERNAL_ERROR);
         goto done;
+    }
 
     for (i = 0; i != msg_len; ++i)
         ptext_buf[i] = C2[i] ^ msg_mask[i];
 
     hash = EVP_MD_CTX_new();
-
-    if (hash == NULL)
-        {
+    if (hash == NULL) {
         SM2err(SM2_F_SM2_DECRYPT, ERR_R_MALLOC_FAILURE);
         goto done;
-        }
-
-    if (EVP_DigestInit(hash, digest) == 0)
-        {
-        SM2err(SM2_F_SM2_DECRYPT, ERR_R_EVP_LIB);
-        goto done;
-        }
-
-    if (EVP_DigestUpdate(hash, x2y2, field_size) == 0)
-        {
-        SM2err(SM2_F_SM2_DECRYPT, ERR_R_EVP_LIB);
-        goto done;
-        }
-
-    if (EVP_DigestUpdate(hash, ptext_buf, msg_len) == 0)
-        {
-        SM2err(SM2_F_SM2_DECRYPT, ERR_R_EVP_LIB);
-        goto done;
-        }
-
-    if (EVP_DigestUpdate(hash, x2y2 + field_size, field_size) == 0)
-        {
-        SM2err(SM2_F_SM2_DECRYPT, ERR_R_EVP_LIB);
-        goto done;
-        }
+    }
 
-    if (EVP_DigestFinal(hash, computed_C3, NULL) == 0)
-        {
+    if (!EVP_DigestInit(hash, digest)
+            || !EVP_DigestUpdate(hash, x2y2, field_size)
+            || !EVP_DigestUpdate(hash, ptext_buf, msg_len)
+            || !EVP_DigestUpdate(hash, x2y2 + field_size, field_size)
+            || !EVP_DigestFinal(hash, computed_C3, NULL)) {
         SM2err(SM2_F_SM2_DECRYPT, ERR_R_EVP_LIB);
         goto done;
-        }
+    }
 
-    if (memcmp(computed_C3, C3, hash_size) != 0)
+    if (CRYPTO_memcmp(computed_C3, C3, hash_size) != 0) {
+        SM2err(SM2_F_SM2_DECRYPT, SM2_R_INVALID_DIGEST);
         goto done;
+    }
 
     rc = 1;
     *ptext_len = msg_len;
 
  done:
-
     if (rc == 0)
         memset(ptext_buf, 0, *ptext_len);
 
diff --git a/crypto/sm2/sm2_err.c b/crypto/sm2/sm2_err.c
index ef3f5e6b1d..c7353642b2 100644
--- a/crypto/sm2/sm2_err.c
+++ b/crypto/sm2/sm2_err.c
@@ -21,8 +21,11 @@ static const ERR_STRING_DATA SM2_str_functs[] = {
     {ERR_PACK(ERR_LIB_SM2, SM2_F_PKEY_SM2_SIGN, 0), "pkey_sm2_sign"},
     {ERR_PACK(ERR_LIB_SM2, SM2_F_SM2_COMPUTE_MSG_HASH, 0),
      "SM2_compute_msg_hash"},
+    {ERR_PACK(ERR_LIB_SM2, SM2_F_SM2_COMPUTE_USERID_DIGEST, 0),
+     "SM2_compute_userid_digest"},
     {ERR_PACK(ERR_LIB_SM2, SM2_F_SM2_DECRYPT, 0), "SM2_decrypt"},
     {ERR_PACK(ERR_LIB_SM2, SM2_F_SM2_ENCRYPT, 0), "SM2_encrypt"},
+    {ERR_PACK(ERR_LIB_SM2, SM2_F_SM2_PLAINTEXT_SIZE, 0), "SM2_plaintext_size"},
     {ERR_PACK(ERR_LIB_SM2, SM2_F_SM2_SIGN, 0), "SM2_sign"},
     {ERR_PACK(ERR_LIB_SM2, SM2_F_SM2_SIG_GEN, 0), "SM2_sig_gen"},
     {ERR_PACK(ERR_LIB_SM2, SM2_F_SM2_SIG_VERIFY, 0), "SM2_sig_verify"},
@@ -117,6 +120,7 @@ static const ERR_STRING_DATA SM2_str_reasons[] = {
     {ERR_PACK(ERR_LIB_SM2, 0, SM2_R_UNKNOWN_GROUP), "unknown group"},
     {ERR_PACK(ERR_LIB_SM2, 0, SM2_R_UNKNOWN_ORDER), "unknown order"},
     {ERR_PACK(ERR_LIB_SM2, 0, SM2_R_UNSUPPORTED_FIELD), "unsupported field"},
+    {ERR_PACK(ERR_LIB_SM2, 0, SM2_R_USER_ID_TOO_LARGE), "user id too large"},
     {ERR_PACK(ERR_LIB_SM2, 0, SM2_R_WRONG_CURVE_PARAMETERS),
     "wrong curve parameters"},
     {ERR_PACK(ERR_LIB_SM2, 0, SM2_R_WRONG_ORDER), "wrong order"},
diff --git a/crypto/sm2/sm2_sign.c b/crypto/sm2/sm2_sign.c
index 61f01eae6e..542c49f044 100644
--- a/crypto/sm2/sm2_sign.c
+++ b/crypto/sm2/sm2_sign.c
@@ -27,47 +27,33 @@ static BIGNUM *SM2_compute_msg_hash(const EVP_MD *digest,
     uint8_t *za = OPENSSL_zalloc(md_size);
     BIGNUM *e = NULL;
 
-    if (za == NULL)
-        {
+    if (hash == NULL || za == NULL) {
         SM2err(SM2_F_SM2_COMPUTE_MSG_HASH, ERR_R_MALLOC_FAILURE);
         goto done;
-        }
-
-    if (hash == NULL)
-        {
-        SM2err(SM2_F_SM2_COMPUTE_MSG_HASH, ERR_R_MALLOC_FAILURE);
-        goto done;
-        }
-
-    if (SM2_compute_userid_digest(za, digest, user_id, key) == 0)
-        goto done;
-
-    if (EVP_DigestInit(hash, digest) == 0)
-        {
-        SM2err(SM2_F_SM2_COMPUTE_MSG_HASH, ERR_R_EVP_LIB);
-        goto done;
-        }
+    }
 
-    if (EVP_DigestUpdate(hash, za, md_size) == 0)
-        {
-        SM2err(SM2_F_SM2_COMPUTE_MSG_HASH, ERR_R_EVP_LIB);
+    if (md_size < 0) {
+        SM2err(SM2_F_SM2_COMPUTE_MSG_HASH, SM2_R_INVALID_DIGEST);
         goto done;
-        }
+    }
 
-    if (EVP_DigestUpdate(hash, msg, msg_len) == 0)
-        {
-        SM2err(SM2_F_SM2_COMPUTE_MSG_HASH, ERR_R_EVP_LIB);
+    if (!SM2_compute_userid_digest(za, digest, user_id, key)) {
+        /* SM2err already called */
         goto done;
-        }
+    }
 
-    /* reuse za buffer to hold H(ZA || M) */
-    if (EVP_DigestFinal(hash, za, NULL) == 0)
-        {
+    if (!EVP_DigestInit(hash, digest)
+            || !EVP_DigestUpdate(hash, za, md_size)
+            || !EVP_DigestUpdate(hash, msg, msg_len)
+               /* reuse za buffer to hold H(ZA || M) */
+            || !EVP_DigestFinal(hash, za, NULL)) {
         SM2err(SM2_F_SM2_COMPUTE_MSG_HASH, ERR_R_EVP_LIB);
         goto done;
-        }
+    }
 
     e = BN_bin2bn(za, md_size, NULL);
+    if (e == NULL)
+        SM2err(SM2_F_SM2_COMPUTE_MSG_HASH, ERR_R_INTERNAL_ERROR);
 
  done:
     OPENSSL_free(za);
@@ -75,13 +61,11 @@ static BIGNUM *SM2_compute_msg_hash(const EVP_MD *digest,
     return e;
 }
 
-static
-ECDSA_SIG *SM2_sig_gen(const EC_KEY *key, const BIGNUM *e)
+static ECDSA_SIG *SM2_sig_gen(const EC_KEY *key, const BIGNUM *e)
 {
     const BIGNUM *dA = EC_KEY_get0_private_key(key);
     const EC_GROUP *group = EC_KEY_get0_group(key);
     const BIGNUM *order = EC_GROUP_get0_order(group);
-
     ECDSA_SIG *sig = NULL;
     EC_POINT *kG = NULL;
     BN_CTX *ctx = NULL;
@@ -93,109 +77,75 @@ ECDSA_SIG *SM2_sig_gen(const EC_KEY *key, const BIGNUM *e)
     BIGNUM *tmp = NULL;
 
     kG = EC_POINT_new(group);
-    if (kG == NULL)
-        {
-        SM2err(SM2_F_SM2_SIG_GEN, ERR_R_MALLOC_FAILURE);
-        goto done;
-        }
-
     ctx = BN_CTX_new();
-    if (ctx == NULL)
-        {
+    if (kG == NULL || ctx == NULL) {
         SM2err(SM2_F_SM2_SIG_GEN, ERR_R_MALLOC_FAILURE);
         goto done;
-        }
+    }
 
-    BN_CTX_start(ctx);
 
+    BN_CTX_start(ctx);
     k = BN_CTX_get(ctx);
     rk = BN_CTX_get(ctx);
     x1 = BN_CTX_get(ctx);
     tmp = BN_CTX_get(ctx);
-
-    if (tmp == NULL)
-        {
-        SM2err(SM2_F_SM2_SIG_GEN, ERR_R_BN_LIB);
+    if (tmp == NULL) {
+        SM2err(SM2_F_SM2_SIG_GEN, ERR_R_MALLOC_FAILURE);
         goto done;
-        }
+    }
 
-    /* These values are returned and so should not be allocated out of the context */
+    /*
+     * These values are returned and so should not be allocated out of the
+     * context
+     */
     r = BN_new();
     s = BN_new();
 
-    if (r == NULL || s == NULL)
-        {
+    if (r == NULL || s == NULL) {
         SM2err(SM2_F_SM2_SIG_GEN, ERR_R_MALLOC_FAILURE);
         goto done;
-        }
+    }
 
     for (;;) {
-        BN_priv_rand_range(k, order);
-
-        if (EC_POINT_mul(group, kG, k, NULL, NULL, ctx) == 0)
-            {
-            SM2err(SM2_F_SM2_SIG_GEN, ERR_R_EC_LIB);
-            goto done;
-            }
+        if (!BN_priv_rand_range(k, order)) {
+            SM2err(SM2_F_SM2_SIG_GEN, ERR_R_INTERNAL_ERROR);
+                goto done;
+        }
 
-        if (EC_POINT_get_affine_coordinates_GFp(group, kG, x1, NULL, ctx) == 0)
-            {
-            SM2err(SM2_F_SM2_SIG_GEN, ERR_R_EC_LIB);
+        if (!EC_POINT_mul(group, kG, k, NULL, NULL, ctx)
+                || !EC_POINT_get_affine_coordinates_GFp(group, kG, x1, NULL,
+                                                        ctx)
+                || !BN_mod_add(r, e, x1, order, ctx)) {
+            SM2err(SM2_F_SM2_SIG_GEN, ERR_R_INTERNAL_ERROR);
             goto done;
-            }
-
-        if (BN_mod_add(r, e, x1, order, ctx) == 0)
-            {
-            SM2err(SM2_F_SM2_SIG_GEN, ERR_R_BN_LIB);
-            goto done;
-            }
+        }
 
         /* try again if r == 0 or r+k == n */
         if (BN_is_zero(r))
             continue;
 
-        BN_add(rk, r, k);
+        if (!BN_add(rk, r, k)) {
+            SM2err(SM2_F_SM2_SIG_GEN, ERR_R_INTERNAL_ERROR);
+            goto done;
+        }
 
         if (BN_cmp(rk, order) == 0)
             continue;
 
-        if(BN_add(s, dA, BN_value_one()) == 0)
-           {
-           SM2err(SM2_F_SM2_SIG_GEN, ERR_R_BN_LIB);
-           goto done;
-           }
-
-        if(BN_mod_inverse(s, s, order, ctx) == 0)
-           {
-           SM2err(SM2_F_SM2_SIG_GEN, ERR_R_BN_LIB);
-           goto done;
-           }
-
-        if(BN_mod_mul(tmp, dA, r, order, ctx) == 0)
-           {
-           SM2err(SM2_F_SM2_SIG_GEN, ERR_R_BN_LIB);
-           goto done;
-           }
-
-        if(BN_sub(tmp, k, tmp) == 0)
-           {
-           SM2err(SM2_F_SM2_SIG_GEN, ERR_R_BN_LIB);
-           goto done;
-           }
-
-        if(BN_mod_mul(s, s, tmp, order, ctx) == 0)
-           {
-           SM2err(SM2_F_SM2_SIG_GEN, ERR_R_BN_LIB);
-           goto done;
-           }
+        if (!BN_add(s, dA, BN_value_one())
+                || !BN_mod_inverse(s, s, order, ctx)
+                || !BN_mod_mul(tmp, dA, r, order, ctx)
+                || !BN_sub(tmp, k, tmp)
+                || !BN_mod_mul(s, s, tmp, order, ctx)) {
+            SM2err(SM2_F_SM2_SIG_GEN, ERR_R_BN_LIB);
+            goto done;
+        }
 
         sig = ECDSA_SIG_new();
-
-        if (sig == NULL)
-            {
+        if (sig == NULL) {
             SM2err(SM2_F_SM2_SIG_GEN, ERR_R_MALLOC_FAILURE);
             goto done;
-            }
+        }
 
          /* takes ownership of r and s */
         ECDSA_SIG_set0(sig, r, s);
@@ -203,7 +153,6 @@ ECDSA_SIG *SM2_sig_gen(const EC_KEY *key, const BIGNUM *e)
     }
 
  done:
-
     if (sig == NULL) {
         BN_free(r);
         BN_free(s);
@@ -212,101 +161,76 @@ ECDSA_SIG *SM2_sig_gen(const EC_KEY *key, const BIGNUM *e)
     BN_CTX_free(ctx);
     EC_POINT_free(kG);
     return sig;
-
 }
 
-static
-int SM2_sig_verify(const EC_KEY *key, const ECDSA_SIG *sig, const BIGNUM *e)
+static int SM2_sig_verify(const EC_KEY *key, const ECDSA_SIG *sig,
+                          const BIGNUM *e)
 {
     int ret = 0;
     const EC_GROUP *group = EC_KEY_get0_group(key);
     const BIGNUM *order = EC_GROUP_get0_order(group);
     BN_CTX *ctx = NULL;
     EC_POINT *pt = NULL;
-
     BIGNUM *t = NULL;
     BIGNUM *x1 = NULL;
     const BIGNUM *r = NULL;
     const BIGNUM *s = NULL;
 
     ctx = BN_CTX_new();
-    if (ctx == NULL)
-        {
-        SM2err(SM2_F_SM2_SIG_VERIFY, ERR_R_MALLOC_FAILURE);
-        goto done;
-        }
-
     pt = EC_POINT_new(group);
-    if (pt == NULL)
-        {
+    if (ctx == NULL || pt == NULL) {
         SM2err(SM2_F_SM2_SIG_VERIFY, ERR_R_MALLOC_FAILURE);
         goto done;
-        }
+    }
 
     BN_CTX_start(ctx);
-
     t = BN_CTX_get(ctx);
     x1 = BN_CTX_get(ctx);
-
-    if (x1 == NULL)
-       {
-       SM2err(SM2_F_SM2_SIG_VERIFY, ERR_R_MALLOC_FAILURE);
-       goto done;
-       }
+    if (x1 == NULL) {
+        SM2err(SM2_F_SM2_SIG_VERIFY, ERR_R_MALLOC_FAILURE);
+        goto done;
+    }
 
     /*
-       B1: verify whether r' in [1,n-1], verification failed if not
-       B2: vefify whether s' in [1,n-1], verification failed if not
-       B3: set M'~=ZA || M'
-       B4: calculate e'=Hv(M'~)
-       B5: calculate t = (r' + s') modn, verification failed if t=0
-       B6: calculate the point (x1', y1')=[s']G + [t]PA
-       B7: calculate R=(e'+x1') modn, verfication pass if yes, otherwise failed
+     * B1: verify whether r' in [1,n-1], verification failed if not
+     * B2: vefify whether s' in [1,n-1], verification failed if not
+     * B3: set M'~=ZA || M'
+     * B4: calculate e'=Hv(M'~)
+     * B5: calculate t = (r' + s') modn, verification failed if t=0
+     * B6: calculate the point (x1', y1')=[s']G + [t]PA
+     * B7: calculate R=(e'+x1') modn, verfication pass if yes, otherwise failed
      */
 
     ECDSA_SIG_get0(sig, &r, &s);
 
-    if ((BN_cmp(r, BN_value_one()) < 0) || (BN_cmp(s, BN_value_one()) < 0))
-        {
+    if (BN_cmp(r, BN_value_one()) < 0
+            || BN_cmp(s, BN_value_one()) < 0
+            || BN_cmp(order, r) <= 0
+            || BN_cmp(order, s) <= 0) {
         SM2err(SM2_F_SM2_SIG_VERIFY, SM2_R_BAD_SIGNATURE);
         goto done;
-        }
-
-    if ((BN_cmp(order, r) <= 0) || (BN_cmp(order, s) <= 0))
-        {
-        SM2err(SM2_F_SM2_SIG_VERIFY, SM2_R_BAD_SIGNATURE);
-        goto done;
-        }
+    }
 
-    if (BN_mod_add(t, r, s, order, ctx) == 0)
-        {
+    if (!BN_mod_add(t, r, s, order, ctx)) {
         SM2err(SM2_F_SM2_SIG_VERIFY, ERR_R_BN_LIB);
         goto done;
-        }
+    }
 
-    if (BN_is_zero(t) == 1)
-        {
+    if (BN_is_zero(t)) {
         SM2err(SM2_F_SM2_SIG_VERIFY, SM2_R_BAD_SIGNATURE);
         goto done;
-        }
-
-    if (EC_POINT_mul(group, pt, s, EC_KEY_get0_public_key(key), t, ctx) == 0)
-        {
-        SM2err(SM2_F_SM2_SIG_VERIFY, ERR_R_EC_LIB);
-        goto done;
-        }
+    }
 
-    if (EC_POINT_get_affine_coordinates_GFp(group, pt, x1, NULL, ctx) == 0)
-        {
+    if (!EC_POINT_mul(group, pt, s, EC_KEY_get0_public_key(key), t, ctx)
+            || !EC_POINT_get_affine_coordinates_GFp(group, pt, x1, NULL, ctx)) {
         SM2err(SM2_F_SM2_SIG_VERIFY, ERR_R_EC_LIB);
         goto done;
-        }
+    }
 
-    if (BN_mod_add(t, e, x1, order, ctx) == 0)
-        {
+    if (!BN_mod_add(t, e, x1, order, ctx)) {
         SM2err(SM2_F_SM2_SIG_VERIFY, ERR_R_BN_LIB);
         goto done;
-        }
+    }
 
     if (BN_cmp(r, t) == 0)
         ret = 1;
@@ -325,8 +249,10 @@ ECDSA_SIG *SM2_do_sign(const EC_KEY *key,
     ECDSA_SIG *sig = NULL;
 
     e = SM2_compute_msg_hash(digest, key, user_id, msg, msg_len);
-    if (e == NULL)
+    if (e == NULL) {
+        /* SM2err already called */
         goto done;
+    }
 
     sig = SM2_sig_gen(key, e);
 
@@ -341,11 +267,13 @@ int SM2_do_verify(const EC_KEY *key,
                   const char *user_id, const uint8_t *msg, size_t msg_len)
 {
     BIGNUM *e = NULL;
-    int ret = -1;
+    int ret = 0;
 
     e = SM2_compute_msg_hash(digest, key, user_id, msg, msg_len);
-    if (e == NULL)
+    if (e == NULL) {
+        /* SM2err already called */
         goto done;
+    }
 
     ret = SM2_sig_verify(key, sig, e);
 
@@ -359,19 +287,28 @@ int SM2_sign(int type, const unsigned char *dgst, int dgstlen,
 {
     BIGNUM *e = NULL;
     ECDSA_SIG *s = NULL;
+    int sigleni;
     int ret = -1;
 
-    if (type != NID_sm3 || dgstlen != 32)
-       {
+    if (type != NID_sm3 || dgstlen != 32) {
        SM2err(SM2_F_SM2_SIGN, SM2_R_INVALID_DIGEST_TYPE);
        goto done;
-       }
+   }
 
     e = BN_bin2bn(dgst, dgstlen, NULL);
+    if (e == NULL) {
+       SM2err(SM2_F_SM2_SIGN, ERR_R_BN_LIB);
+       goto done;
+    }
 
     s = SM2_sig_gen(eckey, e);
 
-    *siglen = i2d_ECDSA_SIG(s, &sig);
+    sigleni = i2d_ECDSA_SIG(s, &sig);
+    if (sigleni < 0) {
+       SM2err(SM2_F_SM2_SIGN, ERR_R_INTERNAL_ERROR);
+       goto done;
+    }
+    *siglen = (unsigned int)sigleni;
 
     ret = 1;
 
@@ -391,32 +328,32 @@ int SM2_verify(int type, const unsigned char *dgst, int dgstlen,
     int derlen = -1;
     int ret = -1;
 
-    if (type != NID_sm3)
-       {
+    if (type != NID_sm3) {
        SM2err(SM2_F_SM2_VERIFY, SM2_R_INVALID_DIGEST_TYPE);
        goto done;
-       }
+   }
 
     s = ECDSA_SIG_new();
-    if (s == NULL)
-        {
+    if (s == NULL) {
         SM2err(SM2_F_SM2_VERIFY, ERR_R_MALLOC_FAILURE);
         goto done;
-        }
-    if (d2i_ECDSA_SIG(&s, &p, sig_len) == NULL)
-        {
+    }
+    if (d2i_ECDSA_SIG(&s, &p, sig_len) == NULL) {
         SM2err(SM2_F_SM2_VERIFY, SM2_R_INVALID_ENCODING);
         goto done;
-        }
+    }
     /* Ensure signature uses DER and doesn't have trailing garbage */
     derlen = i2d_ECDSA_SIG(s, &der);
-    if (derlen != sig_len || memcmp(sig, der, derlen) != 0)
-        {
+    if (derlen != sig_len || memcmp(sig, der, derlen) != 0) {
         SM2err(SM2_F_SM2_VERIFY, SM2_R_INVALID_ENCODING);
         goto done;
-        }
+    }
 
     e = BN_bin2bn(dgst, dgstlen, NULL);
+    if (e == NULL) {
+        SM2err(SM2_F_SM2_VERIFY, ERR_R_BN_LIB);
+        goto done;
+    }
 
     ret = SM2_sig_verify(eckey, s, e);
 
diff --git a/crypto/sm2/sm2_za.c b/crypto/sm2/sm2_za.c
index cf355238b2..9b1f70cddd 100644
--- a/crypto/sm2/sm2_za.c
+++ b/crypto/sm2/sm2_za.c
@@ -10,6 +10,8 @@
  */
 
 #include "internal/sm2.h"
+#include "internal/sm2err.h"
+#include <openssl/err.h>
 #include <openssl/evp.h>
 #include <openssl/bn.h>
 #include <string.h>
@@ -20,21 +22,16 @@ int SM2_compute_userid_digest(uint8_t *out,
                               const EC_KEY *key)
 {
     int rc = 0;
-
     const EC_GROUP *group = EC_KEY_get0_group(key);
-
     BN_CTX *ctx = NULL;
     EVP_MD_CTX *hash = NULL;
-
     BIGNUM *p = NULL;
     BIGNUM *a = NULL;
     BIGNUM *b = NULL;
-
     BIGNUM *xG = NULL;
     BIGNUM *yG = NULL;
     BIGNUM *xA = NULL;
     BIGNUM *yA = NULL;
-
     int p_bytes = 0;
     uint8_t *buf = NULL;
     size_t uid_len = 0;
@@ -42,12 +39,11 @@ int SM2_compute_userid_digest(uint8_t *out,
     uint8_t e_byte = 0;
 
     hash = EVP_MD_CTX_new();
-    if (hash == NULL)
-       goto done;
-
     ctx = BN_CTX_new();
-    if (ctx == NULL)
-       goto done;
+    if (hash == NULL || ctx == NULL) {
+        SM2err(SM2_F_SM2_COMPUTE_USERID_DIGEST, ERR_R_MALLOC_FAILURE);
+        goto done;
+    }
 
     p = BN_CTX_get(ctx);
     a = BN_CTX_get(ctx);
@@ -57,70 +53,75 @@ int SM2_compute_userid_digest(uint8_t *out,
     xA = BN_CTX_get(ctx);
     yA = BN_CTX_get(ctx);
 
-    if (p == NULL || a == NULL || b == NULL ||
-        xG == NULL || yG == NULL || xA == NULL || yA == NULL)
-       goto done;
+    if (yA == NULL) {
+        SM2err(SM2_F_SM2_COMPUTE_USERID_DIGEST, ERR_R_MALLOC_FAILURE);
+        goto done;
+    }
 
     memset(out, 0, EVP_MD_size(digest));
 
-    if (EVP_DigestInit(hash, digest) == 0)
+    if (!EVP_DigestInit(hash, digest)) {
+        SM2err(SM2_F_SM2_COMPUTE_USERID_DIGEST, ERR_R_EVP_LIB);
         goto done;
+    }
 
-    /*
-       ZA=H256(ENTLA || IDA || a || b || xG || yG || xA || yA)
-     */
+    /* ZA = H256(ENTLA || IDA || a || b || xG || yG || xA || yA) */
 
     uid_len = strlen(user_id);
-
-    if (uid_len >= 8192)        /* too large */
+    if (uid_len >= (UINT16_MAX / 8)) {
+        /* too large */
+        SM2err(SM2_F_SM2_COMPUTE_USERID_DIGEST, SM2_R_USER_ID_TOO_LARGE);
         goto done;
+    }
 
-    entla = (unsigned short)(8 * uid_len);
+    entla = (uint16_t)(8 * uid_len);
 
     e_byte = entla >> 8;
-    if (EVP_DigestUpdate(hash, &e_byte, 1) == 0)
+    if (!EVP_DigestUpdate(hash, &e_byte, 1)) {
+        SM2err(SM2_F_SM2_COMPUTE_USERID_DIGEST, ERR_R_EVP_LIB);
         goto done;
+    }
     e_byte = entla & 0xFF;
-    if (EVP_DigestUpdate(hash, &e_byte, 1) == 0)
+    if (!EVP_DigestUpdate(hash, &e_byte, 1)
+            || !EVP_DigestUpdate(hash, user_id, uid_len)) {
+        SM2err(SM2_F_SM2_COMPUTE_USERID_DIGEST, ERR_R_EVP_LIB);
         goto done;
+    }
 
-    if (EVP_DigestUpdate(hash, user_id, uid_len) == 0)
-        goto done;
-
-    if (EC_GROUP_get_curve_GFp(group, p, a, b, ctx) == 0)
+    if (!EC_GROUP_get_curve_GFp(group, p, a, b, ctx)) {
+        SM2err(SM2_F_SM2_COMPUTE_USERID_DIGEST, ERR_R_EC_LIB);
         goto done;
+    }
 
     p_bytes = BN_num_bytes(p);
     buf = OPENSSL_zalloc(p_bytes);
-
-    BN_bn2binpad(a, buf, p_bytes);
-    if (EVP_DigestUpdate(hash, buf, p_bytes) == 0)
-        goto done;
-    BN_bn2binpad(b, buf, p_bytes);
-    if (EVP_DigestUpdate(hash, buf, p_bytes) == 0)
-        goto done;
-    EC_POINT_get_affine_coordinates_GFp(group,
-                                        EC_GROUP_get0_generator(group),
-                                        xG, yG, ctx);
-    BN_bn2binpad(xG, buf, p_bytes);
-    if (EVP_DigestUpdate(hash, buf, p_bytes) == 0)
-        goto done;
-    BN_bn2binpad(yG, buf, p_bytes);
-    if (EVP_DigestUpdate(hash, buf, p_bytes) == 0)
+    if (buf == NULL) {
+        SM2err(SM2_F_SM2_COMPUTE_USERID_DIGEST, ERR_R_MALLOC_FAILURE);
         goto done;
-
-    EC_POINT_get_affine_coordinates_GFp(group,
-                                        EC_KEY_get0_public_key(key),
-                                        xA, yA, ctx);
-    BN_bn2binpad(xA, buf, p_bytes);
-    if (EVP_DigestUpdate(hash, buf, p_bytes) == 0)
-        goto done;
-    BN_bn2binpad(yA, buf, p_bytes);
-    if (EVP_DigestUpdate(hash, buf, p_bytes) == 0)
-        goto done;
-
-    if (EVP_DigestFinal(hash, out, NULL) == 0)
+    }
+
+    if (BN_bn2binpad(a, buf, p_bytes) < 0
+            || !EVP_DigestUpdate(hash, buf, p_bytes)
+            || BN_bn2binpad(b, buf, p_bytes) < 0
+            || !EVP_DigestUpdate(hash, buf, p_bytes)
+            || !EC_POINT_get_affine_coordinates_GFp(group,
+                                                EC_GROUP_get0_generator(group),
+                                                xG, yG, ctx)
+            || BN_bn2binpad(xG, buf, p_bytes) < 0
+            || !EVP_DigestUpdate(hash, buf, p_bytes)
+            || BN_bn2binpad(yG, buf, p_bytes) < 0
+            || !EVP_DigestUpdate(hash, buf, p_bytes)
+            || !EC_POINT_get_affine_coordinates_GFp(group,
+                                                    EC_KEY_get0_public_key(key),
+                                                    xA, yA, ctx)
+            || BN_bn2binpad(xA, buf, p_bytes) < 0
+            || !EVP_DigestUpdate(hash, buf, p_bytes)
+            || BN_bn2binpad(yA, buf, p_bytes) < 0
+            || !EVP_DigestUpdate(hash, buf, p_bytes)
+            || !EVP_DigestFinal(hash, out, NULL)) {
+        SM2err(SM2_F_SM2_COMPUTE_USERID_DIGEST, ERR_R_INTERNAL_ERROR);
         goto done;
+    }
 
     rc = 1;