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

1.17.2 - terminating gateway broken by xds changes #20360

Closed
mr-miles opened this issue Jan 26, 2024 · 1 comment
Closed

1.17.2 - terminating gateway broken by xds changes #20360

mr-miles opened this issue Jan 26, 2024 · 1 comment

Comments

@mr-miles
Copy link
Contributor

mr-miles commented Jan 26, 2024

Upgraded to 1.17.2 via the helm chart and am seeing the terminating gateway is broken by #19954

agent/xds/clusters.go:1626 incorrectly restricts envoy to looking at URI SANs that match the hostname provided in config.

However certificates will have a DNS SAN with the hostname value. As a result outbound TLS connections fail on certificate errors.

I believe the fix is to use SanType: envoy_tls_v3.SubjectAltNameMatcher_DNS

Working config pre-upgrade (1.17.1):
image

Failing config from 1.17.2:
image

@mr-miles mr-miles changed the title 1.17.2 - envoy config update breaks terminating gateway 1.17.2 - terminating gateway broken by xds changes Jan 26, 2024
hashi-derek added a commit that referenced this issue Jan 31, 2024
Fixes issue: #20360

A regression was introduced in #19954 where the SAN validation
matching was reduced from 4 potential types down to just the URI.

Terminating gateways will need to match on many fields depending on user
configuration, since they make egress calls outside of the cluster. Having more
than one matcher behaves like an OR operation, where any match is sufficient to
pass the certificate validation. To maintain backwards compatibility with the
old untyped `match_subject_alt_names` Envoy behavior, we should match on all 4
enum types.

https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/transport_sockets/tls/v3/common.proto#enum-extensions-transport-sockets-tls-v3-subjectaltnamematcher-santype
hashi-derek added a commit that referenced this issue Jan 31, 2024
Fixes issue: #20360

A regression was introduced in #19954 where the SAN validation
matching was reduced from 4 potential types down to just the URI.

Terminating gateways will need to match on many fields depending on user
configuration, since they make egress calls outside of the cluster. Having more
than one matcher behaves like an OR operation, where any match is sufficient to
pass the certificate validation. To maintain backwards compatibility with the
old untyped `match_subject_alt_names` Envoy behavior, we should match on all 4
enum types.

https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/transport_sockets/tls/v3/common.proto#enum-extensions-transport-sockets-tls-v3-subjectaltnamematcher-santype
hashi-derek added a commit that referenced this issue Jan 31, 2024
Fixes issue: #20360

A regression was introduced in #19954 where the SAN validation
matching was reduced from 4 potential types down to just the URI.

Terminating gateways will need to match on many fields depending on user
configuration, since they make egress calls outside of the cluster. Having more
than one matcher behaves like an OR operation, where any match is sufficient to
pass the certificate validation. To maintain backwards compatibility with the
old untyped `match_subject_alt_names` Envoy behavior, we should match on all 4
enum types.

https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/transport_sockets/tls/v3/common.proto#enum-extensions-transport-sockets-tls-v3-subjectaltnamematcher-santype
hashi-derek added a commit that referenced this issue Jan 31, 2024
Fixes issue: #20360

A regression was introduced in #19954 where the SAN validation
matching was reduced from 4 potential types down to just the URI.

Terminating gateways will need to match on many fields depending on user
configuration, since they make egress calls outside of the cluster. Having more
than one matcher behaves like an OR operation, where any match is sufficient to
pass the certificate validation. To maintain backwards compatibility with the
old untyped `match_subject_alt_names` Envoy behavior, we should match on all 4
enum types.

https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/transport_sockets/tls/v3/common.proto#enum-extensions-transport-sockets-tls-v3-subjectaltnamematcher-santype
hashi-derek added a commit that referenced this issue Jan 31, 2024
Fixes issue: #20360

A regression was introduced in #19954 where the SAN validation
matching was reduced from 4 potential types down to just the URI.

Terminating gateways will need to match on many fields depending on user
configuration, since they make egress calls outside of the cluster. Having more
than one matcher behaves like an OR operation, where any match is sufficient to
pass the certificate validation. To maintain backwards compatibility with the
old untyped `match_subject_alt_names` Envoy behavior, we should match on all 4
enum types.

https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/transport_sockets/tls/v3/common.proto#enum-extensions-transport-sockets-tls-v3-subjectaltnamematcher-santype
hashi-derek added a commit that referenced this issue Jan 31, 2024
Fixes issue: #20360

A regression was introduced in #19954 where the SAN validation
matching was reduced from 4 potential types down to just the URI.

Terminating gateways will need to match on many fields depending on user
configuration, since they make egress calls outside of the cluster. Having more
than one matcher behaves like an OR operation, where any match is sufficient to
pass the certificate validation. To maintain backwards compatibility with the
old untyped `match_subject_alt_names` Envoy behavior, we should match on all 4
enum types.

https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/transport_sockets/tls/v3/common.proto#enum-extensions-transport-sockets-tls-v3-subjectaltnamematcher-santype
hashi-derek added a commit that referenced this issue Jan 31, 2024
…6.x (#20418)

Fix SAN matching on terminating gateways (#20417)

Fixes issue: #20360

A regression was introduced in #19954 where the SAN validation
matching was reduced from 4 potential types down to just the URI.

Terminating gateways will need to match on many fields depending on user
configuration, since they make egress calls outside of the cluster. Having more
than one matcher behaves like an OR operation, where any match is sufficient to
pass the certificate validation. To maintain backwards compatibility with the
old untyped `match_subject_alt_names` Envoy behavior, we should match on all 4
enum types.

https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/transport_sockets/tls/v3/common.proto#enum-extensions-transport-sockets-tls-v3-subjectaltnamematcher-santype

Co-authored-by: Derek Menteer <105233703+hashi-derek@users.noreply.github.com>
hashi-derek added a commit that referenced this issue Jan 31, 2024
…7.x (#20419)

Fix SAN matching on terminating gateways (#20417)

Fixes issue: #20360

A regression was introduced in #19954 where the SAN validation
matching was reduced from 4 potential types down to just the URI.

Terminating gateways will need to match on many fields depending on user
configuration, since they make egress calls outside of the cluster. Having more
than one matcher behaves like an OR operation, where any match is sufficient to
pass the certificate validation. To maintain backwards compatibility with the
old untyped `match_subject_alt_names` Envoy behavior, we should match on all 4
enum types.

https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/transport_sockets/tls/v3/common.proto#enum-extensions-transport-sockets-tls-v3-subjectaltnamematcher-santype

Co-authored-by: Derek Menteer <105233703+hashi-derek@users.noreply.github.com>
@david-yu
Copy link
Contributor

Closing, as this is addressed by #20417, this will be released in the next set of patch releases likely around March timeframe.

hashi-derek added a commit that referenced this issue Jan 31, 2024
hashi-derek added a commit that referenced this issue Jan 31, 2024
hashi-derek added a commit that referenced this issue Jan 31, 2024
hashi-derek added a commit that referenced this issue Feb 1, 2024
hashi-derek added a commit that referenced this issue Feb 1, 2024
hashi-derek added a commit that referenced this issue Feb 1, 2024
Add known issue for GH-20360.

Co-authored-by: Derek Menteer <derek.menteer@hashicorp.com>
hashi-derek added a commit that referenced this issue Feb 1, 2024
Add known issue for GH-20360.

Co-authored-by: Derek Menteer <derek.menteer@hashicorp.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

No branches or pull requests

2 participants