From cd1b87b4c5ecffad8a3759c557434ffc64d79138 Mon Sep 17 00:00:00 2001 From: Archit Gupta Date: Thu, 6 Oct 2022 14:52:42 -0700 Subject: [PATCH 1/3] Remove useless check for valid certificate If certificate check fails, SSL_Connect will fail, thus the removed code will not run. It would only runs when successful. That function is meant to check the error code when there is a certificate validation issue. SSL_get_verify_result is being used incorrectly here; it is intended to get the validation error reason when SSL_Connect fails, and that failure is due to an invalid certificate. --- platform/posix/transport/src/openssl_posix.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/platform/posix/transport/src/openssl_posix.c b/platform/posix/transport/src/openssl_posix.c index ec598aeb2a..e81d59bb37 100644 --- a/platform/posix/transport/src/openssl_posix.c +++ b/platform/posix/transport/src/openssl_posix.c @@ -241,7 +241,7 @@ static OpensslStatus_t tlsHandshake( const ServerInfo_t * pServerInfo, const OpensslCredentials_t * pOpensslCredentials ) { OpensslStatus_t returnStatus = OPENSSL_SUCCESS; - int32_t sslStatus = -1, verifyPeerCertStatus = X509_V_OK; + int32_t sslStatus = -1; /* Validate the hostname against the server's certificate. */ sslStatus = SSL_set1_host( pOpensslParams->pSsl, pServerInfo->pHostName ); @@ -282,19 +282,6 @@ static OpensslStatus_t tlsHandshake( const ServerInfo_t * pServerInfo, } } - /* Verify X509 certificate from peer. */ - if( returnStatus == OPENSSL_SUCCESS ) - { - verifyPeerCertStatus = ( int32_t ) SSL_get_verify_result( pOpensslParams->pSsl ); - - if( verifyPeerCertStatus != X509_V_OK ) - { - LogError( ( "SSL_get_verify_result failed to verify X509 " - "certificate from peer." ) ); - returnStatus = OPENSSL_HANDSHAKE_FAILED; - } - } - return returnStatus; } From dff1232d92710c907c2f8191fa9b737a2ea0cc5f Mon Sep 17 00:00:00 2001 From: Archit Gupta Date: Thu, 6 Oct 2022 16:20:19 -0700 Subject: [PATCH 2/3] Allow disabling of server cert hostname check --- platform/posix/transport/include/openssl_posix.h | 5 +++++ platform/posix/transport/src/openssl_posix.c | 13 ++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/platform/posix/transport/include/openssl_posix.h b/platform/posix/transport/include/openssl_posix.h index ea45367204..595aacbe0f 100644 --- a/platform/posix/transport/include/openssl_posix.h +++ b/platform/posix/transport/include/openssl_posix.h @@ -118,6 +118,11 @@ typedef struct OpensslCredentials */ const char * sniHostName; + /** + * @brief If non-zero, don't compare hostname to server certificate subject. + */ + uint8_t disableHostnameCheck; + /** * @brief Set the value for the TLS max fragment length (TLS MFLN) * diff --git a/platform/posix/transport/src/openssl_posix.c b/platform/posix/transport/src/openssl_posix.c index e81d59bb37..5b092d87a2 100644 --- a/platform/posix/transport/src/openssl_posix.c +++ b/platform/posix/transport/src/openssl_posix.c @@ -244,12 +244,15 @@ static OpensslStatus_t tlsHandshake( const ServerInfo_t * pServerInfo, int32_t sslStatus = -1; /* Validate the hostname against the server's certificate. */ - sslStatus = SSL_set1_host( pOpensslParams->pSsl, pServerInfo->pHostName ); - - if( sslStatus != 1 ) + if( pOpensslCredentials->disableHostnameCheck == 0U ) { - LogError( ( "SSL_set1_host failed to set the hostname to validate." ) ); - returnStatus = OPENSSL_API_ERROR; + sslStatus = SSL_set1_host( pOpensslParams->pSsl, pServerInfo->pHostName ); + + if( sslStatus != 1 ) + { + LogError( ( "SSL_set1_host failed to set the hostname to validate." ) ); + returnStatus = OPENSSL_API_ERROR; + } } /* Enable SSL peer verification. */ From 100d7a8b5e3f312e27359176cb6ad6074890cad4 Mon Sep 17 00:00:00 2001 From: Archit Gupta Date: Thu, 6 Oct 2022 16:24:16 -0700 Subject: [PATCH 3/3] Revert "Remove useless check for valid certificate" This reverts commit cd1b87b4c5ecffad8a3759c557434ffc64d79138. --- platform/posix/transport/src/openssl_posix.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/platform/posix/transport/src/openssl_posix.c b/platform/posix/transport/src/openssl_posix.c index 5b092d87a2..4ff5dbdad7 100644 --- a/platform/posix/transport/src/openssl_posix.c +++ b/platform/posix/transport/src/openssl_posix.c @@ -241,7 +241,7 @@ static OpensslStatus_t tlsHandshake( const ServerInfo_t * pServerInfo, const OpensslCredentials_t * pOpensslCredentials ) { OpensslStatus_t returnStatus = OPENSSL_SUCCESS; - int32_t sslStatus = -1; + int32_t sslStatus = -1, verifyPeerCertStatus = X509_V_OK; /* Validate the hostname against the server's certificate. */ if( pOpensslCredentials->disableHostnameCheck == 0U ) @@ -285,6 +285,19 @@ static OpensslStatus_t tlsHandshake( const ServerInfo_t * pServerInfo, } } + /* Verify X509 certificate from peer. */ + if( returnStatus == OPENSSL_SUCCESS ) + { + verifyPeerCertStatus = ( int32_t ) SSL_get_verify_result( pOpensslParams->pSsl ); + + if( verifyPeerCertStatus != X509_V_OK ) + { + LogError( ( "SSL_get_verify_result failed to verify X509 " + "certificate from peer." ) ); + returnStatus = OPENSSL_HANDSHAKE_FAILED; + } + } + return returnStatus; }