[tls13_mps] Fixes to clang 12 compilation issues, removing trailing whitespace #60
Conversation
This commit prevents early return when TLS1_3 enabled,
TLS1_2 disabled and SOME_MODES_USE_MAC enabled.
Without the commit, the compilation fails due to unreachable code, e.g.
```
/Users/oshapira/git/mbedtls/library/ssl_msg.c:1723:13: error: code will never be executed [-Werror,-Wunreachable-code]
if( mbedtls_ssl_safer_memcmp( mac_peer, mac_expect,
^~~~~~~~~~~~~~~~~~~~~~~~
/Users/oshapira/git/mbedtls/library/ssl_msg.c:1625:9: error: code will never be executed [-Werror,-Wunreachable-code]
rec->data_len -= padlen;
^~~
2 errors generated.
```
Signed-off-by: Omer Shapira <[email protected]>
|
Nice! |
Recent clang fails to accept unannotated fallthrough in a switch case,
leading to a compilation error:
/Users/oshapira/git/mbedtls/library/mps/mps.c:2173:13: note: insert 'break;' to avoid fall-through
default:
Signed-off-by: Omer Shapira <[email protected]>
Removed trailing whitespace. Signed-off-by: Omer Shapira <[email protected]>
Signed-off-by: Omer Shapira <[email protected]>
Signed-off-by: Omer Shapira <[email protected]>
Signed-off-by: Omer Shapira <[email protected]>
Signed-off-by: Omer Shapira <[email protected]>
Signed-off-by: Omer Shapira <[email protected]>
oesh
changed the title
| @@ -1613,6 +1613,9 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, | |||
| else | |||
| #endif /* MBEDTLS_SSL_PROTO_TLS1 || MBEDTLS_SSL_PROTO_TLS1_1 || \ | |||
| MBEDTLS_SSL_PROTO_TLS1_2 */ | |||
| #if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) | |||
| if( transform->minor_ver != MBEDTLS_SSL_MINOR_VERSION_4 ) | |||
There was a problem hiding this comment.
@oesh The present code-path is for CBC mode encryption, which isn't supported in TLS 1.3. It is therefore an error if we get to this point if transform->minor_ver == MBEDTLS_SSL_MINOR_VERSION_4, and it seems that the previous version of the code was right. What was the motivation for the change?
There was a problem hiding this comment.
How about guarding the entire CBC codepath with #if defined( SSL3 || TLS1 || TLS1_1 || TLS1_2 ) ... #endif? That should resolve the issue.
| @@ -1710,6 +1713,9 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, | |||
| else | |||
| #endif /* MBEDTLS_SSL_PROTO_TLS1 || MBEDTLS_SSL_PROTO_TLS1_1 || \ | |||
| MBEDTLS_SSL_PROTO_TLS1_2 */ | |||
| #if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) | |||
There was a problem hiding this comment.
Same comment as above.
There was a problem hiding this comment.
Thanks @oesh for spotting that and for opening a PR!
I believe that the issue should be fixed in a slightly different way than proposed, by guarding the entire CBC encryption/decryption logic with a "Some TLS < 1.3 version enabled" guard. Such guard is redundant in previous versions of Mbed TLS, but we'll need it here -- and likely in other places, too -- now that we're adding support for TLS 1.3.
Would you be willing to adjust the PR accordingly?
This commit prevents early return when TLS1_3 enabled,
TLS1_2 disabled and SOME_MODES_USE_MAC enabled.
Without the commit, the compilation fails due to unreachable code, e.g.
Signed-off-by: Omer Shapira [email protected]