Skip to content

fix: Validation of XListenerSet certificateRefs#8168

Merged
zirain merged 1 commit intoenvoyproxy:mainfrom
krishicks:main
Feb 5, 2026
Merged

fix: Validation of XListenerSet certificateRefs#8168
zirain merged 1 commit intoenvoyproxy:mainfrom
krishicks:main

Conversation

@krishicks
Copy link
Copy Markdown
Contributor

@krishicks krishicks commented Feb 3, 2026

What this PR does / why we need it:

Previously, validateTerminateModeAndGetTLSSecrets would always use the namespace of the listener's gateway when verifying a cross-namespace ref.

This meant that if the listener were from an XListenerSet, whether or not the Secret associated with the certificateRef was in the same namespace as the XListenerSet, it would not be permitted.

Additionally, and relatedly, this fixes an issue where an XListenerSet could reference a Secret in the gateway's namespace without a ReferenceGrant being present.

With this change we add a new GetNamespace() method to gatewayapi.ListenerContext which returns the listener's gateway's namespace for a listener added directly to the gateway, or the XListenerSet's namespace otherwise. This is similar to some of the other methods that were added to ListenerContext in support of XListenerSets.

The new method is used when creating the crossNamespaceFrom to determine if the certificateRef is permitted. If the Secret and XListenerSet are in the same namespace, it is permitted. If that is not the case a ReferenceGrant from the XListenerSet to the Secret will be properly searched for.

Release Notes: Yes

@krishicks krishicks requested a review from a team as a code owner February 3, 2026 17:53
@netlify
Copy link
Copy Markdown

netlify bot commented Feb 3, 2026

Deploy Preview for cerulean-figolla-1f9435 canceled.

Name Link
🔨 Latest commit 8190fbb
🔍 Latest deploy log https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/698394c4bf70c5000867e340

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 3, 2026

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.79%. Comparing base (bbd7cde) to head (8190fbb).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/gatewayapi/validate.go 92.85% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8168      +/-   ##
==========================================
+ Coverage   73.68%   73.79%   +0.10%     
==========================================
  Files         241      241              
  Lines       36569    36579      +10     
==========================================
+ Hits        26946    26992      +46     
+ Misses       7713     7681      -32     
+ Partials     1910     1906       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@arkodg arkodg added this to the v1.7.0 Release milestone Feb 3, 2026
@arkodg arkodg requested a review from zhaohuabing February 3, 2026 18:38
@krishicks
Copy link
Copy Markdown
Contributor Author

krishicks commented Feb 3, 2026

It's possible this solution (ed: the solution was updated based on this comment) isn't the right one given ReferenceGrant Semantics, which seems to imply the ReferenceGrant is not necessary when the ListenerSet references a secret in its own namespace:

ReferenceGrant Semantics: - ReferenceGrants applied to a Gateway are not inherited by child ListenerSets - ReferenceGrants applied to a ListenerSet do not grant permission to the parent Gateway's listeners - A ListenerSet can reference secrets/backends in its own namespace without a ReferenceGrant

If that's the case, I think the appropriate fix would be to skip the processSecretRef call entirely iff the secret namespace matches the XListenerSet namespace. However, it is possible for an XListenerSet to refer to a Secret in a different namespace than itself, in which the original code would possibly be valid. I say possibly because even now when I do add the ReferenceGrant for the XListenerSet as the code prior to this PR expects, the Gateway fails to resolve the secret refs as it expects a ReferenceGrant from it (meaning the Gateway) to the Secret.

@krishicks
Copy link
Copy Markdown
Contributor Author

krishicks commented Feb 3, 2026

There was also a similar issue in kgateway. That was fixed by using the listener's parent's namespace rather than the gateway's namespace.

I think the same fix would apply here given this line in internal/gatewayapi/validate.go:

https://github.com/envoyproxy/gateway/blob/main/internal/gatewayapi/validate.go#L399

