Commit 04d39d28 authored by Manuel Pégourié-Gonnard's avatar Manuel Pégourié-Gonnard Committed by Simon Butcher

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.
parent b222cd92
......@@ -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
......
......@@ -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 */
......
......@@ -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 );
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment