Added support for TLS Application Layer Protocol Negotiation (ALPN)

[nmav]

This patch adds support for ALPN, based on draft-ietf-tls-applayerprotoneg-04.
It adds the new keywords 'ALPNtable' and 'prefer'. The latter accepts the
values 'sni' or 'alpn'.

Signed-off-by: Nikos Mavrogiannopoulos [email protected]

[dlundquist]

For consistency can we got with size_t name_len rather than unsigned name_size.

[dlundquist]

Overall looks great. I wanted to avoid creating a dependencies for the protocol parsers on other modules (besides logger module and protocol header), can we avoid passing a reference to the listener to parse_tls_header()?

Could you provide some sample ALPN client hello messages for tls_test.c? Could you make sure make check runs cleanly?

[nmav]

On Mon, Feb 17, 2014 at 7:29 PM, Dustin Lundquist
[email protected]:

Overall looks great. I wanted to avoid creating a dependencies for the
protocol parsers on other modules (besides logger module and protocol
header), can we avoid passing a reference to the listener to
parse_tls_header()?

I needed it so that it can access the information on the alpn_Table. I
don't see how I can avoid it, except from passing the ALPN table directly,
which looks quite worse. If you have any suggestions on that, it's welcome.

Could you provide some sample ALPN client hello messages for tls_test.c?

I've updated the request with tls_test.c as well.

regards,
Nikos

[dlundquist]

I haven't forgotten about this one. Hopefully I'll have some time this weekend to dig in further.

[nmav]

Hello,
Is there anything I can do to help this process?

[dlundquist]

Sorry for the long delay getting back to you, I haven't had much time to work on sniproxy over the last month. I'm still not real happy with the level of coupling this introduces and have been hacking on a local branch trying to find a more orthogonal way to introduce it.

From my reading about ALPN, I did have a few high level observations:

1. We need to find the first ALPN protocol match in common between sniproxy and the client.
   • parse_packet() must either know all the protocols configured or return a list of all the protocols the client advertises support for.
     • we could add a opaque protocol data pointer to parse_packet() arguments and pass in a list of the protocols configured in sniproxy, each listener would contain its own protocol_data with a duplicate list of supported ALPN protocols.
2. The ALPN protocol is an opaque string, and as such it could contain a null character.
   • PCRE match isn't quite appropriate
     • we could add an alternative function to table_lookup_server_address() which does a binary comparison

I'm still not clear in which cases both ALPN and SNI extensions would be present, or how that should be handled.

[nmav]

Hello,
About (2), I am not sure that protocols shouldn't be handled as strings and avoid pcre match. All the defined protocols are strings (http/2.0, http/1.0) and being able to match them with a regex is actually something that will be needed (e.g., match http/.*) . I also find it very unlikely to define protocols in the future that look something like 0x00060421. Nevertheless, the code could fallback to a simpler comparison if strlen()!=length, instead of forbidding pcre match completely.

About (1), I agree and this is the reason the Listener information was passed there. I don't know if passing fewer data there will actually provide any advantage, but it will be a bit cleaner separation as you say. I don't have much time to work on that now though.