if certificateRef.Namespace != nil && string(*certificateRef.Namespace) != "" && string(*certificateRef.Namespace) != listener.gateway.Namespace {

If the listener is an XListenerSet it should use the XListenerSet's namespace, not the gateway's namespace. That would allow an XListenerSet to refer to a secret in the same namespace as it.

I think updating this code would also fix another issue I discovered, which is that an XListenerSet in one namespace can presently reference a secret that exists the Gateway's different namespace without a ReferenceGrant present, but it should not. This is because the listener's Gateway's namespace is used in the comparison instead of the namespace of the XListenerSet.

@krishicks krishicks marked this pull request as draft February 4, 2026 00:57
@krishicks krishicks changed the title fix: Discovery of ReferenceGrants for XListenerSets fix: Validation of XListenerSet certificateRefs Feb 4, 2026
@krishicks krishicks force-pushed the main branch 4 times, most recently from 7a1ab25 to 26e5a76 Compare February 4, 2026 02:16
@krishicks
Copy link
Copy Markdown
Contributor Author

I updated the PR based on my comments above, and am marking it as ready for review.

@krishicks krishicks marked this pull request as ready for review February 4, 2026 02:17
@krishicks krishicks force-pushed the main branch 2 times, most recently from b3473e8 to 877a659 Compare February 4, 2026 02:26
@zhaohuabing
Copy link
Copy Markdown
Member

zhaohuabing commented Feb 4, 2026

@krishicks Thanks for taking care of this. This PR looks good!

Could we also move the secrets to another namespace in these two tests and add ReferenceGrants to cover the cross-ns secret references use case?

https://github.com/envoyproxy/gateway/blob/main/internal%2Fgatewayapi%2Ftestdata%2Fxlistenerset-tlsroute.in.yaml

https://github.com/envoyproxy/gateway/blob/main/test%2Fe2e%2Ftestdata%2Fxlistenerset-tls.yaml

@krishicks
Copy link
Copy Markdown
Contributor Author

@krishicks Thanks for taking care of this. This PR looks good!

Could we also move the secrets to another namespace in these two tests and add ReferenceGrants to cover the cross-ns secret references use case?

https://github.com/envoyproxy/gateway/blob/main/internal%2Fgatewayapi%2Ftestdata%2Fxlistenerset-tlsroute.in.yaml

https://github.com/envoyproxy/gateway/blob/main/test%2Fe2e%2Ftestdata%2Fxlistenerset-tls.yaml

Yes, of course!

I'm assuming you meant e2e/testdata/xlistenerset-base.yaml, as e2e/testdata/xlistenerset-tls.yaml doesn't use TLS termination (it uses passthrough).

I pushed new changes. Please take a look at the e2e test for me; I think it's right but I'm not able to run it locally.

@zirain
Copy link
Copy Markdown
Member

zirain commented Feb 4, 2026

@krishicks can you make CI happy?

@krishicks
Copy link
Copy Markdown
Contributor Author

I understand the e2e failures and am working on an update. I also see how to run them locally, so I'll make sure they're green as well.

@kkk777-7
Copy link
Copy Markdown
Member

kkk777-7 commented Feb 4, 2026

@krishicks thanks for working this, your code looks good to me!

e2e failure root cause is below

=== RUN   TestE2E/XListenerSetHTTPS
    conformance.go:70: 2026-02-04T15:22:39.701304814Z: Applying testdata/xlistenerset-base.yaml
    apply.go:275: 2026-02-04T15:22:39.718643104Z: Creating xlistener-gateway Gateway
    apply.go:294: 2026-02-04T15:22:39.738690269Z: Updating xlistener-base Namespace
    apply.go:275: 2026-02-04T15:22:39.750423435Z: Creating xlistener-https-certificate Secret
    apply.go:277: 
        	Error Trace:	/home/runner/go/pkg/mod/sigs.k8s.io/gateway-api@v1.4.1/conformance/utils/kubernetes/apply.go:277
        	            				/home/runner/go/pkg/mod/sigs.k8s.io/gateway-api@v1.4.1/conformance/utils/suite/conformance.go:71
        	            				/home/runner/go/pkg/mod/sigs.k8s.io/gateway-api@v1.4.1/conformance/utils/suite/suite.go:487
        	Error:      	Received unexpected error:
        	            	secrets "xlistener-https-certificate" is forbidden: unable to create new content in namespace xlistener-base because it is being terminated
        	Test:       	TestE2E/XListenerSetHTTPS
        	Messages:   	error creating resource

During cleanup for each XListenerSet test, we delete xlistenerset-base.yaml, which ends up deleting namespace as well. The problem seems to be that the next test creates Secret in the same namespace before it has been completely removed.

so, we should move namespace and secret and XListenerSet (for HTTPS) into xlistenerset-https.yaml, then may resolve e2e failure.

@krishicks
Copy link
Copy Markdown
Contributor Author

Thanks @kkk777-7 , that was my understanding of the failure. I chose to rename the existing XListenerSetTLS test to XListenerSetTLSPassthrough and add a new one, XListenerSetTLSTermination, which has its own Secret/ReferenceGrant/Namespace.

I was able to run both of those e2e tests locally and see them pass, but the whole suite failed when I ran it in some other, non-XListenerSet area.

Previously, validateTerminateModeAndGetTLSSecrets would always use the
namespace of the listener's gateway when verifying a cross-namespace
ref.

This meant that if the listener were from an XListenerSet, whether or
not the Secret associated with the certificateRef was in the same
namespace as the XListenerSet, it would not be permitted.

Additionally, and relatedly, this fixes an issue where an XListenerSet
could reference a Secret in the gateway's namespace without a
ReferenceGrant being present.

With this change we add a new GetNamespace() method to
gatewayapi.ListenerContext which returns the listener's gateway's
namespace for a listener added directly to the gateway, or the
XListenerSet's namespace otherwise. This is similar to some of the other
methods that were added to ListenerContext in support of XListenerSets.

The new method is used when creating the `crossNamespaceFrom` to
determine if the certificateRef is permitted. If the Secret and
XListenerSet are in the same namespace, it is permitted. If that is not
the case a ReferenceGrant from the XListenerSet to the Secret will be
properly searched for.

Signed-off-by: Kris Hicks <khicks@nvidia.com>
@krishicks
Copy link
Copy Markdown
Contributor Author

This is ready for another round through CI.

@krishicks
Copy link
Copy Markdown
Contributor Author

The test failures all seem unrelated to these changes. 🤔

@zirain
Copy link
Copy Markdown
Member

zirain commented Feb 4, 2026

/retest

@kkk777-7
Copy link
Copy Markdown
Member

kkk777-7 commented Feb 5, 2026

LGTM, thanks!

@zirain zirain merged commit d85a78e into envoyproxy:main Feb 5, 2026
55 of 59 checks passed
cnvergence pushed a commit to cnvergence/gateway that referenced this pull request Feb 5, 2026
Previously, validateTerminateModeAndGetTLSSecrets would always use the
namespace of the listener's gateway when verifying a cross-namespace
ref.

This meant that if the listener were from an XListenerSet, whether or
not the Secret associated with the certificateRef was in the same
namespace as the XListenerSet, it would not be permitted.

Additionally, and relatedly, this fixes an issue where an XListenerSet
could reference a Secret in the gateway's namespace without a
ReferenceGrant being present.

With this change we add a new GetNamespace() method to
gatewayapi.ListenerContext which returns the listener's gateway's
namespace for a listener added directly to the gateway, or the
XListenerSet's namespace otherwise. This is similar to some of the other
methods that were added to ListenerContext in support of XListenerSets.

The new method is used when creating the `crossNamespaceFrom` to
determine if the certificateRef is permitted. If the Secret and
XListenerSet are in the same namespace, it is permitted. If that is not
the case a ReferenceGrant from the XListenerSet to the Secret will be
properly searched for.

Signed-off-by: Kris Hicks <khicks@nvidia.com>
cnvergence pushed a commit to cnvergence/gateway that referenced this pull request Feb 5, 2026
Previously, validateTerminateModeAndGetTLSSecrets would always use the
namespace of the listener's gateway when verifying a cross-namespace
ref.

This meant that if the listener were from an XListenerSet, whether or
not the Secret associated with the certificateRef was in the same
namespace as the XListenerSet, it would not be permitted.

Additionally, and relatedly, this fixes an issue where an XListenerSet
could reference a Secret in the gateway's namespace without a
ReferenceGrant being present.

With this change we add a new GetNamespace() method to
gatewayapi.ListenerContext which returns the listener's gateway's
namespace for a listener added directly to the gateway, or the
XListenerSet's namespace otherwise. This is similar to some of the other
methods that were added to ListenerContext in support of XListenerSets.

The new method is used when creating the `crossNamespaceFrom` to
determine if the certificateRef is permitted. If the Secret and
XListenerSet are in the same namespace, it is permitted. If that is not
the case a ReferenceGrant from the XListenerSet to the Secret will be
properly searched for.

Signed-off-by: krishicks <kris@krishicks.com>
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>
cnvergence pushed a commit to cnvergence/gateway that referenced this pull request Feb 5, 2026
Previously, validateTerminateModeAndGetTLSSecrets would always use the
namespace of the listener's gateway when verifying a cross-namespace
ref.

This meant that if the listener were from an XListenerSet, whether or
not the Secret associated with the certificateRef was in the same
namespace as the XListenerSet, it would not be permitted.

Additionally, and relatedly, this fixes an issue where an XListenerSet
could reference a Secret in the gateway's namespace without a
ReferenceGrant being present.

With this change we add a new GetNamespace() method to
gatewayapi.ListenerContext which returns the listener's gateway's
namespace for a listener added directly to the gateway, or the
XListenerSet's namespace otherwise. This is similar to some of the other
methods that were added to ListenerContext in support of XListenerSets.

The new method is used when creating the `crossNamespaceFrom` to
determine if the certificateRef is permitted. If the Secret and
XListenerSet are in the same namespace, it is permitted. If that is not
the case a ReferenceGrant from the XListenerSet to the Secret will be
properly searched for.

Signed-off-by: krishicks <kris@krishicks.com>
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>
cnvergence added a commit that referenced this pull request Feb 5, 2026
* chore(docs): Update Azure Entra link in OIDC guide (#8167)

Update Azure Entra link in OIDC guide

Signed-off-by: Guy Daich <guy.daich@sap.com>

* fix: continue processing the remaining xDS with invalid EnvoyPatchPolicies (#8153)

continue processing the remaining xDS with invalid EnvoyPatchPolicies

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>

* build(deps): bump the actions group across 1 directory with 2 updates (#8178)

Bumps the actions group with 2 updates in the / directory: [docker/login-action](https://github.com/docker/login-action) and [github/codeql-action](https://github.com/github/codeql-action).

Updates `docker/login-action` from 3.6.0 to 3.7.0
- [Release notes](https://github.com/docker/login-action/releases)
- [Commits](docker/login-action@5e57cd1...c94ce9f)

Updates `github/codeql-action` from 4.32.0 to 4.32.1
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@b20883b...6bc82e0)

---
updated-dependencies:
- dependency-name: docker/login-action
  dependency-version: 3.7.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: actions
- dependency-name: github/codeql-action
  dependency-version: 4.32.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: actions
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Isaac Wilson <10012479+jukie@users.noreply.github.com>
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>

* fix: skip provision when IR Infra is invalid (#7754)

* fix: do not trigger IR deletion when EnvoyProxy is invalid

Signed-off-by: zirain <zirain2009@gmail.com>

* add Invalid to ir.Infra

Signed-off-by: zirain <zirain2009@gmail.com>

* fix gen

Signed-off-by: zirain <zirain2009@gmail.com>

* add e2e

Signed-off-by: zirain <zirain2009@gmail.com>

* remove invalid

Signed-off-by: zirain <zirain2009@gmail.com>

* add comments

Signed-off-by: zirain <zirain2009@gmail.com>

* update

Signed-off-by: zirain <zirain2009@gmail.com>

* merge loop

Signed-off-by: zirain <zirain2009@gmail.com>

* move back

Signed-off-by: zirain <zirain2009@gmail.com>

---------

Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>

* docs: add HTTP header and method based authentication task (#7990)

* docs: add HTTP header and method based authentication task

Signed-off-by: Aditya Sanskar Srivastav <161202916+Aditya7880900936@users.noreply.github.com>

* docs: replace api-key examples with user header

Signed-off-by: Aditya Sanskar Srivastav <161202916+Aditya7880900936@users.noreply.github.com>

* docs: format header and method authentication examples

Signed-off-by: Aditya Sanskar Srivastav <161202916+Aditya7880900936@users.noreply.github.com>

* docs: add header and method based authorization examples

Signed-off-by: Aditya Sanskar Srivastav <161202916+Aditya7880900936@users.noreply.github.com>

---------

Signed-off-by: Aditya Sanskar Srivastav <161202916+Aditya7880900936@users.noreply.github.com>
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>

* fix: Validation of XListenerSet certificateRefs (#8168)

Previously, validateTerminateModeAndGetTLSSecrets would always use the
namespace of the listener's gateway when verifying a cross-namespace
ref.

This meant that if the listener were from an XListenerSet, whether or
not the Secret associated with the certificateRef was in the same
namespace as the XListenerSet, it would not be permitted.

Additionally, and relatedly, this fixes an issue where an XListenerSet
could reference a Secret in the gateway's namespace without a
ReferenceGrant being present.

With this change we add a new GetNamespace() method to
gatewayapi.ListenerContext which returns the listener's gateway's
namespace for a listener added directly to the gateway, or the
XListenerSet's namespace otherwise. This is similar to some of the other
methods that were added to ListenerContext in support of XListenerSets.

The new method is used when creating the `crossNamespaceFrom` to
determine if the certificateRef is permitted. If the Secret and
XListenerSet are in the same namespace, it is permitted. If that is not
the case a ReferenceGrant from the XListenerSet to the Secret will be
properly searched for.

Signed-off-by: krishicks <kris@krishicks.com>
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>

* fix: Remove whitespace for nodeSelector in deployment YAML - helm chart change (#8185)

Remove whitespace for nodeSelector in deployment YAML

Signed-off-by: Jess Belliveau <jess.belliveau@gmail.com>
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>

* [release/v1.7.0] release notes (#8188)

Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>

---------

Signed-off-by: Guy Daich <guy.daich@sap.com>
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: Aditya Sanskar Srivastav <161202916+Aditya7880900936@users.noreply.github.com>
Signed-off-by: krishicks <kris@krishicks.com>
Signed-off-by: Jess Belliveau <jess.belliveau@gmail.com>
Co-authored-by: Guy Daich <guy.daich@sap.com>
Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Isaac Wilson <10012479+jukie@users.noreply.github.com>
Co-authored-by: zirain <zirain2009@gmail.com>
Co-authored-by: Aditya Sanskar Srivastav <161202916+Aditya7880900936@users.noreply.github.com>
Co-authored-by: krishicks <kris@krishicks.com>
Co-authored-by: Jess Belliveau <jess.belliveau@gmail.com>
Inode1 pushed a commit to Inode1/gateway that referenced this pull request Feb 23, 2026
Previously, validateTerminateModeAndGetTLSSecrets would always use the
namespace of the listener's gateway when verifying a cross-namespace
ref.

This meant that if the listener were from an XListenerSet, whether or
not the Secret associated with the certificateRef was in the same
namespace as the XListenerSet, it would not be permitted.

Additionally, and relatedly, this fixes an issue where an XListenerSet
could reference a Secret in the gateway's namespace without a
ReferenceGrant being present.

With this change we add a new GetNamespace() method to
gatewayapi.ListenerContext which returns the listener's gateway's
namespace for a listener added directly to the gateway, or the
XListenerSet's namespace otherwise. This is similar to some of the other
methods that were added to ListenerContext in support of XListenerSets.

The new method is used when creating the `crossNamespaceFrom` to
determine if the certificateRef is permitted. If the Secret and
XListenerSet are in the same namespace, it is permitted. If that is not
the case a ReferenceGrant from the XListenerSet to the Secret will be
properly searched for.

Signed-off-by: Kris Hicks <khicks@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants