The encoding of negative ASN1 INTEGERs and the conversion of BNs to negative

integers was completely broken. Also added a NEG_PUBKEY_BUG compilation option
to compensate for public keys improperly encoded as negative integers.

CHANGES

@@ -10,6 +10,12 @@
                                    [23-Dec-1998] down below; but in later
                                    versions, these hyphens are gone.]
 
+  *) Fix the encoding and decoding of negative ASN1 INTEGERS and conversion
+     to and from BNs: it was completely broken. New compilation option
+     NEG_PUBKEY_BUG to allow for some broken certificates that encode public
+     key elements as negative integers.
+     [Steve Henson]
+
   *) Reorganize and speed up MD5.
      [Andy Polyakov <appro@fy.chalmers.se>]
 

crypto/asn1/a_enum.c

@@ -60,12 +60,15 @@
 #include "cryptlib.h"
 #include <openssl/asn1.h>
 
-/* Support for ASN1 ENUMERATED type: based on a_int.c */
+/* 
+ * Code for ENUMERATED type: identical to INTEGER apart from a different tag.
+ * for comments on encoding see a_int.c
+ */
 
 int i2d_ASN1_ENUMERATED(ASN1_ENUMERATED *a, unsigned char **pp)
 	{
 	int pad=0,ret,r,i,t;
-	unsigned char *p,*pt,*n,pb=0;
+	unsigned char *p,*n,pb=0;
 
 	if ((a == NULL) || (a->data == NULL)) return(0);
 	t=a->type;
@@ -75,16 +78,21 @@ int i2d_ASN1_ENUMERATED(ASN1_ENUMERATED *a, unsigned char **pp)
 		{
 		ret=a->length;
 		i=a->data[0];
-		if ((t == V_ASN1_ENUMERATED) && (i > 127))
-			{
+		if ((t == V_ASN1_ENUMERATED) && (i > 127)) {
 			pad=1;
 			pb=0;
+		} else if(t == V_ASN1_NEG_ENUMERATED) {
+			if(i>128) {
+				pad=1;
+				pb=0xFF;
+			} else if(i == 128) {
+				for(i = 1; i < a->length; i++) if(a->data[i]) {
+						pad=1;
+						pb=0xFF;
+						break;
+				}
 			}
-		else if ((t == V_ASN1_NEG_ENUMERATED) && (i>128))
-			{
-			pad=1;
-			pb=0xFF;
-			}
+		}
 		ret+=pad;
 		}
 	r=ASN1_object_size(0,ret,V_ASN1_ENUMERATED);
@@ -100,14 +108,24 @@ int i2d_ASN1_ENUMERATED(ASN1_ENUMERATED *a, unsigned char **pp)
 		memcpy(p,a->data,(unsigned int)a->length);
 		p+=a->length;
 		}
-	else
-		{
-		n=a->data;
-		pt=p;
-		for (i=a->length; i>0; i--)
-			*(p++)= (*(n++)^0xFF)+1;
-		if (!pad) *pt|=0x80;
+	else {
+		/* Begin at the end of the encoding */
+		n=a->data + a->length - 1;
+		p += a->length - 1;
+		i = a->length;
+		/* Copy zeros to destination as long as source is zero */
+		while(!*n) {
+			*(p--) = 0;
+			n--;
+			i--;
 		}
+		/* Complement and increment next octet */
+		*(p--) = ((*(n--)) ^ 0xff) + 1;
+		i--;
+		/* Complement any octets left */
+		for(;i > 0; i--) *(p--) = *(n--) ^ 0xff;
+		p += a->length;
+	}
 
 	*pp=p;
 	return(r);
@@ -156,16 +174,30 @@ ASN1_ENUMERATED *d2i_ASN1_ENUMERATED(ASN1_ENUMERATED **a, unsigned char **pp,
 	if (*p & 0x80) /* a negative number */
 		{
 		ret->type=V_ASN1_NEG_ENUMERATED;
-		if (*p == 0xff)
-			{
+		if ((*p == 0xff) && (len != 1)) {
 			p++;
 			len--;
-			}
-		for (i=(int)len; i>0; i--)
-			*(to++)= (*(p++)^0xFF)+1;
 		}
-	else
-		{
+		i = len;
+		p += i - 1;
+		to += i - 1;
+		while((!*p) && i) {
+			*(to--) = 0;
+			i--;
+			p--;
+		}
+		if(!i) {
+			*s = 1;
+			s[len] = 0;
+			p += len;
+			len++;
+		} else {
+			*(to--) = (*(p--) ^ 0xff) + 1;
+			i--;
+			for(;i > 0; i--) *(to--) = *(p--) ^ 0xff;
+			p += len;
+		}
+	} else {
 		ret->type=V_ASN1_ENUMERATED;
 		if ((*p == 0) && (len != 1))
 			{
@@ -174,7 +206,7 @@ ASN1_ENUMERATED *d2i_ASN1_ENUMERATED(ASN1_ENUMERATED **a, unsigned char **pp,
 			}
 		memcpy(s,p,(int)len);
 		p+=len;
-		}
+	}
 
 	if (ret->data != NULL) Free((char *)ret->data);
 	ret->data=s;
@@ -222,7 +254,6 @@ int ASN1_ENUMERATED_set(ASN1_ENUMERATED *a, long v)
 		d>>=8;
 		}
 	j=0;
-	if (v < 0) a->data[j++]=0;
 	for (k=i-1; k >=0; k--)
 		a->data[j++]=buf[k];
 	a->length=j;
@@ -272,7 +303,8 @@ ASN1_ENUMERATED *BN_to_ASN1_ENUMERATED(BIGNUM *bn, ASN1_ENUMERATED *ai)
 		ASN1err(ASN1_F_BN_TO_ASN1_ENUMERATED,ERR_R_NESTED_ASN1_ERROR);
 		goto err;
 		}
-	ret->type=V_ASN1_ENUMERATED;
+	if(bn->neg) ret->type = V_ASN1_NEG_ENUMERATED;
+	else ret->type=V_ASN1_ENUMERATED;
 	j=BN_num_bits(bn);
 	len=((j == 0)?0:((j/8)+1));
 	ret->data=(unsigned char *)Malloc(len+4);
@@ -289,5 +321,6 @@ BIGNUM *ASN1_ENUMERATED_to_BN(ASN1_ENUMERATED *ai, BIGNUM *bn)
 
 	if ((ret=BN_bin2bn(ai->data,ai->length,bn)) == NULL)
 		ASN1err(ASN1_F_ASN1_ENUMERATED_TO_BN,ASN1_R_BN_LIB);
+	if(ai->type == V_ASN1_NEG_ENUMERATED) bn->neg = 1;
 	return(ret);
 	}

crypto/asn1/a_int.c

@@ -60,10 +60,35 @@
 #include "cryptlib.h"
 #include <openssl/asn1.h>
 
+/* 
+ * This converts an ASN1 INTEGER into its DER encoding.
+ * The internal representation is an ASN1_STRING whose data is a big endian
+ * representation of the value, ignoring the sign. The sign is determined by
+ * the type: V_ASN1_INTEGER for positive and V_ASN1_NEG_INTEGER for negative. 
+ *
+ * Positive integers are no problem: they are almost the same as the DER
+ * encoding, except if the first byte is >= 0x80 we need to add a zero pad.
+ *
+ * Negative integers are a bit trickier...
+ * The DER representation of negative integers is in 2s complement form.
+ * The internal form is converted by complementing each octet and finally 
+ * adding one to the result. This can be done less messily with a little trick.
+ * If the internal form has trailing zeroes then they will become FF by the
+ * complement and 0 by the add one (due to carry) so just copy as many trailing 
+ * zeros to the destination as there are in the source. The carry will add one
+ * to the last none zero octet: so complement this octet and add one and finally
+ * complement any left over until you get to the start of the string.
+ *
+ * Padding is a little trickier too. If the first bytes is > 0x80 then we pad
+ * with 0xff. However if the first byte is 0x80 and one of the following bytes
+ * is non-zero we pad with 0xff. The reason for this distinction is that 0x80
+ * followed by optional zeros isn't padded.
+ */
+
 int i2d_ASN1_INTEGER(ASN1_INTEGER *a, unsigned char **pp)
 	{
 	int pad=0,ret,r,i,t;
-	unsigned char *p,*pt,*n,pb=0;
+	unsigned char *p,*n,pb=0;
 
 	if ((a == NULL) || (a->data == NULL)) return(0);
 	t=a->type;
@@ -73,16 +98,25 @@ int i2d_ASN1_INTEGER(ASN1_INTEGER *a, unsigned char **pp)
 		{
 		ret=a->length;
 		i=a->data[0];
-		if ((t == V_ASN1_INTEGER) && (i > 127))
-			{
+		if ((t == V_ASN1_INTEGER) && (i > 127)) {
 			pad=1;
 			pb=0;
+		} else if(t == V_ASN1_NEG_INTEGER) {
+			if(i>128) {
+				pad=1;
+				pb=0xFF;
+			} else if(i == 128) {
+			/*
+			 * Special case: if any other bytes non zero we pad:
+			 * otherwise we don't.
+			 */
+				for(i = 1; i < a->length; i++) if(a->data[i]) {
+						pad=1;
+						pb=0xFF;
+						break;
+				}
 			}
-		else if ((t == V_ASN1_NEG_INTEGER) && (i>128))
-			{
-			pad=1;
-			pb=0xFF;
-			}
+		}
 		ret+=pad;
 		}
 	r=ASN1_object_size(0,ret,V_ASN1_INTEGER);
@@ -98,14 +132,24 @@ int i2d_ASN1_INTEGER(ASN1_INTEGER *a, unsigned char **pp)
 		memcpy(p,a->data,(unsigned int)a->length);
 		p+=a->length;
 		}
-	else
-		{
-		n=a->data;
-		pt=p;
-		for (i=a->length; i>0; i--)
-			*(p++)= (*(n++)^0xFF)+1;
-		if (!pad) *pt|=0x80;
+	else {
+		/* Begin at the end of the encoding */
+		n=a->data + a->length - 1;
+		p += a->length - 1;
+		i = a->length;
+		/* Copy zeros to destination as long as source is zero */
+		while(!*n) {
+			*(p--) = 0;
+			n--;
+			i--;
 		}
+		/* Complement and increment next octet */
+		*(p--) = ((*(n--)) ^ 0xff) + 1;
+		i--;
+		/* Complement any octets left */
+		for(;i > 0; i--) *(p--) = *(n--) ^ 0xff;
+		p += a->length;
+	}
 
 	*pp=p;
 	return(r);
@@ -154,16 +198,105 @@ ASN1_INTEGER *d2i_ASN1_INTEGER(ASN1_INTEGER **a, unsigned char **pp,
 	if (*p & 0x80) /* a negative number */
 		{
 		ret->type=V_ASN1_NEG_INTEGER;
-		if (*p == 0xff)
+		if ((*p == 0xff) && (len != 1)) {
+			p++;
+			len--;
+		}
+		i = len;
+		p += i - 1;
+		to += i - 1;
+		while((!*p) && i) {
+			*(to--) = 0;
+			i--;
+			p--;
+		}
+		/* Special case: if all zeros then the number will be of
+		 * the form FF followed by n zero bytes: this corresponds to
+		 * 1 followed by n zero bytes. We've already written n zeros
+		 * so we just append an extra one and set the first byte to
+		 * a 1. This is treated separately because it is the only case
+		 * where the number of bytes is larger than len.
+		 */
+		if(!i) {
+			*s = 1;
+			s[len] = 0;
+			p += len;
+			len++;
+		} else {
+			*(to--) = (*(p--) ^ 0xff) + 1;
+			i--;
+			for(;i > 0; i--) *(to--) = *(p--) ^ 0xff;
+			p += len;
+		}
+	} else {
+		ret->type=V_ASN1_INTEGER;
+		if ((*p == 0) && (len != 1))
 			{
 			p++;
 			len--;
 			}
-		for (i=(int)len; i>0; i--)
-			*(to++)= (*(p++)^0xFF)+1;
+		memcpy(s,p,(int)len);
+		p+=len;
+	}
+
+	if (ret->data != NULL) Free((char *)ret->data);
+	ret->data=s;
+	ret->length=(int)len;
+	if (a != NULL) (*a)=ret;
+	*pp=p;
+	return(ret);
+err:
+	ASN1err(ASN1_F_D2I_ASN1_INTEGER,i);
+	if ((ret != NULL) && ((a == NULL) || (*a != ret)))
+		ASN1_INTEGER_free(ret);
+	return(NULL);
+	}
+
+/* This is a version of d2i_ASN1_INTEGER that ignores the sign bit of
+ * ASN1 integers: some broken software can encode a positive INTEGER
+ * with its MSB set as negative (it doesn't add a padding zero).
+ */
+
+ASN1_INTEGER *d2i_ASN1_UINTEGER(ASN1_INTEGER **a, unsigned char **pp,
+	     long length)
+	{
+	ASN1_INTEGER *ret=NULL;
+	unsigned char *p,*to,*s;
+	long len;
+	int inf,tag,xclass;
+	int i;
+
+	if ((a == NULL) || ((*a) == NULL))
+		{
+		if ((ret=ASN1_INTEGER_new()) == NULL) return(NULL);
+		ret->type=V_ASN1_INTEGER;
 		}
 	else
+		ret=(*a);
+
+	p= *pp;
+	inf=ASN1_get_object(&p,&len,&tag,&xclass,length);
+	if (inf & 0x80)
+		{
+		i=ASN1_R_BAD_OBJECT_HEADER;
+		goto err;
+		}
+
+	if (tag != V_ASN1_INTEGER)
 		{
+		i=ASN1_R_EXPECTING_AN_INTEGER;
+		goto err;
+		}
+
+	/* We must Malloc stuff, even for 0 bytes otherwise it
+	 * signifies a missing NULL parameter. */
+	s=(unsigned char *)Malloc((int)len+1);
+	if (s == NULL)
+		{
+		i=ERR_R_MALLOC_FAILURE;
+		goto err;
+		}
+	to=s;
 		ret->type=V_ASN1_INTEGER;
 		if ((*p == 0) && (len != 1))
 			{
@@ -172,7 +305,6 @@ ASN1_INTEGER *d2i_ASN1_INTEGER(ASN1_INTEGER **a, unsigned char **pp,
 			}
 		memcpy(s,p,(int)len);
 		p+=len;
-		}
 
 	if (ret->data != NULL) Free((char *)ret->data);
 	ret->data=s;
@@ -181,7 +313,7 @@ ASN1_INTEGER *d2i_ASN1_INTEGER(ASN1_INTEGER **a, unsigned char **pp,
 	*pp=p;
 	return(ret);
 err:
-	ASN1err(ASN1_F_D2I_ASN1_INTEGER,i);
+	ASN1err(ASN1_F_D2I_ASN1_UINTEGER,i);
 	if ((ret != NULL) && ((a == NULL) || (*a != ret)))
 		ASN1_INTEGER_free(ret);
 	return(NULL);
@@ -220,7 +352,6 @@ int ASN1_INTEGER_set(ASN1_INTEGER *a, long v)
 		d>>=8;
 		}
 	j=0;
-	if (v < 0) a->data[j++]=0;
 	for (k=i-1; k >=0; k--)
 		a->data[j++]=buf[k];
 	a->length=j;
@@ -270,7 +401,8 @@ ASN1_INTEGER *BN_to_ASN1_INTEGER(BIGNUM *bn, ASN1_INTEGER *ai)
 		ASN1err(ASN1_F_BN_TO_ASN1_INTEGER,ERR_R_NESTED_ASN1_ERROR);
 		goto err;
 		}
-	ret->type=V_ASN1_INTEGER;
+	if(bn->neg) ret->type = V_ASN1_NEG_INTEGER;
+	else ret->type=V_ASN1_INTEGER;
 	j=BN_num_bits(bn);
 	len=((j == 0)?0:((j/8)+1));
 	ret->data=(unsigned char *)Malloc(len+4);
@@ -287,5 +419,6 @@ BIGNUM *ASN1_INTEGER_to_BN(ASN1_INTEGER *ai, BIGNUM *bn)
 
 	if ((ret=BN_bin2bn(ai->data,ai->length,bn)) == NULL)
 		ASN1err(ASN1_F_ASN1_INTEGER_TO_BN,ASN1_R_BN_LIB);
+	if(ai->type == V_ASN1_NEG_INTEGER) bn->neg = 1;
 	return(ret);
 	}

crypto/asn1/asn1.h

@@ -518,6 +518,8 @@ int 		d2i_ASN1_BOOLEAN(int *a,unsigned char **pp,long length);
 int		i2d_ASN1_INTEGER(ASN1_INTEGER *a,unsigned char **pp);
 ASN1_INTEGER *d2i_ASN1_INTEGER(ASN1_INTEGER **a,unsigned char **pp,
 			long length);
+ASN1_INTEGER *d2i_ASN1_UINTEGER(ASN1_INTEGER **a,unsigned char **pp,
+			long length);
 
 int		i2d_ASN1_ENUMERATED(ASN1_ENUMERATED *a,unsigned char **pp);
 ASN1_ENUMERATED *d2i_ASN1_ENUMERATED(ASN1_ENUMERATED **a,unsigned char **pp,
@@ -739,6 +741,7 @@ ASN1_STRING *ASN1_pack_string(char *obj, int (*i2d)(), ASN1_OCTET_STRING **oct);
 #define ASN1_F_D2I_ASN1_TIME				 224
 #define ASN1_F_D2I_ASN1_TYPE				 133
 #define ASN1_F_D2I_ASN1_TYPE_BYTES			 134
+#define ASN1_F_D2I_ASN1_UINTEGER			 280
 #define ASN1_F_D2I_ASN1_UTCTIME				 135
 #define ASN1_F_D2I_ASN1_UTF8STRING			 266
 #define ASN1_F_D2I_ASN1_VISIBLESTRING			 267

crypto/asn1/asn1_err.c

@@ -115,6 +115,7 @@ static ERR_STRING_DATA ASN1_str_functs[]=
 {ERR_PACK(0,ASN1_F_D2I_ASN1_TIME,0),	"d2i_ASN1_TIME"},
 {ERR_PACK(0,ASN1_F_D2I_ASN1_TYPE,0),	"d2i_ASN1_TYPE"},
 {ERR_PACK(0,ASN1_F_D2I_ASN1_TYPE_BYTES,0),	"d2i_ASN1_type_bytes"},
+{ERR_PACK(0,ASN1_F_D2I_ASN1_UINTEGER,0),	"d2i_ASN1_UINTEGER"},
 {ERR_PACK(0,ASN1_F_D2I_ASN1_UTCTIME,0),	"d2i_ASN1_UTCTIME"},
 {ERR_PACK(0,ASN1_F_D2I_ASN1_UTF8STRING,0),	"d2i_ASN1_UTF8STRING"},
 {ERR_PACK(0,ASN1_F_D2I_ASN1_VISIBLESTRING,0),	"d2i_ASN1_VISIBLESTRING"},

crypto/asn1/d2i_dsap.c

@@ -64,6 +64,10 @@
 #include <openssl/objects.h>
 #include <openssl/asn1_mac.h>
 
+#ifdef NEG_PUBKEY_BUG
+#define d2i_ASN1_INTEGER d2i_ASN1_UINTEGER
+#endif
+
 DSA *d2i_DSAparams(DSA **a, unsigned char **pp, long length)
 	{
 	int i=ERR_R_NESTED_ASN1_ERROR;

crypto/asn1/d2i_r_pu.c

@@ -64,6 +64,10 @@
 #include <openssl/objects.h>
 #include <openssl/asn1_mac.h>
 
+#ifdef NEG_PUBKEY_BUG
+#define d2i_ASN1_INTEGER d2i_ASN1_UINTEGER
+#endif
+
 RSA *d2i_RSAPublicKey(RSA **a, unsigned char **pp, long length)
 	{
 	int i=ASN1_R_PARSING;

crypto/asn1/d2i_s_pu.c

@@ -66,6 +66,10 @@
 #include <openssl/objects.h>
 #include <openssl/asn1_mac.h>
 
+#ifdef NEG_PUBKEY_BUG
+#define d2i_ASN1_INTEGER d2i_ASN1_UINTEGER
+#endif
+
 DSA *d2i_DSAPublicKey(DSA **a, unsigned char **pp, long length)
 	{
 	int i=ASN1_R_PARSING;

util/libeay.num

@@ -1624,3 +1624,4 @@ X509V3_EXT_add_list                     1648
 EVP_CIPHER_type                         1649
 EVP_PBE_CipherInit                      1650
 X509V3_add_value_bool_nf                1651
+d2i_ASN1_UINTEGER                       1652