commit 2b314fa3a9b065fc5f0b2fe9567b009afed94d20
author David Benjamin <davidben@google.com> Tue Aug 02 12:49:38 2016 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Tue Aug 02 17:35:16 2016 +0000
tree 8d79cf677d7182fe58c6072bc35045534d3f9ad8
parent 7fcbfdbdf3c42c2ed75d8328d215487a44ddf916

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

diff --git a/crypto/bn/bn_test.cc b/crypto/bn/bn_test.cc
index 14dcead..554d0c6 100644
--- a/crypto/bn/bn_test.cc
+++ b/crypto/bn/bn_test.cc
@@ -1138,7 +1138,7 @@
     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 @@
     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 @@
     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 @@
   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 @@
   }
 
   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();