From: miod Date: Sat, 12 Jul 2014 16:42:47 +0000 (+0000) Subject: A few fixes/improvements: X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=c686d77397dc62ec48c15d670b0d34b8eb5a0877;p=openbsd A few fixes/improvements: - first, BN_free == BN_clear_free in our libcrypto, so we do not need to treat CBIGNUM (crypto BN) separately from BIGNUM (regular BN). - then, in bn_i2c(), since BN_bn2bin returns BN_num_bytes(input), take advantage of this to avoid calling BN_num_bytes() a second time. BN_num_bytes() is cheap, but this not a reason to perform redundant work. - finally, in bn_c2i, if bn_new() fails, return early. Otherwise BN_bin2bn will try to create a BN too, and although this will probably fail since we were already out of memory, if we are on a threaded process and suddenly the allocation succeeds, we will leak it since it will never be stored in *pval. ok jsing@ --- diff --git a/lib/libcrypto/asn1/x_bignum.c b/lib/libcrypto/asn1/x_bignum.c index 18ec64eeefd..dafe9b3016a 100644 --- a/lib/libcrypto/asn1/x_bignum.c +++ b/lib/libcrypto/asn1/x_bignum.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x_bignum.c,v 1.6 2014/07/11 08:44:47 jsing Exp $ */ +/* $OpenBSD: x_bignum.c,v 1.7 2014/07/12 16:42:47 miod Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project 2000. */ @@ -61,14 +61,13 @@ #include #include -/* Custom primitive type for BIGNUM handling. This reads in an ASN1_INTEGER as a - * BIGNUM directly. Currently it ignores the sign which isn't a problem since all - * BIGNUMs used are non negative and anything that looks negative is normally due - * to an encoding error. +/* + * Custom primitive type for BIGNUM handling. This reads in an ASN1_INTEGER as a + * BIGNUM directly. Currently it ignores the sign which isn't a problem since + * all BIGNUMs used are non negative and anything that looks negative is + * normally due to an encoding error. */ -#define BN_SENSITIVE 1 - static int bn_new(ASN1_VALUE **pval, const ASN1_ITEM *it); static void bn_free(ASN1_VALUE **pval, const ASN1_ITEM *it); @@ -92,7 +91,7 @@ ASN1_ITYPE_PRIMITIVE, V_ASN1_INTEGER, NULL, 0, &bignum_pf, 0, "BIGNUM" ASN1_ITEM_end(BIGNUM) ASN1_ITEM_start(CBIGNUM) -ASN1_ITYPE_PRIMITIVE, V_ASN1_INTEGER, NULL, 0, &bignum_pf, BN_SENSITIVE, "BIGNUM" +ASN1_ITYPE_PRIMITIVE, V_ASN1_INTEGER, NULL, 0, &bignum_pf, 0, "BIGNUM" ASN1_ITEM_end(CBIGNUM) static int @@ -108,12 +107,9 @@ bn_new(ASN1_VALUE **pval, const ASN1_ITEM *it) static void bn_free(ASN1_VALUE **pval, const ASN1_ITEM *it) { - if (!*pval) + if (*pval == NULL) return; - if (it->size & BN_SENSITIVE) - BN_clear_free((BIGNUM *)*pval); - else - BN_free((BIGNUM *)*pval); + BN_clear_free((BIGNUM *)*pval); *pval = NULL; } @@ -121,9 +117,9 @@ static int bn_i2c(ASN1_VALUE **pval, unsigned char *cont, int *putype, const ASN1_ITEM *it) { BIGNUM *bn; - int pad; + int pad, len; - if (!*pval) + if (*pval == NULL) return -1; bn = (BIGNUM *)*pval; /* If MSB set in an octet we need a padding byte */ @@ -134,9 +130,10 @@ bn_i2c(ASN1_VALUE **pval, unsigned char *cont, int *putype, const ASN1_ITEM *it) if (cont) { if (pad) *cont++ = 0; - BN_bn2bin(bn, cont); - } - return pad + BN_num_bytes(bn); + len = BN_bn2bin(bn, cont); + } else + len = BN_num_bytes(bn); + return pad + len; } static int @@ -145,8 +142,10 @@ bn_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len, int utype, { BIGNUM *bn; - if (!*pval) - bn_new(pval, it); + if (*pval == NULL) { + if (bn_new(pval, it) == 0) + return 0; + } bn = (BIGNUM *)*pval; if (!BN_bin2bn(cont, len, bn)) { bn_free(pval, it); diff --git a/lib/libssl/src/crypto/asn1/x_bignum.c b/lib/libssl/src/crypto/asn1/x_bignum.c index 18ec64eeefd..dafe9b3016a 100644 --- a/lib/libssl/src/crypto/asn1/x_bignum.c +++ b/lib/libssl/src/crypto/asn1/x_bignum.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x_bignum.c,v 1.6 2014/07/11 08:44:47 jsing Exp $ */ +/* $OpenBSD: x_bignum.c,v 1.7 2014/07/12 16:42:47 miod Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project 2000. */ @@ -61,14 +61,13 @@ #include #include -/* Custom primitive type for BIGNUM handling. This reads in an ASN1_INTEGER as a - * BIGNUM directly. Currently it ignores the sign which isn't a problem since all - * BIGNUMs used are non negative and anything that looks negative is normally due - * to an encoding error. +/* + * Custom primitive type for BIGNUM handling. This reads in an ASN1_INTEGER as a + * BIGNUM directly. Currently it ignores the sign which isn't a problem since + * all BIGNUMs used are non negative and anything that looks negative is + * normally due to an encoding error. */ -#define BN_SENSITIVE 1 - static int bn_new(ASN1_VALUE **pval, const ASN1_ITEM *it); static void bn_free(ASN1_VALUE **pval, const ASN1_ITEM *it); @@ -92,7 +91,7 @@ ASN1_ITYPE_PRIMITIVE, V_ASN1_INTEGER, NULL, 0, &bignum_pf, 0, "BIGNUM" ASN1_ITEM_end(BIGNUM) ASN1_ITEM_start(CBIGNUM) -ASN1_ITYPE_PRIMITIVE, V_ASN1_INTEGER, NULL, 0, &bignum_pf, BN_SENSITIVE, "BIGNUM" +ASN1_ITYPE_PRIMITIVE, V_ASN1_INTEGER, NULL, 0, &bignum_pf, 0, "BIGNUM" ASN1_ITEM_end(CBIGNUM) static int @@ -108,12 +107,9 @@ bn_new(ASN1_VALUE **pval, const ASN1_ITEM *it) static void bn_free(ASN1_VALUE **pval, const ASN1_ITEM *it) { - if (!*pval) + if (*pval == NULL) return; - if (it->size & BN_SENSITIVE) - BN_clear_free((BIGNUM *)*pval); - else - BN_free((BIGNUM *)*pval); + BN_clear_free((BIGNUM *)*pval); *pval = NULL; } @@ -121,9 +117,9 @@ static int bn_i2c(ASN1_VALUE **pval, unsigned char *cont, int *putype, const ASN1_ITEM *it) { BIGNUM *bn; - int pad; + int pad, len; - if (!*pval) + if (*pval == NULL) return -1; bn = (BIGNUM *)*pval; /* If MSB set in an octet we need a padding byte */ @@ -134,9 +130,10 @@ bn_i2c(ASN1_VALUE **pval, unsigned char *cont, int *putype, const ASN1_ITEM *it) if (cont) { if (pad) *cont++ = 0; - BN_bn2bin(bn, cont); - } - return pad + BN_num_bytes(bn); + len = BN_bn2bin(bn, cont); + } else + len = BN_num_bytes(bn); + return pad + len; } static int @@ -145,8 +142,10 @@ bn_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len, int utype, { BIGNUM *bn; - if (!*pval) - bn_new(pval, it); + if (*pval == NULL) { + if (bn_new(pval, it) == 0) + return 0; + } bn = (BIGNUM *)*pval; if (!BN_bin2bn(cont, len, bn)) { bn_free(pval, it);