Support negative zero as MPI test input

The bignum module does not officially support "negative zero" (an
mbedtls_mpi object with s=-1 and all limbs zero). However, we have a
history of bugs where a function that should produce an official
zero (with s=1), produces a negative zero in some circumstances. So it's
good to check that the bignum functions are robust when passed a negative
zero as input. And for that, we need a way to construct a negative zero
from test case arguments.

There are checks that functions don't produce negative zeros as output in
the test suite. Skip those checks if there's a negative zero input: we
don't want functions to _create_ negative zeros, but we don't mind if
they _propagate_ negative zeros.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

tests/include/test/helpers.h

@@ -295,13 +295,19 @@ int mbedtls_test_read_mpi_core( mbedtls_mpi_uint **pX, size_t *plimbs,
 
 /** Read an MPI from a hexadecimal string.
  *
- * Like mbedtls_mpi_read_string(), but size the resulting bignum based
- * on the number of digits in the string. In particular, construct a
- * bignum with 0 limbs for an empty string, and a bignum with leading 0
- * limbs if the string has sufficiently many leading 0 digits.
+ * Like mbedtls_mpi_read_string(), but with tighter guarantees around
+ * edge cases.
  *
- * This is important so that the "0 (null)" and "0 (1 limb)" and
- * "leading zeros" test cases do what they claim.
+ * - This function guarantees that if \p s begins with '-' then the sign
+ *   bit of the result will be negative, even if the value is 0.
+ *   When this function encounters such a "negative 0", it
+ *   increments #mbedtls_test_read_mpi.
+ * - The size of the result is exactly the minimum number of limbs needed
+ *   to fit the digits in the input. In particular, this function constructs
+ *   a bignum with 0 limbs for an empty string, and a bignum with leading 0
+ *   limbs if the string has sufficiently many leading 0 digits.
+ *   This is important so that the "0 (null)" and "0 (1 limb)" and
+ *   "leading zeros" test cases do what they claim.
  *
  * \param[out] X        The MPI object to populate. It must be initialized.
  * \param[in] s         The null-terminated hexadecimal string to read from.
@@ -309,6 +315,14 @@ int mbedtls_test_read_mpi_core( mbedtls_mpi_uint **pX, size_t *plimbs,
  * \return \c 0 on success, an \c MBEDTLS_ERR_MPI_xxx error code otherwise.
  */
 int mbedtls_test_read_mpi( mbedtls_mpi *X, const char *s );
+
+/** Nonzero if the current test case had an input parsed with
+ * mbedtls_test_read_mpi() that is a negative 0 (`"-"`, `"-0"`, `"-00"`, etc.,
+ * constructing a result with the sign bit set to -1 and the value being
+ * all-limbs-0, which is not a valid representation in #mbedtls_mpi but is
+ * tested for robustness).
+ */
+extern unsigned mbedtls_test_case_uses_negative_0;
 #endif /* MBEDTLS_BIGNUM_C */
 
 #endif /* TEST_HELPERS_H */

tests/src/helpers.c

@@ -89,6 +89,10 @@ void mbedtls_test_set_step( unsigned long step )
     mbedtls_test_info.step = step;
 }
 
+#if defined(MBEDTLS_BIGNUM_C)
+unsigned mbedtls_test_case_uses_negative_0 = 0;
+#endif
+
 void mbedtls_test_info_reset( void )
 {
     mbedtls_test_info.result = MBEDTLS_TEST_RESULT_SUCCESS;
@@ -98,6 +102,9 @@ void mbedtls_test_info_reset( void )
     mbedtls_test_info.filename = 0;
     memset( mbedtls_test_info.line1, 0, sizeof( mbedtls_test_info.line1 ) );
     memset( mbedtls_test_info.line2, 0, sizeof( mbedtls_test_info.line2 ) );
+#if defined(MBEDTLS_BIGNUM_C)
+    mbedtls_test_case_uses_negative_0 = 0;
+#endif
 }
 
 int mbedtls_test_equal( const char *test, int line_no, const char* filename,
@@ -396,6 +403,15 @@ exit:
 
 int mbedtls_test_read_mpi( mbedtls_mpi *X, const char *s )
 {
+    int negative = 0;
+    /* Always set the sign bit to -1 if the input has a minus sign, even for 0.
+     * This creates an invalid representation, which mbedtls_mpi_read_string()
+     * avoids but we want to be able to create that in test data. */
+    if( s[0] == '-' )
+    {
+        ++s;
+        negative = 1;
+    }
     /* mbedtls_mpi_read_string() currently retains leading zeros.
      * It always allocates at least one limb for the value 0. */
     if( s[0] == 0 )
@@ -403,7 +419,15 @@ int mbedtls_test_read_mpi( mbedtls_mpi *X, const char *s )
         mbedtls_mpi_free( X );
         return( 0 );
     }
-    else
-        return( mbedtls_mpi_read_string( X, 16, s ) );
+    int ret = mbedtls_mpi_read_string( X, 16, s );
+    if( ret != 0 )
+        return( ret );
+    if( negative )
+    {
+        if( mbedtls_mpi_cmp_int( X, 0 ) == 0 )
+            ++mbedtls_test_case_uses_negative_0;
+        X->s = -1;
+    }
+    return( 0 );
 }
 #endif

