Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mbedtls_ssl_get_version() does not support TLS 1.3 #5406

Closed
mpg opened this issue Jan 10, 2022 · 2 comments · Fixed by #5426
Closed

mbedtls_ssl_get_version() does not support TLS 1.3 #5406

mpg opened this issue Jan 10, 2022 · 2 comments · Fixed by #5426
Assignees
Labels
bug size-s Estimated task size: small (~2d)

Comments

@mpg
Copy link
Contributor

mpg commented Jan 10, 2022

mbedtls_ssl_get_version() currently returns unkown when the connection is using TLS 1.3. It should return TLSv1.3 instead, and a test in ssl-opt.sh should make sure that it does. (Client side for now, and also server-side in the future. Also, when we add version negotiation, we should make sure this keeps working.)

Originally reported in #5331.

@gstrauss
Copy link
Contributor

I can submit a PR, but is a test required for this feature?

diff --git a/ChangeLog.d/mbedtls_ssl_get_version-extend.txt b/ChangeLog.d/mbedtls_ssl_get_version-extend.txt
new file mode 100644
index 000000000..32a913ed4
--- /dev/null
+++ b/ChangeLog.d/mbedtls_ssl_get_version-extend.txt
@@ -0,0 +1,2 @@
+Feature
+   * Extend mbedtls_ssl_get_version() for "TLSv1.3".
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index d868e4965..fd5b82d5c 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -4375,6 +4375,8 @@ const char *mbedtls_ssl_get_version( const mbedtls_ssl_context *ssl )
         {
             case MBEDTLS_SSL_MINOR_VERSION_3:
                 return( "DTLSv1.2" );
+            case MBEDTLS_SSL_MINOR_VERSION_4:
+                return( "DTLSv1.3" );
 
             default:
                 return( "unknown (DTLS)" );
@@ -4386,6 +4388,8 @@ const char *mbedtls_ssl_get_version( const mbedtls_ssl_context *ssl )
     {
         case MBEDTLS_SSL_MINOR_VERSION_3:
             return( "TLSv1.2" );
+        case MBEDTLS_SSL_MINOR_VERSION_4:
+            return( "TLSv1.3" );
 
         default:
             return( "unknown" );

@gilles-peskine-arm
Copy link
Contributor

A test would confirm that it works and would, in the future, confirm that it's still working.

ssl_client2 and ssl_server2 print a message with the version, so it's just a matter of adding a -c/-s check to a run_test somewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug size-s Estimated task size: small (~2d)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants