Clarified how XFCC headers are handled #6586
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
tsaarni
requested changes
Aug 1, 2024
There was a problem hiding this comment.
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 <james@jazzy.id.au>
9c409db to
bcbf1b7
Compare
|
Done. |
tsaarni
approved these changes
Aug 14, 2024
|
Thank you @jroper! |
izturn
added a commit
to projectsesame/contour
that referenced
this pull request
Aug 27, 2024
* add changelog Signed-off-by: gang.liu <gang.liu@daocloud.io> * 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] <support@github.com> 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] <support@github.com> 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] <support@github.com> 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 <james@jazzy.id.au> * 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] <support@github.com> 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] <support@github.com> 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] <support@github.com> 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] <support@github.com> 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 <james@jazzy.id.au> Co-authored-by: Steve Kriss <stephen.kriss@gmail.com> * [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 <ludovic.logiou@gmail.com> * Remove legacy www-http Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * Fix undefined vars Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * Add changelog Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * Fix issues found by the linter Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * Fix format and add unit tests Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> --------- Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * 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] <support@github.com> 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] <support@github.com> 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] <support@github.com> 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] <support@github.com> 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 <sunjayb@vmware.com> * fix lb address Signed-off-by: gang.liu <gang.liu@daocloud.io> * fix ut Signed-off-by: gang.liu <gang.liu@daocloud.io> * revert wrong file Signed-off-by: gang.liu <gang.liu@daocloud.io> --------- Signed-off-by: gang.liu <gang.liu@daocloud.io> Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: James Roper <james@jazzy.id.au> Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: James Roper <james@jazzy.id.au> Co-authored-by: Steve Kriss <stephen.kriss@gmail.com> Co-authored-by: Ludovic Logiou <ludovic.logiou@gmail.com> Co-authored-by: Sunjay Bhatia <5337253+sunjayBhatia@users.noreply.github.com>
SamMHD
pushed a commit
to SamMHD/contour
that referenced
this pull request
Sep 8, 2024
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 <james@jazzy.id.au> Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
ForwardClientCertDetailsin 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 setsForwardClientCertDetailstoSANITIZE_SETin Envoy, which means incoming XFCC headers are blocked, and if an incoming cert is present, a new XFCC header is added.