Merge remote-tracking branch 'janos/mbedtls-2.1-iotssl-1156-ecdsa-sample-and-doc-clarification' into mbedtls-2.1
* janos/mbedtls-2.1-iotssl-1156-ecdsa-sample-and-doc-clarification:
Clarify the use of ECDSA API
diff --git a/ChangeLog b/ChangeLog
index 22b8123..7c04ce0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -23,6 +23,13 @@
* Fixed issue in mutexes to failing to initialise. #667
* Fix insufficient support for signature-hash-algorithm extension,
resulting in compatibility problems with Chrome. Found by hfloyrd. #823
+ * Accept empty trusted CA chain in authentication mode
+ MBEDTLS_SSL_VERIFY_OPTIONAL.
+ Fixes #864. Found by jethrogb.
+ * Fix implementation of mbedtls_ssl_parse_certificate
+ to not annihilate fatal errors in authentication mode
+ MBEDTLS_SSL_VERIFY_OPTIONAL and to reflect bad EC curves
+ within verification result.
Changes
* Clarify ECDSA documentation and improve the sample code to avoid
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index bcefe95..22c3f99 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -4378,12 +4378,6 @@
ca_crl = ssl->conf->ca_crl;
}
- if( ca_chain == NULL )
- {
- MBEDTLS_SSL_DEBUG_MSG( 1, ( "got no CA chain" ) );
- return( MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED );
- }
-
/*
* Main check: verify certificate
*/
@@ -4412,6 +4406,8 @@
if( mbedtls_pk_can_do( pk, MBEDTLS_PK_ECKEY ) &&
mbedtls_ssl_check_curve( ssl, mbedtls_pk_ec( *pk )->grp.id ) != 0 )
{
+ ssl->session_negotiate->verify_result |= MBEDTLS_X509_BADCERT_BAD_KEY;
+
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate (EC key curve)" ) );
if( ret == 0 )
ret = MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE;
@@ -4420,8 +4416,8 @@
#endif /* MBEDTLS_ECP_C */
if( mbedtls_ssl_check_cert_usage( ssl->session_negotiate->peer_cert,
- ciphersuite_info,
- ! ssl->conf->endpoint,
+ ciphersuite_info,
+ ! ssl->conf->endpoint,
&ssl->session_negotiate->verify_result ) != 0 )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate (usage extensions)" ) );
@@ -4429,8 +4425,36 @@
ret = MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE;
}
- if( authmode == MBEDTLS_SSL_VERIFY_OPTIONAL )
+ /* mbedtls_x509_crt_verify_with_profile is supposed to report a
+ * verification failure through MBEDTLS_ERR_X509_CERT_VERIFY_FAILED,
+ * with details encoded in the verification flags. All other kinds
+ * of error codes, including those from the user provided f_vrfy
+ * functions, are treated as fatal and lead to a failure of
+ * ssl_parse_certificate even if verification was optional. */
+ if( authmode == MBEDTLS_SSL_VERIFY_OPTIONAL &&
+ ( ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED ||
+ ret == MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE ) )
+ {
ret = 0;
+ }
+
+ if( ca_chain == NULL && authmode == MBEDTLS_SSL_VERIFY_REQUIRED )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "got no CA chain" ) );
+ ret = MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED;
+ }
+
+#if defined(MBEDTLS_DEBUG_C)
+ if( ssl->session_negotiate->verify_result != 0 )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 3, ( "! Certificate verification flags %x",
+ ssl->session_negotiate->verify_result ) );
+ }
+ else
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 3, ( "Certificate verification flags clear" ) );
+ }
+#endif /* MBEDTLS_DEBUG_C */
}
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate" ) );
diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c
index 27284fc..64b8f5f 100644
--- a/programs/ssl/ssl_client2.c
+++ b/programs/ssl/ssl_client2.c
@@ -94,6 +94,7 @@
#define DFL_RECONNECT_HARD 0
#define DFL_TICKETS MBEDTLS_SSL_SESSION_TICKETS_ENABLED
#define DFL_ALPN_STRING NULL
+#define DFL_CURVES NULL
#define DFL_TRANSPORT MBEDTLS_SSL_TRANSPORT_STREAM
#define DFL_HS_TO_MIN 0
#define DFL_HS_TO_MAX 0
@@ -174,6 +175,17 @@
#define USAGE_ALPN ""
#endif /* MBEDTLS_SSL_ALPN */
+#if defined(MBEDTLS_ECP_C)
+#define USAGE_CURVES \
+ " curves=a,b,c,d default: \"default\" (library default)\n" \
+ " example: \"secp521r1,brainpoolP512r1\"\n" \
+ " - use \"none\" for empty list\n" \
+ " - see mbedtls_ecp_curve_list()\n" \
+ " for acceptable curve names\n"
+#else
+#define USAGE_CURVES ""
+#endif
+
#if defined(MBEDTLS_SSL_PROTO_DTLS)
#define USAGE_DTLS \
" dtls=%%d default: 0 (TLS)\n" \
@@ -248,6 +260,7 @@
USAGE_FALLBACK \
USAGE_EMS \
USAGE_ETM \
+ USAGE_CURVES \
USAGE_RECSPLIT \
USAGE_DHMLEN \
"\n" \
@@ -300,6 +313,7 @@
int reco_delay; /* delay in seconds before resuming session */
int reconnect_hard; /* unexpectedly reconnect from the same port */
int tickets; /* enable / disable session tickets */
+ const char *curves; /* list of supported elliptic curves */
const char *alpn_string; /* ALPN supported protocols */
int transport; /* TLS or DTLS? */
uint32_t hs_to_min; /* Initial value of DTLS handshake timer */
@@ -415,6 +429,11 @@
#if defined(MBEDTLS_SSL_ALPN)
const char *alpn_list[10];
#endif
+#if defined(MBEDTLS_ECP_C)
+ mbedtls_ecp_group_id curve_list[20];
+ const mbedtls_ecp_curve_info *curve_cur;
+#endif
+
const char *pers = "ssl_client2";
#if defined(MBEDTLS_X509_CRT_PARSE_C)
@@ -510,6 +529,7 @@
opt.reconnect_hard = DFL_RECONNECT_HARD;
opt.tickets = DFL_TICKETS;
opt.alpn_string = DFL_ALPN_STRING;
+ opt.curves = DFL_CURVES;
opt.transport = DFL_TRANSPORT;
opt.hs_to_min = DFL_HS_TO_MIN;
opt.hs_to_max = DFL_HS_TO_MAX;
@@ -664,6 +684,8 @@
default: goto usage;
}
}
+ else if( strcmp( p, "curves" ) == 0 )
+ opt.curves = q;
else if( strcmp( p, "etm" ) == 0 )
{
switch( atoi( q ) )
@@ -921,6 +943,64 @@
}
#endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */
+#if defined(MBEDTLS_ECP_C)
+ if( opt.curves != NULL )
+ {
+ p = (char *) opt.curves;
+ i = 0;
+
+ if( strcmp( p, "none" ) == 0 )
+ {
+ curve_list[0] = MBEDTLS_ECP_DP_NONE;
+ }
+ else if( strcmp( p, "default" ) != 0 )
+ {
+ /* Leave room for a final NULL in curve list */
+ while( i < (int) ( sizeof( curve_list ) / sizeof( *curve_list ) ) - 1
+ && *p != '\0' )
+ {
+ q = p;
+
+ /* Terminate the current string */
+ while( *p != ',' && *p != '\0' )
+ p++;
+ if( *p == ',' )
+ *p++ = '\0';
+
+ if( ( curve_cur = mbedtls_ecp_curve_info_from_name( q ) ) != NULL )
+ {
+ curve_list[i++] = curve_cur->grp_id;
+ }
+ else
+ {
+ mbedtls_printf( "unknown curve %s\n", q );
+ mbedtls_printf( "supported curves: " );
+ for( curve_cur = mbedtls_ecp_curve_list();
+ curve_cur->grp_id != MBEDTLS_ECP_DP_NONE;
+ curve_cur++ )
+ {
+ mbedtls_printf( "%s ", curve_cur->name );
+ }
+ mbedtls_printf( "\n" );
+ goto exit;
+ }
+ }
+
+ mbedtls_printf("Number of curves: %d\n", i );
+
+ if( i == (int) ( sizeof( curve_list ) / sizeof( *curve_list ) ) - 1
+ && *p != '\0' )
+ {
+ mbedtls_printf( "curves list too long, maximum %zu",
+ (size_t) ( sizeof( curve_list ) / sizeof( *curve_list ) - 1 ) );
+ goto exit;
+ }
+
+ curve_list[i] = MBEDTLS_ECP_DP_NONE;
+ }
+ }
+#endif /* MBEDTLS_ECP_C */
+
#if defined(MBEDTLS_SSL_ALPN)
if( opt.alpn_string != NULL )
{
@@ -1210,6 +1290,14 @@
}
#endif
+#if defined(MBEDTLS_ECP_C)
+ if( opt.curves != NULL &&
+ strcmp( opt.curves, "default" ) != 0 )
+ {
+ mbedtls_ssl_conf_curves( &conf, curve_list );
+ }
+#endif
+
#if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED)
if( ( ret = mbedtls_ssl_conf_psk( &conf, psk, psk_len,
(const unsigned char *) opt.psk_identity,
diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c
index 602da47..f2db91f 100644
--- a/programs/ssl/ssl_server2.c
+++ b/programs/ssl/ssl_server2.c
@@ -129,6 +129,7 @@
#define DFL_CACHE_TIMEOUT -1
#define DFL_SNI NULL
#define DFL_ALPN_STRING NULL
+#define DFL_CURVES NULL
#define DFL_DHM_FILE NULL
#define DFL_TRANSPORT MBEDTLS_SSL_TRANSPORT_STREAM
#define DFL_COOKIES 1
@@ -299,6 +300,17 @@
#define USAGE_RENEGO ""
#endif
+#if defined(MBEDTLS_ECP_C)
+#define USAGE_CURVES \
+ " curves=a,b,c,d default: \"default\" (library default)\n" \
+ " example: \"secp521r1,brainpoolP512r1\"\n" \
+ " - use \"none\" for empty list\n" \
+ " - see mbedtls_ecp_curve_list()\n" \
+ " for acceptable curve names\n"
+#else
+#define USAGE_CURVES ""
+#endif
+
#define USAGE \
"\n usage: ssl_server2 param=<>...\n" \
"\n acceptable parameters:\n" \
@@ -332,6 +344,7 @@
USAGE_ALPN \
USAGE_EMS \
USAGE_ETM \
+ USAGE_CURVES \
"\n" \
" arc4=%%d default: (library default: 0)\n" \
" allow_sha1=%%d default: 0\n" \
@@ -398,6 +411,7 @@
int cache_max; /* max number of session cache entries */
int cache_timeout; /* expiration delay of session cache entries */
char *sni; /* string describing sni information */
+ const char *curves; /* list of supported elliptic curves */
const char *alpn_string; /* ALPN supported protocols */
const char *dhm_file; /* the file with the DH parameters */
int extended_ms; /* allow negotiation of extended MS? */
@@ -854,6 +868,10 @@
#if defined(SNI_OPTION)
sni_entry *sni_info = NULL;
#endif
+#if defined(MBEDTLS_ECP_C)
+ mbedtls_ecp_group_id curve_list[20];
+ const mbedtls_ecp_curve_info * curve_cur;
+#endif
#if defined(MBEDTLS_SSL_ALPN)
const char *alpn_list[10];
#endif
@@ -963,6 +981,7 @@
opt.cache_timeout = DFL_CACHE_TIMEOUT;
opt.sni = DFL_SNI;
opt.alpn_string = DFL_ALPN_STRING;
+ opt.curves = DFL_CURVES;
opt.dhm_file = DFL_DHM_FILE;
opt.transport = DFL_TRANSPORT;
opt.cookies = DFL_COOKIES;
@@ -1039,6 +1058,8 @@
}
opt.force_ciphersuite[1] = 0;
}
+ else if( strcmp( p, "curves" ) == 0 )
+ opt.curves = q;
else if( strcmp( p, "version_suites" ) == 0 )
opt.version_suites = q;
else if( strcmp( p, "renegotiation" ) == 0 )
@@ -1392,6 +1413,64 @@
}
#endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */
+#if defined(MBEDTLS_ECP_C)
+ if( opt.curves != NULL )
+ {
+ p = (char *) opt.curves;
+ i = 0;
+
+ if( strcmp( p, "none" ) == 0 )
+ {
+ curve_list[0] = MBEDTLS_ECP_DP_NONE;
+ }
+ else if( strcmp( p, "default" ) != 0 )
+ {
+ /* Leave room for a final NULL in curve list */
+ while( i < (int) ( sizeof( curve_list ) / sizeof( *curve_list ) ) - 1
+ && *p != '\0' )
+ {
+ q = p;
+
+ /* Terminate the current string */
+ while( *p != ',' && *p != '\0' )
+ p++;
+ if( *p == ',' )
+ *p++ = '\0';
+
+ if( ( curve_cur = mbedtls_ecp_curve_info_from_name( q ) ) != NULL )
+ {
+ curve_list[i++] = curve_cur->grp_id;
+ }
+ else
+ {
+ mbedtls_printf( "unknown curve %s\n", q );
+ mbedtls_printf( "supported curves: " );
+ for( curve_cur = mbedtls_ecp_curve_list();
+ curve_cur->grp_id != MBEDTLS_ECP_DP_NONE;
+ curve_cur++ )
+ {
+ mbedtls_printf( "%s ", curve_cur->name );
+ }
+ mbedtls_printf( "\n" );
+ goto exit;
+ }
+ }
+
+ mbedtls_printf("Number of curves: %d\n", i );
+
+ if( i == (int) ( sizeof( curve_list ) / sizeof( *curve_list ) ) - 1
+ && *p != '\0' )
+ {
+ mbedtls_printf( "curves list too long, maximum %zu",
+ (size_t) ( sizeof( curve_list ) / sizeof( *curve_list ) - 1 ) );
+ goto exit;
+ }
+
+ curve_list[i] = MBEDTLS_ECP_DP_NONE;
+ }
+ }
+#endif /* MBEDTLS_ECP_C */
+
#if defined(MBEDTLS_SSL_ALPN)
if( opt.alpn_string != NULL )
{
@@ -1840,6 +1919,14 @@
mbedtls_ssl_conf_sni( &conf, sni_callback, sni_info );
#endif
+#if defined(MBEDTLS_ECP_C)
+ if( opt.curves != NULL &&
+ strcmp( opt.curves, "default" ) != 0 )
+ {
+ mbedtls_ssl_conf_curves( &conf, curve_list );
+ }
+#endif
+
#if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED)
if( strlen( opt.psk ) != 0 && strlen( opt.psk_identity ) != 0 )
{
diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
index 156785e..2ca1490 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -1763,6 +1763,54 @@
-C "! mbedtls_ssl_handshake returned" \
-C "X509 - Certificate verification failed"
+run_test "Authentication: server goodcert, client optional, no trusted CA" \
+ "$P_SRV" \
+ "$P_CLI debug_level=3 auth_mode=optional ca_file=none ca_path=none" \
+ 0 \
+ -c "x509_verify_cert() returned" \
+ -c "! The certificate is not correctly signed by the trusted CA" \
+ -c "! Certificate verification flags"\
+ -C "! mbedtls_ssl_handshake returned" \
+ -C "X509 - Certificate verification failed" \
+ -C "SSL - No CA Chain is set, but required to operate"
+
+run_test "Authentication: server goodcert, client required, no trusted CA" \
+ "$P_SRV" \
+ "$P_CLI debug_level=3 auth_mode=required ca_file=none ca_path=none" \
+ 1 \
+ -c "x509_verify_cert() returned" \
+ -c "! The certificate is not correctly signed by the trusted CA" \
+ -c "! Certificate verification flags"\
+ -c "! mbedtls_ssl_handshake returned" \
+ -c "SSL - No CA Chain is set, but required to operate"
+
+# The purpose of the next two tests is to test the client's behaviour when receiving a server
+# certificate with an unsupported elliptic curve. This should usually not happen because
+# the client informs the server about the supported curves - it does, though, in the
+# corner case of a static ECDH suite, because the server doesn't check the curve on that
+# occasion (to be fixed). If that bug's fixed, the test needs to be altered to use a
+# different means to have the server ignoring the client's supported curve list.
+
+requires_config_enabled MBEDTLS_ECP_C
+run_test "Authentication: server ECDH p256v1, client required, p256v1 unsupported" \
+ "$P_SRV debug_level=1 key_file=data_files/server5.key \
+ crt_file=data_files/server5.ku-ka.crt" \
+ "$P_CLI debug_level=3 auth_mode=required curves=secp521r1" \
+ 1 \
+ -c "bad certificate (EC key curve)"\
+ -c "! Certificate verification flags"\
+ -C "bad server certificate (ECDH curve)" # Expect failure at earlier verification stage
+
+requires_config_enabled MBEDTLS_ECP_C
+run_test "Authentication: server ECDH p256v1, client optional, p256v1 unsupported" \
+ "$P_SRV debug_level=1 key_file=data_files/server5.key \
+ crt_file=data_files/server5.ku-ka.crt" \
+ "$P_CLI debug_level=3 auth_mode=optional curves=secp521r1" \
+ 1 \
+ -c "bad certificate (EC key curve)"\
+ -c "! Certificate verification flags"\
+ -c "bad server certificate (ECDH curve)" # Expect failure only at ECDH params check
+
run_test "Authentication: server badcert, client none" \
"$P_SRV crt_file=data_files/server5-badsign.crt \
key_file=data_files/server5.key" \