-
Notifications
You must be signed in to change notification settings - Fork 688
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
Clarified how XFCC headers are handled #6586
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6586 +/- ##
=======================================
Coverage 81.76% 81.76%
=======================================
Files 133 133
Lines 15942 15942
=======================================
Hits 13035 13035
Misses 2614 2614
Partials 293 293 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, the change looks good to me!
Could you add DCO / signed-off-by line to the commit https://github.com/projectcontour/contour/blob/main/CONTRIBUTING.md#commit-message-and-pr-guidelines
Since XFCC headers contain authentication information, it's important to know precisely how Contour (ie Envoy) handle existing XFCC headers from clients - ie, are they blocked, or appended to, and in what circumstances are they blocked? Getting this wrong could allow serious vulnerabilities such as spoofing client certs. This documents Contours behaviour, so that users can know exactly how they are required to handle that header without needing to dive into the Contour source code. My understanding from reading the source code: https://github.com/gautierdelorme/contour/blob/main/internal/envoy/v3/listener.go#L483 as well as the Envoy documentation: https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto#envoy-v3-api-enum-extensions-filters-network-http-connection-manager-v3-httpconnectionmanager-forwardclientcertdetails is that when forwarding client certificate details is not configured in Contour, Contour leaves `ForwardClientCertDetails` in Envoy unset, which means it defaults to `SANITIZE`, which means incoming headers from clients are blocked. Meanwhile, when forwarding client certificate details is configured in Contour, Contour sets `ForwardClientCertDetails` to `SANITIZE_SET` in Envoy, which means incoming XFCC headers are blocked, and if an incoming cert is present, a new XFCC header is added. Signed-off-by: James Roper <[email protected]>
9c409db
to
bcbf1b7
Compare
Done. |
Thank you @jroper! |
* add changelog Signed-off-by: gang.liu <[email protected]> * build(deps): bump actions/upload-artifact in the artifact-actions group (projectcontour#6608) Bumps the artifact-actions group with 1 update: [actions/upload-artifact](https://github.com/actions/upload-artifact). Updates `actions/upload-artifact` from 4.3.5 to 4.3.6 - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@89ef406...834a144) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-patch dependency-group: artifact-actions ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github/codeql-action from 3.25.15 to 3.26.0 (projectcontour#6609) Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.25.15 to 3.26.0. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@afb54ba...eb055d7) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github.com/onsi/ginkgo/v2 from 2.19.1 to 2.20.0 (projectcontour#6607) Bumps [github.com/onsi/ginkgo/v2](https://github.com/onsi/ginkgo) from 2.19.1 to 2.20.0. - [Release notes](https://github.com/onsi/ginkgo/releases) - [Changelog](https://github.com/onsi/ginkgo/blob/master/CHANGELOG.md) - [Commits](onsi/ginkgo@v2.19.1...v2.20.0) --- updated-dependencies: - dependency-name: github.com/onsi/ginkgo/v2 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * docs: Clarify how XFCC headers are handled (projectcontour#6586) Since XFCC headers contain authentication information, it's important to know precisely how Contour (ie Envoy) handle existing XFCC headers from clients - ie, are they blocked, or appended to, and in what circumstances are they blocked? Getting this wrong could allow serious vulnerabilities such as spoofing client certs. This documents Contours behaviour, so that users can know exactly how they are required to handle that header without needing to dive into the Contour source code. My understanding from reading the source code: https://github.com/gautierdelorme/contour/blob/main/internal/envoy/v3/listener.go#L483 as well as the Envoy documentation: https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto#envoy-v3-api-enum-extensions-filters-network-http-connection-manager-v3-httpconnectionmanager-forwardclientcertdetails is that when forwarding client certificate details is not configured in Contour, Contour leaves `ForwardClientCertDetails` in Envoy unset, which means it defaults to `SANITIZE`, which means incoming headers from clients are blocked. Meanwhile, when forwarding client certificate details is configured in Contour, Contour sets `ForwardClientCertDetails` to `SANITIZE_SET` in Envoy, which means incoming XFCC headers are blocked, and if an incoming cert is present, a new XFCC header is added. Signed-off-by: James Roper <[email protected]> * build(deps): bump dario.cat/mergo from 1.0.0 to 1.0.1 (projectcontour#6627) Bumps [dario.cat/mergo](https://github.com/imdario/mergo) from 1.0.0 to 1.0.1. - [Release notes](https://github.com/imdario/mergo/releases) - [Commits](darccio/mergo@v1.0.0...v1.0.1) --- updated-dependencies: - dependency-name: dario.cat/mergo dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github/codeql-action from 3.26.0 to 3.26.2 (projectcontour#6622) Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.26.0 to 3.26.2. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@eb055d7...429e197) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github.com/prometheus/client_golang (projectcontour#6626) Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.19.1 to 1.20.0. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.19.1...v1.20.0) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github.com/envoyproxy/go-control-plane (projectcontour#6625) Bumps [github.com/envoyproxy/go-control-plane](https://github.com/envoyproxy/go-control-plane) from 0.12.1-0.20240111020705-5401a878d8bb to 0.13.0. - [Release notes](https://github.com/envoyproxy/go-control-plane/releases) - [Changelog](https://github.com/envoyproxy/go-control-plane/blob/main/CHANGELOG.md) - [Commits](https://github.com/envoyproxy/go-control-plane/commits/v0.13.0) --- updated-dependencies: - dependency-name: github.com/envoyproxy/go-control-plane dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * docs: Update README.md to be more helpful (projectcontour#6585) The docs/README.md made no sense. Anyone reading it in GitHub clearly wants to contribute to the documentation, that's why they're in the source code of Contour, why else would they have found their way to the source repository? So, it should point to where the documentation lives in the git repository, not to the website where it's served. Signed-off-by: James Roper <[email protected]> Co-authored-by: Steve Kriss <[email protected]> * [api-gateway]: Support http(s) as AppProtocol in Kubernetes svc (projectcontour#6616) * [api-gateway]: Support http, https and www-http as AppProtocol in kubernetes' service Fix projectcontour#6560 Signed-off-by: Ludovic Logiou <[email protected]> * Remove legacy www-http Signed-off-by: Ludovic Logiou <[email protected]> * Fix undefined vars Signed-off-by: Ludovic Logiou <[email protected]> * Add changelog Signed-off-by: Ludovic Logiou <[email protected]> * Fix issues found by the linter Signed-off-by: Ludovic Logiou <[email protected]> * Fix format and add unit tests Signed-off-by: Ludovic Logiou <[email protected]> --------- Signed-off-by: Ludovic Logiou <[email protected]> * build(deps): bump codespell-project/actions-codespell from 2.0 to 2.1 (projectcontour#6635) Bumps [codespell-project/actions-codespell](https://github.com/codespell-project/actions-codespell) from 2.0 to 2.1. - [Release notes](https://github.com/codespell-project/actions-codespell/releases) - [Commits](codespell-project/actions-codespell@94259cd...406322e) --- updated-dependencies: - dependency-name: codespell-project/actions-codespell dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github.com/prometheus/client_golang (projectcontour#6640) Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.20.0 to 1.20.2. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.20.0...v1.20.2) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github.com/onsi/ginkgo/v2 from 2.20.0 to 2.20.1 (projectcontour#6639) Bumps [github.com/onsi/ginkgo/v2](https://github.com/onsi/ginkgo) from 2.20.0 to 2.20.1. - [Release notes](https://github.com/onsi/ginkgo/releases) - [Changelog](https://github.com/onsi/ginkgo/blob/master/CHANGELOG.md) - [Commits](onsi/ginkgo@v2.20.0...v2.20.1) --- updated-dependencies: - dependency-name: github.com/onsi/ginkgo/v2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github.com/vektra/mockery/v2 from 2.44.1 to 2.45.0 (projectcontour#6638) Bumps [github.com/vektra/mockery/v2](https://github.com/vektra/mockery) from 2.44.1 to 2.45.0. - [Release notes](https://github.com/vektra/mockery/releases) - [Changelog](https://github.com/vektra/mockery/blob/master/docs/changelog.md) - [Commits](vektra/mockery@v2.44.1...v2.45.0) --- updated-dependencies: - dependency-name: github.com/vektra/mockery/v2 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump kind and kubectl tools (projectcontour#6642) kind: 0.24.0 kubectl: 1.31.0 Signed-off-by: Sunjay Bhatia <[email protected]> * fix lb address Signed-off-by: gang.liu <[email protected]> * fix ut Signed-off-by: gang.liu <[email protected]> * revert wrong file Signed-off-by: gang.liu <[email protected]> --------- Signed-off-by: gang.liu <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: James Roper <[email protected]> Signed-off-by: Ludovic Logiou <[email protected]> Signed-off-by: Sunjay Bhatia <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: James Roper <[email protected]> Co-authored-by: Steve Kriss <[email protected]> Co-authored-by: Ludovic Logiou <[email protected]> Co-authored-by: Sunjay Bhatia <[email protected]>
Since XFCC headers contain authentication information, it's important to know precisely how Contour (ie Envoy) handle existing XFCC headers from clients - ie, are they blocked, or appended to, and in what circumstances are they blocked? Getting this wrong could allow serious vulnerabilities such as spoofing client certs. This documents Contours behaviour, so that users can know exactly how they are required to handle that header without needing to dive into the Contour source code. My understanding from reading the source code: https://github.com/gautierdelorme/contour/blob/main/internal/envoy/v3/listener.go#L483 as well as the Envoy documentation: https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto#envoy-v3-api-enum-extensions-filters-network-http-connection-manager-v3-httpconnectionmanager-forwardclientcertdetails is that when forwarding client certificate details is not configured in Contour, Contour leaves `ForwardClientCertDetails` in Envoy unset, which means it defaults to `SANITIZE`, which means incoming headers from clients are blocked. Meanwhile, when forwarding client certificate details is configured in Contour, Contour sets `ForwardClientCertDetails` to `SANITIZE_SET` in Envoy, which means incoming XFCC headers are blocked, and if an incoming cert is present, a new XFCC header is added. Signed-off-by: James Roper <[email protected]> Signed-off-by: Saman Mahdanian <[email protected]>
Since XFCC headers contain authentication information, it's important for users to know precisely how Contour (ie Envoy) handles existing XFCC headers from clients - ie, are they blocked, or appended to, and if blocked, in what circumstances are they blocked? Getting this wrong could allow serious vulnerabilities such as spoofing TLS client authentication via the XFCC header.
This documents Contours precise behaviour, so that users can know exactly how they are required to handle that header without needing to dive into the Contour source code.
My understanding from reading the Contour source code, as well as the Envoy documentation, is that when forwarding client certificate details is not configured in Contour, Contour leaves
ForwardClientCertDetails
in Envoy unset, which means it defaults toSANITIZE
, which means incoming headers from clients are blocked. Meanwhile, when forwarding client certificate details is configured in Contour, Contour setsForwardClientCertDetails
toSANITIZE_SET
in Envoy, which means incoming XFCC headers are blocked, and if an incoming cert is present, a new XFCC header is added.