diff --git a/sslyze/connection_helpers/http_response_parser.py b/sslyze/connection_helpers/http_response_parser.py index 61461a59..c62d25be 100644 --- a/sslyze/connection_helpers/http_response_parser.py +++ b/sslyze/connection_helpers/http_response_parser.py @@ -10,6 +10,10 @@ def makefile(self, *args, **kw): # type: ignore return self +class NotAValidHttpResponseError(Exception): + pass + + class HttpResponseParser: """Utility to parse HTTP responses - http://pythonwise.blogspot.com/2010/02/parse-http-response.html. """ @@ -34,4 +38,9 @@ def _parse(read_method: Callable) -> HTTPResponse: fake_sock = _FakeSocket(response) response = HTTPResponse(fake_sock) # type: ignore response.begin() + + if response.version == 9: + # HTTP 0.9 => Probably not an HTTP response + raise NotAValidHttpResponseError() + return response diff --git a/sslyze/plugins/http_headers_plugin.py b/sslyze/plugins/http_headers_plugin.py index 1be347d0..a5b8db3f 100755 --- a/sslyze/plugins/http_headers_plugin.py +++ b/sslyze/plugins/http_headers_plugin.py @@ -1,8 +1,10 @@ import logging +import socket from concurrent.futures._base import Future from http.client import HTTPResponse from dataclasses import dataclass +from traceback import TracebackException from urllib.parse import urlsplit from sslyze.plugins.plugin_base import ( @@ -15,7 +17,7 @@ ) from sslyze.server_connectivity import ServerConnectivityInfo from sslyze.connection_helpers.http_request_generator import HttpRequestGenerator -from sslyze.connection_helpers.http_response_parser import HttpResponseParser +from sslyze.connection_helpers.http_response_parser import HttpResponseParser, NotAValidHttpResponseError from typing import List, Optional @@ -76,20 +78,27 @@ class StrictTransportSecurityHeader: class HttpHeadersScanResult(ScanCommandResult): """The result of testing a server for the presence of security-related HTTP headers. - Each HTTP header described below will be ``None`` if the server did not return it. + Each HTTP header described below will be ``None`` if the server did not return a valid HTTP response, or if the + server returned an HTTP response without the HTTP header. Attributes: + http_request_sent: The initial HTTP request sent to the server by SSLyze. + http_error_trace: An error the server returned after receiving the initial HTTP request. If this field is set, + all the subsequent fields will be ``None`` as SSLyze did not receive a valid HTTP response from the server. + http_path_redirected_to: The path SSLyze was eventually redirected to after sending the initial HTTP request. strict_transport_security_header: The Strict-Transport-Security header returned by the server. public_key_pins_header: The Public-Key-Pins header returned by the server. public_key_pins_report_only_header: The Public-Key-Pins-Report-Only header returned by the server. expect_ct_header: The Expect-CT header returned by the server. """ - strict_transport_security_header: Optional[StrictTransportSecurityHeader] + http_request_sent: str + http_error_trace: Optional[TracebackException] + http_path_redirected_to: Optional[str] + strict_transport_security_header: Optional[StrictTransportSecurityHeader] public_key_pins_header: Optional[PublicKeyPinsHeader] public_key_pins_report_only_header: Optional[PublicKeyPinsHeader] - expect_ct_header: Optional[ExpectCtHeader] @@ -102,6 +111,16 @@ class _HttpHeadersCliConnector(ScanCommandCliConnector[HttpHeadersScanResult, No def result_to_console_output(cls, result: HttpHeadersScanResult) -> List[str]: result_as_txt = [cls._format_title("HTTP Security Headers")] + # If an error occurred after sending the HTTP request, just display it + if result.http_error_trace: + result_as_txt.append( + cls._format_subtitle("Error - Server did not return a valid HTTP response. Is it an HTTP server?") + ) + result_as_txt.append("") + for trace_line in result.http_error_trace.format(chain=False): + result_as_txt.append(f" {trace_line.strip()}") + return result_as_txt + # HSTS result_as_txt.append(cls._format_subtitle("Strict-Transport-Security Header")) if not result.strict_transport_security_header: @@ -174,13 +193,17 @@ def _retrieve_and_analyze_http_response(server_info: ServerConnectivityInfo) -> _logger.info(f"Retrieving HTTP headers from {server_info}") redirections_count = 0 next_location_path: Optional[str] = "/" + http_error_trace = None + while next_location_path and redirections_count < 4: - _logger.info(f"Sending request to {next_location_path}") + _logger.info(f"Sending HTTP request to {next_location_path}") + http_path_redirected_to = next_location_path + + # Perform the TLS handshake ssl_connection = server_info.get_preconfigured_tls_connection() - try: - # Perform the TLS handshake - ssl_connection.connect() + ssl_connection.connect() + try: # Send an HTTP GET request to the server ssl_connection.ssl_client.write( HttpRequestGenerator.get_request( @@ -188,12 +211,16 @@ def _retrieve_and_analyze_http_response(server_info: ServerConnectivityInfo) -> ) ) http_response = HttpResponseParser.parse_from_ssl_connection(ssl_connection.ssl_client) + + except (socket.timeout, ConnectionError, NotAValidHttpResponseError) as e: + # The server didn't return a proper HTTP response + http_error_trace = TracebackException.from_exception(e) + finally: ssl_connection.close() - if http_response.version == 9: - # HTTP 0.9 => Probably not an HTTP response - raise ValueError("Server did not return an HTTP response") + if http_error_trace: + break # Handle redirection if there is one next_location_path = _detect_http_redirection( @@ -203,13 +230,33 @@ def _retrieve_and_analyze_http_response(server_info: ServerConnectivityInfo) -> ) redirections_count += 1 - # Parse and return each header - return HttpHeadersScanResult( - strict_transport_security_header=_parse_hsts_header_from_http_response(http_response), - public_key_pins_header=_parse_hpkp_header_from_http_response(http_response), - public_key_pins_report_only_header=_parse_hpkp_report_only_header_from_http_response(http_response), - expect_ct_header=_parse_expect_ct_header_from_http_response(http_response), - ) + # Prepare the results + initial_http_request = HttpRequestGenerator.get_request( + host=server_info.network_configuration.tls_server_name_indication, path="/" + ).decode("ascii") + + if http_error_trace: + # If the server errored when receiving an HTTP request, return the error as the result + return HttpHeadersScanResult( + http_request_sent=initial_http_request, + http_error_trace=http_error_trace, + http_path_redirected_to=None, + strict_transport_security_header=None, + public_key_pins_header=None, + public_key_pins_report_only_header=None, + expect_ct_header=None, + ) + else: + # If no HTTP error happened, parse and return each header + return HttpHeadersScanResult( + http_request_sent=initial_http_request, + http_path_redirected_to=http_path_redirected_to, + http_error_trace=None, + strict_transport_security_header=_parse_hsts_header_from_http_response(http_response), + public_key_pins_header=_parse_hpkp_header_from_http_response(http_response), + public_key_pins_report_only_header=_parse_hpkp_report_only_header_from_http_response(http_response), + expect_ct_header=_parse_expect_ct_header_from_http_response(http_response), + ) def _detect_http_redirection(http_response: HTTPResponse, server_host_name: str, server_port: int) -> Optional[str]: diff --git a/tests/openssl_server/__init__.py b/tests/openssl_server/__init__.py index 7d391009..8e0931ec 100644 --- a/tests/openssl_server/__init__.py +++ b/tests/openssl_server/__init__.py @@ -10,7 +10,7 @@ import logging import time from threading import Thread -from typing import Optional, List +from typing import Optional, List, IO class NotOnLinux64Error(EnvironmentError): @@ -31,30 +31,34 @@ class _OpenSslServerIOManager: """Thread to log all output from s_server and reply to incoming connections. """ - def __init__(self, s_server_stdout, s_server_stdin): - self.s_server_stdout = s_server_stdout - self.s_server_stdin = s_server_stdin + def __init__( + self, s_server_stdout: IO[bytes], s_server_stdin: IO[bytes], should_reply_to_http_requests: bool + ) -> None: + self._s_server_stdout = s_server_stdout + self._s_server_stdin = s_server_stdin + self._should_reply_to_http_requests = should_reply_to_http_requests self.is_server_ready = False def read_and_log_and_reply(): while True: - s_server_out = self.s_server_stdout.readline() + s_server_out = self._s_server_stdout.readline() if s_server_out: logging.warning(f"s_server output: {s_server_out}") if b"ACCEPT" in s_server_out: - # S_server is ready to receive connections + # The s_server process is ready to receive connections self.is_server_ready = True if _OpenSslServer.HELLO_MSG in s_server_out: # When receiving the special message, we want s_server to reply - self.s_server_stdin.write(b"Hey there") - self.s_server_stdin.flush() - - if b"Connection: close\r\n" in s_server_out: - # We "Connection: close" to detect an HTTP request being sent and we return an HTTP response - self.s_server_stdin.write(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n") - self.s_server_stdin.flush() + self._s_server_stdin.write(b"Hey there") + self._s_server_stdin.flush() + + if self._should_reply_to_http_requests: + if b"Connection: close\r\n" in s_server_out: + # We "Connection: close" to detect an HTTP request being sent and we return an HTTP response + self._s_server_stdin.write(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n") + self._s_server_stdin.flush() else: break @@ -125,6 +129,7 @@ def __init__( client_auth_config: ClientAuthConfigEnum, should_enable_server_cipher_preference: bool, openssl_cipher_string: Optional[str], + should_reply_to_http_requests: bool = True, extra_openssl_args: Optional[List[str]] = None, ) -> None: if not self.is_platform_supported(): @@ -138,6 +143,7 @@ def __init__( self.ip_address = "127.0.0.1" self._server_certificate_path = server_certificate_path self._server_key_path = server_key_path + self._should_reply_to_http_requests = should_reply_to_http_requests # Retrieve one of the available local ports; set.pop() is thread safe self.port = self._AVAILABLE_LOCAL_PORTS.pop() @@ -161,11 +167,13 @@ def __enter__(self): self._process = subprocess.Popen( args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT ) - self._server_io_manager = _OpenSslServerIOManager(self._process.stdout, self._process.stdin) + self._server_io_manager = _OpenSslServerIOManager( + self._process.stdout, self._process.stdin, self._should_reply_to_http_requests + ) # Block until s_server is ready to accept requests while not self._server_io_manager.is_server_ready: - time.sleep(1) + time.sleep(0.5) if self._process.poll() is not None: # s_server has terminated early raise RuntimeError("Could not start s_server") @@ -270,6 +278,7 @@ def __init__( client_auth_config: ClientAuthConfigEnum = ClientAuthConfigEnum.DISABLED, should_enable_server_cipher_preference: bool = False, openssl_cipher_string: Optional[str] = None, + should_reply_to_http_requests: bool = True, max_early_data: Optional[int] = None, groups: Optional[str] = None, ) -> None: @@ -288,4 +297,5 @@ def __init__( should_enable_server_cipher_preference=should_enable_server_cipher_preference, server_certificate_path=server_certificate_path, server_key_path=server_key_path, + should_reply_to_http_requests=should_reply_to_http_requests, ) diff --git a/tests/plugins_tests/test_http_headers_plugin.py b/tests/plugins_tests/test_http_headers_plugin.py index abe73618..bbfbc64e 100644 --- a/tests/plugins_tests/test_http_headers_plugin.py +++ b/tests/plugins_tests/test_http_headers_plugin.py @@ -17,7 +17,7 @@ ClientAuthenticationCredentials, ) from tests.markers import can_only_run_on_linux_64 -from tests.openssl_server import ClientAuthConfigEnum, LegacyOpenSslServer +from tests.openssl_server import ClientAuthConfigEnum, LegacyOpenSslServer, ModernOpenSslServer class TestHttpHeadersPlugin: @@ -30,6 +30,8 @@ def test_hsts_enabled(self): result: HttpHeadersScanResult = HttpHeadersImplementation.scan_server(server_info) # And only HSTS is detected + assert result.http_request_sent + assert result.http_path_redirected_to assert result.strict_transport_security_header assert not result.public_key_pins_header assert not result.public_key_pins_report_only_header @@ -47,6 +49,8 @@ def test_hsts_and_hpkp_disabled(self): result: HttpHeadersScanResult = HttpHeadersImplementation.scan_server(server_info) # And no headers are detected + assert result.http_request_sent + assert result.http_path_redirected_to assert not result.strict_transport_security_header assert not result.public_key_pins_header assert not result.public_key_pins_report_only_header @@ -70,6 +74,33 @@ def test_expect_ct_enabled(self): # And a CLI output can be generated assert HttpHeadersImplementation.cli_connector_cls.result_to_console_output(result) + def test_http_error(self): + # Given a server to scan + with ModernOpenSslServer( + # And the server will trigger an error when receiving an HTTP request + should_reply_to_http_requests=False + ) as server: + server_location = ServerNetworkLocationViaDirectConnection( + hostname=server.hostname, ip_address=server.ip_address, port=server.port + ) + server_info = ServerConnectivityTester().perform(server_location) + + # When scanning for HTTP headers, it succeeds + result: HttpHeadersScanResult = HttpHeadersImplementation.scan_server(server_info) + + # And the result mention the error returned by the server when sending an HTTP request + assert result.http_error_trace + assert result.http_request_sent + + # And the other result fields are not set + assert not result.http_path_redirected_to + assert not result.public_key_pins_header + assert not result.public_key_pins_report_only_header + assert not result.expect_ct_header + + # And a CLI output can be generated + assert HttpHeadersImplementation.cli_connector_cls.result_to_console_output(result) + @can_only_run_on_linux_64 def test_fails_when_client_auth_failed(self): # Given a server that requires client authentication