Skip to content

Commit

Permalink
Allow duplicate CMS attributes
Browse files Browse the repository at this point in the history 
Fixes regression introduced with openssl#21505

Fixes openssl#22266

Reviewed-by: Shane Lontis <[email protected]>
Reviewed-by: Richard Levitte <[email protected]>
(Merged from openssl#23029)

(cherry picked from commit d7e707c)
  • Loading branch information
@t8m
t8m committed Jan 3, 2024
1 parent ec29062 commit 41dd0e0
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 23 deletions.
22 changes: 12 additions & 10 deletions crypto/cms/cms_att.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
Expand All @@ -103,15 +104,15 @@ 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;
}

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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
90 changes: 77 additions & 13 deletions crypto/x509/x509_att.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
}
Expand All @@ -130,13 +175,32 @@ STACK_OF(X509_ATTRIBUTE) *X509at_add1_attr_by_NID(STACK_OF(X509_ATTRIBUTE)
**x, int nid, int type,
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;
}
Expand Down
17 changes: 17 additions & 0 deletions include/crypto/x509.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

0 comments on commit 41dd0e0

Please sign in to comment.