diff --git a/internal/gatewayapi/testdata/sds-invalid.in.yaml b/internal/gatewayapi/testdata/sds-invalid.in.yaml index d96ccf4fab..0688b7a08c 100644 --- a/internal/gatewayapi/testdata/sds-invalid.in.yaml +++ b/internal/gatewayapi/testdata/sds-invalid.in.yaml @@ -76,6 +76,21 @@ backendTLSPolicies: - name: regular-ca-secret kind: Secret hostname: example.com + # Test case 4: Invalid SDS URL (should be rejected) + - apiVersion: gateway.networking.k8s.io/v1alpha2 + kind: BackendTLSPolicy + metadata: + name: policy-invalid-sds-url + namespace: default + spec: + targetRefs: + - kind: Service + name: service-4 + validation: + caCertificateRefs: + - name: sds-ref-invalid-url + kind: Secret + hostname: example.com httpRoutes: - apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute @@ -109,6 +124,13 @@ httpRoutes: backendRefs: - name: service-3 port: 8080 + - matches: + - path: + type: Exact + value: "/service4" + backendRefs: + - name: service-4 + port: 8080 services: - apiVersion: v1 kind: Service @@ -207,7 +229,8 @@ secrets: namespace: default type: gateway.envoyproxy.io/sds data: - url: L3Zhci9ydW4vc2VjcmV0cy93b3JrbG9hZC1zcGlmZmUtdWRzL3NvY2tldA== # /var/run/secrets/workload-spiffe-uds/socket + # unix:///var/run/secrets/workload-spiffe-uds/socket + url: dW5peDovLy92YXIvcnVuL3NlY3JldHMvd29ya2xvYWQtc3BpZmZlLXVkcy9zb2NrZXQ= secretName: Uk9PVENB # ROOTCA - apiVersion: v1 kind: Secret @@ -216,8 +239,19 @@ secrets: namespace: default type: gateway.envoyproxy.io/sds data: - url: L3Zhci9ydW4vc2VjcmV0cy93b3JrbG9hZC1zcGlmZmUtdWRzL3NvY2tldA== # /var/run/secrets/workload-spiffe-uds/socket + # unix:///var/run/secrets/workload-spiffe-uds/socket + url: dW5peDovLy92YXIvcnVuL3NlY3JldHMvd29ya2xvYWQtc3BpZmZlLXVkcy9zb2NrZXQ= secretName: Uk9PVENBMg== # ROOTCA2 + # SDS reference secret with an invalid (scheme-less) URL + - apiVersion: v1 + kind: Secret + metadata: + name: sds-ref-invalid-url + namespace: default + type: gateway.envoyproxy.io/sds + data: + url: L3Zhci9ydW4vc2VjcmV0cy93b3JrbG9hZC1zcGlmZmUtdWRzL3NvY2tldA== # /var/run/secrets/workload-spiffe-uds/socket + secretName: Uk9PVENBMw== # ROOTCA3 # Regular secret with CA certificate - apiVersion: v1 kind: Secret diff --git a/internal/gatewayapi/testdata/sds-invalid.out.yaml b/internal/gatewayapi/testdata/sds-invalid.out.yaml index eef14eadff..5ed75c7f41 100644 --- a/internal/gatewayapi/testdata/sds-invalid.out.yaml +++ b/internal/gatewayapi/testdata/sds-invalid.out.yaml @@ -110,6 +110,44 @@ backendTLSPolicies: status: "False" type: ResolvedRefs controllerName: gateway.envoyproxy.io/gatewayclass-controller +- apiVersion: gateway.networking.k8s.io/v1alpha2 + kind: BackendTLSPolicy + metadata: + name: policy-invalid-sds-url + namespace: default + spec: + targetRefs: + - group: "" + kind: Service + name: service-4 + validation: + caCertificateRefs: + - group: "" + kind: Secret + name: sds-ref-invalid-url + hostname: example.com + status: + ancestors: + - ancestorRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http + conditions: + - lastTransitionTime: null + message: 'Invalid SDS reference secret sds-ref-invalid-url: invalid URL in + SDS reference secret default/sds-ref-invalid-url: unsupported URL scheme: + .' + reason: NoValidCACertificate + status: "False" + type: Accepted + - lastTransitionTime: null + message: 'Invalid SDS reference secret sds-ref-invalid-url: invalid URL in + SDS reference secret default/sds-ref-invalid-url: unsupported URL scheme: + .' + reason: InvalidCACertificateRef + status: "False" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller backends: - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: Backend @@ -201,6 +239,13 @@ httpRoutes: - path: type: Exact value: /service3 + - backendRefs: + - name: service-4 + port: 8080 + matches: + - path: + type: Exact + value: /service4 status: parents: - conditions: @@ -214,6 +259,7 @@ httpRoutes: Failed to process route rule 0 backendRef 0: multiple SDS reference secrets are not supported. Failed to process route rule 1 backendRef 0: cannot mix SDS reference secrets with other CA certificate types. Failed to process route rule 2 backendRef 0: cannot mix SDS reference secrets with other CA certificate types. + Failed to process route rule 3 backendRef 0: invalid SDS reference secret sds-ref-invalid-url: invalid URL in SDS reference secret default/sds-ref-invalid-url: unsupported URL scheme: . reason: InvalidBackendTLS status: "False" type: ResolvedRefs @@ -322,6 +368,19 @@ xdsIR: distinct: false exact: /service3 name: "" + - directResponse: + statusCode: 500 + hostname: '*' + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/3/match/0/* + pathMatch: + distinct: false + exact: /service4 + name: "" readyListener: address: 0.0.0.0 ipFamily: IPv4 diff --git a/internal/gatewayapi/testdata/sds.in.yaml b/internal/gatewayapi/testdata/sds.in.yaml index 37a28ea6a6..6c9c5e709e 100644 --- a/internal/gatewayapi/testdata/sds.in.yaml +++ b/internal/gatewayapi/testdata/sds.in.yaml @@ -80,8 +80,8 @@ secrets: namespace: envoy-gateway-system type: gateway.envoyproxy.io/sds data: - # /var/run/secrets/workload-spiffe-uds/socket - url: L3Zhci9ydW4vc2VjcmV0cy93b3JrbG9hZC1zcGlmZmUtdWRzL3NvY2tldA== + # unix:///var/run/secrets/workload-spiffe-uds/socket + url: dW5peDovLy92YXIvcnVuL3NlY3JldHMvd29ya2xvYWQtc3BpZmZlLXVkcy9zb2NrZXQ= secretName: ZGVmYXVsdA== # base64 for "default" - apiVersion: v1 kind: Secret @@ -90,6 +90,6 @@ secrets: namespace: default type: gateway.envoyproxy.io/sds data: - # /var/run/secrets/workload-spiffe-uds/socket - url: L3Zhci9ydW4vc2VjcmV0cy93b3JrbG9hZC1zcGlmZmUtdWRzL3NvY2tldA== + # unix:///var/run/secrets/workload-spiffe-uds/socket + url: dW5peDovLy92YXIvcnVuL3NlY3JldHMvd29ya2xvYWQtc3BpZmZlLXVkcy9zb2NrZXQ= secretName: Uk9PVENB # base64 for "ROOTCA" diff --git a/internal/gatewayapi/testdata/sds.out.yaml b/internal/gatewayapi/testdata/sds.out.yaml index 4229a2423b..466b028167 100644 --- a/internal/gatewayapi/testdata/sds.out.yaml +++ b/internal/gatewayapi/testdata/sds.out.yaml @@ -228,13 +228,15 @@ xdsIR: caCertificate: name: policy-btls/default-ca sds: + address: /var/run/secrets/workload-spiffe-uds/socket + scheme: unix secretName: ROOTCA - url: /var/run/secrets/workload-spiffe-uds/socket clientCertificates: - name: "" sds: + address: /var/run/secrets/workload-spiffe-uds/socket + scheme: unix secretName: default - url: /var/run/secrets/workload-spiffe-uds/socket maxVersion: "1.3" minVersion: "1.2" sni: example.com diff --git a/internal/ir/xds.go b/internal/ir/xds.go index c8ec2b573d..12bc108714 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -28,6 +28,7 @@ import ( egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" httputils "github.com/envoyproxy/gateway/internal/utils/http" + netutil "github.com/envoyproxy/gateway/internal/utils/net" ) const ( @@ -492,13 +493,23 @@ type TLSCertificate struct { type SDSConfig struct { // SecretName is an identifier for the SDS configuration. SecretName string `json:"secretName" yaml:"secretName"` - // URL is the URL of the SDS server - URL string `json:"url" yaml:"url"` + // Scheme is the communication scheme to use when connecting to the SDS server (e.g., "http", "https", or "unix"). + Scheme string `json:"scheme" yaml:"scheme"` + // Address is the host and port of the SDS server + Address string `json:"address" yaml:"address"` // TODO: support additional SDS configuration options // such as TLS settings for the SDS server, or authentication credentials if needed. } +func (s *SDSConfig) GetURL() string { + if s.Scheme == "" { + return s.Address + } + + return fmt.Sprintf("%s://%s", s.Scheme, s.Address) +} + func NewSDSConfig(s *corev1.Secret) (*SDSConfig, error) { sdsSecretName, hasSecretName := s.Data["secretName"] sdsURLBytes, hasURL := s.Data["url"] @@ -509,10 +520,15 @@ func NewSDSConfig(s *corev1.Secret) (*SDSConfig, error) { if !hasURL || len(sdsURLBytes) == 0 { return nil, fmt.Errorf("no url found in SDS reference secret %s/%s", s.Namespace, s.Name) } + scheme, hostAndPort, err := netutil.ParseURL(string(sdsURLBytes)) // validate the URL format + if err != nil { + return nil, fmt.Errorf("invalid URL in SDS reference secret %s/%s: %w", s.Namespace, s.Name, err) + } return &SDSConfig{ SecretName: string(sdsSecretName), - URL: string(sdsURLBytes), + Scheme: scheme, + Address: hostAndPort, }, nil } diff --git a/internal/utils/net/url.go b/internal/utils/net/url.go new file mode 100644 index 0000000000..5896e9b550 --- /dev/null +++ b/internal/utils/net/url.go @@ -0,0 +1,41 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package net + +import ( + "fmt" + "net/url" +) + +// ParseURL return host and port if the URL is a valid HTTP, HTTPS or UDS, +// return the scheme and host:port if it's valid. +func ParseURL(urlStr string) (scheme, hostAndPort string, err error) { + u, err := url.Parse(urlStr) + if err != nil { + return "", "", fmt.Errorf("invalid URL: %w", err) + } + + switch u.Scheme { + case "http", "https": + // TODO: support http and https scheme + return "", "", fmt.Errorf("unsupported URL scheme: %s", u.Scheme) + case "unix": + // For Unix Domain Socket, return the path as host, empty port. + // unix:///absolute/path has empty host and path="/absolute/path". + // unix://./relative/path (SPIFFE workload API format) has host="." and path="/relative/path"; + // combine them so the caller receives "./relative/path". + if u.Host != "" && u.Host != "." { + return "", "", fmt.Errorf("unix URL must not contain a host, use unix:///absolute/path") + } + if u.Path == "" && u.Host != "." { + return "", "", fmt.Errorf("unix URL must contain a path") + } + + return u.Scheme, u.Host + u.Path, nil + default: + return "", "", fmt.Errorf("unsupported URL scheme: %s", u.Scheme) + } +} diff --git a/internal/utils/net/url_test.go b/internal/utils/net/url_test.go new file mode 100644 index 0000000000..1da76905aa --- /dev/null +++ b/internal/utils/net/url_test.go @@ -0,0 +1,125 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package net + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestParseURL(t *testing.T) { + tests := []struct { + name string + url string + wantScheme string + wantHostAndPort string + wantErr bool + errContains string + }{ + { + name: "valid unix domain socket", + url: "unix:///var/run/app.sock", + wantScheme: "unix", + wantHostAndPort: "/var/run/app.sock", + wantErr: false, + }, + { + name: "spiffe workload api uds url with dot host", + url: "unix://./var/run/secrets/workload-spiffe-uds/socket", + wantScheme: "unix", + wantHostAndPort: "./var/run/secrets/workload-spiffe-uds/socket", + wantErr: false, + }, + { + name: "unix url with host silently dropped", + url: "unix://var/run/app.sock", + wantScheme: "", + wantHostAndPort: "", + wantErr: true, + errContains: "must not contain a host", + }, + { + name: "unix url with dot host as relative path", + url: "unix://./app.sock", + wantScheme: "unix", + wantHostAndPort: "./app.sock", + wantErr: false, + }, + { + name: "invalid url", + url: "://invalid", + wantScheme: "", + wantHostAndPort: "", + wantErr: true, + errContains: "invalid URL", + }, + { + name: "http url not supported", + url: "http://example.com:8080", + wantScheme: "", + wantHostAndPort: "", + wantErr: true, + errContains: "unsupported URL scheme", + }, + { + name: "https url not supported", + url: "https://example.com:9443", + wantScheme: "", + wantHostAndPort: "", + wantErr: true, + errContains: "unsupported URL scheme", + }, + { + name: "unix url without path", + url: "unix://", + wantScheme: "", + wantHostAndPort: "", + wantErr: true, + errContains: "must contain a path", + }, + { + name: "unsupported scheme ftp", + url: "ftp://example.com", + wantScheme: "", + wantHostAndPort: "", + wantErr: true, + errContains: "unsupported URL scheme", + }, + { + name: "unsupported scheme ws", + url: "ws://example.com:8080", + wantScheme: "", + wantHostAndPort: "", + wantErr: true, + errContains: "unsupported URL scheme", + }, + { + name: "empty url", + url: "", + wantScheme: "", + wantHostAndPort: "", + wantErr: true, + errContains: "unsupported URL scheme", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotScheme, gotHostAndPort, err := ParseURL(tt.url) + if tt.wantErr { + require.Error(t, err) + if tt.errContains != "" { + require.Contains(t, err.Error(), tt.errContains) + } + } else { + require.NoError(t, err) + } + require.Equal(t, tt.wantScheme, gotScheme) + require.Equal(t, tt.wantHostAndPort, gotHostAndPort) + }) + } +} diff --git a/internal/xds/translator/listener.go b/internal/xds/translator/listener.go index 1f8569dfcb..7438138c9c 100644 --- a/internal/xds/translator/listener.go +++ b/internal/xds/translator/listener.go @@ -853,7 +853,7 @@ func buildDownstreamQUICTransportSocket(tlsConfig *ir.TLSConfig) (*corev3.Transp } if cert.SDS != nil { // Use external SDS server instead of ADS - clusterName := sdsClusterNameFromURL(cert.SDS.URL) + clusterName := sdsClusterNameFromURL(cert.SDS.GetURL()) sdsConfig = sdsSecretConfig(cert.SDS.SecretName, clusterName) } tlsCtx.DownstreamTlsContext.CommonTlsContext.TlsCertificateSdsSecretConfigs = append( @@ -896,7 +896,7 @@ func buildXdsDownstreamTLSSocket(tlsConfig *ir.TLSConfig) (*corev3.TransportSock } if cert.SDS != nil { // Use external SDS server instead of ADS - clusterName := sdsClusterNameFromURL(cert.SDS.URL) + clusterName := sdsClusterNameFromURL(cert.SDS.GetURL()) sdsConfig = sdsSecretConfig(cert.SDS.SecretName, clusterName) } tlsCtx.CommonTlsContext.TlsCertificateSdsSecretConfigs = append( @@ -980,7 +980,7 @@ func setTLSValidationContext(tlsConfig *ir.TLSConfig, tlsCtx *tlsv3.CommonTlsCon if tlsConfig.CACertificate.SDS != nil { // Use external SDS server instead of ADS - clusterName := sdsClusterNameFromURL(tlsConfig.CACertificate.SDS.URL) + clusterName := sdsClusterNameFromURL(tlsConfig.CACertificate.SDS.GetURL()) sdsConfig = sdsSecretConfig(tlsConfig.CACertificate.SDS.SecretName, clusterName) } diff --git a/internal/xds/translator/sds.go b/internal/xds/translator/sds.go index 71d3279c92..11354934a3 100644 --- a/internal/xds/translator/sds.go +++ b/internal/xds/translator/sds.go @@ -50,9 +50,16 @@ func sdsSecretConfig(secretName, clusterName string) *tlsv3.SdsSecretConfig { // sdsClusterNameFromURL generates a unique cluster name from an SDS URL func sdsClusterNameFromURL(url string) string { // Sanitize the URL to create a valid cluster name - // For Unix domain sockets like "/var/run/sds", create a meaningful name + // For Unix domain sockets like "unix:///var/run/sds" or "/var/run/sds", create a meaningful name + if strings.HasPrefix(url, "unix://") { + // Unix domain socket with scheme - extract the path + path := strings.TrimPrefix(url, "unix://") + sanitized := strings.ReplaceAll(path, "/", "_") + sanitized = strings.Trim(sanitized, "_") + return fmt.Sprintf("sds_%s", sanitized) + } if strings.HasPrefix(url, "/") { - // Unix domain socket path + // Unix domain socket path without scheme sanitized := strings.ReplaceAll(url, "/", "_") sanitized = strings.Trim(sanitized, "_") return fmt.Sprintf("sds_%s", sanitized) @@ -77,6 +84,12 @@ func createSDSCluster(tCtx *types.ResourceVersionTable, sdsURL string) error { } // Create the cluster based on the URL type + pipePath := sdsURL + // Extract path for Unix domain sockets + if strings.HasPrefix(sdsURL, "unix://") { + pipePath = strings.TrimPrefix(sdsURL, "unix://") + } + c := &cluster.Cluster{ Name: clusterName, ClusterDiscoveryType: &cluster.Cluster_Type{ @@ -93,7 +106,7 @@ func createSDSCluster(tCtx *types.ResourceVersionTable, sdsURL string) error { Address: &corev3.Address{ Address: &corev3.Address_Pipe{ Pipe: &corev3.Pipe{ - Path: sdsURL, + Path: pipePath, }, }, }, @@ -128,14 +141,14 @@ func processSDSClusters(tCtx *types.ResourceVersionTable, xdsIR *ir.Xds) error { if dest.TLS != nil { // Check CA certificate if caCert := dest.TLS.CACertificate; caCert != nil { - if caCert.SDS != nil && caCert.SDS.URL != "" { - sdsURLs[caCert.SDS.URL] = true + if caCert.SDS != nil && caCert.SDS.GetURL() != "" { + sdsURLs[caCert.SDS.GetURL()] = true } } // Check client certificates for _, cert := range dest.TLS.ClientCertificates { - if cert.SDS != nil && cert.SDS.URL != "" { - sdsURLs[cert.SDS.URL] = true + if cert.SDS != nil && cert.SDS.GetURL() != "" { + sdsURLs[cert.SDS.GetURL()] = true } } } @@ -153,14 +166,14 @@ func processSDSClusters(tCtx *types.ResourceVersionTable, xdsIR *ir.Xds) error { if dest.TLS != nil { // Check CA certificate if caCert := dest.TLS.CACertificate; caCert != nil { - if caCert.SDS != nil && caCert.SDS.URL != "" { - sdsURLs[caCert.SDS.URL] = true + if caCert.SDS != nil && caCert.SDS.GetURL() != "" { + sdsURLs[caCert.SDS.GetURL()] = true } } // Check client certificates for _, cert := range dest.TLS.ClientCertificates { - if cert.SDS != nil && cert.SDS.URL != "" { - sdsURLs[cert.SDS.URL] = true + if cert.SDS != nil && cert.SDS.GetURL() != "" { + sdsURLs[cert.SDS.GetURL()] = true } } } diff --git a/internal/xds/translator/testdata/in/xds-ir/sds.yaml b/internal/xds/translator/testdata/in/xds-ir/sds.yaml index 8e05a6b992..40f9c8a012 100644 --- a/internal/xds/translator/testdata/in/xds-ir/sds.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/sds.yaml @@ -38,11 +38,13 @@ http: caCertificate: sds: secretName: ROOTCA - url: /var/run/secrets/workload-spiffe-uds/socket + scheme: unix + address: /var/run/secrets/workload-spiffe-uds/socket clientCertificates: - sds: secretName: default - url: /var/run/secrets/workload-spiffe-uds/socket + scheme: unix + address: /var/run/secrets/workload-spiffe-uds/socket sni: example.com weight: 1 hostname: "*" diff --git a/internal/xds/translator/translator.go b/internal/xds/translator/translator.go index ec4272e49d..5010eb7b05 100644 --- a/internal/xds/translator/translator.go +++ b/internal/xds/translator/translator.go @@ -1224,7 +1224,7 @@ func buildValidationContext(tlsConfig *ir.TLSUpstreamConfig) (*tlsv3.CommonTlsCo if tlsConfig.CACertificate.SDS != nil { // get secret from SDS sds := tlsConfig.CACertificate.SDS - clusterName := sdsClusterNameFromURL(sds.URL) + clusterName := sdsClusterNameFromURL(sds.GetURL()) validationContext.ValidationContextSdsSecretConfig = sdsSecretConfig(sds.SecretName, clusterName) } hasSANValidations := false @@ -1319,7 +1319,7 @@ func buildXdsUpstreamTLSSocketWthCert(tlsConfig *ir.TLSUpstreamConfig, requiresA for _, clientCert := range tlsConfig.ClientCertificates { if sds := clientCert.SDS; sds != nil { - clusterName := sdsClusterNameFromURL(sds.URL) + clusterName := sdsClusterNameFromURL(sds.GetURL()) sds := sdsSecretConfig(sds.SecretName, clusterName) tlsCtx.CommonTlsContext.TlsCertificateSdsSecretConfigs = append(tlsCtx.CommonTlsContext.TlsCertificateSdsSecretConfigs, sds) continue diff --git a/release-notes/current/breaking_changes/9030-sds-uds-url-scheme-required.md b/release-notes/current/breaking_changes/9030-sds-uds-url-scheme-required.md new file mode 100644 index 0000000000..fdbea07f04 --- /dev/null +++ b/release-notes/current/breaking_changes/9030-sds-uds-url-scheme-required.md @@ -0,0 +1 @@ +The SDS (Secret Discovery Service) reference secret `url` field must now include the `unix://` scheme (e.g. `unix:///var/run/secrets/workload-spiffe-uds/socket`). Bare filesystem paths that were previously accepted (introduced in v1.9.0-rc.0) are now rejected and must be updated. diff --git a/release-notes/current/bug_fixes/9030-sds-uds-url-validation.md b/release-notes/current/bug_fixes/9030-sds-uds-url-validation.md new file mode 100644 index 0000000000..00b5c6ec42 --- /dev/null +++ b/release-notes/current/bug_fixes/9030-sds-uds-url-validation.md @@ -0,0 +1 @@ +Added validation for the SDS (Secret Discovery Service) cluster URI to ensure Unix Domain Socket (`unix://`) URLs are well-formed, rejecting a host component and requiring a path, instead of silently producing an invalid or unintended socket address.