From d96e52609337454ce6aae3df4eb8025f5c268229 Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Fri, 17 Jun 2016 14:40:41 +0000 Subject: [PATCH 1/3] ssl.h: tidy up the documentation comments (#505) ssl.h: Tidy up and correct documentation errors. --- include/mbedtls/ssl.h | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 96643eb46..0c1365c5d 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -232,7 +232,7 @@ * Signaling ciphersuite values (SCSV) */ #define MBEDTLS_SSL_EMPTY_RENEGOTIATION_INFO 0xFF /**< renegotiation info ext */ -#define MBEDTLS_SSL_FALLBACK_SCSV_VALUE 0x5600 /**< draft-ietf-tls-downgrade-scsv-00 */ +#define MBEDTLS_SSL_FALLBACK_SCSV_VALUE 0x5600 /**< RFC 7507 section 2 */ /* * Supported Signature and Hash algorithms (For TLS 1.2) @@ -466,7 +466,7 @@ typedef int mbedtls_ssl_recv_t( void *ctx, * \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) + * 0 means no timeout (potentially waiting forever) * * \return The callback must return the number of bytes received, * or a non-zero error code: @@ -514,9 +514,9 @@ typedef void mbedtls_ssl_set_timer_t( void * ctx, * * \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. + * 0 if none of the delays have passed, + * 1 if only the intermediate delay has passed, + * 2 if the final delay has passed. */ typedef int mbedtls_ssl_get_timer_t( void * ctx ); @@ -958,7 +958,7 @@ void mbedtls_ssl_init( mbedtls_ssl_context *ssl ); * \note No copy of the configuration context is made, it can be * shared by many mbedtls_ssl_context structures. * - * \warning Modifying the conf structure after is has been used in this + * \warning Modifying the conf structure after it has been used in this * function is unsupported! * * \param ssl SSL context @@ -1024,6 +1024,7 @@ void mbedtls_ssl_conf_transport( mbedtls_ssl_config *conf, int transport ); * * MBEDTLS_SSL_VERIFY_REQUIRED: peer *must* present a valid certificate, * handshake is aborted if verification failed. + * (default on client) * * \note On client, MBEDTLS_SSL_VERIFY_REQUIRED is the recommended mode. * With MBEDTLS_SSL_VERIFY_OPTIONAL, the user needs to call mbedtls_ssl_get_verify_result() at @@ -1161,14 +1162,14 @@ void mbedtls_ssl_set_timer_cb( mbedtls_ssl_context *ssl, * \brief Callback type: generate and write session ticket * * \note This describes what a callback implementation should do. - * This callback should generate and encrypted and + * This callback should generate an encrypted and * authenticated ticket for the session and write it to the * output buffer. Here, ticket means the opaque ticket part * of the NewSessionTicket structure of RFC 5077. * * \param p_ticket Context for the callback - * \param session SSL session to bo written in the ticket - * \param start Start of the outpur buffer + * \param session SSL session to be written in the ticket + * \param start Start of the output buffer * \param end End of the output buffer * \param tlen On exit, holds the length written * \param lifetime On exit, holds the lifetime of the ticket in seconds @@ -1419,7 +1420,7 @@ void mbedtls_ssl_conf_dtls_badmac_limit( mbedtls_ssl_config *conf, unsigned limi #if defined(MBEDTLS_SSL_PROTO_DTLS) /** - * \brief Set retransmit timeout values for the DTLS handshale. + * \brief Set retransmit timeout values for the DTLS handshake. * (DTLS only, no effect on TLS.) * * \param conf SSL configuration @@ -1517,7 +1518,7 @@ int mbedtls_ssl_set_session( mbedtls_ssl_context *ssl, const mbedtls_ssl_session /** * \brief Set the list of allowed ciphersuites and the preference * order. First in the list has the highest preference. - * (Overrides all version specific lists) + * (Overrides all version-specific lists) * * The ciphersuites array is not copied, and must remain * valid for the lifetime of the ssl_config. @@ -1897,8 +1898,8 @@ int mbedtls_ssl_set_hs_ecjpake_password( mbedtls_ssl_context *ssl, * \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. + * the lifetime of the table must be atleast as long as the + * lifetime of the SSL configuration structure. * * \return 0 on success, or MBEDTLS_ERR_SSL_BAD_INPUT_DATA. */ @@ -2012,7 +2013,7 @@ void mbedtls_ssl_conf_extended_master_secret( mbedtls_ssl_config *conf, char ems * \brief Disable or enable support for RC4 * (Default: MBEDTLS_SSL_ARC4_DISABLED) * - * \warning Use of RC4 in DTLS/TLS has been prohibited by RFC-7465 + * \warning Use of RC4 in DTLS/TLS has been prohibited by RFC 7465 * for security reasons. Use at your own risk. * * \note This function is deprecated and will likely be removed in @@ -2094,7 +2095,7 @@ void mbedtls_ssl_conf_session_tickets( mbedtls_ssl_config *conf, int use_tickets * * \warning It is recommended to always disable renegotation unless you * know you need it and you know what you're doing. In the - * past, there has been several issues associated with + * past, there have been several issues associated with * renegotiation or a poor understanding of its properties. * * \note Server-side, enabling renegotiation also makes the server @@ -2334,8 +2335,8 @@ int mbedtls_ssl_handshake( mbedtls_ssl_context *ssl ); * \brief Perform a single step of the SSL handshake * * \note The state of the context (ssl->state) will be at - * the following state after execution of this function. - * Do not call this function if state is MBEDTLS_SSL_HANDSHAKE_OVER. + * the next state after execution of this function. Do not + * call this function if state is MBEDTLS_SSL_HANDSHAKE_OVER. * * \note If this function returns something other than 0 or * MBEDTLS_ERR_SSL_WANT_READ/WRITE, then the ssl context @@ -2356,11 +2357,13 @@ int mbedtls_ssl_handshake_step( mbedtls_ssl_context *ssl ); * \brief Initiate an SSL renegotiation on the running connection. * Client: perform the renegotiation right now. * Server: request renegotiation, which will be performed - * during the next call to mbedtls_ssl_read() if honored by client. + * during the next call to mbedtls_ssl_read() if honored by + * client. * * \param ssl SSL context * - * \return 0 if successful, or any mbedtls_ssl_handshake() return value. + * \return 0 if successful, or any mbedtls_ssl_handshake() return + * value. * * \note If this function returns something other than 0 or * MBEDTLS_ERR_SSL_WANT_READ/WRITE, then the ssl context From 4ae869139a23789f07545241f362a97f04229133 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Tue, 21 Jun 2016 10:09:25 +0100 Subject: [PATCH 2/3] Adds 'get' command to scripts/config.pl to retrieve config state Adds 'get' command to indicate if the option is enabled in the given configuration file, and to returns it's value if one has been set. --- scripts/config.pl | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/scripts/config.pl b/scripts/config.pl index 84ec38ed7..04a9a7452 100755 --- a/scripts/config.pl +++ b/scripts/config.pl @@ -7,12 +7,13 @@ # Purpose # # Comments and uncomments #define lines in the given header file and optionally -# sets their value. This is to provide scripting control of what preprocessor -# symbols, and therefore what build time configuration flags are set in the -# 'config.h' file. +# sets their value or can get the value. This is to provide scripting control of +# what preprocessor symbols, and therefore what build time configuration flags +# are set in the 'config.h' file. # # Usage: config.pl [-f | --file ] [-o | --force] -# [set | unset | full | realfull] +# [set | unset | get | +# full | realfull] # # Full usage description provided below. # @@ -43,18 +44,23 @@ use strict; my $config_file = "include/mbedtls/config.h"; my $usage = < | --file ] [-o | --force] - [set | unset | full | realfull] + [set | unset | get | + full | realfull] Commands - set [ to + set [] - Uncomments or adds a #define for the to the configuration file, and optionally making it of . If the symbol isn't present in the file an error is returned. - unset - Comments out any #define present in the - configuration file. + unset - Comments out the #define for the given symbol if + present in the configuration file. + get - Finds the #define for the given symbol, returning + an exitcode of 0 if the symbol is found, and -1 if + not. The value of the symbol is output if one is + specified in the configuration file. full - Uncomments all #define's in the configuration file - excluding some reserved symbols, until the + excluding some reserved symbols, until the 'Module configuration options' section realfull - Uncomments all #define's with no exclusions @@ -122,7 +128,7 @@ while ($arg = shift) { die $usage if @ARGV; } - elsif ($action eq "unset") { + elsif ($action eq "unset" || $action eq "get") { die $usage unless @ARGV; $name = shift; @@ -195,6 +201,11 @@ for my $line (@config_lines) { $line .= "\n"; $done = 1; } + } elsif (!$done && $action eq "get") { + if ($line =~ /^\s*#define\s*$name\s*(.*)\s*\b/) { + $value = $1; + $done = 1; + } } print $config_write $line; @@ -214,6 +225,15 @@ if ($action eq "set"&& $force_option && !$done) { close $config_write; +if ($action eq "get" && $done) { + if ($value ne '') { + print $value; + } + exit 0; +} else { + exit -1; +} + if ($action eq "full" && !$done) { die "Configuration section was not found in $config_file\n"; From 1ceab6e43ae01054ba1c0c7c031e89453ee1a0d6 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Tue, 21 Jun 2016 10:14:00 +0100 Subject: [PATCH 3/3] Adds a check and warning for the null entropy option If the option MBEDTLS_TEST_NULL_ENTROPY is enabled, the cmake generated makefile will generate an error unless a UNSAFE_BUILD switch is also enabled. Equally, a similar warning will always be generated if the Makefile is built, and another warning is generated on every compilation of entropy.c. This is to ensure the user is aware of what they're doing when they enable the null entropy option. --- CMakeLists.txt | 30 ++++++++++++++++++++++++++++++ Makefile | 16 +++++++++++++++- library/entropy.c | 6 +++--- 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 094d9069b..7ae33ccb6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -6,6 +6,7 @@ option(ENABLE_ZLIB_SUPPORT "Build mbed TLS with zlib library." OFF) option(ENABLE_PROGRAMS "Build mbed TLS programs." ON) +option(UNSAFE_BUILD "Allow unsafe builds. These builds ARE NOT SECURE." OFF) # the test suites currently have compile errors with MSVC if(MSVC) @@ -14,6 +15,35 @@ else() option(ENABLE_TESTING "Build mbed TLS tests." ON) endif() +find_package(Perl) +if(PERL_FOUND) + + # If NULL Entropy is configured, display an appropriate warning + execute_process(COMMAND ${PERL_EXECUTABLE} scripts/config.pl get MBEDTLS_TEST_NULL_ENTROPY + RESULT_VARIABLE result) + if(${result} EQUAL 0) + message(WARNING "\ + ******************************************************* + **** WARNING! MBEDTLS_TEST_NULL_ENTROPY defined! + **** THIS BUILD HAS NO DEFINED ENTROPY SOURCES + **** AND IS *NOT* SUITABLE FOR PRODUCTION USE + *******************************************************") + if(NOT UNSAFE_BUILD) + message(FATAL_ERROR "\ +\n\ +Warning! You have enabled MBEDTLS_TEST_NULL_ENTROPY. \ +This option is not safe for production use and negates all security \ +It is intended for development use only. \ +\n\ +To confirm you want to build with this option, re-run cmake with the \ +option: \n\ + cmake -DUNSAFE_BUILD=ON ") + + return() + endif() + endif() +endif() + set(CMAKE_BUILD_TYPE ${CMAKE_BUILD_TYPE} CACHE STRING "Choose the type of build: None Debug Release Coverage ASan ASanDbg MemSan MemSanDbg Check CheckFull" FORCE) diff --git a/Makefile b/Makefile index 7f03115b0..128362774 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,7 @@ PREFIX=mbedtls_ .PHONY: all no_test programs lib tests install uninstall clean test check covtest lcov apidoc apidoc_clean -all: programs tests +all: programs tests post_build no_test: programs @@ -53,6 +53,20 @@ uninstall: done endif +WARNING_BORDER =*******************************************************\n +NULL_ENTROPY_WARN_L1=**** WARNING! MBEDTLS_TEST_NULL_ENTROPY defined! ****\n +NULL_ENTROPY_WARN_L2=**** THIS BUILD HAS NO DEFINED ENTROPY SOURCES ****\n +NULL_ENTROPY_WARN_L3=**** AND IS *NOT* SUITABLE FOR PRODUCTION USE ****\n + +NULL_ENTROPY_WARNING=\n$(WARNING_BORDER)$(NULL_ENTROPY_WARN_L1)$(NULL_ENTROPY_WARN_L2)$(NULL_ENTROPY_WARN_L3)$(WARNING_BORDER) + +# Post build steps +post_build: + # If NULL Entropy is configured, display an appropriate warning + -scripts/config.pl get MBEDTLS_TEST_NULL_ENTROPY && ([ $$? -eq 0 ]) && \ + echo '$(NULL_ENTROPY_WARNING)' + + clean: $(MAKE) -C library clean $(MAKE) -C programs clean diff --git a/library/entropy.c b/library/entropy.c index 282640f2d..45c894b1d 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -28,9 +28,9 @@ #if defined(MBEDTLS_ENTROPY_C) #if defined(MBEDTLS_TEST_NULL_ENTROPY) -#warning "**** WARNING! MBEDTLS_TEST_NULL_ENTROPY defined! ****" -#warning "**** THIS BUILD HAS NO DEFINED ENTROPY SOURCES ****" -#warning "**** NOT SUITABLE FOR PRODUCTION ****" +#warning "**** WARNING! MBEDTLS_TEST_NULL_ENTROPY defined! " +#warning "**** THIS BUILD HAS NO DEFINED ENTROPY SOURCES " +#warning "**** THIS BUILD IS *NOT* SUITABLE FOR PRODUCTION USE " #endif #include "mbedtls/entropy.h"