Le 28/02/2016 23:32, PiBa-NL a écrit :
> Hi Christopher, Willy,
> 
> I've created a patch that can be applied on top of your tcp-fallback 
> patch to allow for 'conditional' offloading.
> It shows 'ability' to have both offloading and pass-trough for ssl 
> depending on a sni name or other acl criteria.
> 
> -I resorted rather heavily to changing flags and statuses to make it 
> step back to handshake negotiation. And although it seems to work, it 
> should be checked if any of this will possibly cause hangs / memory 
> leaks / other trouble..
> -Task timer added to fix excessive looping while waiting for 
> tcp-fallback to happen in case of a smtp connection.
> -'peek'-ing for filling the data buffer so the FD stream keeps the 
> handshake data it had before for the ssl object to read if the 
> ssl_upgrade() is performed later. I couldnt find how to fill the ssl 
> object otherwise.. I think SSL_set_rbio() would be the solution there, 
> but that isnt available in current openssl releases.
> 
> I hope you guys have the time and ability to check for problems 
> introduced by my 'hackery'.. Or just throw it in the bin and 
> re-implement it using a better understanding of how the flow is supposed 
> to go.
> 
> Any feedback is appreciated :)
> 

Hi guys,

Sorry for the delay, I was pretty busy. I've checked your patch. It is
quite interesting. First of all, I think that "tcp fallback" option and
"conditional SSL offloading" are redundant. Your way to do is more
flexible and generic, but the spirit is the same. So I'm tempted to
trash my patch.

Then, about your changes, there are 2/3 things that can be discussed.
You've added integers in the connection structure. Willy already
expressed the need to keep it as small as possible. So it might be fine
to find another way to do. Using BIO is probably a good alternative (but
no the easier one...). And, your way to peeking data seems to "penalize"
all streams. This is not huge of course, but this could be improved. I'm
also curious to see if it works with a complex "tcp-request content"
ruleset. Finally, defining the "offloadssl" action on "tcp-response
content" is useless and unused.

But I really like your idea. So I've reworked it. Let me known if it's
ok for you.

It uses BIO to let connection structure untouched and to have no
overhead when the feature is not used. I've done some test but not much.
And I don't know if overhead imposed by BIO is huge or not. I'm not an
OpenSSL expert, so maybe there is a better way to do. I've renamed
"offloadssl" action to use "ssl-upgrade" and "no-ssl-upgrade" instead.
My patch is done on the HEAD of the master branch, ignoring my previous
patch. Note that you need to postpone the SSL upgrade to use this
feature. To do so, you must add "defer-ssl-upgrade" on the bind line.

Willy, if you are agree, this new patch can replace my previous one. Of
course, all remarks are welcome. I'll try to do more tests. I quickly
checked it on OpenSSL 0.9.8zg and 1.0.2f.

-- 
Christopher
>From 07133669314724b2b4462e0eb3e8cd114f3fd3b9 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <[email protected]>
Date: Thu, 3 Mar 2016 16:15:28 +0100
Subject: [PATCH] MAJOR: ssl: Allow the postpone of the SSL upgrade and a way
 to enable/disable it

the bind option "defer-ssl-upgrade" has been added. It can be used iff "ssl"
option is set. It postpone the SSL upgrade after the evaluation of all
"tcp-request content" rules.

2 actions have also been added in "tcp-request content" rules to conditionally
enable/disable this upgrade:

  * ssl-upgrade
  * no-ssl-upgrade

The "ssl-upgrade" is used to upgrade a TCP connection to a SSL connection and
"no-ssl-upgrade" is used to disable it. This is only available for connections
opened on a SSL listener configured with the "defer-ssl-upgrade" option and
ignored for all others. When a "ssl-upgrade" rule matches, it invalidates
"no-ssl-upgrade" rules that follow, and conversely. When all rules are
evaluated, a postponed SSL upgrade is always performed when neither the
"ssl-upgrade" rules nor the "no-ssl-upgrade" rules match.

Here are two examples:

  # Do the SSL connection upgrade only if the SNI field is set to "example.com"
  frontend l
      mode tcp
      bind *:1234 ssl crt /path/to/srv.pem defer-ssl-upgrade

      tcp-request inspect-delay 5s
      tcp-request content ssl-upgrade if { req.ssl_sni -i example.com }
      tcp-request content no-ssl-upgrade

      use_backend     offloaded-ssl if ssl_fc
      default_backend raw-ssl

  # Accept SSL and non-SSL connctions on the same listener
  frontend l
      mode tcp
      bind *:1234 ssl crt /path/to/srv.pem defer-ssl-upgrade

      tcp-request inspect-delay 5s
      tcp-request content no-ssl-upgrade ! { req.ssl_ver gt 0 }

      use_backend     ssl if ssl_fc
      default_backend tcp

This feature is based on the work of PiBa-NL <[email protected]>
---
 doc/configuration.txt      | 30 ++++++++++++++++++++
 include/proto/ssl_sock.h   |  1 +
 include/types/action.h     |  2 ++
 include/types/connection.h |  4 ++-
 include/types/listener.h   | 25 +++++++++--------
 src/listener.c             |  7 +++--
 src/proto_tcp.c            | 57 +++++++++++++++++++++++++++++++++++++-
 src/session.c              |  6 ++++
 src/ssl_sock.c             | 68 ++++++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 184 insertions(+), 16 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 279d076..87b667b 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -8718,6 +8718,8 @@ tcp-request content <action> [{if | unless} <condition>]
     - set-gpt0(<sc-id>) <int>
     - set-var(<var-name>) <expr>
     - silent-drop
+    - ssl-upgrade
+    - no-ssl-upgrade
 
   They have the same meaning as their counter-parts in "tcp-request connection"
   so please refer to that section for a complete description.
@@ -8765,6 +8767,14 @@ tcp-request content <action> [{if | unless} <condition>]
     <expr>     Is a standard HAProxy expression formed by a sample-fetch
                followed by some converters.
 
+  The "ssl-upgrade" is used to upgrade a TCP connection to a SSL connection and
+  "no-ssl-upgrade" is used to disable it. This is only available for
+  connections opened on a SSL listener configured with the "defer-ssl-upgrade"
+  option and ignored for all others. When a "ssl-upgrade" rule matches, it
+  invalidates "no-ssl-upgrade" rules that follow, and conversely. When all
+  rules are evaluated, a postponed SSL upgrade is always performed when neither
+  the "ssl-upgrade" rules nor the "no-ssl-upgrade" rules match.
+
   Example:
 
         tcp-request content set-var(sess.my_var) src
@@ -8820,6 +8830,18 @@ tcp-request content <action> [{if | unless} <condition>]
             tcp-request content track-sc1 src
             tcp-request content reject if click_too_fast mark_as_abuser
 
+  Example:
+        # Do the SSL connection upgrade only if the SNI field is set
+        # to "example.com"
+        tcp-request inspect-delay 5s
+        tcp-request content ssl-upgrade if { req.ssl_sni -i example.com }
+        tcp-request content no-ssl-upgrade
+
+  Example:
+        # Handle SSL and non-SSL connctions on the same listener
+        tcp-request inspect-delay 5s
+        tcp-request content no-ssl-upgrade if ! { req.ssl_ver gt 0 }
+
   See section 7 about ACL usage.
 
   See also : "tcp-request connection", "tcp-request inspect-delay"
@@ -9797,6 +9819,14 @@ defer-accept
   an established connection while the proxy will only see it in SYN_RECV. This
   option is only supported on TCPv4/TCPv6 sockets and ignored by other ones.
 
+defer-ssl-upgrade
+
+ This setting is only available when support for OpenSSL was built in. It
+ postpones the SSL connection upgrade after the evaluation of all "tcp-request
+ content" rules. Deferred SSL upgrades can be conditionally enabled or
+ disabled. See also "tcp-request content ssl-upgrade" and
+ "tcp-request content no-ssl-upgrade".
+
 force-sslv3
   This option enforces use of SSLv3 only on SSL connections instantiated from
   this listener. SSLv3 is generally less expensive than the TLS counterparts
diff --git a/include/proto/ssl_sock.h b/include/proto/ssl_sock.h
index cb9a1e9..152aa6e 100644
--- a/include/proto/ssl_sock.h
+++ b/include/proto/ssl_sock.h
@@ -43,6 +43,7 @@ int ssl_sock_is_ssl(struct connection *conn)
 }
 
 int ssl_sock_handshake(struct connection *conn, unsigned int flag);
+int ssl_sock_upgrade(struct connection *conn, struct buffer *buf);
 int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, SSL_CTX *ctx, struct proxy *proxy);
 int ssl_sock_prepare_all_ctx(struct bind_conf *bind_conf, struct proxy *px);
 int ssl_sock_prepare_srv_ctx(struct server *srv, struct proxy *px);
diff --git a/include/types/action.h b/include/types/action.h
index b97f9bf..a437ce6 100644
--- a/include/types/action.h
+++ b/include/types/action.h
@@ -86,6 +86,8 @@ enum act_name {
 	ACT_TCP_EXPECT_PX,
 	ACT_TCP_CLOSE, /* close at the sender's */
 	ACT_TCP_CAPTURE, /* capture a fetched sample */
+	ACT_TCP_SSL_UPGRADE, /* Do SSL upgrade */
+	ACT_TCP_NO_SSL_UPGRADE, /* Skip SSL upgrade */
 
 	/* track stick counters */
 	ACT_ACTION_TRK_SC0,
diff --git a/include/types/connection.h b/include/types/connection.h
index dfbff6a..308abb7 100644
--- a/include/types/connection.h
+++ b/include/types/connection.h
@@ -107,7 +107,8 @@ enum {
 	CO_FL_SEND_PROXY    = 0x01000000,  /* send a valid PROXY protocol header */
 	CO_FL_SSL_WAIT_HS   = 0x02000000,  /* wait for an SSL handshake to complete */
 	CO_FL_ACCEPT_PROXY  = 0x04000000,  /* receive a valid PROXY protocol header */
-	/* unused : 0x08000000 */
+
+	CO_FL_DEF_SSL_UPG   = 0x08000000,  /* postpone the ssl upgrade of the connection */
 
 	/* below we have all handshake flags grouped into one */
 	CO_FL_HANDSHAKE     = CO_FL_SEND_PROXY | CO_FL_SSL_WAIT_HS | CO_FL_ACCEPT_PROXY,
@@ -263,6 +264,7 @@ struct connection {
 			int fd;       /* file descriptor for a stream driver when known */
 		} sock;
 	} t;
+
 	enum obj_type *target;        /* the target to connect to (server, proxy, applet, ...) */
 	struct list list;             /* attach point to various connection lists (idle, ...) */
 	const struct netns_entry *proxy_netns;
diff --git a/include/types/listener.h b/include/types/listener.h
index 4da6cac..5249bd9 100644
--- a/include/types/listener.h
+++ b/include/types/listener.h
@@ -80,18 +80,19 @@ enum li_state {
  */
 
 /* listener socket options */
-#define LI_O_NONE	0x0000
-#define LI_O_NOLINGER	0x0001	/* disable linger on this socket */
-#define LI_O_FOREIGN	0x0002	/* permit listening on foreing addresses */
-#define LI_O_NOQUICKACK	0x0004	/* disable quick ack of immediate data (linux) */
-#define LI_O_DEF_ACCEPT	0x0008	/* wait up to 1 second for data before accepting */
-#define LI_O_TCP_RULES  0x0010  /* run TCP rules checks on the incoming connection */
-#define LI_O_CHK_MONNET 0x0020  /* check the source against a monitor-net rule */
-#define LI_O_ACC_PROXY  0x0040  /* find the proxied address in the first request line */
-#define LI_O_UNLIMITED  0x0080  /* listener not subject to global limits (peers & stats socket) */
-#define LI_O_TCP_FO     0x0100  /* enable TCP Fast Open (linux >= 3.7) */
-#define LI_O_V6ONLY     0x0200  /* bind to IPv6 only on Linux >= 2.4.21 */
-#define LI_O_V4V6       0x0400  /* bind to IPv4/IPv6 on Linux >= 2.4.21 */
+#define LI_O_NONE        0x0000
+#define LI_O_NOLINGER    0x0001  /* disable linger on this socket */
+#define LI_O_FOREIGN     0x0002  /* permit listening on foreing addresses */
+#define LI_O_NOQUICKACK  0x0004  /* disable quick ack of immediate data (linux) */
+#define LI_O_DEF_ACCEPT  0x0008  /* wait up to 1 second for data before accepting */
+#define LI_O_TCP_RULES   0x0010  /* run TCP rules checks on the incoming connection */
+#define LI_O_CHK_MONNET  0x0020  /* check the source against a monitor-net rule */
+#define LI_O_ACC_PROXY   0x0040  /* find the proxied address in the first request line */
+#define LI_O_UNLIMITED   0x0080  /* listener not subject to global limits (peers & stats socket) */
+#define LI_O_TCP_FO      0x0100  /* enable TCP Fast Open (linux >= 3.7) */
+#define LI_O_V6ONLY      0x0200  /* bind to IPv6 only on Linux >= 2.4.21 */
+#define LI_O_V4V6        0x0400  /* bind to IPv4/IPv6 on Linux >= 2.4.21 */
+#define LI_O_DEF_SSL_UPG 0x0800  /* postpone SSL upgrade of the connection */
 
 /* Note: if a listener uses LI_O_UNLIMITED, it is highly recommended that it adds its own
  * maxconn setting to the global.maxsock value so that its resources are reserved.
diff --git a/src/listener.c b/src/listener.c
index 3759c78..07594a9 100644
--- a/src/listener.c
+++ b/src/listener.c
@@ -495,8 +495,11 @@ void listener_accept(int fd)
 				global.sps_max = global.sess_per_sec.curr_ctr;
 		}
 #ifdef USE_OPENSSL
-		if (!(l->options & LI_O_UNLIMITED) && l->bind_conf && l->bind_conf->is_ssl) {
-
+		/* if SSL upgrade is postponed, we do no update ssl/sec
+		 * frequency now */
+		if (!(l->options & LI_O_DEF_SSL_UPG) &&
+		    !(l->options & LI_O_UNLIMITED)   &&
+		    l->bind_conf && l->bind_conf->is_ssl) {
 			update_freq_ctr(&global.ssl_per_sec, 1);
 			if (global.ssl_per_sec.curr_ctr > global.ssl_max)
 				global.ssl_max = global.ssl_per_sec.curr_ctr;
diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index cce0acb..2f09d12 100644
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -61,6 +61,11 @@
 #include <proto/stream_interface.h>
 #include <proto/task.h>
 
+#ifdef USE_OPENSSL
+#include <proto/ssl_sock.h>
+#endif /*USE_OPENSSL */
+
+
 static int tcp_bind_listeners(struct protocol *proto, char *errmsg, int errlen);
 static int tcp_bind_listener(struct listener *listener, char *errmsg, int errlen);
 
@@ -1043,6 +1048,7 @@ int tcp_pause_listener(struct listener *l)
 int tcp_inspect_request(struct stream *s, struct channel *req, int an_bit)
 {
 	struct session *sess = s->sess;
+	struct connection *conn = objt_conn(sess->origin);
 	struct act_rule *rule;
 	struct stksess *ts;
 	struct stktable *t;
@@ -1099,7 +1105,6 @@ int tcp_inspect_request(struct stream *s, struct channel *req, int an_bit)
 			if (rule->cond->pol == ACL_COND_UNLESS)
 				ret = !ret;
 		}
-
 		if (ret) {
 			act_flags |= ACT_FLAG_FIRST;
 resume_execution:
@@ -1172,6 +1177,16 @@ resume_execution:
 				memcpy(cap[h->index], key->data.u.str.str, len);
 				cap[h->index][len] = 0;
 			}
+#ifdef USE_OPENSSL
+			else if (rule->action == ACT_TCP_SSL_UPGRADE) {
+				if (conn && (conn->flags & CO_FL_DEF_SSL_UPG))
+					goto ssl_upgrade;
+			}
+			else if (rule->action == ACT_TCP_NO_SSL_UPGRADE) {
+				if (conn)
+					conn->flags &= ~CO_FL_DEF_SSL_UPG;
+			}
+#endif /* USE_OPENSSL */
 			else {
 				/* Custom keywords. */
 				if (!rule->action_ptr)
@@ -1195,6 +1210,16 @@ resume_execution:
 		}
 	}
 
+#ifdef USE_OPENSSL
+ ssl_upgrade:
+	/* Do a deferred SSL upgrade if at lease one "ssl-upgrade rule has
+	 * matched and no "no-ssl-upgrade" rule. */
+	if (conn && (conn->flags & CO_FL_DEF_SSL_UPG)) {
+		if (ssl_sock_upgrade(conn, req->buf))
+			goto missing_data;
+	}
+#endif /* USE_OPENSSL */
+
 	/* if we get there, it means we have no rule which matches, or
 	 * we have an explicit accept, so we apply the default accept.
 	 */
@@ -1573,6 +1598,36 @@ static int tcp_parse_request_rule(char **args, int arg, int section_type,
 		arg++;
 		rule->action = ACT_ACTION_DENY;
 	}
+	else if (!strcmp(args[arg], "ssl-upgrade")) {
+#ifdef USE_OPENSSL
+		if (!(curpx->cap & PR_CAP_FE)) {
+			memprintf(err,
+			          "'%s %s %s' : proxy '%s' has no frontend capability",
+			          args[0], args[1], args[arg], curpx->id);
+			return -1;
+		}
+		arg++;
+		rule->action = ACT_TCP_SSL_UPGRADE;
+#else /* USE_OPENSSL */
+		memprintf(err, "'%s %s %s' : not implemented", args[0], args[1], args[arg]);
+		return -1;
+#endif /* USE_OPENSSL */
+	}
+	else if (!strcmp(args[arg], "no-ssl-upgrade")) {
+#ifdef USE_OPENSSL
+		if (!(curpx->cap & PR_CAP_FE)) {
+			memprintf(err,
+			          "'%s %s %s' : proxy '%s' has no frontend capability",
+			          args[0], args[1], args[arg], curpx->id);
+			return -1;
+		}
+		arg++;
+		rule->action = ACT_TCP_NO_SSL_UPGRADE;
+#else /* USE_OPENSSL */
+		memprintf(err, "'%s %s %s' : not implemented", args[0], args[1], args[arg]);
+		return -1;
+#endif /* USE_OPENSSL */
+	}
 	else if (strcmp(args[arg], "capture") == 0) {
 		struct sample_expr *expr;
 		struct cap_hdr *hdr;
diff --git a/src/session.c b/src/session.c
index fdb2404..f80a0f9 100644
--- a/src/session.c
+++ b/src/session.c
@@ -136,6 +136,12 @@ int session_accept_fd(struct listener *l, int cfd, struct sockaddr_storage *addr
 
 	conn_ctrl_init(cli_conn);
 
+	/* postpone ssl upgrade of the connection */
+	if (l->options & LI_O_DEF_SSL_UPG) {
+		cli_conn->flags &= ~CO_FL_SSL_WAIT_HS;
+		cli_conn->flags |= CO_FL_DEF_SSL_UPG;
+	}
+
 	/* wait for a PROXY protocol header */
 	if (l->options & LI_O_ACC_PROXY) {
 		cli_conn->flags |= CO_FL_ACCEPT_PROXY;
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index bdd228f..5d79650 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -83,9 +83,11 @@
 #include <proto/server.h>
 #include <proto/log.h>
 #include <proto/proxy.h>
+#include <proto/raw_sock.h>
 #include <proto/shctx.h>
 #include <proto/ssl_sock.h>
 #include <proto/stream.h>
+#include <proto/stream_interface.h>
 #include <proto/task.h>
 
 /* Warning, these are bits, not integers! */
@@ -3547,6 +3549,53 @@ reneg_ok:
 	return 0;
 }
 
+static long ssl_sock_upgrade_callback(BIO *bio, int cmd, const char *argp,
+				      int argi, long argl, long ret)
+{
+	if ((cmd & (BIO_CB_READ|BIO_CB_RETURN)) == (BIO_CB_READ|BIO_CB_RETURN)) {
+		struct connection *conn = (struct connection *)BIO_get_callback_arg(bio);
+		if (ret >= 0) {
+			struct buffer *buf = si_ic(conn->owner)->buf;
+			bi_fast_delete(buf, ret);
+			if (buf->i == 0)
+				SSL_set_fd(conn->xprt_ctx, conn->t.sock.fd);
+		}
+	}
+	return ret;
+}
+
+int ssl_sock_upgrade(struct connection *conn, struct buffer *buf)
+{
+	BIO *mbio;
+
+	conn->xprt = &ssl_sock;
+	conn->flags &= ~CO_FL_XPRT_READY;
+	conn_sock_want_recv(conn);
+	conn_data_want_recv(conn);
+	if (conn_xprt_init(conn) < 0)  {
+		conn->flags |= CO_FL_ERROR;
+		return 0;
+	}
+
+	if (!(objt_listener(conn->target)->options & LI_O_UNLIMITED)) {
+		update_freq_ctr(&global.ssl_per_sec, 1);
+		if (global.ssl_per_sec.curr_ctr > global.ssl_max)
+			global.ssl_max = global.ssl_per_sec.curr_ctr;
+	}
+
+	conn->flags |= (CO_FL_SSL_WAIT_HS | CO_FL_WAIT_L6_CONN);
+	conn->flags &= ~(CO_FL_CONNECTED|CO_FL_DEF_SSL_UPG);
+
+	mbio = BIO_new_mem_buf(buf->p, buf->i);
+	SSL_set_bio(conn->xprt_ctx, mbio, NULL);
+	SSL_set_wfd(conn->xprt_ctx, conn->t.sock.fd);
+	BIO_set_callback(mbio, ssl_sock_upgrade_callback);
+	BIO_set_callback_arg(mbio, (char *)conn);
+	fd_may_recv(conn->t.sock.fd);
+
+	return 1;
+}
+
 /* Receive up to <count> bytes from connection <conn>'s socket and store them
  * into buffer <buf>. Only one call to recv() is performed, unless the
  * buffer wraps, in which case a second call may be performed. The connection's
@@ -5454,6 +5503,24 @@ static int bind_parse_verify(char **args, int cur_arg, struct proxy *px, struct
 	return 0;
 }
 
+/* parse the "defer-ssl-upgrade" bind keyword */
+static int bind_parse_def_ssl_upg(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
+{
+	struct listener *l;
+
+	if (!conf->is_ssl) {
+		if (err)
+			memprintf(err, "'%s' : must be declared after 'ssl' keyword", args[cur_arg]);
+		return ERR_ALERT | ERR_FATAL;
+	}
+
+	list_for_each_entry(l, &conf->listeners, by_bind) {
+		l->xprt = &raw_sock;
+		l->options |= LI_O_DEF_SSL_UPG;
+	}
+	return 0;
+}
+
 /************** "server" keywords ****************/
 
 /* parse the "ca-file" server keyword */
@@ -5899,6 +5966,7 @@ static struct bind_kw_list bind_kws = { "SSL", { }, {
 	{ "crt",                   bind_parse_crt,             1 }, /* load SSL certificates from this location */
 	{ "crt-ignore-err",        bind_parse_ignore_err,      1 }, /* set error IDs to ingore on verify depth == 0 */
 	{ "crt-list",              bind_parse_crt_list,        1 }, /* load a list of crt from this location */
+	{ "defer-ssl-upgrade",     bind_parse_def_ssl_upg,     0 }, /* Defer the SSL connection upgrade */
 	{ "ecdhe",                 bind_parse_ecdhe,           1 }, /* defines named curve for elliptic curve Diffie-Hellman */
 	{ "force-sslv3",           bind_parse_force_sslv3,     0 }, /* force SSLv3 */
 	{ "force-tlsv10",          bind_parse_force_tlsv10,    0 }, /* force TLSv10 */
-- 
2.5.0

Reply via email to