rudimentary cipher order checks

checks/tasks/tls.py

@@ -27,6 +27,7 @@
 from django.core.cache import cache
 from django.db import transaction
 from nassl.ephemeral_key_info import DhEphemeralKeyInfo, EcDhEphemeralKeyInfo, OpenSslEvpPkeyEnum
+from nassl.ssl_client import ClientCertificateRequested
 from sslyze import (
     Scanner,
     ServerScanRequest,
@@ -38,13 +39,19 @@
     ServerNetworkConfiguration,
     ProtocolWithOpportunisticTlsEnum,
     ScanCommandsExtraArguments,
-    CertificateInfoExtraArgument, CipherSuite,
+    CertificateInfoExtraArgument,
+    CipherSuite,
 )
+from sslyze.errors import ServerTlsConfigurationNotSupported, ServerRejectedTlsHandshake, TlsHandshakeTimedOut
 
 from sslyze.plugins.certificate_info._certificate_utils import (
     parse_subject_alternative_name_extension,
     get_common_names,
 )
+from sslyze.plugins.openssl_cipher_suites._test_cipher_suite import _set_cipher_suite_string
+from sslyze.plugins.openssl_cipher_suites._tls12_workaround import WorkaroundForTls12ForCipherSuites
+from sslyze.plugins.openssl_cipher_suites.cipher_suites import CipherSuitesRepository
+from sslyze.server_connectivity import ServerConnectivityInfo
 
 from checks import categories, scoring
 from checks.http_client import http_get_ip
@@ -60,7 +67,6 @@
     WebTestTls,
     ZeroRttStatus,
 )
-from checks.scoring import Score
 from checks.tasks import SetupUnboundContext
 from checks.tasks.dispatcher import check_registry, post_callback_hook
 from checks.tasks.http_headers import (
@@ -1374,6 +1380,7 @@ def has_daneTA(tlsa_records):
             return True
     return False
 
+
 def check_web_tls(url, af_ip_pair=None, *args, **kwargs):
     """
     Check the webserver's TLS configuration.
@@ -1398,7 +1405,16 @@ def check_web_tls(url, af_ip_pair=None, *args, **kwargs):
     prots_bad, prots_phase_out, prots_good, prots_sufficient, prots_score = evaluate_tls_protocols(prots_accepted)
     dh_param, ec_param, fs_bad, fs_phase_out, fs_score = evaluate_tls_fs_params(ciphers_accepted)
     cipher_evaluation = TLSCipherEvaluation.from_ciphers_accepted(ciphers_accepted)
-    cipher_order_violation, cipher_order_status, cipher_order_score = test_cipher_order(ciphers_accepted)
+    # TODO: pick best TLS version
+    cipher_order_violation, cipher_order_status, cipher_order_score = test_cipher_order(
+        ServerConnectivityInfo(
+            server_location=result.server_location,
+            network_configuration=result.network_configuration,
+            tls_probing_result=result.connectivity_result,
+        ),
+        prots_accepted[0],
+        cipher_evaluation,
+    )
 
     ocsp_status = OcspStatus.ok
     if any(
@@ -1585,15 +1601,18 @@ def from_ciphers_accepted(cls, ciphers_accepted: List[CipherSuiteAcceptedByServe
             elif suite.cipher_suite.name in CIPHERS_PHASE_OUT:
                 ciphers_phase_out.append(suite.cipher_suite)
             else:
-                ciphers_bad.append(f"{suite.cipher_suite.openssl_name} ({suite.cipher_suite.name})")
+                ciphers_bad.append(suite.cipher_suite)
         return cls(
-            ciphers_good=ciphers_good, ciphers_sufficient=ciphers_sufficient, ciphers_phase_out=ciphers_phase_out,
+            ciphers_good=ciphers_good,
+            ciphers_sufficient=ciphers_sufficient,
+            ciphers_phase_out=ciphers_phase_out,
             ciphers_bad=ciphers_bad,
             ciphers_good_str=cls._format_str(ciphers_good),
             ciphers_sufficient_str=cls._format_str(ciphers_sufficient),
             ciphers_phase_out_str=cls._format_str(ciphers_phase_out),
             ciphers_bad_str=cls._format_str(ciphers_bad),
         )
+
     @staticmethod
     def _format_str(suites: List[CipherSuite]) -> List[str]:
         # TODO: remove IANA name, just here for debugging now
@@ -1604,13 +1623,99 @@ def score(self) -> scoring.Score:
         return scoring.WEB_TLS_SUITES_BAD if self.ciphers_bad else scoring.WEB_TLS_SUITES_GOOD
 
 
-def test_cipher_order(cipher_evaluation: TLSCipherEvaluation) -> Tuple[List[str], CipherOrderStatus, scoring.Score]:
+def test_cipher_order(
+    server_connectivity_info: ServerConnectivityInfo,
+    tls_version: TlsVersionEnum,
+    cipher_evaluation: TLSCipherEvaluation,
+) -> Tuple[List[str], CipherOrderStatus, scoring.Score]:
     cipher_order_violation = []
-    cipher_order_status = CipherOrderStatus.na
+    cipher_order_status = CipherOrderStatus.good
     cipher_order_score = scoring.WEB_TLS_CIPHER_ORDER_OK
+    if (
+        not cipher_evaluation.ciphers_bad
+        and not cipher_evaluation.ciphers_phase_out_str
+        and not cipher_evaluation.ciphers_sufficient
+    ):
+        cipher_order_status = CipherOrderStatus.na
+        return cipher_order_violation, cipher_order_status, cipher_order_score
+
+    order_tuples = [
+        (
+            cipher_evaluation.ciphers_bad + cipher_evaluation.ciphers_phase_out + cipher_evaluation.ciphers_sufficient,
+            cipher_evaluation.ciphers_good,
+        ),
+        (cipher_evaluation.ciphers_bad + cipher_evaluation.ciphers_phase_out, cipher_evaluation.ciphers_sufficient),
+        (cipher_evaluation.ciphers_bad, cipher_evaluation.ciphers_phase_out),
+    ]
+    for expected_less_preferred, expected_more_preferred in order_tuples:
+        print(
+            f"evaluating less {[s.openssl_name for s in expected_less_preferred]} vs "
+            f"more {[s.openssl_name for s in expected_more_preferred]} TLS {tls_version}"
+        )
+        if not expected_less_preferred or not expected_more_preferred:
+            continue
+        preferred_suite = find_most_preferred_cipher_suite(
+            server_connectivity_info, tls_version, expected_less_preferred + expected_more_preferred
+        )
+        if preferred_suite not in expected_more_preferred:
+            cipher_order_violation = [preferred_suite.openssl_name]  # TODO: check which name to report
+            cipher_order_status = CipherOrderStatus.bad
+            cipher_order_score = scoring.WEB_TLS_CIPHER_ORDER_BAD
+
     return cipher_order_violation, cipher_order_status, cipher_order_score
 
 
+# TODO: maybe move to a utils module?
+# adapted from sslyze.plugins.openssl_cipher_suites._test_cipher_suite.connect_with_cipher_suite
+def find_most_preferred_cipher_suite(
+    server_connectivity_info: ServerConnectivityInfo, tls_version: TlsVersionEnum, cipher_suites: List[CipherSuite]
+) -> CipherSuite:
+    suite_names = [suite.openssl_name for suite in cipher_suites]
+    requires_legacy_openssl = True
+    if tls_version == TlsVersionEnum.TLS_1_2:
+        # For TLS 1.2, we need to pick the right version of OpenSSL depending on which cipher suite
+        requires_legacy_openssl = any(
+            [WorkaroundForTls12ForCipherSuites.requires_legacy_openssl(name) for name in suite_names]
+        )
+    elif tls_version == TlsVersionEnum.TLS_1_3:
+        requires_legacy_openssl = False
+
+    ssl_connection = server_connectivity_info.get_preconfigured_tls_connection(
+        override_tls_version=tls_version, should_use_legacy_openssl=requires_legacy_openssl
+    )
+    _set_cipher_suite_string(tls_version, ":".join(suite_names), ssl_connection.ssl_client)
+
+    try:
+        # Perform the SSL handshake
+        ssl_connection.connect()
+
+    except ServerTlsConfigurationNotSupported:
+        # SSLyze rejected the handshake because the server's DH config was too insecure; this means the
+        # cipher suite is actually supported
+        pass
+
+    except ClientCertificateRequested:
+        # When the handshake failed due to ClientCertificateRequested
+        pass
+    except ServerRejectedTlsHandshake:
+        return False
+
+    except TlsHandshakeTimedOut:
+        # Sometimes triggered by servers that don't support (at all) a specific version of TLS
+        # Amazon Cloudfront does that with TLS 1.3
+        # There's no easy way to differentiate this error from a network glitch/timeout
+        return False
+
+    finally:
+        ssl_connection.close()
+
+    selected_cipher = CipherSuitesRepository.get_cipher_suite_with_openssl_name(
+        tls_version, ssl_connection.ssl_client.get_current_cipher_name()
+    )
+    print(f"from CS {suite_names} selected {selected_cipher}")
+    return selected_cipher
+
+
 def do_web_http(af_ip_pairs, url, task, *args, **kwargs):
     """
     Start all the HTTP related checks for the web test.

checks/tasks/tls_constants.py

@@ -32,18 +32,22 @@
     OpenSslEcNidEnum.SECP224R1,
 ]
 
+# ECDHE-RSA-AES256-GCM-SHA384
 CIPHERS_GOOD = [
     "TLS_AES_256_GCM_SHA384",
     "TLS_CHACHA20_POLY1305_SHA256",
     "TLS_AES_128_GCM_SHA256",
-]
-CIPHERS_SUFFICIENT = [
+    # NCSC appendix C lists these as sufficient, but read
+    # footnote 52 carefully. As we test TLS version separate
+    # from cipher list, we consider them good.
     "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
     "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256",
     "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
     "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
     "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256",
     "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
+]
+CIPHERS_SUFFICIENT = [
     "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384",
     "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
     "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256",