Tolerate -0 better in BN_bn2{dec,hex}

Negative zeros are nuts, but it will probably be a while before we've
fixed everything that can create them. Fix both to consistently print
'-0' rather than '0' so failures are easier to diagnose (BN_cmp believes
the values are different.)

Change-Id: Ic38d90601b43f66219d8f44ca085432106cf98e3
Reviewed-on: https://boringssl-review.googlesource.com/9073
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

crypto/bn/bn_test.cc

@@ -1138,7 +1138,7 @@ static bool TestNegativeZero(BN_CTX *ctx) {
     return false;
   }
   if (!BN_is_zero(c.get()) || BN_is_negative(c.get())) {
-    fprintf(stderr, "Multiplication test failed!\n");
+    fprintf(stderr, "Multiplication test failed.\n");
     return false;
   }
 
@@ -1152,7 +1152,7 @@ static bool TestNegativeZero(BN_CTX *ctx) {
     return false;
   }
   if (!BN_is_zero(d.get()) || BN_is_negative(d.get())) {
-    fprintf(stderr, "Division test failed!\n");
+    fprintf(stderr, "Division test failed.\n");
     return false;
   }
 
@@ -1164,7 +1164,27 @@ static bool TestNegativeZero(BN_CTX *ctx) {
     return false;
   }
   if (!BN_is_zero(c.get()) || BN_is_negative(c.get())) {
-    fprintf(stderr, "Division test failed!\n");
+    fprintf(stderr, "Division test failed.\n");
+    return false;
+  }
+
+  // Test that BN_set_negative will not produce a negative zero.
+  BN_zero(a.get());
+  BN_set_negative(a.get(), 1);
+  if (BN_is_negative(a.get())) {
+    fprintf(stderr, "BN_set_negative produced a negative zero.\n");
+    return false;
+  }
+
+  // Test that forcibly creating a negative zero does not break |BN_bn2hex| or
+  // |BN_bn2dec|.
+  a->neg = 1;
+  ScopedOpenSSLString dec(BN_bn2dec(a.get()));
+  ScopedOpenSSLString hex(BN_bn2hex(a.get()));
+  if (!dec || !hex ||
+      strcmp(dec.get(), "-0") != 0 ||
+      strcmp(hex.get(), "-0") != 0) {
+    fprintf(stderr, "BN_bn2dec or BN_bn2hex failed with negative zero.\n");
     return false;
   }
 
