diff --git a/HISTORY.rst b/HISTORY.rst index 3d50672d2..b171ee811 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -24,6 +24,8 @@ Bugfixes ~~~~~~~~ - Correctly forbid pseudo-headers that were not defined in RFC 7540. +- Automatically ensure that all ``Authorization`` headers and short ``Cookie`` + headers are prevented from being added to encoding contexts. 2.2.3 (2016-04-13) ------------------ diff --git a/h2/stream.py b/h2/stream.py index 08296625e..db14abbb8 100644 --- a/h2/stream.py +++ b/h2/stream.py @@ -24,7 +24,8 @@ ProtocolError, StreamClosedError, InvalidBodyLengthError ) from .utilities import ( - guard_increment_window, is_informational_response, authority_from_headers + guard_increment_window, is_informational_response, authority_from_headers, + secure_headers ) @@ -988,8 +989,10 @@ def _build_headers_frames(self, """ Helper method to build headers or push promise frames. """ - # We need to lowercase the header names. + # We need to lowercase the header names, and to ensure that secure + # header fields are kept out of compression contexts. headers = _lowercase_header_names(headers) + headers = secure_headers(headers) encoded_headers = encoder.encode(headers) # Slice into blocks of max_outbound_frame_size. Be careful with this: diff --git a/h2/utilities.py b/h2/utilities.py index 132eb3473..f770ad7da 100644 --- a/h2/utilities.py +++ b/h2/utilities.py @@ -7,6 +7,8 @@ """ import re +from hpack import NeverIndexedHeaderTuple + from .exceptions import ProtocolError, FlowControlError UPPER_RE = re.compile(b"[A-Z]") @@ -31,6 +33,32 @@ ]) +def secure_headers(headers): + """ + Certain headers are at risk of being attacked during the header compression + phase, and so need to be kept out of header compression contexts. This + function automatically transforms certain specific headers into HPACK + never-indexed fields to ensure they don't get added to header compression + contexts. + + This function currently implements two rules: + + - All 'authorization' headers are automatically made never-indexed. + - Any 'cookie' header field shorter than 20 bytes long is made + never-indexed. + + These two fields are the most at-risk. These rules are inspired by Firefox + and nghttp2. + """ + for header in headers: + if header[0] in (b'authorization', u'authorization'): + yield NeverIndexedHeaderTuple(*header) + elif header[0] in (b'cookie', u'cookie') and len(header[1]) < 20: + yield NeverIndexedHeaderTuple(*header) + else: + yield header + + def is_informational_response(headers): """ Searches a header block for a :status header to confirm that a given diff --git a/test/test_header_indexing.py b/test/test_header_indexing.py index ea4fa5e01..10ff42670 100644 --- a/test/test_header_indexing.py +++ b/test/test_header_indexing.py @@ -311,3 +311,291 @@ def test_header_tuples_are_decoded_push_promise(self, assert isinstance(event, h2.events.PushedStreamReceived) assert_header_blocks_actually_equal(headers, event.headers) + + +class TestSecureHeaders(object): + """ + Certain headers should always be transformed to their never-indexed form. + """ + example_request_headers = [ + (u':authority', u'example.com'), + (u':path', u'/'), + (u':scheme', u'https'), + (u':method', u'GET'), + ] + bytes_example_request_headers = [ + (b':authority', b'example.com'), + (b':path', b'/'), + (b':scheme', b'https'), + (b':method', b'GET'), + ] + possible_auth_headers = [ + (u'authorization', u'test'), + (u'Authorization', u'test'), + (u'authorization', u'really long test'), + HeaderTuple(u'authorization', u'test'), + HeaderTuple(u'Authorization', u'test'), + HeaderTuple(u'authorization', u'really long test'), + NeverIndexedHeaderTuple(u'authorization', u'test'), + NeverIndexedHeaderTuple(u'Authorization', u'test'), + NeverIndexedHeaderTuple(u'authorization', u'really long test'), + (b'authorization', b'test'), + (b'Authorization', b'test'), + (b'authorization', b'really long test'), + HeaderTuple(b'authorization', b'test'), + HeaderTuple(b'Authorization', b'test'), + HeaderTuple(b'authorization', b'really long test'), + NeverIndexedHeaderTuple(b'authorization', b'test'), + NeverIndexedHeaderTuple(b'Authorization', b'test'), + NeverIndexedHeaderTuple(b'authorization', b'really long test'), + ] + secured_cookie_headers = [ + (u'cookie', u'short'), + (u'Cookie', u'short'), + (u'cookie', u'nineteen byte cooki'), + HeaderTuple(u'cookie', u'short'), + HeaderTuple(u'Cookie', u'short'), + HeaderTuple(u'cookie', u'nineteen byte cooki'), + NeverIndexedHeaderTuple(u'cookie', u'short'), + NeverIndexedHeaderTuple(u'Cookie', u'short'), + NeverIndexedHeaderTuple(u'cookie', u'nineteen byte cooki'), + NeverIndexedHeaderTuple(u'cookie', u'longer manually secured cookie'), + (b'cookie', b'short'), + (b'Cookie', b'short'), + (b'cookie', b'nineteen byte cooki'), + HeaderTuple(b'cookie', b'short'), + HeaderTuple(b'Cookie', b'short'), + HeaderTuple(b'cookie', b'nineteen byte cooki'), + NeverIndexedHeaderTuple(b'cookie', b'short'), + NeverIndexedHeaderTuple(b'Cookie', b'short'), + NeverIndexedHeaderTuple(b'cookie', b'nineteen byte cooki'), + NeverIndexedHeaderTuple(b'cookie', b'longer manually secured cookie'), + ] + unsecured_cookie_headers = [ + (u'cookie', u'twenty byte cookie!!'), + (u'Cookie', u'twenty byte cookie!!'), + (u'cookie', u'substantially longer than 20 byte cookie'), + HeaderTuple(u'cookie', u'twenty byte cookie!!'), + HeaderTuple(u'cookie', u'twenty byte cookie!!'), + HeaderTuple(u'Cookie', u'twenty byte cookie!!'), + (b'cookie', b'twenty byte cookie!!'), + (b'Cookie', b'twenty byte cookie!!'), + (b'cookie', b'substantially longer than 20 byte cookie'), + HeaderTuple(b'cookie', b'twenty byte cookie!!'), + HeaderTuple(b'cookie', b'twenty byte cookie!!'), + HeaderTuple(b'Cookie', b'twenty byte cookie!!'), + ] + + @pytest.mark.parametrize( + 'headers', (example_request_headers, bytes_example_request_headers) + ) + @pytest.mark.parametrize('auth_header', possible_auth_headers) + def test_authorization_headers_never_indexed(self, + headers, + auth_header, + frame_factory): + """ + Authorization headers are always forced to be never-indexed, regardless + of their form. + """ + # Regardless of what we send, we expect it to be never indexed. + send_headers = headers + [auth_header] + expected_headers = headers + [ + NeverIndexedHeaderTuple(auth_header[0].lower(), auth_header[1]) + ] + + c = h2.connection.H2Connection() + c.initiate_connection() + + # Clear the data, then send headers. + c.clear_outbound_data_buffer() + c.send_headers(1, send_headers) + + f = frame_factory.build_headers_frame(headers=expected_headers) + assert c.data_to_send() == f.serialize() + + @pytest.mark.parametrize( + 'headers', (example_request_headers, bytes_example_request_headers) + ) + @pytest.mark.parametrize('auth_header', possible_auth_headers) + def test_authorization_headers_never_indexed_push(self, + headers, + auth_header, + frame_factory): + """ + Authorization headers are always forced to be never-indexed, regardless + of their form, when pushed by a server. + """ + # Regardless of what we send, we expect it to be never indexed. + send_headers = headers + [auth_header] + expected_headers = headers + [ + NeverIndexedHeaderTuple(auth_header[0].lower(), auth_header[1]) + ] + + c = h2.connection.H2Connection(client_side=False) + c.receive_data(frame_factory.preamble()) + + # We can use normal headers for the request. + f = frame_factory.build_headers_frame( + self.example_request_headers + ) + c.receive_data(f.serialize()) + + frame_factory.refresh_encoder() + expected_frame = frame_factory.build_push_promise_frame( + stream_id=1, + promised_stream_id=2, + headers=expected_headers, + flags=['END_HEADERS'], + ) + + c.clear_outbound_data_buffer() + c.push_stream( + stream_id=1, + promised_stream_id=2, + request_headers=send_headers + ) + + assert c.data_to_send() == expected_frame.serialize() + + @pytest.mark.parametrize( + 'headers', (example_request_headers, bytes_example_request_headers) + ) + @pytest.mark.parametrize('cookie_header', secured_cookie_headers) + def test_short_cookie_headers_never_indexed(self, + headers, + cookie_header, + frame_factory): + """ + Short cookie headers, and cookies provided as NeverIndexedHeaderTuple, + are never indexed. + """ + # Regardless of what we send, we expect it to be never indexed. + send_headers = headers + [cookie_header] + expected_headers = headers + [ + NeverIndexedHeaderTuple(cookie_header[0].lower(), cookie_header[1]) + ] + + c = h2.connection.H2Connection() + c.initiate_connection() + + # Clear the data, then send headers. + c.clear_outbound_data_buffer() + c.send_headers(1, send_headers) + + f = frame_factory.build_headers_frame(headers=expected_headers) + assert c.data_to_send() == f.serialize() + + @pytest.mark.parametrize( + 'headers', (example_request_headers, bytes_example_request_headers) + ) + @pytest.mark.parametrize('cookie_header', secured_cookie_headers) + def test_short_cookie_headers_never_indexed_push(self, + headers, + cookie_header, + frame_factory): + """ + Short cookie headers, and cookies provided as NeverIndexedHeaderTuple, + are never indexed when pushed by servers. + """ + # Regardless of what we send, we expect it to be never indexed. + send_headers = headers + [cookie_header] + expected_headers = headers + [ + NeverIndexedHeaderTuple(cookie_header[0].lower(), cookie_header[1]) + ] + + c = h2.connection.H2Connection(client_side=False) + c.receive_data(frame_factory.preamble()) + + # We can use normal headers for the request. + f = frame_factory.build_headers_frame( + self.example_request_headers + ) + c.receive_data(f.serialize()) + + frame_factory.refresh_encoder() + expected_frame = frame_factory.build_push_promise_frame( + stream_id=1, + promised_stream_id=2, + headers=expected_headers, + flags=['END_HEADERS'], + ) + + c.clear_outbound_data_buffer() + c.push_stream( + stream_id=1, + promised_stream_id=2, + request_headers=send_headers + ) + + assert c.data_to_send() == expected_frame.serialize() + + @pytest.mark.parametrize( + 'headers', (example_request_headers, bytes_example_request_headers) + ) + @pytest.mark.parametrize('cookie_header', unsecured_cookie_headers) + def test_long_cookie_headers_can_be_indexed(self, + headers, + cookie_header, + frame_factory): + """ + Longer cookie headers can be indexed. + """ + # Regardless of what we send, we expect it to be indexed. + send_headers = headers + [cookie_header] + expected_headers = headers + [ + HeaderTuple(cookie_header[0].lower(), cookie_header[1]) + ] + + c = h2.connection.H2Connection() + c.initiate_connection() + + # Clear the data, then send headers. + c.clear_outbound_data_buffer() + c.send_headers(1, send_headers) + + f = frame_factory.build_headers_frame(headers=expected_headers) + assert c.data_to_send() == f.serialize() + + @pytest.mark.parametrize( + 'headers', (example_request_headers, bytes_example_request_headers) + ) + @pytest.mark.parametrize('cookie_header', unsecured_cookie_headers) + def test_long_cookie_headers_can_be_indexed_push(self, + headers, + cookie_header, + frame_factory): + """ + Longer cookie headers can be indexed. + """ + # Regardless of what we send, we expect it to be never indexed. + send_headers = headers + [cookie_header] + expected_headers = headers + [ + HeaderTuple(cookie_header[0].lower(), cookie_header[1]) + ] + + c = h2.connection.H2Connection(client_side=False) + c.receive_data(frame_factory.preamble()) + + # We can use normal headers for the request. + f = frame_factory.build_headers_frame( + self.example_request_headers + ) + c.receive_data(f.serialize()) + + frame_factory.refresh_encoder() + expected_frame = frame_factory.build_push_promise_frame( + stream_id=1, + promised_stream_id=2, + headers=expected_headers, + flags=['END_HEADERS'], + ) + + c.clear_outbound_data_buffer() + c.push_stream( + stream_id=1, + promised_stream_id=2, + request_headers=send_headers + ) + + assert c.data_to_send() == expected_frame.serialize()