Allow duplicate CMS attributes

Fixes regression introduced with https://github.com/openssl/openssl/pull/21505

Fixes #22266

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23029)

(cherry picked from commit d7e707cb4983a35b1a265c6042da410d829f3b19)

3 files changed, 106 insertions, 23 deletions

diff --git a/crypto/cms/cms_att.c b/crypto/cms/cms_att.c
index 5b99516b29..6c7fb349f5 100644
--- a/crypto/cms/cms_att.c
+++ b/crypto/cms/cms_att.c
@@ -12,8 +12,9 @@
 #include <openssl/x509v3.h>
 #include <openssl/err.h>
 #include <openssl/cms.h>
-#include "cms_local.h"
 #include "internal/nelem.h"
+#include "crypto/x509.h"
+#include "cms_local.h"
 
 /*-
  * Attribute flags.
@@ -94,7 +95,7 @@ X509_ATTRIBUTE *CMS_signed_delete_attr(CMS_SignerInfo *si, int loc)
 
 int CMS_signed_add1_attr(CMS_SignerInfo *si, X509_ATTRIBUTE *attr)
 {
-    if (X509at_add1_attr(&si->signedAttrs, attr))
+    if (ossl_x509at_add1_attr(&si->signedAttrs, attr))
         return 1;
     return 0;
 }
@@ -103,7 +104,7 @@ int CMS_signed_add1_attr_by_OBJ(CMS_SignerInfo *si,
                                 const ASN1_OBJECT *obj, int type,
                                 const void *bytes, int len)
 {
-    if (X509at_add1_attr_by_OBJ(&si->signedAttrs, obj, type, bytes, len))
+    if (ossl_x509at_add1_attr_by_OBJ(&si->signedAttrs, obj, type, bytes, len))
         return 1;
     return 0;
 }
@@ -111,7 +112,7 @@ int CMS_signed_add1_attr_by_OBJ(CMS_SignerInfo *si,
 int CMS_signed_add1_attr_by_NID(CMS_SignerInfo *si,
                                 int nid, int type, const void *bytes, int len)
 {
-    if (X509at_add1_attr_by_NID(&si->signedAttrs, nid, type, bytes, len))
+    if (ossl_x509at_add1_attr_by_NID(&si->signedAttrs, nid, type, bytes, len))
         return 1;
     return 0;
 }
@@ -120,7 +121,8 @@ int CMS_signed_add1_attr_by_txt(CMS_SignerInfo *si,
                                 const char *attrname, int type,
                                 const void *bytes, int len)
 {
-    if (X509at_add1_attr_by_txt(&si->signedAttrs, attrname, type, bytes, len))
+    if (ossl_x509at_add1_attr_by_txt(&si->signedAttrs, attrname, type, bytes,
+                                     len))
         return 1;
     return 0;
 }
@@ -161,7 +163,7 @@ X509_ATTRIBUTE *CMS_unsigned_delete_attr(CMS_SignerInfo *si, int loc)
 
 int CMS_unsigned_add1_attr(CMS_SignerInfo *si, X509_ATTRIBUTE *attr)
 {
-    if (X509at_add1_attr(&si->unsignedAttrs, attr))
+    if (ossl_x509at_add1_attr(&si->unsignedAttrs, attr))
         return 1;
     return 0;
 }
@@ -170,7 +172,7 @@ int CMS_unsigned_add1_attr_by_OBJ(CMS_SignerInfo *si,
                                   const ASN1_OBJECT *obj, int type,
                                   const void *bytes, int len)
 {
-    if (X509at_add1_attr_by_OBJ(&si->unsignedAttrs, obj, type, bytes, len))
+    if (ossl_x509at_add1_attr_by_OBJ(&si->unsignedAttrs, obj, type, bytes, len))
         return 1;
     return 0;
 }
@@ -179,7 +181,7 @@ int CMS_unsigned_add1_attr_by_NID(CMS_SignerInfo *si,
                                   int nid, int type,
                                   const void *bytes, int len)
 {
-    if (X509at_add1_attr_by_NID(&si->unsignedAttrs, nid, type, bytes, len))
+    if (ossl_x509at_add1_attr_by_NID(&si->unsignedAttrs, nid, type, bytes, len))
         return 1;
     return 0;
 }
@@ -188,8 +190,8 @@ int CMS_unsigned_add1_attr_by_txt(CMS_SignerInfo *si,
                                   const char *attrname, int type,
                                   const void *bytes, int len)
 {
-    if (X509at_add1_attr_by_txt(&si->unsignedAttrs, attrname,
-                                type, bytes, len))
+    if (ossl_x509at_add1_attr_by_txt(&si->unsignedAttrs, attrname,
+                                     type, bytes, len))
         return 1;
     return 0;
 }
diff --git a/crypto/x509/x509_att.c b/crypto/x509/x509_att.c
index d9fe7a3791..8614d3b86d 100644
--- a/crypto/x509/x509_att.c
+++ b/crypto/x509/x509_att.c
@@ -71,8 +71,8 @@ X509_ATTRIBUTE *X509at_delete_attr(STACK_OF(X509_ATTRIBUTE) *x, int loc)
     return ret;
 }
 
-STACK_OF(X509_ATTRIBUTE) *X509at_add1_attr(STACK_OF(X509_ATTRIBUTE) **x,
-                                           X509_ATTRIBUTE *attr)
+STACK_OF(X509_ATTRIBUTE) *ossl_x509at_add1_attr(STACK_OF(X509_ATTRIBUTE) **x,
+                                                X509_ATTRIBUTE *attr)
 {
     X509_ATTRIBUTE *new_attr = NULL;
     STACK_OF(X509_ATTRIBUTE) *sk = NULL;
@@ -82,11 +82,6 @@ STACK_OF(X509_ATTRIBUTE) *X509at_add1_attr(STACK_OF(X509_ATTRIBUTE) **x,
         return NULL;
     }
 
-    if (*x != NULL && X509at_get_attr_by_OBJ(*x, attr->object, -1) != -1) {
-        ERR_raise(ERR_LIB_X509, X509_R_DUPLICATE_ATTRIBUTE);
-        return NULL;
-    }
-
     if (*x == NULL) {
         if ((sk = sk_X509_ATTRIBUTE_new_null()) == NULL)
             goto err;
@@ -110,18 +105,68 @@ STACK_OF(X509_ATTRIBUTE) *X509at_add1_attr(STACK_OF(X509_ATTRIBUTE) **x,
     return NULL;
 }
 
+STACK_OF(X509_ATTRIBUTE) *X509at_add1_attr(STACK_OF(X509_ATTRIBUTE) **x,
+                                           X509_ATTRIBUTE *attr)
+{
+    if (x == NULL || attr == NULL) {
+        ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
+        return NULL;
+    }
+    if (*x != NULL && X509at_get_attr_by_OBJ(*x, attr->object, -1) != -1) {
+        ERR_raise(ERR_LIB_X509, X509_R_DUPLICATE_ATTRIBUTE);
+        return NULL;
+    }
+
+    return ossl_x509at_add1_attr(x, attr);
+}
+
+STACK_OF(X509_ATTRIBUTE) *ossl_x509at_add1_attr_by_OBJ(STACK_OF(X509_ATTRIBUTE) **x,
+                                                       const ASN1_OBJECT *obj,
+                                                       int type,
+                                                       const unsigned char *bytes,
+                                                       int len)
+{
+    X509_ATTRIBUTE *attr;
+    STACK_OF(X509_ATTRIBUTE) *ret;
+
+    attr = X509_ATTRIBUTE_create_by_OBJ(NULL, obj, type, bytes, len);
+    if (attr == NULL)
+        return 0;
+    ret = ossl_x509at_add1_attr(x, attr);
+    X509_ATTRIBUTE_free(attr);
+    return ret;
+}
+
 STACK_OF(X509_ATTRIBUTE) *X509at_add1_attr_by_OBJ(STACK_OF(X509_ATTRIBUTE)
                                                   **x, const ASN1_OBJECT *obj,
                                                   int type,
                                                   const unsigned char *bytes,
                                                   int len)
 {
+    if (x == NULL || obj == NULL) {
+        ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
+        return NULL;
+    }
+    if (*x != NULL && X509at_get_attr_by_OBJ(*x, obj, -1) != -1) {
+        ERR_raise(ERR_LIB_X509, X509_R_DUPLICATE_ATTRIBUTE);
+        return NULL;
+    }
+
+    return ossl_x509at_add1_attr_by_OBJ(x, obj, type, bytes, len);
+}
+
+STACK_OF(X509_ATTRIBUTE) *ossl_x509at_add1_attr_by_NID(STACK_OF(X509_ATTRIBUTE) **x,
+                                                       int nid, int type,
+                                                       const unsigned char *bytes,
+                                                       int len)
+{
     X509_ATTRIBUTE *attr;
     STACK_OF(X509_ATTRIBUTE) *ret;
-    attr = X509_ATTRIBUTE_create_by_OBJ(NULL, obj, type, bytes, len);
-    if (!attr)
+
+    attr = X509_ATTRIBUTE_create_by_NID(NULL, nid, type, bytes, len);
+    if (attr == NULL)
         return 0;
-    ret = X509at_add1_attr(x, attr);
+    ret = ossl_x509at_add1_attr(x, attr);
     X509_ATTRIBUTE_free(attr);
     return ret;
 }
@@ -131,12 +176,31 @@ STACK_OF(X509_ATTRIBUTE) *X509at_add1_attr_by_NID(STACK_OF(X509_ATTRIBUTE)
                                                   const unsigned char *bytes,
                                                   int len)
 {
+    if (x == NULL) {
+        ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
+        return NULL;
+    }
+    if (*x != NULL && X509at_get_attr_by_NID(*x, nid, -1) != -1) {
+        ERR_raise(ERR_LIB_X509, X509_R_DUPLICATE_ATTRIBUTE);
+        return NULL;
+    }
+
+    return ossl_x509at_add1_attr_by_NID(x, nid, type, bytes, len);
+}
+
+STACK_OF(X509_ATTRIBUTE) *ossl_x509at_add1_attr_by_txt(STACK_OF(X509_ATTRIBUTE) **x,
+                                                       const char *attrname,
+                                                       int type,
+                                                       const unsigned char *bytes,
+                                                       int len)
+{
     X509_ATTRIBUTE *attr;
     STACK_OF(X509_ATTRIBUTE) *ret;
-    attr = X509_ATTRIBUTE_create_by_NID(NULL, nid, type, bytes, len);
-    if (!attr)
+
+    attr = X509_ATTRIBUTE_create_by_txt(NULL, attrname, type, bytes, len);
+    if (attr == NULL)
         return 0;
-    ret = X509at_add1_attr(x, attr);
+    ret = ossl_x509at_add1_attr(x, attr);
     X509_ATTRIBUTE_free(attr);
     return ret;
 }
diff --git a/include/crypto/x509.h b/include/crypto/x509.h
index 631150b769..6e79f1f288 100644
--- a/include/crypto/x509.h
+++ b/include/crypto/x509.h
@@ -367,4 +367,21 @@ EVP_PKEY *ossl_d2i_PUBKEY_legacy(EVP_PKEY **a, const unsigned char **pp,
 
 int x509v3_add_len_value_uchar(const char *name, const unsigned char *value,
                                size_t vallen, STACK_OF(CONF_VALUE) **extlist);
+/* Attribute addition functions not checking for duplicate attributes */
+STACK_OF(X509_ATTRIBUTE) *ossl_x509at_add1_attr(STACK_OF(X509_ATTRIBUTE) **x,
+                                                X509_ATTRIBUTE *attr);
+STACK_OF(X509_ATTRIBUTE) *ossl_x509at_add1_attr_by_OBJ(STACK_OF(X509_ATTRIBUTE) **x,
+                                                       const ASN1_OBJECT *obj,
+                                                       int type,
+                                                       const unsigned char *bytes,
+                                                       int len);
+STACK_OF(X509_ATTRIBUTE) *ossl_x509at_add1_attr_by_NID(STACK_OF(X509_ATTRIBUTE) **x,
+                                                       int nid, int type,
+                                                       const unsigned char *bytes,
+                                                       int len);
+STACK_OF(X509_ATTRIBUTE) *ossl_x509at_add1_attr_by_txt(STACK_OF(X509_ATTRIBUTE) **x,
+                                                       const char *attrname,
+                                                       int type,
+                                                       const unsigned char *bytes,
+                                                       int len);
 #endif  /* OSSL_CRYPTO_X509_H */