From d0c56de93415a44671716dc927e1033d038ab913 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Tue, 10 Oct 2017 17:03:08 +0300 Subject: [PATCH 1/9] Add support for public keys encoded with PKCS#1 1) Add support for public keys encoded with PKCS#1 2) Add tests for PKCS#1 PEM and DER, and PKCS#8 DER --- ChangeLog | 1 + library/pkparse.c | 163 +++++++++++++++++++-------- tests/data_files/format_gen_der.pub | Bin 0 -> 162 bytes tests/data_files/public_rsa_key.der | Bin 0 -> 294 bytes tests/data_files/public_rsa_key.pem | 8 ++ tests/suites/test_suite_pkparse.data | 12 ++ 6 files changed, 135 insertions(+), 49 deletions(-) create mode 100644 tests/data_files/format_gen_der.pub create mode 100644 tests/data_files/public_rsa_key.der create mode 100644 tests/data_files/public_rsa_key.pem diff --git a/ChangeLog b/ChangeLog index b3d4d519a..f13982b62 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,7 @@ mbed TLS ChangeLog (Sorted per branch, date) Features * Allow comments in test data files. + * Add support for public keys encoded in PKCS#1 format Bugfix * Fix memory leak in mbedtls_ssl_set_hostname() when called multiple times. diff --git a/library/pkparse.c b/library/pkparse.c index efdf43746..1d573a400 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -637,11 +637,11 @@ int mbedtls_pk_parse_subpubkey( unsigned char **p, const unsigned char *end, #if defined(MBEDTLS_RSA_C) /* - * Parse a PKCS#1 encoded private RSA key + * Parse a PKCS#1 encoded private( mode 0 )/public( mode 1 ) RSA key */ static int pk_parse_key_pkcs1_der( mbedtls_rsa_context *rsa, const unsigned char *key, - size_t keylen ) + size_t keylen , int mode) { int ret; size_t len; @@ -649,7 +649,16 @@ 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 ) + { /* * This function parses the RSAPrivateKey (PKCS#1) * @@ -666,52 +675,77 @@ 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 ); + if( ( ret = mbedtls_asn1_get_int( &p, end, &rsa->ver ) ) != 0 ) + { + return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + ret ); + } + + if( rsa->ver != 0 ) + { + return( MBEDTLS_ERR_PK_KEY_INVALID_VERSION ); + } + + if( ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->N ) ) != 0 || + ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->E ) ) != 0 || + ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->D ) ) != 0 || + ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->P ) ) != 0 || + ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->Q ) ) != 0 || + ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->DP ) ) != 0 || + ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->DQ ) ) != 0 || + ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->QP ) ) != 0 ) + { + mbedtls_rsa_free( rsa ); + return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + 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_privkey( rsa ) ) != 0 ) + { + mbedtls_rsa_free( rsa ); + return( ret ); + } } - - end = p + len; - - if( ( ret = mbedtls_asn1_get_int( &p, end, &rsa->ver ) ) != 0 ) + else /* public key*/ { - return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + ret ); + /* + * 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 ) + { + mbedtls_rsa_free( rsa ); + return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + 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 ); + } + } - - if( rsa->ver != 0 ) - { - return( MBEDTLS_ERR_PK_KEY_INVALID_VERSION ); - } - - if( ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->N ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->E ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->D ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->P ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->Q ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->DP ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->DQ ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->QP ) ) != 0 ) - { - mbedtls_rsa_free( rsa ); - return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + 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_privkey( rsa ) ) != 0 ) - { - mbedtls_rsa_free( rsa ); - return( ret ); - } - return( 0 ); } #endif /* MBEDTLS_RSA_C */ @@ -907,7 +941,7 @@ static int pk_parse_key_pkcs8_unencrypted_der( #if defined(MBEDTLS_RSA_C) if( pk_alg == MBEDTLS_PK_RSA ) { - if( ( ret = pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *pk ), p, len ) ) != 0 ) + if( ( ret = pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *pk ), p, len, 0 ) ) != 0 ) { mbedtls_pk_free( pk ); return( ret ); @@ -1086,7 +1120,7 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, if( ( ret = mbedtls_pk_setup( pk, pk_info ) ) != 0 || ( ret = pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *pk ), - pem.buf, pem.buflen ) ) != 0 ) + pem.buf, pem.buflen, 0 ) ) != 0 ) { mbedtls_pk_free( pk ); } @@ -1218,7 +1252,7 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, return( MBEDTLS_ERR_PK_UNKNOWN_PK_ALG ); if( ( ret = mbedtls_pk_setup( pk, pk_info ) ) != 0 || - ( ret = pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *pk ), key, keylen ) ) == 0 ) + ( ret = pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *pk ), key, keylen, 0 ) ) == 0 ) { return( 0 ); } @@ -1255,8 +1289,39 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, mbedtls_pem_context pem; mbedtls_pem_init( &pem ); - +#if defined(MBEDTLS_RSA_C) /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */ + if( keylen == 0 || key[keylen - 1] != '\0' ) + ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT; + else + ret = mbedtls_pem_read_buffer( &pem, + "-----BEGIN RSA PUBLIC KEY-----", + "-----END RSA PUBLIC KEY-----", + key, NULL, 0, &len ); + + 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 ); + + if( ( ret = mbedtls_pk_setup( ctx, pk_info ) ) != 0 ) + return( ret ); + + if( ( ret = pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *ctx ), + pem.buf, pem.buflen, 1 ) ) != 0 ) + mbedtls_pk_free( ctx ); + mbedtls_pem_free( &pem ); + return( ret ); + } + else if( ret != MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT ) + { + mbedtls_pem_free( &pem ); + return( ret ); + } +#endif /* MBEDTLS_RSA_C */ + + /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */ if( keylen == 0 || key[keylen - 1] != '\0' ) ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT; else diff --git a/tests/data_files/format_gen_der.pub b/tests/data_files/format_gen_der.pub new file mode 100644 index 0000000000000000000000000000000000000000..fe429985bf29b545b3d52a24b692807062a827b5 GIT binary patch literal 162 zcmV;T0A2qufuAr91_>&LNQUm7(YwGJ1K49Joe8Zo!w`XNif5NQbH1%0ondAf&n5h4F(A+hDe6@4FLfG1potr0S^E$f&mHwf&l>l*+EQ+)gnMp6SCRo zNgj~*v=fEN&(1&-bb~o94M#At+spgeT+|;r0~RDR}=#Z?|>tl{KOuq z>sU7xJQQ^`1*|*ikdk}dMm(;uNzxXg`zx^Uik|N(Ko$l~U(Q2@Ix6)`_H z4r0-653FbEfi~qV5^|-3=PnS;9nqV+w%-szaJv4@z8_XFP8lt4Aya3@KH3d}QwP>g z+Ap~=M{_xH$7t{buyk$sPyTmYVyjBY>+C}}= sh~f Date: Mon, 16 Oct 2017 12:40:27 +0300 Subject: [PATCH 2/9] 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 1d573a400..9c84e36e8 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 f7826d435..bfcdc684a 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+spgeT+|;r0~RDR}=#Z?|>tl{KOuq z>sU7xJQQ^`1*|*ikdk}dMm(;uNzxXg`zx^Uik|N(Ko$l~U(Q2@Ix6)`_H z4r0-653FbEfi~qV5^|-3=PnS;9nqV+w%-szaJv4@z8_XFP8lt4Aya3@KH3d}QwP>g z+Ap~=M{_xH$7t{buyk$sPyTmYVyjBY>+C}}= sh~fl+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^HpQ)K&<1_5c>I=1DUhzqOKcg+`0SwGSns%GS&^Obu7Xe`b7Fm8A7sFHi(Q?a7 zU=`YZ;_9tX?~dY&)M|HC)^OQtyYcQh1URF;;?dw{YvPz(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 000000000..9040cb04d --- /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 f34fc3c83..b0464e5d2 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 From 84df1aeeaf4c212e60ec72998068262e3789adbc Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Mon, 16 Oct 2017 17:11:52 +0300 Subject: [PATCH 3/9] use internal pk_get_rsapubkey function 1) use `pk_get_rsapubkey` function instead of `pk_parse_key_pkcs1_der` 2) revert changes in `pk_parse_key_pkcs1_der` --- library/pkparse.c | 128 +++++++++++++++++++++------------------------- 1 file changed, 59 insertions(+), 69 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 9c84e36e8..6e527530b 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -637,11 +637,11 @@ int mbedtls_pk_parse_subpubkey( unsigned char **p, const unsigned char *end, #if defined(MBEDTLS_RSA_C) /* - * Parse a PKCS#1 encoded private( mode 0 )/public( mode 1 ) RSA key + * Parse a PKCS#1 encoded private RSA key */ static int pk_parse_key_pkcs1_der( mbedtls_rsa_context *rsa, const unsigned char *key, - size_t keylen , int mode) + size_t keylen ) { int ret; size_t len; @@ -649,8 +649,7 @@ static int pk_parse_key_pkcs1_der( mbedtls_rsa_context *rsa, p = (unsigned char *) key; end = p + keylen; - if( mode == 0 ) - { + /* * This function parses the RSAPrivateKey (PKCS#1) * @@ -667,60 +666,52 @@ 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 ); - } - - if( rsa->ver != 0 ) - { - return( MBEDTLS_ERR_PK_KEY_INVALID_VERSION ); - } - - if( ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->N ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->E ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->D ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->P ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->Q ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->DP ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->DQ ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->QP ) ) != 0 ) - { - mbedtls_rsa_free( rsa ); - return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + 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_privkey( rsa ) ) != 0 ) - { - mbedtls_rsa_free( rsa ); - return( ret ); - } - } - else /* public key*/ + if( ( ret = mbedtls_asn1_get_tag( &p, end, &len, + MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 ) { - if( ( ret = pk_get_rsapubkey( &p, end, rsa ) ) != 0 ) - { - mbedtls_rsa_free( rsa ); - return( ret ); - } + 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 ); + } + + if( rsa->ver != 0 ) + { + return( MBEDTLS_ERR_PK_KEY_INVALID_VERSION ); + } + + if( ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->N ) ) != 0 || + ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->E ) ) != 0 || + ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->D ) ) != 0 || + ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->P ) ) != 0 || + ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->Q ) ) != 0 || + ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->DP ) ) != 0 || + ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->DQ ) ) != 0 || + ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->QP ) ) != 0 ) + { + mbedtls_rsa_free( rsa ); + return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + 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_privkey( rsa ) ) != 0 ) + { + mbedtls_rsa_free( rsa ); + return( ret ); + } + return( 0 ); } #endif /* MBEDTLS_RSA_C */ @@ -916,7 +907,7 @@ static int pk_parse_key_pkcs8_unencrypted_der( #if defined(MBEDTLS_RSA_C) if( pk_alg == MBEDTLS_PK_RSA ) { - if( ( ret = pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *pk ), p, len, 0 ) ) != 0 ) + if( ( ret = pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *pk ), p, len ) ) != 0 ) { mbedtls_pk_free( pk ); return( ret ); @@ -1095,7 +1086,7 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, if( ( ret = mbedtls_pk_setup( pk, pk_info ) ) != 0 || ( ret = pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *pk ), - pem.buf, pem.buflen, 0 ) ) != 0 ) + pem.buf, pem.buflen ) ) != 0 ) { mbedtls_pk_free( pk ); } @@ -1227,7 +1218,7 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, return( MBEDTLS_ERR_PK_UNKNOWN_PK_ALG ); if( ( ret = mbedtls_pk_setup( pk, pk_info ) ) != 0 || - ( ret = pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *pk ), key, keylen, 0 ) ) == 0 ) + ( ret = pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *pk ), key, keylen ) ) == 0 ) { return( 0 ); } @@ -1263,7 +1254,6 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, size_t len; mbedtls_pem_context pem; const mbedtls_pk_info_t *pk_info; - mbedtls_pem_init( &pem ); #if defined(MBEDTLS_RSA_C) /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */ @@ -1277,15 +1267,15 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, if( ret == 0 ) { - if( ( pk_info = mbedtls_pk_info_from_type( MBEDTLS_PK_RSA ) ) == NULL ) - return( MBEDTLS_ERR_PK_UNKNOWN_PK_ALG ); + p = pem.buf; + 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 ); + if( ( ret = mbedtls_pk_setup( ctx, pk_info ) ) != 0 ) + return( ret ); - if( ( ret = pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *ctx ), - pem.buf, pem.buflen, 1 ) ) != 0 ) - mbedtls_pk_free( ctx ); + if ( ( ret = pk_get_rsapubkey( &p, p + pem.buflen, mbedtls_pk_rsa( *ctx ) ) ) != 0 ) + mbedtls_pk_free( ctx ); mbedtls_pem_free( &pem ); return( ret ); } @@ -1301,8 +1291,8 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, if( ( ret = mbedtls_pk_setup( ctx, pk_info ) ) != 0 ) return( ret ); - ret = pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *ctx ), - key, keylen, 1 ); + p = (unsigned char *) key; + ret = pk_get_rsapubkey( &p, p + keylen, mbedtls_pk_rsa( *ctx ) ); if ( ret == 0 ) { mbedtls_pem_free( &pem ); From 40b14a894bafeff2df30d669a810ec90b86cd37f Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Mon, 16 Oct 2017 19:30:00 +0300 Subject: [PATCH 4/9] change order of parsing public key First parse PEM, and if fails, parse DER. Use some convention as in parsing the private key (`mbedtls_pk_parse_key`) --- library/pkparse.c | 47 ++++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 6e527530b..7c9983f5c 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -1284,21 +1284,6 @@ 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 ); - - p = (unsigned char *) key; - ret = pk_get_rsapubkey( &p, p + keylen, mbedtls_pk_rsa( *ctx ) ); - 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 */ @@ -1315,8 +1300,11 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, /* * Was PEM encoded */ - key = pem.buf; - keylen = pem.buflen; + p = pem.buf; + + ret = mbedtls_pk_parse_subpubkey( &p, p + pem.buflen, ctx ); + mbedtls_pem_free( &pem ); + return( ret ); } else if( ret != MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT ) { @@ -1324,14 +1312,31 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, return( ret ); } #endif /* MBEDTLS_PEM_PARSE_C */ + +#if defined(MBEDTLS_RSA_C) + 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 ); + + p = (unsigned char *) key; + ret = pk_get_rsapubkey( &p, p + keylen, mbedtls_pk_rsa( *ctx ) ); + if ( ret == 0 ) + { + mbedtls_pem_free( &pem ); + return( ret ); + } + mbedtls_pk_free( ctx ); + if ( ret != ( MBEDTLS_ERR_PK_INVALID_PUBKEY + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG ) ) + { + return ( ret ); + } +#endif /* MBEDTLS_RSA_C */ p = (unsigned char *) key; ret = mbedtls_pk_parse_subpubkey( &p, p + keylen, ctx ); -#if defined(MBEDTLS_PEM_PARSE_C) - mbedtls_pem_free( &pem ); -#endif - return( ret ); } From 5472d43ffbc05a142d18100770322095fb6138b4 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Tue, 17 Oct 2017 09:49:00 +0300 Subject: [PATCH 5/9] Fix issues when `MBEDTLS_PEM_PARSE_C` not defined 1) Fix compilatoin issues when `MBEDTLS_PEM_PARSE_C` not defined 2) remove dependency for `MBEDTLS_PEM_PARSE_C` in DER tests --- library/pkparse.c | 6 ++++-- tests/suites/test_suite_pkparse.data | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 7c9983f5c..75f1620b0 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -1250,10 +1250,12 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, { int ret; unsigned char *p; +#if defined(MBEDTLS_RSA_C) + const mbedtls_pk_info_t *pk_info; +#endif #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) /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */ @@ -1311,6 +1313,7 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, mbedtls_pem_free( &pem ); return( ret ); } + mbedtls_pem_free( &pem ); #endif /* MBEDTLS_PEM_PARSE_C */ #if defined(MBEDTLS_RSA_C) @@ -1324,7 +1327,6 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, ret = pk_get_rsapubkey( &p, p + keylen, mbedtls_pk_rsa( *ctx ) ); if ( ret == 0 ) { - mbedtls_pem_free( &pem ); return( ret ); } mbedtls_pk_free( ctx ); diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data index b0464e5d2..391d6c5b1 100644 --- a/tests/suites/test_suite_pkparse.data +++ b/tests/suites/test_suite_pkparse.data @@ -107,7 +107,7 @@ depends_on:MBEDTLS_MD5_C:MBEDTLS_PEM_PARSE_C 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 +depends_on:MBEDTLS_MD5_C pk_parse_public_keyfile_rsa:"data_files/rsa_pkcs8_1024_public.der":0 Parse Public RSA Key #3 (PKCS#1 wrapped) @@ -115,7 +115,7 @@ depends_on:MBEDTLS_RSA_C:MBEDTLS_PEM_PARSE_C 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 +depends_on:MBEDTLS_RSA_C pk_parse_public_keyfile_rsa:"data_files/rsa_pkcs1_2048_public.der":0 Parse Public EC Key #1 (RFC 5480, DER) From 3f2da84bca52ef2712ecd533005ae0af0e4759a2 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Tue, 17 Oct 2017 15:50:30 +0300 Subject: [PATCH 6/9] Resolve PR review comments 1) Fix style comments 2) Fix typo in Makefile 3) Remove the `MBEDTLS_MD5_C` dependency from test data file, as the used keys are not encrypted --- library/pkparse.c | 4 +++- tests/data_files/Makefile | 14 +++++++++++--- tests/data_files/rsa_pkcs8_2048_public.der | Bin 0 -> 294 bytes tests/data_files/rsa_pkcs8_2048_public.pem | 9 +++++++++ tests/suites/test_suite_pkparse.data | 7 +++---- 5 files changed, 26 insertions(+), 8 deletions(-) create mode 100644 tests/data_files/rsa_pkcs8_2048_public.der create mode 100644 tests/data_files/rsa_pkcs8_2048_public.pem diff --git a/library/pkparse.c b/library/pkparse.c index 75f1620b0..41eeadf45 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -1256,6 +1256,7 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, #if defined(MBEDTLS_PEM_PARSE_C) size_t len; mbedtls_pem_context pem; + mbedtls_pem_init( &pem ); #if defined(MBEDTLS_RSA_C) /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */ @@ -1278,6 +1279,7 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, if ( ( ret = pk_get_rsapubkey( &p, p + pem.buflen, mbedtls_pk_rsa( *ctx ) ) ) != 0 ) mbedtls_pk_free( ctx ); + mbedtls_pem_free( &pem ); return( ret ); } @@ -1288,7 +1290,7 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, } #endif /* MBEDTLS_RSA_C */ - /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */ + /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */ if( keylen == 0 || key[keylen - 1] != '\0' ) ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT; else diff --git a/tests/data_files/Makefile b/tests/data_files/Makefile index bfcdc684a..3e20f6641 100644 --- a/tests/data_files/Makefile +++ b/tests/data_files/Makefile @@ -65,11 +65,19 @@ server2-sha256.crt: server2-rsa.csr 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 + $(OPENSSL) rsa -in $< -outform PEM -RSAPublicKey_out -out $@ +all_final += rsa_pkcs1_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 $@ + $(OPENSSL) rsa -RSAPublicKey_in -in $< -outform DER -RSAPublicKey_out -out $@ +all_final += rsa_pkcs1_2048_public.der + +rsa_pkcs8_2048_public.pem: server8.key + $(OPENSSL) rsa -in $< -outform PEM -pubout -out $@ +all_final += rsa_pkcs8_2048_public.pem + +rsa_pkcs8_2048_public.der: rsa_pkcs8_2048_public.pem + $(OPENSSL) rsa -pubin -in $< -outform DER -pubout -out $@ all_final += rsa_pkcs8_2048_public.der ################################################################ diff --git a/tests/data_files/rsa_pkcs8_2048_public.der b/tests/data_files/rsa_pkcs8_2048_public.der new file mode 100644 index 0000000000000000000000000000000000000000..8644a5647e63e647adedd78acc7c631b3bd0a4cb GIT binary patch literal 294 zcmV+>0ondAf&n5h4F(A+hDe6@4FLfG1potr0S^E$f&mHwf&l>l+Z=x`3(ddI(RC1@ zqPWg|s^SIUde}r`kF~wPuo<~GxEV?g@m+L)XGVtx-dleL1HHkGUI!J_TlC!J&pr9U z5iG81xr)6VXJ!}bPQBX|nu3Sq@OZ^HpQ)K&<1_5c>I=1DUhzqOKcg+`0SwGS zns%GS&^Obu7Xe`b7Fm8A7sFHi(Q?a7U=`YZ;_9tX?~dY&)M|HC)^OQtyYcQh1URF; z;?dw{YvPz(f!sWL%K7u0_tq#ICwP3r(A8t7fi#J& sC2GC$>h1bh{N*&p!4GjQ(g+Y6twcfK{&}EdlZvrj>9Fo^0s{d60qM1nivR!s literal 0 HcmV?d00001 diff --git a/tests/data_files/rsa_pkcs8_2048_public.pem b/tests/data_files/rsa_pkcs8_2048_public.pem new file mode 100644 index 000000000..f1e29cc6e --- /dev/null +++ b/tests/data_files/rsa_pkcs8_2048_public.pem @@ -0,0 +1,9 @@ +-----BEGIN PUBLIC KEY----- +MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA2xx/LgvNv87RdRCgorjO +fariBeB62ERjj7W9wLAZuTe4GUoO8V10gGdGhwbeW38GA73BjV4HFdRb9Nzlzz35 +wREsrmq5ir0dZ2YX6k692xWagofk8HjDo4WHsP2fqZlf4zPszOoLtWFe8Ul+P6Mt +6gEMzEKadpvE0DfTsRcBYQEWWX4cF8NT/dFyy0xgFdp94uqtUO+O4ovUandV1nDZ +a7vx7jkEOKO94tHgZmvinEeZ6SjmtvwuymdDhOjVg9admGsBPoHcPHrK+fOc99Yo +Gyd4fMPQ1WOngTSJrSVqvfLq7fpX/OU0xsEPcS3SCBAbrURB4P55oGOTirFd6bDu +bwIDAQAB +-----END PUBLIC KEY----- diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data index 391d6c5b1..32957266c 100644 --- a/tests/suites/test_suite_pkparse.data +++ b/tests/suites/test_suite_pkparse.data @@ -103,12 +103,11 @@ depends_on:MBEDTLS_DES_C:MBEDTLS_SHA1_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_PKCS5_C:MBED pk_parse_keyfile_rsa:"data_files/pkcs8_pbes2_pbkdf2_des.key":"PolarSSLTest":0 Parse Public RSA Key #1 (PKCS#8 wrapped) -depends_on:MBEDTLS_MD5_C:MBEDTLS_PEM_PARSE_C -pk_parse_public_keyfile_rsa:"data_files/format_gen.pub":0 +depends_on:MBEDTLS_PEM_PARSE_C +pk_parse_public_keyfile_rsa:"data_files/rsa_pkcs8_2048_public.pem":0 Parse Public RSA Key #1 (PKCS#8 wrapped, DER) -depends_on:MBEDTLS_MD5_C -pk_parse_public_keyfile_rsa:"data_files/rsa_pkcs8_1024_public.der":0 +pk_parse_public_keyfile_rsa:"data_files/rsa_pkcs8_2048_public.der":0 Parse Public RSA Key #3 (PKCS#1 wrapped) depends_on:MBEDTLS_RSA_C:MBEDTLS_PEM_PARSE_C From 1072e5c7e59f18db27d56720072e0b5fcfa3573b Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Wed, 7 Feb 2018 18:43:02 +0200 Subject: [PATCH 7/9] Update ChangeLog style Add dot at end of change in ChangeLog --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index f13982b62..2f1736713 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,7 +4,7 @@ mbed TLS ChangeLog (Sorted per branch, date) Features * Allow comments in test data files. - * Add support for public keys encoded in PKCS#1 format + * Add support for public keys encoded in PKCS#1 format. Bugfix * Fix memory leak in mbedtls_ssl_set_hostname() when called multiple times. From 9566ff791337d6645943d1123e041b65149b3a99 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Wed, 7 Feb 2018 18:59:41 +0200 Subject: [PATCH 8/9] Fix minor issues raised in PR review 1. Style issues fixes - remove redundant spacing. 2. Remove depency of `MBEDTLS_RSA_C` in `pk_parse_public_keyfile_rsa()` tests, as the function itself is dependent on it. --- library/pkparse.c | 8 ++++---- tests/suites/test_suite_pkparse.data | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 41eeadf45..cccc0b596 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -1325,16 +1325,16 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, if( ( ret = mbedtls_pk_setup( ctx, pk_info ) ) != 0 ) return( ret ); - p = (unsigned char *) key; + p = (unsigned char *)key; ret = pk_get_rsapubkey( &p, p + keylen, mbedtls_pk_rsa( *ctx ) ); - if ( ret == 0 ) + if( ret == 0 ) { return( ret ); } mbedtls_pk_free( ctx ); - if ( ret != ( MBEDTLS_ERR_PK_INVALID_PUBKEY + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG ) ) + if( ret != ( MBEDTLS_ERR_PK_INVALID_PUBKEY + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG ) ) { - return ( ret ); + return( ret ); } #endif /* MBEDTLS_RSA_C */ p = (unsigned char *) key; diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data index 32957266c..e9f65c9c9 100644 --- a/tests/suites/test_suite_pkparse.data +++ b/tests/suites/test_suite_pkparse.data @@ -110,11 +110,10 @@ Parse Public RSA Key #1 (PKCS#8 wrapped, DER) pk_parse_public_keyfile_rsa:"data_files/rsa_pkcs8_2048_public.der":0 Parse Public RSA Key #3 (PKCS#1 wrapped) -depends_on:MBEDTLS_RSA_C:MBEDTLS_PEM_PARSE_C +depends_on:MBEDTLS_PEM_PARSE_C 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 pk_parse_public_keyfile_rsa:"data_files/rsa_pkcs1_2048_public.der":0 Parse Public EC Key #1 (RFC 5480, DER) From 3dabd6a145fc8807a8c02de60d5fe876faa01a56 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 14 Feb 2018 17:19:41 +0100 Subject: [PATCH 9/9] Add issue number to ChangeLog Resolves #1122 --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 2f1736713..14e09825b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,7 +4,7 @@ mbed TLS ChangeLog (Sorted per branch, date) Features * Allow comments in test data files. - * Add support for public keys encoded in PKCS#1 format. + * Add support for public keys encoded in PKCS#1 format. #1122 Bugfix * Fix memory leak in mbedtls_ssl_set_hostname() when called multiple times.