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

Properly check for signature_algorithms from the client in a TLS 1.3 server. #8356

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

kareem-wolfssl
Copy link
Contributor

Description

The server was checking ssl->extensions which will always have an entry for TLSX_SIGNATURE_ALGORITHMS as it is unconditionally added by TLSX_PopulateExtensions earlier in the DoTls13ClientHello function. Instead, check args->clSuites->hashSigAlgoSz which is only set if signature_algorithms is found and parsed by TLSX_Parse.

Fixes #8355

Testing

Hacked client to not send sig_algs, confirmed server disconnected as expected, and that server connected to normal client as expected.
Hack used to prevent client from sending sig_algs:

diff --git a/src/tls.c b/src/tls.c
index e3829cb38..5156c2f89 100644
--- a/src/tls.c
+++ b/src/tls.c
@@ -13673,10 +13673,12 @@ int TLSX_PopulateExtensions(WOLFSSL* ssl, byte isServer)
     } /* is not server */

 #if !defined(NO_CERTS) && !defined(WOLFSSL_NO_SIGALG)
-    WOLFSSL_MSG("Adding signature algorithms extension");
-    if ((ret = TLSX_SetSignatureAlgorithms(&ssl->extensions, ssl, ssl->heap))
-                                                                         != 0) {
-            return ret;
+    if (isServer) {
+        WOLFSSL_MSG("Adding signature algorithms extension");
+        if ((ret = TLSX_SetSignatureAlgorithms(&ssl->extensions, ssl, ssl->heap))
+                                                                            != 0) {
+                return ret;
+        }
     }
 #else
     ret = 0;

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

…server.

The server was checking ssl->extensions which will always have an entry for TLSX_SIGNATURE_ALGORITHMS
as it is unconditionally added by TLSX_PopulateExtensions earlier in the DoTls13ClientHello function.
Instead, check args->clSuites->hashSigAlgoSz which is only set if signature_algorithms is found and parsed by TLSX_Parse.
@kareem-wolfssl
Copy link
Contributor Author

Retest this please

@SparkiDev SparkiDev self-assigned this Jan 14, 2025
@SparkiDev SparkiDev merged commit e76186f into wolfSSL:master Jan 14, 2025
152 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: TLS 1.3 RFC 8446 violations
3 participants