From 7e76016b0e4522207a526edcf8d60ab04eb1f7fd Mon Sep 17 00:00:00 2001 From: Leo McArdle Date: Thu, 25 Jan 2024 11:20:42 +0000 Subject: [PATCH] feat: remove hpkp tests (#521) --- README.md | 2 +- httpobs/docs/api.md | 15 --- httpobs/docs/scoring.md | 11 -- httpobs/scanner/analyzer/__init__.py | 2 - httpobs/scanner/analyzer/headers.py | 88 +-------------- httpobs/scanner/analyzer/utils.py | 19 ---- httpobs/scanner/grader/grade.py | 31 ------ httpobs/scanner/scanner.py | 2 +- httpobs/scripts/scan.py | 4 +- httpobs/tests/unittests/test_grades.py | 6 +- httpobs/tests/unittests/test_headers.py | 138 +----------------------- httpobs/tests/unittests/test_preload.py | 32 +----- 12 files changed, 9 insertions(+), 341 deletions(-) diff --git a/README.md b/README.md index d2e77252..c4bcbf2c 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,7 @@ $ poetry install path='/foo/bar', # don't scan /, instead scan /foo/bar cookies={'foo': 'bar'}, # set the "foo" cookie to "bar" headers={'X-Foo': 'bar'}, # send an X-Foo: bar HTTP header - verify=False) # treat self-signed certs as valid for tests like HSTS/HPKP + verify=False) # treat self-signed certs as valid for tests like HSTS ``` ### The same, but with the local CLI diff --git a/httpobs/docs/api.md b/httpobs/docs/api.md index ce1c9046..582b505f 100644 --- a/httpobs/docs/api.md +++ b/httpobs/docs/api.md @@ -394,21 +394,6 @@ Example: "score_description": "Content is not visible via cross-origin resource sharing (CORS) files or headers", "score_modifier": 0 }, - "public-key-pinning": { - "expectation": "hpkp-not-implemented", - "name": "public-key-pinning", - "output": { - "data": null, - "includeSubDomains": false, - "max-age": null, - "numPins": null, - "preloaded": false - }, - "pass": true, - "result": "hpkp-not-implemented", - "score_description": "HTTP Public Key Pinning (HPKP) header not implemented", - "score_modifier": 0 - }, "redirection": { "expectation": "redirection-to-https", "name": "redirection", diff --git a/httpobs/docs/scoring.md b/httpobs/docs/scoring.md index f6d5483e..2c5b8494 100644 --- a/httpobs/docs/scoring.md +++ b/httpobs/docs/scoring.md @@ -72,17 +72,6 @@ csp-not-implemented | Content Security Policy (CSP) header not implemented | -25 csp-header-invalid | Content Security Policy (CSP) header cannot be parsed successfully | -25
-[HTTP Public Key Pinning](https://infosec.mozilla.org/guidelines/web_security#http-public-key-pinning) | Description | Modifier ---- | --- | :---: -hpkp-preloaded | Preloaded via the HTTP Public Key Pinning (HPKP) preloading process | 0 -hpkp-implemented-
max-age-at-least-fifteen-days | HTTP Public Key Pinning (HPKP) header set to a minimum of 15 days (1296000) | 0 -hpkp-implemented-
max-age-less-than-fifteen-days | HTTP Public Key Pinning (HPKP) header set to less than 15 days (1296000) | 0 -hpkp-not-implemented | HTTP Public Key Pinning (HPKP) header not implemented | 0 -hpkp-invalid-cert | HTTP Public Key Pinning (HPKP) header cannot be set, as site contains an invalid certificate chain | 0 -hpkp-not-implemented-no-https | HTTP Public Key Pinning (HPKP) header can't be implemented without https | 0 -hpkp-header-invalid | HTTP Public Key Pinning (HPKP) header cannot be recognized | -5 -
- [HTTP Strict Transport Security](https://infosec.mozilla.org/guidelines/web_security#http-strict-transport-security) | Description | Modifier --- | --- | :---: hsts-preloaded | Preloaded via the HTTP Strict Transport Security (HSTS) preloading process | 5 diff --git a/httpobs/scanner/analyzer/__init__.py b/httpobs/scanner/analyzer/__init__.py index 7f3e6234..a3e7d1a6 100644 --- a/httpobs/scanner/analyzer/__init__.py +++ b/httpobs/scanner/analyzer/__init__.py @@ -2,7 +2,6 @@ from .headers import ( content_security_policy, cookies, - public_key_pinning, referrer_policy, strict_transport_security, x_content_type_options, @@ -18,7 +17,6 @@ cookies, contribute, cross_origin_resource_sharing, - public_key_pinning, redirection, referrer_policy, strict_transport_security, diff --git a/httpobs/scanner/analyzer/headers.py b/httpobs/scanner/analyzer/headers.py index d9ed6803..17fcc8d0 100644 --- a/httpobs/scanner/analyzer/headers.py +++ b/httpobs/scanner/analyzer/headers.py @@ -3,7 +3,7 @@ from urllib.parse import urlparse, urlunparse from httpobs.scanner.analyzer.decorators import scored_test -from httpobs.scanner.analyzer.utils import is_hpkp_preloaded, is_hsts_preloaded, only_if_worse +from httpobs.scanner.analyzer.utils import is_hsts_preloaded, only_if_worse from httpobs.scanner.retriever import get_duplicate_header_values # Ignore the CloudFlare __cfduid tracking cookies. They *are* actually bad, but it is out of a site's @@ -488,92 +488,6 @@ def cookies(reqs: dict, expectation='cookies-secure-with-httponly-sessions') -> return output -@scored_test -def public_key_pinning(reqs: dict, expectation='hpkp-not-implemented') -> dict: - """ - :param reqs: dictionary containing all the request and response objects - :param expectation: test expectation; possible results: - hpkp-not-implemented-no-https - hpkp-not-implemented - hpkp-implemented-max-age-less-than-fifteen-days - hpkp-implemented-max-age-at-least-fifteen-days - hpkp-preloaded - hpkp-header-invalid - hpkp-invalid-cert - :return: dictionary with: - data: the raw HPKP header - includesubdomains: whether the includeSubDomains directive is set - max-age: what the max - num-pins: the number of pins - expectation: test expectation - pass: whether the site's configuration met its expectation - result: short string describing the result of the test - """ - FIFTEEN_DAYS = 1296000 - - output = { - 'data': None, - 'expectation': expectation, - 'includeSubDomains': False, - 'max-age': None, - 'numPins': None, - 'pass': True, - 'preloaded': False, - 'result': 'hpkp-not-implemented', - } - response = reqs['responses']['https'] - - # If there's no HTTPS, we can't have HPKP - if response is None: - output['result'] = 'hpkp-not-implemented-no-https' - - # Can't have HPKP without a valid certificate chain - elif not response.verified: - output['result'] = 'hpkp-invalid-cert' - - elif 'Public-Key-Pins' in response.headers: - output['data'] = response.headers['Public-Key-Pins'][0:2048] # code against malicious headers - - try: - pkp = [i.lower().strip() for i in output['data'].split(';')] - pins = [] - - for parameter in pkp: - if parameter.startswith('max-age='): - output['max-age'] = int(parameter[8:128]) # defense - elif parameter.startswith('pin-sha256=') and parameter not in pins: - pins.append(parameter) - elif parameter == 'includesubdomains': - output['includeSubDomains'] = True - output['numPins'] = len(pins) - - # You must set a max-age with HPKP - if output['max-age']: - if output['max-age'] < FIFTEEN_DAYS: - output['result'] = 'hpkp-implemented-max-age-less-than-fifteen-days' - else: - output['result'] = 'hpkp-implemented-max-age-at-least-fifteen-days' - - # You must have at least two pins with HPKP and set max-age - if not output['max-age'] or len(pins) < 2: - raise ValueError - - except: - output['result'] = 'hpkp-header-invalid' - output['pass'] = False - - # If they're in the preloaded list, this overrides most anything else - if response is not None: - preloaded = is_hpkp_preloaded(urlparse(response.url).netloc) - if preloaded: - output['result'] = 'hpkp-preloaded' - output['includeSubDomains'] = preloaded['includeSubDomainsForPinning'] - output['preloaded'] = True - - # No need to check pass/fail here, the only way to fail is to have an invalid header - return output - - @scored_test def referrer_policy(reqs: dict, expectation='referrer-policy-private') -> dict: """ diff --git a/httpobs/scanner/analyzer/utils.py b/httpobs/scanner/analyzer/utils.py index 3f20e847..5f33f176 100644 --- a/httpobs/scanner/analyzer/utils.py +++ b/httpobs/scanner/analyzer/utils.py @@ -9,25 +9,6 @@ hsts = json.load(f) -def is_hpkp_preloaded(hostname): - # Just see if the hostname is in the HSTS list and pinned - if hsts.get(hostname, {}).get('pinned'): - return hsts[hostname] - - # Either the hostname is in the list *or* one of its subdomains is - host = hostname.split('.') - levels = len(host) - - # If hostname is foo.bar.baz.mozilla.org, check bar.baz.mozilla.org, baz.mozilla.org, mozilla.org, and .org - for i in range(1, levels): - domain = '.'.join(host[i:levels]) - - if hsts.get(domain, {}).get('pinned') is True and hsts.get(domain, {}).get('includeSubDomainsForPinning'): - return hsts[domain] - - return False - - def is_hsts_preloaded(hostname): # Just see if the hostname is the HSTS list with the right mode -- no need to check includeSubDomains if hsts.get(hostname, {}).get('mode') == 'force-https': diff --git a/httpobs/scanner/grader/grade.py b/httpobs/scanner/grader/grade.py index e9fc4909..e6133ef0 100644 --- a/httpobs/scanner/grader/grade.py +++ b/httpobs/scanner/grader/grade.py @@ -171,37 +171,6 @@ 'description': 'Content is visible via cross-origin resource sharing (CORS) file or headers', 'modifier': -50, }, - # Public Key Pinning - 'hpkp-preloaded': { - 'description': 'Preloaded via the HTTP Public Key Pinning (HPKP) preloading process', - 'modifier': 0, - }, - 'hpkp-implemented-max-age-at-least-fifteen-days': { - 'description': 'HTTP Public Key Pinning (HPKP) header set to a minimum of 15 days (1296000)', - 'modifier': 0, - }, - 'hpkp-implemented-max-age-less-than-fifteen-days': { - 'description': 'HTTP Public Key Pinning (HPKP) header set to less than 15 days (1296000)', - 'modifier': 0, - }, - 'hpkp-not-implemented': { - 'description': 'HTTP Public Key Pinning (HPKP) header not implemented', - 'modifier': 0, - }, - 'hpkp-not-implemented-no-https': { - 'description': 'HTTP Public Key Pinning (HPKP) header can\'t be implemented without HTTPS', - 'modifier': 0, - }, - 'hpkp-invalid-cert': { - 'description': ( - 'HTTP Public Key Pinning (HPKP) header cannot be set, ' 'as site contains an invalid certificate chain' - ), - 'modifier': 0, - }, - 'hpkp-header-invalid': { - 'description': 'HTTP Public Key Pinning (HPKP) header cannot be recognized', - 'modifier': -5, - }, # Redirection 'redirection-all-redirects-preloaded': { 'description': 'All hosts redirected to are in the HTTP Strict Transport Security (HSTS) preload list', diff --git a/httpobs/scanner/scanner.py b/httpobs/scanner/scanner.py index ba0f4984..042e2eb3 100644 --- a/httpobs/scanner/scanner.py +++ b/httpobs/scanner/scanner.py @@ -24,7 +24,7 @@ def scan(hostname: str, **kwargs): path (str): path to scan, instead of "/" verify (bool): whether to enable or disable certificate verification, enabled by default. This can allow tested sites to pass the HSTS - and HPKP tests, even with self-signed certificates. + tests, even with self-signed certificates. cookies (dict): Cookies sent to the system being scanned. Matches the requests cookie dict. diff --git a/httpobs/scripts/scan.py b/httpobs/scripts/scan.py index 7ce0011d..f4d0bc62 100755 --- a/httpobs/scripts/scan.py +++ b/httpobs/scripts/scan.py @@ -15,9 +15,7 @@ def main(): parser.add_argument('--http-port', default=80, help='port to use for the HTTP scan (instead of 80)', type=int) parser.add_argument('--https-port', default=443, help='port to use for the HTTPS scan (instead of 443)', type=int) parser.add_argument('--path', default=argparse.SUPPRESS, help='path to scan, instead of /', type=str) - parser.add_argument( - '--no-verify', action='store_true', help='disable certificate verification in the HSTS/HPKP tests' - ) + parser.add_argument('--no-verify', action='store_true', help='disable certificate verification in the HSTS tests') parser.add_argument( '--cookies', default=argparse.SUPPRESS, help='cookies to send in scan (json formatted)', type=json.loads ) diff --git a/httpobs/tests/unittests/test_grades.py b/httpobs/tests/unittests/test_grades.py index 74eb4ee7..9b02904a 100644 --- a/httpobs/tests/unittests/test_grades.py +++ b/httpobs/tests/unittests/test_grades.py @@ -6,9 +6,9 @@ class TestGrader(TestCase): def test_get_score_description(self): self.assertEquals( - 'Preloaded via the HTTP Public Key Pinning (HPKP) preloading process', - get_score_description('hpkp-preloaded'), + 'Contribute.json implemented with the required contact information', + get_score_description('contribute-json-with-required-keys'), ) def test_get_score_modifier(self): - self.assertEquals(0, get_score_modifier('hpkp-preloaded')) + self.assertEquals(0, get_score_modifier('contribute-json-with-required-keys')) diff --git a/httpobs/tests/unittests/test_headers.py b/httpobs/tests/unittests/test_headers.py index 2531f1fa..a1903ad6 100644 --- a/httpobs/tests/unittests/test_headers.py +++ b/httpobs/tests/unittests/test_headers.py @@ -4,7 +4,6 @@ from httpobs.scanner.analyzer.headers import ( content_security_policy, cookies, - public_key_pinning, referrer_policy, strict_transport_security, x_content_type_options, @@ -987,141 +986,6 @@ def test_session_no_secure(self): self.assertFalse(result['sameSite']) -class TestPublicKeyPinning(TestCase): - def setUp(self): - self.reqs = empty_requests() - - def tearDown(self): - self.reqs = None - - def test_missing(self): - result = public_key_pinning(self.reqs) - - self.assertEquals('hpkp-not-implemented', result['result']) - self.assertTrue(result['pass']) - - def test_header_invalid(self): - # No pins - self.reqs['responses']['https'].headers['Public-Key-Pins'] = 'max-age=15768000; includeSubDomains; preload' - - result = public_key_pinning(self.reqs) - - self.assertEquals('hpkp-header-invalid', result['result']) - self.assertEquals(0, result['numPins']) - self.assertFalse(result['pass']) - - # No max-age - self.reqs['responses']['https'].headers['Public-Key-Pins'] = ( - 'pin-sha256="E9CZ9INDbd+2eRQozYqqbQ2yXLVKB9+xcprMF+44U1g="; ' - 'pin-sha256="LPJNul+wow4m6DsqxbninhsWHlwfp0JecwQzYpOLmCQ="; ' - 'report-uri="http://example.com/pkp-report"' - ) - result = public_key_pinning(self.reqs) - - self.assertEquals('hpkp-header-invalid', result['result']) - self.assertEquals(None, result['max-age']) - self.assertEquals(2, result['numPins']) - self.assertFalse(result['pass']) - - # Not enough pins - self.reqs['responses']['https'].headers['Public-Key-Pins'] = ( - 'max-age=15768000; ' - 'pin-sha256="LPJNul+wow4m6DsqxbninhsWHlwfp0JecwQzYpOLmCQ="; ' - 'report-uri="http://example.com/pkp-report"' - ) - - result = public_key_pinning(self.reqs) - - self.assertEquals('hpkp-header-invalid', result['result']) - self.assertEquals(15768000, result['max-age']) - self.assertEquals(1, result['numPins']) - self.assertFalse(result['pass']) - - def test_no_https(self): - self.reqs['responses']['auto'].headers['Public-Key-Pins'] = 'max-age=15768000' - self.reqs['responses']['http'].headers['Public-Key-Pins'] = 'max-age=15768000' - self.reqs['responses']['https'] = None - - result = public_key_pinning(self.reqs) - - self.assertEquals('hpkp-not-implemented-no-https', result['result']) - self.assertTrue(result['pass']) - - def test_invalid_cert(self): - self.reqs['responses']['https'].headers['Public-Key-Pins'] = ( - 'max-age=15768000; ' - 'includeSubDomains; ' - 'pin-sha256="E9CZ9INDbd+2eRQozYqqbQ2yXLVKB9+xcprMF+44U1g="; ' - 'pin-sha256="LPJNul+wow4m6DsqxbninhsWHlwfp0JecwQzYpOLmCQ="; ' - 'report-uri="http://example.com/pkp-report"' - ) - self.reqs['responses']['https'].verified = False - - result = public_key_pinning(self.reqs) - - self.assertEquals('hpkp-invalid-cert', result['result']) - self.assertTrue(result['pass']) - - def test_max_age_too_low(self): - self.reqs['responses']['https'].headers['Public-Key-Pins'] = ( - 'max-age=86400; ' - 'pin-sha256="E9CZ9INDbd+2eRQozYqqbQ2yXLVKB9+xcprMF+44U1g="; ' - 'pin-sha256="LPJNul+wow4m6DsqxbninhsWHlwfp0JecwQzYpOLmCQ="; ' - 'report-uri="http://example.com/pkp-report"' - ) - - result = public_key_pinning(self.reqs) - - self.assertEquals('hpkp-implemented-max-age-less-than-fifteen-days', result['result']) - self.assertTrue(result['pass']) - - def test_implemented(self): - self.reqs['responses']['https'].headers['Public-Key-Pins'] = ( - 'max-age=15768000; ' - 'includeSubDomains; ' - 'pin-sha256="E9CZ9INDbd+2eRQozYqqbQ2yXLVKB9+xcprMF+44U1g="; ' - 'pin-sha256="LPJNul+wow4m6DsqxbninhsWHlwfp0JecwQzYpOLmCQ="; ' - 'report-uri="http://example.com/pkp-report"' - ) - - result = public_key_pinning(self.reqs) - - self.assertEquals('hpkp-implemented-max-age-at-least-fifteen-days', result['result']) - self.assertEquals(15768000, result['max-age']) - self.assertEquals(15768000, result['max-age']) - self.assertTrue(result['includeSubDomains']) - self.assertTrue(result['pass']) - - def test_preloaded(self): - # apis.google.com has regular includeSubDomains - self.reqs['responses']['https'].url = 'https://apis.google.com/' - - result = public_key_pinning(self.reqs) - - self.assertEquals('hpkp-preloaded', result['result']) - self.assertTrue(result['includeSubDomains']) - self.assertTrue(result['pass']) - self.assertTrue(result['preloaded']) - self.reqs['responses']['https'].url = 'https://foo.apis.google.com' - - result = public_key_pinning(self.reqs) - - self.assertEquals('hpkp-preloaded', result['result']) - self.assertTrue(result['includeSubDomains']) - self.assertTrue(result['pass']) - self.assertTrue(result['preloaded']) - - # Dropbox Static uses include_subdomains_for_pinning - self.reqs['responses']['https'].url = 'https://foo.dropboxstatic.com/' - - result = public_key_pinning(self.reqs) - - self.assertEquals('hpkp-preloaded', result['result']) - self.assertTrue(result['includeSubDomains']) - self.assertTrue(result['pass']) - self.assertTrue(result['preloaded']) - - class TestReferrerPolicy(TestCase): def setUp(self): self.reqs = empty_requests() @@ -1318,7 +1182,7 @@ def test_preloaded(self): self.assertTrue(result['pass']) self.assertTrue(result['preloaded']) - # dropboxusercontent.com is preloaded, but only for HPKP, not HSTS + # dropboxusercontent.com is not preloaded self.reqs['responses']['https'].url = 'https://dropboxusercontent.com/' result = strict_transport_security(self.reqs) diff --git a/httpobs/tests/unittests/test_preload.py b/httpobs/tests/unittests/test_preload.py index 478696fa..12099c38 100644 --- a/httpobs/tests/unittests/test_preload.py +++ b/httpobs/tests/unittests/test_preload.py @@ -1,36 +1,6 @@ from unittest import TestCase -from httpobs.scanner.analyzer.utils import is_hpkp_preloaded, is_hsts_preloaded - - -class TestPreloadPublicKeyPinning(TestCase): - def test_not_preloaded(self): - result = is_hpkp_preloaded('totallyfakehostname.insertsuperduperfakedomainhere.wtftld') - - self.assertFalse(result) - - def test_preloaded(self): - result = is_hpkp_preloaded('apis.google.com') - - self.assertTrue(result['pinned']) - self.assertTrue(result['includeSubDomainsForPinning']) - - result = is_hpkp_preloaded('foo.apis.google.com') - - self.assertTrue(result['pinned']) - self.assertTrue(result['includeSubDomainsForPinning']) - - # uses include_subdomains_for_pinning - result = is_hpkp_preloaded('dropboxstatic.com') - - self.assertTrue(result['pinned']) - self.assertTrue(result['includeSubDomainsForPinning']) - - # this domain is manually pinned - result = is_hpkp_preloaded('aus4.mozilla.org') - - self.assertTrue(result['pinned']) - self.assertTrue(result['includeSubDomainsForPinning']) +from httpobs.scanner.analyzer.utils import is_hsts_preloaded class TestPreloadStrictTransportSecurity(TestCase):