Enable SCSV check unconditionally

Author: yosuke-wolfssl

CMakeLists.txt

@@ -1881,7 +1881,6 @@ endif()
 #       - Truncated HMAC
 #       - Renegotiation indication
 #       - Secure renegotiation
-#       - Fallback SCSV
 
 add_option(WOLFSSL_OCSP "Enable OCSP (default: disabled)" "no" "yes;no")
 add_option(WOLFSSL_OCSPSTAPLING "Enable OCSP Stapling (default: disabled)" "no" "yes;no")

configure.ac

@@ -1250,7 +1250,6 @@ then
     test "$enable_savecert" = "" && enable_savecert=yes
     test "$enable_postauth" = "" && enable_postauth=yes
     test "$enable_hrrcookie" = "" && enable_hrrcookie=yes
-    test "$enable_fallback_scsv" = "" && enable_fallback_scsv=yes
     test "$enable_crl_monitor" = "" && enable_crl_monitor=yes
     test "$enable_sni" = "" && enable_sni=yes
     test "$enable_maxfragment" = "" && enable_maxfragment=yes
@@ -8313,18 +8312,6 @@ AC_ARG_ENABLE([secure-renegotiation-info],
     )
 
 
-# Fallback SCSV
-AC_ARG_ENABLE([fallback-scsv],
-    [AS_HELP_STRING([--enable-fallback-scsv],[Enable Fallback SCSV (default: disabled)])],
-    [ ENABLED_FALLBACK_SCSV=$enableval ],
-    [ ENABLED_FALLBACK_SCSV=no ]
-    )
-
-if test "x$ENABLED_FALLBACK_SCSV" = "xyes"
-then
-    AM_CFLAGS="$AM_CFLAGS -DHAVE_FALLBACK_SCSV"
-fi
-
 # Exporting Keying Material
 AC_ARG_ENABLE([keying-material],
     [AS_HELP_STRING([--enable-keying-material],[Enable Keying Material Exporters (default: disabled)])],
@@ -12937,7 +12924,6 @@ echo "   * Session Ticket:             $ENABLED_SESSION_TICKET"
 echo "   * Extended Master Secret:     $ENABLED_EXTENDED_MASTER"
 echo "   * Renegotiation Indication:   $ENABLED_RENEGOTIATION_INDICATION"
 echo "   * Secure Renegotiation:       $ENABLED_SECURE_RENEGOTIATION"
-echo "   * Fallback SCSV:              $ENABLED_FALLBACK_SCSV"
 echo "   * Keying Material Exporter:   $ENABLED_KEYING_MATERIAL"
 echo "   * All TLS Extensions:         $ENABLED_TLSX"
 echo "   * S/MIME:                     $ENABLED_SMIME"

examples/configs/user_settings_all.h

@@ -96,7 +96,6 @@ extern "C" {
 #define WOLFSSL_POST_HANDSHAKE_AUTH
 #define WOLFSSL_SEND_HRR_COOKIE /* Used by DTLS v1.3 */
 #define HAVE_ANON /* anon cipher suites */
-#define HAVE_FALLBACK_SCSV /* TLS_FALLBACK_SCSV */
 #define WOLFSSL_EARLY_DATA
 #define HAVE_SERVER_RENEGOTIATION_INFO
 

src/internal.c

@@ -60,7 +60,6 @@
  * WOLFSSL_TEST_APPLE_NATIVE_CERT_VALIDATION:
  *                  Testing mode for Apple cert validation             default: off
  * HAVE_DANE:                  DNS-based cert validation (DNSSEC)      default: off
- * HAVE_FALLBACK_SCSV:         TLS Fallback SCSV anti-downgrade        default: off
  * WOLFSSL_ACERT:              Attribute certificate support           default: off
  * WOLFSSL_DEBUG_CERTS:        Debug logging for cert processing       default: off
  * WOLFSSL_HOSTNAME_VERIFY_ALT_NAME_ONLY:
@@ -38463,6 +38462,7 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
         word32          begin = i;
         int             ret = 0;
         byte            lesserVersion;
+        byte            maxMinor;
 
         WOLFSSL_START(WC_FUNC_CLIENT_HELLO_DO);
         WOLFSSL_ENTER("DoClientHello");
@@ -38526,6 +38526,14 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
         if (pv.major == SSLv3_MAJOR && pv.minor >= TLSv1_3_MINOR)
             pv.minor = TLSv1_2_MINOR;
 
+        /* Snapshot the server's effective max version before the downgrade
+         * logic below lowers ssl->version.minor to the negotiated version.
+         * This honors runtime restrictions (e.g. SSL_OP_NO_TLSv1_3 on a
+         * TLS 1.3 capable method), unlike ssl->ctx->method->version.minor.
+         * Used by the TLS_FALLBACK_SCSV check, which runs after the cipher
+         * suites are parsed (and thus after ssl->version.minor is mutated). */
+        maxMinor = ssl->version.minor;
+
         lesserVersion = (byte)(!ssl->options.dtls &&
                                     ssl->version.minor > pv.minor);
         lesserVersion |= ssl->options.dtls &&ssl->version.minor < pv.minor;
@@ -38810,18 +38818,19 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
             }
         }
 #endif /* HAVE_SERVER_RENEGOTIATION_INFO */
