diff options
-rw-r--r-- | README.ASN1 | 36 | ||||
-rw-r--r-- | crypto/asn1/t_req.c | 14 | ||||
-rw-r--r-- | crypto/asn1/tasn_new.c | 2 | ||||
-rw-r--r-- | crypto/asn1/tasn_utl.c | 6 | ||||
-rw-r--r-- | crypto/asn1/x_attrib.c | 110 | ||||
-rw-r--r-- | crypto/pkcs12/p12_attr.c | 6 | ||||
-rw-r--r-- | crypto/pkcs7/pk7_doit.c | 2 | ||||
-rw-r--r-- | crypto/x509/x509.h | 6 | ||||
-rw-r--r-- | crypto/x509/x509_att.c | 6 | ||||
-rw-r--r-- | crypto/x509/x509_req.c | 6 |
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; |