From e85b49ad18c6045c024545097c4e3126cbf3cdd8 Mon Sep 17 00:00:00 2001 From: ActoryOu Date: Mon, 5 Dec 2022 14:28:17 +0800 Subject: [PATCH] Add error handling on Socket_t in transport layer. (#887) * Set tcpSocket to SOCKETS_INVALID_SOCKET when any error occurred in TLS_FreeRTOS_Connect. * Initialize Socket_t to NULL. --- .../include/tcp_sockets_wrapper.h | 2 +- .../freertos_plus_tcp/tcp_sockets_wrapper.c | 3 ++- .../network_transport/transport_mbedtls.c | 19 +++++++++++++-- .../transport_mbedtls_pkcs11.c | 13 +++++++++- .../network_transport/transport_plaintext.c | 4 ++++ .../network_transport/transport_wolfSSL.c | 24 ++++++++++++------- 6 files changed, 52 insertions(+), 13 deletions(-) diff --git a/FreeRTOS-Plus/Source/Application-Protocols/network_transport/tcp_sockets_wrapper/include/tcp_sockets_wrapper.h b/FreeRTOS-Plus/Source/Application-Protocols/network_transport/tcp_sockets_wrapper/include/tcp_sockets_wrapper.h index 4602b4f5287..91dc86b98db 100644 --- a/FreeRTOS-Plus/Source/Application-Protocols/network_transport/tcp_sockets_wrapper/include/tcp_sockets_wrapper.h +++ b/FreeRTOS-Plus/Source/Application-Protocols/network_transport/tcp_sockets_wrapper/include/tcp_sockets_wrapper.h @@ -25,7 +25,7 @@ */ /** - * @file sockets_wrapper.h + * @file tcp_sockets_wrapper.h * @brief TCP transport functions wrapper. */ diff --git a/FreeRTOS-Plus/Source/Application-Protocols/network_transport/tcp_sockets_wrapper/ports/freertos_plus_tcp/tcp_sockets_wrapper.c b/FreeRTOS-Plus/Source/Application-Protocols/network_transport/tcp_sockets_wrapper/ports/freertos_plus_tcp/tcp_sockets_wrapper.c index 57abcad92fe..d1113aafe6e 100644 --- a/FreeRTOS-Plus/Source/Application-Protocols/network_transport/tcp_sockets_wrapper/ports/freertos_plus_tcp/tcp_sockets_wrapper.c +++ b/FreeRTOS-Plus/Source/Application-Protocols/network_transport/tcp_sockets_wrapper/ports/freertos_plus_tcp/tcp_sockets_wrapper.c @@ -170,6 +170,7 @@ BaseType_t TCP_Sockets_Connect( Socket_t * pTcpSocket, if( tcpSocket != FREERTOS_INVALID_SOCKET ) { ( void ) FreeRTOS_closesocket( tcpSocket ); + tcpSocket = FREERTOS_INVALID_SOCKET; } } else @@ -192,7 +193,7 @@ void TCP_Sockets_Disconnect( Socket_t tcpSocket ) BaseType_t waitForShutdownLoopCount = 0; uint8_t pDummyBuffer[ 2 ]; - if( tcpSocket != FREERTOS_INVALID_SOCKET ) + if( ( tcpSocket != NULL ) && ( tcpSocket != FREERTOS_INVALID_SOCKET ) ) { /* Initiate graceful shutdown. */ ( void ) FreeRTOS_shutdown( tcpSocket, FREERTOS_SHUT_RDWR ); diff --git a/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_mbedtls.c b/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_mbedtls.c index 04ffab50be5..00ae58b3037 100644 --- a/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_mbedtls.c +++ b/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_mbedtls.c @@ -633,6 +633,7 @@ TlsTransportStatus_t TLS_FreeRTOS_Connect( NetworkContext_t * pNetworkContext, TlsTransportParams_t * pTlsTransportParams = NULL; TlsTransportStatus_t returnStatus = TLS_TRANSPORT_SUCCESS; BaseType_t socketStatus = 0; + BaseType_t isSocketConnected = pdFALSE, isTlsSetup = pdFALSE; if( ( pNetworkContext == NULL ) || ( pNetworkContext->pParams == NULL ) || @@ -660,6 +661,10 @@ TlsTransportStatus_t TLS_FreeRTOS_Connect( NetworkContext_t * pNetworkContext, if( returnStatus == TLS_TRANSPORT_SUCCESS ) { pTlsTransportParams = pNetworkContext->pParams; + + /* Initialize tcpSocket. */ + pTlsTransportParams->tcpSocket = NULL; + socketStatus = TCP_Sockets_Connect( &( pTlsTransportParams->tcpSocket ), pHostName, port, @@ -678,6 +683,8 @@ TlsTransportStatus_t TLS_FreeRTOS_Connect( NetworkContext_t * pNetworkContext, /* Initialize mbedtls. */ if( returnStatus == TLS_TRANSPORT_SUCCESS ) { + isSocketConnected = pdTRUE; + returnStatus = initMbedtls( &( pTlsTransportParams->sslContext.entropyContext ), &( pTlsTransportParams->sslContext.ctrDrgbContext ) ); } @@ -691,17 +698,25 @@ TlsTransportStatus_t TLS_FreeRTOS_Connect( NetworkContext_t * pNetworkContext, /* Perform TLS handshake. */ if( returnStatus == TLS_TRANSPORT_SUCCESS ) { + isTlsSetup = pdTRUE; + returnStatus = tlsHandshake( pNetworkContext, pNetworkCredentials ); } /* Clean up on failure. */ if( returnStatus != TLS_TRANSPORT_SUCCESS ) { - if( ( pNetworkContext != NULL ) && ( pNetworkContext->pParams != NULL ) ) + /* Free SSL context if it's setup. */ + if( isTlsSetup == pdTRUE ) { sslContextFree( &( pTlsTransportParams->sslContext ) ); + } - TCP_Sockets_Disconnect(pTlsTransportParams->tcpSocket); + /* Call Sockets_Disconnect if socket was connected. */ + if( isSocketConnected == pdTRUE ) + { + TCP_Sockets_Disconnect( pTlsTransportParams->tcpSocket ); + pTlsTransportParams->tcpSocket = NULL; } } else diff --git a/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_mbedtls_pkcs11.c b/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_mbedtls_pkcs11.c index 3e07d24ae37..765a6ed3a92 100644 --- a/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_mbedtls_pkcs11.c +++ b/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_mbedtls_pkcs11.c @@ -669,6 +669,7 @@ TlsTransportStatus_t TLS_FreeRTOS_Connect( NetworkContext_t * pNetworkContext, TlsTransportParams_t * pTlsTransportParams = NULL; TlsTransportStatus_t returnStatus = TLS_TRANSPORT_SUCCESS; BaseType_t socketStatus = 0; + BaseType_t isSocketConnected = pdFALSE; if( ( pNetworkContext == NULL ) || ( pNetworkContext->pParams == NULL ) || @@ -696,6 +697,10 @@ TlsTransportStatus_t TLS_FreeRTOS_Connect( NetworkContext_t * pNetworkContext, if( returnStatus == TLS_TRANSPORT_SUCCESS ) { pTlsTransportParams = pNetworkContext->pParams; + + /* Initialize tcpSocket. */ + pTlsTransportParams->tcpSocket = NULL; + socketStatus = TCP_Sockets_Connect( &( pTlsTransportParams->tcpSocket ), pHostName, port, @@ -714,13 +719,19 @@ TlsTransportStatus_t TLS_FreeRTOS_Connect( NetworkContext_t * pNetworkContext, /* Perform TLS handshake. */ if( returnStatus == TLS_TRANSPORT_SUCCESS ) { + isSocketConnected = pdTRUE; + returnStatus = tlsSetup( pNetworkContext, pHostName, pNetworkCredentials ); } /* Clean up on failure. */ if( returnStatus != TLS_TRANSPORT_SUCCESS ) { - TCP_Sockets_Disconnect( pTlsTransportParams->tcpSocket ); + if( isSocketConnected == pdTRUE ) + { + TCP_Sockets_Disconnect( pTlsTransportParams->tcpSocket ); + pTlsTransportParams->tcpSocket = NULL; + } } else { diff --git a/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_plaintext.c b/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_plaintext.c index 3f2c78e77b9..d4d8233ce68 100644 --- a/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_plaintext.c +++ b/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_plaintext.c @@ -71,6 +71,10 @@ PlaintextTransportStatus_t Plaintext_FreeRTOS_Connect( NetworkContext_t * pNetwo else { pPlaintextTransportParams = pNetworkContext->pParams; + + /* Initialize tcpSocket. */ + pPlaintextTransportParams->tcpSocket = NULL; + /* Establish a TCP connection with the server. */ socketStatus = TCP_Sockets_Connect( &( pPlaintextTransportParams->tcpSocket ), pHostName, diff --git a/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_wolfSSL.c b/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_wolfSSL.c index ab9fa2004ee..3d44835cae7 100644 --- a/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_wolfSSL.c +++ b/FreeRTOS-Plus/Source/Application-Protocols/network_transport/transport_wolfSSL.c @@ -143,7 +143,7 @@ static int wolfSSL_IORecvGlue( WOLFSSL * ssl, read = TCP_Sockets_Recv( xSocket, ( void * ) buf, ( size_t ) sz ); if( ( read == 0 ) || - ( read == -TCP_SOCKETS_ERRNO_EWOULDBLOCK) ) + ( read == -TCP_SOCKETS_ERRNO_EWOULDBLOCK ) ) { read = WOLFSSL_CBIO_ERR_WANT_READ; } @@ -169,7 +169,7 @@ static int wolfSSL_IOSendGlue( WOLFSSL * ssl, Socket_t xSocket = ( Socket_t ) context; BaseType_t sent = TCP_Sockets_Send( xSocket, ( void * ) buf, ( size_t ) sz ); - if( sent == -TCP_SOCKETS_ERRNO_EWOULDBLOCK) + if( sent == -TCP_SOCKETS_ERRNO_EWOULDBLOCK ) { sent = WOLFSSL_CBIO_ERR_WANT_WRITE; } @@ -380,7 +380,7 @@ TlsTransportStatus_t TLS_FreeRTOS_Connect( NetworkContext_t * pNetworkContext, { TlsTransportStatus_t returnStatus = TLS_TRANSPORT_SUCCESS; BaseType_t socketStatus = 0; - + BaseType_t isSocketConnected = pdFALSE; if( ( pNetworkContext == NULL ) || ( pHostName == NULL ) || @@ -402,11 +402,13 @@ TlsTransportStatus_t TLS_FreeRTOS_Connect( NetworkContext_t * pNetworkContext, /* Establish a TCP connection with the server. */ if( returnStatus == TLS_TRANSPORT_SUCCESS ) { + pNetworkContext->tcpSocket = NULL; + socketStatus = TCP_Sockets_Connect( &( pNetworkContext->tcpSocket ), - pHostName, - port, - receiveTimeoutMs, - sendTimeoutMs ); + pHostName, + port, + receiveTimeoutMs, + sendTimeoutMs ); if( socketStatus != 0 ) { @@ -420,6 +422,8 @@ TlsTransportStatus_t TLS_FreeRTOS_Connect( NetworkContext_t * pNetworkContext, /* Initialize tls. */ if( returnStatus == TLS_TRANSPORT_SUCCESS ) { + isSocketConnected = pdTRUE; + returnStatus = initTLS(); } @@ -432,7 +436,11 @@ TlsTransportStatus_t TLS_FreeRTOS_Connect( NetworkContext_t * pNetworkContext, /* Clean up on failure. */ if( returnStatus != TLS_TRANSPORT_SUCCESS ) { - TCP_Sockets_Disconnect( pNetworkContext->tcpSocket ); + if( isSocketConnected == pdTRUE ) + { + TCP_Sockets_Disconnect( pNetworkContext->tcpSocket ); + pNetworkContext->tcpSocket = NULL; + } } else {