From 8c4472af397c58c0a28416881f31229d554bdecd Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Sat, 13 Feb 2016 23:19:04 +0000 Subject: [PATCH 01/32] Clarified mbedtls_ssl_conf_alpn_protocols() doc Clarified the lifetime of the protos parameter passed in the function mbedtls_ssl_conf_alpn_protocols(). --- include/mbedtls/ssl.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 00e1c6c6e..ad0e42c6c 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1762,8 +1762,11 @@ int mbedtls_ssl_set_hs_ecjpake_password( mbedtls_ssl_context *ssl, * \brief Set the supported Application Layer Protocols. * * \param conf SSL configuration - * \param protos NULL-terminated list of supported protocols, - * in decreasing preference order. + * \param protos Pointer to a NULL-terminated list of supported protocols, + * in decreasing preference order. The pointer to the list is + * recorded by the library for later reference as required, so + * the lifetime of the table should be as long as the + * SSL configuration structure. * * \return 0 on success, or MBEDTLS_ERR_SSL_BAD_INPUT_DATA. */ From 4b17e53c72b9bc8a4d3993abff23040b3c7b2f9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 22 Feb 2016 10:47:43 +0100 Subject: [PATCH 02/32] Fix Unix detection in mini_client fixes #398 --- programs/ssl/mini_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/ssl/mini_client.c b/programs/ssl/mini_client.c index d61312425..26082ef5b 100644 --- a/programs/ssl/mini_client.c +++ b/programs/ssl/mini_client.c @@ -36,7 +36,7 @@ * This is not a good example for general use. This programs has the specific * goal of minimizing use of the libc functions on full-blown OSes. */ -#if defined(unix) || defined(__unix__) || defined(__unix) +#if defined(unix) || defined(__unix__) || defined(__unix) || defined(__APPLE__) #define UNIX #endif From b967c15e4034e3e7bd890d28346ec0590db172d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 22 Feb 2016 09:33:52 +0100 Subject: [PATCH 03/32] Improve documentation of some SSL callbacks The previous documentation was not explicit about what was expected of the callbacks - the user had to infer that from the descriptions in net.h or timing.h, and it was not clear what was part of the calling convention and what was specific to our implementation. --- include/mbedtls/ssl.h | 153 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 136 insertions(+), 17 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index ad0e42c6c..4aad2a829 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -969,6 +969,76 @@ void mbedtls_ssl_conf_dbg( mbedtls_ssl_config *conf, void (*f_dbg)(void *, int, const char *, int, const char *), void *p_dbg ); +/** + * \brief Callback type: send data on the network. + * + * \note That callback may be either blocking or non-blocking. + * + * \param ctx Context for the send callback (typically a file descriptor) + * \param buf Buffer holding the date to send + * \param len Length of the data to send + * + * \return The callback must return the number of bytes sent if any, + * or a non-zero error code. + * If performing non-blocking I/O, \c MBEDTLS_ERR_SSL_WANT_WRITE + * must be returned when the operation would block. + * + * \note The callback is allowed to send less bytes than requested. + * It must always return the number of bytes actually sent. + */ +typedef int mbedtls_ssl_send_t( void *ctx, + const unsigned char *buf, + size_t len ); + +/** + * \brief Callback type: receive data from the network. + * + * \note That callback may be either blocking or non-blocking. + * + * \param ctx Context for the send callback (typically a file descriptor) + * \param buf Buffer to write the received data to + * \param len Length of the receive buffer + * + * \return The callback must return the number of bytes received, + * or a non-zero error code. + * If performing non-blocking I/O, \c MBEDTLS_ERR_SSL_WANT_READ + * must be returned when the operation would block. + * + * \note The callback may receive less bytes than the length of the + * buffer. It must always return the number of bytes actually + * received and written to the buffer. + */ +typedef int mbedtls_ssl_recv_t( void *ctx, + unsigned char *buf, + size_t len ); + +/** + * \brief Callback type: receive data from the network, with timeout + * + * \note That callback must block until data is received, or the + * timeout delay expires, or the operation is interrupted by a + * signal. + * + * \param ctx Context for the send callback (typically a file descriptor) + * \param buf Buffer to write the received data to + * \param len Length of the receive buffer + * \param timeout Maximum nomber of millisecondes to wait for data + * 0 means no timeout (potentially wait forever) + * + * \return The callback must return the number of bytes received, + * or a non-zero error code: + * \c MBEDTLS_ERR_SSL_TIMEOUT if the operation timed out, + * \c MBEDTLS_ERR_SSL_WANT_READ if interrupted by a signal. + * + * \note The callback may receive less bytes than the length of the + * buffer. It must always return the number of bytes actually + * received and written to the buffer. + */ +typedef int mbedtls_ssl_recv_timeout_t( void *ctx, + unsigned char *buf, + size_t len, + uint32_t timeout ); + /** * \brief Set the underlying BIO callbacks for write, read and * read-with-timeout. @@ -978,8 +1048,6 @@ void mbedtls_ssl_conf_dbg( mbedtls_ssl_config *conf, * \param f_send write callback * \param f_recv read callback * \param f_recv_timeout blocking read callback with timeout. - * The last argument is the timeout in milliseconds, - * 0 means no timeout (block forever until a message comes) * * \note One of f_recv or f_recv_timeout can be NULL, in which case * the other is used. If both are non-NULL, f_recv_timeout is @@ -991,12 +1059,20 @@ void mbedtls_ssl_conf_dbg( mbedtls_ssl_config *conf, * * \note For DTLS, you need to provide either a non-NULL * f_recv_timeout callback, or a f_recv that doesn't block. + * + * \note See the documentations of \c mbedtls_ssl_sent_t, + * \c mbedtls_ssl_recv_t and \c mbedtls_ssl_recv_timeout_t for + * the convetions those callbacks must follow. + * + * \note On some platforms, net.c provides \c mbedtls_net_send(), + * \c mbedtls_net_recv() and \c mbedtls_net_recv_timeout() + * that are suitable to be used here. */ void mbedtls_ssl_set_bio( mbedtls_ssl_context *ssl, - void *p_bio, - int (*f_send)(void *, const unsigned char *, size_t), - int (*f_recv)(void *, unsigned char *, size_t), - int (*f_recv_timeout)(void *, unsigned char *, size_t, uint32_t) ); + void *p_bio, + mbedtls_ssl_send_t *f_send, + mbedtls_ssl_recv_t *f_recv, + mbedtls_ssl_recv_timeout_t *f_recv_timeout ); /** * \brief Set the timeout period for mbedtls_ssl_read() @@ -1017,24 +1093,67 @@ void mbedtls_ssl_set_bio( mbedtls_ssl_context *ssl, void mbedtls_ssl_conf_read_timeout( mbedtls_ssl_config *conf, uint32_t timeout ); /** - * \brief Set the timer callbacks - * (Mandatory for DTLS.) + * \brief Callback type: set a pair of timers/delays to watch + * + * \param ctx Context pointer + * \param int_ms Intermediate delay in milliseconds + * \param fin_ms Final delay in milliseconds + * 0 cancels the current timer. + * + * \note This callback must at least store the necessary information + * for the associated \c mbedtls_ssl_get_timer_t callback to + * return correct information. + * + * \note If using a event-driven style of programming, an event must + * be generated when the final delay is passed. The event must + * cause a call to \c mbedtls_ssl_handshake() with the proper + * SSL context to be scheduled. Care must be taken to ensure + * that at most one such call happens at a time. + * + * \note Only one timer at a time must be running. Calling this + * function while a timer is running must cancel it. Cancelled + * timers must not generate any event. + */ +typedef void mbedtls_ssl_set_timer_t( void * ctx, + uint32_t int_ms, + uint32_t fin_ms ); + +/** + * \brief Callback type: get status of timers/delays + * + * \param ctx Context pointer + * + * \return This callback must return: + * -1 if cancelled (fin_ms == 0), + * 0 if none of the delays is passed, + * 1 if only the intermediate delay is passed, + * 2 if the final delay is passed. + */ +typedef int mbedtls_ssl_get_timer_t( void * ctx ); + +/** + * \brief Set the timer callbacks (Mandatory for DTLS.) * * \param ssl SSL context - * \param p_timer parameter (context) shared by timer callback + * \param p_timer parameter (context) shared by timer callbacks * \param f_set_timer set timer callback - * Accepts an intermediate and a final delay in milliseconcs - * If the final delay is 0, cancels the running timer. * \param f_get_timer get timer callback. Must return: - * -1 if cancelled - * 0 if none of the delays is expired - * 1 if the intermediate delay only is expired - * 2 if the final delay is expired + * + * \note See the documentation of \c mbedtls_ssl_set_timer_t and + * \c mbedtls_ssl_get_timer_t for the conventions this pair of + * callbacks must fallow. + * + * \note On some platforms, timing.c provides + * \c mbedtls_timing_set_delay() and + * \c mbedtls_timing_get_delay() that are suitable for using + * here, except if using an event-driven style. + * + * \note See also the "DTLS tutorial" article in our knowledge base. */ void mbedtls_ssl_set_timer_cb( mbedtls_ssl_context *ssl, void *p_timer, - void (*f_set_timer)(void *, uint32_t int_ms, uint32_t fin_ms), - int (*f_get_timer)(void *) ); + mbedtls_ssl_set_timer_t *f_set_timer, + mbedtls_ssl_get_timer_t *f_get_timer ); /** * \brief Callback type: generate and write session ticket From 7ff4b774b7a0ed48e432a3fdee46269a602034af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 22 Feb 2016 10:33:34 +0100 Subject: [PATCH 04/32] Give better error messages for semi-portable parts Previously it was failing with errors about headers not found, which is suboptimal in terms of clarity. Now give a clean error with pointer to the documentation. Do the checks in the .c files rather than check_config.h as it keeps them closer to the platform-specific implementations. --- include/mbedtls/config.h | 17 ++++++++++++++--- library/entropy_poll.c | 6 ++++++ library/net.c | 5 +++++ library/timing.c | 5 +++++ 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index d1db0d825..c69ba1bcb 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1897,11 +1897,15 @@ /** * \def MBEDTLS_NET_C * - * Enable the TCP/IP networking routines. + * Enable the TCP and UDP over IPv6/IPv4 networking routines. + * + * \note This module only works on Unix (including Linux, BSD and OS X) and + * Windows. For other platforms, you'll want to disable it, and write your + * own networking callbacks to be passed to \c mbedtls_ssl_set_bio(). * * Module: library/net.c * - * This module provides TCP/IP networking routines. + * This module provides networking routines. */ #define MBEDTLS_NET_C @@ -2264,7 +2268,14 @@ /** * \def MBEDTLS_TIMING_C * - * Enable the portable timing interface. + * Enable the semi-portable timing interface. + * + * \note The provided implementation only works on Unix (including Linux, BSD + * and OS X) and Windows. On other platforms, you can either disable that + * module and provide your own implementations of the callbacks needed by + * \c mbedtls_ssl_set_timer_cb() for DTLS, or leave it enabled and provide + * your own implementation of the whole module by setting + * \c MBEDTLS_TIMING_ALT in the current file. * * Module: library/timing.c * Caller: library/havege.c diff --git a/library/entropy_poll.c b/library/entropy_poll.c index 25a27bef3..972ad2aea 100644 --- a/library/entropy_poll.c +++ b/library/entropy_poll.c @@ -39,6 +39,12 @@ #endif #if !defined(MBEDTLS_NO_PLATFORM_ENTROPY) + +#if !defined(unix) && !defined(__unix__) && !defined(__unix) && \ + !defined(__APPLE__) && !defined(_WIN32) +#error "Platform entropy sources only work on Unix and Windows, see MBEDTLS_NO_PLATFORM_ENTROPY in config.h" +#endif + #if defined(_WIN32) && !defined(EFIX64) && !defined(EFI32) #if !defined(_WIN32_WINNT) diff --git a/library/net.c b/library/net.c index a77268c55..3b78b6b15 100644 --- a/library/net.c +++ b/library/net.c @@ -27,6 +27,11 @@ #if defined(MBEDTLS_NET_C) +#if !defined(unix) && !defined(__unix__) && !defined(__unix) && \ + !defined(__APPLE__) && !defined(_WIN32) +#error "This module only works on Unix and Windows, see MBEDTLS_NET_C in config.h" +#endif + #include "mbedtls/net.h" #include diff --git a/library/timing.c b/library/timing.c index 5d8b25b99..a7c7ff027 100644 --- a/library/timing.c +++ b/library/timing.c @@ -38,6 +38,11 @@ #if !defined(MBEDTLS_TIMING_ALT) +#if !defined(unix) && !defined(__unix__) && !defined(__unix) && \ + !defined(__APPLE__) && !defined(_WIN32) +#error "This module only works on Unix and Windows, see MBEDTLS_TIMING_C in config.h" +#endif + #ifndef asm #define asm __asm #endif From c3cb4c75a58e9793019e643a48874dccf41e556f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 22 Feb 2016 11:10:14 +0100 Subject: [PATCH 05/32] Add note about not implementing PSK id_hint --- include/mbedtls/ssl.h | 5 +++++ library/ssl_cli.c | 7 +++++-- library/ssl_srv.c | 3 ++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 4aad2a829..e367c474a 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1613,6 +1613,11 @@ int mbedtls_ssl_conf_own_cert( mbedtls_ssl_config *conf, * \note This is mainly useful for clients. Servers will usually * want to use \c mbedtls_ssl_conf_psk_cb() instead. * + * \note Currently clients can only register one pre-shared key. + * In other words, the servers' idendity hint is ignored. + * Please contact us if you need ability to set multiple PSKs + * on clients and select one based on the identity hint. + * * \param conf SSL configuration * \param psk pointer to the pre-shared key * \param psk_len pre-shared key length diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 4452169d9..1d22d1518 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1981,8 +1981,11 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } - // TODO: Retrieve PSK identity hint and callback to app - // + /* + * Note: we currently ignore the PKS identity hint, as we only allow one + * PSK to be provisionned on the client. This could be changed later if + * someone needs that feature. + */ *p += len; ret = 0; diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 6b5b461e1..6bd0b598a 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2718,7 +2718,8 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_PSK || ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ) { - /* TODO: Support identity hints */ + /* Note: we don't support identity hints, until someone asks + * for them. */ *(p++) = 0x00; *(p++) = 0x00; From fc0e286c0e5484d7524474a0b3d78e000e8881cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 22 Feb 2016 11:18:35 +0100 Subject: [PATCH 06/32] Remove unused code. After the record contents are decompressed, in_len is no longer accessed directly, only in_msglen is accessed. in_len is only read by ssl_parse_record_header() which happens before ssl_prepare_record_contents(). This is also made clear by the fact that in_len is not touched after decrypting anyway, so if it was accessed after that it would be wrong unless decryption is used - as this is not the case, it show in_len is not accessed. --- library/ssl_tls.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index a4cc1ca05..0c1a7cccf 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3706,10 +3706,6 @@ static int ssl_prepare_record_content( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_RET( 1, "ssl_decompress_buf", ret ); return( ret ); } - - // TODO: what's the purpose of these lines? is in_len used? - ssl->in_len[0] = (unsigned char)( ssl->in_msglen >> 8 ); - ssl->in_len[1] = (unsigned char)( ssl->in_msglen ); } #endif /* MBEDTLS_ZLIB_SUPPORT */ From 982b9adc96ac56be49ef2bffaab0b01e8fbe2ac5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 22 Feb 2016 11:27:26 +0100 Subject: [PATCH 07/32] Update note about hardcoded verify_data_length --- library/ssl_tls.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 0c1a7cccf..afbcdd99c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5011,7 +5011,12 @@ int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl ) ssl->handshake->calc_finished( ssl, ssl->out_msg + 4, ssl->conf->endpoint ); - // TODO TLS/1.2 Hash length is determined by cipher suite (Page 63) + /* + * RFC 5246 7.4.9 (Page 63) says 12 is the default length and ciphersuites + * may define some other value. Currently (early 2016), no defined + * ciphersuite does this (and this is unlikely to change as activity has + * moved to TLS 1.3 now) so we can keep the hardcoded 12 here. + */ hash_len = ( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) ? 36 : 12; #if defined(MBEDTLS_SSL_RENEGOTIATION) From 0fa5b055c93834b31f63b965d54761d9ac09b414 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 22 Feb 2016 11:36:55 +0100 Subject: [PATCH 08/32] Clarify documentation about missing CRLs Also tune up some working while at it. --- include/mbedtls/x509_crt.h | 17 +++++++++++------ library/x509_crt.c | 9 ++------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index fe821d1cf..41b6bfe57 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -271,9 +271,14 @@ int mbedtls_x509_crt_verify_info( char *buf, size_t size, const char *prefix, * \note Same as \c mbedtls_x509_crt_verify_with_profile() with the * default security profile. * - * \param crt a certificate to be verified - * \param trust_ca the trusted CA chain - * \param ca_crl the CRL chain for trusted CA's + * \note It is your responsibility to provide up-to-date CRLs for + * all trusted CAs. If no CRL is provided for the CA that was + * used to sign the certificate, CRL verification is skipped + * silently, that is *without* setting any flag. + * + * \param crt a certificate (chain) to be verified + * \param trust_ca the list of trusted CAs + * \param ca_crl the list of CRLs for trusted CAs (see note above) * \param cn expected Common Name (can be set to * NULL if the CN must not be verified) * \param flags result of the verification @@ -304,9 +309,9 @@ int mbedtls_x509_crt_verify( mbedtls_x509_crt *crt, * for ECDSA) apply to all certificates: trusted root, * intermediate CAs if any, and end entity certificate. * - * \param crt a certificate to be verified - * \param trust_ca the trusted CA chain - * \param ca_crl the CRL chain for trusted CA's + * \param crt a certificate (chain) to be verified + * \param trust_ca the list of trusted CAs + * \param ca_crl the list of CRLs for trusted CAs * \param profile security profile for verification * \param cn expected Common Name (can be set to * NULL if the CN must not be verified) diff --git a/library/x509_crt.c b/library/x509_crt.c index 6dc5ad34f..0606eb96d 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1600,7 +1600,8 @@ int mbedtls_x509_crt_is_revoked( const mbedtls_x509_crt *crt, const mbedtls_x509 } /* - * Check that the given certificate is valid according to the CRL. + * Check that the given certificate is not revoked according to the CRL. + * Skip validation is no CRL for the given CA is present. */ static int x509_crt_verifycrl( mbedtls_x509_crt *crt, mbedtls_x509_crt *ca, mbedtls_x509_crl *crl_list, @@ -1613,12 +1614,6 @@ static int x509_crt_verifycrl( mbedtls_x509_crt *crt, mbedtls_x509_crt *ca, if( ca == NULL ) return( flags ); - /* - * TODO: What happens if no CRL is present? - * Suggestion: Revocation state should be unknown if no CRL is present. - * For backwards compatibility this is not yet implemented. - */ - while( crl_list != NULL ) { if( crl_list->version == 0 || From b222cd92c17bb8bae5958fd1b29c266387d5a3ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 22 Feb 2016 12:02:30 +0100 Subject: [PATCH 09/32] Remove unnecessary TODO comment We don't implement anonymous key exchanges, and we don't intend to, so it can never happen that an unauthenticated server requests a certificate from us. --- library/ssl_cli.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 1d22d1518..5ce7d2529 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2581,9 +2581,6 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) ssl->record_read = 0; - // TODO: handshake_failure alert for an anonymous server to request - // client authentication - /* * struct { * ClientCertificateType certificate_types<1..2^8-1>; From 04d39d282569e33dd5c53a5074b6bab55e5f3727 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 24 Feb 2016 14:13:22 +0000 Subject: [PATCH 10/32] ssl: ignore CertificateRequest's content for real - document why we made that choice - remove the two TODOs about checking hash and CA - remove the code that parsed certificate_type: it did nothing except store the selected type in handshake->cert_type, but that field was never accessed afterwards. Since handshake_params is now an internal type, we can remove that field without breaking the ABI. --- include/mbedtls/ssl.h | 7 +++- include/mbedtls/ssl_internal.h | 1 - library/ssl_cli.c | 67 +++++++++++++--------------------- 3 files changed, 31 insertions(+), 44 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index e367c474a..3a8b73362 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1593,7 +1593,12 @@ void mbedtls_ssl_conf_ca_chain( mbedtls_ssl_config *conf, * adequate, preference is given to the one set by the first * call to this function, then second, etc. * - * \note On client, only the first call has any effect. + * \note On client, only the first call has any effect. That is, + * only one client certificate can be provisioned. The + * server's preferences in its CertficateRequest message will + * be ignored and our only cert will be sent regardless of + * whether it matches those preferences - the server can then + * decide what it wants to do with it. * * \param conf SSL configuration * \param own_cert own public certificate chain diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 3af059f89..d63d7d4e7 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -166,7 +166,6 @@ struct mbedtls_ssl_handshake_params * Handshake specific crypto variables */ int sig_alg; /*!< Hash algorithm for signature */ - int cert_type; /*!< Requested cert type */ int verify_sig_alg; /*!< Signature algorithm for verify */ #if defined(MBEDTLS_DHM_C) mbedtls_dhm_context dhm_ctx; /*!< DHM key exchange */ diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 5ce7d2529..bf6c22101 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2532,8 +2532,8 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) { int ret; - unsigned char *buf, *p; - size_t n = 0, m = 0; + unsigned char *buf; + size_t n = 0; size_t cert_type_len = 0, dn_len = 0; const mbedtls_ssl_ciphersuite_t *ciphersuite_info = ssl->transform_negotiate->ciphersuite_info; @@ -2588,11 +2588,26 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) * supported_signature_algorithms<2^16-1>; -- TLS 1.2 only * DistinguishedName certificate_authorities<0..2^16-1>; * } CertificateRequest; + * + * Since we only support a single certificate on clients, let's just + * ignore all the information that's supposed to help us pick a + * certificate. + * + * We could check that our certificate matches the request, and bail out + * if it doesn't, but it's simpler to just send the certificate anyway, + * and give the server the opportunity to decide if it should terminate + * the connection when it doesn't like our certificate. + * + * Same goes for the hash in TLS 1.2's signature_algorithms: at this + * point we only have one hash available (see comments in + * write_certificate_verify), so let's jsut use what we have. + * + * However, we still minimally parse the message to check it is at least + * superficially sane. */ buf = ssl->in_msg; - // Retrieve cert types - // + /* certificate_types */ cert_type_len = buf[mbedtls_ssl_hs_hdr_len( ssl )]; n = cert_type_len; @@ -2602,45 +2617,14 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE_REQUEST ); } - p = buf + mbedtls_ssl_hs_hdr_len( ssl ) + 1; - while( cert_type_len > 0 ) - { -#if defined(MBEDTLS_RSA_C) - if( *p == MBEDTLS_SSL_CERT_TYPE_RSA_SIGN && - mbedtls_pk_can_do( mbedtls_ssl_own_key( ssl ), MBEDTLS_PK_RSA ) ) - { - ssl->handshake->cert_type = MBEDTLS_SSL_CERT_TYPE_RSA_SIGN; - break; - } - else -#endif -#if defined(MBEDTLS_ECDSA_C) - if( *p == MBEDTLS_SSL_CERT_TYPE_ECDSA_SIGN && - mbedtls_pk_can_do( mbedtls_ssl_own_key( ssl ), MBEDTLS_PK_ECDSA ) ) - { - ssl->handshake->cert_type = MBEDTLS_SSL_CERT_TYPE_ECDSA_SIGN; - break; - } - else -#endif - { - ; /* Unsupported cert type, ignore */ - } - - cert_type_len--; - p++; - } - + /* supported_signature_algorithms */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) { - /* Ignored, see comments about hash in write_certificate_verify */ - // TODO: should check the signature part against our pk_key though size_t sig_alg_len = ( ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 1 + n] << 8 ) | ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n] ) ); - m += 2; - n += sig_alg_len; + n += 2 + sig_alg_len; if( ssl->in_hslen < mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n ) { @@ -2650,13 +2634,12 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ - /* Ignore certificate_authorities, we only have one cert anyway */ - // TODO: should not send cert if no CA matches - dn_len = ( ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 1 + m + n] << 8 ) - | ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 2 + m + n] ) ); + /* certificate_authorities */ + dn_len = ( ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 1 + n] << 8 ) + | ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n] ) ); n += dn_len; - if( ssl->in_hslen != mbedtls_ssl_hs_hdr_len( ssl ) + 3 + m + n ) + if( ssl->in_hslen != mbedtls_ssl_hs_hdr_len( ssl ) + 3 + n ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); return( MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE_REQUEST ); From a0e924fa7bb8ea4ce480ceb32e8b715a3443bf25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 24 Feb 2016 14:36:05 +0000 Subject: [PATCH 11/32] x509: - --- include/mbedtls/x509_csr.h | 6 ++++++ library/x509_csr.c | 8 +++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/x509_csr.h b/include/mbedtls/x509_csr.h index 34998a3a5..7a9c2e055 100644 --- a/include/mbedtls/x509_csr.h +++ b/include/mbedtls/x509_csr.h @@ -83,6 +83,8 @@ mbedtls_x509write_csr; /** * \brief Load a Certificate Signing Request (CSR) in DER format * + * \note CSR attributes (if any) are currently silently ignored. + * * \param csr CSR context to fill * \param buf buffer holding the CRL data * \param buflen size of the buffer @@ -95,6 +97,8 @@ int mbedtls_x509_csr_parse_der( mbedtls_x509_csr *csr, /** * \brief Load a Certificate Signing Request (CSR), DER or PEM format * + * \note See notes for \c mbedtls_x509_csr_parse_der() + * * \param csr CSR context to fill * \param buf buffer holding the CRL data * \param buflen size of the buffer @@ -108,6 +112,8 @@ int mbedtls_x509_csr_parse( mbedtls_x509_csr *csr, const unsigned char *buf, siz /** * \brief Load a Certificate Signing Request (CSR) * + * \note See notes for \c mbedtls_x509_csr_parse() + * * \param csr CSR context to fill * \param path filename to read the CSR from * diff --git a/library/x509_csr.c b/library/x509_csr.c index dbf659b44..f8c45f8d2 100644 --- a/library/x509_csr.c +++ b/library/x509_csr.c @@ -207,6 +207,13 @@ int mbedtls_x509_csr_parse_der( mbedtls_x509_csr *csr, /* * attributes [0] Attributes + * + * The list of possible attributes is open-ended, though RFC 2985 + * (PKCS#9) defines a few in section 5.4. We currently don't support any, + * so we just ignore them. This is a safe thing to do as the worst thing + * that could happen is that we issue a certificate that does not match + * the requester's expectations - this cannot cause a violation of our + * signature policies. */ if( ( ret = mbedtls_asn1_get_tag( &p, end, &len, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_CONTEXT_SPECIFIC ) ) != 0 ) @@ -214,7 +221,6 @@ int mbedtls_x509_csr_parse_der( mbedtls_x509_csr *csr, mbedtls_x509_csr_free( csr ); return( MBEDTLS_ERR_X509_INVALID_FORMAT + ret ); } - // TODO Parse Attributes / extension requests p += len; From 347700ebe2893711cbf717cef883a42afdaf2b1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 24 Feb 2016 17:11:40 +0000 Subject: [PATCH 12/32] x509: remove obsolete TODO comment - basicContraints checks are done during verification - there is no need to set extensions that are not present to default values, as the code using the extension will check if it was present using ext_types. (And default values would not make sense anyway.) --- library/x509_crt.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 0606eb96d..3eaf5bc14 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -516,9 +516,6 @@ static int x509_get_subject_alt_name( unsigned char **p, /* * X.509 v3 extensions * - * TODO: Perform all of the basic constraints tests required by the RFC - * TODO: Set values for undetected extensions to a sane default? - * */ static int x509_get_crt_ext( unsigned char **p, const unsigned char *end, From a766576a7409722453df112d66770f177eb2ae60 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Tue, 1 Mar 2016 13:16:57 +0000 Subject: [PATCH 13/32] Fix some minor typos in comments Fix spelling mistakes and typos. --- include/mbedtls/ssl.h | 12 +++++++----- library/ssl_cli.c | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 3a8b73362..b89d4ed0d 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -995,7 +995,8 @@ typedef int mbedtls_ssl_send_t( void *ctx, * * \note That callback may be either blocking or non-blocking. * - * \param ctx Context for the send callback (typically a file descriptor) + * \param ctx Context for the receive callback (typically a file + * descriptor) * \param buf Buffer to write the received data to * \param len Length of the receive buffer * @@ -1019,7 +1020,7 @@ typedef int mbedtls_ssl_recv_t( void *ctx, * timeout delay expires, or the operation is interrupted by a * signal. * - * \param ctx Context for the send callback (typically a file descriptor) + * \param ctx Context for the receive callback (typically a file descriptor) * \param buf Buffer to write the received data to * \param len Length of the receive buffer * \param timeout Maximum nomber of millisecondes to wait for data @@ -1619,9 +1620,10 @@ int mbedtls_ssl_conf_own_cert( mbedtls_ssl_config *conf, * want to use \c mbedtls_ssl_conf_psk_cb() instead. * * \note Currently clients can only register one pre-shared key. - * In other words, the servers' idendity hint is ignored. - * Please contact us if you need ability to set multiple PSKs - * on clients and select one based on the identity hint. + * In other words, the servers' identity hint is ignored. + * Support for setting multiple PSKs on clients and selecting + * one based on the identity hint is not a planned feature but + * feedback is welcomed. * * \param conf SSL configuration * \param psk pointer to the pre-shared key diff --git a/library/ssl_cli.c b/library/ssl_cli.c index bf6c22101..52ddf9a92 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2600,7 +2600,7 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) * * Same goes for the hash in TLS 1.2's signature_algorithms: at this * point we only have one hash available (see comments in - * write_certificate_verify), so let's jsut use what we have. + * write_certificate_verify), so let's just use what we have. * * However, we still minimally parse the message to check it is at least * superficially sane. From 1b6044ded24e967f7901c646c0f5a754e2cd086b Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Tue, 1 Mar 2016 17:31:49 +0000 Subject: [PATCH 14/32] Use the SSL IO and time callback typedefs consistently The callback typedefs defined for mbedtls_ssl_set_bio() and mbedtls_ssl_set_timer_cb() were not used consistently where the callbacks were referenced in structures or in code. --- include/mbedtls/ssl.h | 236 +++++++++++++++++++++--------------------- library/ssl_tls.c | 10 +- 2 files changed, 123 insertions(+), 123 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index b89d4ed0d..9bd105149 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -411,6 +411,116 @@ typedef enum } mbedtls_ssl_states; +/** + * \brief Callback type: send data on the network. + * + * \note That callback may be either blocking or non-blocking. + * + * \param ctx Context for the send callback (typically a file descriptor) + * \param buf Buffer holding the date to send + * \param len Length of the data to send + * + * \return The callback must return the number of bytes sent if any, + * or a non-zero error code. + * If performing non-blocking I/O, \c MBEDTLS_ERR_SSL_WANT_WRITE + * must be returned when the operation would block. + * + * \note The callback is allowed to send less bytes than requested. + * It must always return the number of bytes actually sent. + */ +typedef int mbedtls_ssl_send_t( void *ctx, + const unsigned char *buf, + size_t len ); + +/** + * \brief Callback type: receive data from the network. + * + * \note That callback may be either blocking or non-blocking. + * + * \param ctx Context for the receive callback (typically a file + * descriptor) + * \param buf Buffer to write the received data to + * \param len Length of the receive buffer + * + * \return The callback must return the number of bytes received, + * or a non-zero error code. + * If performing non-blocking I/O, \c MBEDTLS_ERR_SSL_WANT_READ + * must be returned when the operation would block. + * + * \note The callback may receive less bytes than the length of the + * buffer. It must always return the number of bytes actually + * received and written to the buffer. + */ +typedef int mbedtls_ssl_recv_t( void *ctx, + unsigned char *buf, + size_t len ); + +/** + * \brief Callback type: receive data from the network, with timeout + * + * \note That callback must block until data is received, or the + * timeout delay expires, or the operation is interrupted by a + * signal. + * + * \param ctx Context for the receive callback (typically a file descriptor) + * \param buf Buffer to write the received data to + * \param len Length of the receive buffer + * \param timeout Maximum nomber of millisecondes to wait for data + * 0 means no timeout (potentially wait forever) + * + * \return The callback must return the number of bytes received, + * or a non-zero error code: + * \c MBEDTLS_ERR_SSL_TIMEOUT if the operation timed out, + * \c MBEDTLS_ERR_SSL_WANT_READ if interrupted by a signal. + * + * \note The callback may receive less bytes than the length of the + * buffer. It must always return the number of bytes actually + * received and written to the buffer. + */ +typedef int mbedtls_ssl_recv_timeout_t( void *ctx, + unsigned char *buf, + size_t len, + uint32_t timeout ); +/** + * \brief Callback type: set a pair of timers/delays to watch + * + * \param ctx Context pointer + * \param int_ms Intermediate delay in milliseconds + * \param fin_ms Final delay in milliseconds + * 0 cancels the current timer. + * + * \note This callback must at least store the necessary information + * for the associated \c mbedtls_ssl_get_timer_t callback to + * return correct information. + * + * \note If using a event-driven style of programming, an event must + * be generated when the final delay is passed. The event must + * cause a call to \c mbedtls_ssl_handshake() with the proper + * SSL context to be scheduled. Care must be taken to ensure + * that at most one such call happens at a time. + * + * \note Only one timer at a time must be running. Calling this + * function while a timer is running must cancel it. Cancelled + * timers must not generate any event. + */ +typedef void mbedtls_ssl_set_timer_t( void * ctx, + uint32_t int_ms, + uint32_t fin_ms ); + +/** + * \brief Callback type: get status of timers/delays + * + * \param ctx Context pointer + * + * \return This callback must return: + * -1 if cancelled (fin_ms == 0), + * 0 if none of the delays is passed, + * 1 if only the intermediate delay is passed, + * 2 if the final delay is passed. + */ +typedef int mbedtls_ssl_get_timer_t( void * ctx ); + + /* Defined below */ typedef struct mbedtls_ssl_session mbedtls_ssl_session; typedef struct mbedtls_ssl_context mbedtls_ssl_context; @@ -662,12 +772,11 @@ struct mbedtls_ssl_context unsigned badmac_seen; /*!< records with a bad MAC received */ #endif - /* - * Callbacks - */ - int (*f_send)(void *, const unsigned char *, size_t); - int (*f_recv)(void *, unsigned char *, size_t); - int (*f_recv_timeout)(void *, unsigned char *, size_t, uint32_t); + mbedtls_ssl_send_t *f_send; /*!< Callback for network send */ + mbedtls_ssl_recv_t *f_recv; /*!< Callback for network receive */ + mbedtls_ssl_recv_timeout_t *f_recv_timeout; + /*!< Callback for network receive with timeout */ + void *p_bio; /*!< context for I/O operations */ /* @@ -693,8 +802,9 @@ struct mbedtls_ssl_context * Timers */ void *p_timer; /*!< context for the timer callbacks */ - void (*f_set_timer)(void *, uint32_t, uint32_t); /*!< set timer callback */ - int (*f_get_timer)(void *); /*!< get timer callback */ + + mbedtls_ssl_set_timer_t *f_set_timer; /*!< set timer callback */ + mbedtls_ssl_get_timer_t *f_get_timer; /*!< get timer callback */ /* * Record layer (incoming data) @@ -969,77 +1079,6 @@ void mbedtls_ssl_conf_dbg( mbedtls_ssl_config *conf, void (*f_dbg)(void *, int, const char *, int, const char *), void *p_dbg ); -/** - * \brief Callback type: send data on the network. - * - * \note That callback may be either blocking or non-blocking. - * - * \param ctx Context for the send callback (typically a file descriptor) - * \param buf Buffer holding the date to send - * \param len Length of the data to send - * - * \return The callback must return the number of bytes sent if any, - * or a non-zero error code. - * If performing non-blocking I/O, \c MBEDTLS_ERR_SSL_WANT_WRITE - * must be returned when the operation would block. - * - * \note The callback is allowed to send less bytes than requested. - * It must always return the number of bytes actually sent. - */ -typedef int mbedtls_ssl_send_t( void *ctx, - const unsigned char *buf, - size_t len ); - -/** - * \brief Callback type: receive data from the network. - * - * \note That callback may be either blocking or non-blocking. - * - * \param ctx Context for the receive callback (typically a file - * descriptor) - * \param buf Buffer to write the received data to - * \param len Length of the receive buffer - * - * \return The callback must return the number of bytes received, - * or a non-zero error code. - * If performing non-blocking I/O, \c MBEDTLS_ERR_SSL_WANT_READ - * must be returned when the operation would block. - * - * \note The callback may receive less bytes than the length of the - * buffer. It must always return the number of bytes actually - * received and written to the buffer. - */ -typedef int mbedtls_ssl_recv_t( void *ctx, - unsigned char *buf, - size_t len ); - -/** - * \brief Callback type: receive data from the network, with timeout - * - * \note That callback must block until data is received, or the - * timeout delay expires, or the operation is interrupted by a - * signal. - * - * \param ctx Context for the receive callback (typically a file descriptor) - * \param buf Buffer to write the received data to - * \param len Length of the receive buffer - * \param timeout Maximum nomber of millisecondes to wait for data - * 0 means no timeout (potentially wait forever) - * - * \return The callback must return the number of bytes received, - * or a non-zero error code: - * \c MBEDTLS_ERR_SSL_TIMEOUT if the operation timed out, - * \c MBEDTLS_ERR_SSL_WANT_READ if interrupted by a signal. - * - * \note The callback may receive less bytes than the length of the - * buffer. It must always return the number of bytes actually - * received and written to the buffer. - */ -typedef int mbedtls_ssl_recv_timeout_t( void *ctx, - unsigned char *buf, - size_t len, - uint32_t timeout ); - /** * \brief Set the underlying BIO callbacks for write, read and * read-with-timeout. @@ -1093,45 +1132,6 @@ void mbedtls_ssl_set_bio( mbedtls_ssl_context *ssl, */ void mbedtls_ssl_conf_read_timeout( mbedtls_ssl_config *conf, uint32_t timeout ); -/** - * \brief Callback type: set a pair of timers/delays to watch - * - * \param ctx Context pointer - * \param int_ms Intermediate delay in milliseconds - * \param fin_ms Final delay in milliseconds - * 0 cancels the current timer. - * - * \note This callback must at least store the necessary information - * for the associated \c mbedtls_ssl_get_timer_t callback to - * return correct information. - * - * \note If using a event-driven style of programming, an event must - * be generated when the final delay is passed. The event must - * cause a call to \c mbedtls_ssl_handshake() with the proper - * SSL context to be scheduled. Care must be taken to ensure - * that at most one such call happens at a time. - * - * \note Only one timer at a time must be running. Calling this - * function while a timer is running must cancel it. Cancelled - * timers must not generate any event. - */ -typedef void mbedtls_ssl_set_timer_t( void * ctx, - uint32_t int_ms, - uint32_t fin_ms ); - -/** - * \brief Callback type: get status of timers/delays - * - * \param ctx Context pointer - * - * \return This callback must return: - * -1 if cancelled (fin_ms == 0), - * 0 if none of the delays is passed, - * 1 if only the intermediate delay is passed, - * 2 if the final delay is passed. - */ -typedef int mbedtls_ssl_get_timer_t( void * ctx ); - /** * \brief Set the timer callbacks (Mandatory for DTLS.) * diff --git a/library/ssl_tls.c b/library/ssl_tls.c index afbcdd99c..1c44b7ddb 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5598,9 +5598,9 @@ void mbedtls_ssl_conf_dbg( mbedtls_ssl_config *conf, void mbedtls_ssl_set_bio( mbedtls_ssl_context *ssl, void *p_bio, - int (*f_send)(void *, const unsigned char *, size_t), - int (*f_recv)(void *, unsigned char *, size_t), - int (*f_recv_timeout)(void *, unsigned char *, size_t, uint32_t) ) + mbedtls_ssl_send_t *f_send, + mbedtls_ssl_recv_t *f_recv, + mbedtls_ssl_recv_timeout_t *f_recv_timeout ) { ssl->p_bio = p_bio; ssl->f_send = f_send; @@ -5615,8 +5615,8 @@ void mbedtls_ssl_conf_read_timeout( mbedtls_ssl_config *conf, uint32_t timeout ) void mbedtls_ssl_set_timer_cb( mbedtls_ssl_context *ssl, void *p_timer, - void (*f_set_timer)(void *, uint32_t int_ms, uint32_t fin_ms), - int (*f_get_timer)(void *) ) + mbedtls_ssl_set_timer_t *f_set_timer, + mbedtls_ssl_get_timer_t *f_get_timer ) { ssl->p_timer = p_timer; ssl->f_set_timer = f_set_timer; From 6c545a87c22814ec02932b1f389d2b01eeafcd00 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Tue, 26 Jan 2016 22:13:58 +0000 Subject: [PATCH 15/32] Parameterised the test suite applications All test suites can now take an arbitrary test file. --- tests/suites/main_test.function | 143 ++++++++++++++++++-------------- 1 file changed, 83 insertions(+), 60 deletions(-) diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function index 420ee7697..61c7337a6 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -240,10 +240,13 @@ static int run_test_snprintf( void ) test_snprintf( 5, "123", 3 ) != 0 ); } -int main() +int main(int argc, const char *argv[]) { - int ret, i, cnt, total_errors = 0, total_tests = 0, total_skipped = 0; - const char *filename = "TEST_FILENAME"; + int testfile_index, testfile_count, ret, i, cnt; + int total_errors = 0, total_tests = 0, total_skipped = 0; + const char *default_filename = "TEST_FILENAME"; + const char *test_filename = NULL; + const char **test_files = NULL; FILE *file; char buf[5000]; char *params[50]; @@ -276,78 +279,98 @@ int main() return( 0 ); } - file = fopen( filename, "r" ); - if( file == NULL ) + if ( argc <= 1 ) { - mbedtls_fprintf( stderr, "Failed to open\n" ); - return( 1 ); + test_files = &default_filename; + testfile_count = 1; + } + else + { + test_files = &argv[1]; + testfile_count = argc - 1; } - while( !feof( file ) ) + for ( testfile_index = 0; + testfile_index < testfile_count; + testfile_index++ ) { - int skip = 0; + test_filename = test_files[ testfile_index ]; - if( ( ret = get_line( file, buf, sizeof(buf) ) ) != 0 ) - break; - mbedtls_fprintf( stdout, "%s%.66s", test_errors ? "\n" : "", buf ); - mbedtls_fprintf( stdout, " " ); - for( i = strlen( buf ) + 1; i < 67; i++ ) - mbedtls_fprintf( stdout, "." ); - mbedtls_fprintf( stdout, " " ); - fflush( stdout ); - - total_tests++; - - if( ( ret = get_line( file, buf, sizeof(buf) ) ) != 0 ) - break; - cnt = parse_arguments( buf, strlen(buf), params ); - - if( strcmp( params[0], "depends_on" ) == 0 ) + file = fopen( test_filename, "r" ); + if( file == NULL ) { - for( i = 1; i < cnt; i++ ) - if( dep_check( params[i] ) != 0 ) - skip = 1; + mbedtls_fprintf( stderr, "Failed to open test file: %s\n", + test_filename ); + return( 1 ); + } + + while( !feof( file ) ) + { + int skip = 0; + + if( ( ret = get_line( file, buf, sizeof(buf) ) ) != 0 ) + break; + mbedtls_fprintf( stdout, "%s%.66s", test_errors ? "\n" : "", buf ); + mbedtls_fprintf( stdout, " " ); + for( i = strlen( buf ) + 1; i < 67; i++ ) + mbedtls_fprintf( stdout, "." ); + mbedtls_fprintf( stdout, " " ); + fflush( stdout ); + + total_tests++; if( ( ret = get_line( file, buf, sizeof(buf) ) ) != 0 ) break; cnt = parse_arguments( buf, strlen(buf), params ); - } - if( skip == 0 ) - { - test_errors = 0; - ret = dispatch_test( cnt, params ); - } + if( strcmp( params[0], "depends_on" ) == 0 ) + { + for( i = 1; i < cnt; i++ ) + if( dep_check( params[i] ) != 0 ) + skip = 1; - if( skip == 1 || ret == 3 ) - { - total_skipped++; - mbedtls_fprintf( stdout, "----\n" ); - fflush( stdout ); - } - else if( ret == 0 && test_errors == 0 ) - { - mbedtls_fprintf( stdout, "PASS\n" ); - fflush( stdout ); - } - else if( ret == 2 ) - { - mbedtls_fprintf( stderr, "FAILED: FATAL PARSE ERROR\n" ); - fclose(file); - mbedtls_exit( 2 ); - } - else - total_errors++; + if( ( ret = get_line( file, buf, sizeof(buf) ) ) != 0 ) + break; + cnt = parse_arguments( buf, strlen(buf), params ); + } - if( ( ret = get_line( file, buf, sizeof(buf) ) ) != 0 ) - break; - if( strlen(buf) != 0 ) - { - mbedtls_fprintf( stderr, "Should be empty %d\n", (int) strlen(buf) ); - return( 1 ); + if( skip == 0 ) + { + test_errors = 0; + ret = dispatch_test( cnt, params ); + } + + if( skip == 1 || ret == 3 ) + { + total_skipped++; + mbedtls_fprintf( stdout, "----\n" ); + fflush( stdout ); + } + else if( ret == 0 && test_errors == 0 ) + { + mbedtls_fprintf( stdout, "PASS\n" ); + fflush( stdout ); + } + else if( ret == 2 ) + { + mbedtls_fprintf( stderr, "FAILED: FATAL PARSE ERROR\n" ); + fclose(file); + mbedtls_exit( 2 ); + } + else + total_errors++; + + if( ( ret = get_line( file, buf, sizeof(buf) ) ) != 0 ) + break; + if( strlen(buf) != 0 ) + { + mbedtls_fprintf( stderr, "Should be empty %d\n", + (int) strlen(buf) ); + return( 1 ); + } } + fclose(file); } - fclose(file); mbedtls_fprintf( stdout, "\n----------------------------------------------------------------------------\n\n"); if( total_errors == 0 ) From 2bed20d670f50b105a614bcedfbff355eba2ad64 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Tue, 26 Jan 2016 22:15:11 +0000 Subject: [PATCH 16/32] Added script to split the test case data files Script generate-afl-tests.sh will split the test suite data files into individual test case files, suitable for fuzzing. --- tests/scripts/generate-afl-tests.sh | 68 +++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100755 tests/scripts/generate-afl-tests.sh diff --git a/tests/scripts/generate-afl-tests.sh b/tests/scripts/generate-afl-tests.sh new file mode 100755 index 000000000..cbc2f5906 --- /dev/null +++ b/tests/scripts/generate-afl-tests.sh @@ -0,0 +1,68 @@ +#!/bin/sh + +# This script splits the data test files containing the test cases into +# individual files (one test case per file) suitable for use with afl +# (American Fuzzy Lop). http://lcamtuf.coredump.cx/afl/ +# +# Usage: generate-afl-tests.sh +# - should be the path to one of the test suite files +# such as 'test_suite_mpi.data' + +# Abort on errors +set -e + +if [ -z $1 ] +then + echo " [!] No test file specified" >&2 + echo "Usage: $0 " >&2 + exit 1 +fi + +SRC_FILEPATH=$(dirname $1)/$(basename $1) +TESTSUITE=$(basename $1 .data) + +THIS_DIR=$(basename $PWD) + +if [ -d ../library -a -d ../include -a -d ../tests -a $THIS_DIR == "tests" ]; +then :; +else + echo " [!] Must be run from mbed TLS tests directory" >&2 + exit 1 +fi + +DEST_TESTCASE_DIR=$TESTSUITE-afl-tests +DEST_OUTPUT_DIR=$TESTSUITE-afl-out + +echo " [+] Creating output directories" >&2 + +if [ -e $DEST_OUTPUT_DIR/* ]; +then : + echo " [!] Test output files already exist." >&2 + exit 1 +else + mkdir -p $DEST_OUTPUT_DIR +fi + +if [ -e $DEST_TESTCASE_DIR/* ]; +then : + echo " [!] Test output files already exist." >&2 +else + mkdir -p $DEST_TESTCASE_DIR +fi + +echo " [+] Creating test cases" >&2 +cd $DEST_TESTCASE_DIR + +split -p '^\s*$' ../$SRC_FILEPATH + +for f in *; +do + # Strip out any blank lines (no trim on OS X) + sed '/^\s*$/d' $f >testcase_$f + rm $f +done + +cd .. + +echo " [+] Test cases in $DEST_TESTCASE_DIR" >&2 + From 718548d5c98c161941325d4588f3d39639be80e5 Mon Sep 17 00:00:00 2001 From: SimonB Date: Wed, 10 Feb 2016 23:50:28 +0000 Subject: [PATCH 17/32] Clarified purpose and usage of generate_code.pl Added comments to explain purpose and usage of generate_code.pl --- tests/scripts/generate_code.pl | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/scripts/generate_code.pl b/tests/scripts/generate_code.pl index 1c7a281d7..581320e2d 100755 --- a/tests/scripts/generate_code.pl +++ b/tests/scripts/generate_code.pl @@ -1,5 +1,12 @@ #!/usr/bin/env perl + +# generate_code.pl # +# Generates the test suite code given inputs of the test suite directory that +# contain the test suites, and the test suite file names for the test code and +# test data. +# +# Usage: generate_code.pl [main code file] use strict; From 6fb9db3afd113d8b4a7ca2ad94dd78539818e667 Mon Sep 17 00:00:00 2001 From: SimonB Date: Mon, 15 Feb 2016 23:27:28 +0000 Subject: [PATCH 18/32] Added support for per test suite helper functions Added to generate_code.pl: - support for per test suite helper functions - description of the structure of the files the script uses to construct the test suite file - delimiters through the source code to make the machine generated code easier to understand --- tests/scripts/generate_code.pl | 73 +++++++++++++++++++++++++++++++-- tests/suites/main_test.function | 12 ++++++ 2 files changed, 81 insertions(+), 4 deletions(-) diff --git a/tests/scripts/generate_code.pl b/tests/scripts/generate_code.pl index 581320e2d..ba61b680a 100755 --- a/tests/scripts/generate_code.pl +++ b/tests/scripts/generate_code.pl @@ -2,11 +2,47 @@ # generate_code.pl # +# Purpose +# # Generates the test suite code given inputs of the test suite directory that # contain the test suites, and the test suite file names for the test code and # test data. # # Usage: generate_code.pl [main code file] +# +# Structure of files +# +# - main code file - 'main_test.function' +# Template file that contains the main() function for the test suite, +# test dispatch code as well as support functions. It contains the +# following symbols which are substituted by this script during +# processing: +# TEST_FILENAME +# SUITE_PRE_DEP +# MAPPING_CODE +# FUNCTION CODE +# SUITE_POST_DEP +# DEP_CHECK_CODE +# DISPATCH_FUNCTION +# +# - common helper code file - 'helpers.function' +# Common helper functions +# +# - test suite code file - file name in the form 'test_suite_xxx.function' +# Code file that contains the actual test cases. The file contains a +# series of code sequences delimited by the following: +# BEGIN_HEADER / END_HEADER - list of headers files +# BEGIN_SUITE_HELPERS / END_SUITE_HELPERS - helper functions common to +# the test suite +# BEGIN_CASE / END_CASE - the test cases in the test suite. Each test +# case contains at least one function that is used to create the +# dispatch code. +# +# - test data file - file name in the form 'test_suite_xxxx.data' +# The test case parameters to to be used in execution of the test. The +# file name is used to replace the symbol 'TEST_FILENAME' in the main code +# file above. +# use strict; @@ -15,15 +51,16 @@ my $suite_name = shift or die "Missing suite name"; my $data_name = shift or die "Missing data name"; my $test_main_file = do { my $arg = shift; defined($arg) ? $arg : $suite_dir."/main_test.function" }; my $test_file = $data_name.".c"; -my $test_helper_file = $suite_dir."/helpers.function"; +my $test_common_helper_file = $suite_dir."/helpers.function"; my $test_case_file = $suite_dir."/".$suite_name.".function"; my $test_case_data = $suite_dir."/".$data_name.".data"; my $line_separator = $/; undef $/; -open(TEST_HELPERS, "$test_helper_file") or die "Opening test helpers '$test_helper_file': $!"; -my $test_helpers = ; +open(TEST_HELPERS, "$test_common_helper_file") or die "Opening test helpers +'$test_common_helper_file': $!"; +my $test_common_helpers = ; close(TEST_HELPERS); open(TEST_MAIN, "$test_main_file") or die "Opening test main '$test_main_file': $!"; @@ -40,6 +77,7 @@ close(TEST_DATA); my ( $suite_header ) = $test_cases =~ /\/\* BEGIN_HEADER \*\/\n(.*?)\n\/\* END_HEADER \*\//s; my ( $suite_defines ) = $test_cases =~ /\/\* BEGIN_DEPENDENCIES\n \* (.*?)\n \* END_DEPENDENCIES/s; +my ( $suite_helpers ) = $test_cases =~ /\/\* BEGIN_SUITE_HELPERS \*\/\n(.*?)\n\/\* END_SUITE_HELPERS \*\//s; my $requirements; if ($suite_defines =~ /^depends_on:/) @@ -67,16 +105,43 @@ $/ = $line_separator; open(TEST_FILE, ">$test_file") or die "Opening destination file '$test_file': $!"; print TEST_FILE << "END"; +/* + * *** THIS FILE HAS BEEN MACHINE GENERATED *** + * + * This file has been machine generated using the script: $0 + * + * Test file : $test_file + * + * The following files were used to create this file. + * + * Main code file : $test_main_file + * Helper file : $test_common_helper_file + * Test suite file : $test_case_file + * Test suite daya : $test_case_data + * + * + * This file is part of mbed TLS (https://tls.mbed.org) + */ + #if !defined(MBEDTLS_CONFIG_FILE) #include #else #include MBEDTLS_CONFIG_FILE #endif -$test_helpers + +/*----------------------------------------------------------------------------*/ +/* Common helper functions */ + +$test_common_helpers + + +/*----------------------------------------------------------------------------*/ +/* Test Suite Code */ $suite_pre_code $suite_header +$suite_helpers $suite_post_code END diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function index 61c7337a6..2a21441a4 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -101,9 +101,17 @@ MAPPING_CODE return( -1 ); } + +/*----------------------------------------------------------------------------*/ +/* Test Case code */ + FUNCTION_CODE SUITE_POST_DEP + +/*----------------------------------------------------------------------------*/ +/* Test dispatch code */ + int dep_check( char *str ) { if( str == NULL ) @@ -133,6 +141,10 @@ DISPATCH_FUNCTION return( ret ); } + +/*----------------------------------------------------------------------------*/ +/* Main Test code */ + int get_line( FILE *f, char *buf, size_t len ) { char *ret; From beff85aaee4cd1849432f87b8760134af6de3f62 Mon Sep 17 00:00:00 2001 From: SimonB Date: Wed, 17 Feb 2016 23:34:30 +0000 Subject: [PATCH 19/32] Refactored test suite template code Restructed test suite helper and main code to support tests suite helper functions, changed C++ comments to C-style, and made the generated source code more navigable. --- tests/scripts/generate_code.pl | 2 +- tests/suites/helpers.function | 41 ++++++++++++++++++++++++++++++++- tests/suites/main_test.function | 40 +------------------------------- 3 files changed, 42 insertions(+), 41 deletions(-) diff --git a/tests/scripts/generate_code.pl b/tests/scripts/generate_code.pl index ba61b680a..30ee6b01c 100755 --- a/tests/scripts/generate_code.pl +++ b/tests/scripts/generate_code.pl @@ -131,7 +131,7 @@ print TEST_FILE << "END"; /*----------------------------------------------------------------------------*/ -/* Common helper functions */ +/* Common helper code */ $test_common_helpers diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index 8f681dbd4..c18eed895 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -1,3 +1,6 @@ +/*----------------------------------------------------------------------------*/ +/* Headers */ + #if defined(MBEDTLS_PLATFORM_C) #include "mbedtls/platform.h" #else @@ -12,6 +15,10 @@ #define mbedtls_snprintf snprintf #endif +#if defined(MBEDTLS_MEMORY_BUFFER_ALLOC_C) +#include "mbedtls/memory_buffer_alloc.h" +#endif + #ifdef _MSC_VER #include typedef UINT32 uint32_t; @@ -23,6 +30,25 @@ typedef UINT32 uint32_t; #include #include + +/*----------------------------------------------------------------------------*/ +/* Global variables */ + +static int test_errors = 0; + + +/*----------------------------------------------------------------------------*/ +/* Macros */ + +#define TEST_ASSERT( TEST ) \ + do { \ + if( ! (TEST) ) \ + { \ + test_fail( #TEST ); \ + goto exit; \ + } \ + } while( 0 ) + #define assert(a) if( !( a ) ) \ { \ mbedtls_fprintf( stderr, "Assertion Failed at %s:%d - %s\n", \ @@ -53,11 +79,15 @@ typedef UINT32 uint32_t; } #endif + +/*----------------------------------------------------------------------------*/ +/* Helper Functions */ + static int unhexify( unsigned char *obuf, const char *ibuf ) { unsigned char c, c2; int len = strlen( ibuf ) / 2; - assert( strlen( ibuf ) % 2 == 0 ); // must be even number of bytes + assert( strlen( ibuf ) % 2 == 0 ); /* must be even number of bytes */ while( *ibuf != 0 ) { @@ -298,3 +328,12 @@ static int rnd_pseudo_rand( void *rng_state, unsigned char *output, size_t len ) return( 0 ); } + +static void test_fail( const char *test ) +{ + test_errors++; + if( test_errors == 1 ) + mbedtls_printf( "FAILED\n" ); + mbedtls_printf( " %s\n", test ); +} + diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function index 2a21441a4..7ec69b45d 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -1,44 +1,6 @@ -#include - -#if defined(MBEDTLS_PLATFORM_C) -#include "mbedtls/platform.h" -#else -#include -#include -#define mbedtls_exit exit -#define mbedtls_free free -#define mbedtls_calloc calloc -#define mbedtls_fprintf fprintf -#define mbedtls_printf printf -#define mbedtls_snprintf snprintf -#endif - -#if defined(MBEDTLS_MEMORY_BUFFER_ALLOC_C) -#include "mbedtls/memory_buffer_alloc.h" -#endif - -static int test_errors = 0; - SUITE_PRE_DEP #define TEST_SUITE_ACTIVE -static void test_fail( const char *test ) -{ - test_errors++; - if( test_errors == 1 ) - mbedtls_printf( "FAILED\n" ); - mbedtls_printf( " %s\n", test ); -} - -#define TEST_ASSERT( TEST ) \ - do { \ - if( ! (TEST) ) \ - { \ - test_fail( #TEST ); \ - goto exit; \ - } \ - } while( 0 ) - int verify_string( char **str ) { if( (*str)[0] != '"' || @@ -190,7 +152,7 @@ int parse_arguments( char *buf, size_t len, char *params[50] ) p++; } - // Replace newlines, question marks and colons in strings + /* Replace newlines, question marks and colons in strings */ for( i = 0; i < cnt; i++ ) { p = params[i]; From f18e02c22e1ead278288dee5811a1809fd2ef3ac Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Tue, 1 Mar 2016 18:35:02 +0000 Subject: [PATCH 20/32] Fix typos and add copyright statement to generate_code.pl --- tests/scripts/generate_code.pl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/scripts/generate_code.pl b/tests/scripts/generate_code.pl index 30ee6b01c..5c623f8a7 100755 --- a/tests/scripts/generate_code.pl +++ b/tests/scripts/generate_code.pl @@ -2,6 +2,8 @@ # generate_code.pl # +# Copyright (c) 2009-2016, ARM Limited, All Rights Reserved +# # Purpose # # Generates the test suite code given inputs of the test suite directory that @@ -117,7 +119,7 @@ print TEST_FILE << "END"; * Main code file : $test_main_file * Helper file : $test_common_helper_file * Test suite file : $test_case_file - * Test suite daya : $test_case_data + * Test suite data : $test_case_data * * * This file is part of mbed TLS (https://tls.mbed.org) From ede75f06c598a047cbbe29c796e59a3550367412 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 18 Feb 2016 17:28:04 +0000 Subject: [PATCH 21/32] X509: Future CA among trusted: add unit tests --- tests/data_files/test-ca2-future.crt | 13 +++++++++ .../test-ca2_cat-future-present.crt | 28 +++++++++++++++++++ .../test-ca2_cat-present-future.crt | 28 +++++++++++++++++++ tests/suites/test_suite_x509parse.data | 8 ++++++ 4 files changed, 77 insertions(+) create mode 100644 tests/data_files/test-ca2-future.crt create mode 100644 tests/data_files/test-ca2_cat-future-present.crt create mode 100644 tests/data_files/test-ca2_cat-present-future.crt diff --git a/tests/data_files/test-ca2-future.crt b/tests/data_files/test-ca2-future.crt new file mode 100644 index 000000000..d75729936 --- /dev/null +++ b/tests/data_files/test-ca2-future.crt @@ -0,0 +1,13 @@ +-----BEGIN CERTIFICATE----- +MIIB+zCCAYCgAwIBAgIBATAMBggqhkjOPQQDAgUAMD4xCzAJBgNVBAYTAk5MMREw +DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTAe +Fw0yMzA5MjIxNTQ5NDlaFw0zMDEyMzEyMzU5NTlaMD4xCzAJBgNVBAYTAk5MMREw +DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTB2 +MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBuww5XUzM5 +WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiyaY7zQa0p +w7RfdadHb9UZKVVpmlM7ILRmFmAzHqNQME4wDAYDVR0TBAUwAwEB/zAdBgNVHQ4E +FgQUnW0gJEkBPyvLeLUZvH4kydv7NnwwHwYDVR0jBBgwFoAUnW0gJEkBPyvLeLUZ +vH4kydv7NnwwDAYIKoZIzj0EAwIFAANnADBkAjB1ZNdOM7KRJiPo45hP17A1sJSH +qHFPEJbml6KdNevoVZ1HqvP8AoFGcPJRpQVtzC0CMDa7JEqn0dOss8EmW9pVF/N2 ++XvzNczj89mWMgPhJJlT+MONQx3LFQO+TMSI9hLdkw== +-----END CERTIFICATE----- diff --git a/tests/data_files/test-ca2_cat-future-present.crt b/tests/data_files/test-ca2_cat-future-present.crt new file mode 100644 index 000000000..776e725cb --- /dev/null +++ b/tests/data_files/test-ca2_cat-future-present.crt @@ -0,0 +1,28 @@ +-----BEGIN CERTIFICATE----- +MIIB+zCCAYCgAwIBAgIBATAMBggqhkjOPQQDAgUAMD4xCzAJBgNVBAYTAk5MMREw +DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTAe +Fw0yMzA5MjIxNTQ5NDlaFw0zMDEyMzEyMzU5NTlaMD4xCzAJBgNVBAYTAk5MMREw +DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTB2 +MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBuww5XUzM5 +WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiyaY7zQa0p +w7RfdadHb9UZKVVpmlM7ILRmFmAzHqNQME4wDAYDVR0TBAUwAwEB/zAdBgNVHQ4E +FgQUnW0gJEkBPyvLeLUZvH4kydv7NnwwHwYDVR0jBBgwFoAUnW0gJEkBPyvLeLUZ +vH4kydv7NnwwDAYIKoZIzj0EAwIFAANnADBkAjB1ZNdOM7KRJiPo45hP17A1sJSH +qHFPEJbml6KdNevoVZ1HqvP8AoFGcPJRpQVtzC0CMDa7JEqn0dOss8EmW9pVF/N2 ++XvzNczj89mWMgPhJJlT+MONQx3LFQO+TMSI9hLdkw== +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIICUjCCAdegAwIBAgIJAMFD4n5iQ8zoMAoGCCqGSM49BAMCMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTAeFw0xMzA5MjQxNTQ5NDhaFw0yMzA5MjIxNTQ5NDhaMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTB2MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBu +ww5XUzM5WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiy +aY7zQa0pw7RfdadHb9UZKVVpmlM7ILRmFmAzHqOBoDCBnTAdBgNVHQ4EFgQUnW0g +JEkBPyvLeLUZvH4kydv7NnwwbgYDVR0jBGcwZYAUnW0gJEkBPyvLeLUZvH4kydv7 +NnyhQqRAMD4xCzAJBgNVBAYTAk5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UE +AxMTUG9sYXJzc2wgVGVzdCBFQyBDQYIJAMFD4n5iQ8zoMAwGA1UdEwQFMAMBAf8w +CgYIKoZIzj0EAwIDaQAwZgIxAMO0YnNWKJUAfXgSJtJxexn4ipg+kv4znuR50v56 +t4d0PCu412mUC6Nnd7izvtE2MgIxAP1nnJQjZ8BWukszFQDG48wxCCyci9qpdSMv +uCjn8pwUOkABXK8Mss90fzCfCEOtIA== +-----END CERTIFICATE----- diff --git a/tests/data_files/test-ca2_cat-present-future.crt b/tests/data_files/test-ca2_cat-present-future.crt new file mode 100644 index 000000000..d62ed09cd --- /dev/null +++ b/tests/data_files/test-ca2_cat-present-future.crt @@ -0,0 +1,28 @@ +-----BEGIN CERTIFICATE----- +MIICUjCCAdegAwIBAgIJAMFD4n5iQ8zoMAoGCCqGSM49BAMCMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTAeFw0xMzA5MjQxNTQ5NDhaFw0yMzA5MjIxNTQ5NDhaMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTB2MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBu +ww5XUzM5WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiy +aY7zQa0pw7RfdadHb9UZKVVpmlM7ILRmFmAzHqOBoDCBnTAdBgNVHQ4EFgQUnW0g +JEkBPyvLeLUZvH4kydv7NnwwbgYDVR0jBGcwZYAUnW0gJEkBPyvLeLUZvH4kydv7 +NnyhQqRAMD4xCzAJBgNVBAYTAk5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UE +AxMTUG9sYXJzc2wgVGVzdCBFQyBDQYIJAMFD4n5iQ8zoMAwGA1UdEwQFMAMBAf8w +CgYIKoZIzj0EAwIDaQAwZgIxAMO0YnNWKJUAfXgSJtJxexn4ipg+kv4znuR50v56 +t4d0PCu412mUC6Nnd7izvtE2MgIxAP1nnJQjZ8BWukszFQDG48wxCCyci9qpdSMv +uCjn8pwUOkABXK8Mss90fzCfCEOtIA== +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIB+zCCAYCgAwIBAgIBATAMBggqhkjOPQQDAgUAMD4xCzAJBgNVBAYTAk5MMREw +DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTAe +Fw0yMzA5MjIxNTQ5NDlaFw0zMDEyMzEyMzU5NTlaMD4xCzAJBgNVBAYTAk5MMREw +DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTB2 +MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBuww5XUzM5 +WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiyaY7zQa0p +w7RfdadHb9UZKVVpmlM7ILRmFmAzHqNQME4wDAYDVR0TBAUwAwEB/zAdBgNVHQ4E +FgQUnW0gJEkBPyvLeLUZvH4kydv7NnwwHwYDVR0jBBgwFoAUnW0gJEkBPyvLeLUZ +vH4kydv7NnwwDAYIKoZIzj0EAwIFAANnADBkAjB1ZNdOM7KRJiPo45hP17A1sJSH +qHFPEJbml6KdNevoVZ1HqvP8AoFGcPJRpQVtzC0CMDa7JEqn0dOss8EmW9pVF/N2 ++XvzNczj89mWMgPhJJlT+MONQx3LFQO+TMSI9hLdkw== +-----END CERTIFICATE----- diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 2f2137f54..ef6ba3c88 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -699,6 +699,14 @@ X509 Certificate verification #81 (multiple CRLs, none relevant) depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA256_C:MBEDTLS_RSA_C x509_verify:"data_files/enco-cert-utf8str.pem":"data_files/enco-ca-prstr.pem":"data_files/crl_cat_rsa-ec.pem":"NULL":0:0:"NULL" +X509 Certificate verification #82 (Not yet valid CA and valid CA) +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +x509_verify:"data_files/server5.crt":"data_files/test-ca2_cat-future-present.crt":"data_files/crl-ec-sha1.pem":"NULL":0:0:"NULL" + +X509 Certificate verification #83 (valid CA and Not yet valid CA) +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +x509_verify:"data_files/server5.crt":"data_files/test-ca2_cat-present-future.crt":"data_files/crl-ec-sha1.pem":"NULL":0:0:"NULL" + X509 Certificate verification callback: trusted EE cert depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED x509_verify_callback:"data_files/server5-selfsigned.crt":"data_files/server5-selfsigned.crt":0:"depth 0 - serial 53\:A2\:CB\:4B\:12\:4E\:AD\:83\:7D\:A8\:94\:B2 - subject CN=selfsigned, OU=testing, O=PolarSSL, C=NL\n" From 884b4fc2e99bd27a1d1ee6b940732afae12f4f53 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Fri, 19 Feb 2016 15:57:17 +0000 Subject: [PATCH 22/32] X509: Future CA among trusted: add more tests --- tests/data_files/test-ca2-expired.crt | 13 +++++++++ .../data_files/test-ca2_cat-past-present.crt | 28 +++++++++++++++++++ .../data_files/test-ca2_cat-present-past.crt | 28 +++++++++++++++++++ tests/suites/test_suite_x509parse.data | 8 ++++++ 4 files changed, 77 insertions(+) create mode 100644 tests/data_files/test-ca2-expired.crt create mode 100644 tests/data_files/test-ca2_cat-past-present.crt create mode 100644 tests/data_files/test-ca2_cat-present-past.crt diff --git a/tests/data_files/test-ca2-expired.crt b/tests/data_files/test-ca2-expired.crt new file mode 100644 index 000000000..22e4797f3 --- /dev/null +++ b/tests/data_files/test-ca2-expired.crt @@ -0,0 +1,13 @@ +-----BEGIN CERTIFICATE----- +MIIB/TCCAYCgAwIBAgIBATAMBggqhkjOPQQDAgUAMD4xCzAJBgNVBAYTAk5MMREw +DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTAe +Fw0wMzA5MjQxNTQ5NDhaFw0xMzA5MjQxNTQ5NDhaMD4xCzAJBgNVBAYTAk5MMREw +DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTB2 +MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBuww5XUzM5 +WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiyaY7zQa0p +w7RfdadHb9UZKVVpmlM7ILRmFmAzHqNQME4wDAYDVR0TBAUwAwEB/zAdBgNVHQ4E +FgQUnW0gJEkBPyvLeLUZvH4kydv7NnwwHwYDVR0jBBgwFoAUnW0gJEkBPyvLeLUZ +vH4kydv7NnwwDAYIKoZIzj0EAwIFAANpADBmAjEAvQ/49lXXrLYdOIGtTaYWjpZP +tRBXQiGPMzUvmKBk7gM7bF4iFPsdJikyXHmuwv3RAjEA8vtUX8fAAB3fbh5dEXRm +l7tz0Sw/RW6AHFtaIauGkhHqeKIaKIi6WSgHu6x97uyg +-----END CERTIFICATE----- diff --git a/tests/data_files/test-ca2_cat-past-present.crt b/tests/data_files/test-ca2_cat-past-present.crt new file mode 100644 index 000000000..bc1ba9a2e --- /dev/null +++ b/tests/data_files/test-ca2_cat-past-present.crt @@ -0,0 +1,28 @@ +-----BEGIN CERTIFICATE----- +MIIB/TCCAYCgAwIBAgIBATAMBggqhkjOPQQDAgUAMD4xCzAJBgNVBAYTAk5MMREw +DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTAe +Fw0wMzA5MjQxNTQ5NDhaFw0xMzA5MjQxNTQ5NDhaMD4xCzAJBgNVBAYTAk5MMREw +DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTB2 +MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBuww5XUzM5 +WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiyaY7zQa0p +w7RfdadHb9UZKVVpmlM7ILRmFmAzHqNQME4wDAYDVR0TBAUwAwEB/zAdBgNVHQ4E +FgQUnW0gJEkBPyvLeLUZvH4kydv7NnwwHwYDVR0jBBgwFoAUnW0gJEkBPyvLeLUZ +vH4kydv7NnwwDAYIKoZIzj0EAwIFAANpADBmAjEAvQ/49lXXrLYdOIGtTaYWjpZP +tRBXQiGPMzUvmKBk7gM7bF4iFPsdJikyXHmuwv3RAjEA8vtUX8fAAB3fbh5dEXRm +l7tz0Sw/RW6AHFtaIauGkhHqeKIaKIi6WSgHu6x97uyg +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIICUjCCAdegAwIBAgIJAMFD4n5iQ8zoMAoGCCqGSM49BAMCMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTAeFw0xMzA5MjQxNTQ5NDhaFw0yMzA5MjIxNTQ5NDhaMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTB2MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBu +ww5XUzM5WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiy +aY7zQa0pw7RfdadHb9UZKVVpmlM7ILRmFmAzHqOBoDCBnTAdBgNVHQ4EFgQUnW0g +JEkBPyvLeLUZvH4kydv7NnwwbgYDVR0jBGcwZYAUnW0gJEkBPyvLeLUZvH4kydv7 +NnyhQqRAMD4xCzAJBgNVBAYTAk5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UE +AxMTUG9sYXJzc2wgVGVzdCBFQyBDQYIJAMFD4n5iQ8zoMAwGA1UdEwQFMAMBAf8w +CgYIKoZIzj0EAwIDaQAwZgIxAMO0YnNWKJUAfXgSJtJxexn4ipg+kv4znuR50v56 +t4d0PCu412mUC6Nnd7izvtE2MgIxAP1nnJQjZ8BWukszFQDG48wxCCyci9qpdSMv +uCjn8pwUOkABXK8Mss90fzCfCEOtIA== +-----END CERTIFICATE----- diff --git a/tests/data_files/test-ca2_cat-present-past.crt b/tests/data_files/test-ca2_cat-present-past.crt new file mode 100644 index 000000000..a321d5dd7 --- /dev/null +++ b/tests/data_files/test-ca2_cat-present-past.crt @@ -0,0 +1,28 @@ +-----BEGIN CERTIFICATE----- +MIICUjCCAdegAwIBAgIJAMFD4n5iQ8zoMAoGCCqGSM49BAMCMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTAeFw0xMzA5MjQxNTQ5NDhaFw0yMzA5MjIxNTQ5NDhaMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTB2MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBu +ww5XUzM5WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiy +aY7zQa0pw7RfdadHb9UZKVVpmlM7ILRmFmAzHqOBoDCBnTAdBgNVHQ4EFgQUnW0g +JEkBPyvLeLUZvH4kydv7NnwwbgYDVR0jBGcwZYAUnW0gJEkBPyvLeLUZvH4kydv7 +NnyhQqRAMD4xCzAJBgNVBAYTAk5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UE +AxMTUG9sYXJzc2wgVGVzdCBFQyBDQYIJAMFD4n5iQ8zoMAwGA1UdEwQFMAMBAf8w +CgYIKoZIzj0EAwIDaQAwZgIxAMO0YnNWKJUAfXgSJtJxexn4ipg+kv4znuR50v56 +t4d0PCu412mUC6Nnd7izvtE2MgIxAP1nnJQjZ8BWukszFQDG48wxCCyci9qpdSMv +uCjn8pwUOkABXK8Mss90fzCfCEOtIA== +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIB/TCCAYCgAwIBAgIBATAMBggqhkjOPQQDAgUAMD4xCzAJBgNVBAYTAk5MMREw +DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTAe +Fw0wMzA5MjQxNTQ5NDhaFw0xMzA5MjQxNTQ5NDhaMD4xCzAJBgNVBAYTAk5MMREw +DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTB2 +MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBuww5XUzM5 +WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiyaY7zQa0p +w7RfdadHb9UZKVVpmlM7ILRmFmAzHqNQME4wDAYDVR0TBAUwAwEB/zAdBgNVHQ4E +FgQUnW0gJEkBPyvLeLUZvH4kydv7NnwwHwYDVR0jBBgwFoAUnW0gJEkBPyvLeLUZ +vH4kydv7NnwwDAYIKoZIzj0EAwIFAANpADBmAjEAvQ/49lXXrLYdOIGtTaYWjpZP +tRBXQiGPMzUvmKBk7gM7bF4iFPsdJikyXHmuwv3RAjEA8vtUX8fAAB3fbh5dEXRm +l7tz0Sw/RW6AHFtaIauGkhHqeKIaKIi6WSgHu6x97uyg +-----END CERTIFICATE----- diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index ef6ba3c88..0008d3d2c 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -707,6 +707,14 @@ X509 Certificate verification #83 (valid CA and Not yet valid CA) depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED x509_verify:"data_files/server5.crt":"data_files/test-ca2_cat-present-future.crt":"data_files/crl-ec-sha1.pem":"NULL":0:0:"NULL" +X509 Certificate verification #84 (valid CA and Not yet valid CA) +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +x509_verify:"data_files/server5.crt":"data_files/test-ca2_cat-present-past.crt":"data_files/crl-ec-sha1.pem":"NULL":0:0:"NULL" + +X509 Certificate verification #85 (Not yet valid CA and valid CA) +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +x509_verify:"data_files/server5.crt":"data_files/test-ca2_cat-past-present.crt":"data_files/crl-ec-sha1.pem":"NULL":0:0:"NULL" + X509 Certificate verification callback: trusted EE cert depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED x509_verify_callback:"data_files/server5-selfsigned.crt":"data_files/server5-selfsigned.crt":0:"depth 0 - serial 53\:A2\:CB\:4B\:12\:4E\:AD\:83\:7D\:A8\:94\:B2 - subject CN=selfsigned, OU=testing, O=PolarSSL, C=NL\n" From c72d6425955c780f09c9347c9ebf7c0bc18e32b9 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Fri, 19 Feb 2016 15:58:21 +0000 Subject: [PATCH 23/32] X509: Fix bug triggered by future CA among trusted Fix an issue that caused valid certificates being rejected whenever an expired or not yet valid version of the trusted certificate was before the valid version in the trusted certificate list. --- ChangeLog | 3 +++ library/x509_crt.c | 16 ++++++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index e9b67908f..a1afbaae6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -11,6 +11,9 @@ Bugfix * Fix issue in Makefile that prevented building using armar. #386 * Fix memory leak that occured only when ECJPAKE was enabled and ECDHE and ECDSA was disabled in config.h . The leak didn't occur by default. + * Fix an issue that caused valid certificates being rejected whenever an + expired or not yet valid version of the trusted certificate was before the + valid version in the trusted certificate list. Changes * On ARM platforms, when compiling with -O0 with GCC, Clang or armcc5, diff --git a/library/x509_crt.c b/library/x509_crt.c index 3eaf5bc14..334b8ef51 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1932,6 +1932,16 @@ static int x509_crt_verify_top( continue; } + if( mbedtls_x509_time_is_past( &trust_ca->valid_to ) ) + { + continue; + } + + if( mbedtls_x509_time_is_future( &trust_ca->valid_from ) ) + { + continue; + } + if( mbedtls_pk_verify_ext( child->sig_pk, child->sig_opts, &trust_ca->pk, child->sig_md, hash, mbedtls_md_get_size( md_info ), child->sig.p, child->sig.len ) != 0 ) @@ -1967,12 +1977,6 @@ static int x509_crt_verify_top( ((void) ca_crl); #endif - if( mbedtls_x509_time_is_past( &trust_ca->valid_to ) ) - ca_flags |= MBEDTLS_X509_BADCERT_EXPIRED; - - if( mbedtls_x509_time_is_future( &trust_ca->valid_from ) ) - ca_flags |= MBEDTLS_X509_BADCERT_FUTURE; - if( NULL != f_vrfy ) { if( ( ret = f_vrfy( p_vrfy, trust_ca, path_cnt + 1, From a418ff8eb5d2acf65a58eeb73b1dcc4960f38416 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Tue, 1 Mar 2016 20:26:16 +0000 Subject: [PATCH 24/32] Remove redundant test certificates and clarify ChangeLog --- ChangeLog | 6 +++--- tests/data_files/test-ca2-expired.crt | 13 ------------- tests/data_files/test-ca2-future.crt | 13 ------------- 3 files changed, 3 insertions(+), 29 deletions(-) delete mode 100644 tests/data_files/test-ca2-expired.crt delete mode 100644 tests/data_files/test-ca2-future.crt diff --git a/ChangeLog b/ChangeLog index a1afbaae6..56464ceb0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -11,9 +11,9 @@ Bugfix * Fix issue in Makefile that prevented building using armar. #386 * Fix memory leak that occured only when ECJPAKE was enabled and ECDHE and ECDSA was disabled in config.h . The leak didn't occur by default. - * Fix an issue that caused valid certificates being rejected whenever an - expired or not yet valid version of the trusted certificate was before the - valid version in the trusted certificate list. + * Fix an issue that caused valid certificates to be rejected whenever an + expired or not yet valid certificate was parsed before a valid certificate + in the trusted certificate list. Changes * On ARM platforms, when compiling with -O0 with GCC, Clang or armcc5, diff --git a/tests/data_files/test-ca2-expired.crt b/tests/data_files/test-ca2-expired.crt deleted file mode 100644 index 22e4797f3..000000000 --- a/tests/data_files/test-ca2-expired.crt +++ /dev/null @@ -1,13 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIB/TCCAYCgAwIBAgIBATAMBggqhkjOPQQDAgUAMD4xCzAJBgNVBAYTAk5MMREw -DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTAe -Fw0wMzA5MjQxNTQ5NDhaFw0xMzA5MjQxNTQ5NDhaMD4xCzAJBgNVBAYTAk5MMREw -DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTB2 -MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBuww5XUzM5 -WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiyaY7zQa0p -w7RfdadHb9UZKVVpmlM7ILRmFmAzHqNQME4wDAYDVR0TBAUwAwEB/zAdBgNVHQ4E -FgQUnW0gJEkBPyvLeLUZvH4kydv7NnwwHwYDVR0jBBgwFoAUnW0gJEkBPyvLeLUZ -vH4kydv7NnwwDAYIKoZIzj0EAwIFAANpADBmAjEAvQ/49lXXrLYdOIGtTaYWjpZP -tRBXQiGPMzUvmKBk7gM7bF4iFPsdJikyXHmuwv3RAjEA8vtUX8fAAB3fbh5dEXRm -l7tz0Sw/RW6AHFtaIauGkhHqeKIaKIi6WSgHu6x97uyg ------END CERTIFICATE----- diff --git a/tests/data_files/test-ca2-future.crt b/tests/data_files/test-ca2-future.crt deleted file mode 100644 index d75729936..000000000 --- a/tests/data_files/test-ca2-future.crt +++ /dev/null @@ -1,13 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIB+zCCAYCgAwIBAgIBATAMBggqhkjOPQQDAgUAMD4xCzAJBgNVBAYTAk5MMREw -DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTAe -Fw0yMzA5MjIxNTQ5NDlaFw0zMDEyMzEyMzU5NTlaMD4xCzAJBgNVBAYTAk5MMREw -DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTB2 -MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBuww5XUzM5 -WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiyaY7zQa0p -w7RfdadHb9UZKVVpmlM7ILRmFmAzHqNQME4wDAYDVR0TBAUwAwEB/zAdBgNVHQ4E -FgQUnW0gJEkBPyvLeLUZvH4kydv7NnwwHwYDVR0jBBgwFoAUnW0gJEkBPyvLeLUZ -vH4kydv7NnwwDAYIKoZIzj0EAwIFAANnADBkAjB1ZNdOM7KRJiPo45hP17A1sJSH -qHFPEJbml6KdNevoVZ1HqvP8AoFGcPJRpQVtzC0CMDa7JEqn0dOss8EmW9pVF/N2 -+XvzNczj89mWMgPhJJlT+MONQx3LFQO+TMSI9hLdkw== ------END CERTIFICATE----- From 25f2c4c028f45ec1e44dfc6d53c2b23bb7b13625 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Wed, 2 Mar 2016 17:00:16 +0000 Subject: [PATCH 25/32] Update mbed-drivers dependency to v1.0.0 --- yotta/data/module.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yotta/data/module.json b/yotta/data/module.json index 6345f080e..0569e6246 100644 --- a/yotta/data/module.json +++ b/yotta/data/module.json @@ -13,6 +13,6 @@ "mbed": { "cmsis-core": "^1.0.0" } }, "testTargetDependencies": { - "mbed": { "mbed-drivers": "~0.11.0" } + "mbed": { "mbed-drivers": "^1.0.0" } } } From 5d23716e20d6d2b7877a440808c4d5839bfc0306 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Fri, 4 Mar 2016 22:21:52 +0000 Subject: [PATCH 26/32] Add missing dependencies to X509 Parse test suite for P-384 curve The test script curves.pl was failing on testing dependencies for the P-384 curve on the new test cases introduced by ede75f0 and 884b4fc. --- tests/suites/test_suite_x509parse.data | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 0008d3d2c..b21a64090 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -700,19 +700,19 @@ depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED: x509_verify:"data_files/enco-cert-utf8str.pem":"data_files/enco-ca-prstr.pem":"data_files/crl_cat_rsa-ec.pem":"NULL":0:0:"NULL" X509 Certificate verification #82 (Not yet valid CA and valid CA) -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED x509_verify:"data_files/server5.crt":"data_files/test-ca2_cat-future-present.crt":"data_files/crl-ec-sha1.pem":"NULL":0:0:"NULL" X509 Certificate verification #83 (valid CA and Not yet valid CA) -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED x509_verify:"data_files/server5.crt":"data_files/test-ca2_cat-present-future.crt":"data_files/crl-ec-sha1.pem":"NULL":0:0:"NULL" X509 Certificate verification #84 (valid CA and Not yet valid CA) -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED x509_verify:"data_files/server5.crt":"data_files/test-ca2_cat-present-past.crt":"data_files/crl-ec-sha1.pem":"NULL":0:0:"NULL" X509 Certificate verification #85 (Not yet valid CA and valid CA) -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED x509_verify:"data_files/server5.crt":"data_files/test-ca2_cat-past-present.crt":"data_files/crl-ec-sha1.pem":"NULL":0:0:"NULL" X509 Certificate verification callback: trusted EE cert From b3c6978c7e21823b111402194c4b85102a2785cb Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Fri, 4 Mar 2016 23:26:57 +0000 Subject: [PATCH 27/32] Add copright, and better documentation to curves.pl The purpose and use of the test script, curves.pl was not obvious without reading the source code, plus the file was missing a copyright statement. --- tests/scripts/curves.pl | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/tests/scripts/curves.pl b/tests/scripts/curves.pl index 654bc5c3e..85eb7e651 100755 --- a/tests/scripts/curves.pl +++ b/tests/scripts/curves.pl @@ -1,10 +1,25 @@ #!/usr/bin/perl -# test dependencies on individual curves in tests -# - build -# - run test suite +# curves.pl # -# Usage: tests/scripts/curves.pl +# Copyright (c) 2014-2016, ARM Limited, All Rights Reserved +# +# Purpose +# +# To test the code dependencies on individual curves in each test suite. This +# is a verification step to ensure we don't ship test suites that do not work +# for some build options. +# +# The process is: +# for each possible curve +# build the library and test suites with the curve disabled +# execute the test suites +# +# And any test suite with the wrong dependencies will fail. +# +# Usage: curves.pl +# +# This script should be executed from the root of the project directory. use warnings; use strict; From a720ced4030d8fabf47373037642617cab9dc990 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 7 Mar 2016 15:57:05 +0000 Subject: [PATCH 28/32] Update default configuration Change the default settings for SSL and modify the tests accordingly. --- CMakeLists.txt | 2 +- include/mbedtls/config.h | 2 +- tests/ssl-opt.sh | 11 +++++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 094d9069b..ffaf677c5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -100,7 +100,7 @@ if(ENABLE_TESTING) ADD_CUSTOM_TARGET(covtest COMMAND make test COMMAND programs/test/selftest - COMMAND tests/compat.sh + COMMAND tests/compat.sh -m 'tls1 tls1_1 tls1_2 dtls1 dtls1_2' COMMAND tests/ssl-opt.sh ) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index c69ba1bcb..ee1a23a51 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1058,7 +1058,7 @@ * * Comment this macro to disable support for SSL 3.0 */ -#define MBEDTLS_SSL_PROTO_SSL3 +//#define MBEDTLS_SSL_PROTO_SSL3 /** * \def MBEDTLS_SSL_PROTO_TLS1 diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index c0b6f94d6..8792b21c2 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -695,6 +695,7 @@ run_test "Encrypt then MAC: client disabled, server enabled" \ -C "using encrypt then mac" \ -S "using encrypt then mac" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Encrypt then MAC: client SSLv3, server enabled" \ "$P_SRV debug_level=3 min_version=ssl3 \ force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ @@ -707,6 +708,7 @@ run_test "Encrypt then MAC: client SSLv3, server enabled" \ -C "using encrypt then mac" \ -S "using encrypt then mac" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Encrypt then MAC: client enabled, server SSLv3" \ "$P_SRV debug_level=3 force_version=ssl3 \ force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ @@ -754,6 +756,7 @@ run_test "Extended Master Secret: client disabled, server enabled" \ -C "using extended master secret" \ -S "using extended master secret" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Extended Master Secret: client SSLv3, server enabled" \ "$P_SRV debug_level=3 min_version=ssl3" \ "$P_CLI debug_level=3 force_version=ssl3" \ @@ -765,6 +768,7 @@ run_test "Extended Master Secret: client SSLv3, server enabled" \ -C "using extended master secret" \ -S "using extended master secret" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Extended Master Secret: client enabled, server SSLv3" \ "$P_SRV debug_level=3 force_version=ssl3" \ "$P_CLI debug_level=3 min_version=ssl3" \ @@ -883,6 +887,7 @@ run_test "CBC Record splitting: TLS 1.0, splitting" \ -s "Read from client: 1 bytes read" \ -s "122 bytes read" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "CBC Record splitting: SSLv3, splitting" \ "$P_SRV min_version=ssl3" \ "$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA \ @@ -1674,6 +1679,7 @@ run_test "Authentication: client no cert, openssl server optional" \ -c "skip write certificate verify" \ -C "! mbedtls_ssl_handshake returned" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Authentication: client no cert, ssl3" \ "$P_SRV debug_level=3 auth_mode=optional force_version=ssl3" \ "$P_CLI debug_level=3 crt_file=none key_file=none min_version=ssl3" \ @@ -2593,6 +2599,7 @@ run_test "ECJPAKE: working, DTLS, nolog" \ # Tests for ciphersuites per version +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Per-version suites: SSL3" \ "$P_SRV min_version=ssl3 version_suites=TLS-RSA-WITH-3DES-EDE-CBC-SHA,TLS-RSA-WITH-AES-256-CBC-SHA,TLS-RSA-WITH-AES-128-CBC-SHA,TLS-RSA-WITH-AES-128-GCM-SHA256" \ "$P_CLI force_version=ssl3" \ @@ -2642,6 +2649,7 @@ run_test "mbedtls_ssl_get_bytes_avail: extra data" \ # Tests for small packets +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Small packet SSLv3 BlockCipher" \ "$P_SRV min_version=ssl3" \ "$P_CLI request_size=1 force_version=ssl3 \ @@ -2649,6 +2657,7 @@ run_test "Small packet SSLv3 BlockCipher" \ 0 \ -s "Read from client: 1 bytes read" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Small packet SSLv3 StreamCipher" \ "$P_SRV min_version=ssl3 arc4=1 force_ciphersuite=TLS-RSA-WITH-RC4-128-SHA" \ "$P_CLI request_size=1 force_version=ssl3 \ @@ -2783,6 +2792,7 @@ run_test "Small packet TLS 1.2 AEAD shorter tag" \ # Test for large packets +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Large packet SSLv3 BlockCipher" \ "$P_SRV min_version=ssl3" \ "$P_CLI request_size=16384 force_version=ssl3 recsplit=0 \ @@ -2790,6 +2800,7 @@ run_test "Large packet SSLv3 BlockCipher" \ 0 \ -s "Read from client: 16384 bytes read" +requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Large packet SSLv3 StreamCipher" \ "$P_SRV min_version=ssl3 arc4=1 force_ciphersuite=TLS-RSA-WITH-RC4-128-SHA" \ "$P_CLI request_size=16384 force_version=ssl3 \ From 29b215001640383592b1c5882e074675d88754b9 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Mon, 7 Mar 2016 17:35:59 +0000 Subject: [PATCH 29/32] Fix the 'all tests' script for baremetal builds Fixes the test script test/scripts/all.sh which was failing at the baremetal ARM builds due to the entropy platform check introduced in 7ff4b77. --- tests/scripts/all.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 2f716bbe5..2c63ab546 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -222,6 +222,7 @@ scripts/config.pl full scripts/config.pl unset MBEDTLS_NET_C scripts/config.pl unset MBEDTLS_TIMING_C scripts/config.pl unset MBEDTLS_FS_IO +scripts/config.pl set MBEDTLS_NO_PLATFORM_ENTROPY # following things are not in the default config scripts/config.pl unset MBEDTLS_HAVEGE_C # depends on timing.c scripts/config.pl unset MBEDTLS_THREADING_PTHREAD @@ -241,6 +242,7 @@ scripts/config.pl unset MBEDTLS_TIMING_C scripts/config.pl unset MBEDTLS_FS_IO scripts/config.pl unset MBEDTLS_HAVE_TIME scripts/config.pl unset MBEDTLS_HAVE_TIME_DATE +scripts/config.pl set MBEDTLS_NO_PLATFORM_ENTROPY # following things are not in the default config scripts/config.pl unset MBEDTLS_DEPRECATED_WARNING scripts/config.pl unset MBEDTLS_HAVEGE_C # depends on timing.c From 14ecd0439f7556d46d4d2a2659e6aa3b23a1ed7c Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Mon, 7 Mar 2016 17:39:05 +0000 Subject: [PATCH 30/32] Fix yotta builds for change in default configs The change to defaults configurations in a720ced broke the yotta build. This fix addresses that. --- yotta/data/adjust-config.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/yotta/data/adjust-config.sh b/yotta/data/adjust-config.sh index 9088fd5e3..170d3070a 100755 --- a/yotta/data/adjust-config.sh +++ b/yotta/data/adjust-config.sh @@ -68,7 +68,6 @@ conf unset MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED conf unset MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED conf unset MBEDTLS_SSL_FALLBACK_SCSV conf unset MBEDTLS_SSL_CBC_RECORD_SPLITTING -conf unset MBEDTLS_SSL_PROTO_SSL3 conf unset MBEDTLS_SSL_PROTO_TLS1 conf unset MBEDTLS_SSL_PROTO_TLS1_1 conf unset MBEDTLS_SSL_TRUNCATED_HMAC From 342671f98295424fc5b399a0e1a8ead1b207db55 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Mon, 7 Mar 2016 23:22:10 +0000 Subject: [PATCH 31/32] Update interop tests to default configuration Removed SSLv3 from the default tests in compat.sh, and adapted the test cases in all.sh to include an additional SSLv3 regression test suite. --- CMakeLists.txt | 2 +- tests/compat.sh | 2 +- tests/scripts/all.sh | 22 ++++++++++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ffaf677c5..094d9069b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -100,7 +100,7 @@ if(ENABLE_TESTING) ADD_CUSTOM_TARGET(covtest COMMAND make test COMMAND programs/test/selftest - COMMAND tests/compat.sh -m 'tls1 tls1_1 tls1_2 dtls1 dtls1_2' + COMMAND tests/compat.sh COMMAND tests/ssl-opt.sh ) diff --git a/tests/compat.sh b/tests/compat.sh index 4b43e33a5..a333a1916 100755 --- a/tests/compat.sh +++ b/tests/compat.sh @@ -45,7 +45,7 @@ else fi # default values for options -MODES="ssl3 tls1 tls1_1 tls1_2 dtls1 dtls1_2" +MODES="tls1 tls1_1 tls1_2 dtls1 dtls1_2" VERIFIES="NO YES" TYPES="ECDSA RSA PSK" FILTER="" diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 2c63ab546..467f22a93 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1,5 +1,11 @@ #!/bin/sh +# all.sh +# +# Copyright (c) 2014-2016, ARM Limited, All Rights Reserved +# +# Purpose +# # Run all available tests (mostly). # # Warning: includes various build modes, so it will mess with the current @@ -125,6 +131,22 @@ make msg "test: compat.sh (ASan build)" # ~ 6 min tests/compat.sh +msg "build: Default + SSLv3 (ASan build)" # ~ 6 min +cleanup +scripts/config.pl set MBEDTLS_SSL_PROTO_SSL3 +CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . +make + +msg "test: SSLv3 - main suites and selftest (ASan build)" # ~ 50s +make test +programs/test/selftest + +msg "build: SSLv3 - compat.sh (ASan build)" # ~ 6 min +tests/compat.sh -m 'ssl3 tls1 tls1_1 tls1_2 dtls1 dtls1_2' + +msg "build: SSLv3 - ssl-opt.sh (ASan build)" # ~ 6 min +tests/ssl-opt.sh + msg "build: cmake, full config, clang" # ~ 50s cleanup cp "$CONFIG_H" "$CONFIG_BAK" From 8b4a1bdbb044cb1693fd36cc134d8a1493b69b15 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Mon, 7 Mar 2016 23:30:50 +0000 Subject: [PATCH 32/32] Update the ChangeLog --- ChangeLog | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog b/ChangeLog index 56464ceb0..55391816c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -19,6 +19,7 @@ Changes * On ARM platforms, when compiling with -O0 with GCC, Clang or armcc5, don't use the optimized assembly for bignum multiplication. This removes the need to pass -fomit-frame-pointer to avoid a build error with -O0. + * Disabled SSLv3 in the default configuration. = mbed TLS 2.2.1 released 2016-01-05