summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--README.ASN136
-rw-r--r--crypto/asn1/t_req.c14
-rw-r--r--crypto/asn1/tasn_new.c2
-rw-r--r--crypto/asn1/tasn_utl.c6
-rw-r--r--crypto/asn1/x_attrib.c110
-rw-r--r--crypto/pkcs12/p12_attr.c6
-rw-r--r--crypto/pkcs7/pk7_doit.c2
-rw-r--r--crypto/x509/x509.h6
-rw-r--r--crypto/x509/x509_att.c6
-rw-r--r--crypto/x509/x509_req.c6
10 files changed, 78 insertions, 116 deletions
diff --git a/README.ASN1 b/README.ASN1
index f7bbdad78a..38c5f5b2c6 100644
--- a/README.ASN1
+++ b/README.ASN1
@@ -108,19 +108,27 @@ The BOOLEAN type now takes three values. 0xff is TRUE, 0 is FALSE and -1 is
absent. The meaning of absent depends on the context. If for example the
boolean type is DEFAULT FALSE (as in the case of the critical flag for
certificate extensions) then -1 is FALSE, if DEFAULT TRUE then -1 is TRUE.
-Usually the value will only ever be read via an API which will hide this from an
-application.
+Usually the value will only ever be read via an API which will hide this from
+an application.
There is an evil bug in the old ASN1 code that mishandles OPTIONAL with
-SEQUENCE OF or SET OF. These are both implemented as a STACK structure. The old
-code would omit the structure if the STACK was NULL (which is fine) or if it had
-zero elements (which is NOT OK). This causes problems because an empty SEQUENCE OF
-or SET OF will result in an empty STACK when it is decoded but when it is encoded
-it will be omitted resulting in different encodings. The new code only omits the
-encoding if the STACK is NULL, if it contains zero elements it is encoded and
-empty. There is an additional problem though: because an empty STACK was omitted
-sometimes the corresponding *_new() function would initialize the STACK to empty
-so an application could immediately use it, if this is done with the new code
-(i.e. a NULL) it wont work. Therefore a new STACK should be allocated first.
-One instance of this is the X509_CRL list of revoked certificates: a helper
-function X509_CRL_add0_revoked() has been added for this purpose.
+SEQUENCE OF or SET OF. These are both implemented as a STACK structure. The
+old code would omit the structure if the STACK was NULL (which is fine) or if
+it had zero elements (which is NOT OK). This causes problems because an empty
+SEQUENCE OF or SET OF will result in an empty STACK when it is decoded but when
+it is encoded it will be omitted resulting in different encodings. The new code
+only omits the encoding if the STACK is NULL, if it contains zero elements it
+is encoded and empty. There is an additional problem though: because an empty
+STACK was omitted, sometimes the corresponding *_new() function would
+initialize the STACK to empty so an application could immediately use it, if
+this is done with the new code (i.e. a NULL) it wont work. Therefore a new
+STACK should be allocated first. One instance of this is the X509_CRL list of
+revoked certificates: a helper function X509_CRL_add0_revoked() has been added
+for this purpose.
+
+The X509_ATTRIBUTE structure used to have an element called 'set' which took
+the value 1 if the attribute value was a SET OF or 0 if it was a single. Due
+to the behaviour of CHOICE in the new code this has been changed to a field
+called 'single' which is 0 for a SET OF and 1 for single. The old field has
+been deleted to deliberately break source compatibility. Since this structure
+is normally accessed via higher level functions this shouldn't break too much.
diff --git a/crypto/asn1/t_req.c b/crypto/asn1/t_req.c
index ea1af092db..9f38446a5d 100644
--- a/crypto/asn1/t_req.c
+++ b/crypto/asn1/t_req.c
@@ -170,7 +170,13 @@ int X509_REQ_print(BIO *bp, X509_REQ *x)
if (BIO_puts(bp,str) <= 0) goto err;
if ((j=i2a_ASN1_OBJECT(bp,a->object)) > 0)
{
- if (a->set)
+ if (a->single)
+ {
+ t=a->value.single;
+ type=t->type;
+ bs=t->value.bit_string;
+ }
+ else
{
ii=0;
count=sk_ASN1_TYPE_num(a->value.set);
@@ -179,12 +185,6 @@ get_next:
type=at->type;
bs=at->value.asn1_string;
}
- else
- {
- t=a->value.single;
- type=t->type;
- bs=t->value.bit_string;
- }
}
for (j=25-j; j>0; j--)
if (BIO_write(bp," ",1) != 1) goto err;
diff --git a/crypto/asn1/tasn_new.c b/crypto/asn1/tasn_new.c
index 23a313acdb..7e48c193cd 100644
--- a/crypto/asn1/tasn_new.c
+++ b/crypto/asn1/tasn_new.c
@@ -91,7 +91,7 @@ static int asn1_item_ex_combine_new(ASN1_VALUE **pval, const ASN1_ITEM *it, int
if(aux && aux->asn1_cb) asn1_cb = aux->asn1_cb;
else asn1_cb = 0;
- *pval = NULL;
+ if(!combine) *pval = NULL;
switch(it->itype) {
diff --git a/crypto/asn1/tasn_utl.c b/crypto/asn1/tasn_utl.c
index 92ab6df7e9..e3212bdffb 100644
--- a/crypto/asn1/tasn_utl.c
+++ b/crypto/asn1/tasn_utl.c
@@ -126,8 +126,9 @@ int asn1_do_lock(ASN1_VALUE *pval, int op, const ASN1_ITEM *it)
ASN1_VALUE *asn1_get_field(ASN1_VALUE *val, const ASN1_TEMPLATE *tt)
{
- ASN1_VALUE **ptr = offset2ptr(val, tt->offset);
+ ASN1_VALUE **ptr;
if(tt->flags & ASN1_TFLG_COMBINE) return val;
+ ptr = offset2ptr(val, tt->offset);
/* NOTE for BOOLEAN types the field is just a plain
* int so we don't dereference it. This means that
* BOOLEAN is an (int *).
@@ -147,8 +148,9 @@ ASN1_VALUE *asn1_get_field(ASN1_VALUE *val, const ASN1_TEMPLATE *tt)
/* Given an ASN1_TEMPLATE get a pointer to a field */
ASN1_VALUE ** asn1_get_field_ptr(ASN1_VALUE **pval, const ASN1_TEMPLATE *tt)
{
- ASN1_VALUE **pvaltmp = offset2ptr(*pval, tt->offset);
+ ASN1_VALUE **pvaltmp;
if(tt->flags & ASN1_TFLG_COMBINE) return pval;
+ pvaltmp = offset2ptr(*pval, tt->offset);
/* NOTE for BOOLEAN types the field is just a plain
* int so we can't return int **, so settle for
* (int *).
diff --git a/crypto/asn1/x_attrib.c b/crypto/asn1/x_attrib.c
index 14e5ea27aa..3c0d3ab759 100644
--- a/crypto/asn1/x_attrib.c
+++ b/crypto/asn1/x_attrib.c
@@ -59,64 +59,41 @@
#include <stdio.h>
#include "cryptlib.h"
#include <openssl/objects.h>
-#include <openssl/asn1_mac.h>
+#include <openssl/asn1t.h>
#include <openssl/x509.h>
-/* sequence */
-int i2d_X509_ATTRIBUTE(X509_ATTRIBUTE *a, unsigned char **pp)
- {
- int k=0;
- int r=0,ret=0;
- unsigned char **p=NULL;
-
- if (a == NULL) return(0);
-
- p=NULL;
- for (;;)
- {
- if (k)
- {
- r=ASN1_object_size(1,ret,V_ASN1_SEQUENCE);
- if (pp == NULL) return(r);
- p=pp;
- ASN1_put_object(p,1,ret,V_ASN1_SEQUENCE,
- V_ASN1_UNIVERSAL);
- }
-
- ret+=i2d_ASN1_OBJECT(a->object,p);
- if (a->set)
- ret+=i2d_ASN1_SET_OF_ASN1_TYPE(a->value.set,p,i2d_ASN1_TYPE,
- V_ASN1_SET,V_ASN1_UNIVERSAL,IS_SET);
- else
- ret+=i2d_ASN1_TYPE(a->value.single,p);
- if (k++) return(r);
- }
- }
-
-X509_ATTRIBUTE *d2i_X509_ATTRIBUTE(X509_ATTRIBUTE **a, unsigned char **pp,
- long length)
- {
- M_ASN1_D2I_vars(a,X509_ATTRIBUTE *,X509_ATTRIBUTE_new);
+/* 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.
+ */
- M_ASN1_D2I_Init();
- M_ASN1_D2I_start_sequence();
- M_ASN1_D2I_get(ret->object,d2i_ASN1_OBJECT);
+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);
- if ((c.slen != 0) &&
- (M_ASN1_next == (V_ASN1_CONSTRUCTED|V_ASN1_UNIVERSAL|V_ASN1_SET)))
- {
- ret->set=1;
- M_ASN1_D2I_get_set_type(ASN1_TYPE,ret->value.set,d2i_ASN1_TYPE,
- ASN1_TYPE_free);
- }
- else
- {
- ret->set=0;
- M_ASN1_D2I_get(ret->value.single,d2i_ASN1_TYPE);
- }
+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_SEQUENCE_END(X509_ATTRIBUTE);
- M_ASN1_D2I_Finish(a,X509_ATTRIBUTE_free,ASN1_F_D2I_X509_ATTRIBUTE);
- }
+IMPLEMENT_ASN1_FUNCTIONS(X509_ATTRIBUTE)
X509_ATTRIBUTE *X509_ATTRIBUTE_create(int nid, int atrtype, void *value)
{
@@ -126,7 +103,7 @@ X509_ATTRIBUTE *X509_ATTRIBUTE_create(int nid, int atrtype, void *value)
if ((ret=X509_ATTRIBUTE_new()) == NULL)
return(NULL);
ret->object=OBJ_nid2obj(nid);
- ret->set=1;
+ ret->single=0;
if ((ret->value.set=sk_ASN1_TYPE_new_null()) == NULL) goto err;
if ((val=ASN1_TYPE_new()) == NULL) goto err;
if (!sk_ASN1_TYPE_push(ret->value.set,val)) goto err;
@@ -138,28 +115,3 @@ err:
if (val != NULL) ASN1_TYPE_free(val);
return(NULL);
}
-
-X509_ATTRIBUTE *X509_ATTRIBUTE_new(void)
- {
- X509_ATTRIBUTE *ret=NULL;
- ASN1_CTX c;
-
- M_ASN1_New_Malloc(ret,X509_ATTRIBUTE);
- ret->object=OBJ_nid2obj(NID_undef);
- ret->set=0;
- ret->value.ptr=NULL;
- return(ret);
- M_ASN1_New_Error(ASN1_F_X509_ATTRIBUTE_NEW);
- }
-
-void X509_ATTRIBUTE_free(X509_ATTRIBUTE *a)
- {
- if (a == NULL) return;
- ASN1_OBJECT_free(a->object);
- if (a->set)
- sk_ASN1_TYPE_pop_free(a->value.set,ASN1_TYPE_free);
- else
- ASN1_TYPE_free(a->value.single);
- OPENSSL_free(a);
- }
-
diff --git a/crypto/pkcs12/p12_attr.c b/crypto/pkcs12/p12_attr.c
index f1a210b5d2..64bf4173fc 100644
--- a/crypto/pkcs12/p12_attr.c
+++ b/crypto/pkcs12/p12_attr.c
@@ -92,7 +92,7 @@ int PKCS12_add_localkeyid (PKCS12_SAFEBAG *bag, unsigned char *name,
return 0;
}
sk_ASN1_TYPE_push (attrib->value.set,keyid);
- attrib->set = 1;
+ attrib->single = 0;
if (!bag->attrib && !(bag->attrib = sk_X509_ATTRIBUTE_new_null ())) {
PKCS12err(PKCS12_F_PKCS12_ADD_LOCALKEYID, ERR_R_MALLOC_FAILURE);
return 0;
@@ -134,7 +134,7 @@ int PKCS8_add_keyusage (PKCS8_PRIV_KEY_INFO *p8, int usage)
return 0;
}
sk_ASN1_TYPE_push (attrib->value.set,keyid);
- attrib->set = 1;
+ attrib->single = 0;
if (!p8->attributes
&& !(p8->attributes = sk_X509_ATTRIBUTE_new_null ())) {
PKCS12err(PKCS12_F_PKCS8_ADD_KEYUSAGE, ERR_R_MALLOC_FAILURE);
@@ -201,7 +201,7 @@ int PKCS12_add_friendlyname_uni (PKCS12_SAFEBAG *bag,
return 0;
}
sk_ASN1_TYPE_push (attrib->value.set,fname);
- attrib->set = 1;
+ attrib->single = 0;
if (!bag->attrib && !(bag->attrib = sk_X509_ATTRIBUTE_new_null ())) {
PKCS12err(PKCS12_F_PKCS12_ADD_FRIENDLYNAME_UNI,
ERR_R_MALLOC_FAILURE);
diff --git a/crypto/pkcs7/pk7_doit.c b/crypto/pkcs7/pk7_doit.c
index 099e9651c1..c63d27e537 100644
--- a/crypto/pkcs7/pk7_doit.c
+++ b/crypto/pkcs7/pk7_doit.c
@@ -838,7 +838,7 @@ static ASN1_TYPE *get_attribute(STACK_OF(X509_ATTRIBUTE) *sk, int nid)
xa=sk_X509_ATTRIBUTE_value(sk,i);
if (OBJ_cmp(xa->object,o) == 0)
{
- if (xa->set && sk_ASN1_TYPE_num(xa->value.set))
+ if (!xa->single && sk_ASN1_TYPE_num(xa->value.set))
return(sk_ASN1_TYPE_value(xa->value.set,0));
else
return(NULL);
diff --git a/crypto/x509/x509.h b/crypto/x509/x509.h
index f1b9924c91..a0d53ac5e6 100644
--- a/crypto/x509/x509.h
+++ b/crypto/x509/x509.h
@@ -193,11 +193,11 @@ DECLARE_ASN1_SET_OF(X509_EXTENSION)
typedef struct x509_attributes_st
{
ASN1_OBJECT *object;
- int set; /* 1 for a set, 0 for a single item (which is wrong) */
+ int single; /* 0 for a set, 1 for a single item (which is wrong) */
union {
char *ptr;
-/* 1 */ STACK_OF(ASN1_TYPE) *set;
-/* 0 */ ASN1_TYPE *single;
+/* 0 */ STACK_OF(ASN1_TYPE) *set;
+/* 1 */ ASN1_TYPE *single;
} value;
} X509_ATTRIBUTE;
diff --git a/crypto/x509/x509_att.c b/crypto/x509/x509_att.c
index caafde658f..f074d2ab18 100644
--- a/crypto/x509/x509_att.c
+++ b/crypto/x509/x509_att.c
@@ -283,7 +283,7 @@ int X509_ATTRIBUTE_set1_data(X509_ATTRIBUTE *attr, int attrtype, void *data, int
if(!(attr->value.set = sk_ASN1_TYPE_new_null())) goto err;
if(!(ttmp = ASN1_TYPE_new())) goto err;
if(!sk_ASN1_TYPE_push(attr->value.set, ttmp)) goto err;
- attr->set = 1;
+ attr->single = 0;
ASN1_TYPE_set(ttmp, atype, stmp);
return 1;
err:
@@ -293,7 +293,7 @@ int X509_ATTRIBUTE_set1_data(X509_ATTRIBUTE *attr, int attrtype, void *data, int
int X509_ATTRIBUTE_count(X509_ATTRIBUTE *attr)
{
- if(attr->set) return sk_ASN1_TYPE_num(attr->value.set);
+ if(!attr->single) return sk_ASN1_TYPE_num(attr->value.set);
if(attr->value.single) return 1;
return 0;
}
@@ -321,6 +321,6 @@ ASN1_TYPE *X509_ATTRIBUTE_get0_type(X509_ATTRIBUTE *attr, int idx)
{
if (attr == NULL) return(NULL);
if(idx >= X509_ATTRIBUTE_count(attr)) return NULL;
- if(attr->set) return sk_ASN1_TYPE_value(attr->value.set, idx);
+ if(!attr->single) return sk_ASN1_TYPE_value(attr->value.set, idx);
else return attr->value.single;
}
diff --git a/crypto/x509/x509_req.c b/crypto/x509/x509_req.c
index 7eca1bd57a..e2766e1a5f 100644
--- a/crypto/x509/x509_req.c
+++ b/crypto/x509/x509_req.c
@@ -156,9 +156,9 @@ STACK_OF(X509_EXTENSION) *X509_REQ_get_extensions(X509_REQ *req)
for(i = 0; i < sk_X509_ATTRIBUTE_num(sk); i++) {
attr = sk_X509_ATTRIBUTE_value(sk, i);
if(X509_REQ_extension_nid(OBJ_obj2nid(attr->object))) {
- if(attr->set && sk_ASN1_TYPE_num(attr->value.set))
+ if(attr->single) ext = attr->value.single;
+ else if(sk_ASN1_TYPE_num(attr->value.set))
ext = sk_ASN1_TYPE_value(attr->value.set, 0);
- else ext = attr->value.single;
break;
}
}
@@ -199,7 +199,7 @@ int X509_REQ_add_extensions_nid(X509_REQ *req, STACK_OF(X509_EXTENSION) *exts,
if(!(attr->value.set = sk_ASN1_TYPE_new_null())) goto err;
if(!sk_ASN1_TYPE_push(attr->value.set, at)) goto err;
at = NULL;
- attr->set = 1;
+ attr->single = 0;
attr->object = OBJ_nid2obj(nid);
if(!sk_X509_ATTRIBUTE_push(req->req_info->attributes, attr)) goto err;
return 1;