Skip to content

Conversation

@ScheererJ
Copy link
Contributor

@ScheererJ ScheererJ commented Aug 29, 2025

What this PR does / why we need it:

Add support for PROXY protocol v2.

PROXY protocol can be useful to preserve the client IP address when a non-transparent load balancer is used. PROXY protocol v1 is human readable while v2 is binary.
github.com/armon/go-proxyproto supports only PROXY protocol v1. github.com/pires/go-proxyproto supports PROXY protocol v1 & v2. Some load balancers support PROXY protocol v1, e.g. AWS classic load balancer. Others only support PROXY protocol v2, e.g. AWS network load balancer.
If the previous implementation received PROXY protocl v2 it did not detect it and forwarded it as workload data. In conjunction with TLS this lead to a packet including PROXY protocol v2 AND the TLS client hello being forwarded to nginx causing issues (400 Bad Request). With the new library, it is possible to run ingress-nginx with ssl passthrough and PROXY protocol enabled in environments using PROXY protocol v2.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • CVE Report (Scanner found CVE and adding report)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation only

Which issue/s this PR fixes

fixes #10181

How Has This Been Tested?

  1. I created a dual-stack Kubernetes cluster on AWS (using https://github.com/gardener/gardener to manage to cluster).
  2. ingress-nginx was deployed into the cluster with use-proxy-protocol: true set in the config map and --enable-ssl-passthrough=true being added as command line option for the controller.
  3. I created a dual-stack AWS network load balancer using AWS load balancer controller with PROXY protocol enabled. (AWS dual-stack network load balancer can translate incoming IPv6 connections to IPv4 connections to the backends. Hence, PROXY protocol is useful to preserve the client IP address.)
  4. To verify the connectivity, I created an Ingress resource with a corresponding TLS certificate and a corresponding DNS entry:
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: nginx
spec:
  rules:
  - host: nginx.<my-fancy-domain>
    http:
      paths:
      - pathType: Prefix
        path: "/"
        backend:
          service:
            name: nginx
            port:
              number: 80
  tls:
    - hosts:
        - nginx.<my-fancy-domain>
      secretName: tls-secret
  1. Check connectivity to the service via nginx.<my-fancy-domain> via curl and a web browser (only curl output shown below).

Before the change:

% curl -v https://nginx.<my-fancy-domain>
* Host nginx.<my-fancy-domain>:443 was resolved.
* IPv6: 2a05:d018:....:....:....:....:....:....
* IPv4: 52.210......
*   Trying [2a05:d018:....:....:....:....:....:....]:443...
* Connected to nginx.<my-fancy-domain> (2a05:d018:....:....:....:....:....:....) port 443
* ALPN: curl offers h2,http/1.1
* (304) (OUT), TLS handshake, Client hello (1):
*  CAfile: /etc/ssl/cert.pem
*  CApath: none
* LibreSSL/3.3.6: error:1404B42E:SSL routines:ST_CONNECT:tlsv1 alert protocol version
* Closing connection
curl: (35) LibreSSL/3.3.6: error:1404B42E:SSL routines:ST_CONNECT:tlsv1 alert protocol version

After the change:

% curl -v https://nginx.<my-fancy-domain>
* Host nginx.<my-fancy-domain>:443 was resolved.
* IPv6: 2a05:d018:....:....:....:....:....:....
* IPv4: 52.210......
*   Trying [2a05:d018:....:....:....:....:....:....]:443...
* Connected to nginx.<my-fancy-domain> (2a05:d018:....:....:....:....:....:....) port 443
* ALPN: curl offers h2,http/1.1
* (304) (OUT), TLS handshake, Client hello (1):
*  CAfile: /etc/ssl/cert.pem
*  CApath: none
* (304) (IN), TLS handshake, Server hello (2):
* (304) (IN), TLS handshake, Unknown (8):
* (304) (IN), TLS handshake, Certificate (11):
* (304) (IN), TLS handshake, CERT verify (15):
* (304) (IN), TLS handshake, Finished (20):
* (304) (OUT), TLS handshake, Finished (20):
* SSL connection using TLSv1.3 / AEAD-AES256-GCM-SHA384 / [blank] / UNDEF
* ALPN: server accepted h2
* Server certificate:
*  subject: CN=nginx.<my-fancy-domain>
*  start date: Aug 29 06:38:21 2025 GMT
*  expire date: Nov 27 06:38:20 2025 GMT
*  subjectAltName: host "nginx.<my-fancy-domain>" matched cert's "nginx.<my-fancy-domain>"
*  issuer: C=US; O=Let's Encrypt; CN=R13
*  SSL certificate verify ok.
* using HTTP/2
* [HTTP/2] [1] OPENED stream for https://nginx.<my-fancy-domain>/
* [HTTP/2] [1] [:method: GET]
* [HTTP/2] [1] [:scheme: https]
* [HTTP/2] [1] [:authority: nginx.<my-fancy-domain>]
* [HTTP/2] [1] [:path: /]
* [HTTP/2] [1] [user-agent: curl/8.7.1]
* [HTTP/2] [1] [accept: */*]
> GET / HTTP/2
> Host: nginx.<my-fancy-domain>
> User-Agent: curl/8.7.1
> Accept: */*
>
* Request completely sent off
< HTTP/2 200
< date: Fri, 29 Aug 2025 07:58:54 GMT
< content-type: text/html
< content-length: 615
< last-modified: Wed, 13 Aug 2025 14:33:41 GMT
< etag: "689ca245-267"
< accept-ranges: bytes
< strict-transport-security: max-age=31536000; includeSubDomains
<
<!DOCTYPE html>
<html>
<head>
<title>Welcome to nginx!</title>
<style>
html { color-scheme: light dark; }
body { width: 35em; margin: 0 auto;
font-family: Tahoma, Verdana, Arial, sans-serif; }
</style>
</head>
<body>
<h1>Welcome to nginx!</h1>
<p>If you see this page, the nginx web server is successfully installed and
working. Further configuration is required.</p>

<p>For online documentation and support please refer to
<a href="http://nginx.org/">nginx.org</a>.<br/>
Commercial support is available at
<a href="http://nginx.com/">nginx.com</a>.</p>

<p><em>Thank you for using nginx.</em></p>
</body>
</html>
* Connection #0 to host nginx.<my-fancy-domain> left intact

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have added unit and/or e2e tests to cover my changes.
  • All new and existing tests passed.

I did not find a hint that only PROXY protocol v1 was supported. Hence, I guess there is no need to document v2 support. Feel free to comment if you have a different point of view.

@netlify
Copy link

netlify bot commented Aug 29, 2025

Deploy Preview for kubernetes-ingress-nginx ready!

Name Link
🔨 Latest commit 2f232d6
🔍 Latest deploy log https://app.netlify.com/projects/kubernetes-ingress-nginx/deploys/68b9ff9bbe4b510008a48803
😎 Deploy Preview https://deploy-preview-13861--kubernetes-ingress-nginx.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 29, 2025
@k8s-ci-robot
Copy link
Contributor

Welcome @ScheererJ!

It looks like this is your first PR to kubernetes/ingress-nginx 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/ingress-nginx has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 29, 2025
@k8s-ci-robot k8s-ci-robot requested review from Gacko and strongjz August 29, 2025 09:24
@k8s-ci-robot
Copy link
Contributor

Hi @ScheererJ. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 29, 2025
@strongjz strongjz removed their request for review September 4, 2025 18:49
@Gacko Gacko force-pushed the enhancement/support-for-proxy-protocol-v2 branch from 7598339 to 2f232d6 Compare September 4, 2025 21:07
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 4, 2025
@Gacko Gacko changed the title Add support for PROXY protocol v2. SSL Proxy: Support PROXY protocol v2. Sep 4, 2025
Copy link
Member

@Gacko Gacko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/triage accepted
/kind feature
/priority backlog
/lgtm
/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. triage/accepted Indicates an issue or PR is ready to be actively worked on. kind/feature Categorizes issue or PR as related to a new feature. priority/backlog Higher priority than priority/awaiting-more-evidence. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Sep 4, 2025
@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-priority labels Sep 4, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Gacko, ScheererJ

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 4, 2025
@k8s-ci-robot k8s-ci-robot merged commit 4a04d61 into kubernetes:main Sep 4, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. priority/backlog Higher priority than priority/awaiting-more-evidence. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When ssl-passthrough is enabled at the same time as proxy-protocol, proxy-proto doesn't work.

3 participants