Re: [PATCH] MINOR: acl: add support for TLS ALPN matching

2019-01-01 Thread Willy Tarreau
On Mon, Dec 31, 2018 at 08:36:01PM -0500, Alex Zorin wrote:
> On Tue, Jan 1, 2019, at 12:32 AM, Willy Tarreau wrote:
> > You're welcome. I was about to apply it until I figured that a doc update
> > is missing. Please add an entry to configuration.txt for this new keyword
> > (and take care of respecting the alphabetical order).
> 
> Okay, updated the docs. 

Perfect, this looks very good, and generally well documented (code, doc,
commit), thank you! I've merged it now.

> On reflection I was not able to discern the distinction of the sample fetch
> keyword list and the ACL keyword list in payload.c. I hope that having added
> this to the sample fetch list only was correct, even though it does not match
> the example set by "req_ssl_sni".

You're absolutely right. The ACLs are here for historic compatibility,
they provide a default matching method, which most of the time is now
properly picked thanks to the output type of the sample. Another
difference often lies in the keyword name. For example "req_ssl_sni"
used to be the name used in ACLs, while the sample fetch function is
"req.ssl_sni", so there's nothing which can map one to the other
automatically. The good choice is indeed to only add new keywords to
the sample fetch list as you did, so that they can be used everywhere.

Thanks,
Willy



Re: [PATCH] MINOR: acl: add support for TLS ALPN matching

2018-12-31 Thread Alex Zorin
On Tue, Jan 1, 2019, at 12:32 AM, Willy Tarreau wrote:
> You're welcome. I was about to apply it until I figured that a doc update
> is missing. Please add an entry to configuration.txt for this new keyword
> (and take care of respecting the alphabetical order).

Okay, updated the docs. 

On reflection I was not able to discern the distinction of the sample fetch 
keyword list and the ACL keyword list in payload.c. I hope that having added 
this to the sample fetch list only was correct, even though it does not match 
the example set by "req_ssl_sni".

Alex>From cc8877b49e862cd84ed5634f104500b6b743bc39 Mon Sep 17 00:00:00 2001
From: Alex Zorin 
Date: Sun, 30 Dec 2018 13:56:28 +1100
Subject: [PATCH] MINOR: payload: add sample fetch for TLS ALPN

Application-Layer Protocol Negotiation (ALPN, RFC7301) is a TLS
extension which allows a client to present a preference for which
protocols it wishes to connect to, when a single port supports multiple
multiple application protocols.
It allows a transparent proxy to take a decision based on the beginning
of an SSL/TLS stream without deciphering it.

The new fetch "req.ssl_alpn" extracts the ALPN protocol names that may
be present in the ClientHello message.
---
 doc/configuration.txt |  16 
 src/payload.c | 172 ++
 2 files changed, 188 insertions(+)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 6ca63d64..dc1f2224 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -15465,6 +15465,22 @@ rdp_cookie_cnt([name]) : integer (deprecated)
   ACL derivatives :
 req_rdp_cookie_cnt([]) : integer match
 
+req.ssl_alpn : string
+  Returns a string containing the values of the Application-Layer Protocol
+  Negotiation (ALPN) TLS extension (RFC7301), sent by the client within the SSL
+  ClientHello message. Note that this only applies to raw contents found in the
+  request buffer and not to the contents deciphered via an SSL data layer, so
+  this will not work with "bind" lines having the "ssl" option. This is useful
+  in ACL to make a routing decision based upon the ALPN preferences of a TLS
+  client, like in the example below.
+
+  Examples :
+ # Wait for a client hello for at most 5 seconds
+ tcp-request inspect-delay 5s
+ tcp-request content accept if { req_ssl_hello_type 1 }
+ use_backend bk_acme if { req_ssl.alpn acme-tls/1 }
+ default_backend bk_default
+
 req.ssl_ec_ext : boolean
   Returns a boolean identifying if client sent the Supported Elliptic Curves
   Extension as defined in RFC4492, section 5.1. within the SSL ClientHello
diff --git a/src/payload.c b/src/payload.c
index 7ef6d97e..a16f8c60 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -659,6 +659,177 @@ smp_fetch_ssl_hello_sni(const struct arg *args, struct sample *smp, const char *
 	return 0;
 }
 
+/* Try to extract the Application-Layer Protocol Negotiation (ALPN) protocol
+ * names that may be presented in a TLS client hello handshake message. As the
+ * message presents a list of protocol names in descending order of preference,
+ * it may return iteratively. The format of the message is the following
+ * (cf RFC5246 + RFC7301) :
+ * TLS frame :
+ *   - uint8  type= 0x16   (Handshake)
+ *   - uint16 version>= 0x0301 (TLSv1)
+ *   - uint16 length   (frame length)
+ *   - TLS handshake :
+ * - uint8  msg_type  = 0x01   (ClientHello)
+ * - uint24 length (handshake message length)
+ * - ClientHello :
+ *   - uint16 client_version >= 0x0301 (TLSv1)
+ *   - uint8 Random[32]  (4 first ones are timestamp)
+ *   - SessionID :
+ * - uint8 session_id_len (0..32)  (SessionID len in bytes)
+ * - uint8 session_id[session_id_len]
+ *   - CipherSuite :
+ * - uint16 cipher_len   >= 2  (Cipher length in bytes)
+ * - uint16 ciphers[cipher_len/2]
+ *   - CompressionMethod :
+ * - uint8 compression_len   >= 1  (# of supported methods)
+ * - uint8 compression_methods[compression_len]
+ *   - optional client_extension_len   (in bytes)
+ *   - optional sequence of ClientHelloExtensions  (as many bytes as above):
+ * - uint16 extension_type= 16 for application_layer_protocol_negotiation
+ * - uint16 extension_len
+ * - opaque extension_data[extension_len]
+ *   - uint16 protocol_names_len   (# of bytes here)
+ *   - opaque protocol_names[protocol_names_len bytes]
+ * - uint8 name_len
+ * - opaque protocol_name[name_len bytes]
+ */
+static int
+smp_fetch_ssl_hello_alpn(const struct arg *args, struct sample *smp, const char *kw, void *private)
+{
+	int hs_len, ext_len, bleft;
+	struct channel *chn;
+	unsigned char *data;
+
+	if 

Re: [PATCH] MINOR: acl: add support for TLS ALPN matching

2018-12-31 Thread Willy Tarreau
On Mon, Dec 31, 2018 at 06:23:59AM -0500, Alex Zorin wrote:
> Thanks for the generous review and pointers - that does sound much better and
> appears to work well for the ClientHellos I have tried. Sorry for not posting
> this as RFC.

You're welcome. I was about to apply it until I figured that a doc update
is missing. Please add an entry to configuration.txt for this new keyword
(and take care of respecting the alphabetical order).

Thanks!
Willy



Re: [PATCH] MINOR: acl: add support for TLS ALPN matching

2018-12-31 Thread Alex Zorin
Hi Willy,

Thanks for the generous review and pointers - that does sound much better and 
appears to work well for the ClientHellos I have tried. Sorry for not posting 
this as RFC.

Alex

- Original message -
From: Willy Tarreau 
To: Alex Zorin 
Cc: haproxy@formilux.org
Subject: Re: [PATCH] MINOR: acl: add support for TLS ALPN matching
Date: Monday, December 31, 2018 8:37 PM

I'm seeing that you extract only the first protocol name, and since
with ALPN it is possible to advertise a series of names, I think it
would make sense to be able to return each of them iteratively. We
already support doing this for headers or cookies for example. It
requires to set the SMP_F_NOT_LAST flag when returning, and to check
a context in smp->ctx (e.g. position of the next name to return in
smp->ctx->i).

This way you'll be able to steer incoming connections to a given
destination even if the advertised protocol is not the first one,
provided it's present in the list. Please have a look at
smp_fetch_cookie() for example to see how to proceed.>From 455ab2a2b593db177961a4794a93735e92aca8ed Mon Sep 17 00:00:00 2001
From: Alex Zorin 
Date: Sun, 30 Dec 2018 13:56:28 +1100
Subject: [PATCH] MINOR: acl: add support for TLS ALPN matching

Application-Layer Protocol Negotiation (ALPN, RFC7301) is a TLS
extension which allows a client to present a preference for which
protocols it wishes to connect to, when a single port supports multiple
multiple application protocols.
It allows a transparent proxy to take a decision based on the beginning
of an SSL/TLS stream without deciphering it.

The new ACL "req.ssl_alpn" matches the first matching protocol name
presented in the TLS ClientHello request.
---
 src/payload.c | 172 ++
 1 file changed, 172 insertions(+)

diff --git a/src/payload.c b/src/payload.c
index 7ef6d97e..a16f8c60 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -659,6 +659,177 @@ smp_fetch_ssl_hello_sni(const struct arg *args, struct sample *smp, const char *
 	return 0;
 }
 
+/* Try to extract the Application-Layer Protocol Negotiation (ALPN) protocol
+ * names that may be presented in a TLS client hello handshake message. As the
+ * message presents a list of protocol names in descending order of preference,
+ * it may return iteratively. The format of the message is the following
+ * (cf RFC5246 + RFC7301) :
+ * TLS frame :
+ *   - uint8  type= 0x16   (Handshake)
+ *   - uint16 version>= 0x0301 (TLSv1)
+ *   - uint16 length   (frame length)
+ *   - TLS handshake :
+ * - uint8  msg_type  = 0x01   (ClientHello)
+ * - uint24 length (handshake message length)
+ * - ClientHello :
+ *   - uint16 client_version >= 0x0301 (TLSv1)
+ *   - uint8 Random[32]  (4 first ones are timestamp)
+ *   - SessionID :
+ * - uint8 session_id_len (0..32)  (SessionID len in bytes)
+ * - uint8 session_id[session_id_len]
+ *   - CipherSuite :
+ * - uint16 cipher_len   >= 2  (Cipher length in bytes)
+ * - uint16 ciphers[cipher_len/2]
+ *   - CompressionMethod :
+ * - uint8 compression_len   >= 1  (# of supported methods)
+ * - uint8 compression_methods[compression_len]
+ *   - optional client_extension_len   (in bytes)
+ *   - optional sequence of ClientHelloExtensions  (as many bytes as above):
+ * - uint16 extension_type= 16 for application_layer_protocol_negotiation
+ * - uint16 extension_len
+ * - opaque extension_data[extension_len]
+ *   - uint16 protocol_names_len   (# of bytes here)
+ *   - opaque protocol_names[protocol_names_len bytes]
+ * - uint8 name_len
+ * - opaque protocol_name[name_len bytes]
+ */
+static int
+smp_fetch_ssl_hello_alpn(const struct arg *args, struct sample *smp, const char *kw, void *private)
+{
+	int hs_len, ext_len, bleft;
+	struct channel *chn;
+	unsigned char *data;
+
+	if (!smp->strm)
+		goto not_ssl_hello;
+
+	chn = ((smp->opt & SMP_OPT_DIR) == SMP_OPT_DIR_RES) ? >strm->res : >strm->req;
+	bleft = ci_data(chn);
+	data = (unsigned char *)ci_head(chn);
+
+	/* Check for SSL/TLS Handshake */
+	if (!bleft)
+		goto too_short;
+	if (*data != 0x16)
+		goto not_ssl_hello;
+
+	/* Check for SSLv3 or later (SSL version >= 3.0) in the record layer*/
+	if (bleft < 3)
+		goto too_short;
+	if (data[1] < 0x03)
+		goto not_ssl_hello;
+
+	if (bleft < 5)
+		goto too_short;
+	hs_len = (data[3] << 8) + data[4];
+	if (hs_len < 1 + 3 + 2 + 32 + 1 + 2 + 2 + 1 + 1 + 2 + 2)
+		goto not_ssl_hello; /* too short to have an extension */
+
+	data += 5; /* enter TLS handshake */
+	bleft -= 5;
+
+	/* Check for a complete 

Re: [PATCH] MINOR: acl: add support for TLS ALPN matching

2018-12-31 Thread Willy Tarreau
Hello Alex,

On Sat, Dec 29, 2018 at 10:22:05PM -0500, Alex Zorin wrote:
> >From 59c8e558d1e46dc20bfffc683f8c89e06b0dbaef Mon Sep 17 00:00:00 2001
> From: Alex Zorin 
> Date: Sun, 30 Dec 2018 13:56:28 +1100
> Subject: [PATCH] MINOR: acl: add support for TLS ALPN matching
> 
> Application-Layer Protocol Negotiation (ALPN, RFC7301) is a TLS
> extension which allows a client to present the name of the protocol
> it is connecting to, when a single port supports multiple application
> protocols.
> It allows a transparent proxy to take a decision based on the beginning
> of an SSL/TLS stream without deciphering it.
> 
> The new ACL "req.ssl_alpn" matches the protocol name extracted from the
> TLS ClientHello request.

I'm seeing that you extract only the first protocol name, and since
with ALPN it is possible to advertise a series of names, I think it
would make sense to be able to return each of them iteratively. We
already support doing this for headers or cookies for example. It
requires to set the SMP_F_NOT_LAST flag when returning, and to check
a context in smp->ctx (e.g. position of the next name to return in
smp->ctx->i).

This way you'll be able to steer incoming connections to a given
destination even if the advertised protocol is not the first one,
provided it's present in the list. Please have a look at
smp_fetch_cookie() for example to see how to proceed.

Thanks!
Willy



Re: [PATCH] MINOR: acl: add support for TLS ALPN matching

2018-12-29 Thread Alex Zorin
Unfortunately I attached the wrong patch file. Attaching in reply.

Alex

On Sun, Dec 30, 2018, at 2:20 PM, Alex Zorin wrote:
> Hello,
> 
> The attached patch adds acl support for the TLS ALPN extension 
> (RFC7301) extension via  "req.ssl_alpn", in a similar vein to 
> "req.ssl_sni".
> 
> It is useful for pass-thru of TLS connections in scenarios like ACME's 
> tls-alpn-01.
> 
> Thank you
> Alex
> Attachments:
> * 0001-MINOR-update-proxy-protocol-v2-define.patch>From 59c8e558d1e46dc20bfffc683f8c89e06b0dbaef Mon Sep 17 00:00:00 2001
From: Alex Zorin 
Date: Sun, 30 Dec 2018 13:56:28 +1100
Subject: [PATCH] MINOR: acl: add support for TLS ALPN matching

Application-Layer Protocol Negotiation (ALPN, RFC7301) is a TLS
extension which allows a client to present the name of the protocol
it is connecting to, when a single port supports multiple application
protocols.
It allows a transparent proxy to take a decision based on the beginning
of an SSL/TLS stream without deciphering it.

The new ACL "req.ssl_alpn" matches the protocol name extracted from the
TLS ClientHello request.
---
 src/payload.c | 164 ++
 1 file changed, 164 insertions(+)

diff --git a/src/payload.c b/src/payload.c
index 7ef6d97e..ab989864 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -659,6 +659,169 @@ smp_fetch_ssl_hello_sni(const struct arg *args, struct sample *smp, const char *
 	return 0;
 }
 
+/* Try to extract the Application-Layer Protocol Negotiation (ALPN) that may
+ * be presented in a TLS client hello handshake message. The format of the
+ * message is the following (cf RFC5246 + RFC7301) :
+ * TLS frame :
+ *   - uint8  type= 0x16   (Handshake)
+ *   - uint16 version>= 0x0301 (TLSv1)
+ *   - uint16 length   (frame length)
+ *   - TLS handshake :
+ * - uint8  msg_type  = 0x01   (ClientHello)
+ * - uint24 length (handshake message length)
+ * - ClientHello :
+ *   - uint16 client_version >= 0x0301 (TLSv1)
+ *   - uint8 Random[32]  (4 first ones are timestamp)
+ *   - SessionID :
+ * - uint8 session_id_len (0..32)  (SessionID len in bytes)
+ * - uint8 session_id[session_id_len]
+ *   - CipherSuite :
+ * - uint16 cipher_len   >= 2  (Cipher length in bytes)
+ * - uint16 ciphers[cipher_len/2]
+ *   - CompressionMethod :
+ * - uint8 compression_len   >= 1  (# of supported methods)
+ * - uint8 compression_methods[compression_len]
+ *   - optional client_extension_len   (in bytes)
+ *   - optional sequence of ClientHelloExtensions  (as many bytes as above):
+ * - uint16 extension_type= 16 for application_layer_protocol_negotiation
+ * - uint16 extension_len
+ * - opaque extension_data[extension_len]
+ *   - uint16 protocol_names_len   (# of bytes here)
+ *   - opaque protocol_names[protocol_names_len bytes]
+ * - uint8 name_len
+ * - opaque protocol_name[name_len bytes]
+ */
+static int
+smp_fetch_ssl_hello_alpn(const struct arg *args, struct sample *smp, const char *kw, void *private)
+{
+	int hs_len, ext_len, bleft;
+	struct channel *chn;
+	unsigned char *data;
+
+	if (!smp->strm)
+		goto not_ssl_hello;
+
+	chn = ((smp->opt & SMP_OPT_DIR) == SMP_OPT_DIR_RES) ? >strm->res : >strm->req;
+	bleft = ci_data(chn);
+	data = (unsigned char *)ci_head(chn);
+
+	/* Check for SSL/TLS Handshake */
+	if (!bleft)
+		goto too_short;
+	if (*data != 0x16)
+		goto not_ssl_hello;
+
+	/* Check for SSLv3 or later (SSL version >= 3.0) in the record layer*/
+	if (bleft < 3)
+		goto too_short;
+	if (data[1] < 0x03)
+		goto not_ssl_hello;
+
+	if (bleft < 5)
+		goto too_short;
+	hs_len = (data[3] << 8) + data[4];
+	if (hs_len < 1 + 3 + 2 + 32 + 1 + 2 + 2 + 1 + 1 + 2 + 2)
+		goto not_ssl_hello; /* too short to have an extension */
+
+	data += 5; /* enter TLS handshake */
+	bleft -= 5;
+
+	/* Check for a complete client hello starting at  */
+	if (bleft < 1)
+		goto too_short;
+	if (data[0] != 0x01) /* msg_type = Client Hello */
+		goto not_ssl_hello;
+
+	/* Check the Hello's length */
+	if (bleft < 4)
+		goto too_short;
+	hs_len = (data[1] << 16) + (data[2] << 8) + data[3];
+	if (hs_len < 2 + 32 + 1 + 2 + 2 + 1 + 1 + 2 + 2)
+		goto not_ssl_hello; /* too short to have an extension */
+
+	/* We want the full handshake here */
+	if (bleft < hs_len)
+		goto too_short;
+
+	data += 4;
+	/* Start of the ClientHello message */
+	if (data[0] < 0x03 || data[1] < 0x01) /* TLSv1 minimum */
+		goto not_ssl_hello;
+
+	ext_len = data[34]; /* session_id_len */
+	if (ext_len > 32 |

[PATCH] MINOR: acl: add support for TLS ALPN matching

2018-12-29 Thread Alex Zorin
Hello,

The attached patch adds acl support for the TLS ALPN extension (RFC7301) 
extension via  "req.ssl_alpn", in a similar vein to "req.ssl_sni".

It is useful for pass-thru of TLS connections in scenarios like ACME's 
tls-alpn-01.

Thank you
Alex>From 8008e5e8f23747741ed005f56c247bcd366cfda9 Mon Sep 17 00:00:00 2001
From: Emmanuel Hocdet 
Date: Fri, 13 Oct 2017 12:15:28 +0200
Subject: [PATCH 1/3] MINOR: update proxy-protocol-v2 #define

Report #define from doc/proxy-protocol.txt.
---
 include/types/connection.h | 15 +++
 src/connection.c   |  4 ++--
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/include/types/connection.h b/include/types/connection.h
index 1c923c578..eee75ec42 100644
--- a/include/types/connection.h
+++ b/include/types/connection.h
@@ -373,10 +373,17 @@ struct proxy_hdr_v2 {
 	} addr;
 };
 
-#define PP2_TYPE_SSL   0x20
-#define PP2_TYPE_SSL_VERSION   0x21
-#define PP2_TYPE_SSL_CN0x22
-#define PP2_TYPE_NETNS 0x30
+#define PP2_TYPE_ALPN   0x01
+#define PP2_TYPE_AUTHORITY  0x02
+#define PP2_TYPE_CRC32C 0x03
+#define PP2_TYPE_NOOP   0x04
+#define PP2_TYPE_SSL0x20
+#define PP2_SUBTYPE_SSL_VERSION 0x21
+#define PP2_SUBTYPE_SSL_CN  0x22
+#define PP2_SUBTYPE_SSL_CIPHER  0x23
+#define PP2_SUBTYPE_SSL_SIG_ALG 0x24
+#define PP2_SUBTYPE_SSL_KEY_ALG 0x25
+#define PP2_TYPE_NETNS  0x30
 
 #define TLV_HEADER_SIZE  3
 struct tlv {
diff --git a/src/connection.c b/src/connection.c
index 48f0ec331..a29bc2c32 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -1037,7 +1037,7 @@ int make_proxy_line_v2(char *buf, int buf_len, struct server *srv, struct connec
 			tlv->client |= PP2_CLIENT_SSL;
 			value = ssl_sock_get_version(remote);
 			if (value) {
-ssl_tlv_len += make_tlv([ret+ssl_tlv_len], (buf_len-ret-ssl_tlv_len), PP2_TYPE_SSL_VERSION, strlen(value), value);
+ssl_tlv_len += make_tlv([ret+ssl_tlv_len], (buf_len-ret-ssl_tlv_len), PP2_SUBTYPE_SSL_VERSION, strlen(value), value);
 			}
 			if (ssl_sock_get_cert_used_sess(remote)) {
 tlv->client |= PP2_CLIENT_CERT_SESS;
@@ -1048,7 +1048,7 @@ int make_proxy_line_v2(char *buf, int buf_len, struct server *srv, struct connec
 			if (srv->pp_opts & SRV_PP_V2_SSL_CN) {
 cn_trash = get_trash_chunk();
 if (ssl_sock_get_remote_common_name(remote, cn_trash) > 0) {
-	ssl_tlv_len += make_tlv([ret+ssl_tlv_len], (buf_len - ret - ssl_tlv_len), PP2_TYPE_SSL_CN, cn_trash->len, cn_trash->str);
+	ssl_tlv_len += make_tlv([ret+ssl_tlv_len], (buf_len - ret - ssl_tlv_len), PP2_SUBTYPE_SSL_CN, cn_trash->len, cn_trash->str);
 }
 			}
 		}
-- 
2.11.0