dynamic allowlisting on host and ip prefix#937
Conversation
a7ddd66 to
9d7a770
Compare
4ab36c7 to
3fea9a2
Compare
There was a problem hiding this comment.
Absolutely fine for the current POC state.
I am however worried about the performance impact of this if we decide to use it.
Wondering if it's worth to investigate an alternative approach:
- a separate list that contains only the domains and their pre-calculated parent domains
- First check if
hdr(host)is contained in this list (via full match instead ofbeg, which should be more performant) - Only run the other acls if this first check is true.
Could be more performant in the end since we expect that only a small fraction of all domains will be in this per-domain-allowlist.
Also added some more comment that should be tackled if/when we decide to proceed.
| @@ -0,0 +1,25 @@ | |||
| # generated from domain_ip_allowlist.acl.erb | |||
There was a problem hiding this comment.
We usually use .txt as file ending (instead of .acl), e.g. https://github.com/cloudfoundry/haproxy-boshrelease/blob/master/jobs/haproxy/templates/whitelist_cidrs.txt.erb
| http-request set-var(req.ip_proto) str(4) | ||
| http-request set-var(req.ip_proto) str(6) if { var(req.ip_bin),length gt 32 } | ||
| http-request set-var(req.domain) hdr(host),host_only,lower | ||
| http-request set-var-fmt(req.domain_key) %[var(req.domain)]|%[var(req.ip_proto)]|%[var(req.ip_bin)] |
There was a problem hiding this comment.
There's probably a way to inline the ip_proto variable here and save on variable allocation.
| http-request set-var(req.parent_domain) var(req.domain),regsub(^[^\.]+,) | ||
| http-request set-var-fmt(req.parent_domain_key) %[var(req.parent_domain)]|%[var(req.ip_proto)]|%[var(req.ip_bin)] | ||
| acl domain_ip_matched var(req.domain_key) -m beg -f opt@/var/vcap/jobs/haproxy/config/domain_ip_allowlist.acl | ||
| acl parent_domain_ip_matched var(req.parent_domain_key) -m beg -f opt@/var/vcap/jobs/haproxy/config/domain_ip_allowlist.acl |
There was a problem hiding this comment.
Oh, it's unfortunate that we have to match again with the parent domain, but I guess there's no way around it.
However, couldn't we skip the parent_domain_ip_matched acl, as well as http-request set-var-fmt(req.parent_domain_key) %[var(req.parent_domain)]|%[var(req.ip_proto)]|%[var(req.ip_bin)] if domain_ip_matched is already true?
| # foo.bar.example.com|6|010101...01 (IPv6 exact host) | ||
| http-request set-var(req.ip_bin) src,base2 | ||
| http-request set-var(req.ip_proto) str(4) | ||
| http-request set-var(req.ip_proto) str(6) if { var(req.ip_bin),length gt 32 } |
There was a problem hiding this comment.
I would be surprised if there isn't a HAProxy native way of checking if src is ipv6.
| http-request set-var(req.ip_proto) str(6) if { var(req.ip_bin),length gt 32 } | ||
| http-request set-var(req.domain) hdr(host),host_only,lower | ||
| http-request set-var-fmt(req.domain_key) %[var(req.domain)]|%[var(req.ip_proto)]|%[var(req.ip_bin)] | ||
| http-request set-var(req.parent_domain) var(req.domain),regsub(^[^\.]+,) |
There was a problem hiding this comment.
regsub invokes a full regex engine (PCRE or POSIX depending on build), which is noticeably heavier than HAProxy's plain string converters. For a fixed-structure operation like "drop the first label," you can use the field converter instead, which just does delimiter-based splitting — no regex compilation or matching involved.
http-request set-var(req.parent_domain) req.domain,field(2,.,-1)
domain|ip-version|ip-binaryfor the exact hostname, and.parent-domain|ip-version|ip-binaryfor all subdomainsadd acl / del aclcommands