From 1a65dcd44fe2f45a085f35425138eeed289f94c2 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 31 Jan 2019 08:57:44 +0000 Subject: [PATCH 1/4] Add a new X.509 API call for copy-less parsing of CRTs Context: The existing API `mbedtls_x509_parse_crt_der()` for parsing DER encoded X.509 CRTs unconditionally makes creates a copy of the input buffer in RAM. While this comes at the benefit of easy use, -- specifically: allowing the user to free or re-use the input buffer right after the call -- it creates a significant memory overhead, as the CRT is duplicated in memory (at least temporarily). This might not be tolerable a resource constrained device. As a remedy, this commit adds a new X.509 API call `mbedtls_x509_parse_crt_der_nocopy()` which has the same signature as `mbedtls_x509_parse_crt_der()` and almost the same semantics, with one difference: The input buffer must persist and be unmodified for the lifetime of the established instance of `mbedtls_x509_crt`, that is, until `mbedtls_x509_crt_free()` is called. --- include/mbedtls/x509_crt.h | 58 ++++++++++++++++++++++++++---- library/x509_crt.c | 73 ++++++++++++++++++++++++-------------- 2 files changed, 98 insertions(+), 33 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index 3dd592248..72c39019b 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -52,6 +52,8 @@ extern "C" { */ typedef struct mbedtls_x509_crt { + int own_buffer; /**< Indicates if \c raw is owned + * by the structure or not. */ mbedtls_x509_buf raw; /**< The raw certificate data (DER). */ mbedtls_x509_buf tbs; /**< The raw certificate body (DER). The part that is To Be Signed. */ @@ -220,16 +222,58 @@ extern const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_suiteb; /** * \brief Parse a single DER formatted certificate and add it - * to the chained list. + * to the end of the provided chained list. * - * \param chain points to the start of the chain - * \param buf buffer holding the certificate DER data - * \param buflen size of the buffer + * \param chain The pointer to the start of the CRT chain to attach to. + * When parsing the first CRT in a chain, this should point + * to an instance of ::mbedtls_x509_crt initialized through + * mbedtls_x509_crt_init(). + * \param buf The buffer holding the DER encoded certificate. + * \param buflen The size in Bytes of \p buf. * - * \return 0 if successful, or a specific X509 or PEM error code + * \note This function makes an internal copy of the CRT buffer + * \p buf. In particular, \p buf may be destroyed or reused + * after this call returns. To avoid duplicating the CRT + * buffer (at the cost of stricter lifetime constraints), + * use mbedtls_x509_crt_parse_der_nocopy() instead. + * + * \return \c 0 if successful. + * \return A negative error code on failure. */ -int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, const unsigned char *buf, - size_t buflen ); +int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, + const unsigned char *buf, + size_t buflen ); + +/** + * \brief Parse a single DER formatted certificate and add it + * to the end of the provided chained list. This is a + * variant of mbedtls_x509_crt_parse_der() which takes + * temporary ownership of the CRT buffer until the CRT + * is destroyed. + * + * \param chain The pointer to the start of the CRT chain to attach to. + * When parsing the first CRT in a chain, this should point + * to an instance of ::mbedtls_x509_crt initialized through + * mbedtls_x509_crt_init(). + * \param buf The address of the readable buffer holding the DER encoded + * certificate to use. On success, this buffer must be + * retained and not be changed for the liftetime of the + * CRT chain \p chain, that is, until \p chain is destroyed + * through a call to mbedtls_x509_crt_free(). + * \param buflen The size in Bytes of \p buf. + * + * \note This call is functionally equivalent to + * mbedtls_x509_crt_parse_der(), but it avoids creating a + * copy of the input buffer at the cost of stronger lifetime + * constraints. This is useful in constrained environments + * where duplication of the CRT cannot be tolerated. + * + * \return \c 0 if successful. + * \return A negative error code on failure. + */ +int mbedtls_x509_crt_parse_der_nocopy( mbedtls_x509_crt *chain, + const unsigned char *buf, + size_t buflen ); /** * \brief Parse one DER-encoded or one or more concatenated PEM-encoded diff --git a/library/x509_crt.c b/library/x509_crt.c index 32aca012b..2c1170323 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -829,8 +829,10 @@ static int x509_get_crt_ext( unsigned char **p, /* * Parse and fill a single X.509 certificate in DER format */ -static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, const unsigned char *buf, - size_t buflen ) +static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, + const unsigned char *buf, + size_t buflen, + int make_copy ) { int ret; size_t len; @@ -847,7 +849,7 @@ static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, const unsigned char * if( crt == NULL || buf == NULL ) return( MBEDTLS_ERR_X509_BAD_INPUT_DATA ); - // Use the original buffer until we figure out actual length + /* Use the original buffer until we figure out actual length. */ p = (unsigned char*) buf; len = buflen; end = p + len; @@ -865,25 +867,26 @@ static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, const unsigned char * return( MBEDTLS_ERR_X509_INVALID_FORMAT ); } - if( len > (size_t) ( end - p ) ) - { - mbedtls_x509_crt_free( crt ); - return( MBEDTLS_ERR_X509_INVALID_FORMAT + - MBEDTLS_ERR_ASN1_LENGTH_MISMATCH ); - } - crt_end = p + len; - - // Create and populate a new buffer for the raw field - crt->raw.len = crt_end - buf; - crt->raw.p = p = mbedtls_calloc( 1, crt->raw.len ); - if( p == NULL ) - return( MBEDTLS_ERR_X509_ALLOC_FAILED ); - - memcpy( p, buf, crt->raw.len ); - - // Direct pointers to the new buffer - p += crt->raw.len - len; end = crt_end = p + len; + crt->raw.len = crt_end - buf; + if( make_copy != 0 ) + { + /* Create and populate a new buffer for the raw field. */ + crt->raw.p = p = mbedtls_calloc( 1, crt->raw.len ); + if( crt->raw.p == NULL ) + return( MBEDTLS_ERR_X509_ALLOC_FAILED ); + + memcpy( crt->raw.p, buf, crt->raw.len ); + crt->own_buffer = 1; + + p += crt->raw.len - len; + end = crt_end = p + len; + } + else + { + crt->raw.p = (unsigned char*) buf; + crt->own_buffer = 0; + } /* * TBSCertificate ::= SEQUENCE { @@ -1086,8 +1089,10 @@ static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, const unsigned char * * Parse one X.509 certificate in DER format from a buffer and add them to a * chained list */ -int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, const unsigned char *buf, - size_t buflen ) +static int mbedtls_x509_crt_parse_der_internal( mbedtls_x509_crt *chain, + const unsigned char *buf, + size_t buflen, + int make_copy ) { int ret; mbedtls_x509_crt *crt = chain, *prev = NULL; @@ -1119,7 +1124,7 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, const unsigned char *bu crt = crt->next; } - if( ( ret = x509_crt_parse_der_core( crt, buf, buflen ) ) != 0 ) + if( ( ret = x509_crt_parse_der_core( crt, buf, buflen, make_copy ) ) != 0 ) { if( prev ) prev->next = NULL; @@ -1133,11 +1138,27 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, const unsigned char *bu return( 0 ); } +int mbedtls_x509_crt_parse_der_nocopy( mbedtls_x509_crt *chain, + const unsigned char *buf, + size_t buflen ) +{ + return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, 0 ) ); +} + +int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, + const unsigned char *buf, + size_t buflen ) +{ + return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, 1 ) ); +} + /* * Parse one or more PEM certificates from a buffer and add them to the chained * list */ -int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen ) +int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, + const unsigned char *buf, + size_t buflen ) { #if defined(MBEDTLS_PEM_PARSE_C) int success = 0, first_error = 0, total_failed = 0; @@ -2675,7 +2696,7 @@ void mbedtls_x509_crt_free( mbedtls_x509_crt *crt ) mbedtls_free( seq_prv ); } - if( cert_cur->raw.p != NULL ) + if( cert_cur->raw.p != NULL && cert_cur->own_buffer ) { mbedtls_platform_zeroize( cert_cur->raw.p, cert_cur->raw.len ); mbedtls_free( cert_cur->raw.p ); From 462c3e5210aa89d931341576541bd58f9ef7bba0 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 31 Jan 2019 10:55:42 +0000 Subject: [PATCH 2/4] Add test for mbedtls_x509_parse_file() with DER encoded CRT --- tests/data_files/Makefile | 8 ++++++-- tests/data_files/server1.der | Bin 0 -> 835 bytes tests/data_files/server2.der | Bin 0 -> 827 bytes tests/data_files/test-ca.der | Bin 0 -> 837 bytes tests/suites/test_suite_x509parse.data | 12 ++++++++++++ 5 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 tests/data_files/server1.der create mode 100644 tests/data_files/server2.der create mode 100644 tests/data_files/test-ca.der diff --git a/tests/data_files/Makefile b/tests/data_files/Makefile index 2ed32e689..ff8947686 100644 --- a/tests/data_files/Makefile +++ b/tests/data_files/Makefile @@ -45,7 +45,9 @@ all_intermediate += test-ca.req.sha256 test-ca.crt: $(test_ca_key_file_rsa) test-ca.req.sha256 $(MBEDTLS_CERT_WRITE) is_ca=1 serial=3 request_file=test-ca.req.sha256 selfsign=1 issuer_name="C=NL,O=PolarSSL,CN=PolarSSL Test CA" issuer_key=$(test_ca_key_file_rsa) issuer_pwd=$(test_ca_pwd_rsa) not_before=20110212144400 not_after=20210212144400 md=SHA1 version=3 output_file=$@ -all_final += test-ca.crt +test-ca.der: test-ca.crt + $(OPENSSL) x509 -inform PEM -in $< -outform DER -out $@ +all_final += test-ca.crt test-ca.der test-ca-sha1.crt: $(test_ca_key_file_rsa) test-ca.req.sha256 $(MBEDTLS_CERT_WRITE) is_ca=1 serial=3 request_file=test-ca.req.sha256 selfsign=1 issuer_name="C=NL,O=PolarSSL,CN=PolarSSL Test CA" issuer_key=$(test_ca_key_file_rsa) issuer_pwd=$(test_ca_pwd_rsa) not_before=20110212144400 not_after=20210212144400 md=SHA1 version=3 output_file=$@ @@ -873,7 +875,9 @@ server1_all: server1.crt server1.noauthid.crt server1.crt.openssl server1.v1.crt server2.crt: server2.req.sha256 $(MBEDTLS_CERT_WRITE) request_file=server2.req.sha256 serial=2 issuer_crt=$(test_ca_crt) issuer_key=$(test_ca_key_file_rsa) issuer_pwd=$(test_ca_pwd_rsa) not_before=20110212144406 not_after=20210212144406 md=SHA1 version=3 output_file=$@ -all_final += server2.crt +server2.der: server2.crt + $(OPENSSL) x509 -inform PEM -in $< -outform DER -out $@ +all_final += server2.crt server2.der server2-sha256.crt: server2.req.sha256 $(MBEDTLS_CERT_WRITE) request_file=server2.req.sha256 serial=2 issuer_crt=$(test_ca_crt) issuer_key=$(test_ca_key_file_rsa) issuer_pwd=$(test_ca_pwd_rsa) not_before=20110212144406 not_after=20210212144406 md=SHA256 version=3 output_file=$@ diff --git a/tests/data_files/server1.der b/tests/data_files/server1.der new file mode 100644 index 0000000000000000000000000000000000000000..fcf45cd7cc3886757acfafa089118900184195e4 GIT binary patch literal 835 zcmXqLVzxJEVp3ng%*4pV#K>sC%f_kI=F#?@mywZ`mBGN;klTQhjX9KsO_<5g$57CK zAH?C};RwjjNh}Hu_A!(+5C;h{^9aC%6hcyqOB9?P4dldm4Gj&942=v;OiT>SqQrTP zkhumn1PzxmkboF22sb=9wWut$NWsvciBSpJwT!F`%uS5^3_x)%rY1&4hLue6whmcW zLxa2jn!RgE)e}vO>)gNNh3kad?>fYSE`M|maGxd=nbMy9SNnn6&*FV|&rfK`77Q9{jMpRO6g)xWwK~|@ge|-*bxqp{U-d7;d zA-!0b-{D7YqiQ_Y#^7THb)uGQen!2kpEPe7YxHyB>8)FpC*8cF!giHYwX>A{?lP%< zdrrxHYg2VnUQeBU=bvMo__A9$(V1tMc8TbSsm$@ZbN0gbp!DL8x(k&5)_pNNrCV^S zlbhwX-ZKA!ym{yLMsz+3j+~blH7WH`hds{}$;Ee{zL+~z_^syc)dfO#qE2OtuMTPo z*~rAq$iTSR*T5SbfwICZOa=@FvcTY!FH%ccPe~-etutVQ*+wqDnp6m_JcfzA8T@! z2_4#?pxHEGwy}oaoy!&9XSv6&+kN)tBZ-cE+n*oVexg{pW2@8JB_Rtuw_Njkbn3>( zwTEW^|4}IR@x>9_+>%qXcl|gs)nv~rr=WGNJp6ong3V9&{W4z|@u*t-)+TEX&O+Zs z<;vTH)_U*$ac;Ll%;nGDGb%sa5T9cm5d2X3+f|2et~z@4x6W7p4 literal 0 HcmV?d00001 diff --git a/tests/data_files/server2.der b/tests/data_files/server2.der new file mode 100644 index 0000000000000000000000000000000000000000..ec03190e12610688838c1ff3f27b0fb26632885d GIT binary patch literal 827 zcmXqLVm3EuVv=9L%*4n9LZ#K0^{oYx4M zYhXgqa3KQ$h~b<$`N@en8TrK}22G4g$gX5$WngY%%5h)*oCMkXBRVcEPp4LJ#Bf=mI?25i>mBYX8qY_vPR`=`2)3- zo;e#dY8T#m)${D6%(OlK4zdJoHoLO;*TaovHzpiR+>#b#wn!~_)#{Qs_FBoN+gdl| z7u@8P(e+IG9<5sJ_JX_1Ka*!G!-R*ongr5n*M(?zr&dl}_$cx4SqD#!cNsh%yW1|g z?Z2>Nl_0ZReb@>qITs0j{?_hW-7ayDB#tHNA5ZK36?>!hvwEi{6u~JpTQa5Tn6# zCT2zk#>Kt{-r$In6=q>FU@(vc2BRz=ix`W@Qq~tKJMP?1;13Y;O<0k#-nZL%vVlBE zTA4+{K&(MzOVpF4o9|r;`nL1xvZ?&9?e-l1`yV;Lfyn|G;6OWyxxJ2_UU2VYvP>C^ zwlMF37Qv(aR?CmhF|8`!p&-)qF66_f4MC?X&PB5O2WI}etpAepdF!MbyEeW)S{9qA z`?788J*}Vi!5U6&&Be|SSmN0yh@{TX6R6q~A*poewPjJ@r0ZK`OZ6{XX)`{*9kA}v z$1A?kHoG0QwU#{cVtSe&qBBQ+*%>x()lR=@21;{cB76_ux{wzj*OU77rnu>{2Wt7#C=BTy4jaQ zS-KblwmeR%$PpA>swH^PQ)c!Nfz*Wdn{t_ve*M|65B|%3lw2`2?}6_1<>o5@#6dhp literal 0 HcmV?d00001 diff --git a/tests/data_files/test-ca.der b/tests/data_files/test-ca.der new file mode 100644 index 0000000000000000000000000000000000000000..039fb9e43004e622bd1404116f68208800005c6d GIT binary patch literal 837 zcmXqLVs&G%f_kI=F#?@mywZ`mBGN;klTQhjX9KsO_<5g$57CK zAH?C};RwjjNh}Hu_A!(+5C;h{^9aC%6hcyqOB9?P4dldm4Gj&942=v;OiT<6qQrTP zkhzo@-o&Vc>{v!t2IeM4eg=akMlPl%Mn;AM_s#!^?|v|Cu6^6RX-2g!OT`wPRs1;f z%9~fGYa}8#rYwCk`)K!lDY=;zGu!2=5A<5zw}>sMV81-?=HwSUivo|HTWk=t^3!vN z0+G`$i;B1pJ$3kL_jDQG=AUo8k`L_AWGI;vZoOhD%Y?#@dz)|CUt9XfMyvn5dcxsj z^H1-3lTf?;S&Pv=|KAa6O3cw$wp{)F_3<>lf&)+V_Wsd(_sB8yfQeqMN>S!%_l+VB z&9&)Y+P)dC{#dzW(^fs9pDp4alJeExvi&hv5v`G zd4cFBi_{d(S3G%r(&7sWPi&rja`nr@pU$^W>u+E(nm02df6-MYW=00a#Q_F>20Xy{ zkrifPHDG3B{BIx&;_TP!7QC4hV&D%ba$=(yp zSf5T{U|xOMYODB`ORhngYUdu$ke${2W5Fa@4<>WHgK<+21^?T)$E3-`#B5?uM^*ZC z6Nm2K9(kA78#MN@`c78-w( Date: Thu, 31 Jan 2019 09:15:53 +0000 Subject: [PATCH 3/4] Modify existing X.509 test for also test new copyless API The existing test `x509parse_crt()` for X.509 CRT parsing so far used the generic parsing API `mbedtls_x509_crt_parse()` capable of parsing both PEM encoded and DER encoded certficates, but was actually only used with DER encoded input data. Moreover, as the purpose of the test is the testing of the core DER X.509 parsing functionality, not the PEM vs. DER dispatch (which is now already tested in the various `x509_crt_info()` tests), the call can be replaced with a direct call to `mbedtls_x509_parse_crt_der()`. This commit does that, and further adds to the test an analogous call to the new API `mbedtls_x509_parse_crt_der_nocopy()` to test copyless parsing of X.509 certificates. --- tests/suites/test_suite_x509parse.function | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index 552c494b0..a921310c6 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -505,8 +505,22 @@ void x509parse_crt( data_t * buf, char * result_str, int result ) mbedtls_x509_crt_init( &crt ); memset( output, 0, 2000 ); + TEST_ASSERT( mbedtls_x509_crt_parse_der( &crt, buf->x, buf->len ) == ( result ) ); + if( ( result ) == 0 ) + { + res = mbedtls_x509_crt_info( (char *) output, 2000, "", &crt ); - TEST_ASSERT( mbedtls_x509_crt_parse( &crt, buf->x, buf->len ) == ( result ) ); + TEST_ASSERT( res != -1 ); + TEST_ASSERT( res != -2 ); + + TEST_ASSERT( strcmp( (char *) output, result_str ) == 0 ); + } + + mbedtls_x509_crt_free( &crt ); + mbedtls_x509_crt_init( &crt ); + memset( output, 0, 2000 ); + + TEST_ASSERT( mbedtls_x509_crt_parse_der_nocopy( &crt, buf->x, buf->len ) == ( result ) ); if( ( result ) == 0 ) { res = mbedtls_x509_crt_info( (char *) output, 2000, "", &crt ); From ac4172c5bb619c5b96699294012d7629acaf38b3 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 31 Jan 2019 09:27:04 +0000 Subject: [PATCH 4/4] Adapt ChangeLog --- ChangeLog | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ChangeLog b/ChangeLog index 5c2fbbbd4..6c3e4763e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,16 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS 2.x.x branch released xxxx-xx-xx +Features + * Add a new X.509 API call `mbedtls_x509_parse_der_nocopy()` + which allows copy-less parsing of DER encoded X.509 CRTs, + at the cost of additional lifetime constraints on the input + buffer, but at the benefit of reduced RAM consumption. + +API Changes + * Add a new X.509 API call `mbedtls_x509_parse_der_nocopy()`. + See the Features section for more information. + Bugfix * Fix a compilation issue with mbedtls_ecp_restart_ctx not being defined when MBEDTLS_ECP_ALT is defined. Reported by jwhui. Fixes #2242.