From b494399609d17017696b36bf05b4e2842fc20e91 Mon Sep 17 00:00:00 2001 From: Saptak S Date: Wed, 20 Jan 2021 00:15:47 +0530 Subject: [PATCH 01/15] Adds support for sslyze>=3.0.0 - fixes import errors for the current required modules - uses the new format for certificate analyzer - updates dependency of sslyze to >=3.0.0 --- setup.py | 2 +- src/pshtt/pshtt.py | 152 +++++++++++++++++++-------------------------- 2 files changed, 65 insertions(+), 89 deletions(-) diff --git a/setup.py b/setup.py index ce25344..cd20722 100644 --- a/setup.py +++ b/setup.py @@ -103,7 +103,7 @@ def get_version(version_file): "requests>=2.18.4", # This is necessary to support the python_requires kwarg "setuptools >= 24.2.0", - "sslyze>=2.1.3,<3.0.0", + "sslyze>=3.0.0", "wget>=3.2", ], extras_require={ diff --git a/src/pshtt/pshtt.py b/src/pshtt/pshtt.py index fff7fed..b49d6ea 100644 --- a/src/pshtt/pshtt.py +++ b/src/pshtt/pshtt.py @@ -3,6 +3,7 @@ # Standard Python Libraries import base64 import codecs +import datetime import json import logging import os @@ -18,16 +19,20 @@ # Unable to find type stubs for the sslyze package. import sslyze # type: ignore -from sslyze.server_connectivity_tester import ( # type: ignore - ServerConnectivityError, +from sslyze import ( # type: ignore + Scanner, ServerConnectivityTester, + ServerNetworkLocationViaDirectConnection, + ServerScanRequest, ) -import sslyze.synchronous_scanner # type: ignore +from sslyze.errors import ConnectionToServerFailed # type: ignore +from sslyze.plugins.scan_commands import ScanCommand # type: ignore import urllib3 from . import utils from .models import Domain, Endpoint + # We're going to be making requests with certificate validation # disabled. Commented next line due to pylint warning that urllib3 is # not in requests.packages @@ -643,24 +648,26 @@ def https_check(endpoint): # remove the https:// from prefix for sslyze try: hostname = endpoint.url[8:] - server_tester = ServerConnectivityTester(hostname=hostname, port=443) - server_info = server_tester.perform() + server_location = ServerNetworkLocationViaDirectConnection.with_ip_address_lookup( + hostname=hostname, port=443 + ) + server_tester = ServerConnectivityTester() + server_info = server_tester.perform(server_location) endpoint.live = True - ip = server_info.ip_address + ip = server_location.ip_address if endpoint.ip is None: endpoint.ip = ip else: if endpoint.ip != ip: - utils.debug( - "%s: Endpoint IP is already %s, but requests IP is %s.", + utils.debug("%s: Endpoint IP is already %s, but requests IP is %s.", endpoint.url, endpoint.ip, - ip, + ip ) - if server_info.client_auth_requirement.name == "REQUIRED": + if server_info.tls_probing_result.client_auth_requirement.name == 'REQUIRED': endpoint.https_client_auth_required = True logging.warning("%s: Client Authentication REQUIRED", endpoint.url) - except ServerConnectivityError as err: + except ConnectionToServerFailed as err: endpoint.live = False endpoint.https_valid = False logging.exception( @@ -680,18 +687,23 @@ def https_check(endpoint): try: cert_plugin_result = None - command = sslyze.plugins.certificate_info_plugin.CertificateInfoScanCommand( - ca_file=CA_FILE - ) - scanner = sslyze.synchronous_scanner.SynchronousScanner() - cert_plugin_result = scanner.run_scan_command(server_info, command) + command = ScanCommand.CERTIFICATE_INFO + scanner = Scanner() + scan_request = ServerScanRequest(server_info=server_info, scan_commands=[command]) + scanner.queue_scan(scan_request) + # Retrieve results from generator object + scan_result = [x for x in scanner.get_results()][0] + cert_plugin_result = scan_result.scan_commands_results[ScanCommand.CERTIFICATE_INFO] except Exception as err: try: if "timed out" in str(err): logging.exception( "%s: Retrying sslyze scanner certificate plugin.", endpoint.url ) - cert_plugin_result = scanner.run_scan_command(server_info, command) + scanner.queue_scan(scan_request) + # Retrieve results from generator object + scan_result = [x for x in scanner.get_results()][0] + cert_plugin_result = scan_result.scan_commands_results[ScanCommand.CERTIFICATE_INFO] else: logging.exception( "%s: Unknown exception in sslyze scanner certificate plugin.", @@ -719,7 +731,7 @@ def https_check(endpoint): public_trust = True custom_trust = True public_not_trusted_names = [] - validation_results = cert_plugin_result.path_validation_result_list + validation_results = cert_plugin_result.certificate_deployments[0].path_validation_results for result in validation_results: if result.was_validation_successful: # We're assuming that it is trusted to start with @@ -754,88 +766,52 @@ def https_check(endpoint): logging.exception("%s: Unknown exception examining trust.", endpoint.url) utils.debug("%s: Unknown exception examining trust: %s", endpoint.url, err) - try: - cert_response = cert_plugin_result.as_text() - except AttributeError: - logging.exception( - "%s: Known error in sslyze 1.X with EC public keys. See https://github.com/nabla-c0d3/sslyze/issues/215", - endpoint.url, - ) - return - except Exception as err: - endpoint.unknown_error = True - logging.exception("%s: Unknown exception in cert plugin.", endpoint.url) - utils.debug("%s: %s", endpoint.url, err) - return - - # Debugging - # for msg in cert_response: - # print(msg) - # Default endpoint assessments to False until proven True. endpoint.https_expired_cert = False endpoint.https_self_signed_cert = False endpoint.https_bad_chain = False endpoint.https_bad_hostname = False - # STORE will be either "Mozilla" or "Custom" - # depending on what the user chose. - - # A certificate can have multiple issues. - for msg in cert_response: - # Check for missing SAN. - if (("DNS Subject Alternative Names") in msg) and (("[]") in msg): - endpoint.https_bad_hostname = True - - # Check for certificate expiration. - if ( - (STORE in msg) - and (("FAILED") in msg) - and (("certificate has expired") in msg) - ): - endpoint.https_expired_cert = True + cert_chain = cert_plugin_result.certificate_deployments[0].received_certificate_chain + leaf_cert = cert_chain[0] + + # Check for leaf certificate expiration/self-signature. + if leaf_cert.not_valid_after < datetime.datetime.now(): + endpoint.https_expired_cert = True + + # Check to see if the cert is self-signed + if leaf_cert.issuer is leaf_cert.subject: + endpoint.https_self_signed_cert = True + + # Check certificate chain + # NOTE: If this is the only flag that's set, it's probably + # an incomplete chain + # If this isnt the only flag that is set, it's might be + # because there is another error. More debugging would + # need to be done at this point, but not through sslyze + # because sslyze doesn't have enough granularity + for cert in cert_chain[1:]: + # Check for certificate expiration + if cert.not_valid_after < datetime.datetime.now(): + endpoint.https_bad_chain = True # Check to see if the cert is self-signed - if ( - (STORE in msg) - and (("FAILED") in msg) - and (("self signed certificate") in msg) - ): - endpoint.https_self_signed_cert = True - - # Check to see if there is a bad chain - - # NOTE: If this is the only flag that's set, it's probably - # an incomplete chain - # If this isnt the only flag that is set, it's might be - # because there is another error. More debugging would - # need to be done at this point, but not through sslyze - # because sslyze doesn't have enough granularity - - if ( - (STORE in msg) - and (("FAILED") in msg) - and ( - (("unable to get local issuer certificate") in msg) - or (("self signed certificate") in msg) - ) - ): + if cert.issuer is (cert.subject or None): endpoint.https_bad_chain = True - # Check for whether the hostname validates. - if ( - (("Hostname Validation") in msg) - and (("FAILED") in msg) - and (("Certificate does NOT match") in msg) - ): - endpoint.https_bad_hostname = True + # If leaf certificate subject does NOT match hostname, bad hostname + # NOTE: Since sslyze 3.0.0, ever since JSON output for certinfo, + # SAN(s) are checked as part of _certificate_matches_hostname which + # called as part of leaf_certificate_subject_matches_hostname + if not cert_plugin_result.certificate_deployments[0].leaf_certificate_subject_matches_hostname: + endpoint.https_bad_hostname = True try: - endpoint.https_cert_chain_len = len( - cert_plugin_result.received_certificate_chain - ) - if endpoint.https_self_signed_cert is False and ( - endpoint.https_cert_chain_len < 2 + endpoint.https_cert_chain_len = len(cert_plugin_result.certificate_deployments[0].received_certificate_chain) + if ( + endpoint.https_self_signed_cert is False and ( + len(cert_plugin_result.certificate_deployments[0].received_certificate_chain) < 2 + ) ): # *** TODO check that it is not a bad hostname and that the root cert is trusted before suggesting that it is an intermediate cert issue. endpoint.https_missing_intermediate_cert = True From f4512a6638373f78a5f007010b0c8e99af46b966 Mon Sep 17 00:00:00 2001 From: Saptak S Date: Wed, 20 Jan 2021 02:00:02 +0530 Subject: [PATCH 02/15] Adds extra certificate informations for user trusted CA --- src/pshtt/pshtt.py | 43 +++++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/src/pshtt/pshtt.py b/src/pshtt/pshtt.py index b49d6ea..6b6cd64 100644 --- a/src/pshtt/pshtt.py +++ b/src/pshtt/pshtt.py @@ -17,8 +17,7 @@ from publicsuffixlist.update import updatePSL # type: ignore import requests -# Unable to find type stubs for the sslyze package. -import sslyze # type: ignore +from pathlib import Path # Python3 from sslyze import ( # type: ignore Scanner, ServerConnectivityTester, @@ -27,6 +26,7 @@ ) from sslyze.errors import ConnectionToServerFailed # type: ignore from sslyze.plugins.scan_commands import ScanCommand # type: ignore +from sslyze.plugins.certificate_info.implementation import CertificateInfoExtraArguments # type: ignore import urllib3 from . import utils @@ -687,9 +687,22 @@ def https_check(endpoint): try: cert_plugin_result = None - command = ScanCommand.CERTIFICATE_INFO scanner = Scanner() - scan_request = ServerScanRequest(server_info=server_info, scan_commands=[command]) + command = ScanCommand.CERTIFICATE_INFO + if CA_FILE is not None: + command_extra_args = { + ScanCommand.CERTIFICATE_INFO: CertificateInfoExtraArguments(custom_ca_file=Path(CA_FILE)) + } + scan_request = ServerScanRequest( + server_info=server_info, + scan_commands_extra_arguments=command_extra_args, + scan_commands=[command] + ) + else: + scan_request = ServerScanRequest( + server_info=server_info, + scan_commands=[command] + ) scanner.queue_scan(scan_request) # Retrieve results from generator object scan_result = [x for x in scanner.get_results()][0] @@ -830,13 +843,23 @@ def https_check(endpoint): if PT_INT_CA_FILE is not None: try: cert_plugin_result = None - command = sslyze.plugins.certificate_info_plugin.CertificateInfoScanCommand( - ca_file=PT_INT_CA_FILE - ) - cert_plugin_result = scanner.run_scan_command( - server_info, command + scanner = Scanner() + command = ScanCommand.CERTIFICATE_INFO + command_extra_args = { + ScanCommand.CERTIFICATE_INFO: CertificateInfoExtraArguments( + custom_ca_file=Path(PT_INT_CA_FILE) + ) + } + scan_request = ServerScanRequest( + server_info=server_info, + scan_commands_extra_arguments=command_extra_args, + scan_commands=[command] ) - if cert_plugin_result.verified_certificate_chain is not None: + scanner.queue_scan(scan_request) + # Retrieve results from generator object + scan_result = [x for x in scanner.get_results()][0] + cert_plugin_result = scan_result.scan_commands_results[ScanCommand.CERTIFICATE_INFO] + if cert_plugin_result.certificate_deployments[0].verified_certificate_chain is not None: public_trust = True endpoint.https_public_trusted = public_trust logging.warning( From 81882c488e099cd96e1525e62e275cf7759f506f Mon Sep 17 00:00:00 2001 From: Saptak S Date: Tue, 26 Jan 2021 23:56:16 +0530 Subject: [PATCH 03/15] Fixes typo in comment --- src/pshtt/pshtt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pshtt/pshtt.py b/src/pshtt/pshtt.py index 6b6cd64..1c637cf 100644 --- a/src/pshtt/pshtt.py +++ b/src/pshtt/pshtt.py @@ -799,7 +799,7 @@ def https_check(endpoint): # Check certificate chain # NOTE: If this is the only flag that's set, it's probably # an incomplete chain - # If this isnt the only flag that is set, it's might be + # If this isn't the only flag that is set, it might be # because there is another error. More debugging would # need to be done at this point, but not through sslyze # because sslyze doesn't have enough granularity From bd6957c7e94858d3d2e834b9efc39775e5c3f6f4 Mon Sep 17 00:00:00 2001 From: Saptak Sengupta Date: Wed, 3 Feb 2021 11:23:11 +0530 Subject: [PATCH 04/15] Add fixes and suggestions from code review - fix comments - use == instead of is Co-authored-by: Nick M. <50747025+mcdonnnj@users.noreply.github.com> --- src/pshtt/pshtt.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/pshtt/pshtt.py b/src/pshtt/pshtt.py index 1c637cf..ed0e54f 100644 --- a/src/pshtt/pshtt.py +++ b/src/pshtt/pshtt.py @@ -691,7 +691,7 @@ def https_check(endpoint): command = ScanCommand.CERTIFICATE_INFO if CA_FILE is not None: command_extra_args = { - ScanCommand.CERTIFICATE_INFO: CertificateInfoExtraArguments(custom_ca_file=Path(CA_FILE)) + command: CertificateInfoExtraArguments(custom_ca_file=Path(CA_FILE)) } scan_request = ServerScanRequest( server_info=server_info, @@ -714,7 +714,7 @@ def https_check(endpoint): "%s: Retrying sslyze scanner certificate plugin.", endpoint.url ) scanner.queue_scan(scan_request) - # Retrieve results from generator object + # Consume the generator object and retrieve the first result scan_result = [x for x in scanner.get_results()][0] cert_plugin_result = scan_result.scan_commands_results[ScanCommand.CERTIFICATE_INFO] else: @@ -793,7 +793,7 @@ def https_check(endpoint): endpoint.https_expired_cert = True # Check to see if the cert is self-signed - if leaf_cert.issuer is leaf_cert.subject: + if leaf_cert.issuer == leaf_cert.subject: endpoint.https_self_signed_cert = True # Check certificate chain @@ -809,7 +809,7 @@ def https_check(endpoint): endpoint.https_bad_chain = True # Check to see if the cert is self-signed - if cert.issuer is (cert.subject or None): + if cert.issuer == (cert.subject or None): endpoint.https_bad_chain = True # If leaf certificate subject does NOT match hostname, bad hostname @@ -823,7 +823,7 @@ def https_check(endpoint): endpoint.https_cert_chain_len = len(cert_plugin_result.certificate_deployments[0].received_certificate_chain) if ( endpoint.https_self_signed_cert is False and ( - len(cert_plugin_result.certificate_deployments[0].received_certificate_chain) < 2 + endpoint.https_cert_chain_len < 2 ) ): # *** TODO check that it is not a bad hostname and that the root cert is trusted before suggesting that it is an intermediate cert issue. @@ -846,7 +846,7 @@ def https_check(endpoint): scanner = Scanner() command = ScanCommand.CERTIFICATE_INFO command_extra_args = { - ScanCommand.CERTIFICATE_INFO: CertificateInfoExtraArguments( + command: CertificateInfoExtraArguments( custom_ca_file=Path(PT_INT_CA_FILE) ) } @@ -856,7 +856,7 @@ def https_check(endpoint): scan_commands=[command] ) scanner.queue_scan(scan_request) - # Retrieve results from generator object + # Consume the generator object and retrieve the first result scan_result = [x for x in scanner.get_results()][0] cert_plugin_result = scan_result.scan_commands_results[ScanCommand.CERTIFICATE_INFO] if cert_plugin_result.certificate_deployments[0].verified_certificate_chain is not None: From 4a3207f8f7495adf06f5cfb515a3ed9f43e637b7 Mon Sep 17 00:00:00 2001 From: Saptak S Date: Thu, 4 Feb 2021 03:22:19 +0530 Subject: [PATCH 05/15] Consider all certificate deployments instead of first one - consider all deployments and if all validations succeed, then consider it trusted - use path_validation_results instead of received_certificate_chain to consider if STORE is included in trust_store - check self signed validation in cert_chain till the second last cert since the last cert is root cert and hence is self signed anyways - put all logic inside one loop over path_validation_results --- src/pshtt/pshtt.py | 123 +++++++++++++++++++++++++-------------------- 1 file changed, 68 insertions(+), 55 deletions(-) diff --git a/src/pshtt/pshtt.py b/src/pshtt/pshtt.py index ed0e54f..7a3922c 100644 --- a/src/pshtt/pshtt.py +++ b/src/pshtt/pshtt.py @@ -741,20 +741,66 @@ def https_check(endpoint): return try: + # Default endpoint assessments to False until proven True. + endpoint.https_expired_cert = False + endpoint.https_self_signed_cert = False + endpoint.https_bad_chain = False + endpoint.https_bad_hostname = False + + # Default trust to Fase until proven True public_trust = True custom_trust = True public_not_trusted_names = [] - validation_results = cert_plugin_result.certificate_deployments[0].path_validation_results - for result in validation_results: - if result.was_validation_successful: - # We're assuming that it is trusted to start with - pass - else: - if "Custom" in result.trust_store.name: - custom_trust = False + for certificate_deployment in cert_plugin_result.certificate_deployments: + validation_results = certificate_deployment.path_validation_results + for result in validation_results: + if result.was_validation_successful: + # We're assuming that it is trusted to start with + pass else: - public_trust = False - public_not_trusted_names.append(result.trust_store.name) + if 'Custom' in result.trust_store.name: + custom_trust = False + else: + public_trust = False + public_not_trusted_names.append(result.trust_store.name) + + if STORE in result.trust_store.name: + cert_chain = result.verified_certificate_chain + leaf_cert = cert_chain[0] + + # Check for leaf certificate expiration/self-signature. + if leaf_cert.not_valid_after < datetime.datetime.now(): + endpoint.https_expired_cert = True + + # Check to see if the cert is self-signed + if leaf_cert.issuer == leaf_cert.subject: + endpoint.https_self_signed_cert = True + + # Check certificate chain till the second last element + # The last cert being the root cert is self signed and + # hence the self signed check is not valid + # NOTE: If this is the only flag that's set, it's probably + # an incomplete chain + # If this isn't the only flag that is set, it might be + # because there is another error. More debugging would + # need to be done at this point, but not through sslyze + # because sslyze doesn't have enough granularity + for cert in cert_chain[:-1]: + # Check for certificate expiration + if cert.not_valid_after < datetime.datetime.now(): + endpoint.https_bad_chain = True + + # Check to see if the cert is self-signed + if cert.issuer == cert.subject or not cert.issuer: + endpoint.https_bad_chain = True + + # If leaf certificate subject does NOT match hostname, bad hostname + # NOTE: Since sslyze 3.0.0, ever since JSON output for certinfo, + # SAN(s) are checked as part of _certificate_matches_hostname which + # called as part of leaf_certificate_subject_matches_hostname + if not certificate_deployment.leaf_certificate_subject_matches_hostname: + endpoint.https_bad_hostname = True + if public_trust: logging.warning( "%s: Publicly trusted by common trust stores.", endpoint.url @@ -776,48 +822,15 @@ def https_check(endpoint): endpoint.https_custom_trusted = custom_trust except Exception as err: # Ignore exception - logging.exception("%s: Unknown exception examining trust.", endpoint.url) - utils.debug("%s: Unknown exception examining trust: %s", endpoint.url, err) - - # Default endpoint assessments to False until proven True. - endpoint.https_expired_cert = False - endpoint.https_self_signed_cert = False - endpoint.https_bad_chain = False - endpoint.https_bad_hostname = False - - cert_chain = cert_plugin_result.certificate_deployments[0].received_certificate_chain - leaf_cert = cert_chain[0] - - # Check for leaf certificate expiration/self-signature. - if leaf_cert.not_valid_after < datetime.datetime.now(): - endpoint.https_expired_cert = True - - # Check to see if the cert is self-signed - if leaf_cert.issuer == leaf_cert.subject: - endpoint.https_self_signed_cert = True - - # Check certificate chain - # NOTE: If this is the only flag that's set, it's probably - # an incomplete chain - # If this isn't the only flag that is set, it might be - # because there is another error. More debugging would - # need to be done at this point, but not through sslyze - # because sslyze doesn't have enough granularity - for cert in cert_chain[1:]: - # Check for certificate expiration - if cert.not_valid_after < datetime.datetime.now(): - endpoint.https_bad_chain = True - - # Check to see if the cert is self-signed - if cert.issuer == (cert.subject or None): - endpoint.https_bad_chain = True - - # If leaf certificate subject does NOT match hostname, bad hostname - # NOTE: Since sslyze 3.0.0, ever since JSON output for certinfo, - # SAN(s) are checked as part of _certificate_matches_hostname which - # called as part of leaf_certificate_subject_matches_hostname - if not cert_plugin_result.certificate_deployments[0].leaf_certificate_subject_matches_hostname: - endpoint.https_bad_hostname = True + logging.exception( + "%s: Unknown exception examining certificate deployment.", + endpoint.url + ) + utils.debug( + "%s: Unknown exception examining certificate deployment: %s", + endpoint.url, + err + ) try: endpoint.https_cert_chain_len = len(cert_plugin_result.certificate_deployments[0].received_certificate_chain) @@ -831,12 +844,12 @@ def https_check(endpoint): if cert_plugin_result.verified_certificate_chain is None: logging.warning( "%s: Untrusted certificate chain, probably due to missing intermediate certificate.", - endpoint.url, + endpoint.url ) utils.debug( - "%s: Only %d certificates in certificate chain received.", + "%s: Only {} certificates in certificate chain received.", endpoint.url, - cert_plugin_result.received_certificate_chain.__len__(), + cert_plugin_result.received_certificate_chain.__len__() ) elif custom_trust is True and public_trust is False: # recheck public trust using custom public trust store with manually added intermediate certificates From 35047c897b938c4bbfa0ad9d90967ed2386f5d59 Mon Sep 17 00:00:00 2001 From: Saptak S Date: Thu, 18 Mar 2021 02:01:45 +0530 Subject: [PATCH 06/15] Consider all certificate deployments for verified certificate chain --- src/pshtt/pshtt.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/pshtt/pshtt.py b/src/pshtt/pshtt.py index 7a3922c..9fd8008 100644 --- a/src/pshtt/pshtt.py +++ b/src/pshtt/pshtt.py @@ -673,7 +673,7 @@ def https_check(endpoint): logging.exception( "%s: Error in sslyze server connectivity check when connecting to %s", endpoint.url, - err.server_info.hostname, + err.server_location.hostname, ) utils.debug("%s: %s", endpoint.url, err) return @@ -833,7 +833,9 @@ def https_check(endpoint): ) try: - endpoint.https_cert_chain_len = len(cert_plugin_result.certificate_deployments[0].received_certificate_chain) + endpoint.https_cert_chain_len = 0 + for certificate_deployment in cert_plugin_result.certificate_deployments: + endpoint.https_cert_chain_len += len(certificate_deployment.received_certificate_chain) if ( endpoint.https_self_signed_cert is False and ( endpoint.https_cert_chain_len < 2 @@ -841,15 +843,19 @@ def https_check(endpoint): ): # *** TODO check that it is not a bad hostname and that the root cert is trusted before suggesting that it is an intermediate cert issue. endpoint.https_missing_intermediate_cert = True - if cert_plugin_result.verified_certificate_chain is None: + has_verfied_cert_chain = True + for certificate_deployment in cert_plugin_result.certificate_deployments: + if certificate_deployment.verified_certificate_chain is None: + has_verfied_cert_chain = False + if not has_verfied_cert_chain: logging.warning( "%s: Untrusted certificate chain, probably due to missing intermediate certificate.", endpoint.url ) utils.debug( - "%s: Only {} certificates in certificate chain received.", + "%s: Only %s certificates in certificate chain received.", endpoint.url, - cert_plugin_result.received_certificate_chain.__len__() + endpoint.https_cert_chain_len ) elif custom_trust is True and public_trust is False: # recheck public trust using custom public trust store with manually added intermediate certificates @@ -872,7 +878,11 @@ def https_check(endpoint): # Consume the generator object and retrieve the first result scan_result = [x for x in scanner.get_results()][0] cert_plugin_result = scan_result.scan_commands_results[ScanCommand.CERTIFICATE_INFO] - if cert_plugin_result.certificate_deployments[0].verified_certificate_chain is not None: + has_verfied_cert_chain = True + for certificate_deployment in cert_plugin_result.certificate_deployments: + if certificate_deployment.verified_certificate_chain is None: + has_verfied_cert_chain = False + if has_verfied_cert_chain: public_trust = True endpoint.https_public_trusted = public_trust logging.warning( From f11b04c0742ff34405f3571c7a01551b94bb0a20 Mon Sep 17 00:00:00 2001 From: Saptak S Date: Thu, 3 Nov 2022 19:11:50 +0530 Subject: [PATCH 07/15] Updates sslyze version to <5.0.0, upgrade required python to >=3.7 --- setup.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/setup.py b/setup.py index cd20722..085c9a2 100644 --- a/setup.py +++ b/setup.py @@ -77,16 +77,13 @@ def get_version(version_file): "Programming Language :: Python :: 3 :: Only", "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", - # "Programming Language :: Python :: 3.8", - # "Programming Language :: Python :: 3.9", - # "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.8", + "Programming Language :: Python :: 3.9", + "Programming Language :: Python :: 3.10", # "Programming Language :: Python :: 3.11", "Programming Language :: Python :: Implementation :: CPython", ], - # The versions of nassl pinned by our sslyze version constraint only have - # bdists available for cp36 and cp37 on PyPI so we can only support Python - # 3.6 and 3.7 at this time. - python_requires=">=3.6, <3.8", + python_requires=">=3.7", # What does your project relate to? keywords="https best practices", packages=find_packages(where="src"), @@ -103,7 +100,7 @@ def get_version(version_file): "requests>=2.18.4", # This is necessary to support the python_requires kwarg "setuptools >= 24.2.0", - "sslyze>=3.0.0", + "sslyze>=3.0.0,<5.0.0", "wget>=3.2", ], extras_require={ From 822a381bb94c3066b10d52ce0424048c2b180e99 Mon Sep 17 00:00:00 2001 From: Saptak S Date: Thu, 3 Nov 2022 19:12:30 +0530 Subject: [PATCH 08/15] Lints the python code --- src/pshtt/pshtt.py | 78 +++++++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/src/pshtt/pshtt.py b/src/pshtt/pshtt.py index 9fd8008..2031a2f 100644 --- a/src/pshtt/pshtt.py +++ b/src/pshtt/pshtt.py @@ -7,6 +7,7 @@ import json import logging import os +from pathlib import Path # Python3 import re import sys from urllib import parse as urlparse @@ -16,8 +17,6 @@ from publicsuffixlist.compat import PublicSuffixList # type: ignore from publicsuffixlist.update import updatePSL # type: ignore import requests - -from pathlib import Path # Python3 from sslyze import ( # type: ignore Scanner, ServerConnectivityTester, @@ -25,14 +24,15 @@ ServerScanRequest, ) from sslyze.errors import ConnectionToServerFailed # type: ignore +from sslyze.plugins.certificate_info.implementation import ( # type: ignore + CertificateInfoExtraArguments, +) from sslyze.plugins.scan_commands import ScanCommand # type: ignore -from sslyze.plugins.certificate_info.implementation import CertificateInfoExtraArguments # type: ignore import urllib3 from . import utils from .models import Domain, Endpoint - # We're going to be making requests with certificate validation # disabled. Commented next line due to pylint warning that urllib3 is # not in requests.packages @@ -648,8 +648,10 @@ def https_check(endpoint): # remove the https:// from prefix for sslyze try: hostname = endpoint.url[8:] - server_location = ServerNetworkLocationViaDirectConnection.with_ip_address_lookup( - hostname=hostname, port=443 + server_location = ( + ServerNetworkLocationViaDirectConnection.with_ip_address_lookup( + hostname=hostname, port=443 + ) ) server_tester = ServerConnectivityTester() server_info = server_tester.perform(server_location) @@ -659,12 +661,13 @@ def https_check(endpoint): endpoint.ip = ip else: if endpoint.ip != ip: - utils.debug("%s: Endpoint IP is already %s, but requests IP is %s.", + utils.debug( + "%s: Endpoint IP is already %s, but requests IP is %s.", endpoint.url, endpoint.ip, - ip + ip, ) - if server_info.tls_probing_result.client_auth_requirement.name == 'REQUIRED': + if server_info.tls_probing_result.client_auth_requirement.name == "REQUIRED": endpoint.https_client_auth_required = True logging.warning("%s: Client Authentication REQUIRED", endpoint.url) except ConnectionToServerFailed as err: @@ -696,17 +699,18 @@ def https_check(endpoint): scan_request = ServerScanRequest( server_info=server_info, scan_commands_extra_arguments=command_extra_args, - scan_commands=[command] + scan_commands=[command], ) else: scan_request = ServerScanRequest( - server_info=server_info, - scan_commands=[command] + server_info=server_info, scan_commands=[command] ) scanner.queue_scan(scan_request) # Retrieve results from generator object scan_result = [x for x in scanner.get_results()][0] - cert_plugin_result = scan_result.scan_commands_results[ScanCommand.CERTIFICATE_INFO] + cert_plugin_result = scan_result.scan_commands_results[ + ScanCommand.CERTIFICATE_INFO + ] except Exception as err: try: if "timed out" in str(err): @@ -716,7 +720,9 @@ def https_check(endpoint): scanner.queue_scan(scan_request) # Consume the generator object and retrieve the first result scan_result = [x for x in scanner.get_results()][0] - cert_plugin_result = scan_result.scan_commands_results[ScanCommand.CERTIFICATE_INFO] + cert_plugin_result = scan_result.scan_commands_results[ + ScanCommand.CERTIFICATE_INFO + ] else: logging.exception( "%s: Unknown exception in sslyze scanner certificate plugin.", @@ -758,7 +764,7 @@ def https_check(endpoint): # We're assuming that it is trusted to start with pass else: - if 'Custom' in result.trust_store.name: + if "Custom" in result.trust_store.name: custom_trust = False else: public_trust = False @@ -798,7 +804,9 @@ def https_check(endpoint): # NOTE: Since sslyze 3.0.0, ever since JSON output for certinfo, # SAN(s) are checked as part of _certificate_matches_hostname which # called as part of leaf_certificate_subject_matches_hostname - if not certificate_deployment.leaf_certificate_subject_matches_hostname: + if ( + not certificate_deployment.leaf_certificate_subject_matches_hostname + ): endpoint.https_bad_hostname = True if public_trust: @@ -823,23 +831,22 @@ def https_check(endpoint): except Exception as err: # Ignore exception logging.exception( - "%s: Unknown exception examining certificate deployment.", - endpoint.url + "%s: Unknown exception examining certificate deployment.", endpoint.url ) utils.debug( "%s: Unknown exception examining certificate deployment: %s", endpoint.url, - err + err, ) try: endpoint.https_cert_chain_len = 0 for certificate_deployment in cert_plugin_result.certificate_deployments: - endpoint.https_cert_chain_len += len(certificate_deployment.received_certificate_chain) - if ( - endpoint.https_self_signed_cert is False and ( - endpoint.https_cert_chain_len < 2 - ) + endpoint.https_cert_chain_len += len( + certificate_deployment.received_certificate_chain + ) + if endpoint.https_self_signed_cert is False and ( + endpoint.https_cert_chain_len < 2 ): # *** TODO check that it is not a bad hostname and that the root cert is trusted before suggesting that it is an intermediate cert issue. endpoint.https_missing_intermediate_cert = True @@ -850,12 +857,12 @@ def https_check(endpoint): if not has_verfied_cert_chain: logging.warning( "%s: Untrusted certificate chain, probably due to missing intermediate certificate.", - endpoint.url + endpoint.url, ) utils.debug( "%s: Only %s certificates in certificate chain received.", endpoint.url, - endpoint.https_cert_chain_len + endpoint.https_cert_chain_len, ) elif custom_trust is True and public_trust is False: # recheck public trust using custom public trust store with manually added intermediate certificates @@ -866,21 +873,28 @@ def https_check(endpoint): command = ScanCommand.CERTIFICATE_INFO command_extra_args = { command: CertificateInfoExtraArguments( - custom_ca_file=Path(PT_INT_CA_FILE) - ) + custom_ca_file=Path(PT_INT_CA_FILE) + ) } scan_request = ServerScanRequest( server_info=server_info, scan_commands_extra_arguments=command_extra_args, - scan_commands=[command] + scan_commands=[command], ) scanner.queue_scan(scan_request) # Consume the generator object and retrieve the first result scan_result = [x for x in scanner.get_results()][0] - cert_plugin_result = scan_result.scan_commands_results[ScanCommand.CERTIFICATE_INFO] + cert_plugin_result = scan_result.scan_commands_results[ + ScanCommand.CERTIFICATE_INFO + ] has_verfied_cert_chain = True - for certificate_deployment in cert_plugin_result.certificate_deployments: - if certificate_deployment.verified_certificate_chain is None: + for ( + certificate_deployment + ) in cert_plugin_result.certificate_deployments: + if ( + certificate_deployment.verified_certificate_chain + is None + ): has_verfied_cert_chain = False if has_verfied_cert_chain: public_trust = True From 67cfaf920069d7fe93b1c27c050f969295f73464 Mon Sep 17 00:00:00 2001 From: Saptak S Date: Mon, 7 Nov 2022 22:10:31 +0530 Subject: [PATCH 09/15] Updates python versions in github workflow build --- .github/workflows/build.yml | 34 ++++++---------------------------- 1 file changed, 6 insertions(+), 28 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3a0ca03..96a90ee 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -115,24 +115,12 @@ jobs: matrix: os: - ubuntu-latest - # The versions of nassl pinned by our sslyze version constraint only - # have bdists available for Python 3.6 and 3.7, so we can only support - # those versions of Python. The error seen when trying to install on - # Python 3.8+ is: - # ERROR: Cannot install pshtt because these package versions have - # conflicting dependencies. - # The conflict is caused by: - # sslyze 2.1.4 depends on nassl<2.3.0 and >=2.2.0 - # sslyze 2.1.3 depends on nassl<2.3.0 and >=2.2.0 python-version: - "3.7" - # - "3.8" - # - "3.9" - # - "3.10" + - "3.8" + - "3.9" + - "3.10" # - "3.11" - include: - - os: ubuntu-20.04 - python-version: "3.6" steps: - uses: actions/checkout@v3 - id: setup-python @@ -224,20 +212,10 @@ jobs: - ubuntu-latest python-version: - "3.7" - # Disabled due to an unresolvable dependency issue between sslyze and - # nassl: - # ERROR: Cannot install pshtt because these package versions have - # conflicting dependencies. - # The conflict is caused by: - # sslyze 2.1.4 depends on nassl<2.3.0 and >=2.2.0 - # sslyze 2.1.3 depends on nassl<2.3.0 and >=2.2.0 - # - "3.8" - # - "3.9" - # - "3.10" + - "3.8" + - "3.9" + - "3.10" # - "3.11" - include: - - os: ubuntu-20.04 - python-version: "3.6" steps: - uses: actions/checkout@v3 - id: setup-python From aae0882f221a001a6f437987843b7e37f19987a9 Mon Sep 17 00:00:00 2001 From: Saptak S Date: Tue, 8 Nov 2022 00:48:10 +0530 Subject: [PATCH 10/15] Updates python version in build.yml --- .github/workflows/build.yml | 21 +++++---------------- setup.py | 1 - 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 96a90ee..5dbaf3d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -23,9 +23,7 @@ jobs: - id: setup-python uses: actions/setup-python@v4 with: - # A lower version is used because of a dependency issue in Python - # versions 3.8-3.11 - python-version: "3.7" + python-version: "3.10" # We need the Go version and Go cache location for the actions/cache step, # so the Go installation must happen before that. - id: setup-go @@ -170,9 +168,7 @@ jobs: - id: setup-python uses: actions/setup-python@v4 with: - # A lower version is used because of a dependency issue in Python - # versions 3.8-3.11 - python-version: "3.7" + python-version: "3.10" - uses: actions/cache@v3 env: BASE_CACHE_KEY: "${{ github.job }}-${{ runner.os }}-\ @@ -262,16 +258,9 @@ jobs: - ubuntu-latest python-version: - "3.7" - # Disabled due to an unresolvable dependency issue between sslyze and - # nassl: - # ERROR: Cannot install pshtt because these package versions have - # conflicting dependencies. - # The conflict is caused by: - # sslyze 2.1.4 depends on nassl<2.3.0 and >=2.2.0 - # sslyze 2.1.3 depends on nassl<2.3.0 and >=2.2.0 - # - "3.8" - # - "3.9" - # - "3.10" + - "3.8" + - "3.9" + - "3.10" # - "3.11" include: - os: ubuntu-20.04 diff --git a/setup.py b/setup.py index 085c9a2..4bb7498 100644 --- a/setup.py +++ b/setup.py @@ -75,7 +75,6 @@ def get_version(version_file): # that you indicate whether you support Python 2, Python 3 or both. "Programming Language :: Python :: 3", "Programming Language :: Python :: 3 :: Only", - "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", From 2e4fb16a3e10e93dbe40df7fa32addbaac6d16ea Mon Sep 17 00:00:00 2001 From: Saptak S Date: Fri, 26 May 2023 21:08:07 +0530 Subject: [PATCH 11/15] Removes Python 3.6 from build --- .github/workflows/build.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5dbaf3d..fc8b10c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -262,9 +262,6 @@ jobs: - "3.9" - "3.10" # - "3.11" - include: - - os: ubuntu-20.04 - python-version: "3.6" steps: - uses: actions/checkout@v3 - id: setup-python From 6aec862b84acc66361f0f147920b73ad17da2443 Mon Sep 17 00:00:00 2001 From: Felddy Date: Mon, 16 Oct 2023 14:33:29 -0400 Subject: [PATCH 12/15] Fix boolean typo in comment --- src/pshtt/pshtt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pshtt/pshtt.py b/src/pshtt/pshtt.py index 2031a2f..637807f 100644 --- a/src/pshtt/pshtt.py +++ b/src/pshtt/pshtt.py @@ -753,7 +753,7 @@ def https_check(endpoint): endpoint.https_bad_chain = False endpoint.https_bad_hostname = False - # Default trust to Fase until proven True + # Default trust to False until proven True public_trust = True custom_trust = True public_not_trusted_names = [] From 35b73be1a3e5283484616240182027e23c85d888 Mon Sep 17 00:00:00 2001 From: Felddy Date: Mon, 16 Oct 2023 14:35:34 -0400 Subject: [PATCH 13/15] Bump version from 0.6.10 to 0.7.0 --- src/pshtt/_version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pshtt/_version.py b/src/pshtt/_version.py index fae5dfb..de7f1a2 100644 --- a/src/pshtt/_version.py +++ b/src/pshtt/_version.py @@ -1,2 +1,2 @@ """This file defines the version of this module.""" -__version__ = "0.6.10" +__version__ = "0.7.0" From 3005fc5ff8719e9acae75d388a7ccbf7e39a1e1c Mon Sep 17 00:00:00 2001 From: Felddy Date: Mon, 16 Oct 2023 14:50:21 -0400 Subject: [PATCH 14/15] Remove Python requirements from README This is documented in setup.py. No one will be amazed that it doesn't support Python 2. --- README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/README.md b/README.md index 2e41ba6..3eaa20a 100644 --- a/README.md +++ b/README.md @@ -30,8 +30,6 @@ organizations](https://github.com/cisagov/pshtt/graphs/contributors). ## Getting started ## -`pshtt` requires **Python 3.6 or 3.7**. Python 2 is not supported. - `pshtt` can be installed as a module, or run directly from the repository. From 234369071b8f74564431316f4260d1fc6ee31317 Mon Sep 17 00:00:00 2001 From: David Redmin Date: Mon, 16 Oct 2023 15:06:52 -0400 Subject: [PATCH 15/15] Fix a typo in a variable name --- src/pshtt/pshtt.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/pshtt/pshtt.py b/src/pshtt/pshtt.py index 637807f..6ca76b6 100644 --- a/src/pshtt/pshtt.py +++ b/src/pshtt/pshtt.py @@ -850,11 +850,11 @@ def https_check(endpoint): ): # *** TODO check that it is not a bad hostname and that the root cert is trusted before suggesting that it is an intermediate cert issue. endpoint.https_missing_intermediate_cert = True - has_verfied_cert_chain = True + has_verified_cert_chain = True for certificate_deployment in cert_plugin_result.certificate_deployments: if certificate_deployment.verified_certificate_chain is None: - has_verfied_cert_chain = False - if not has_verfied_cert_chain: + has_verified_cert_chain = False + if not has_verified_cert_chain: logging.warning( "%s: Untrusted certificate chain, probably due to missing intermediate certificate.", endpoint.url, @@ -887,7 +887,7 @@ def https_check(endpoint): cert_plugin_result = scan_result.scan_commands_results[ ScanCommand.CERTIFICATE_INFO ] - has_verfied_cert_chain = True + has_verified_cert_chain = True for ( certificate_deployment ) in cert_plugin_result.certificate_deployments: @@ -895,8 +895,8 @@ def https_check(endpoint): certificate_deployment.verified_certificate_chain is None ): - has_verfied_cert_chain = False - if has_verfied_cert_chain: + has_verified_cert_chain = False + if has_verified_cert_chain: public_trust = True endpoint.https_public_trusted = public_trust logging.warning(