From f262aa863390c9c1df085a543104eef0343ef5a5 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Tue, 20 Aug 2024 20:11:19 +0200 Subject: [PATCH 1/2] Make sensitivity of is_vulnerable_to_client_renegotiation_dos configurable --- sslyze/__init__.py | 1 + .../plugins/session_renegotiation_plugin.py | 30 ++++++++++++++----- sslyze/scanner/models.py | 6 +++- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/sslyze/__init__.py b/sslyze/__init__.py index 817d38e1..db6db287 100644 --- a/sslyze/__init__.py +++ b/sslyze/__init__.py @@ -22,6 +22,7 @@ # Classes for setting up scan commands and extra arguments from sslyze.plugins.scan_commands import ScanCommand from sslyze.plugins.certificate_info.implementation import CertificateInfoExtraArgument +from sslyze.plugins.session_renegotiation_plugin import SessionRenegotiationExtraArgument # Classes for scanning the servers from sslyze.scanner.models import ( diff --git a/sslyze/plugins/session_renegotiation_plugin.py b/sslyze/plugins/session_renegotiation_plugin.py index ea033c89..fde5adcd 100755 --- a/sslyze/plugins/session_renegotiation_plugin.py +++ b/sslyze/plugins/session_renegotiation_plugin.py @@ -14,13 +14,25 @@ ScanCommandExtraArgument, ScanJob, ScanCommandResult, - ScanCommandWrongUsageError, ScanCommandCliConnector, ScanJobResult, ) from sslyze.server_connectivity import ServerConnectivityInfo, TlsVersionEnum +@dataclass(frozen=True) +class SessionRenegotiationExtraArgument(ScanCommandExtraArgument): + """Additional configuration for testing a server for client-initiated renegotiation. + + Attributes: + client_renegotiation_attempts: The number of attempts to make when testing the client initiated + renegotiation DoS vector. If the server accepts this many attempts, + is_vulnerable_to_client_renegotiation_dos is set. Default: 10. + """ + + client_renegotiation_attempts: int + + @dataclass(frozen=True) class SessionRenegotiationScanResult(ScanCommandResult): """The result of testing a server for insecure TLS renegotiation and client-initiated renegotiation. @@ -82,14 +94,16 @@ class SessionRenegotiationImplementation(ScanCommandImplementation[SessionRenego @classmethod def scan_jobs_for_scan_command( - cls, server_info: ServerConnectivityInfo, extra_arguments: Optional[ScanCommandExtraArgument] = None + cls, server_info: ServerConnectivityInfo, extra_arguments: Optional[SessionRenegotiationExtraArgument] = None ) -> List[ScanJob]: - if extra_arguments: - raise ScanCommandWrongUsageError("This plugin does not take extra arguments") + client_renegotiation_attempts = extra_arguments.client_renegotiation_attempts if extra_arguments else 10 return [ ScanJob(function_to_call=_test_secure_renegotiation, function_arguments=[server_info]), - ScanJob(function_to_call=_test_client_renegotiation, function_arguments=[server_info]), + ScanJob( + function_to_call=_test_client_renegotiation, + function_arguments=[server_info, client_renegotiation_attempts], + ), ] @classmethod @@ -147,7 +161,9 @@ def _test_secure_renegotiation(server_info: ServerConnectivityInfo) -> Tuple[_Sc return _ScanJobResultEnum.SUPPORTS_SECURE_RENEG, supports_secure_renegotiation -def _test_client_renegotiation(server_info: ServerConnectivityInfo) -> Tuple[_ScanJobResultEnum, bool]: +def _test_client_renegotiation( + server_info: ServerConnectivityInfo, client_renegotiation_attempts: int +) -> Tuple[_ScanJobResultEnum, bool]: """Check whether the server honors session renegotiation requests.""" # Try with TLS 1.2 even if the server supports TLS 1.3 or higher as there is no reneg with TLS 1.3 if server_info.tls_probing_result.highest_tls_version_supported.value >= TlsVersionEnum.TLS_1_3.value: @@ -180,7 +196,7 @@ def _test_client_renegotiation(server_info: ServerConnectivityInfo) -> Tuple[_Sc try: # Do a reneg multiple times in a row to be 100% sure that the server has no mitigations in place # https://github.com/nabla-c0d3/sslyze/issues/473 - for i in range(10): + for i in range(client_renegotiation_attempts): ssl_connection.ssl_client.do_renegotiate() accepts_client_renegotiation = True diff --git a/sslyze/scanner/models.py b/sslyze/scanner/models.py index 08ca7946..2735d730 100644 --- a/sslyze/scanner/models.py +++ b/sslyze/scanner/models.py @@ -18,7 +18,10 @@ from sslyze.plugins.openssl_cipher_suites.implementation import CipherSuitesScanResult from sslyze.plugins.robot.implementation import RobotScanResult from sslyze.plugins.scan_commands import ScanCommand, ScanCommandsRepository -from sslyze.plugins.session_renegotiation_plugin import SessionRenegotiationScanResult +from sslyze.plugins.session_renegotiation_plugin import ( + SessionRenegotiationScanResult, + SessionRenegotiationExtraArgument, +) from sslyze.plugins.session_resumption.implementation import ( SessionResumptionSupportScanResult, SessionResumptionSupportExtraArgument, @@ -33,6 +36,7 @@ class ScanCommandsExtraArguments: # Field is present if extra arguments were provided for the corresponding scan command certificate_info: Optional[CertificateInfoExtraArgument] = None session_resumption: Optional[SessionResumptionSupportExtraArgument] = None + session_renegotiation: Optional[SessionRenegotiationExtraArgument] = None @dataclass(frozen=True) From fca777bb89e9c9ba255fbd9ec70b041ae85b616e Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Tue, 8 Oct 2024 13:30:23 +0200 Subject: [PATCH 2/2] Add tracking of session renegotiation successs count and test --- .../plugins/session_renegotiation_plugin.py | 23 +++++++++++++------ tests/json_tests/sslyze_output.json | 8 ++++--- .../test_session_renegotiation_plugin.py | 12 +++++++++- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/sslyze/plugins/session_renegotiation_plugin.py b/sslyze/plugins/session_renegotiation_plugin.py index fde5adcd..1cd19220 100755 --- a/sslyze/plugins/session_renegotiation_plugin.py +++ b/sslyze/plugins/session_renegotiation_plugin.py @@ -40,15 +40,18 @@ class SessionRenegotiationScanResult(ScanCommandResult): Attributes: accepts_client_renegotiation: True if the server honors client-initiated renegotiation attempts. supports_secure_renegotiation: True if the server supports secure renegotiation. + client_renegotiations_success_count: the number of successful client-initiated renegotiation attempts. """ supports_secure_renegotiation: bool is_vulnerable_to_client_renegotiation_dos: bool + client_renegotiations_success_count: int class SessionRenegotiationScanResultAsJson(BaseModelWithOrmModeAndForbid): supports_secure_renegotiation: bool is_vulnerable_to_client_renegotiation_dos: bool + client_renegotiations_success_count: int class SessionRenegotiationScanAttemptAsJson(ScanCommandAttemptAsJson): @@ -56,7 +59,7 @@ class SessionRenegotiationScanAttemptAsJson(ScanCommandAttemptAsJson): class _ScanJobResultEnum(Enum): - IS_VULNERABLE_TO_CLIENT_RENEG_DOS = 1 + CLIENT_RENEG_RESULT = 1 SUPPORTS_SECURE_RENEG = 2 @@ -87,7 +90,9 @@ def result_to_console_output(cls, result: SessionRenegotiationScanResult) -> Lis return result_txt -class SessionRenegotiationImplementation(ScanCommandImplementation[SessionRenegotiationScanResult, None]): +class SessionRenegotiationImplementation( + ScanCommandImplementation[SessionRenegotiationScanResult, SessionRenegotiationExtraArgument] +): """Test a server for insecure TLS renegotiation and client-initiated renegotiation.""" cli_connector_cls = _SessionRenegotiationCliConnector @@ -118,11 +123,13 @@ def result_for_completed_scan_jobs( result_enum, value = job.get_result() results_dict[result_enum] = value + is_vulnerable_to_client_renegotiation_dos, client_renegotiations_success_count = results_dict[ + _ScanJobResultEnum.CLIENT_RENEG_RESULT + ] return SessionRenegotiationScanResult( - is_vulnerable_to_client_renegotiation_dos=results_dict[ - _ScanJobResultEnum.IS_VULNERABLE_TO_CLIENT_RENEG_DOS - ], + is_vulnerable_to_client_renegotiation_dos=is_vulnerable_to_client_renegotiation_dos, supports_secure_renegotiation=results_dict[_ScanJobResultEnum.SUPPORTS_SECURE_RENEG], + client_renegotiations_success_count=client_renegotiations_success_count, ) @@ -163,9 +170,10 @@ def _test_secure_renegotiation(server_info: ServerConnectivityInfo) -> Tuple[_Sc def _test_client_renegotiation( server_info: ServerConnectivityInfo, client_renegotiation_attempts: int -) -> Tuple[_ScanJobResultEnum, bool]: +) -> Tuple[_ScanJobResultEnum, Tuple[bool, int]]: """Check whether the server honors session renegotiation requests.""" # Try with TLS 1.2 even if the server supports TLS 1.3 or higher as there is no reneg with TLS 1.3 + client_renegotiations_success_count = 0 if server_info.tls_probing_result.highest_tls_version_supported.value >= TlsVersionEnum.TLS_1_3.value: tls_version_to_use = TlsVersionEnum.TLS_1_2 downgraded_from_tls_1_3 = True @@ -198,6 +206,7 @@ def _test_client_renegotiation( # https://github.com/nabla-c0d3/sslyze/issues/473 for i in range(client_renegotiation_attempts): ssl_connection.ssl_client.do_renegotiate() + client_renegotiations_success_count += 1 accepts_client_renegotiation = True # Errors caused by a server rejecting the renegotiation @@ -246,4 +255,4 @@ def _test_client_renegotiation( finally: ssl_connection.close() - return _ScanJobResultEnum.IS_VULNERABLE_TO_CLIENT_RENEG_DOS, accepts_client_renegotiation + return _ScanJobResultEnum.CLIENT_RENEG_RESULT, (accepts_client_renegotiation, client_renegotiations_success_count) diff --git a/tests/json_tests/sslyze_output.json b/tests/json_tests/sslyze_output.json index 86853877..c1121592 100644 --- a/tests/json_tests/sslyze_output.json +++ b/tests/json_tests/sslyze_output.json @@ -8232,7 +8232,8 @@ "error_trace": null, "result": { "supports_secure_renegotiation": true, - "is_vulnerable_to_client_renegotiation_dos": false + "is_vulnerable_to_client_renegotiation_dos": false, + "client_renegotiations_success_count": 0 } }, "session_resumption": { @@ -17403,7 +17404,8 @@ "error_trace": null, "result": { "supports_secure_renegotiation": true, - "is_vulnerable_to_client_renegotiation_dos": false + "is_vulnerable_to_client_renegotiation_dos": false, + "client_renegotiations_success_count": 0 } }, "session_resumption": { @@ -17545,4 +17547,4 @@ "date_scans_completed": "2024-02-24T18:51:11.055270", "sslyze_version": "6.0.0b0", "sslyze_url": "https://github.com/nabla-c0d3/sslyze" -} \ No newline at end of file +} diff --git a/tests/plugins_tests/test_session_renegotiation_plugin.py b/tests/plugins_tests/test_session_renegotiation_plugin.py index 8fa1587e..8f1fadfe 100644 --- a/tests/plugins_tests/test_session_renegotiation_plugin.py +++ b/tests/plugins_tests/test_session_renegotiation_plugin.py @@ -4,6 +4,7 @@ SessionRenegotiationImplementation, SessionRenegotiationScanResult, SessionRenegotiationScanResultAsJson, + SessionRenegotiationExtraArgument, ) from sslyze.server_setting import ( @@ -40,17 +41,26 @@ def test_renegotiation_good(self) -> None: @can_only_run_on_linux_64 def test_renegotiation_is_vulnerable_to_client_renegotiation_dos(self) -> None: # Given a server that is vulnerable to client renegotiation DOS + expected_renegotiations_success_count = 3 + with LegacyOpenSslServer() as server: server_location = ServerNetworkLocation( hostname=server.hostname, ip_address=server.ip_address, port=server.port ) + extra_arg = SessionRenegotiationExtraArgument( + client_renegotiation_attempts=expected_renegotiations_success_count + ) server_info = check_connectivity_to_server_and_return_info(server_location) # When testing for insecure reneg, it succeeds - result: SessionRenegotiationScanResult = SessionRenegotiationImplementation.scan_server(server_info) + result: SessionRenegotiationScanResult = SessionRenegotiationImplementation.scan_server( + server_info, + extra_arguments=extra_arg, + ) # And the server is reported as vulnerable assert result.is_vulnerable_to_client_renegotiation_dos + assert result.client_renegotiations_success_count == expected_renegotiations_success_count # And a CLI output can be generated assert SessionRenegotiationImplementation.cli_connector_cls.result_to_console_output(result)