Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ECDH and ECDSA test suite fails on big-endian MIPS #33

Closed
tcoppi opened this issue Oct 18, 2013 · 4 comments
Closed

ECDH and ECDSA test suite fails on big-endian MIPS #33

tcoppi opened this issue Oct 18, 2013 · 4 comments

Comments

@tcoppi
Copy link

tcoppi commented Oct 18, 2013

It passes on my x86-64 dev box and on mipsel, so I'm guessing this is an endianness issue, but I haven't tested further. I have bisected it back to d589a0d.

Here's the test suite output:
$ make check
Running checks (Success if all tests PASSED)

  • test_suite_aes.ecb
    PASSED (77 / 77 tests (0 skipped))
  • test_suite_aes.cbc
    PASSED (72 / 72 tests (0 skipped))
  • test_suite_aes.cfb
    PASSED (72 / 72 tests (0 skipped))
  • test_suite_aes.rest
    PASSED (5 / 5 tests (0 skipped))
  • test_suite_arc4
    PASSED (9 / 9 tests (0 skipped))
  • test_suite_base64
    PASSED (20 / 20 tests (0 skipped))
  • test_suite_blowfish
    PASSED (103 / 103 tests (0 skipped))
  • test_suite_camellia
    PASSED (59 / 59 tests (0 skipped))
  • test_suite_cipher.aes
    PASSED (275 / 275 tests (0 skipped))
  • test_suite_cipher.arc4
    PASSED (26 / 26 tests (0 skipped))
  • test_suite_cipher.gcm
    PASSED (32 / 32 tests (0 skipped))
  • test_suite_cipher.blowfish
    PASSED (138 / 138 tests (0 skipped))
  • test_suite_cipher.camellia
    PASSED (190 / 190 tests (0 skipped))
  • test_suite_cipher.des
    PASSED (138 / 138 tests (0 skipped))
  • test_suite_cipher.null
    PASSED (24 / 24 tests (24 skipped))
  • test_suite_cipher.padding
    PASSED (43 / 43 tests (1 skipped))
  • test_suite_ctr_drbg
    PASSED (240 / 240 tests (0 skipped))
  • test_suite_debug
    PASSED (8 / 8 tests (2 skipped))
  • test_suite_des
    PASSED (80 / 80 tests (0 skipped))
  • test_suite_dhm
    PASSED (4 / 4 tests (0 skipped))
  • test_suite_ecdh
    ECDH primitive rfc 5903 p256 ...................................... FAILED
    mpi_cmp_mpi( &qA.X, &check ) == 0
    ECDH primitive rfc 5903 p384 ...................................... FAILED
    mpi_cmp_mpi( &qA.X, &check ) == 0
    ECDH primitive rfc 5903 p521 ...................................... FAILED
    mpi_cmp_mpi( &qA.X, &check ) == 0
    FAILED (7 / 10 tests (0 skipped))
    **** Failed ***************
  • test_suite_ecdsa
    ECDSA primitive rfc 4754 p256 ..................................... FAILED
    mpi_cmp_mpi( &r, &r_check ) == 0
    ECDSA primitive rfc 4754 p384 ..................................... FAILED
    mpi_cmp_mpi( &r, &r_check ) == 0
    ECDSA primitive rfc 4754 p521 ..................................... FAILED
    mpi_cmp_mpi( &r, &r_check ) == 0
    FAILED (10 / 13 tests (0 skipped))
    **** Failed ***************
  • test_suite_ecp
    PASSED (87 / 87 tests (0 skipped))
  • test_suite_error
    PASSED (6 / 6 tests (0 skipped))
  • test_suite_gcm.decrypt_128
    PASSED (169 / 169 tests (0 skipped))
  • test_suite_gcm.decrypt_192
    PASSED (169 / 169 tests (0 skipped))
  • test_suite_gcm.decrypt_256
    PASSED (169 / 169 tests (0 skipped))
  • test_suite_gcm.encrypt_128
    PASSED (169 / 169 tests (0 skipped))
  • test_suite_gcm.encrypt_192
    PASSED (169 / 169 tests (0 skipped))
  • test_suite_gcm.encrypt_256
    PASSED (169 / 169 tests (0 skipped))
  • test_suite_hmac_shax
    PASSED (36 / 36 tests (0 skipped))
  • test_suite_md
    PASSED (258 / 258 tests (48 skipped))
  • test_suite_mdx
    PASSED (55 / 55 tests (32 skipped))
  • test_suite_mpi
    PASSED (197 / 197 tests (0 skipped))
  • test_suite_pbkdf2
    PASSED (5 / 5 tests (0 skipped))
  • test_suite_pkcs1_v21
    PASSED (247 / 247 tests (0 skipped))
  • test_suite_pkcs5
    PASSED (5 / 5 tests (0 skipped))
  • test_suite_pkparse
    PASSED (57 / 57 tests (0 skipped))
  • test_suite_pkwrite
    PASSED (4 / 4 tests (0 skipped))
  • test_suite_rsa
    PASSED (95 / 95 tests (4 skipped))
  • test_suite_shax
    PASSED (63 / 63 tests (0 skipped))
  • test_suite_x509parse
    PASSED (197 / 197 tests (2 skipped))
  • test_suite_x509write
    PASSED (8 / 8 tests (1 skipped))
  • test_suite_xtea
    PASSED (13 / 13 tests (0 skipped))
  • test_suite_version
    PASSED (2 / 2 tests (0 skipped))

make[1]: *** [check] Error 1
make: *** [check] Error 2

@mpg
Copy link
Contributor

mpg commented Oct 18, 2013

After a quick glance, I think the test suite is to blame; more specifically, the not_rnd() helper function. So if my analysis is correct, ecdh and ecdsa are probably working properly on big-endian, despite the failing tests. (Experimental support for this hypothesis: the underlying ecp functions pass the test suite, and only the test using not_rnd() fail.) I'll prepare a patch for this shortly.

@mpg
Copy link
Contributor

mpg commented Oct 18, 2013

The following patch should work. As I don't have easy access to a big-endian
machine right now, I'd really appreciate if you could test it!

diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function
index 881a0ac..b334954 100644
--- a/tests/suites/helpers.function
+++ b/tests/suites/helpers.function
@@ -2,6 +2,14 @@
 #include "polarssl/memory.h"
 #endif

+#if defined(WANT_NOT_RND_MPI)
+#if defined(POLARSSL_BIGNUM_C)
+#include "polarssl/bignum.h"
+#else
+#error "not_rnd_mpi() need bignum.c"
+#endif
+#endif
+
 #ifdef _MSC_VER
 #include <basetsd.h>
 typedef UINT32 uint32_t;
@@ -225,48 +233,36 @@ static int rnd_pseudo_rand( void *rng_state, unsigned char *output, size_t len )
     return( 0 );
 }

+#if defined(WANT_NOT_RND_MPI)
 /**
- * This function returns a buffer given as a hex string.
+ * NOT random function, to match test vectors.
  *
- * The buffer is reversed so that the following are equivalent:
- *   mpi_fill_random( x, len, not_rnd, str );
+ * The following are equivalent:
+ *   mpi_fill_random( x, strlen( str ) / 2, not_rnd, str );
  *   mpi_read_string( x, 16, str );
- * (So, not random at all. Usefull to match test vectors.)
- * Based on unhexify(), just reversed (changes marked by "sic")
+ * Warning: no other use is supported!
  */
-static int not_rnd( void *in, unsigned char *out, size_t len )
+#define ciL    (sizeof(t_uint))         /* chars in limb  */
+#define CHARS_TO_LIMBS(i) (((i) + ciL - 1) / ciL)
+static int not_rnd_mpi( void *in, unsigned char *out, size_t len )
 {
-    unsigned char *obuf;
-    const char *ibuf = in;
-    unsigned char c, c2;
-    assert( len == strlen(ibuf) / 2 );
-    assert(!(strlen(ibuf) %1)); // must be even number of bytes
-
-    obuf = out + (len - 1); // sic
-    while (*ibuf != 0)
-    {
-        c = *ibuf++;
-        if( c >= '0' && c <= '9' )
-            c -= '0';
-        else if( c >= 'a' && c <= 'f' )
-            c -= 'a' - 10;
-        else if( c >= 'A' && c <= 'F' )
-            c -= 'A' - 10;
-        else
-            assert( 0 );
-
-        c2 = *ibuf++;
-        if( c2 >= '0' && c2 <= '9' )
-            c2 -= '0';
-        else if( c2 >= 'a' && c2 <= 'f' )
-            c2 -= 'a' - 10;
-        else if( c2 >= 'A' && c2 <= 'F' )
-            c2 -= 'A' - 10;
-        else
-            assert( 0 );
-
-        *obuf-- = ( c << 4 ) | c2; // sic
-    }
-
-    return( 0 );
+    char *str = (char *) in;
+    mpi X;
+
+    /*
+     * The 'in' pointer we get is from an MPI prepared by mpi_fill_random(),
+     * just reconstruct the rest in order to be able to call mpi_read_string()
+     */
+    X.s = 1;
+    X.p = (t_uint *) out;
+    X.n = CHARS_TO_LIMBS( len );
+
+    /*
+     * If str is too long, mpi_read_string() will try to allocate a new buffer
+     * for X.p, which we want to avoid at all costs.
+     */
+    assert( strlen( str ) / 2 == len );
+
+    return( mpi_read_string( &X, 16, str ) );
 }
