Re: [PATCH] MINOR: connection: add "set-authority" and "authority" sample fetch
On 9/26/19 19:27, Emmanuel Hocdet wrote:
>
>>>
>>> Proposal reworking after playing with « authority » and look at how « src
>>> »/« dst » are working.
>>>
>>> Authority » can come from transport layer (TLS), ProxyV2 TLV or «
>>> set-authority ».
>>> « src/dst » is set from transport layer (TCP), overwrite by Proxy-protocol
>>> and « set-{src,dst} »
>>> I propose to do the same for « authority » sample fetch:
>>> pick « authority » from « set-authority, Proxy-protocol, and transport
>>> layer (in this order)
>>> . It’s already what authority is in « proxy-v2-options authority"
>>> => « fc_pp_authority » disappears in favour of the generic « authority »
>>> sample fetch
>>
>> Some thoughts that come to mind -- it sounds like there will be a bit of
>> "magic" at work here, so will it be transparent to the user? Will users
>> find that the authority field is being set and they wonder where it came
>> from?
>
> I think we can. It will simplify the usage in the vast majority of cases.
OK
>> And I wonder if there are situations in which someone will want to
>> specifically choose one source of truth for authority over the other.
>> Suppose an incoming connection uses TLS with an SNI, and the peer
>> component also sends an authority TLV via Proxy. Is a situation
>> imaginable in which only one of them is getting it "right", for the
>> purposes of haproxy, and the config author wants to be sure to catch
>> that one only?
>
> You can with the sample fetch from transport layer, « ssl_fc_sni » for TLS.
Then if I understand correctly:
- when you prefer the authority value from TLS, use the ssl_fc_sni fetch
- if you prefer the value from the Proxy TLV, just use the authority
fetch, since that one prefers the TLV over the value from TLS, according
to the rules described above.
Is that right?
Best,
Geoff
--
** * * UPLEX - Nils Goroll Systemoptimierung
Scheffelstraße 32
22301 Hamburg
Tel +49 40 2880 5731
Mob +49 176 636 90917
Fax +49 40 42949753
http://uplex.de
signature.asc
Description: OpenPGP digital signature
Re: [PATCH] MINOR: connection: add "set-authority" and "authority" sample fetch
On 9/26/19 11:43, Emmanuel Hocdet wrote:
>
> Proposal reworking after playing with « authority » and look at how « src »/«
> dst » are working.
>
> Authority » can come from transport layer (TLS), ProxyV2 TLV or «
> set-authority ».
> « src/dst » is set from transport layer (TCP), overwrite by Proxy-protocol
> and « set-{src,dst} »
> I propose to do the same for « authority » sample fetch:
> pick « authority » from « set-authority, Proxy-protocol, and transport layer
> (in this order)
> . It’s already what authority is in « proxy-v2-options authority"
> => « fc_pp_authority » disappears in favour of the generic « authority »
> sample fetch
Some thoughts that come to mind -- it sounds like there will be a bit of
"magic" at work here, so will it be transparent to the user? Will users
find that the authority field is being set and they wonder where it came
from?
And I wonder if there are situations in which someone will want to
specifically choose one source of truth for authority over the other.
Suppose an incoming connection uses TLS with an SNI, and the peer
component also sends an authority TLV via Proxy. Is a situation
imaginable in which only one of them is getting it "right", for the
purposes of haproxy, and the config author wants to be sure to catch
that one only?
To be honest I'm not sure, I'm still a bit of an "outsider" around here,
and other readers of the list will have better intuitions about what's
common and possible. So I'd be happy to be assured that this will be fine.
Incidentally, I too have not seen a patch on this thread since September
10th (not sure if you meant to send a new one today).
Best,
Geoff
--
** * * UPLEX - Nils Goroll Systemoptimierung
Scheffelstraße 32
22301 Hamburg
Tel +49 40 2880 5731
Mob +49 176 636 90917
Fax +49 40 42949753
http://uplex.de
signature.asc
Description: OpenPGP digital signature
Re: RFC uuid for log-format
On 9/6/19 11:22, Willy Tarreau wrote: > > We can then have 4 being the only one implemented for now, we can then > set it to the default one if unspecified, but I really want to support > such a parameter so that it's easy to extend the form once a new one > arrives. Still picking nits (although I think this is a good plan) -- if uuid() takes a parameter, and version 4 is the only one that's implemented, then uuid(1) or uuid(3) etc throws an error? Best, Geoff -- ** * * UPLEX - Nils Goroll Systemoptimierung Scheffelstraße 32 22301 Hamburg Tel +49 40 2880 5731 Mob +49 176 636 90917 Fax +49 40 42949753 http://uplex.de signature.asc Description: OpenPGP digital signature
Re: RFC uuid for log-format
On 9/6/19 09:57, Schimweg, Luca wrote: > > UUIDs generally have different versions. I implemented version 4 > right now, but if you want to we can implement different UUID types. > I personally think one is enough, because most people won't care if > they get an UUID v1, v2, ..., I agree with this. Most people want UUIDs just for the uniqueness, and don't care if there's something like a MAC address or timestamp in there somewhere. I would suggest just v4 first, and the rest only if anyone ever asks for them. I doubt that anyone ever will. Best, Geoff -- ** * * UPLEX - Nils Goroll Systemoptimierung Scheffelstraße 32 22301 Hamburg Tel +49 40 2880 5731 Mob +49 176 636 90917 Fax +49 40 42949753 http://uplex.de signature.asc Description: OpenPGP digital signature
Re: RFC uuid for log-format
On 9/5/19 17:43, Schimweg, Luca wrote: > > actually, I am doing that, I generate a random number from 0-16383 > and after that I add 32768 onto it. 32768 is 0x8000, > 32768+16383=49151 is 0xBFFF. Ah, I missed it, sorry again. Geoff -- ** * * UPLEX - Nils Goroll Systemoptimierung Scheffelstraße 32 22301 Hamburg Tel +49 40 2880 5731 Mob +49 176 636 90917 Fax +49 40 42949753 http://uplex.de signature.asc Description: OpenPGP digital signature
Re: RFC uuid for log-format
On 9/5/19 16:58, Schimweg, Luca wrote: > > > unique-id-format > %[rand,hex,bytes(8,8),lower]-%[rand(65536),hex,bytes(12,4),lower]-4%[rand(4096),hex,bytes(13,3),lower]-%[rand(16384),add(32768),hex,bytes(12,4),lower]-%[rand(65536),hex,bytes(12,4),lower]%[rand,hex,bytes(8,8),lower] This is going to be very nit-picky (I apologize): that comes close but isn't actually guaranteed to produce an RFC 4122 version 4 UUID (the "random" UUID). You're right about the 4 at the beginning of the third group of hex digits. But the first digit of the fourth group is required to be one of 8, 9, a or b. I've never understood why, but there it is. Whether or not that matters, it seems to me, depends on requirements. If you just need 16 bytes of randomness for a request ID, why not just generate that? base64 encoding would be shorter, by the way -- 25 chars for base64 with padding, 36 for the UUID format. But if someone has an interoperability requirement dictating a UUID that has to meet the standard format, then I agree that a fetch that gets it just right would be warranted. Best, Geoff -- ** * * UPLEX - Nils Goroll Systemoptimierung Scheffelstraße 32 22301 Hamburg Tel +49 40 2880 5731 Mob +49 176 636 90917 Fax +49 40 42949753 http://uplex.de signature.asc Description: OpenPGP digital signature
Re: [PATCH] MINOR: Add the fc_pp_authority fetch -- authority TLV, from PROXYv2
On 8/28/19 17:19, Willy Tarreau wrote: > > Thank you Geoff. The code is pretty clean and straightforward, I've > now merged it. I'm pretty sure it will be very useful! Now you and Nils > have everything you need to finish your onloader project ;-) Great news, thanks for your help. It feels good to have contributed a modest few lines to haproxy, will sip some fine French wine to celebrate. Thanks, Geoff -- ** * * UPLEX - Nils Goroll Systemoptimierung Scheffelstraße 32 22301 Hamburg Tel +49 40 2880 5731 Mob +49 176 636 90917 Fax +49 40 42949753 http://uplex.de signature.asc Description: OpenPGP digital signature
Re: [PATCH] MINOR: Add the fc_pp_authority fetch -- authority TLV, from PROXYv2
This is with the fix for the misplaced #endif spotted by Emmanuel, thx again. Geoff -- ** * * UPLEX - Nils Goroll Systemoptimierung Scheffelstraße 32 22301 Hamburg Tel +49 40 2880 5731 Mob +49 176 636 90917 Fax +49 40 42949753 http://uplex.de signature.asc Description: OpenPGP digital signature
[PATCH] MINOR: Add the fc_pp_authority fetch -- authority TLV, from PROXYv2
Save the authority TLV in a PROXYv2 header from the client connection,
if present, and make it available as fc_pp_authority.
The fetch can be used, for example, to set the SNI for a backend TLS
connection.
---
doc/configuration.txt | 4
include/proto/connection.h | 7 +++
include/types/connection.h | 4
src/connection.c | 37 +
4 files changed, 52 insertions(+)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 4e18f0f6e..4c820d266 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -14460,6 +14460,10 @@ fc_http_major : integer
for HTTP/0.9 to HTTP/1.1 or 2 for HTTP/2. Note, this is based on the on-wire
encoding and not on the version present in the request header.
+fc_pp_authority : string
+ Returns the authority TLV sent by the client in the PROXY protocol header,
+ if any.
+
fc_rcvd_proxy : boolean
Returns true if the client initiated the connection with a PROXY protocol
header.
diff --git a/include/proto/connection.h b/include/proto/connection.h
index bbb9759ac..059d5d85a 100644
--- a/include/proto/connection.h
+++ b/include/proto/connection.h
@@ -35,6 +35,7 @@
extern struct pool_head *pool_head_connection;
extern struct pool_head *pool_head_connstream;
extern struct pool_head *pool_head_sockaddr;
+extern struct pool_head *pool_head_authority;
extern struct xprt_ops *registered_xprt[XPRT_ENTRIES];
extern struct mux_proto_list mux_proto_list;
@@ -471,6 +472,7 @@ static inline void conn_init(struct connection *conn)
conn->idle_time = 0;
conn->src = NULL;
conn->dst = NULL;
+ conn->proxy_authority = NULL;
}
/* sets as the connection's owner */
@@ -617,6 +619,11 @@ static inline void conn_free(struct connection *conn)
sockaddr_free(&conn->src);
sockaddr_free(&conn->dst);
+ if (conn->proxy_authority != NULL) {
+ pool_free(pool_head_authority, conn->proxy_authority);
+ conn->proxy_authority = NULL;
+ }
+
/* By convention we always place a NULL where the ctx points to if the
* mux is null. It may have been used to store the connection as a
* stream_interface's end point for example.
diff --git a/include/types/connection.h b/include/types/connection.h
index f8e4c07f9..f6a7d1b7e 100644
--- a/include/types/connection.h
+++ b/include/types/connection.h
@@ -461,7 +461,9 @@ struct connection {
void (*destroy_cb)(struct connection *conn); /* callback to notify of
imminent death of the connection */
struct sockaddr_storage *src; /* source address (pool), when known,
otherwise NULL */
struct sockaddr_storage *dst; /* destination address (pool), when
known, otherwise NULL */
+ char *proxy_authority;/* Value of authority TLV received via
PROXYv2 */
unsigned int idle_time; /* Time the connection was
added to the idle list, or 0 if not in the idle list */
+ uint8_t proxy_authority_len; /* Length of authority TLV received via
PROXYv2 */
};
/* PROTO token registration */
@@ -578,6 +580,8 @@ struct tlv_ssl {
#define PP2_CLIENT_CERT_CONN 0x02
#define PP2_CLIENT_CERT_SESS 0x04
+/* Max length of the authority TLV */
+#define PP2_AUTHORITY_MAX 255
/*
* Linux seems to be able to send 253 fds per sendmsg(), not sure
diff --git a/src/connection.c b/src/connection.c
index 602fc79ec..7bdd6f0dd 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -32,6 +32,7 @@
DECLARE_POOL(pool_head_connection, "connection", sizeof(struct connection));
DECLARE_POOL(pool_head_connstream, "conn_stream", sizeof(struct conn_stream));
DECLARE_POOL(pool_head_sockaddr, "sockaddr",sizeof(struct
sockaddr_storage));
+DECLARE_POOL(pool_head_authority, "authority", PP2_AUTHORITY_MAX);
struct xprt_ops *registered_xprt[XPRT_ENTRIES] = { NULL, };
@@ -631,6 +632,16 @@ int conn_recv_proxy(struct connection *conn, int flag)
break;
}
#endif
+ case PP2_TYPE_AUTHORITY: {
+ if (tlv_len > PP2_AUTHORITY_MAX)
+ goto bad_header;
+ conn->proxy_authority =
pool_alloc(pool_head_authority);
+ if (conn->proxy_authority == NULL)
+ goto fail;
+ memcpy(conn->proxy_authority, (const
char *)tlv_packet->value, tlv_len);
+ conn->proxy_authority_len = tlv_len;
+ break;
+ }
default:
break;
}
@@ -1415,6 +1426,31 @@ int smp_fetch_fc_rcvd_proxy(const struct arg *args,
struct sample *smp, co
Re: [PATCH] MINOR: Add the fc_pp_authority fetch -- authority TLV from PROXYv2
On 8/27/19 18:15, Emmanuel Hocdet wrote:
>
>> @@ -630,6 +631,17 @@ int conn_recv_proxy(struct connection *conn, int flag)
>> conn->proxy_netns = ns;
>> break;
>> }
>> +
>> +case PP2_TYPE_AUTHORITY: {
>
> Is inside #ifdef USE_NS
Oh, now I see it. Will fix.
Thx,
Geoff
--
** * * UPLEX - Nils Goroll Systemoptimierung
Scheffelstraße 32
22301 Hamburg
Tel +49 40 2880 5731
Mob +49 176 636 90917
Fax +49 40 42949753
http://uplex.de
signature.asc
Description: OpenPGP digital signature
[PATCH] MINOR: Add the fc_pp_authority fetch -- authority TLV from PROXYv2
Save the authority TLV sent in a PROXYv2 header from the client
connection, if present, and make it available as fc_pp_authority.
The fetch can be used, for example, to set the SNI for a backend TLS
connection.
---
doc/configuration.txt | 4
include/proto/connection.h | 7 +++
include/types/connection.h | 4
src/connection.c | 38 ++
4 files changed, 53 insertions(+)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 4e18f0f6e..4c820d266 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -14460,6 +14460,10 @@ fc_http_major : integer
for HTTP/0.9 to HTTP/1.1 or 2 for HTTP/2. Note, this is based on the on-wire
encoding and not on the version present in the request header.
+fc_pp_authority : string
+ Returns the authority TLV sent by the client in the PROXY protocol header,
+ if any.
+
fc_rcvd_proxy : boolean
Returns true if the client initiated the connection with a PROXY protocol
header.
diff --git a/include/proto/connection.h b/include/proto/connection.h
index bbb9759ac..059d5d85a 100644
--- a/include/proto/connection.h
+++ b/include/proto/connection.h
@@ -35,6 +35,7 @@
extern struct pool_head *pool_head_connection;
extern struct pool_head *pool_head_connstream;
extern struct pool_head *pool_head_sockaddr;
+extern struct pool_head *pool_head_authority;
extern struct xprt_ops *registered_xprt[XPRT_ENTRIES];
extern struct mux_proto_list mux_proto_list;
@@ -471,6 +472,7 @@ static inline void conn_init(struct connection *conn)
conn->idle_time = 0;
conn->src = NULL;
conn->dst = NULL;
+ conn->proxy_authority = NULL;
}
/* sets as the connection's owner */
@@ -617,6 +619,11 @@ static inline void conn_free(struct connection *conn)
sockaddr_free(&conn->src);
sockaddr_free(&conn->dst);
+ if (conn->proxy_authority != NULL) {
+ pool_free(pool_head_authority, conn->proxy_authority);
+ conn->proxy_authority = NULL;
+ }
+
/* By convention we always place a NULL where the ctx points to if the
* mux is null. It may have been used to store the connection as a
* stream_interface's end point for example.
diff --git a/include/types/connection.h b/include/types/connection.h
index f8e4c07f9..f6a7d1b7e 100644
--- a/include/types/connection.h
+++ b/include/types/connection.h
@@ -461,7 +461,9 @@ struct connection {
void (*destroy_cb)(struct connection *conn); /* callback to notify of
imminent death of the connection */
struct sockaddr_storage *src; /* source address (pool), when known,
otherwise NULL */
struct sockaddr_storage *dst; /* destination address (pool), when
known, otherwise NULL */
+ char *proxy_authority;/* Value of authority TLV received via
PROXYv2 */
unsigned int idle_time; /* Time the connection was
added to the idle list, or 0 if not in the idle list */
+ uint8_t proxy_authority_len; /* Length of authority TLV received via
PROXYv2 */
};
/* PROTO token registration */
@@ -578,6 +580,8 @@ struct tlv_ssl {
#define PP2_CLIENT_CERT_CONN 0x02
#define PP2_CLIENT_CERT_SESS 0x04
+/* Max length of the authority TLV */
+#define PP2_AUTHORITY_MAX 255
/*
* Linux seems to be able to send 253 fds per sendmsg(), not sure
diff --git a/src/connection.c b/src/connection.c
index 602fc79ec..a4f9fdbac 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -32,6 +32,7 @@
DECLARE_POOL(pool_head_connection, "connection", sizeof(struct connection));
DECLARE_POOL(pool_head_connstream, "conn_stream", sizeof(struct conn_stream));
DECLARE_POOL(pool_head_sockaddr, "sockaddr",sizeof(struct
sockaddr_storage));
+DECLARE_POOL(pool_head_authority, "authority", PP2_AUTHORITY_MAX);
struct xprt_ops *registered_xprt[XPRT_ENTRIES] = { NULL, };
@@ -630,6 +631,17 @@ int conn_recv_proxy(struct connection *conn, int flag)
conn->proxy_netns = ns;
break;
}
+
+ case PP2_TYPE_AUTHORITY: {
+ if (tlv_len > PP2_AUTHORITY_MAX)
+ goto bad_header;
+ conn->proxy_authority =
pool_alloc(pool_head_authority);
+ if (conn->proxy_authority == NULL)
+ goto fail;
+ memcpy(conn->proxy_authority, (const
char *)tlv_packet->value, tlv_len);
+ conn->proxy_authority_len = tlv_len;
+ break;
+ }
#endif
default:
break;
@@ -1415,6 +1427,31 @@ int smp_fetch_fc_rcvd_proxy(c
Re: [RFC] setting the backend SNI from the client's authority TLV, when the target address was forwarded
On 8/26/19 18:03, Emmanuel Hocdet wrote: > > Great to see TLS onloader continue. Working on it ... > About the TLS onloader configuration. If i understand the principle of > servers set to 0.0.0.0 and stick table: > The server configuration will look like: >server s0 0.0.0.0:0 ssl sni fc_pp_authority >[…] Yes, I'm currently testing a new patch, and the config looks very much like that. Real-world use cases may want to implement the fallback logic that we were talking about earlier in the thread, since fc_pp_authority may or may not have been present in the PROXY header. "Set SNI to fc_pp_authority if it was sent, otherwise set it to ssl_fc_sni". Best, Geoff -- ** * * UPLEX - Nils Goroll Systemoptimierung Scheffelstraße 32 22301 Hamburg Tel +49 40 2880 5731 Mob +49 176 636 90917 Fax +49 40 42949753 http://uplex.de signature.asc Description: OpenPGP digital signature
Re: [RFC] setting the backend SNI from the client's authority TLV, when the target address was forwarded
On 8/22/19 14:40, Willy Tarreau wrote: > >> I would suggest naming it something like fc_authority or >> fc_pp_authority, to be specific about where it came from. Since you used fc_pp_authority in an example further down, I'll take that as the choice (unless somebody yells). Seems better to me, since just "authority" could refer to a number of things. > You don't need to have the extra boolean because our sample fetch functions > already return this information for you. The "-m found" match method > indicates whether or not the requested information was present, regardless > of its possible emptiness. > >> I assume the fetch for the authority TLV should return the empty string >> if none was read; correct? > > No it returns a "not found" (return 0 technically speaking). Ah, OK thx. >> - Use a dedicated memory pool to allocate the name from TLV, if there is >> one in the proxy header. > > Yep. Please name it "authority" then. At least if we need an authority > anywhere else we know we can share it. OK All right, I think we've covered enough that I can take another go at coding it up. And I believe I can mail a patch next time, if there are no objections. Since the patch will be adding a fetch, I'd say the regression risk is MINOR, as no one could have ever used it before. Willy, thanks again for the feedback, the first impression about working with haproxy development has been very pleasant. Best, Geoff -- ** * * UPLEX - Nils Goroll Systemoptimierung Scheffelstraße 32 22301 Hamburg Tel +49 40 2880 5731 Mob +49 176 636 90917 Fax +49 40 42949753 http://uplex.de signature.asc Description: OpenPGP digital signature
Re: [RFC] setting the backend SNI from the client's authority TLV, when the target address was forwarded
On 8/21/19 21:50, Willy Tarreau wrote: > > Thus welcome to the list :-) Thanks for the informative and welcoming response. %^) > Just to know a bit more about your use case, thus your client speaks > in clear to haproxy by prepending a PPv2 header and lets haproxy serve > as a TLS "onloader" if I can call it like this, that's it ? If so, this > is pretty close to what is also being done in Varnish to allow it to > use haproxy to gateway to TLS servers : > > https://github.com/varnishcache/varnish-cache/pull/2957 Spot on, that's the PR that I'm working on with my colleague Nils. Mine is a PR to his PR, if that makes any sense; after review (assuming he approves), the part about setting the authority TLV will go into the Varnish PR. I suspect that there are other ways that the authority TLV can be useful for haproxy besides the specific Varnish case. Someone connecting via TLS, for example, might want to send the TLV to "override" the SNI for certain backends. >> It seemed to make sense to use a memory pool for the allocation, but it >> also felt like overkill to add a new memory pool just for this feature. > > No it's not overkill, pools are extremely cheap and they are fused when > they have very close sizes. I just had a quick look and saw that appctx > is 248 bytes si it will end up being fused with your pool, so by adding > a pool, your entries will cost exactly zero. So really, don't be shy on > this. > >> Maybe define a >> dedicated pool after all? > > Yep :-) All right then, a new memory pool will be introduced. I thought there could be an objection to special-casing just the one class of TLV, since there's a variety of them. A general solution might be to have the field point to a table of TLVs read from the connection, if there were any. That would complicate the allocation, though. My instinct is to not generalize from a single use case, especially if it introduces new complexities before it's clear whether anyone wants to use them. If haproxy users find that they like accessing the authority TLV and want more, that can be accommodated when the time comes. > So I'm having a small disagreement on the way to configure this, but > there is a nice solution that should not change much of what you've > done. [...] > > But the solution is actually very simple. The current "sni" directive > takes a sample expression. You "just" have to write a sample fetch to > return the SNI extracted from the PP header. Just call it "fc_sni", we > already have a number of other "fc_*" sample fetch functions for stuff > extracted from the front connection. I agree, the fetch looks like the better solution. I would suggest naming it something like fc_authority or fc_pp_authority, to be specific about where it came from. If it's available as a fetch, then SNI doesn't have to be its only purpose. Conceivably someone could use it to set a Host header, define an ACL, or do other things we haven't considered yet. And I think there would have to be a boolean like fc_rcvd_authority, to find out if the TLV had been sent at all. That way, the fallback logic can be made explicit in the config: set SNI from fc_authority if present, otherwise set it from fc_sni. I assume the fetch for the authority TLV should return the empty string if none was read; correct? > Also one thing I think we're missing is fc_has_pp() or something like > this to indicate that the front connection used the proxy protocol. > And in this case it can prove very useful. Isn't that what fc_rcvd_proxy says? So to summarize my takeaways thus far: - Use a dedicated memory pool to allocate the name from TLV, if there is one in the proxy header. - Introduce a fetch to use it in configuration. - I suggest naming the fetch to identify it specifically as the authority TLV from PP, and also introducing a boolean fetch that says whether any such was read. Thanks again, this has been an encouraging start. %^) Best, Geoff -- ** * * UPLEX - Nils Goroll Systemoptimierung Scheffelstraße 32 22301 Hamburg Tel +49 40 2880 5731 Mob +49 176 636 90917 Fax +49 40 42949753 http://uplex.de signature.asc Description: OpenPGP digital signature
[RFC] setting the backend SNI from the client's authority TLV, when the target address was forwarded
Hello to readers of the haproxy list, This is my first-ever mail to the list, to propose my first-ever contribution to the project, so I apologize in advance if anything here runs afoul of your customs. I hope I've come close enough to make it worth your while. What I'm proposing has been coded up here: https://github.com/slimhazard/haproxy/commit/2093dc54f0d5f7d0fb30c918c2c7689b8764bec1 And of course I can send a git-formatted patch to the list (but my understanding is that an RFC discussion is preferred first). The idea is probably best understood from a sample config: listen fe # ... server s0 0.0.0.0:0 ssl verify none forward-dst-sni This is only relevant for the case of "forwarding" the client's destination address, by using 0.0.0.0 or * in the server config. When the new option 'forward-dst-sni' is enabled for such a server, then for clients who send an authority TLV in the PROXY header, the SNI for the backend is set to the value of the TLV. The use case is for clients who want to use haproxy's "address forwarding" feature, but also need to communicate with backends via haproxy that use SNI for "virtual hosing with SSL". With this feature they can do that by using PROXYv2 and TLV. The option is a NOP for servers that don't have the 0.0.0.0/* config. When it's enabled for such a server, and when the authority TLV is sent, then the authority value overrides any SNI set in the client TLS handshake. The rationale is that if a client goes to the trouble of sending the TLV, then they want that to be used for the SNI (otherwise they can just let the SNI be passed on from the client TLS). I considered adding a test in reg-tests, but I don't see a way to set a PROXY TLV value using VTest and haproxy (happy to be corrected if I'm wrong). But we do have some varnishtest VTCs for it here, for a PR that my colleague and I are working on for Varnish: https://github.com/slimhazard/varnish-cache/blob/proxy_via_authority/bin/varnishtest/tests/c00100.vtc https://github.com/slimhazard/varnish-cache/blob/proxy_via_authority/bin/varnishtest/tests/c00101.vtc As for the coding, I suspect that my newbie status with haproxy shows mostly in the way space is allocated for the authority string, which the patch adds as a new field to struct connection. Since the feature is likely to be used only rarely, it doesn't seem sensible to foresee all of the space for a host name right in the struct. In the patch I have it as a pointer to struct buffer, which is NULL unless the authority TLV is read from the PROXY header. It seemed to make sense to use a memory pool for the allocation, but it also felt like overkill to add a new memory pool just for this feature. So in the patch I'm just using the "trash pool". Aside from the fact that the name "trash pool" makes me think that it might not be meant to be used this way, if I understand correctly the default allocation size is 16KiB, and users are advised not to reduce it. That's far too large for a host name (usually max 255 bytes). So I suspect that this may not be best practice for haproxy. Maybe define a dedicated pool after all? I'd be grateful for any feedback about the feature and the code (or anything else). And of course I can send a patch to the list if you're interested. Thanks, Geoff Simmons -- ** * * UPLEX - Nils Goroll Systemoptimierung Scheffelstraße 32 22301 Hamburg Tel +49 40 2880 5731 Mob +49 176 636 90917 Fax +49 40 42949753 http://uplex.de signature.asc Description: OpenPGP digital signature
