Willy,
here I removed the BUG_ON() and added the blank lines you requested.
> > + BUG_ON(sess != conn->owner); /* XXX is this correct? */
>
> I'm pretty sure this one will break from time to time, maybe during
> retries or when reusing an idle connection or something. Better drop
> it. You don't really care who's the owner of the connection you're
> using anyway. In case of multiplexing it could be anyone because
> the connection will plugged onto by several streams before it's
> finally connected and once it's OK and the sending code starts, you
> have no way to tell whether the selected stream is the one that asked
> for the connection first.
>
Then it served the purpose well, you've taken a look at my assumptions to check
whether `strm_sess()` is the correct thing to use :-)
> So modulo the very few minor stuff above I'm overall OK without your
> patch, just let me know if you have any question.
No, the review was crystal clear.
Best regards
Tim Düsterhus
Apply with `git am --scissors` to automatically cut the commit message.
-- >8 --
This patch adds the `unique-id` option to `proxy-v2-options`. If this
option is set a unique ID will be generated based on the `unique-id-format`
while sending the proxy protocol v2 header and stored as the unique id for
the first stream of the connection.
This feature is meant to be used in `tcp` mode. It works on HTTP mode, but
might result in inconsistent unique IDs for the first request on a keep-alive
connection, because the unique ID for the first stream is generated earlier
than the others.
Now that we can send unique IDs in `tcp` mode the `%ID` log variable is made
available in TCP mode.
---
doc/configuration.txt | 24 +++++++----
include/proto/connection.h | 4 +-
include/types/server.h | 1 +
.../proxy_protocol_send_unique_id.vtc | 42 +++++++++++++++++++
src/connection.c | 20 +++++++--
src/log.c | 2 +-
src/server.c | 2 +
src/stream_interface.c | 11 +++--
8 files changed, 90 insertions(+), 16 deletions(-)
create mode 100644 reg-tests/connection/proxy_protocol_send_unique_id.vtc
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 57f777a78..cc5fbf2d0 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -12657,13 +12657,23 @@ send-proxy-v2
this section and send-proxy" option of the "bind" keyword.
proxy-v2-options <option>[,<option>]*
- The "proxy-v2-options" parameter add option to send in PROXY protocol version
- 2 when "send-proxy-v2" is used. Options available are "ssl" (see also
- send-proxy-v2-ssl), "cert-cn" (see also "send-proxy-v2-ssl-cn"),
"ssl-cipher":
- name of the used cipher, "cert-sig": signature algorithm of the used
- certificate, "cert-key": key algorithm of the used certificate), "authority":
- host name value passed by the client (only sni from a tls connection is
- supported), "crc32c": checksum of the proxy protocol v2 header.
+ The "proxy-v2-options" parameter add options to send in PROXY protocol
+ version 2 when "send-proxy-v2" is used. Options available are:
+
+ - ssl : See also "send-proxy-v2-ssl".
+ - cert-cn : See also "send-proxy-v2-ssl-cn".
+ - ssl-cipher: Name of the used cipher.
+ - cert-sig : Signature algorithm of the used certificate.
+ - cert-key : Key algorithm of the used certificate
+ - authority : Host name value passed by the client (only SNI from a TLS
+ connection is supported).
+ - crc32c : Checksum of the PROXYv2 header.
+ - unique-id : Send a unique ID generated using the frontend's
+ "unique-id-format" within the PROXYv2 header.
+ This unique-id is primarily meant for "mode tcp". It can
+ lead to unexpected results in "mode http", because the
+ generated unique ID is also used for the first HTTP request
+ within a Keep-Alive connection.
send-proxy-v2-ssl
The "send-proxy-v2-ssl" parameter enforces use of the PROXY protocol version
diff --git a/include/proto/connection.h b/include/proto/connection.h
index 9b8eb8ad3..ecc03de8a 100644
--- a/include/proto/connection.h
+++ b/include/proto/connection.h
@@ -47,9 +47,9 @@ int conn_fd_check(struct connection *conn);
/* receive a PROXY protocol header over a connection */
int conn_recv_proxy(struct connection *conn, int flag);
-int make_proxy_line(char *buf, int buf_len, struct server *srv, struct
connection *remote);
+int make_proxy_line(char *buf, int buf_len, struct server *srv, struct
connection *remote, struct stream *strm);
int make_proxy_line_v1(char *buf, int buf_len, struct sockaddr_storage *src,
struct sockaddr_storage *dst);
-int make_proxy_line_v2(char *buf, int buf_len, struct server *srv, struct
connection *remote);
+int make_proxy_line_v2(char *buf, int buf_len, struct server *srv, struct
connection *remote, struct stream *strm);
int conn_subscribe(struct connection *conn, void *xprt_ctx, int event_type,
struct wait_event *es);
int conn_unsubscribe(struct connection *conn, void *xprt_ctx, int event_type,
struct wait_event *es);
diff --git a/include/types/server.h b/include/types/server.h
index 099b1d8a8..0257f580f 100644
--- a/include/types/server.h
+++ b/include/types/server.h
@@ -154,6 +154,7 @@ enum srv_initaddr {
#define SRV_PP_V2_SSL_CIPHER 0x0040 /* proxy protocol version 2 with
cipher used */
#define SRV_PP_V2_AUTHORITY 0x0080 /* proxy protocol version 2 with
authority */
#define SRV_PP_V2_CRC32C 0x0100 /* proxy protocol version 2 with
crc32c */
+#define SRV_PP_V2_UNIQUE_ID 0x0200 /* proxy protocol version 2 with
unique ID */
/* function which act on servers need to return various errors */
#define SRV_STATUS_OK 0 /* everything is OK. */
diff --git a/reg-tests/connection/proxy_protocol_send_unique_id.vtc
b/reg-tests/connection/proxy_protocol_send_unique_id.vtc
new file mode 100644
index 000000000..ab1e8f60a
--- /dev/null
+++ b/reg-tests/connection/proxy_protocol_send_unique_id.vtc
@@ -0,0 +1,42 @@
+varnishtest "Check that the unique ID TLV is properly sent"
+
+#REQUIRE_VERSION=2.2
+
+feature ignore_unknown_macro
+
+haproxy h1 -conf {
+ defaults
+ mode http
+ log global
+ unique-id-format %{+X}o\ TEST-%[req.hdr(in)]
+
+ listen sender
+ bind "fd@${feS}"
+
+ unique-id-header unique_id
+
+ server example ${h1_feR_addr}:${h1_feR_port} send-proxy-v2
proxy-v2-options unique-id
+
+ listen receiver
+ bind "fd@${feR}" accept-proxy
+
+ http-request set-var(txn.http_unique_id) req.hdr(unique_id)
+ http-request set-var(txn.proxy_unique_id) fc_pp_unique_id
+ http-after-response set-header http_unique_id
%[var(txn.http_unique_id)]
+ http-after-response set-header proxy_unique_id
%[var(txn.proxy_unique_id)]
+ http-request return status 200
+} -start
+
+# Validate that a correct header passes
+client c1 -connect ${h1_feS_sock} {
+ txreq -url "/" \
+ -hdr "in: foo"
+ rxresp
+ expect resp.http.http_unique_id == "TEST-foo"
+ expect resp.http.proxy_unique_id == "TEST-foo"
+ txreq -url "/" \
+ -hdr "in: bar"
+ rxresp
+ expect resp.http.http_unique_id == "TEST-bar"
+ expect resp.http.proxy_unique_id == "TEST-foo"
+} -run
diff --git a/src/connection.c b/src/connection.c
index 728c0ec39..ba0e0fcd1 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -1241,12 +1241,12 @@ int conn_recv_socks4_proxy_response(struct connection
*conn)
}
/* Note: <remote> is explicitly allowed to be NULL */
-int make_proxy_line(char *buf, int buf_len, struct server *srv, struct
connection *remote)
+int make_proxy_line(char *buf, int buf_len, struct server *srv, struct
connection *remote, struct stream *strm)
{
int ret = 0;
if (srv && (srv->pp_opts & SRV_PP_V2)) {
- ret = make_proxy_line_v2(buf, buf_len, srv, remote);
+ ret = make_proxy_line_v2(buf, buf_len, srv, remote, strm);
}
else {
if (remote && conn_get_src(remote) && conn_get_dst(remote))
@@ -1353,7 +1353,7 @@ static int make_tlv(char *dest, int dest_len, char type,
uint16_t length, const
}
/* Note: <remote> is explicitly allowed to be NULL */
-int make_proxy_line_v2(char *buf, int buf_len, struct server *srv, struct
connection *remote)
+int make_proxy_line_v2(char *buf, int buf_len, struct server *srv, struct
connection *remote, struct stream *strm)
{
const char pp2_signature[] = PP2_SIGNATURE;
void *tlv_crc32c_p = NULL;
@@ -1467,6 +1467,20 @@ int make_proxy_line_v2(char *buf, int buf_len, struct
server *srv, struct connec
ret += make_tlv(&buf[ret], (buf_len - ret),
PP2_TYPE_AUTHORITY, value_len, value);
}
}
+
+ if (srv->pp_opts & SRV_PP_V2_UNIQUE_ID) {
+ struct session* sess = strm_sess(strm);
+ struct ist unique_id = stream_generate_unique_id(strm,
&sess->fe->format_unique_id);
+
+ value = unique_id.ptr;
+ value_len = unique_id.len;
+
+ if (value_len >= 0) {
+ if ((buf_len - ret) < sizeof(struct tlv))
+ return 0;
+ ret += make_tlv(&buf[ret], (buf_len - ret),
PP2_TYPE_UNIQUE_ID, value_len, value);
+ }
+ }
#ifdef USE_OPENSSL
if (srv->pp_opts & SRV_PP_V2_SSL) {
diff --git a/src/log.c b/src/log.c
index 8f502ac7e..2cac07486 100644
--- a/src/log.c
+++ b/src/log.c
@@ -137,7 +137,7 @@ static const struct logformat_type logformat_keywords[] = {
{ "CC", LOG_FMT_CCLIENT, PR_MODE_HTTP, LW_REQHDR, NULL }, /* client
cookie */
{ "CS", LOG_FMT_CSERVER, PR_MODE_HTTP, LW_RSPHDR, NULL }, /* server
cookie */
{ "H", LOG_FMT_HOSTNAME, PR_MODE_TCP, LW_INIT, NULL }, /* Hostname */
- { "ID", LOG_FMT_UNIQUEID, PR_MODE_HTTP, LW_BYTES, NULL }, /* Unique ID
*/
+ { "ID", LOG_FMT_UNIQUEID, PR_MODE_TCP, LW_BYTES, NULL }, /* Unique ID */
{ "ST", LOG_FMT_STATUS, PR_MODE_TCP, LW_RESP, NULL }, /* status code
*/
{ "T", LOG_FMT_DATEGMT, PR_MODE_TCP, LW_INIT, NULL }, /* date GMT */
{ "Ta", LOG_FMT_Ta, PR_MODE_HTTP, LW_BYTES, NULL }, /* Time active
(tr to end) */
diff --git a/src/server.c b/src/server.c
index 965570222..908439d00 100644
--- a/src/server.c
+++ b/src/server.c
@@ -656,6 +656,8 @@ static int srv_parse_proxy_v2_options(char **args, int
*cur_arg,
newsrv->pp_opts |= SRV_PP_V2_AUTHORITY;
} else if (!strcmp(p, "crc32c")) {
newsrv->pp_opts |= SRV_PP_V2_CRC32C;
+ } else if (!strcmp(p, "unique-id")) {
+ newsrv->pp_opts |= SRV_PP_V2_UNIQUE_ID;
} else
goto fail;
}
diff --git a/src/stream_interface.c b/src/stream_interface.c
index 47cbd9693..7bb864d52 100644
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -354,9 +354,12 @@ int conn_si_send_proxy(struct connection *conn, unsigned
int flag)
if (cs && cs->data_cb == &si_conn_cb) {
struct stream_interface *si = cs->data;
struct conn_stream *remote_cs =
objt_cs(si_opposite(si)->end);
+ struct stream *strm = si_strm(si);
+
ret = make_proxy_line(trash.area, trash.size,
objt_server(conn->target),
- remote_cs ? remote_cs->conn :
NULL);
+ remote_cs ? remote_cs->conn :
NULL,
+ strm);
/* We may not have a conn_stream yet, if we don't
* know which mux to use, because it will be decided
* during the SSL handshake. In this case, there should
@@ -371,7 +374,8 @@ int conn_si_send_proxy(struct connection *conn, unsigned
int flag)
ret = make_proxy_line(trash.area, trash.size,
objt_server(conn->target),
- objt_conn(sess->origin));
+ objt_conn(sess->origin),
+ NULL);
}
else {
/* The target server expects a LOCAL line to be sent
first. Retrieving
@@ -381,7 +385,8 @@ int conn_si_send_proxy(struct connection *conn, unsigned
int flag)
goto out_wait;
ret = make_proxy_line(trash.area, trash.size,
- objt_server(conn->target), conn);
+ objt_server(conn->target), conn,
+ NULL);
}
if (!ret)
--
2.25.1