-#if defined(HAVE_FALLBACK_SCSV) || defined(OPENSSL_ALL)
-        /* check for TLS_FALLBACK_SCSV suite */
+        /* Check for TLS_FALLBACK_SCSV (RFC 7507). Always enforced. */
         if (FindSuite(ssl->clSuites, TLS_FALLBACK_SCSV, 0) >= 0) {
             WOLFSSL_MSG("Found Fallback SCSV");
-            if (ssl->ctx->method->version.minor > pv.minor) {
+            /* Abort if the server supports a version higher than the client
+             * offered. DTLS version minors decrease as the version increases. */
+            if ((!ssl->options.dtls && maxMinor > pv.minor) ||
+                (ssl->options.dtls && maxMinor < pv.minor)) {
                 WOLFSSL_MSG("Client trying to connect with lesser version");
                 SendAlert(ssl, alert_fatal, inappropriate_fallback);
                 ret = VERSION_ERROR;
                 goto out;
             }
         }
-#endif
 
         i += ssl->clSuites->suiteSz;
         ssl->clSuites->hashSigAlgoSz = 0;

tests/api/test_tls.c

@@ -938,6 +938,73 @@ int test_tls12_ec_point_formats_no_uncompressed(void)
     return EXPECT_RESULT();
 }
 
