Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change order of use_backend haproxy.tmpl #1109

Open
jessequinn opened this issue Apr 10, 2024 · 4 comments
Open

Change order of use_backend haproxy.tmpl #1109

jessequinn opened this issue Apr 10, 2024 · 4 comments

Comments

@jessequinn
Copy link

jessequinn commented Apr 10, 2024

What are you trying to do

We require that the SNIBACKEND come before the HOSTBACKEND on the frontend configuration.

What HAProxy Ingress should do or how it should behave differently

frontend _front_https
    mode http
    bind :443 accept-proxy ssl alpn h2,http/1.1 crt-list /etc/haproxy/maps/_front_bind_crt.list ca-ignore-err all crt-ignore-err all
    log-format %ci:%cp\ method=%HM\ uri=%HU\ rcvms=%TR\ serverms=%Tr\ activems=%Ta\ bytes=%B\ status=%ST
    http-request set-var(req.path) path
    http-request set-var(req.host) hdr(host),field(1,:),lower
    http-request set-var(req.base) var(req.host),concat(\#,req.path)
    http-request set-var(req.hostbackend) var(req.base),map_reg(/etc/haproxy/maps/_front_https_host__regex.map)
    http-request set-header X-Forwarded-Proto https
    http-request del-header X-SSL-Client-CN
    http-request del-header X-SSL-Client-DN
    http-request del-header X-SSL-Client-SHA1
    http-request del-header X-SSL-Client-SHA2
    http-request del-header X-SSL-Client-Cert
    acl tls-has-crt ssl_c_used
    acl tls-need-crt ssl_fc_sni -i -m str -f /etc/haproxy/maps/_front_tls_needcrt__exact.list
    acl tls-host-need-crt var(req.host) -i -m str -f /etc/haproxy/maps/_front_tls_needcrt__exact.list
    acl tls-has-invalid-crt ssl_c_verify gt 0
    acl tls-check-crt ssl_fc_sni -i -m str -f /etc/haproxy/maps/_front_tls_auth__exact.list
    http-request set-var(req.snibase) ssl_fc_sni,lower,concat(\#,req.path)
    http-request set-var(req.snibackend) var(req.snibase),map_dir(/etc/haproxy/maps/_front_https_sni__prefix.map)
    http-request set-var(req.snibackend) var(req.base),map_dir(/etc/haproxy/maps/_front_https_sni__prefix.map) if !{ var(req.snibackend) -m found } !tls-has-crt !tls-host-need-crt
    http-request set-var(req.tls_nocrt_redir) str(_internal) if !tls-has-crt tls-need-crt
    http-request set-var(req.tls_invalidcrt_redir) str(_internal) if tls-has-invalid-crt tls-check-crt
    http-request use-service lua.send-421 if tls-has-crt { ssl_fc_has_sni } !{ ssl_fc_sni,strcmp(req.host) eq 0 }
    http-request use-service lua.send-496 if { var(req.tls_nocrt_redir) -m str _internal }
    http-request use-service lua.send-421 if !tls-has-crt tls-host-need-crt
    http-request use-service lua.send-495 if { var(req.tls_invalidcrt_redir) -m str _internal }
    use_backend %[var(req.hostbackend)] if { var(req.hostbackend) -m found }
    use_backend %[var(req.snibackend)] if { var(req.snibackend) -m found } <-- move this up above the hostbackend
    default_backend _error404

This would require a change in the haproxy.tmpl

https://github.com/jcmoraisjr/haproxy-ingress/blob/v0.14.6/rootfs/etc/templates/haproxy/haproxy.tmpl

Original code:

{{- /*------------------------------------*/}}
    use_backend %[var(req.hostbackend)]
        {{- "" }} if { var(req.hostbackend) -m found }
{{- if $hasTLSAuth }}
    use_backend %[var(req.snibackend)]
        {{- "" }} if { var(req.snibackend) -m found }
{{- end }}
{{- template "defaultbackend" map $hosts $defaultbackend }}

{{- end }}{{/* has $fmaps */}}
{{- end }}{{/* define "frontends" */}}

{{- /*------------------------------------*/}}

New code:

{{- /*------------------------------------*/}}
{{- if $hasTLSAuth }}
    use_backend %[var(req.snibackend)]
        {{- "" }} if { var(req.snibackend) -m found }
{{- end }}
    use_backend %[var(req.hostbackend)]
        {{- "" }} if { var(req.hostbackend) -m found }
{{- template "defaultbackend" map $hosts $defaultbackend }}

{{- end }}{{/* has $fmaps */}}
{{- end }}{{/* define "frontends" */}}

{{- /*------------------------------------*/}}

Would this be feasible @jcmoraisjr or do you suggest another way to attack our problem?

@jcmoraisjr
Copy link
Owner

Hi, can you describe a bit more about your need? How your ingresses resources are configured, how the request looks like.

The hostname in the SNI extension is not supposed to be used to route requests, but instead to choose a proper server certificate for the TLS handshake. Note also that it's a common practice for a http client (e.g. browsers), over H2 connections, to reuse the same TCP connection for requests with distinct hostnames, leading the request with distinct values between SNI and Host header, being the Host header the correct one to be used. The proposed configuration change might lead to wrong routing and maybe to a security issue depending on the ingress configurations.

@GuilhermeAbraham
Copy link

GuilhermeAbraham commented May 8, 2024

To give you more insight, the context for this issue is:

  1. The ingress is in front of a service that creates a communication/proxy between two other external services
  2. One of the external services will communicate to <random>-something.domain.com and authenticate using a session
  3. The other external service will communicate to something.domain.com and authenticate using mTLS

Since <random>-something.domain.com is not acceptable as a host, we matching for *.domain.com, we were under the assumption that the host that is more specific, something.domain.com, would have priority in the match over the host that uses regex/glob. And this would be true if both are on the maps/_front_https_host__* because HaProxy default behavior is to prioritize matches in this order: exact,prefix,begin,regex (can be overwritten on the path-type-order parameter).

But since one of the endpoints uses mTLS, it's mapping is being stored on the SNI mapping instead of the Host mapping.

Jesse suggestion is just the more simple way we could go to solve this, any solution from changing the order, making the order configurable or even allowing us to specify the regex and match type for the host (instead of only basing it on the host) would work for us.

Let me know if this gives you enough information or if you need any further details.
Thanks in advance for your attention.

@jcmoraisjr
Copy link
Owner

Hi thanks for the clarification. The detailed description of your use case led me to identify a missing piece in the frontend configuration that I think it should be there. Can you folks share the ingress resources you are using? Better if using fake, but semantically compatible with the real data. Those resources along with the provided descriptions should be enough for me to create an e2e test covering the expectations. Maybe we can fix this behavior for you in the code.

In the mean time I think you can safely overwrite the template file and change the order of the use_backend evaluation. The SNI validation I was worried about happens on another place of the template, so changing the order should be safe, except for some non expected behavior changed on other routing you might have in your cluster.

@GuilhermeAbraham
Copy link

GuilhermeAbraham commented May 16, 2024

Hey, sorry for the delayed response. Bellow the details you requested.

First let me just define the 2 external services:

  1. customer frontend (CF)
  2. destination resource (DR)

Ingress objects:
customer *.domain.com
destination something.domain.com

Ingress Yaml:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    ingress.kubernetes.io/auth-tls-secret: project/cfssl-chained-cas
    ingress.kubernetes.io/auth-tls-verify-client: "on"
    ingress.kubernetes.io/hsts: "true"
    kubernetes.io/ingress.class: haproxy-destination
  name: destination
  namespace: project
spec:
  rules:
  - host: something.domain.com
    http:
      paths:
      - backend:
          service:
            name: proxy-service
            port:
              number: 7171
        path: /
        pathType: Prefix
  tls:
  - hosts:
    - something.domain.com
    secretName: tls-secret
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    ingress.kubernetes.io/config-backend: |
      acl allowed_subdomains req.hdr(host) -m reg -i '.+(-|\.)something\.domain\.com$'
      http-request reject if !allowed_subdomains
      acl is_websocket hdr(Upgrade) -i WebSocket
      http-request set-var(req.domain_hash) req.hdr(host),lower,regsub('.something\.domain\.com$','')
      http-request set-path /v1/project/destination/%[var(req.domain_hash)]/http/443%[path] if !is_websocket
      http-request set-path /v1/project/destination/%[var(req.domain_hash)]/ws/443%[path] if is_websocket
    ingress.kubernetes.io/cors-allow-credentials: "true"
    ingress.kubernetes.io/cors-allow-headers: Content-Type, Accept, Origin, User-Agent,
      DNT, Cache-Control, X-Mx-ReqToken, Keep-Alive, X-Requested-With, If-Modified-Since,
      X-XSRF-Token, X-Request-Id
    ingress.kubernetes.io/cors-allow-origin: https://www.domain.com,https://domain.com
    ingress.kubernetes.io/cors-enable: "true"
    kubernetes.io/ingress.class: haproxy-customer
  name: customer
  namespace: project
spec:
  rules:
  - host: '*.domain.com'
    http:
      paths:
      - backend:
          service:
            name: proxy-service
            port:
              number: 7171
        path: /
        pathType: Prefix
  tls:
  - hosts:
    - '*.domain.com'
    secretName: tls-secret

Please let me know if you need anything else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants