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