+/* RFC 7507: a server that supports a version higher than the one offered by the
+ * client MUST abort with a fatal inappropriate_fallback alert when the client
+ * includes TLS_FALLBACK_SCSV (0x5600) in its cipher list.  This enforcement is
+ * always compiled in, so it must hold in the default build.
+ *
+ * The ClientHello below offers TLS 1.2 with TLS_FALLBACK_SCSV present, against a
+ * downgrade-capable server whose maximum is TLS 1.3 - i.e. a genuine downgrade.
+ * The server must reject it with VERSION_ERROR and send inappropriate_fallback.
+ */
+int test_tls_fallback_scsv(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
+    !defined(WOLFSSL_NO_TLS12) && defined(WOLFSSL_TLS13)
+    const byte clientHello[] = {
+        /* record header: handshake, TLS 1.2, length 47 */
+        0x16, 0x03, 0x03, 0x00, 0x2f,
+        /* handshake header: ClientHello, length 43 */
+        0x01, 0x00, 0x00, 0x2b,
+        /* client version: TLS 1.2 (lower than the server's TLS 1.3 max) */
+        0x03, 0x03,
+        /* random: 32 bytes */
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+        0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
+        0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+        0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f,
+        /* session id length: 0 */
+        0x00,
+        /* cipher suites length: 4 */
+        0x00, 0x04,
+        /* TLS_FALLBACK_SCSV, TLS_RSA_WITH_AES_128_CBC_SHA */
+        0x56, 0x00, 0x00, 0x2f,
+        /* compression methods: 1 entry, null */
+        0x01, 0x00,
+    };
+    /* Expected fatal inappropriate_fallback (86) alert on the wire. */
+    const byte fallbackAlert[] = {
+        0x15,             /* alert content type */
+        0x03, 0x03,       /* version: TLS 1.2 */
+        0x00, 0x02,       /* length: 2 */
+        0x02,             /* level: fatal */
+        0x56,             /* description: inappropriate_fallback (86) */
+    };
+    WOLFSSL_CTX *ctx_s = NULL;
+    WOLFSSL *ssl_s = NULL;
+    struct test_memio_ctx test_ctx;
+
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+    ExpectIntEQ(test_memio_inject_message(&test_ctx, 0,
+            (const char*)clientHello, sizeof(clientHello)), 0);
+    ExpectIntEQ(test_memio_setup(&test_ctx, NULL, &ctx_s, NULL, &ssl_s,
+                    NULL, wolfSSLv23_server_method), 0);
+    ExpectIntEQ(wolfSSL_accept(ssl_s), WOLFSSL_FATAL_ERROR);
+    ExpectIntEQ(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR),
+            WC_NO_ERR_TRACE(VERSION_ERROR));
+    /* The first record on the wire must be the inappropriate_fallback alert.
+     * The accept state machine then also queues a generic protocol_version
+     * alert for the VERSION_ERROR, which RFC 7507 permits. */
+    ExpectIntGE(test_ctx.c_len, (int)sizeof(fallbackAlert));
+    ExpectBufEQ(test_ctx.c_buff, fallbackAlert, sizeof(fallbackAlert));
+
+    wolfSSL_free(ssl_s);
+    wolfSSL_CTX_free(ctx_s);
+#endif
+    return EXPECT_RESULT();
+}
+
 /* RFC 8422 Section 5.1.2 ties the missing-uncompressed-format abort to the
  * server actually negotiating an ECC cipher suite. A client that omits the
  * uncompressed point format but negotiates a NON-ECC suite (here DHE_RSA) must
@@ -988,6 +1055,201 @@ int test_tls12_ec_point_formats_no_uncompressed_non_ecc(void)
     return EXPECT_RESULT();
 }
 
+/* DTLS counterpart of test_tls_fallback_scsv, exercising the DTLS branch of the
+ * fallback-SCSV check.  DTLS version minors decrease as the version increases
+ * (DTLS 1.0=0xff, 1.2=0xfd, 1.3=0xfc), so the downgrade comparison is inverted.
+ *
+ * The ClientHello offers DTLS 1.2 with TLS_FALLBACK_SCSV present, against a
+ * downgrade-capable server whose maximum is DTLS 1.3 - i.e. a genuine
+ * downgrade.  dtlsStateful is forced on so DoClientHello skips the stateless
+ * cookie exchange (HelloVerifyRequest) and reaches the cipher-suite parsing.
+ */
+int test_dtls_fallback_scsv(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
+    defined(WOLFSSL_DTLS) && defined(WOLFSSL_DTLS13)
+    const byte clientHello[] = {
+        /* DTLS record header: handshake, DTLS 1.2, epoch 0, seq 0, length 56 */
+        0x16, 0xfe, 0xfd, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x38,
+        /* DTLS handshake header: ClientHello, length 44, msg_seq 0,
+         * fragment_offset 0, fragment_length 44 */
+        0x01, 0x00, 0x00, 0x2c, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x2c,
+        /* client version: DTLS 1.2 (lower than the server's DTLS 1.3 max) */
+        0xfe, 0xfd,
+        /* random: 32 bytes */
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+        0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
+        0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+        0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f,
+        /* session id length: 0 */
+        0x00,
+        /* cookie length: 0 */
+        0x00,
+        /* cipher suites length: 4 */
+        0x00, 0x04,
+        /* TLS_FALLBACK_SCSV, TLS_RSA_WITH_AES_128_CBC_SHA */
+        0x56, 0x00, 0x00, 0x2f,
+        /* compression methods: 1 entry, null */
+        0x01, 0x00,
+    };
+    WOLFSSL_CTX *ctx_s = NULL;
+    WOLFSSL *ssl_s = NULL;
+    struct test_memio_ctx test_ctx;
+
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+    ExpectIntEQ(test_memio_inject_message(&test_ctx, 0,
+            (const char*)clientHello, sizeof(clientHello)), 0);
+    ExpectIntEQ(test_memio_setup(&test_ctx, NULL, &ctx_s, NULL, &ssl_s,
+                    NULL, wolfDTLS_server_method), 0);
+    /* Skip the stateless cookie exchange so the suites are parsed directly. */
+    if (ssl_s != NULL)
+        ssl_s->options.dtlsStateful = 1;
+    ExpectIntEQ(wolfSSL_accept(ssl_s), WOLFSSL_FATAL_ERROR);
+    ExpectIntEQ(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR),
+            WC_NO_ERR_TRACE(VERSION_ERROR));
+    /* The first record on the wire must be a fatal inappropriate_fallback alert.
+     * Check by offset to stay robust against the DTLS epoch/sequence bytes. */
+    ExpectIntGE(test_ctx.c_len, 15);
+    ExpectIntEQ((byte)test_ctx.c_buff[0], 0x15);   /* alert content type */
+    ExpectIntEQ((byte)test_ctx.c_buff[1], 0xfe);   /* DTLS 1.2 */
+    ExpectIntEQ((byte)test_ctx.c_buff[2], 0xfd);
+    ExpectIntEQ((byte)test_ctx.c_buff[11], 0x00);  /* record length: 2 */
+    ExpectIntEQ((byte)test_ctx.c_buff[12], 0x02);
+    ExpectIntEQ((byte)test_ctx.c_buff[13], 0x02);  /* level: fatal */
+    ExpectIntEQ((byte)test_ctx.c_buff[14], 0x56);  /* inappropriate_fallback */
+
+    wolfSSL_free(ssl_s);
+    wolfSSL_CTX_free(ctx_s);
+#endif
+    return EXPECT_RESULT();
+}
+
+/* Negative control for the fallback-SCSV check: a ClientHello that carries
+ * TLS_FALLBACK_SCSV but offers the server's highest supported version is NOT a
+ * downgrade, so RFC 7507 requires the handshake to proceed (no
+ * inappropriate_fallback).  Guards against over-aggressive enforcement now that
+ * the check is always compiled in.
+ */
+int test_tls_fallback_scsv_no_downgrade(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && !defined(WOLFSSL_NO_TLS12)
+    /* TLS 1.2 ClientHello with TLS_FALLBACK_SCSV against a TLS 1.2 server, i.e.
+     * the client offers the server's maximum version - not a downgrade. */
+    const byte clientHello[] = {
+        /* record header: handshake, TLS 1.2, length 47 */
+        0x16, 0x03, 0x03, 0x00, 0x2f,
+        /* handshake header: ClientHello, length 43 */
+        0x01, 0x00, 0x00, 0x2b,
+        /* client version: TLS 1.2 (== server max) */
+        0x03, 0x03,
+        /* random: 32 bytes */
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+        0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
+        0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+        0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f,
+        /* session id length: 0 */
+        0x00,
+        /* cipher suites length: 4 */
+        0x00, 0x04,
+        /* TLS_FALLBACK_SCSV, TLS_RSA_WITH_AES_128_CBC_SHA */
+        0x56, 0x00, 0x00, 0x2f,
+        /* compression methods: 1 entry, null */
+        0x01, 0x00,
+    };
+    WOLFSSL_CTX *ctx_s = NULL;
+    WOLFSSL *ssl_s = NULL;
+    struct test_memio_ctx test_ctx;
+
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+    ExpectIntEQ(test_memio_inject_message(&test_ctx, 0,
+            (const char*)clientHello, sizeof(clientHello)), 0);
+    ExpectIntEQ(test_memio_setup(&test_ctx, NULL, &ctx_s, NULL, &ssl_s,
+                    NULL, wolfTLSv1_2_server_method), 0);
+    /* The handshake must not be rejected for the fallback reason: no
+     * VERSION_ERROR and no inappropriate_fallback alert on the wire.  Whatever
+     * happens afterwards (suite match or not) is not under test here. */
+    (void)wolfSSL_accept(ssl_s);
+    ExpectIntNE(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR),
+            WC_NO_ERR_TRACE(VERSION_ERROR));
+    ExpectFalse(test_ctx.c_len >= 7 && (byte)test_ctx.c_buff[0] == 0x15 &&
+            (byte)test_ctx.c_buff[6] == 0x56 /* inappropriate_fallback */);
+
+    wolfSSL_free(ssl_s);
+    wolfSSL_CTX_free(ctx_s);
+#endif
+    return EXPECT_RESULT();
+}
+
+/* Negative control for runtime max version handling: the fallback-SCSV ceiling must honor a
+ * runtime-disabled version, not the compiled method version.  A TLS 1.3 capable
+ * method (wolfSSLv23_server_method) has SSL_OP_NO_TLSv1_3 applied at runtime, so
+ * its effective maximum is TLS 1.2.  A client that legitimately offers TLS 1.2
+ * with TLS_FALLBACK_SCSV is therefore NOT downgrading and must not be rejected
+ * with inappropriate_fallback (RFC 7507).  Before the fix the check compared
+ * against ssl->ctx->method->version.minor (still TLS 1.3), wrongly aborting.
+ */
+int test_tls_fallback_scsv_no_downgrade_runtime_max(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \
+    !defined(WOLFSSL_NO_TLS12)
+    /* TLS 1.2 ClientHello with TLS_FALLBACK_SCSV.  The server's compiled max is
+     * TLS 1.3 but TLS 1.3 is disabled at runtime, so TLS 1.2 == effective max -
+     * not a downgrade. */
+    const byte clientHello[] = {
+        /* record header: handshake, TLS 1.2, length 47 */
+        0x16, 0x03, 0x03, 0x00, 0x2f,
+        /* handshake header: ClientHello, length 43 */
+        0x01, 0x00, 0x00, 0x2b,
+        /* client version: TLS 1.2 (== server effective max) */
+        0x03, 0x03,
+        /* random: 32 bytes */
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+        0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
+        0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+        0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f,
+        /* session id length: 0 */
+        0x00,
+        /* cipher suites length: 4 */
+        0x00, 0x04,
+        /* TLS_FALLBACK_SCSV, TLS_RSA_WITH_AES_128_CBC_SHA */
+        0x56, 0x00, 0x00, 0x2f,
+        /* compression methods: 1 entry, null */
+        0x01, 0x00,
+    };
+    WOLFSSL_CTX *ctx_s = NULL;
+    WOLFSSL *ssl_s = NULL;
+    struct test_memio_ctx test_ctx;
+
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+    ExpectIntEQ(test_memio_inject_message(&test_ctx, 0,
+            (const char*)clientHello, sizeof(clientHello)), 0);
+    ExpectIntEQ(test_memio_setup(&test_ctx, NULL, &ctx_s, NULL, &ssl_s,
+                    NULL, wolfSSLv23_server_method), 0);
+    /* Lower the effective max to TLS 1.2 at runtime; this updates
+     * ssl->version.minor but leaves ssl->ctx->method->version.minor at TLS 1.3,
+     * which is exactly the condition that triggered the false positive. */
+    if (ssl_s != NULL)
+        ExpectIntNE(wolfSSL_set_options(ssl_s, WOLFSSL_OP_NO_TLSv1_3), 0);
+    /* The handshake must not be rejected for the fallback reason: no
+     * VERSION_ERROR and no inappropriate_fallback alert on the wire.  Whatever
+     * happens afterwards (suite match or not) is not under test here. */
+    (void)wolfSSL_accept(ssl_s);
+    ExpectIntNE(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR),
+            WC_NO_ERR_TRACE(VERSION_ERROR));
+    ExpectFalse(test_ctx.c_len >= 7 && (byte)test_ctx.c_buff[0] == 0x15 &&
+            (byte)test_ctx.c_buff[6] == 0x56 /* inappropriate_fallback */);
+
+    wolfSSL_free(ssl_s);
+    wolfSSL_CTX_free(ctx_s);
+#endif
+    return EXPECT_RESULT();
+}
+
 /* Test that set_curves_list correctly resolves ECC curve names that fall
  * through the kNistCurves table and reach the wc_ecc_get_curve_idx_from_name
  * fallback path.  The kNistCurves lookup uses a case-sensitive XSTRNCMP, so