From b00651828943a505315e5ca1de0cdd6c96e449ce Mon Sep 17 00:00:00 2001
From: Ron Eldor <Ron.Eldor@arm.com>
Date: Mon, 16 Oct 2017 12:40:27 +0300
Subject: [PATCH] Resolve PR review comments

1) use `pk_get_rsapubkey` instead of reimplementing the parsing
2) rename the key files, according to their type and key size
3) comment in the data_files/Makefile hoe the keys were generated
4) Fix issue of failure parsing pkcs#1 DER format parsing, missed in previous commit
---
 library/pkparse.c                             |  62 ++++++++----------
 tests/data_files/Makefile                     |   6 ++
 tests/data_files/public_rsa_key.der           | Bin 294 -> 0 bytes
 tests/data_files/public_rsa_key.pem           |   8 ---
 tests/data_files/rsa_pkcs1_2048_public.der    | Bin 0 -> 270 bytes
 tests/data_files/rsa_pkcs1_2048_public.pem    |   8 +++
 ..._gen_der.pub => rsa_pkcs8_1024_public.der} | Bin
 tests/suites/test_suite_pkparse.data          |   6 +-
 8 files changed, 43 insertions(+), 47 deletions(-)
 delete mode 100644 tests/data_files/public_rsa_key.der
 delete mode 100644 tests/data_files/public_rsa_key.pem
 create mode 100644 tests/data_files/rsa_pkcs1_2048_public.der
 create mode 100644 tests/data_files/rsa_pkcs1_2048_public.pem
 rename tests/data_files/{format_gen_der.pub => rsa_pkcs8_1024_public.der} (100%)

diff --git a/library/pkparse.c b/library/pkparse.c
index 1d573a4005..9c84e36e82 100644
--- a/library/pkparse.c
+++ b/library/pkparse.c
@@ -649,14 +649,6 @@ static int pk_parse_key_pkcs1_der( mbedtls_rsa_context *rsa,
 
     p = (unsigned char *) key;
     end = p + keylen;
-    if( ( ret = mbedtls_asn1_get_tag( &p, end, &len,
-            MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 )
-    {
-        return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + ret );
-    }
-
-    end = p + len;
-
     if( mode == 0 )
     {
     /*
@@ -675,6 +667,14 @@ static int pk_parse_key_pkcs1_der( mbedtls_rsa_context *rsa,
      *      otherPrimeInfos   OtherPrimeInfos OPTIONAL
      *  }
      */
+        if( ( ret = mbedtls_asn1_get_tag( &p, end, &len,
+                MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 )
+        {
+            return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + ret );
+        }
+
+        end = p + len;
+
         if( ( ret = mbedtls_asn1_get_int( &p, end, &rsa->ver ) ) != 0 )
         {
             return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + ret );
@@ -715,36 +715,11 @@ static int pk_parse_key_pkcs1_der( mbedtls_rsa_context *rsa,
     }
     else /* public key*/
     {
-    /*
-     * This function parses the RSAPublicKey (PKCS#1)
-     *
-     *  RSAPublicKey ::= SEQUENCE {
-     *                   modulus           INTEGER,  -- n
-     *                   publicExponent    INTEGER   -- e
-     *                   }
-     */
-        if( ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->N  ) ) != 0 ||
-            ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->E  ) ) != 0 )
+        if( ( ret = pk_get_rsapubkey( &p, end, rsa ) ) != 0 )
         {
             mbedtls_rsa_free( rsa );
-            return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + ret );
+            return( ret );
         }
-
-        rsa->len = mbedtls_mpi_size( &rsa->N );
-
-        if( p != end )
-        {
-             mbedtls_rsa_free( rsa );
-             return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT +
-                     MBEDTLS_ERR_ASN1_LENGTH_MISMATCH );
-        }
-
-        if( ( ret = mbedtls_rsa_check_pubkey( rsa ) ) != 0 )
-        {
-             mbedtls_rsa_free( rsa );
-             return( ret );
-        }
-
     }
     return( 0 );
 }
@@ -1287,6 +1262,7 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx,
 #if defined(MBEDTLS_PEM_PARSE_C)
     size_t len;
     mbedtls_pem_context pem;
+    const mbedtls_pk_info_t *pk_info;
 
     mbedtls_pem_init( &pem );
 #if defined(MBEDTLS_RSA_C)
@@ -1301,7 +1277,6 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx,
 
     if( ret == 0 )
     {
-         const mbedtls_pk_info_t *pk_info;
          if( ( pk_info = mbedtls_pk_info_from_type( MBEDTLS_PK_RSA ) ) == NULL )
               return( MBEDTLS_ERR_PK_UNKNOWN_PK_ALG );
 
@@ -1319,6 +1294,21 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx,
         mbedtls_pem_free( &pem );
         return( ret );
     }
+
+    if( ( pk_info = mbedtls_pk_info_from_type( MBEDTLS_PK_RSA ) ) == NULL )
+          return( MBEDTLS_ERR_PK_UNKNOWN_PK_ALG );
+
+    if( ( ret = mbedtls_pk_setup( ctx, pk_info ) ) != 0 )
+        return( ret );
+
+    ret = pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *ctx ),
+                                  key, keylen, 1 );
+    if ( ret == 0 )
+    {
+        mbedtls_pem_free( &pem );
+        return( ret );
+    }
+    mbedtls_pk_free( ctx );
 #endif /* MBEDTLS_RSA_C */
 
        /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */
diff --git a/tests/data_files/Makefile b/tests/data_files/Makefile
index f7826d4359..bfcdc684a1 100644
--- a/tests/data_files/Makefile
+++ b/tests/data_files/Makefile
@@ -64,7 +64,13 @@ server2-sha256.crt: server2-rsa.csr
 	$(OPENSSL) x509 -req -extfile $(cli_crt_extensions_file) -extensions cli-rsa -CA test-ca-sha256.crt -CAkey $(test_ca_key_file_rsa) -passin "pass:$(test_ca_pwd_rsa)" -set_serial 4 -days 3653 -sha256 -in server2-rsa.csr -out $@
 all_final += server2-sha256.crt
 
+rsa_pkcs1_2048_public.pem: server8.key
+	$(OPENSSL)  rsa -in server8.key -outform PEM -RSAPublicKey_out -out $@
+all_final += rsa_pkcs8_2048_public.pem
 
+rsa_pkcs1_2048_public.der: rsa_pkcs1_2048_public.pem
+	$(OPENSSL) -RSAPublicKey_in -in rsa_pkcs1_2048_public.pem -outform DER -RSAPublicKey_out -out $@
+all_final += rsa_pkcs8_2048_public.der
 
 ################################################################
 #### Meta targets
diff --git a/tests/data_files/public_rsa_key.der b/tests/data_files/public_rsa_key.der
deleted file mode 100644
index 376b79a442857eea9410120b18090d443073f906..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001

literal 294
zcmV+>0ondAf&n5h4F(A+hDe6@4FLfG1potr0S^E$f&mHwf&l>l*+EQ+)gnMp6SCRo
zNgj~*v=fEN&(1&-bb~o94M#At+s<U`Ef%$!Io>pgeT+|;r0~RDR}=#Z?|>tl{KOuq
z>sU7xJQQ^`1*|*ikdk}dMm(;uNzxXg`zx^Uik|N(Ko$l~U(Q2@Ix6)`<lEdX#2>_H
z4r0-653FbEfi~qV5^|-3=PnS;9nqV+w%-szaJv4@z8_XFP8lt4Aya3@KH3d}QwP>g
z+Ap~=M<LJ?$A-suf&c%zkpjqzXLI0r(W+Z>{_xH$7t{buyk$sPyTmYVyjBY>+C}}=
sh~f<PNqRrr+Y-@&oZ5lzk{Ch|h#=5Zi=9Zx^Ja-{$i*f%0s{d60SMoMoB#j-

diff --git a/tests/data_files/public_rsa_key.pem b/tests/data_files/public_rsa_key.pem
deleted file mode 100644
index 3add85c6c3..0000000000
--- a/tests/data_files/public_rsa_key.pem
+++ /dev/null
@@ -1,8 +0,0 @@
------BEGIN RSA PUBLIC KEY-----
-MIIBCgKCAQEA2UFMidUiQFATstnnSR6Q97QThcnPzkATdIM5LQ1HMLLbzmTrLRa1
-mjneNIh9jE+ZpPDEXVcUAwrvgCOb/MQeqetYNxU8FHU1Baw76ZCSe91GPK6xSdIW
-ovsrsPCKnu8qQBYGTV/OQ4Y6KvVL5NvcLsQfxGgOYtFuD6xn6oE25SwScqWD5y4Q
-zB3Rm7u23xBBcLr+zb4fVjBOGS1vIVNnxj7aDYJTB9ZO2i+5MUch0BPHhsd3gf//
-u5ECyItnc+B50apbc/7wziwX1ABMvGVIWbvEMG68Vgst2kX91ojiDPZJej/c2xLR
-gpzage6SGEIQiCDQVIudSMnzZoltyMUmNwIDAQAB
------END RSA PUBLIC KEY-----
diff --git a/tests/data_files/rsa_pkcs1_2048_public.der b/tests/data_files/rsa_pkcs1_2048_public.der
new file mode 100644
index 0000000000000000000000000000000000000000..b6865144ab245817b04e3507d4aaec3dcff605fb
GIT binary patch
literal 270
zcmV+p0rCDYf&mHwf&l>l+Z=x`3(ddI(RC1@qPWg|s^SIUde}r`kF~wPuo<~GxEV?g
z@m+L)XGVtx-dleL1HHkGUI!J_TlC!J&pr9U5iG81xr)6VXJ!}bPQBX|nu3Sq@OZ<c
zg@>^HpQ)K&<1_5c>I=1DUhzqOKcg+`0SwGSns%GS&^Obu7Xe`b7Fm8A7sFHi(Q?a7
zU=`YZ;_9tX?~dY&)M|HC)^OQtyYcQh1URF;;?dw{YvP<ondvCzw)`&2XG4VO)q~cZ
zm}>z(f!sWL%K7u0_tq#ICwP3r(A8t7fi#J&C2GC$>h1bh{N*&p!4GjQ(g+Y6twcfK
U{&}EdlZvrj>9Fo^0s{d60Wn65O8@`>

literal 0
HcmV?d00001

diff --git a/tests/data_files/rsa_pkcs1_2048_public.pem b/tests/data_files/rsa_pkcs1_2048_public.pem
new file mode 100644
index 0000000000..9040cb04d4
--- /dev/null
+++ b/tests/data_files/rsa_pkcs1_2048_public.pem
@@ -0,0 +1,8 @@
+-----BEGIN RSA PUBLIC KEY-----
+MIIBCgKCAQEA2xx/LgvNv87RdRCgorjOfariBeB62ERjj7W9wLAZuTe4GUoO8V10
+gGdGhwbeW38GA73BjV4HFdRb9Nzlzz35wREsrmq5ir0dZ2YX6k692xWagofk8HjD
+o4WHsP2fqZlf4zPszOoLtWFe8Ul+P6Mt6gEMzEKadpvE0DfTsRcBYQEWWX4cF8NT
+/dFyy0xgFdp94uqtUO+O4ovUandV1nDZa7vx7jkEOKO94tHgZmvinEeZ6Sjmtvwu
+ymdDhOjVg9admGsBPoHcPHrK+fOc99YoGyd4fMPQ1WOngTSJrSVqvfLq7fpX/OU0
+xsEPcS3SCBAbrURB4P55oGOTirFd6bDubwIDAQAB
+-----END RSA PUBLIC KEY-----
diff --git a/tests/data_files/format_gen_der.pub b/tests/data_files/rsa_pkcs8_1024_public.der
similarity index 100%
rename from tests/data_files/format_gen_der.pub
rename to tests/data_files/rsa_pkcs8_1024_public.der
diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data
index f34fc3c83b..b0464e5d29 100644
--- a/tests/suites/test_suite_pkparse.data
+++ b/tests/suites/test_suite_pkparse.data
@@ -108,15 +108,15 @@ pk_parse_public_keyfile_rsa:"data_files/format_gen.pub":0
 
 Parse Public RSA Key #1 (PKCS#8 wrapped, DER)
 depends_on:MBEDTLS_MD5_C:MBEDTLS_PEM_PARSE_C
-pk_parse_public_keyfile_rsa:"data_files/format_gen_der.pub":0
+pk_parse_public_keyfile_rsa:"data_files/rsa_pkcs8_1024_public.der":0
 
 Parse Public RSA Key #3 (PKCS#1 wrapped)
 depends_on:MBEDTLS_RSA_C:MBEDTLS_PEM_PARSE_C
-pk_parse_public_keyfile_rsa:"data_files/public_rsa_key.pem":0
+pk_parse_public_keyfile_rsa:"data_files/rsa_pkcs1_2048_public.pem":0
 
 Parse Public RSA Key #4 (PKCS#1 wrapped, DER)
 depends_on:MBEDTLS_RSA_C:MBEDTLS_PEM_PARSE_C
-pk_parse_public_keyfile_rsa:"data_files/public_rsa_key.der":0
+pk_parse_public_keyfile_rsa:"data_files/rsa_pkcs1_2048_public.der":0
 
 Parse Public EC Key #1 (RFC 5480, DER)
 depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED