From 2829bbf59ba3443c2726407b8401f54ef0704031 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 19 Sep 2019 10:45:14 +0200 Subject: [PATCH] Remove dependency from SSL on PK internals So far, with MBEDTLS_SSL_KEEP_PEER_CERTIFICATE disabled, the SSL module relied on a undocumented feature of the PK module: that you can distinguish between contexts that have been setup and context that haven't. This feature is going to go away in the case of PK_SINGLE_TYPE, as we'll soon (as in: the next commit does that) no longer be storing the (now two-valued) pk_info member. Note even with this change, we could still distinguish if the context has been set up by look if pk_ctx is NULL or not, but this is also going away in the near future (a few more commits down the road), so not a good option either. --- include/mbedtls/ssl_internal.h | 6 +++--- library/ssl_cli.c | 6 +----- library/ssl_srv.c | 11 +++-------- library/ssl_tls.c | 1 + 4 files changed, 8 insertions(+), 16 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index a98a458c4..826581a38 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -382,9 +382,9 @@ struct mbedtls_ssl_sig_hash_set_t */ struct mbedtls_ssl_handshake_params { - /* - * Handshake specific crypto variables - */ +#if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) + uint8_t got_peer_pubkey; /*!< Did we store the peer's public key from its certificate? */ +#endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ #if defined(MBEDTLS_SSL_PROTO_DTLS) unsigned char verify_cookie_len; /*!< Cli: cookie length Srv: flag for sending a cookie */ diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 79d5598f1..ebc2a6359 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2379,11 +2379,7 @@ static int ssl_rsa_encrypt_partial_pms( mbedtls_ssl_context *ssl, } #if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) - /* Because the peer CRT pubkey is embedded into the handshake - * params currently, and there's no 'is_init' functions for PK - * contexts, we need to break the abstraction and peek into - * the PK context to see if it has been initialized. */ - if( ssl->handshake->peer_pubkey.pk_info != MBEDTLS_PK_INVALID_HANDLE ) + if( ssl->handshake->got_peer_pubkey ) peer_pk = &ssl->handshake->peer_pubkey; #else /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ if( ssl->session_negotiate->peer_cert != NULL ) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index bfd659e4b..747b9f45f 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4454,15 +4454,10 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) /* Skip if we haven't received a certificate from the client. * If MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is set, this can be * inferred from the setting of mbedtls_ssl_session::peer_cert. - * If MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is not set, it can - * be inferred from whether we've held back the peer CRT's - * public key in mbedtls_ssl_handshake_params::peer_pubkey. */ + * If MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is not set, it is tracked in a + * specific variable. */ #if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) - /* Because the peer CRT pubkey is embedded into the handshake - * params currently, and there's no 'is_init' functions for PK - * contexts, we need to break the abstraction and peek into - * the PK context to see if it has been initialized. */ - if( ssl->handshake->peer_pubkey.pk_info != MBEDTLS_PK_INVALID_HANDLE ) + if( ssl->handshake->got_peer_pubkey ) peer_pk = &ssl->handshake->peer_pubkey; #else /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ if( ssl->session_negotiate->peer_cert != NULL ) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 850bcb172..981009051 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7357,6 +7357,7 @@ static int ssl_remember_peer_pubkey( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } + ssl->handshake->got_peer_pubkey = 1; return( 0 ); } #endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */