nigoroll / libvmod-dynamic

The Varnish dns/named director continued
BSD 2-Clause "Simplified" License
95 stars 34 forks source link

Add the authority parameter to .director(). #76

Closed slimhazard closed 1 year ago

slimhazard commented 2 years ago

Previously this was always set to the value of the VRT_backend host header. That is still the default, but with the new parameter a different value may be set, or the parameter may be set to the empty string, to specify that no authority TLV should be sent.

nigoroll commented 2 years ago

I have the nagging feeling that I still need to think about this more, just some thoughts:

The share parameter exists to define under which circumstances connections can be reused. Historically, DIRECTOR was the only option: If different host names resulted in the same tcp endpoint, they would share that endpoint.

16576f0e796e56171c7478851c04a6989894e13e changed this because of basically two cases:

While, for the latter case, "sharing by host(name)" is adequate, I wonder if, for the former case, "sharing by authority" would actually be better.

Therefor, should we have a share = AUTHORITY option and make that the default for .via?

Somehow related, and in light of this question: While we probably would want to keep an authority parameter at the director level, I think that the more common use case would be to have it as a .backend() argument.

In my world, a common setup (by design, not necessarily in all cases) is to have just two directors:

new http = dynamic.director()
new https = dynamic.director(via = sslon, port = 443)

Then the usage pattern becomes

set bereq.backend = http.backend("http1.site");
# OR
set bereq.backend = https.backend("tls.site");

The way I understand this patch, the latter case would still work as before.

The way I understand the new use case, we now want to be able to do, say, a DNS lookup on "tls.site.cdn.com", but still require the certificate to be valid for authority "tls.site".

So should we support an optional authority argument?

set bereq.backend = https.backend("tls.site.cdn.com", authority="tls.site");

or would it make more sense to keep the semantics of the existing host argument as auhtority also and rather specify a different dns name?

set bereq.backend = https.backend("tls.site", dns="tls.site.cdn.com");
SoerenSilkjaer commented 1 year ago

Hi @nigoroll. I am trying to create a backend that uses tls, are the changes in this pr needed for that or is there already support for it in current release?

Edit: Nvm i found the answer in the documentation https://github.com/nigoroll/libvmod-dynamic/blob/762970fd0d208b6fa477d0f874fd14800a0b966c/src/vmod_dynamic.vcc#L522-L523

nigoroll commented 1 year ago

I think I have made up my mind, my opinion/judgement: