On Mon, Nov 27, 2017 at 06:19:41PM +0100, Emmanuel Hocdet wrote:
> > Maybe the best is to add a new flag per conn_stream, CS_FL_WAITING_FOR_HS or
> > something, instead of relying on CO_FL_EARLY_DATA.
> > I think I'm going to do something like that.
>
> I think it's a good idea, two different things to deal with one tag.
>
> > That still doesn't help with your problem with TCP mode, though. I still
> > want
> > the CO_FL_EARLY_DATA to be removed after the handshake, so that we don't
> > add the "Early-Data: 1" header if it is not needed. But it just occured
> > to me that I can easily fix that by adding the header, not only if
> > CO_FL_EARLY_DATA is set, but if CO_FL_EARLY_SSL_HS or CO_FL_SSL_WAIT_HS is
> > set.
> > That way we will both be happy :)
>
So, this if my first cut at it. It basically does as I said.
Only thing you may be unhappy with, I made the sample fetch ssl_fc_has_early
return 0 if we had early data but the handshake happened, because the main
use case is to know if there are early data and if they're a potential
security risk.
If you can give me a case where we have a need a sample fetch to know there
were early data, even after the handshake, maybe we can introduce a new
sample fetch, ssl_fc_has_insecure_early, or something ?
Regards,
Olivier
>From bda3b7800677184ea19fb81f75f9a9b44c79efeb Mon Sep 17 00:00:00 2001
From: Olivier Houchard <ohouch...@haproxy.com>
Date: Mon, 27 Nov 2017 18:41:32 +0100
Subject: [PATCH 1/2] MINOR: early data: Don't rely on CO_FL_EARLY_DATA to wake
up streams.
Instead of looking for CO_FL_EARLY_DATA to know if we have to try to wake
up a stream, because it is waiting for a SSL handshake, instead add a new
conn_stream flag, CS_FL_WAIT_FOR_HS. This way we don't have to rely on
CO_FL_EARLY_DATA, and we will only wake streams that are actually waiting.
---
include/types/connection.h | 1 +
src/mux_h2.c | 24 ++++++++++++++++++------
src/ssl_sock.c | 5 ++++-
src/stream_interface.c | 4 +++-
4 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/include/types/connection.h b/include/types/connection.h
index a9d04474d..f220c42a9 100644
--- a/include/types/connection.h
+++ b/include/types/connection.h
@@ -69,6 +69,7 @@ enum {
CS_FL_ERROR = 0x00000100, /* a fatal error was reported */
CS_FL_EOS = 0x00001000, /* End of stream */
+ CS_FL_WAIT_FOR_HS = 0x00010000, /* This stream is waiting for
handhskae */
};
/* cs_shutr() modes */
diff --git a/src/mux_h2.c b/src/mux_h2.c
index 4567b8ff0..4db90d4c8 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -54,6 +54,7 @@ static struct pool_head *pool_head_h2s;
/* other flags */
#define H2_CF_GOAWAY_SENT 0x00000100 // a GOAWAY frame was successfully
sent
#define H2_CF_GOAWAY_FAILED 0x00000200 // a GOAWAY frame failed to be sent
+#define H2_CF_WAIT_FOR_HS 0x00000400 // We did check that at least a
stream was waiting for handshake
/* H2 connection state, in h2c->st0 */
@@ -2091,14 +2092,25 @@ static int h2_wake(struct connection *conn)
struct h2c *h2c = conn->mux_ctx;
/*
- * If we received early data, try to wake any stream, just in case
- * at least one of them was waiting for the handshake
+ * If we received early data, and the handshake is done, wake
+ * any stream that was waiting for it.
*/
- if ((conn->flags & (CO_FL_EARLY_SSL_HS | CO_FL_EARLY_DATA |
CO_FL_HANDSHAKE)) ==
- CO_FL_EARLY_DATA) {
- h2_wake_some_streams(h2c, 0, 0);
- conn->flags &= ~CO_FL_EARLY_DATA;
+ if (!(h2c->flags & H2_CF_WAIT_FOR_HS) &&
+ (conn->flags & (CO_FL_EARLY_SSL_HS | CO_FL_HANDSHAKE |
CO_FL_EARLY_DATA)) == CO_FL_EARLY_DATA) {
+ struct eb32_node *node;
+ struct h2s *h2s;
+
+ h2c->flags |= H2_CF_WAIT_FOR_HS;
+ node = eb32_lookup_ge(&h2c->streams_by_id, 1);
+
+ while (node) {
+ h2s = container_of(node, struct h2s, by_id);
+ if (h2s->cs->flags & CS_FL_WAIT_FOR_HS)
+ h2s->cs->data_cb->wake(h2s->cs);
+ node = eb32_next(node);
+ }
}
+
if (conn->flags & CO_FL_ERROR || conn_xprt_read0_pending(conn) ||
h2c->st0 == H2_CS_ERROR2 || h2c->flags & H2_CF_GOAWAY_FAILED ||
(eb_is_empty(&h2c->streams_by_id) && h2c->last_sid >= 0 &&
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index da1aecbcc..7c5fd6b10 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -8705,11 +8705,14 @@ enum act_return ssl_action_wait_for_hs(struct act_rule
*rule, struct proxy *px,
struct session *sess, struct stream *s,
int flags)
{
struct connection *conn;
+ struct conn_stream *cs;
conn = objt_conn(sess->origin);
+ cs = objt_cs(s->si[0].end);
- if (conn) {
+ if (conn && cs) {
if (conn->flags & (CO_FL_EARLY_SSL_HS | CO_FL_SSL_WAIT_HS)) {
+ cs->flags |= CS_FL_WAIT_FOR_HS;
s->req.flags |= CF_READ_NULL;
return ACT_RET_YIELD;
}
diff --git a/src/stream_interface.c b/src/stream_interface.c
index 6f9f27b98..4c2fff94c 100644
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -584,7 +584,9 @@ static int si_cs_wake_cb(struct conn_stream *cs)
* in the event there's an analyser waiting for the end of
* the handshake.
*/
- if ((conn->flags & (CO_FL_EARLY_DATA | CO_FL_EARLY_SSL_HS)) ==
CO_FL_EARLY_DATA) {
+ if (!(conn->flags & (CO_FL_HANDSHAKE | CO_FL_EARLY_SSL_HS)) &&
+ (cs->flags & CS_FL_WAIT_FOR_HS)) {
+ cs->flags &= ~CS_FL_WAIT_FOR_HS;
task_wakeup(si_task(si), TASK_WOKEN_MSG);
}
--
2.14.3
>From e697b44db0e187ae6d822f7ed163fb2696cd4b97 Mon Sep 17 00:00:00 2001
From: Olivier Houchard <ohouch...@haproxy.com>
Date: Wed, 29 Nov 2017 19:51:19 +0100
Subject: [PATCH 2/2] MINOR: early data: Never remove the CO_FL_EARLY_DATA
flag.
It may be useful to keep the CO_FL_EARLY_DATA flag, so that we know early
data were used, so instead of doing this, only add the Early-data header,
and have the sample fetch ssl_fc_has_early return 1, if CO_FL_EARLY_DATA is
set, and if the handshake isn't done yet.
---
src/proto_http.c | 3 ++-
src/ssl_sock.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/proto_http.c b/src/proto_http.c
index 04771b78d..48fb69e32 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -3413,7 +3413,8 @@ int http_process_req_common(struct stream *s, struct
channel *req, int an_bit, s
}
}
- if (conn && conn->flags & CO_FL_EARLY_DATA) {
+ if (conn && (conn->flags & CO_FL_EARLY_DATA) &&
+ (conn->flags & (CO_FL_EARLY_SSL_HS | CO_FL_HANDSHAKE))) {
struct hdr_ctx ctx;
ctx.idx = 0;
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 7c5fd6b10..d4d9dbaa7 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -6020,7 +6020,8 @@ smp_fetch_ssl_fc_has_early(const struct arg *args, struct
sample *smp, const cha
smp->flags = 0;
smp->data.type = SMP_T_BOOL;
- smp->data.u.sint = (conn->flags & CO_FL_EARLY_DATA) ? 1 : 0;
+ smp->data.u.sint = ((conn->flags & CO_FL_EARLY_DATA) &&
+ (conn->flags & (CO_FL_EARLY_SSL_HS | CO_FL_HANDSHAKE))) ? 1 : 0;
return 1;
}
--
2.14.3
