REQ_DENY/REQ_CAPTURE rules drop the https frontend for non-passthrough ingresses once any ingress in the cluster uses ssl-passthrough

[KlausNie]

Summary

haproxy.SSLPassthrough is a controller-wide (process-global) flag: it becomes true as soon as any single ingress in the cluster sets ssl-passthrough: true, not just the ingress currently being processed.
addRules() (pkg/ingress/ingress.go) uses this flag to decide which frontends an ingress's REQ_DENY/REQ_CAPTURE rule (i.e. its allow-list/deny-list or capture annotation) gets attached to:

case rules.REQ_DENY, rules.REQ_CAPTURE:
    if haproxy.SSLPassthrough {
        frontends = []string{h.FrontHTTP, h.FrontSSL}   // FrontHTTPS dropped
    }

When the flag is true, FrontHTTPS is dropped from the frontend list for every ingress carrying such a rule including ingresses that never opted into ssl-passthrough themselves. But a non-passthrough ingress's TLS traffic still terminates on FrontHTTPS: HTTPS.toggleSSLPassthrough (pkg/handler/https.go) replaces FrontHTTPS's public binds with an internal PROXY-protocol unix socket once SSLPassthrough is active, and FrontSSL's default backend (ssl-backend) forwards any non-passthrough SNI there for termination. So that traffic ends up completely unchecked by its own allow-list/deny-list.

Environment

• Controller version: 3.2.9 through 3.2.12 (verified at the exact lines in both tags), still present on master@fc120676
• Chart: kubernetes-ingress-1.52.0 / 1.52.1
• Observed on a cluster with two ingresses on different hosts: one plain (haproxy.org/allow-list, no passthrough) and one with ssl-passthrough: true (fronting Pinniped Concierge's impersonation proxy, which needs the raw mTLS handshake to terminate at the backend for client-cert auth passthrough is a hard requirement there, not incidental)

Impact observed

With the allow-list ingress and the passthrough ingress coexisting:

• Plain HTTP to the allow-listed host: correctly 403 for non-allow-listed source IPs (FrontHTTP always keeps the rule)
• HTTPS (both TCP/HTTP2 and QUIC/HTTP3) to the same host: reached the backend with no enforcement at all for the same non-allow-listed IPs
  This is a silent fail-open on the allow-list/deny-list mechanism for any non-passthrough ingress, whenever any other ingress in the cluster uses passthrough.

Relationship to #770

This is a distinct bug from #770 ("whitelist/allow-list works intermittently"), though in the same area. #770's root cause diagnosed by ivanmatmati in that thread and addressed by 79d754a (shipped in v3.2.12 as 4088ca7) was that the global SSLPassthrough flag was set mid-iteration over ingresses, in Go map order, so the value observed by each ingress's rule construction was non-deterministic within a single reconcile pass. That fix moved flag resolution into a dedicated pre-pass, making the flag's value stable per-pass.
This issue is different: even with a stable, correctly-resolved flag value, addRules()'s frontend-selection logic itself unconditionally drops FrontHTTPS whenever the flag is true a deterministic, 100%-reproducible bug, not a timing/ordering one. (79d754a's own commit message notes that "during the cycles where the rule sat on the wrong frontend the IP allow/deny-list was silently not enforced" that fix stabilized which frontend set is chosen, but the stable choice still drops FrontHTTPS.)

I hit this directly: after upgrading to 3.2.12 to pick up that fix, the reload-flapping stopped, but the allow-list became permanently (not intermittently) unenforced on FrontHTTPS traffic for the non-passthrough ingress.

Proposed fix

Keep FrontHTTPS in the frontend list alongside FrontHTTP and FrontSSL:

case rules.REQ_DENY, rules.REQ_CAPTURE:
    if haproxy.SSLPassthrough {
        frontends = []string{h.FrontHTTP, h.FrontSSL, h.FrontHTTPS}
    }

This is safe for the mixed-cluster case: a non-passthrough ingress's host is never written into the SNI map (route.AddHostPathRoute only populates it if route.SSLPassthrough), so the rule's ingress-scoped ACL guard ({ var(txn.sni_match) -m dom }) never matches on FrontSSL for it anyway that placement was already inert before and after this fix. The only thing that was missing was FrontHTTPS, which is where its real traffic lands.
Since the third placement is harmless where it's unreachable and essential where it isn't, that never justified dropping it.
The {http, ssl} selection is correct for the passthrough ingress's own traffic path; the bug is that a global flag applies that per-ingress reasoning to every ingress in the cluster.

A draft PR reproducing this with a failing e2e test (an unrelated ssl-passthrough ingress coexisting with a plain allow-list ingress, asserting the latter's HTTPS traffic is still denied): #831
I can include the fix in this PR also

If I'm using it wrong, please let me know!

[ivanmatmati]

Hi @KlausNie , a fix has already been commited, can you check the latest version to validate ?

[KlausNie]

hey @ivanmatmati ! as of now, on newest master and on the newest tag, my tests are still failing, and the whitelist still fails to apply.

do you mind pointing me to the commit fixing it? I'm happy to investigate further!