+#endif /* WANT_NOT_RND_MPI */
diff --git a/tests/suites/test_suite_ecdh.function b/tests/suites/test_suite_ecdh.function
index ba35c76..63917d7 100644
--- a/tests/suites/test_suite_ecdh.function
+++ b/tests/suites/test_suite_ecdh.function
@@ -1,5 +1,6 @@
 /* BEGIN_HEADER */
 #include <polarssl/ecdh.h>
+#define WANT_NOT_RND_MPI
 /* END_HEADER */

 /* BEGIN_DEPENDENCIES
@@ -57,14 +58,14 @@ void ecdh_primitive_testvec( int id, char *dA_str, char *xA_str, char *yA_str,

     TEST_ASSERT( ecp_use_known_dp( &grp, id ) == 0 );

-    TEST_ASSERT( ecdh_gen_public( &grp, &dA, &qA, &not_rnd, dA_str ) == 0 );
+    TEST_ASSERT( ecdh_gen_public( &grp, &dA, &qA, &not_rnd_mpi, dA_str ) == 0 );
     TEST_ASSERT( ! ecp_is_zero( &qA ) );
     TEST_ASSERT( mpi_read_string( &check, 16, xA_str ) == 0 );
     TEST_ASSERT( mpi_cmp_mpi( &qA.X, &check ) == 0 );
     TEST_ASSERT( mpi_read_string( &check, 16, yA_str ) == 0 );
     TEST_ASSERT( mpi_cmp_mpi( &qA.Y, &check ) == 0 );

-    TEST_ASSERT( ecdh_gen_public( &grp, &dB, &qB, &not_rnd, dB_str ) == 0 );
+    TEST_ASSERT( ecdh_gen_public( &grp, &dB, &qB, &not_rnd_mpi, dB_str ) == 0 );
     TEST_ASSERT( ! ecp_is_zero( &qB ) );
     TEST_ASSERT( mpi_read_string( &check, 16, xB_str ) == 0 );
     TEST_ASSERT( mpi_cmp_mpi( &qB.X, &check ) == 0 );
diff --git a/tests/suites/test_suite_ecdsa.function b/tests/suites/test_suite_ecdsa.function
index 5ccb39d..34307ca 100644
--- a/tests/suites/test_suite_ecdsa.function
+++ b/tests/suites/test_suite_ecdsa.function
@@ -1,5 +1,6 @@
 /* BEGIN_HEADER */
 #include <polarssl/ecdsa.h>
+#define WANT_NOT_RND_MPI
 /* END_HEADER */

 /* BEGIN_DEPENDENCIES
@@ -63,7 +64,7 @@ void ecdsa_prim_test_vectors( int id, char *d_str, char *xQ_str, char *yQ_str,
     len = unhexify(buf, hash_str);

     TEST_ASSERT( ecdsa_sign( &grp, &r, &s, &d, buf, len,
-                &not_rnd, k_str ) == 0 );
+                &not_rnd_mpi, k_str ) == 0 );

     TEST_ASSERT( mpi_cmp_mpi( &r, &r_check ) == 0 );
     TEST_ASSERT( mpi_cmp_mpi( &s, &s_check ) == 0 );

(I hope github markup doesn't make the patch unusable.)

EDIT: just in case: http://pastebin.com/raw.php?i=UaEf2XXz

@tcoppi
Copy link
Author

tcoppi commented Oct 18, 2013

That fixed it! Thanks for responding so quickly :)

@pjbakker
Copy link
Contributor

Integrated into development branch and included for 1.3.2 release

gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Sep 5, 2017
hanno-becker pushed a commit to hanno-becker/mbedtls that referenced this issue Oct 12, 2020
TLS 1.3 code added to ssl_client2.c and ssl_server2.c
SebastianBoe pushed a commit to SebastianBoe/mbedtls-1 that referenced this issue Jan 26, 2022
Disable dependency check for `MBEDTLS_USE_PSA_CRYPTO` on `MBEDTLS_PSA_CRYPTO_C`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants