From ef441784749d14ea9f8afad3f258489d2445c438 Mon Sep 17 00:00:00 2001
From: Janos Follath <janos.follath@arm.com>
Date: Wed, 21 Sep 2016 13:18:12 +0100
Subject: [PATCH] Restore P>Q in RSA key generation (#558)

The PKCS#1 standard says nothing about the relation between P and Q
but many libraries guarantee P>Q and mbed TLS did so too in earlier
versions.

This commit restores this behaviour.
---
 ChangeLog                            |  1 +
 library/rsa.c                        | 18 ++++++++----------
 tests/suites/test_suite_rsa.data     |  2 +-
 tests/suites/test_suite_rsa.function |  1 +
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 2e89c6afe..ab626b96a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -40,6 +40,7 @@ Bugfix
      by inestlerode. #559.
    * Fix documentation and implementation missmatch for function arguments of
      mbedtls_gcm_finish(). Found by cmiatpaar. #602
+   * Guarantee that P>Q at RSA key generation. #558
 
 Changes
    * Extended test coverage of special cases, and added new timing test suite.
diff --git a/library/rsa.c b/library/rsa.c
index 7a33689b2..40ef2a948 100644
--- a/library/rsa.c
+++ b/library/rsa.c
@@ -102,7 +102,10 @@ int mbedtls_rsa_gen_key( mbedtls_rsa_context *ctx,
     if( f_rng == NULL || nbits < 128 || exponent < 3 )
         return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA );
 
-    mbedtls_mpi_init( &P1 ); mbedtls_mpi_init( &Q1 ); 
+    if( nbits % 2 )
+        return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA );
+
+    mbedtls_mpi_init( &P1 ); mbedtls_mpi_init( &Q1 );
     mbedtls_mpi_init( &H ); mbedtls_mpi_init( &G );
 
     /*
@@ -116,16 +119,8 @@ int mbedtls_rsa_gen_key( mbedtls_rsa_context *ctx,
         MBEDTLS_MPI_CHK( mbedtls_mpi_gen_prime( &ctx->P, nbits >> 1, 0,
                                 f_rng, p_rng ) );
 
-        if( nbits % 2 )
-        {
-            MBEDTLS_MPI_CHK( mbedtls_mpi_gen_prime( &ctx->Q, ( nbits >> 1 ) + 1, 0,
+        MBEDTLS_MPI_CHK( mbedtls_mpi_gen_prime( &ctx->Q, nbits >> 1, 0,
                                 f_rng, p_rng ) );
-        }
-        else
-        {
-            MBEDTLS_MPI_CHK( mbedtls_mpi_gen_prime( &ctx->Q, nbits >> 1, 0,
-                                f_rng, p_rng ) );
-        }
 
         if( mbedtls_mpi_cmp_mpi( &ctx->P, &ctx->Q ) == 0 )
             continue;
@@ -134,6 +129,9 @@ int mbedtls_rsa_gen_key( mbedtls_rsa_context *ctx,
         if( mbedtls_mpi_bitlen( &ctx->N ) != nbits )
             continue;
 
+        if( mbedtls_mpi_cmp_mpi( &ctx->P, &ctx->Q ) < 0 )
+                                mbedtls_mpi_swap( &ctx->P, &ctx->Q );
+
         MBEDTLS_MPI_CHK( mbedtls_mpi_sub_int( &P1, &ctx->P, 1 ) );
         MBEDTLS_MPI_CHK( mbedtls_mpi_sub_int( &Q1, &ctx->Q, 1 ) );
         MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &H, &P1, &Q1 ) );
diff --git a/tests/suites/test_suite_rsa.data b/tests/suites/test_suite_rsa.data
index d522332a2..af168805f 100644
--- a/tests/suites/test_suite_rsa.data
+++ b/tests/suites/test_suite_rsa.data
@@ -361,7 +361,7 @@ RSA Generate Key - 2048 bit key
 mbedtls_rsa_gen_key:2048:3:0
 
 RSA Generate Key - 1025 bit key
-mbedtls_rsa_gen_key:1025:3:0
+mbedtls_rsa_gen_key:1025:3:MBEDTLS_ERR_RSA_BAD_INPUT_DATA
 
 RSA PKCS1 Encrypt Bad RNG
 depends_on:MBEDTLS_PKCS1_V15
diff --git a/tests/suites/test_suite_rsa.function b/tests/suites/test_suite_rsa.function
index 8837e3a83..d48bc8595 100644
--- a/tests/suites/test_suite_rsa.function
+++ b/tests/suites/test_suite_rsa.function
@@ -678,6 +678,7 @@ void mbedtls_rsa_gen_key( int nrbits, int exponent, int result)
     if( result == 0 )
     {
         TEST_ASSERT( mbedtls_rsa_check_privkey( &ctx ) == 0 );
+        TEST_ASSERT( mbedtls_mpi_cmp_mpi( &ctx.P, &ctx.Q ) > 0 );
     }
 
 exit: