SNI extension

[ashfall]

Implementing as specified in https://tools.ietf.org/html/rfc6066#section-3

[coveralls]

Coverage remained the same at 100.0% when pulling 412e91340c2281ff74049b55234a0be43ed29037 on ashfall:sni-extension into 67c2ef31297151043da2096d3e24a2f5092d234c on pyca:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 412e91340c2281ff74049b55234a0be43ed29037 on ashfall:sni-extension into 67c2ef31297151043da2096d3e24a2f5092d234c on pyca:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 406f198a23f58664e108247342cec1ea83a3c480 on ashfall:sni-extension into 67c2ef31297151043da2096d3e24a2f5092d234c on pyca:master.

[coveralls]

Coverage remained the same at 100.0% when pulling f3663a66613cf53158b979e852757036475d65b4 on ashfall:sni-extension into 67c2ef31297151043da2096d3e24a2f5092d234c on pyca:master.

[coveralls]

Coverage remained the same at 100.0% when pulling f3663a66613cf53158b979e852757036475d65b4 on ashfall:sni-extension into 67c2ef31297151043da2096d3e24a2f5092d234c on pyca:master.

[coveralls]

Coverage remained the same at 100.0% when pulling f3663a66613cf53158b979e852757036475d65b4 on ashfall:sni-extension into 67c2ef31297151043da2096d3e24a2f5092d234c on pyca:master.

[ashfall]

I'm not very happy about doing server_name_list[0].name.data and would rather 'localhost' be server_name_list[0].name, i.e. not have the HostName container as a thing. From the RFC (https://tools.ietf.org/html/rfc6066):

      struct {
          NameType name_type;
          select (name_type) {
              case host_name: HostName;
          } name;
      } ServerName;

      enum {
          host_name(0), (255)
      } NameType;

      opaque HostName<1..2^16-1>;

      struct {
          ServerName server_name_list<1..2^16-1>
      } ServerNameList;

Thoughts?

[coveralls]

Coverage remained the same at 100.0% when pulling df5c1744f77060e2cb612f76449f692be8b8aa85 on ashfall:sni-extension into 67c2ef31297151043da2096d3e24a2f5092d234c on pyca:master.

[coveralls]

Coverage decreased (-0.08%) to 99.924% when pulling 085f5d6cabbf7c67089935efac5a3561cfb900fc on ashfall:sni-extension into 67c2ef31297151043da2096d3e24a2f5092d234c on pyca:master.

[coveralls]

Coverage decreased (-0.08%) to 99.924% when pulling 085f5d6cabbf7c67089935efac5a3561cfb900fc on ashfall:sni-extension into 67c2ef31297151043da2096d3e24a2f5092d234c on pyca:master.

[coveralls]

Coverage decreased (-0.08%) to 99.924% when pulling 085f5d6cabbf7c67089935efac5a3561cfb900fc on ashfall:sni-extension into 67c2ef31297151043da2096d3e24a2f5092d234c on pyca:master.

[ashfall]

@markrwilliams Thanks for the review, updates coming!

I have a question for you about the EnumSwitch API. In order to satisfy the coverage requirement, I was writing a test with an unsupported extension in ClientHello:

    def test_client_hello_fails_with_unsupported_extension(self):
        """
        :py:func:`tls.hello_message.ClientHello` does not parse a packet
        with an unsupported extension, and raises an error.
        """
        server_certificate_type_extension_data = (
            b'\x00\x14'  # Extension Type: Server Certificate Type
            b'\x00\x00'  # Length
            b''  # Data
        )

        client_hello_packet = self.common_client_hello_data + (
            b'\x00\x04'  # extensions length
        ) + server_certificate_type_extension_data

        with pytest.raises(UnsupportedExtensionException):
            ClientHello.from_bytes(
                client_hello_packet
            )

Since we haven't implemented the server_certificate_type extension yet, Extension raised a SwitchError: no default case defined on parsing server_certificate_type_extension_data.

To define a default, EnumSwitch says:

    :param value_choices: A dictionary that maps members of
        `type_enum` to subconstructs.  This follows
        :py:func:`construct.core.Switch`'s API, so ``_default_`` will
        match any members without an explicit mapping.
    :type value_choices: :py:class:`dict`

and it was unclear to me how/where to provide the __default__.

To make my test pass, I hacked around a bit and made the following changes:

I modified EnumSwitch slightly, a rough implementation is below (the if-else is ugly, I should clean that up):

def EnumSwitch(type_field, type_enum, value_field, value_choices, default=None): 
    if not default:  # we don't want to overwrite Switch's default NoDefault (https://github.com/construct/construct/blob/master/construct/core.py#L1550)
        return (EnumClass(type_field, type_enum),
                construct.Switch(value_field,
                                 operator.attrgetter(type_field.name),
                                 value_choices))
    else:  
        return (EnumClass(type_field, type_enum),
                construct.Switch(value_field,
                                 operator.attrgetter(type_field.name),
                                 value_choices,
                                 default=default))

And added a default construct.Pass to the Extension construct:

Extension = Struct(
    "extensions",
    *EnumSwitch(
        type_field=UBInt16("type"),
        type_enum=enums.ExtensionType,
        value_field="data",
        value_choices={
            enums.ExtensionType.SERVER_NAME: Opaque(ServerNameList),
            enums.ExtensionType.SIGNATURE_ALGORITHMS: Opaque(
                SupportedSignatureAlgorithms
            ),
        },
        default=Pass,
    )
)

This made test_client_hello_fails_with_unsupported_extension pass.

All the hackery aside, is there an existing way to provide default values to EnumSwitch?

[coveralls]

Coverage decreased (-0.08%) to 99.924% when pulling 9d1e243331913d872dd494317969dcec3202d496 on ashfall:sni-extension into 67c2ef31297151043da2096d3e24a2f5092d234c on pyca:master.

[coveralls]

Coverage decreased (-0.08%) to 99.924% when pulling 9d1e243331913d872dd494317969dcec3202d496 on ashfall:sni-extension into 67c2ef31297151043da2096d3e24a2f5092d234c on pyca:master.

[coveralls]

Coverage decreased (-0.08%) to 99.924% when pulling 9d1e243331913d872dd494317969dcec3202d496 on ashfall:sni-extension into 67c2ef31297151043da2096d3e24a2f5092d234c on pyca:master.

[coveralls]

Coverage decreased (-0.08%) to 99.924% when pulling 9d1e243331913d872dd494317969dcec3202d496 on ashfall:sni-extension into 67c2ef31297151043da2096d3e24a2f5092d234c on pyca:master.

[coveralls]

Coverage decreased (-0.08%) to 99.925% when pulling 1edcbed94bf41a3a60a0e503a54b7d1afea511d4 on ashfall:sni-extension into 67c2ef31297151043da2096d3e24a2f5092d234c on pyca:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 7c77cc85cb99784ed5c4e007173613990904977e on ashfall:sni-extension into 5d48cc44364d44ba7251e470168276638224de6f on pyca:master.

[markrwilliams]

@ashfall This looks great! I'm going to merge. We can address any lingering concerns in subsequent PRs.

Thanks so much for implementing this!