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.
diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h
index e367c47..3a8b733 100644
--- a/include/mbedtls/ssl.h
+++ b/include/mbedtls/ssl.h
@@ -1593,7 +1593,12 @@
* 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 3af059f..d63d7d4 100644
--- a/include/mbedtls/ssl_internal.h
+++ b/include/mbedtls/ssl_internal.h
@@ -166,7 +166,6 @@
* 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 5ce7d25..bf6c221 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 )
{
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 @@
* 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 @@
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 @@
}
#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 );
