mirror of
				https://github.com/cuberite/polarssl.git
				synced 2025-11-03 20:22:59 -05:00 
			
		
		
		
	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.
This commit is contained in:
		
							parent
							
								
									56e9ae2bf2
								
							
						
					
					
						commit
						d1b7f2b8cf
					
				@ -1594,7 +1594,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 );
 | 
			
		||||
 | 
			
		||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user