From c740345c5b2f4aa5119bbb91951cc258053f0112 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 23 Jun 2022 03:24:12 +0000 Subject: [PATCH] Adress review comments Change Code styles Add test cases Change-Id: I022bfc66fe509fe767319c4fe5f2541ee05e96fd Signed-off-by: XiaokangQian --- library/ssl_misc.h | 2 +- library/ssl_tls.c | 57 ++++++++++++++++++++++++++-------------------- tests/ssl-opt.sh | 26 +++++++++++++-------- 3 files changed, 49 insertions(+), 36 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 20e94dc45..119826f72 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2289,7 +2289,7 @@ int mbedtls_ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, int mbedtls_ssl_write_alpn_ext( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, - size_t *olen ); + size_t *out_len ); #endif /* MBEDTLS_SSL_ALPN */ #endif /* ssl_misc.h */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 6fa169f71..faf807fc5 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8291,36 +8291,42 @@ int mbedtls_ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, const unsigned char *end ) { const unsigned char *p = buf; - size_t list_len; + size_t protocol_name_list_len; - const unsigned char *cur_alpn; - size_t cur_alpn_len; + const unsigned char *protocol_name; + size_t protocol_name_len; /* If ALPN not configured, just ignore the extension */ if( ssl->conf->alpn_list == NULL ) return( 0 ); /* - * opaque ProtocolName<1..2^8-1>; + * RFC7301, section 3.1 + * opaque ProtocolName<1..2^8-1>; * - * struct { - * ProtocolName protocol_name_list<2..2^16-1> - * } ProtocolNameList; + * struct { + * ProtocolName protocol_name_list<2..2^16-1> + * } ProtocolNameList; */ - /* Min length is 2 ( list_len ) + 1 ( name_len ) + 1 ( name ) */ + /* + * protocal_name_list_len 2 bytes + * protocal_name_len 1 bytes + * protocal_name >=1 byte + */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 4 ); - list_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + protocol_name_list_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, list_len ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, protocol_name_list_len ); /* Validate peer's list (lengths) */ - for( cur_alpn = p; cur_alpn != end; cur_alpn += cur_alpn_len ) + for( protocol_name = p; protocol_name != end; + protocol_name += protocol_name_len ) { - cur_alpn_len = *cur_alpn++; - MBEDTLS_SSL_CHK_BUF_READ_PTR( cur_alpn, end, cur_alpn_len ); - if( cur_alpn_len == 0 ) + protocol_name_len = *protocol_name++; + MBEDTLS_SSL_CHK_BUF_READ_PTR( protocol_name, end, protocol_name_len ); + if( protocol_name_len == 0 ) return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } @@ -8328,12 +8334,13 @@ int mbedtls_ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, for( const char **alpn = ssl->conf->alpn_list; *alpn != NULL; alpn++ ) { size_t const alpn_len = strlen( *alpn ); - for( cur_alpn = p; cur_alpn != end; cur_alpn += cur_alpn_len ) + for( protocol_name = p; protocol_name != end; + protocol_name += protocol_name_len ) { - cur_alpn_len = *cur_alpn++; + protocol_name_len = *protocol_name++; - if( cur_alpn_len == alpn_len && - memcmp( cur_alpn, *alpn, alpn_len ) == 0 ) + if( protocol_name_len == alpn_len && + memcmp( protocol_name, *alpn, alpn_len ) == 0 ) { ssl->alpn_chosen = *alpn; return( 0 ); @@ -8351,10 +8358,10 @@ int mbedtls_ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, int mbedtls_ssl_write_alpn_ext( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, - size_t *olen ) + size_t *out_len ) { unsigned char *p = buf; - *olen = 0; + *out_len = 0; if( ssl->alpn_chosen == NULL ) { @@ -8373,14 +8380,14 @@ int mbedtls_ssl_write_alpn_ext( mbedtls_ssl_context *ssl, */ MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_ALPN, p, 0 ); - *olen = 7 + strlen( ssl->alpn_chosen ); + *out_len = 7 + strlen( ssl->alpn_chosen ); - MBEDTLS_PUT_UINT16_BE( *olen - 4, p, 2 ); - MBEDTLS_PUT_UINT16_BE( *olen - 6, p, 4 ); - p[6] = MBEDTLS_BYTE_0( *olen - 7 ); + MBEDTLS_PUT_UINT16_BE( *out_len - 4, p, 2 ); + MBEDTLS_PUT_UINT16_BE( *out_len - 6, p, 4 ); + p[6] = MBEDTLS_BYTE_0( *out_len - 7 ); p += 7; - memcpy( p, ssl->alpn_chosen, *olen - 7 ); + memcpy( p, ssl->alpn_chosen, *out_len - 7 ); return ( 0 ); } #endif /* MBEDTLS_SSL_ALPN */ diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 07918231c..0178e7e27 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -10520,16 +10520,22 @@ run_test "TLS 1.3: server alpn - openssl" \ "$P_SRV debug_level=3 tickets=0 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 alpn=h2" \ "$O_NEXT_CLI -msg -tls1_3 -no_middlebox -alpn h2" \ 0 \ - -s "server state: MBEDTLS_SSL_HELLO_REQUEST" \ - -s "server state: MBEDTLS_SSL_SERVER_HELLO" \ - -s "server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ - -s "server state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ - -s "server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ - -s "server state: MBEDTLS_SSL_CERTIFICATE_VERIFY" \ - -s "server state: MBEDTLS_SSL_SERVER_FINISHED" \ - -s "server state: MBEDTLS_SSL_CLIENT_FINISHED" \ - -s "server state: MBEDTLS_SSL_HANDSHAKE_WRAPUP" \ - -s "<= parse client hello" \ + -s "found alpn extension" \ + -s "server side, adding alpn extension" \ + -s "Protocol is TLSv1.3" \ + -s "HTTP/1.0 200 OK" \ + -s "Application Layer Protocol is h2" + +requires_gnutls_tls1_3 +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE +requires_config_enabled MBEDTLS_DEBUG_C +requires_config_enabled MBEDTLS_SSL_CLI_C +requires_config_enabled MBEDTLS_SSL_ALPN +run_test "TLS 1.3: server alpn - gnutls" \ + "$P_SRV debug_level=3 tickets=0 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 alpn=h2" \ + "$G_NEXT_CLI localhost -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%NO_TICKETS:%DISABLE_TLS13_COMPAT_MODE -V --alpn h2" \ + 0 \ -s "found alpn extension" \ -s "server side, adding alpn extension" \ -s "Protocol is TLSv1.3" \