tests/suites/test_suite_bignum.function

@@ -13,10 +13,21 @@
  * constructing the value. */
 static int sign_is_valid( const mbedtls_mpi *X )
 {
+    /* Only +1 and -1 are valid sign bits, not e.g. 0 */
     if( X->s != 1 && X->s != -1 )
-        return( 0 ); // invalid sign bit, e.g. 0
-    if( mbedtls_mpi_bitlen( X ) == 0 && X->s != 1 )
-        return( 0 ); // negative zero
+        return( 0 );
+
+    /* The value 0 must be represented with the sign +1. A "negative zero"
+     * with s=-1 is an invalid representation. Forbid that. As an exception,
+     * we sometimes test the robustness of library functions when given
+     * a negative zero input. If a test case has a negative zero as input,
+     * we don't mind if the function has a negative zero output. */
+    if( ! mbedtls_test_case_uses_negative_0 &&
+        mbedtls_mpi_bitlen( X ) == 0 && X->s != 1 )
+    {
+        return( 0 );
+    }
+
     return( 1 );
 }
 

tests/suites/test_suite_bignum.misc.data

@@ -1144,6 +1144,18 @@ mpi_div_mpi:"":"1":"":"":0
 Test mbedtls_mpi_div_mpi: 0 (null) / -1
 mpi_div_mpi:"":"-1":"":"":0
 
+Test mbedtls_mpi_div_mpi: -0 (null) / 1
+mpi_div_mpi:"-":"1":"":"":0
+
+Test mbedtls_mpi_div_mpi: -0 (null) / -1
+mpi_div_mpi:"-":"-1":"":"":0
+
+Test mbedtls_mpi_div_mpi: -0 (null) / 42
+mpi_div_mpi:"-":"2a":"":"":0
+
+Test mbedtls_mpi_div_mpi: -0 (null) / -42
+mpi_div_mpi:"-":"-2a":"":"":0
+
 Test mbedtls_mpi_div_mpi #1
 mpi_div_mpi:"9e22d6da18a33d1ef28d2a82242b3f6e9c9742f63e5d440f58a190bfaf23a7866e67589adb80":"22":"4a6abf75b13dc268ea9cc8b5b6aaf0ac85ecd437a4e0987fb13cf8d2acc57c0306c738c1583":"1a":0
 
@@ -1204,6 +1216,18 @@ mpi_mod_mpi:"":"1":"":0
 Test mbedtls_mpi_mod_mpi: 0 (null) % -1
 mpi_mod_mpi:"":"-1":"":MBEDTLS_ERR_MPI_NEGATIVE_VALUE
 
+Test mbedtls_mpi_mod_mpi: -0 (null) % 1
+mpi_mod_mpi:"-":"1":"":0
+
+Test mbedtls_mpi_mod_mpi: -0 (null) % -1
+mpi_mod_mpi:"-":"-1":"":MBEDTLS_ERR_MPI_NEGATIVE_VALUE
+
+Test mbedtls_mpi_mod_mpi: -0 (null) % 42
+mpi_mod_mpi:"-":"2a":"":0
+
+Test mbedtls_mpi_mod_mpi: -0 (null) % -42
+mpi_mod_mpi:"-":"-2a":"":MBEDTLS_ERR_MPI_NEGATIVE_VALUE
+
 Base test mbedtls_mpi_mod_int #1
 mpi_mod_int:"3e8":"d":"c":0