From 9c4e4f5e17e6779989df4eebd5bf09918cb03008 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Wed, 23 Nov 2016 15:18:41 +0530 Subject: [PATCH 01/27] Implement CertificateURL constructs --- tls/_common/enums.py | 5 +++++ tls/_constructs.py | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/tls/_common/enums.py b/tls/_common/enums.py index ea36d1e..eab5e06 100644 --- a/tls/_common/enums.py +++ b/tls/_common/enums.py @@ -128,3 +128,8 @@ class CompressionMethod(Enum): class NameType(Enum): HOST_NAME = 0 + + +class CertChainType(Enum): + INDIVIDUAL_CERTS = 0 + PKIPATH = 1 diff --git a/tls/_constructs.py b/tls/_constructs.py index 4da2757..bf3de31 100644 --- a/tls/_constructs.py +++ b/tls/_constructs.py @@ -195,3 +195,20 @@ UBInt8("level"), UBInt8("description"), ) + +URLAndHash = Struct( + "url_and_hash", + SizeWithin(UBInt16("length"), + min_size=1, max_size=2 ** 16 - 1), + Bytes("url", lambda ctx: ctx.length), + UBInt8("padding"), + # The "padding" byte MUST be 0x01. It is present to make the structure + # backwards compatible. + Bytes("SHA1Hash", 20), +) + +CertificateURL = Struct( + "CertificateURL", + EnumClass(UBInt8("type"), enums.CertChainType), + TLSPrefixedArray("url_and_hash_list", URLAndHash), +) From d43b7540785550251194bf2bcc6721feee85b64d Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Wed, 23 Nov 2016 15:21:54 +0530 Subject: [PATCH 02/27] Add some new alerts to the AlertDescription enum --- tls/_common/enums.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tls/_common/enums.py b/tls/_common/enums.py index eab5e06..6ac7089 100644 --- a/tls/_common/enums.py +++ b/tls/_common/enums.py @@ -85,6 +85,10 @@ class AlertDescription(Enum): USER_CANCELED = 90 NO_RENEGOTIATION = 100 UNSUPPORTED_EXTENSION = 110 + CERTIFICATE_UNOBTAINABLE = 111 + UNRECOGNIZED_NAME = 112 + BAD_CERTIFICATE_STATUS_RESPONSE = 113 + BAD_CERTIFICATE_HASH_VALUE = 114 class ExtensionType(Enum): From dbcb7dba048f7e56d66fb7ab9d6515b3a9cceef9 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Wed, 23 Nov 2016 15:40:02 +0530 Subject: [PATCH 03/27] Define an object representation of a URLAndHash struct --- tls/_constructs.py | 2 +- tls/message.py | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/tls/_constructs.py b/tls/_constructs.py index bf3de31..c3eef1f 100644 --- a/tls/_constructs.py +++ b/tls/_constructs.py @@ -204,7 +204,7 @@ UBInt8("padding"), # The "padding" byte MUST be 0x01. It is present to make the structure # backwards compatible. - Bytes("SHA1Hash", 20), + Bytes("sha1_hash", 20), ) CertificateURL = Struct( diff --git a/tls/message.py b/tls/message.py index c82470c..55ab0a8 100644 --- a/tls/message.py +++ b/tls/message.py @@ -204,6 +204,30 @@ def from_bytes(cls, bytes): ) +@attr.s +class URLAndHash(object): + """ + An object representing a URLAndHash struct. + """ + url = attr.ib() + padding = attr.ib() + sha1_hash = attr.ib() + + def as_bytes(self): + + # XXX: The "padding" byte MUST be 0x01. It is + # present to make the structure backwards + # compatible. + # Should the validation for padding go here? + assert self.padding == 1 + return _constructs.URLAndHash.build(Container( + length=len(self.url), + url=self.url, + padding=self.padding, + sha1_hash=self.sha1_hash, + )) + + @attr.s class Finished(object): verify_data = attr.ib() From 6fa7c53e441ec38fae7b4461a22f695d9f4ee4c0 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Wed, 23 Nov 2016 15:47:08 +0530 Subject: [PATCH 04/27] Define CertificateURL.as_bytes() --- tls/message.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tls/message.py b/tls/message.py index 55ab0a8..7185cb6 100644 --- a/tls/message.py +++ b/tls/message.py @@ -228,6 +228,30 @@ def as_bytes(self): )) +@attr.s +class CertificateURL(object): + """ + An object representing a CertificateURL struct. + """ + type = attr.ib() + url_and_hash_list = attr.ib() + + def as_bytes(self): + return _constructs.CertificateURL.build( + Container( + type=self.type, + url_and_hash_list=ListContainer( + Container( + length=len(url_and_hash.url), + url=url_and_hash.url, + padding=url_and_hash.padding, + sha1_hash=url_and_hash.sha1_hash + ) + for url_and_hash in self.url_and_hash_list + ) + )) + + @attr.s class Finished(object): verify_data = attr.ib() From 8c732bb1e699d30cff62799e2cbeb5677c410b04 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Wed, 23 Nov 2016 15:49:46 +0530 Subject: [PATCH 05/27] Something about not repeating yourself --- tls/message.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tls/message.py b/tls/message.py index 7185cb6..3996221 100644 --- a/tls/message.py +++ b/tls/message.py @@ -240,13 +240,8 @@ def as_bytes(self): return _constructs.CertificateURL.build( Container( type=self.type, - url_and_hash_list=ListContainer( - Container( - length=len(url_and_hash.url), - url=url_and_hash.url, - padding=url_and_hash.padding, - sha1_hash=url_and_hash.sha1_hash - ) + url_and_hash_list=b''.join( + url_and_hash.as_bytes() for url_and_hash in self.url_and_hash_list ) )) From f1d59a176536f3699784bfbc2ee33611acaf856a Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Wed, 23 Nov 2016 22:53:10 +0530 Subject: [PATCH 06/27] A barebones CertificateURL.from_bytes --- tls/message.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tls/message.py b/tls/message.py index 3996221..754a937 100644 --- a/tls/message.py +++ b/tls/message.py @@ -246,6 +246,15 @@ def as_bytes(self): ) )) + def from_bytes(cls, bytes): + """ + """ + construct = _constructs.CertificateURL.parse(bytes) + return cls( + type=None, # FIXME + url_and_hash_list=None, # FIXME + ) + @attr.s class Finished(object): From eba6d749b3e27c138c598c1caf459fa252b360ba Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Thu, 1 Dec 2016 10:12:50 +0530 Subject: [PATCH 07/27] Implement CertificateURL.from_bytes --- tls/message.py | 12 ++++++++++-- tls/test/test_message.py | 35 ++++++++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/tls/message.py b/tls/message.py index 754a937..df36279 100644 --- a/tls/message.py +++ b/tls/message.py @@ -246,13 +246,21 @@ def as_bytes(self): ) )) + @classmethod def from_bytes(cls, bytes): """ """ construct = _constructs.CertificateURL.parse(bytes) return cls( - type=None, # FIXME - url_and_hash_list=None, # FIXME + type=construct.type, + url_and_hash_list=[ + URLAndHash( + url=url_and_hash.url, + padding=url_and_hash.padding, + sha1_hash=url_and_hash.sha1_hash, + ) + for url_and_hash in construct.url_and_hash_list + ], ) diff --git a/tls/test/test_message.py b/tls/test/test_message.py index c58b93d..4a04dbb 100644 --- a/tls/test/test_message.py +++ b/tls/test/test_message.py @@ -13,9 +13,8 @@ from tls.hello_message import ClientHello, ProtocolVersion, ServerHello from tls.message import (ASN1Cert, Certificate, CertificateRequest, - Finished, Handshake, HelloRequest, - PreMasterSecret, ServerDHParams, - ServerHelloDone) + CertificateURL, Finished, Handshake, HelloRequest, + PreMasterSecret, ServerDHParams, ServerHelloDone) class TestCertificateRequestParsing(object): @@ -283,6 +282,36 @@ def test_as_bytes_too_long(self): certificate.as_bytes() +class TestCertificateURLParsing(object): + """ + Tests for parsing of :py:class:`tls.message.CertificateURL` messages. + """ + url_and_hash_list_bytes = ( + b'\x00\x10' # url length + b'cert.example.com' # url + b'\x01' # padding + b'abcdefghijklmnopqrst' # SHA1Hash[20] + ) + + certificate_url_packet = ( + b'\x00' # CertChainType + b'\x00\x27' # url_and_hash_list length + ) + url_and_hash_list_bytes + + def test_parse_certificate_url(self): + """ + :py:meth:`tls.message.CertificateURL.from_bytes` parses a valid + packet. + """ + record = CertificateURL.from_bytes(self.certificate_url_packet) + assert isinstance(record, CertificateURL) + assert record.type == enums.CertChainType.INDIVIDUAL_CERTS + assert len(record.url_and_hash_list) == 1 + assert record.url_and_hash_list[0].url == b'cert.example.com' + assert record.url_and_hash_list[0].padding == 1 + assert record.url_and_hash_list[0].sha1_hash == b'abcdefghijklmnopqrst' + + class TestHandshakeStructParsing(object): """ Tests for parsing of :py:class:`tls.messages.Handshake` structs. From a3228607e0a7427c0195fca919125b9e583f9dc3 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Thu, 1 Dec 2016 10:49:01 +0530 Subject: [PATCH 08/27] Raise an exception when the padding byte is not 1 --- tls/exceptions.py | 7 +++++++ tls/message.py | 31 +++++++++++++++++++++---------- tls/test/test_message.py | 20 ++++++++++++++++++++ 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/tls/exceptions.py b/tls/exceptions.py index 417cbe4..12ec917 100644 --- a/tls/exceptions.py +++ b/tls/exceptions.py @@ -11,3 +11,10 @@ class UnsupportedCipherException(Exception): class UnsupportedExtensionException(Exception): pass + + +class InvalidPadding(Exception): + # XXX: Is leaking this info in CertificateURL (i.e. revealing the specific + # byte that is invalid) a good idea? Need to check -- could be a + # vulnerability in disguise. + pass diff --git a/tls/message.py b/tls/message.py index df36279..c8f337d 100644 --- a/tls/message.py +++ b/tls/message.py @@ -14,6 +14,8 @@ from tls._common import enums +from tls.exceptions import InvalidPadding + from tls.hello_message import ClientHello, ProtocolVersion, ServerHello @@ -214,12 +216,12 @@ class URLAndHash(object): sha1_hash = attr.ib() def as_bytes(self): - # XXX: The "padding" byte MUST be 0x01. It is # present to make the structure backwards # compatible. # Should the validation for padding go here? - assert self.padding == 1 + if self.padding != 1: + raise InvalidPadding return _constructs.URLAndHash.build(Container( length=len(self.url), url=self.url, @@ -249,18 +251,27 @@ def as_bytes(self): @classmethod def from_bytes(cls, bytes): """ + Parse a ``CertificateURL`` struct. + + :param bytes: the bytes representing the input. + :return: CertificateURL object. """ construct = _constructs.CertificateURL.parse(bytes) + url_and_hash_list = [] + for url_and_hash in construct.url_and_hash_list: + # TODO: Figure how attrs does validation and move this assert to + # URLAndHash + if url_and_hash.padding != 1: + raise InvalidPadding + url_and_hash_list.append(URLAndHash( + url=url_and_hash.url, + padding=url_and_hash.padding, + sha1_hash=url_and_hash.sha1_hash, + )) + return cls( type=construct.type, - url_and_hash_list=[ - URLAndHash( - url=url_and_hash.url, - padding=url_and_hash.padding, - sha1_hash=url_and_hash.sha1_hash, - ) - for url_and_hash in construct.url_and_hash_list - ], + url_and_hash_list=url_and_hash_list, ) diff --git a/tls/test/test_message.py b/tls/test/test_message.py index 4a04dbb..25e9a8a 100644 --- a/tls/test/test_message.py +++ b/tls/test/test_message.py @@ -10,6 +10,8 @@ from tls._common import enums +from tls.exceptions import InvalidPadding + from tls.hello_message import ClientHello, ProtocolVersion, ServerHello from tls.message import (ASN1Cert, Certificate, CertificateRequest, @@ -311,6 +313,24 @@ def test_parse_certificate_url(self): assert record.url_and_hash_list[0].padding == 1 assert record.url_and_hash_list[0].sha1_hash == b'abcdefghijklmnopqrst' + def test_incorrect_padding(self): + bad_padding_bytes = ( + b'\x00\x10' # url length + b'cert.example.com' # url + b'\x03' # padding + b'abcdefghijklmnopqrst' # SHA1Hash[20] + ) + certificate_url_packet = ( + b'\x00' # CertChainType + b'\x00\x27' # url_and_hash_list length + ) + bad_padding_bytes + + with pytest.raises(InvalidPadding): + CertificateURL.from_bytes(certificate_url_packet) + +# with pytest.raises(ValidationError): + + class TestHandshakeStructParsing(object): """ From ca558156ce3839f5bcfd70be1a1ceed732420902 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Thu, 1 Dec 2016 11:08:48 +0530 Subject: [PATCH 09/27] implement CertificateURL.as_bytes() --- tls/exceptions.py | 2 +- tls/message.py | 42 +++++++++++++++++++--------------------- tls/test/test_message.py | 26 ++++++++++++++++++++++--- 3 files changed, 44 insertions(+), 26 deletions(-) diff --git a/tls/exceptions.py b/tls/exceptions.py index 12ec917..831c93e 100644 --- a/tls/exceptions.py +++ b/tls/exceptions.py @@ -13,7 +13,7 @@ class UnsupportedExtensionException(Exception): pass -class InvalidPadding(Exception): +class InvalidPaddingException(Exception): # XXX: Is leaking this info in CertificateURL (i.e. revealing the specific # byte that is invalid) a good idea? Need to check -- could be a # vulnerability in disguise. diff --git a/tls/message.py b/tls/message.py index c8f337d..8e5142b 100644 --- a/tls/message.py +++ b/tls/message.py @@ -14,7 +14,7 @@ from tls._common import enums -from tls.exceptions import InvalidPadding +from tls.exceptions import InvalidPaddingException from tls.hello_message import ClientHello, ProtocolVersion, ServerHello @@ -215,19 +215,10 @@ class URLAndHash(object): padding = attr.ib() sha1_hash = attr.ib() - def as_bytes(self): - # XXX: The "padding" byte MUST be 0x01. It is - # present to make the structure backwards - # compatible. - # Should the validation for padding go here? - if self.padding != 1: - raise InvalidPadding - return _constructs.URLAndHash.build(Container( - length=len(self.url), - url=self.url, - padding=self.padding, - sha1_hash=self.sha1_hash, - )) + # XXX: The "padding" byte MUST be 0x01. It is + # present to make the structure backwards + # compatible. + # Should the validation for padding go here? @attr.s @@ -239,14 +230,21 @@ class CertificateURL(object): url_and_hash_list = attr.ib() def as_bytes(self): - return _constructs.CertificateURL.build( - Container( - type=self.type, - url_and_hash_list=b''.join( - url_and_hash.as_bytes() - for url_and_hash in self.url_and_hash_list + for url_and_hash in self.url_and_hash_list: + if url_and_hash.padding != 1: + raise InvalidPaddingException + return _constructs.CertificateURL.build(Container( + type=self.type, + url_and_hash_list=ListContainer( + Container( + length=len(url_and_hash.url), + url=url_and_hash.url, + padding=url_and_hash.padding, + sha1_hash=url_and_hash.sha1_hash, ) - )) + for url_and_hash in self.url_and_hash_list + ) + )) @classmethod def from_bytes(cls, bytes): @@ -262,7 +260,7 @@ def from_bytes(cls, bytes): # TODO: Figure how attrs does validation and move this assert to # URLAndHash if url_and_hash.padding != 1: - raise InvalidPadding + raise InvalidPaddingException url_and_hash_list.append(URLAndHash( url=url_and_hash.url, padding=url_and_hash.padding, diff --git a/tls/test/test_message.py b/tls/test/test_message.py index 25e9a8a..c39959f 100644 --- a/tls/test/test_message.py +++ b/tls/test/test_message.py @@ -10,7 +10,7 @@ from tls._common import enums -from tls.exceptions import InvalidPadding +from tls.exceptions import InvalidPaddingException from tls.hello_message import ClientHello, ProtocolVersion, ServerHello @@ -314,6 +314,10 @@ def test_parse_certificate_url(self): assert record.url_and_hash_list[0].sha1_hash == b'abcdefghijklmnopqrst' def test_incorrect_padding(self): + """ + :py:meth:`tls.message.CertificateURL.from_bytes` rejects a packet + whose ``padding`` is not 1. + """ bad_padding_bytes = ( b'\x00\x10' # url length b'cert.example.com' # url @@ -325,11 +329,27 @@ def test_incorrect_padding(self): b'\x00\x27' # url_and_hash_list length ) + bad_padding_bytes - with pytest.raises(InvalidPadding): + with pytest.raises(InvalidPaddingException): CertificateURL.from_bytes(certificate_url_packet) -# with pytest.raises(ValidationError): + def test_as_bytes(self): + """ + :py:meth:`tls.message.CertificateUrl.as_bytes` returns a valid + packet. + """ + record = CertificateURL.from_bytes(self.certificate_url_packet) + assert record.as_bytes() == self.certificate_url_packet + + def test_as_bytes_with_bad_padding(self): + """ + :py:meth:`tls.message.CertificateURL.as_bytes` fails to serialize a + record whose ``padding`` is not 1. + """ + record = CertificateURL.from_bytes(self.certificate_url_packet) + record.url_and_hash_list[0].padding = 5 + with pytest.raises(InvalidPaddingException): + record.as_bytes() class TestHandshakeStructParsing(object): From 188d7a3fc54f3fda4be02bcff64714e0980feb68 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Thu, 1 Dec 2016 11:12:56 +0530 Subject: [PATCH 10/27] whitespace --- tls/test/test_message.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tls/test/test_message.py b/tls/test/test_message.py index c39959f..ebc713f 100644 --- a/tls/test/test_message.py +++ b/tls/test/test_message.py @@ -340,7 +340,6 @@ def test_as_bytes(self): record = CertificateURL.from_bytes(self.certificate_url_packet) assert record.as_bytes() == self.certificate_url_packet - def test_as_bytes_with_bad_padding(self): """ :py:meth:`tls.message.CertificateURL.as_bytes` fails to serialize a From c1917f2f2bf192c4f07b28d66ee440f06d16b3d6 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Thu, 1 Dec 2016 11:13:56 +0530 Subject: [PATCH 11/27] Remove duplicate note --- tls/_constructs.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tls/_constructs.py b/tls/_constructs.py index c3eef1f..041c03e 100644 --- a/tls/_constructs.py +++ b/tls/_constructs.py @@ -202,8 +202,6 @@ min_size=1, max_size=2 ** 16 - 1), Bytes("url", lambda ctx: ctx.length), UBInt8("padding"), - # The "padding" byte MUST be 0x01. It is present to make the structure - # backwards compatible. Bytes("sha1_hash", 20), ) From 41c871442f3d028afac7166c5190415018cf2768 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Thu, 1 Dec 2016 11:34:49 +0530 Subject: [PATCH 12/27] Add a test to parse a ClientHello message with CLIENT_CERTIFICATE_URL extension --- tls/test/test_hello_message.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tls/test/test_hello_message.py b/tls/test/test_hello_message.py index 1fa7862..2fabd70 100644 --- a/tls/test/test_hello_message.py +++ b/tls/test/test_hello_message.py @@ -219,6 +219,26 @@ def test_hello_from_bytes_with_unsupported_extension(self): client_hello_packet ) + def test_parse_client_certificate_url_extension(self): + """ + :py:func:`tls.hello_message.ClientHello` parses a packet with + CLIENT_CERTIFICATE_URL extension. + """ + client_certificate_url_extension = ( + b'\x00\x02' # Extension Type: Server Certificate Type + b'\x00\x00' # Length + b'' # Data + ) + client_hello_packet = self.common_client_hello_data + ( + b'\x00\x04' + ) + client_certificate_url_extension + + record = ClientHello.from_bytes(client_hello_packet) + assert len(record.extensions) == 1 + assert (record.extensions[0].type == + enums.ExtensionType.CLIENT_CERTIFICATE_URL) + assert record.extensions[0].data is None + def test_as_bytes_unsupported_extension(self): """ :func:`ClientHello.as_bytes` fails to serialize a message that From 346a63b8f3b0f84a3aa7492397a8cb8d767c611e Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Thu, 1 Dec 2016 11:44:46 +0530 Subject: [PATCH 13/27] Add a test for serializing a ClientHello message with CLIENT_CERTIFICATE_URL extension --- tls/test/test_hello_message.py | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/tls/test/test_hello_message.py b/tls/test/test_hello_message.py index 2fabd70..325d76b 100644 --- a/tls/test/test_hello_message.py +++ b/tls/test/test_hello_message.py @@ -104,6 +104,15 @@ class TestClientHello(object): b'\x00\x12' ) + server_name_extension_data + client_certificate_url_extension = ( + b'\x00\x02' # Extension Type: Server Certificate Type + b'\x00\x00' # Length + b'' # Data + ) + client_hello_packet_with_client_certificate_url_extension = common_client_hello_data + ( + b'\x00\x04' + ) + client_certificate_url_extension + def test_resumption_no_extensions(self): """ :func:`parse_client_hello` returns an instance of @@ -224,21 +233,20 @@ def test_parse_client_certificate_url_extension(self): :py:func:`tls.hello_message.ClientHello` parses a packet with CLIENT_CERTIFICATE_URL extension. """ - client_certificate_url_extension = ( - b'\x00\x02' # Extension Type: Server Certificate Type - b'\x00\x00' # Length - b'' # Data - ) - client_hello_packet = self.common_client_hello_data + ( - b'\x00\x04' - ) + client_certificate_url_extension - - record = ClientHello.from_bytes(client_hello_packet) + record = ClientHello.from_bytes(self.client_hello_packet_with_client_certificate_url_extension) assert len(record.extensions) == 1 assert (record.extensions[0].type == enums.ExtensionType.CLIENT_CERTIFICATE_URL) assert record.extensions[0].data is None + def test_as_bytes_client_certificate_url_extension(self): + """ + :py:func:`tls.hello_message.ClientHello` serializes a message + containing the CLIENT_CERTIFICATE_URL extension. + """ + record = ClientHello.from_bytes(self.client_hello_packet_with_client_certificate_url_extension) + assert record.as_bytes() == self.client_hello_packet_with_client_certificate_url_extension + def test_as_bytes_unsupported_extension(self): """ :func:`ClientHello.as_bytes` fails to serialize a message that From 3aa50d15cb2240738e38e045e0df037477b7a1fa Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Thu, 1 Dec 2016 12:01:15 +0530 Subject: [PATCH 14/27] Define a minimal construct for the ClientCertificateURL extension so when we serialize it, we get the extension_data.length bytes (if only '\x00\x00') properly --- tls/_constructs.py | 8 ++++++++ tls/test/test_hello_message.py | 6 ++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/tls/_constructs.py b/tls/_constructs.py index 041c03e..7f0da30 100644 --- a/tls/_constructs.py +++ b/tls/_constructs.py @@ -84,6 +84,11 @@ ServerNameList = TLSPrefixedArray("server_name_list", ServerName) +ClientCertificateURL = Struct( + "client_certificate_url", + # The "extension_data" field of this extension SHALL be empty. +) + SignatureAndHashAlgorithm = Struct( "algorithms", EnumClass(UBInt8("hash"), enums.HashAlgorithm), @@ -107,6 +112,9 @@ enums.ExtensionType.SIGNATURE_ALGORITHMS: Opaque( SupportedSignatureAlgorithms ), + enums.ExtensionType.CLIENT_CERTIFICATE_URL: Opaque( + ClientCertificateURL + ), }, default=Pass, ) diff --git a/tls/test/test_hello_message.py b/tls/test/test_hello_message.py index 325d76b..b41f5bd 100644 --- a/tls/test/test_hello_message.py +++ b/tls/test/test_hello_message.py @@ -233,11 +233,13 @@ def test_parse_client_certificate_url_extension(self): :py:func:`tls.hello_message.ClientHello` parses a packet with CLIENT_CERTIFICATE_URL extension. """ - record = ClientHello.from_bytes(self.client_hello_packet_with_client_certificate_url_extension) + record = ClientHello.from_bytes( + self.client_hello_packet_with_client_certificate_url_extension + ) assert len(record.extensions) == 1 assert (record.extensions[0].type == enums.ExtensionType.CLIENT_CERTIFICATE_URL) - assert record.extensions[0].data is None + assert record.extensions[0].data == {} def test_as_bytes_client_certificate_url_extension(self): """ From 4be9ced610905b6bea603b23752a949ed51d2f6a Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Thu, 1 Dec 2016 12:04:05 +0530 Subject: [PATCH 15/27] pep8 --- tls/test/test_hello_message.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tls/test/test_hello_message.py b/tls/test/test_hello_message.py index b41f5bd..0ee8be4 100644 --- a/tls/test/test_hello_message.py +++ b/tls/test/test_hello_message.py @@ -109,7 +109,9 @@ class TestClientHello(object): b'\x00\x00' # Length b'' # Data ) - client_hello_packet_with_client_certificate_url_extension = common_client_hello_data + ( + + client_hello_packet_with_client_certificate_url_extension = ( + common_client_hello_data + b'\x00\x04' ) + client_certificate_url_extension @@ -246,8 +248,11 @@ def test_as_bytes_client_certificate_url_extension(self): :py:func:`tls.hello_message.ClientHello` serializes a message containing the CLIENT_CERTIFICATE_URL extension. """ - record = ClientHello.from_bytes(self.client_hello_packet_with_client_certificate_url_extension) - assert record.as_bytes() == self.client_hello_packet_with_client_certificate_url_extension + record = ClientHello.from_bytes( + self.client_hello_packet_with_client_certificate_url_extension + ) + assert (record.as_bytes() == + self.client_hello_packet_with_client_certificate_url_extension) def test_as_bytes_unsupported_extension(self): """ From 3e1758aa9b3ec3e48e814bcef4b8aceccb51bf2b Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Thu, 1 Dec 2016 13:27:04 +0530 Subject: [PATCH 16/27] Add CertificateURL as a handshake message type and parse Handshake structs containing CertificateURL messages --- tls/_common/enums.py | 2 ++ tls/message.py | 8 ++++++-- tls/test/test_message.py | 43 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/tls/_common/enums.py b/tls/_common/enums.py index 6ac7089..acba44c 100644 --- a/tls/_common/enums.py +++ b/tls/_common/enums.py @@ -45,6 +45,8 @@ class HandshakeType(Enum): CERTIFICATE_VERIFY = 15 CLIENT_KEY_EXCHANGE = 16 FINISHED = 20 + CERTIFICATE_URL = 21 + CERTIFICATE_STATUS = 22 class ContentType(Enum): diff --git a/tls/message.py b/tls/message.py index 8e5142b..924ac7a 100644 --- a/tls/message.py +++ b/tls/message.py @@ -299,7 +299,8 @@ def as_bytes(self): enums.HandshakeType.CERTIFICATE_REQUEST, enums.HandshakeType.HELLO_REQUEST, enums.HandshakeType.SERVER_HELLO_DONE, - enums.HandshakeType.FINISHED + enums.HandshakeType.FINISHED, + enums.HandshakeType.CERTIFICATE_URL, ]: _body_as_bytes = self.body.as_bytes() else: @@ -340,6 +341,8 @@ def _get_handshake_message(msg_type, body): CertificateRequest.from_bytes, # 15: parse_certificate_verify, # 16: parse_client_key_exchange, + enums.HandshakeType.CERTIFICATE_URL: CertificateURL.from_bytes, + # 22: parse certificate_status, } try: @@ -351,7 +354,8 @@ def _get_handshake_message(msg_type, body): return Finished(verify_data=body) elif msg_type in [enums.HandshakeType.SERVER_KEY_EXCHANGE, enums.HandshakeType.CERTIFICATE_VERIFY, - enums.HandshakeType.CLIENT_KEY_EXCHANGE]: + enums.HandshakeType.CLIENT_KEY_EXCHANGE, + enums.HandshakeType.CERTIFICATE_STATUS]: raise NotImplementedError else: return _handshake_message_parser[msg_type](body) diff --git a/tls/test/test_message.py b/tls/test/test_message.py index ebc713f..c38b1d2 100644 --- a/tls/test/test_message.py +++ b/tls/test/test_message.py @@ -16,7 +16,8 @@ from tls.message import (ASN1Cert, Certificate, CertificateRequest, CertificateURL, Finished, Handshake, HelloRequest, - PreMasterSecret, ServerDHParams, ServerHelloDone) + PreMasterSecret, ServerDHParams, ServerHelloDone, + URLAndHash) class TestCertificateRequestParsing(object): @@ -455,6 +456,20 @@ class TestHandshakeStructParsing(object): b'some-encrypted-bytes' ) + certificate_url_packet = ( + b'\x00' # CertChainType + b'\x00\x27' # url_and_hash_list length + b'\x00\x10' # url length + b'cert.example.com' # url + b'\x01' # padding + b'abcdefghijklmnopqrst' # SHA1Hash[20] + ) + + certificate_url_handshake_packet = ( + b'\x15' + b'\x00\x00\x2a' + ) + certificate_url_packet + def test_parse_client_hello_in_handshake(self): record = Handshake.from_bytes(self.client_hello_handshake_packet) assert isinstance(record, Handshake) @@ -545,3 +560,29 @@ def test_as_bytes_server_hello_done(self): def test_as_bytes_finished(self): record = Handshake.from_bytes(self.finished_handshake) assert record.as_bytes() == self.finished_handshake + + def test_from_bytes_certificate_url(self): + """ + :py:class:`tls.messages.Handshake` parses a valid packet with a + ``CertificateURL`` message. + """ + record = Handshake.from_bytes(self.certificate_url_handshake_packet) + assert isinstance(record, Handshake) + assert record.msg_type == enums.HandshakeType.CERTIFICATE_URL + assert record.length == 42 + assert isinstance(record.body, CertificateURL) + assert record.body.type == enums.CertChainType.INDIVIDUAL_CERTS + assert len(record.body.url_and_hash_list) == 1 + assert isinstance(record.body.url_and_hash_list[0], URLAndHash) + assert record.body.url_and_hash_list[0].url == b'cert.example.com' + assert record.body.url_and_hash_list[0].padding == 1 + assert (record.body.url_and_hash_list[0].sha1_hash == + b'abcdefghijklmnopqrst') + + def test_as_bytes_certificate_url(self): + """ + :py:meth:`tls.message.Handshake.as_bytes` returns a valid packet when + the body contains a ``CertificateURL`` message. + """ + record = Handshake.from_bytes(self.certificate_url_handshake_packet) + assert record.as_bytes() == self.certificate_url_handshake_packet From 17a32807cdfc7d0060301bc116984a7384ea9882 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Fri, 2 Dec 2016 19:42:18 -0800 Subject: [PATCH 17/27] Define a TLSException that other more specific exceptions inherit from --- tls/exceptions.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tls/exceptions.py b/tls/exceptions.py index 831c93e..3037b96 100644 --- a/tls/exceptions.py +++ b/tls/exceptions.py @@ -5,16 +5,21 @@ from __future__ import absolute_import, division, print_function -class UnsupportedCipherException(Exception): +class TLSException(Exception): + """ + This is the root exception from which all other exceptions inherit. + Lower-level parsing code raises very specific exceptions that higher-level + code can catch with this exception. + """ + + +class UnsupportedCipherException(TLSException): pass -class UnsupportedExtensionException(Exception): +class UnsupportedExtensionException(TLSException): pass -class InvalidPaddingException(Exception): - # XXX: Is leaking this info in CertificateURL (i.e. revealing the specific - # byte that is invalid) a good idea? Need to check -- could be a - # vulnerability in disguise. +class InvalidPaddingException(TLSException): pass From 18c869db4c774020779bcd9d7d3bd586a81ded7d Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Fri, 2 Dec 2016 20:22:58 -0800 Subject: [PATCH 18/27] Validate padding early, and a failing test --- tls/_constructs.py | 6 +++--- tls/message.py | 6 +++--- tls/test/test_message.py | 18 +++++++++--------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/tls/_constructs.py b/tls/_constructs.py index 7f0da30..6eb24cc 100644 --- a/tls/_constructs.py +++ b/tls/_constructs.py @@ -6,8 +6,8 @@ from functools import partial -from construct import (Array, Bytes, Pass, Struct, Switch, UBInt16, UBInt32, - UBInt8) +from construct import (Array, Bytes, OneOf, Pass, Struct, Switch, UBInt16, + UBInt32, UBInt8) from tls._common import enums @@ -209,7 +209,7 @@ SizeWithin(UBInt16("length"), min_size=1, max_size=2 ** 16 - 1), Bytes("url", lambda ctx: ctx.length), - UBInt8("padding"), + OneOf(UBInt8('padding'), [1]), Bytes("sha1_hash", 20), ) diff --git a/tls/message.py b/tls/message.py index 924ac7a..526e720 100644 --- a/tls/message.py +++ b/tls/message.py @@ -230,9 +230,9 @@ class CertificateURL(object): url_and_hash_list = attr.ib() def as_bytes(self): - for url_and_hash in self.url_and_hash_list: - if url_and_hash.padding != 1: - raise InvalidPaddingException + #for url_and_hash in self.url_and_hash_list: + # if url_and_hash.padding != 1: + # raise InvalidPaddingException return _constructs.CertificateURL.build(Container( type=self.type, url_and_hash_list=ListContainer( diff --git a/tls/test/test_message.py b/tls/test/test_message.py index c38b1d2..6027133 100644 --- a/tls/test/test_message.py +++ b/tls/test/test_message.py @@ -8,6 +8,8 @@ import pytest +from tls import _constructs + from tls._common import enums from tls.exceptions import InvalidPaddingException @@ -314,9 +316,9 @@ def test_parse_certificate_url(self): assert record.url_and_hash_list[0].padding == 1 assert record.url_and_hash_list[0].sha1_hash == b'abcdefghijklmnopqrst' - def test_incorrect_padding(self): + def test_incorrect_padding_parsing(self): """ - :py:meth:`tls.message.CertificateURL.from_bytes` rejects a packet + :py:meth:`tls._constructs.URLAndHash.parse` rejects a packet whose ``padding`` is not 1. """ bad_padding_bytes = ( @@ -325,13 +327,11 @@ def test_incorrect_padding(self): b'\x03' # padding b'abcdefghijklmnopqrst' # SHA1Hash[20] ) - certificate_url_packet = ( - b'\x00' # CertChainType - b'\x00\x27' # url_and_hash_list length - ) + bad_padding_bytes - with pytest.raises(InvalidPaddingException): - CertificateURL.from_bytes(certificate_url_packet) + with pytest.raises(ValidationError) as exc_info: + _constructs.URLAndHash.parse(bad_padding_bytes) + + assert exc_info.value.args == ('invalid object', 3) def test_as_bytes(self): """ @@ -348,7 +348,7 @@ def test_as_bytes_with_bad_padding(self): """ record = CertificateURL.from_bytes(self.certificate_url_packet) record.url_and_hash_list[0].padding = 5 - with pytest.raises(InvalidPaddingException): + with pytest.raises(ValidationError): record.as_bytes() From f839d07075dd33275be819dd32b9d424c84b94f0 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Fri, 2 Dec 2016 21:30:43 -0800 Subject: [PATCH 19/27] Define a custom TLSOneOf method that uses TLSExprValidator to raise a TLSValidationException that's not a ConstructError so that Range implementation (and hence TLSPrefixedArray) doesn't break --- tls/_common/_constructs.py | 28 ++++++++++++++++++++++++++++ tls/_constructs.py | 10 ++++++---- tls/exceptions.py | 4 ++++ tls/test/test_message.py | 15 ++++++++++----- 4 files changed, 48 insertions(+), 9 deletions(-) diff --git a/tls/_common/_constructs.py b/tls/_common/_constructs.py index 75ddd24..216b79e 100644 --- a/tls/_common/_constructs.py +++ b/tls/_common/_constructs.py @@ -10,6 +10,8 @@ import six +from tls.exceptions import TLSValidationException + class _UBInt24(construct.Adapter): def _encode(self, obj, context): @@ -211,6 +213,32 @@ def EnumSwitch(type_field, type_enum, value_field, value_choices, # noqa default=default)) +class TLSExprValidator(construct.Validator): + """ + Like :py:class:`construct.ExprValidator`, but raises a + :py:class:`tls.exceptions.TLSValidationException` on validation failure. + """ + def __init__(self, subcon, validator): + super(TLSExprValidator, self).__init__(subcon) + self._validate = validator + + def _decode(self, obj, context): + if not self._validate(obj, context): + raise TLSValidationException("object failed validation", obj) + return obj + + +def TLSOneOf(subcon, valids): + """ + Validates that the object is one of the listed values, both during + parsing and building. Like :py:meth:`construct.OneOf`, but raises a + :py:class:`tls.exceptions.TLSValidationException` on mismatch. + + This is necessary because + """ + return TLSExprValidator(subcon, lambda obj, ctx: obj in valids) + + class SizeAtLeast(construct.Validator): """ A :py:class:`construct.adapter.Validator` that validates a diff --git a/tls/_constructs.py b/tls/_constructs.py index 6eb24cc..e5bc8ed 100644 --- a/tls/_constructs.py +++ b/tls/_constructs.py @@ -6,14 +6,16 @@ from functools import partial -from construct import (Array, Bytes, OneOf, Pass, Struct, Switch, UBInt16, - UBInt32, UBInt8) +from construct import (Array, Bytes, Pass, Struct, + Switch, UBInt16, UBInt32, + UBInt8) from tls._common import enums from tls._common._constructs import (EnumClass, EnumSwitch, Opaque, PrefixedBytes, SizeAtLeast, SizeAtMost, - SizeWithin, TLSPrefixedArray, UBInt24) + SizeWithin, TLSOneOf, TLSPrefixedArray, + UBInt24) from tls.ciphersuites import CipherSuites @@ -209,7 +211,7 @@ SizeWithin(UBInt16("length"), min_size=1, max_size=2 ** 16 - 1), Bytes("url", lambda ctx: ctx.length), - OneOf(UBInt8('padding'), [1]), + TLSOneOf(UBInt8('padding'), [1]), Bytes("sha1_hash", 20), ) diff --git a/tls/exceptions.py b/tls/exceptions.py index 3037b96..e308a98 100644 --- a/tls/exceptions.py +++ b/tls/exceptions.py @@ -23,3 +23,7 @@ class UnsupportedExtensionException(TLSException): class InvalidPaddingException(TLSException): pass + + +class TLSValidationException(TLSException): + pass diff --git a/tls/test/test_message.py b/tls/test/test_message.py index 6027133..7c9f245 100644 --- a/tls/test/test_message.py +++ b/tls/test/test_message.py @@ -12,7 +12,7 @@ from tls._common import enums -from tls.exceptions import InvalidPaddingException +from tls.exceptions import InvalidPaddingException, TLSValidationException from tls.hello_message import ClientHello, ProtocolVersion, ServerHello @@ -328,10 +328,15 @@ def test_incorrect_padding_parsing(self): b'abcdefghijklmnopqrst' # SHA1Hash[20] ) - with pytest.raises(ValidationError) as exc_info: - _constructs.URLAndHash.parse(bad_padding_bytes) + certificate_url_packet = ( + b'\x00' # CertChainType + b'\x00\x27' # url_and_hash_list length + ) + bad_padding_bytes + + with pytest.raises(TLSValidationException) as exc_info: + CertificateURL.from_bytes(certificate_url_packet) - assert exc_info.value.args == ('invalid object', 3) + assert exc_info.value.args == ('object failed validation', 3) def test_as_bytes(self): """ @@ -348,7 +353,7 @@ def test_as_bytes_with_bad_padding(self): """ record = CertificateURL.from_bytes(self.certificate_url_packet) record.url_and_hash_list[0].padding = 5 - with pytest.raises(ValidationError): + with pytest.raises(TLSValidationException): record.as_bytes() From e9b6c84aec2768568ca39c3710bf1eac5a04c9b6 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Fri, 2 Dec 2016 21:33:41 -0800 Subject: [PATCH 20/27] pep8 + docstrings --- tls/_common/_constructs.py | 15 +++++++++------ tls/exceptions.py | 4 ---- tls/message.py | 3 --- tls/test/test_message.py | 4 +--- 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/tls/_common/_constructs.py b/tls/_common/_constructs.py index 216b79e..d8e6a46 100644 --- a/tls/_common/_constructs.py +++ b/tls/_common/_constructs.py @@ -228,13 +228,16 @@ def _decode(self, obj, context): return obj -def TLSOneOf(subcon, valids): +def TLSOneOf(subcon, valids): # noqa """ - Validates that the object is one of the listed values, both during - parsing and building. Like :py:meth:`construct.OneOf`, but raises a - :py:class:`tls.exceptions.TLSValidationException` on mismatch. - - This is necessary because + Validates that the object is one of the listed values, both during parsing + and building. Like :py:meth:`construct.OneOf`, but raises a + :py:class:`tls.exceptions.TLSValidationException` instead of a + ``ConstructError`` subclass on mismatch. + + This is necessary because any ConstructError signifies the end of + subconstruct repetition to Range, which in turn breaks use with + ``TLSPrefixedArray``. """ return TLSExprValidator(subcon, lambda obj, ctx: obj in valids) diff --git a/tls/exceptions.py b/tls/exceptions.py index e308a98..41f71a0 100644 --- a/tls/exceptions.py +++ b/tls/exceptions.py @@ -21,9 +21,5 @@ class UnsupportedExtensionException(TLSException): pass -class InvalidPaddingException(TLSException): - pass - - class TLSValidationException(TLSException): pass diff --git a/tls/message.py b/tls/message.py index 526e720..f7c4e18 100644 --- a/tls/message.py +++ b/tls/message.py @@ -230,9 +230,6 @@ class CertificateURL(object): url_and_hash_list = attr.ib() def as_bytes(self): - #for url_and_hash in self.url_and_hash_list: - # if url_and_hash.padding != 1: - # raise InvalidPaddingException return _constructs.CertificateURL.build(Container( type=self.type, url_and_hash_list=ListContainer( diff --git a/tls/test/test_message.py b/tls/test/test_message.py index 7c9f245..5b21292 100644 --- a/tls/test/test_message.py +++ b/tls/test/test_message.py @@ -8,11 +8,9 @@ import pytest -from tls import _constructs - from tls._common import enums -from tls.exceptions import InvalidPaddingException, TLSValidationException +from tls.exceptions import TLSValidationException from tls.hello_message import ClientHello, ProtocolVersion, ServerHello From 800ad1fe3f18841afafa5bdbb65ca6b037bf3646 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Fri, 2 Dec 2016 21:52:04 -0800 Subject: [PATCH 21/27] Delete outdated import --- tls/message.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tls/message.py b/tls/message.py index f7c4e18..7a14f2b 100644 --- a/tls/message.py +++ b/tls/message.py @@ -14,8 +14,6 @@ from tls._common import enums -from tls.exceptions import InvalidPaddingException - from tls.hello_message import ClientHello, ProtocolVersion, ServerHello From fe03620df12e927740138e86192652b053c56b00 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Sat, 3 Dec 2016 01:59:51 -0800 Subject: [PATCH 22/27] Tests for TLSExprValidator and TLSOneOf --- tls/_common/test/test_constructs.py | 116 ++++++++++++++++++++++++++-- tls/message.py | 4 - 2 files changed, 111 insertions(+), 9 deletions(-) diff --git a/tls/_common/test/test_constructs.py b/tls/_common/test/test_constructs.py index 514710f..7ea717d 100644 --- a/tls/_common/test/test_constructs.py +++ b/tls/_common/test/test_constructs.py @@ -12,12 +12,13 @@ import pytest -from tls._common._constructs import (BytesAdapter, EnumClass, - EnumSwitch, Opaque, - PrefixedBytes, SizeAtLeast, +from tls._common._constructs import (BytesAdapter, EnumClass, EnumSwitch, + Opaque, PrefixedBytes, SizeAtLeast, SizeAtMost, SizeWithin, - TLSPrefixedArray, UBInt24, - _UBInt24) + TLSExprValidator, TLSOneOf, + TLSPrefixedArray, UBInt24, _UBInt24) + +from tls.exceptions import TLSValidationException @pytest.mark.parametrize("byte,number", [ @@ -94,6 +95,111 @@ def test_decode_passes_value_through(self, bytes_adapted, value): assert bytes_adapted._decode(value, context=object()) is value +class TestTLSExprValidator(object): + """ + Tests for :py:class:`tls._common._constructs.TLSExprValidator`. + """ + @pytest.fixture + def data_class(self): + """ + A :py:func:`construct.macros.UBInt8` construct that requires the + input value to be equal to 6. + """ + return TLSExprValidator(UBInt8('input_byte'), + lambda obj, ctx: obj == 6) + + def test_parse_invalid(self, data_class): + """ + :py:class:`tls.common._constructs.TLSExprValidator` raises a + ``TLSValidationException`` when parsing a value that does not + evaluate to the provided expression. + """ + with pytest.raises(TLSValidationException): + data_class.parse(b'\xff') + + def test_parse_valid(self, data_class): + """ + :py:class:`tls.common._constructs.TLSExprValidator` parses a value + that evaluates to the provided expression. + """ + assert data_class.parse(b'\x06') == 6 + + def test_build_invalid(self, data_class): + """ + :py:class:`tls.common._constructs.TLSExprValidator` raises a + ``TLSValidationException`` when serializing a value that does not + evaluate to the provided expression. + """ + with pytest.raises(TLSValidationException): + data_class.build(2) + + def test_build_valid(self, data_class): + """ + :py:class:`tls.common._construct.TLSExprValidator` successfully + serializes a value into bytes when it evaluates to the provided + expression. + """ + assert data_class.build(6) == b'\x06' + + +class TestTLSOneOf(object): + """ + Tests for :py:meth:`tls._common._constructs.TLSOneOf`. + """ + + @pytest.fixture + def data_class(self): + """ + A :py:func:`construct.macros.UBInt8` construct that requires the + input value to be equal to one of 1, 3, or 5. + """ + return TLSOneOf(UBInt8('input'), + [1, 3, 5]) + + def test_parse_invalid(self, data_class): + """ + :py:meth:`tls.common._constructs.TLSOneOf` raises a + ``TLSValidationException`` when parsing a value that is not one of + the values in the provided list. + """ + with pytest.raises(TLSValidationException): + data_class.parse(b'\xff') + + @pytest.mark.parametrize('input_bytes,parsed_output', [ + (b'\x01', 1), + (b'\x03', 3), + (b'\x05', 5), + ]) + def test_parse_valid(self, data_class, input_bytes, parsed_output): + """ + :py:meth:`tls.common._constructs.TLSOneOf` parses a value that + equals one of the values in the provided list. + """ + assert data_class.parse(input_bytes) == parsed_output + + def test_build_invalid(self, data_class): + """ + :py:meth:`tls.common._constructs.TLSOneOf` raises a + ``TLSValidationException`` when serializing a value that is not one + of the values in the provided list. + """ + with pytest.raises(TLSValidationException): + data_class.build(2) + + @pytest.mark.parametrize('input,built_bytes', [ + (1, b'\x01'), + (3, b'\x03'), + (5, b'\x05'), + ]) + def test_build_valid(self, data_class, input, built_bytes): + """ + :py:meth:`tls.common._construct.TLSOneOf` successfully serializes a + value into bytes when it evaluates to one of the values in the + provided list. + """ + assert data_class.build(input) == built_bytes + + @pytest.mark.parametrize("bytestring,encoded", [ (b"", b"\x00" + b""), (b"some value", b"\x0A" + b"some value"), diff --git a/tls/message.py b/tls/message.py index 7a14f2b..ee1a787 100644 --- a/tls/message.py +++ b/tls/message.py @@ -252,10 +252,6 @@ def from_bytes(cls, bytes): construct = _constructs.CertificateURL.parse(bytes) url_and_hash_list = [] for url_and_hash in construct.url_and_hash_list: - # TODO: Figure how attrs does validation and move this assert to - # URLAndHash - if url_and_hash.padding != 1: - raise InvalidPaddingException url_and_hash_list.append(URLAndHash( url=url_and_hash.url, padding=url_and_hash.padding, From 3ede2d683a056fb0e92542dc3fe4ca2ab17fdad7 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Sat, 3 Dec 2016 02:03:41 -0800 Subject: [PATCH 23/27] Use list comprehension to build the url_and_hash_list --- tls/message.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/tls/message.py b/tls/message.py index ee1a787..ff9b94b 100644 --- a/tls/message.py +++ b/tls/message.py @@ -250,17 +250,15 @@ def from_bytes(cls, bytes): :return: CertificateURL object. """ construct = _constructs.CertificateURL.parse(bytes) - url_and_hash_list = [] - for url_and_hash in construct.url_and_hash_list: - url_and_hash_list.append(URLAndHash( - url=url_and_hash.url, - padding=url_and_hash.padding, - sha1_hash=url_and_hash.sha1_hash, - )) - return cls( type=construct.type, - url_and_hash_list=url_and_hash_list, + url_and_hash_list=[ + URLAndHash( + url=url_and_hash.url, + padding=url_and_hash.padding, + sha1_hash=url_and_hash.sha1_hash, + ) + for url_and_hash in construct.url_and_hash_list], ) From cd8d51859aa8cb53a613f2603cbbb39ca825a626 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Sat, 3 Dec 2016 02:12:54 -0800 Subject: [PATCH 24/27] Tell travis that subconstruct is a thing --- docs/spelling_wordlist.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index 3fcdad8..b7b6944 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -15,6 +15,7 @@ Submodules Subpackages ciphersuites prepending +subconstruct subconstructs versa prf From 77e451d7f5324edd42ae896ea03b70674ab1e1b4 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Sat, 3 Dec 2016 02:21:22 -0800 Subject: [PATCH 25/27] remove inline XXX --- tls/message.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tls/message.py b/tls/message.py index ff9b94b..3722b7e 100644 --- a/tls/message.py +++ b/tls/message.py @@ -213,11 +213,6 @@ class URLAndHash(object): padding = attr.ib() sha1_hash = attr.ib() - # XXX: The "padding" byte MUST be 0x01. It is - # present to make the structure backwards - # compatible. - # Should the validation for padding go here? - @attr.s class CertificateURL(object): From 1cc0cd444facb1ab63db4d3ed27ae02f9df5fe24 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Mon, 5 Dec 2016 22:40:52 -0800 Subject: [PATCH 26/27] Data is really a Container, asserting to that is better than asserting to '{}' (Container subclasses dict) --- tls/test/test_hello_message.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tls/test/test_hello_message.py b/tls/test/test_hello_message.py index 0ee8be4..8a424c9 100644 --- a/tls/test/test_hello_message.py +++ b/tls/test/test_hello_message.py @@ -4,6 +4,8 @@ from __future__ import absolute_import, division, print_function +from construct import Container + from construct.adapters import ValidationError import pytest @@ -241,7 +243,7 @@ def test_parse_client_certificate_url_extension(self): assert len(record.extensions) == 1 assert (record.extensions[0].type == enums.ExtensionType.CLIENT_CERTIFICATE_URL) - assert record.extensions[0].data == {} + assert record.extensions[0].data == Container() def test_as_bytes_client_certificate_url_extension(self): """ From dd6936511e2a69081b1c4c6623c8c032ae94317b Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Mon, 5 Dec 2016 22:44:24 -0800 Subject: [PATCH 27/27] Update docstring for TLSExprValidator to include why we needed to reimplement it --- tls/_common/_constructs.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tls/_common/_constructs.py b/tls/_common/_constructs.py index d8e6a46..c20ff5d 100644 --- a/tls/_common/_constructs.py +++ b/tls/_common/_constructs.py @@ -217,6 +217,10 @@ class TLSExprValidator(construct.Validator): """ Like :py:class:`construct.ExprValidator`, but raises a :py:class:`tls.exceptions.TLSValidationException` on validation failure. + + This is necessary because any ConstructError signifies the end of + subconstruct repetition to Range, which in turn breaks use with + ``TLSPrefixedArray``. """ def __init__(self, subcon, validator): super(TLSExprValidator, self).__init__(subcon)