@@ -1183,39 +1203,43 @@ static bool TestBadModulus(BN_CTX *ctx) {
   BN_zero(zero.get());
 
   if (BN_div(a.get(), b.get(), BN_value_one(), zero.get(), ctx)) {
-    fprintf(stderr, "Division by zero succeeded!\n");
+    fprintf(stderr, "Division by zero unexpectedly succeeded.\n");
     return false;
   }
   ERR_clear_error();
 
   if (BN_mod_mul(a.get(), BN_value_one(), BN_value_one(), zero.get(), ctx)) {
-    fprintf(stderr, "BN_mod_mul with zero modulus succeeded!\n");
+    fprintf(stderr, "BN_mod_mul with zero modulus unexpectedly succeeded.\n");
     return false;
   }
   ERR_clear_error();
 
   if (BN_mod_exp(a.get(), BN_value_one(), BN_value_one(), zero.get(), ctx)) {
-    fprintf(stderr, "BN_mod_exp with zero modulus succeeded!\n");
+    fprintf(stderr, "BN_mod_exp with zero modulus unexpectedly succeeded.\n");
     return 0;
   }
   ERR_clear_error();
 
   if (BN_mod_exp_mont(a.get(), BN_value_one(), BN_value_one(), zero.get(), ctx,
                       NULL)) {
-    fprintf(stderr, "BN_mod_exp_mont with zero modulus succeeded!\n");
+    fprintf(stderr,
+            "BN_mod_exp_mont with zero modulus unexpectedly succeeded.\n");
     return 0;
   }
   ERR_clear_error();
 
   if (BN_mod_exp_mont_consttime(a.get(), BN_value_one(), BN_value_one(),
                                 zero.get(), ctx, nullptr)) {
-    fprintf(stderr, "BN_mod_exp_mont_consttime with zero modulus succeeded!\n");
+    fprintf(stderr,
+            "BN_mod_exp_mont_consttime with zero modulus unexpectedly "
+            "succeeded.\n");
     return 0;
   }
   ERR_clear_error();
 
   if (BN_MONT_CTX_set(mont.get(), zero.get(), ctx)) {
-    fprintf(stderr, "BN_MONT_CTX_set succeeded for zero modulus!\n");
+    fprintf(stderr,
+            "BN_MONT_CTX_set unexpectedly succeeded for zero modulus.\n");
     return false;
   }
   ERR_clear_error();
@@ -1227,21 +1251,25 @@ static bool TestBadModulus(BN_CTX *ctx) {
   }
 
   if (BN_MONT_CTX_set(mont.get(), b.get(), ctx)) {
-    fprintf(stderr, "BN_MONT_CTX_set succeeded for even modulus!\n");
+    fprintf(stderr,
+            "BN_MONT_CTX_set unexpectedly succeeded for even modulus.\n");
     return false;
   }
   ERR_clear_error();
 
   if (BN_mod_exp_mont(a.get(), BN_value_one(), BN_value_one(), b.get(), ctx,
                       NULL)) {
-    fprintf(stderr, "BN_mod_exp_mont with even modulus succeeded!\n");
+    fprintf(stderr,
+            "BN_mod_exp_mont with even modulus unexpectedly succeeded.\n");
     return 0;
   }
   ERR_clear_error();
 
   if (BN_mod_exp_mont_consttime(a.get(), BN_value_one(), BN_value_one(),
                                 b.get(), ctx, nullptr)) {
-    fprintf(stderr, "BN_mod_exp_mont_consttime with even modulus succeeded!\n");
+    fprintf(stderr,
+            "BN_mod_exp_mont_consttime with even modulus unexpectedly "
+            "succeeded.\n");
     return 0;
   }
   ERR_clear_error();

crypto/bn/convert.c

@@ -204,17 +204,14 @@ int BN_bn2cbb_padded(CBB *out, size_t len, const BIGNUM *in) {
 static const char hextable[] = "0123456789abcdef";
 
 char *BN_bn2hex(const BIGNUM *bn) {
-  int i, j, v, z = 0;
-  char *buf;
-  char *p;
-
-  buf = OPENSSL_malloc(bn->top * BN_BYTES * 2 + 2);
+  char *buf = OPENSSL_malloc(1 /* leading '-' */ + 1 /* zero is non-empty */ +
+                             bn->top * BN_BYTES * 2 + 1 /* trailing NUL */);
   if (buf == NULL) {
     OPENSSL_PUT_ERROR(BN, ERR_R_MALLOC_FAILURE);
     return NULL;
   }
 
-  p = buf;
+  char *p = buf;
   if (bn->neg) {
     *(p++) = '-';
   }
@@ -223,10 +220,11 @@ char *BN_bn2hex(const BIGNUM *bn) {
     *(p++) = '0';
   }
 
-  for (i = bn->top - 1; i >= 0; i--) {
-    for (j = BN_BITS2 - 8; j >= 0; j -= 8) {
+  int z = 0;
+  for (int i = bn->top - 1; i >= 0; i--) {
+    for (int j = BN_BITS2 - 8; j >= 0; j -= 8) {
       /* strip leading zeros */
-      v = ((int)(bn->d[i] >> (long)j)) & 0xff;
+      int v = ((int)(bn->d[i] >> (long)j)) & 0xff;
       if (z || v != 0) {
         *(p++) = hextable[v >> 4];
         *(p++) = hextable[v & 0x0f];
@@ -399,14 +397,15 @@ char *BN_bn2dec(const BIGNUM *a) {
 #define BUF_REMAIN (num + 3 - (size_t)(p - buf))
   p = buf;
   lp = bn_data;
+
+  if (BN_is_negative(t)) {
+    *p++ = '-';
+  }
+
   if (BN_is_zero(t)) {
     *(p++) = '0';
     *(p++) = '\0';
   } else {
-    if (BN_is_negative(t)) {
-      *p++ = '-';
-    }
-
     while (!BN_is_zero(t)) {
       *lp = BN_div_word(t, BN_DEC_CONV);
       lp++;