asn1/x_algor.c: add internal ossl_X509_ALGOR_from_nid() simplifying code

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/17363)

6 files changed, 45 insertions, 46 deletions

diff --git a/crypto/asn1/p5_pbev2.c b/crypto/asn1/p5_pbev2.c
index 711743a77b..82292626de 100644
--- a/crypto/asn1/p5_pbev2.c
+++ b/crypto/asn1/p5_pbev2.c
@@ -9,6 +9,7 @@
 
 #include <stdio.h>
 #include "internal/cryptlib.h"
+#include "crypto/asn1.h"
 #include <openssl/asn1t.h>
 #include <openssl/core.h>
 #include <openssl/core_names.h>
@@ -208,10 +209,9 @@ X509_ALGOR *PKCS5_pbkdf2_set_ex(int iter, unsigned char *salt, int saltlen,
 
     /* prf can stay NULL if we are using hmacWithSHA1 */
     if (prf_nid > 0 && prf_nid != NID_hmacWithSHA1) {
-        kdf->prf = X509_ALGOR_new();
+        kdf->prf = ossl_X509_ALGOR_from_nid(prf_nid, V_ASN1_NULL, NULL);
         if (kdf->prf == NULL)
             goto merr;
-        X509_ALGOR_set0(kdf->prf, OBJ_nid2obj(prf_nid), V_ASN1_NULL, NULL);
     }
 
     /* Finally setup the keyfunc structure */
diff --git a/crypto/asn1/x_algor.c b/crypto/asn1/x_algor.c
index c0a5f76803..f56ec92f65 100644
--- a/crypto/asn1/x_algor.c
+++ b/crypto/asn1/x_algor.c
@@ -43,7 +43,7 @@ int X509_ALGOR_set0(X509_ALGOR *alg, ASN1_OBJECT *aobj, int ptype, void *pval)
     ASN1_OBJECT_free(alg->algorithm);
     alg->algorithm = aobj;
 
-    if (ptype == 0)
+    if (ptype == V_ASN1_EOC)
         return 1;
     if (ptype == V_ASN1_UNDEF) {
         ASN1_TYPE_free(alg->parameter);
@@ -53,6 +53,25 @@ int X509_ALGOR_set0(X509_ALGOR *alg, ASN1_OBJECT *aobj, int ptype, void *pval)
     return 1;
 }
 
+X509_ALGOR *ossl_X509_ALGOR_from_nid(int nid, int ptype, void *pval)
+{
+    ASN1_OBJECT *algo = OBJ_nid2obj(nid);
+    X509_ALGOR *alg = NULL;
+
+    if (algo == NULL)
+        return NULL;
+    if ((alg = X509_ALGOR_new()) == NULL)
+        goto err;
+    if (X509_ALGOR_set0(alg, algo, ptype, pval))
+        return alg;
+    alg->algorithm = NULL; /* precaution to prevent double free */
+
+ err:
+    X509_ALGOR_free(alg);
+    ASN1_OBJECT_free(algo);
+    return NULL;
+}
+
 void X509_ALGOR_get0(const ASN1_OBJECT **paobj, int *pptype,
                      const void **ppval, const X509_ALGOR *algor)
 {
@@ -176,15 +195,12 @@ int ossl_x509_algor_md_to_mgf1(X509_ALGOR **palg, const EVP_MD *mgf1md)
         goto err;
     if (ASN1_item_pack(algtmp, ASN1_ITEM_rptr(X509_ALGOR), &stmp) == NULL)
          goto err;
-    *palg = X509_ALGOR_new();
+    *palg = ossl_X509_ALGOR_from_nid(NID_mgf1, V_ASN1_SEQUENCE, stmp);
     if (*palg == NULL)
         goto err;
-    X509_ALGOR_set0(*palg, OBJ_nid2obj(NID_mgf1), V_ASN1_SEQUENCE, stmp);
     stmp = NULL;
  err:
     ASN1_STRING_free(stmp);
     X509_ALGOR_free(algtmp);
-    if (*palg != NULL)
-        return 1;
-    return 0;
+    return *palg != NULL;
 }
diff --git a/crypto/cmp/cmp_protect.c b/crypto/cmp/cmp_protect.c
index a7ca580cc9..a35944f2c2 100644
--- a/crypto/cmp/cmp_protect.c
+++ b/crypto/cmp/cmp_protect.c
@@ -10,6 +10,7 @@
  */
 
 #include "cmp_local.h"
+#include "crypto/asn1.h"
 
 /* explicit #includes not strictly needed since implied by the above: */
 #include <openssl/asn1t.h>
@@ -184,15 +185,16 @@ int ossl_cmp_msg_add_extraCerts(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg)
  * Create an X509_ALGOR structure for PasswordBasedMAC protection based on
  * the pbm settings in the context
  */
-static int set_pbmac_algor(const OSSL_CMP_CTX *ctx, X509_ALGOR **alg)
+static X509_ALGOR *pbmac_algor(const OSSL_CMP_CTX *ctx)
 {
     OSSL_CRMF_PBMPARAMETER *pbm = NULL;
     unsigned char *pbm_der = NULL;
     int pbm_der_len;
     ASN1_STRING *pbm_str = NULL;
+    X509_ALGOR *alg = NULL;
 
     if (!ossl_assert(ctx != NULL))
-        return 0;
+        return NULL;
 
     pbm = OSSL_CRMF_pbmp_new(ctx->libctx, ctx->pbm_slen,
                              EVP_MD_get_type(ctx->pbm_owf), ctx->pbm_itercnt,
@@ -200,47 +202,30 @@ static int set_pbmac_algor(const OSSL_CMP_CTX *ctx, X509_ALGOR **alg)
     pbm_str = ASN1_STRING_new();
     if (pbm == NULL || pbm_str == NULL)
         goto err;
-
     if ((pbm_der_len = i2d_OSSL_CRMF_PBMPARAMETER(pbm, &pbm_der)) < 0)
         goto err;
-
     if (!ASN1_STRING_set(pbm_str, pbm_der, pbm_der_len))
         goto err;
-    if (*alg == NULL && (*alg = X509_ALGOR_new()) == NULL)
-        goto err;
-    OPENSSL_free(pbm_der);
-
-    X509_ALGOR_set0(*alg, OBJ_nid2obj(NID_id_PasswordBasedMAC),
-                    V_ASN1_SEQUENCE, pbm_str);
-    OSSL_CRMF_PBMPARAMETER_free(pbm);
-    return 1;
-
+    alg = ossl_X509_ALGOR_from_nid(NID_id_PasswordBasedMAC,
+                                   V_ASN1_SEQUENCE, pbm_str);
  err:
-    ASN1_STRING_free(pbm_str);
+    if (alg == NULL)
+        ASN1_STRING_free(pbm_str);
     OPENSSL_free(pbm_der);
     OSSL_CRMF_PBMPARAMETER_free(pbm);
-    return 0;
+    return alg;
 }
 
-static int set_sig_algor(const OSSL_CMP_CTX *ctx, X509_ALGOR **alg)
+static X509_ALGOR *sig_algor(const OSSL_CMP_CTX *ctx)
 {
     int nid = 0;
-    ASN1_OBJECT *algo = NULL;
 
     if (!OBJ_find_sigid_by_algs(&nid, EVP_MD_get_type(ctx->digest),
                                 EVP_PKEY_get_id(ctx->pkey))) {
         ERR_raise(ERR_LIB_CMP, CMP_R_UNSUPPORTED_KEY_TYPE);
         return 0;
     }
-    if ((algo = OBJ_nid2obj(nid)) == NULL)
-        return 0;
-    if (*alg == NULL && (*alg = X509_ALGOR_new()) == NULL)
-        return 0;
-
-    if (X509_ALGOR_set0(*alg, algo, V_ASN1_UNDEF, NULL))
-        return 1;
-    ASN1_OBJECT_free(algo);
-    return 0;
+    return ossl_X509_ALGOR_from_nid(nid, V_ASN1_UNDEF, NULL);
 }
 
 static int set_senderKID(const OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg,
@@ -269,7 +254,7 @@ int ossl_cmp_msg_protect(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg)
             goto err;
     } else if (ctx->secretValue != NULL) {
         /* use PasswordBasedMac according to 5.1.3.1 if secretValue is given */
-        if (!set_pbmac_algor(ctx, &msg->header->protectionAlg))
+        if ((msg->header->protectionAlg = pbmac_algor(ctx)) == NULL)
             goto err;
         if (!set_senderKID(ctx, msg, NULL))
             goto err;
@@ -288,7 +273,7 @@ int ossl_cmp_msg_protect(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg)
             goto err;
         }
 
-        if (!set_sig_algor(ctx, &msg->header->protectionAlg))
+        if ((msg->header->protectionAlg = sig_algor(ctx)) == NULL)
             goto err;
         /* set senderKID to keyIdentifier of the cert according to 5.1.1 */
         if (!set_senderKID(ctx, msg, X509_get0_subject_key_id(ctx->cert)))
diff --git a/crypto/cms/cms_rsa.c b/crypto/cms/cms_rsa.c
index 20ed816918..eafa1788de 100644
--- a/crypto/cms/cms_rsa.c
+++ b/crypto/cms/cms_rsa.c
@@ -145,20 +145,18 @@ static int rsa_cms_encrypt(CMS_RecipientInfo *ri)
     if (!ossl_x509_algor_md_to_mgf1(&oaep->maskGenFunc, mgf1md))
         goto err;
     if (labellen > 0) {
-        ASN1_OCTET_STRING *los;
+        ASN1_OCTET_STRING *los = ASN1_OCTET_STRING_new();
 
-        oaep->pSourceFunc = X509_ALGOR_new();
-        if (oaep->pSourceFunc == NULL)
-            goto err;
-        los = ASN1_OCTET_STRING_new();
         if (los == NULL)
             goto err;
         if (!ASN1_OCTET_STRING_set(los, label, labellen)) {
             ASN1_OCTET_STRING_free(los);
             goto err;
         }
-        X509_ALGOR_set0(oaep->pSourceFunc, OBJ_nid2obj(NID_pSpecified),
-                        V_ASN1_OCTET_STRING, los);
+        oaep->pSourceFunc = ossl_X509_ALGOR_from_nid(NID_pSpecified,
+                                                     V_ASN1_OCTET_STRING, los);
+        if (oaep->pSourceFunc == NULL)
+            goto err;
     }
     /* create string with pss parameter encoding. */
     if (!ASN1_item_pack(oaep, ASN1_ITEM_rptr(RSA_OAEP_PARAMS), &os))
diff --git a/crypto/cms/cms_sd.c b/crypto/cms/cms_sd.c
index 34c021bba6..7a77a0870a 100644
--- a/crypto/cms/cms_sd.c
+++ b/crypto/cms/cms_sd.c
@@ -1039,14 +1039,13 @@ int CMS_add_simple_smimecap(STACK_OF(X509_ALGOR) **algs,
             return 0;
         }
     }
-    alg = X509_ALGOR_new();
+    alg = ossl_X509_ALGOR_from_nid(algnid, key != NULL ? V_ASN1_INTEGER :
+                                   V_ASN1_UNDEF, key);
     if (alg == NULL) {
         ASN1_INTEGER_free(key);
         return 0;
     }
 
-    X509_ALGOR_set0(alg, OBJ_nid2obj(algnid),
-                    key ? V_ASN1_INTEGER : V_ASN1_UNDEF, key);
     if (*algs == NULL)
         *algs = sk_X509_ALGOR_new_null();
     if (*algs == NULL || !sk_X509_ALGOR_push(*algs, alg)) {
diff --git a/include/crypto/asn1.h b/include/crypto/asn1.h
index ec76ae6fc6..ff02cac573 100644
--- a/include/crypto/asn1.h
+++ b/include/crypto/asn1.h
@@ -145,5 +145,6 @@ int ossl_asn1_time_print_ex(BIO *bp, const ASN1_TIME *tm, unsigned long flags);
 EVP_PKEY * ossl_d2i_PrivateKey_legacy(int keytype, EVP_PKEY **a,
                                       const unsigned char **pp, long length,
                                       OSSL_LIB_CTX *libctx, const char *propq);
+X509_ALGOR *ossl_X509_ALGOR_from_nid(int nid, int ptype, void *pval);
 
 #endif /* ndef OSSL_CRYPTO_ASN1_H */