Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

Commit

Permalink
Improve the way we detect whether all cookies have a valid SameSite a…
Browse files Browse the repository at this point in the history 
…ttribute. Fix failing tests due to None and Bool values not being strings. Add additional tests to cover the changed behaviour.
  • Loading branch information
@thecontrarycat
thecontrarycat committed Nov 11, 2020
1 parent b544fbf commit 0488c3c
Show file tree
Hide file tree
Showing 2 changed files with 169 additions and 27 deletions.
24 changes: 11 additions & 13 deletions httpobs/scanner/analyzer/headers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,9 @@ def cookies(reqs: dict, expectation='cookies-secure-with-httponly-sessions') ->
expectation: test expectation
pass: whether the site's configuration met its expectation
result: short string describing the result of the test
sameSite: True if all session cookies have a valid SameSite attribute
False if any session cookie has an invalid or missing SameSite attribute
None if there are no session cookies
"""

output = {
Expand Down Expand Up @@ -322,7 +325,6 @@ def cookies(reqs: dict, expectation='cookies-secure-with-httponly-sessions') ->

else:
jar = {}
samesiteCount = 0

# There are certain cookies we ignore, because they are set by service providers and sites have
# no control over them.
Expand All @@ -336,18 +338,13 @@ def cookies(reqs: dict, expectation='cookies-secure-with-httponly-sessions') ->
if key.lower() == 'httponly' and getattr(cookie, 'httponly') is False:
cookie.httponly = True
elif key.lower() == 'samesite' and getattr(cookie, 'samesite') is False:
if cookie._rest[key].strip().lower() == 'lax':
samesiteVal = '' if cookie._rest[key] == None else str(cookie._rest[key])
if samesiteVal.strip().lower() == 'lax':
cookie.samesite = 'Lax'
samesiteCount += 1
output['sameSite'] = True
elif cookie._rest[key].strip().lower() == 'strict':
elif samesiteVal.strip().lower() == 'strict':
cookie.samesite = 'Strict'
samesiteCount += 1
output['sameSite'] = True
elif cookie._rest[key].strip().lower() == 'none':
elif samesiteVal.strip().lower() == 'none':
cookie.samesite = 'None'
samesiteCount += 1
output['sameSite'] = True
else:
output['result'] = only_if_worse('cookies-samesite-flag-invalid',
output['result'],
Expand Down Expand Up @@ -400,11 +397,12 @@ def cookies(reqs: dict, expectation='cookies-secure-with-httponly-sessions') ->

# Store whether or not we saw SameSite cookies, if cookies were set
if output['result'] is None:
if output['sameSite'] and samesiteCount == len(jar):
output['result'] = 'cookies-secure-with-httponly-sessions-and-samesite'
else:
if any(c for c in session.cookies if c.samesite == False):
output['result'] = 'cookies-secure-with-httponly-sessions'
output['sameSite'] = False
else:
output['result'] = 'cookies-secure-with-httponly-sessions-and-samesite'
output['sameSite'] = True

# Save the cookie jar
output['data'] = jar if len(str(jar)) < 32768 else {}
Expand Down
172 changes: 158 additions & 14 deletions httpobs/tests/unittests/test_headers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ def test_secure_with_httponly_sessions_and_samesite(self):
value='bar')
self.reqs['session'].cookies.set_cookie(cookie)

cookie = Cookie(name='SESSIONID_SAMESITE_LAX_TRUE',
cookie = Cookie(name='SESSIONID_SAMESITE_LAX',
comment=None,
comment_url=None,
discard=False,
Expand All @@ -433,7 +433,101 @@ def test_secure_with_httponly_sessions_and_samesite(self):
port=443,
port_specified=443,
rfc2109=False,
rest={'HttpOnly': True, 'SameSite': True},
rest={'HttpOnly': True, 'SameSite': 'Lax'},
secure=True,
version=1,
value='bar')
self.reqs['session'].cookies.set_cookie(cookie)

cookie = Cookie(name='SESSIONID_SAMESITE_NONE',
comment=None,
comment_url=None,
discard=False,
domain='mozilla.com',
domain_initial_dot=False,
domain_specified='mozilla.com',
expires=None,
path='/',
path_specified='/',
port=443,
port_specified=443,
rfc2109=False,
rest={'HttpOnly': True, 'SameSite': 'None'},
secure=True,
version=1,
value='bar')
self.reqs['session'].cookies.set_cookie(cookie)

result = cookies(self.reqs)

self.assertEquals('cookies-secure-with-httponly-sessions-and-samesite', result['result'])
self.assertEquals({
'SESSIONID_SAMESITE_STRICT': {
'domain': 'mozilla.com',
'expires': None,
'httponly': True,
'max-age': None,
'path': '/',
'port': 443,
'samesite': 'Strict',
'secure': True},
'SESSIONID_SAMESITE_LAX': {
'domain': 'mozilla.com',
'expires': None,
'httponly': True,
'max-age': None,
'path': '/',
'port': 443,
'samesite': 'Lax',
'secure': True},
'SESSIONID_SAMESITE_NONE': {
'domain': 'mozilla.com',
'expires': None,
'httponly': True,
'max-age': None,
'path': '/',
'port': 443,
'samesite': 'None',
'secure': True}
},
result['data'])
self.assertTrue(result['pass'])
self.assertTrue(result['sameSite'])

def test_secure_with_httponly_sessions_and_samesite_not_awarded_if_not_all_cookies_samesite(self):
cookie = Cookie(name='SESSIONID_SAMESITE_STRICT',
comment=None,
comment_url=None,
discard=False,
domain='mozilla.com',
domain_initial_dot=False,
domain_specified='mozilla.com',
expires=None,
path='/',
path_specified='/',
port=443,
port_specified=443,
rfc2109=False,
rest={'HttpOnly': True, 'SameSite': 'Strict'},
secure=True,
version=1,
value='bar')
self.reqs['session'].cookies.set_cookie(cookie)

cookie = Cookie(name='SESSIONID_NO_SAMESITE',
comment=None,
comment_url=None,
discard=False,
domain='mozilla.com',
domain_initial_dot=False,
domain_specified='mozilla.com',
expires=None,
path='/',
path_specified='/',
port=443,
port_specified=443,
rfc2109=False,
rest={'HttpOnly': True},
secure=True,
version=1,
value='bar')
Expand All @@ -458,7 +552,7 @@ def test_secure_with_httponly_sessions_and_samesite(self):
value='bar')
self.reqs['session'].cookies.set_cookie(cookie)

cookie = Cookie(name='SESSIONID_SAMESITE_LAX_NONE',
cookie = Cookie(name='SESSIONID_SAMESITE_NONE',
comment=None,
comment_url=None,
discard=False,
Expand All @@ -471,15 +565,15 @@ def test_secure_with_httponly_sessions_and_samesite(self):
port=443,
port_specified=443,
rfc2109=False,
rest={'HttpOnly': True, 'SameSite': None},
rest={'HttpOnly': True, 'SameSite': 'None'},
secure=True,
version=1,
value='bar')
self.reqs['session'].cookies.set_cookie(cookie)

result = cookies(self.reqs)

self.assertEquals('cookies-secure-with-httponly-sessions-and-samesite', result['result'])
self.assertEquals('cookies-secure-with-httponly-sessions', result['result'])
self.assertEquals({
'SESSIONID_SAMESITE_STRICT': {
'domain': 'mozilla.com',
Expand All @@ -489,16 +583,15 @@ def test_secure_with_httponly_sessions_and_samesite(self):
'path': '/',
'port': 443,
'samesite': 'Strict',
'secure': True
},
'SESSIONID_SAMESITE_LAX_TRUE': {
'secure': True},
'SESSIONID_NO_SAMESITE': {
'domain': 'mozilla.com',
'expires': None,
'httponly': True,
'max-age': None,
'path': '/',
'port': 443,
'samesite': 'Lax',
'samesite': False,
'secure': True},
'SESSIONID_SAMESITE_LAX': {
'domain': 'mozilla.com',
Expand All @@ -508,21 +601,20 @@ def test_secure_with_httponly_sessions_and_samesite(self):
'path': '/',
'port': 443,
'samesite': 'Lax',
'secure': True
},
'SESSIONID_SAMESITE_LAX_NONE': {
'secure': True},
'SESSIONID_SAMESITE_NONE': {
'domain': 'mozilla.com',
'expires': None,
'httponly': True,
'max-age': None,
'path': '/',
'port': 443,
'samesite': 'Lax',
'samesite': 'None',
'secure': True}
},
result['data'])
self.assertTrue(result['pass'])
self.assertTrue(result['sameSite'])
self.assertFalse(result['sameSite'])

def test_anticsrf_without_samesite(self):
cookie = Cookie(name='CSRFTOKEN',
Expand Down Expand Up @@ -550,6 +642,58 @@ def test_anticsrf_without_samesite(self):
self.assertFalse(result['pass'])
self.assertFalse(result['sameSite'])

def test_samesite_invalid_empty(self):
cookie = Cookie(name='SESSIONID',
comment=None,
comment_url=None,
discard=False,
domain='mozilla.com',
domain_initial_dot=False,
domain_specified='mozilla.com',
expires=None,
path='/',
path_specified='/',
port=443,
port_specified=443,
rfc2109=False,
rest={'HttpOnly': True, 'SameSite': None},
secure=True,
version=1,
value='bar')
self.reqs['session'].cookies.set_cookie(cookie)

result = cookies(self.reqs)

self.assertEquals('cookies-samesite-flag-invalid', result['result'])
self.assertFalse(result['pass'])
self.assertIsNone(result['sameSite'])

def test_samesite_invalid_true(self):
cookie = Cookie(name='SESSIONID',
comment=None,
comment_url=None,
discard=False,
domain='mozilla.com',
domain_initial_dot=False,
domain_specified='mozilla.com',
expires=None,
path='/',
path_specified='/',
port=443,
port_specified=443,
rfc2109=False,
rest={'HttpOnly': True, 'SameSite': True},
secure=True,
version=1,
value='bar')
self.reqs['session'].cookies.set_cookie(cookie)

result = cookies(self.reqs)

self.assertEquals('cookies-samesite-flag-invalid', result['result'])
self.assertFalse(result['pass'])
self.assertIsNone(result['sameSite'])

def test_samesite_invalid(self):
cookie = Cookie(name='SESSIONID',
comment=None,
Expand Down

0 comments on commit 0488c3c

Please sign in to comment.