Remove support for malformed X509_ATTRIBUTEs.
The X509_ATTRIBUTE structure includes a hack to tolerate malformed
attributes that encode the value directly instead of a set of values.
This form is never created by OpenSSL and shouldn't be needed any more.
(Imported from upstream's e20b57270dece66ce2c68aeb5d14dd6d9f3c5d68.)
This also changes X509_ATTRIBUTE_set1_data slightly. Previously,
set1_data would override whatever was previously in the X509_ATTRIBUTE,
but leak memory. Now set1_data appends to the set. (PKCS#10 attributes
use SET OF ANY as value.) It's unclear to me if this was intentional on
upstream's part. (The attrtype == 0 case only makes sense in the old
behavior.) Since there is no other way to create a two-element SET and
upstream has long since released this behavior, I left it matching
upstream.
Update-Note: Given OpenSSL hasn't accepted these for five years, it's
unlikely anything depends on it. If something breaks, we can revert this
and revisit. No one calls X509_ATTRIBUTE_set1_data on a non-empty
X509_ATTRIBUTE, so the behavior change there should be safe.
Change-Id: Ic03c793b7d42784072ec0d9a7b6424aecc738632
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46947
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index 6b7d0c6..a5f31c2 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h
@@ -39,12 +39,7 @@
struct x509_attributes_st {
ASN1_OBJECT *object;
- int single; // 0 for a set, 1 for a single item (which is wrong)
- union {
- char *ptr;
- /* 0 */ STACK_OF(ASN1_TYPE) *set;
- /* 1 */ ASN1_TYPE *single;
- } value;
+ STACK_OF(ASN1_TYPE) *set;
} /* X509_ATTRIBUTE */;
diff --git a/crypto/x509/x509_att.c b/crypto/x509/x509_att.c
index 07edde9..90f722a 100644
--- a/crypto/x509/x509_att.c
+++ b/crypto/x509/x509_att.c
@@ -311,9 +311,6 @@
goto err;
atype = attrtype;
}
- if (!(attr->value.set = sk_ASN1_TYPE_new_null()))
- goto err;
- attr->single = 0;
/*
* This is a bit naughty because the attribute should really have at
* least one value but some types use and zero length SET and require
@@ -332,7 +329,7 @@
ASN1_TYPE_set(ttmp, atype, stmp);
stmp = NULL;
}
- if (!sk_ASN1_TYPE_push(attr->value.set, ttmp))
+ if (!sk_ASN1_TYPE_push(attr->set, ttmp))
goto err;
return 1;
err:
@@ -344,11 +341,7 @@
int X509_ATTRIBUTE_count(X509_ATTRIBUTE *attr)
{
- if (!attr->single)
- return sk_ASN1_TYPE_num(attr->value.set);
- if (attr->value.single)
- return 1;
- return 0;
+ return sk_ASN1_TYPE_num(attr->set);
}
ASN1_OBJECT *X509_ATTRIBUTE_get0_object(X509_ATTRIBUTE *attr)
@@ -375,11 +368,8 @@
ASN1_TYPE *X509_ATTRIBUTE_get0_type(X509_ATTRIBUTE *attr, int idx)
{
if (attr == NULL)
- return (NULL);
+ return NULL;
if (idx >= X509_ATTRIBUTE_count(attr))
return NULL;
- if (!attr->single)
- return sk_ASN1_TYPE_value(attr->value.set, idx);
- else
- return attr->value.single;
+ return sk_ASN1_TYPE_value(attr->set, idx);
}
diff --git a/crypto/x509/x_attrib.c b/crypto/x509/x_attrib.c
index d906df8..3ee8bd4 100644
--- a/crypto/x509/x_attrib.c
+++ b/crypto/x509/x_attrib.c
@@ -62,25 +62,9 @@
#include "internal.h"
-/*
- * X509_ATTRIBUTE: this has the following form: typedef struct
- * x509_attributes_st { ASN1_OBJECT *object; int single; union { char *ptr;
- * STACK_OF(ASN1_TYPE) *set; ASN1_TYPE *single; } value; } X509_ATTRIBUTE;
- * this needs some extra thought because the CHOICE type is merged with the
- * main structure and because the value can be anything at all we *must* try
- * the SET OF first because the ASN1_ANY type will swallow anything including
- * the whole SET OF structure.
- */
-
-ASN1_CHOICE(X509_ATTRIBUTE_SET) = {
- ASN1_SET_OF(X509_ATTRIBUTE, value.set, ASN1_ANY),
- ASN1_SIMPLE(X509_ATTRIBUTE, value.single, ASN1_ANY)
-} ASN1_CHOICE_END_selector(X509_ATTRIBUTE, X509_ATTRIBUTE_SET, single)
-
ASN1_SEQUENCE(X509_ATTRIBUTE) = {
ASN1_SIMPLE(X509_ATTRIBUTE, object, ASN1_OBJECT),
- /* CHOICE type merged with parent */
- ASN1_EX_COMBINE(0, 0, X509_ATTRIBUTE_SET)
+ ASN1_SET_OF(X509_ATTRIBUTE, set, ASN1_ANY),
} ASN1_SEQUENCE_END(X509_ATTRIBUTE)
IMPLEMENT_ASN1_FUNCTIONS(X509_ATTRIBUTE)
@@ -100,10 +84,7 @@
}
ret->object = obj;
- ret->single = 0;
- ret->value.set = sk_ASN1_TYPE_new_null();
- if (ret->value.set == NULL ||
- !sk_ASN1_TYPE_push(ret->value.set, val)) {
+ if (!sk_ASN1_TYPE_push(ret->set, val)) {
goto err;
}
