From 3f1014960353af29a05a7b5dac40afd30d4f9bb3 Mon Sep 17 00:00:00 2001 From: Rich Salz Date: Mon, 5 Sep 2016 18:08:43 -0400 Subject: Misc BN fixes Never output -0; make "negative zero" an impossibility. Do better checking on BN_rand top/bottom requirements and #bits. Update doc. Ignoring trailing garbage in BN_asc2bn. Port this commit from boringSSL: https://boringssl.googlesource.com/boringssl/+/899b9b19a4cd3fe526aaf5047ab9234cdca19f7d%5E!/ Ensure |BN_div| never gives negative zero in the no_branch code. Have |bn_correct_top| fix |bn->neg| if the input is zero so that we don't have negative zeros lying around. Thanks to Brian Smith for noticing. Reviewed-by: Richard Levitte (cherry picked from commit 01c09f9fde5793e0b3712d602b02e2aed4908e8d) (Some manual work required) --- crypto/bn/bn.h | 2 ++ crypto/bn/bn_print.c | 23 +++++++++++++---------- crypto/bn/bn_rand.c | 13 ++++++++----- doc/crypto/BN_bn2bin.pod | 4 +++- doc/crypto/BN_rand.pod | 6 +++++- 5 files changed, 31 insertions(+), 17 deletions(-) diff --git a/crypto/bn/bn.h b/crypto/bn/bn.h index 86264ae631..633d1b1f60 100644 --- a/crypto/bn/bn.h +++ b/crypto/bn/bn.h @@ -842,6 +842,8 @@ int RAND_pseudo_bytes(unsigned char *buf, int num); if (*(ftl--)) break; \ (a)->top = tmp_top; \ } \ + if ((a)->top == 0) \ + (a)->neg = 0; \ bn_pollute(a); \ } diff --git a/crypto/bn/bn_print.c b/crypto/bn/bn_print.c index a9ff271b9a..f121fb6e9a 100644 --- a/crypto/bn/bn_print.c +++ b/crypto/bn/bn_print.c @@ -72,12 +72,9 @@ char *BN_bn2hex(const BIGNUM *a) char *buf; char *p; - if (a->neg && BN_is_zero(a)) { - /* "-0" == 3 bytes including NULL terminator */ - buf = OPENSSL_malloc(3); - } else { - buf = OPENSSL_malloc(a->top * BN_BYTES * 2 + 2); - } + if (BN_is_zero(a)) + return OPENSSL_strdup("0"); + buf = OPENSSL_malloc(a->top * BN_BYTES * 2 + 2); if (buf == NULL) { BNerr(BN_F_BN_BN2HEX, ERR_R_MALLOC_FAILURE); goto err; @@ -244,10 +241,12 @@ int BN_hex2bn(BIGNUM **bn, const char *a) } ret->top = h; bn_correct_top(ret); - ret->neg = neg; *bn = ret; bn_check_top(ret); + /* Don't set the negative flag if it's zero. */ + if (ret->top != 0) + ret->neg = neg; return (num); err: if (*bn == NULL) @@ -299,7 +298,7 @@ int BN_dec2bn(BIGNUM **bn, const char *a) if (j == BN_DEC_NUM) j = 0; l = 0; - while (*a) { + while (--i >= 0) { l *= 10; l += *a - '0'; a++; @@ -310,11 +309,13 @@ int BN_dec2bn(BIGNUM **bn, const char *a) j = 0; } } - ret->neg = neg; bn_correct_top(ret); *bn = ret; bn_check_top(ret); + /* Don't set the negative flag if it's zero. */ + if (ret->top != 0) + ret->neg = neg; return (num); err: if (*bn == NULL) @@ -325,6 +326,7 @@ int BN_dec2bn(BIGNUM **bn, const char *a) int BN_asc2bn(BIGNUM **bn, const char *a) { const char *p = a; + if (*p == '-') p++; @@ -335,7 +337,8 @@ int BN_asc2bn(BIGNUM **bn, const char *a) if (!BN_dec2bn(bn, p)) return 0; } - if (*a == '-') + /* Don't set the negative flag if it's zero. */ + if (*a == '-' && (*bn)->top != 0) (*bn)->neg = 1; return 1; } diff --git a/crypto/bn/bn_rand.c b/crypto/bn/bn_rand.c index 2266d22b66..60d3f2260b 100644 --- a/crypto/bn/bn_rand.c +++ b/crypto/bn/bn_rand.c @@ -121,15 +121,14 @@ static int bnrand(int pseudorand, BIGNUM *rnd, int bits, int top, int bottom) int ret = 0, bit, bytes, mask; time_t tim; - if (bits < 0 || (bits == 1 && top > 0)) { - BNerr(BN_F_BNRAND, BN_R_BITS_TOO_SMALL); - return 0; - } - if (bits == 0) { + if (top != -1 || bottom != 0) + goto toosmall; BN_zero(rnd); return 1; } + if (bits < 0 || (bits == 1 && top > 0)) + goto toosmall; bytes = (bits + 7) / 8; bit = (bits - 1) % 8; @@ -195,6 +194,10 @@ static int bnrand(int pseudorand, BIGNUM *rnd, int bits, int top, int bottom) } bn_check_top(rnd); return (ret); + +toosmall: + BNerr(BN_F_BNRAND, BN_R_BITS_TOO_SMALL); + return 0; } int BN_rand(BIGNUM *rnd, int bits, int top, int bottom) diff --git a/doc/crypto/BN_bn2bin.pod b/doc/crypto/BN_bn2bin.pod index a4b17ca60a..3bed47f8f1 100644 --- a/doc/crypto/BN_bn2bin.pod +++ b/doc/crypto/BN_bn2bin.pod @@ -42,7 +42,9 @@ BN_hex2bn() converts the string B containing a hexadecimal number to a B and stores it in **B. If *B is NULL, a new B is created. If B is NULL, it only computes the number's length in hexadecimal digits. If the string starts with '-', the -number is negative. BN_dec2bn() is the same using the decimal system. +number is negative. +A "negative zero" is converted to zero. +BN_dec2bn() is the same using the decimal system. BN_print() and BN_print_fp() write the hexadecimal encoding of B, with a leading '-' for negative numbers, to the B or B diff --git a/doc/crypto/BN_rand.pod b/doc/crypto/BN_rand.pod index e8cbf658b4..a1513a9526 100644 --- a/doc/crypto/BN_rand.pod +++ b/doc/crypto/BN_rand.pod @@ -19,7 +19,11 @@ BN_rand, BN_pseudo_rand, BN_rand_range, BN_pseudo_rand_range - generate pseudo-r =head1 DESCRIPTION BN_rand() generates a cryptographically strong pseudo-random number of -B in length and stores it in B. If B is -1, the +B in length and stores it in B. +If B is less than zero, or too small to +accomodate the requirements specified by the B and B +parameters, an error is returned. +If B is -1, the most significant bit of the random number can be zero. If B is 0, it is set to 1, and if B is 1, the two most significant bits of the number will be set to 1, so that the product of two such random -- cgit v1.2.3