From e1eb85fc3913b2d6788bbfd304c962b8abbbb2d4 Mon Sep 17 00:00:00 2001 From: Chris Thach Date: Thu, 6 Nov 2025 16:44:23 -0500 Subject: [PATCH 1/8] fix: App public_addr allows setting a conflicting hostname with the proxy when using capital letters Signed-off-by: Chris Thach --- lib/services/app.go | 82 ++++++++++++++++++++++++++---------- lib/services/app_test.go | 88 +++++++++++++++++++++++++++++++++++++++ lib/services/fuzz_test.go | 38 +++++++++++++++++ 3 files changed, 185 insertions(+), 23 deletions(-) diff --git a/lib/services/app.go b/lib/services/app.go index 75d271514e492..312f885b82435 100644 --- a/lib/services/app.go +++ b/lib/services/app.go @@ -28,12 +28,12 @@ import ( "net/http" "net/url" "os" - "slices" "strconv" "strings" "sync" "github.com/gravitational/trace" + "golang.org/x/net/idna" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/validation" kyaml "k8s.io/apimachinery/pkg/util/yaml" @@ -72,41 +72,77 @@ type Applications interface { // ValidateApp validates the Application resource. func ValidateApp(app types.Application, proxyGetter ProxyGetter) error { + // If no public address is set, no need to validate further. + if app.GetPublicAddr() == "" { + return nil + } + + // Ensure the public address is a valid hostname. It cannot have a port or scheme or path. + appPublicAddrURL, err := utils.ParseAddr(app.GetPublicAddr()) + if err != nil { + return trace.Wrap(err) + } + if appPublicAddrURL.Port(0) != 0 || strings.Contains(app.GetPublicAddr(), "://") || appPublicAddrURL.Path != "" { + return trace.BadParameter( + "Application %q has an invalid public address %q. The public address must be a valid hostname e.g., 'app.example.com'. "+ + "Refer to https://goteleport.com/docs/enroll-resources/application-access/guides/connecting-apps/#customize-public-address.", + app.GetName(), app.GetPublicAddr()) + } + // Prevent routing conflicts and session hijacking by ensuring the application's public address does not match the // public address of any proxy. If an application shares a public address with a proxy, requests intended for the // proxy could be misrouted to the application, compromising security. - if app.GetPublicAddr() != "" { - proxyServers, err := proxyGetter.GetProxies() + proxyServers, err := proxyGetter.GetProxies() + if err != nil { + return trace.Wrap(err) + } + + for _, proxyServer := range proxyServers { + proxyAddrs, err := utils.ParseAddrs(proxyServer.GetPublicAddrs()) if err != nil { return trace.Wrap(err) } - for _, proxyServer := range proxyServers { - proxyAddrs, err := utils.ParseAddrs(proxyServer.GetPublicAddrs()) - if err != nil { - return trace.Wrap(err) - } - - if slices.ContainsFunc( - proxyAddrs, - func(proxyAddr utils.NetAddr) bool { - return app.GetPublicAddr() == proxyAddr.Host() - }, - ) { - return trace.BadParameter( - "Application %q public address %q conflicts with the Teleport Proxy public address. "+ - "Configure the application to use a unique public address that does not match the proxy's public addresses. "+ - "Refer to https://goteleport.com/docs/enroll-resources/application-access/guides/connecting-apps/.", - app.GetName(), - app.GetPublicAddr(), - ) - } + matches, err := containsHostname(proxyAddrs, appPublicAddrURL.Host()) + if err != nil { + return trace.Wrap(err) + } + if matches { + return trace.BadParameter( + "Application %q public address %q conflicts with the Teleport Proxy public address. "+ + "Configure the application to use a unique public address that does not match the proxy's public addresses. "+ + "Refer to https://goteleport.com/docs/enroll-resources/application-access/guides/connecting-apps/#customize-public-address.", + app.GetName(), + app.GetPublicAddr(), + ) } } return nil } +// containsHostname returns true if the hostname is contained within the given list, taking into account IDNA encoding +// and case sensitivity. +func containsHostname(addresses []utils.NetAddr, targetHost string) (bool, error) { + asciiTargetHost, err := idna.ToASCII(targetHost) + if err != nil { + return false, trace.BadParameter("invalid IDNA hostname %q: %v", targetHost, err) + } + + for _, addr := range addresses { + asciiAddrHost, err := idna.ToASCII(addr.Host()) + if err != nil { + return false, trace.BadParameter("invalid IDNA hostname %q: %v", addr.Host(), err) + } + + if strings.EqualFold(asciiAddrHost, asciiTargetHost) { + return true, nil + } + } + + return false, nil +} + // MarshalApp marshals Application resource to JSON. func MarshalApp(app types.Application, opts ...MarshalOption) ([]byte, error) { cfg, err := CollectOptions(opts) diff --git a/lib/services/app_test.go b/lib/services/app_test.go index a517968e6cab0..cdac2907819ee 100644 --- a/lib/services/app_test.go +++ b/lib/services/app_test.go @@ -66,6 +66,15 @@ func TestValidateApp(t *testing.T) { proxyAddrs: []string{"web.example.com:443"}, wantErr: "conflicts with the Teleport Proxy public address", }, + { + name: "public addr with capital letters matches proxy host", + app: func() types.Application { + app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "WeB.ExAmPle.CoM"}) + return app + }(), + proxyAddrs: []string{"web.example.com:443"}, + wantErr: "conflicts with the Teleport Proxy public address", + }, { name: "multiple proxy addrs, one matches", app: func() types.Application { @@ -75,6 +84,85 @@ func TestValidateApp(t *testing.T) { proxyAddrs: []string{"other.com:443", "web.example.com:443"}, wantErr: "conflicts with the Teleport Proxy public address", }, + { + name: "public addr with IDN matches proxy host", + app: func() types.Application { + app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "例.cn"}) + return app + }(), + proxyAddrs: []string{"xn--fsq.cn:443"}, + wantErr: "conflicts with the Teleport Proxy public address", + }, + { + name: "public addr with IDN does not conflict with non-IDN proxy host", + app: func() types.Application { + app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "münchen.de"}) + return app + }(), + proxyAddrs: []string{"example.com:443"}, + }, + { + name: "IDN with mixed case matches proxy host", + app: func() types.Application { + app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "MünchEn.de"}) + return app + }(), + proxyAddrs: []string{"münchen.de:443"}, + wantErr: "conflicts with the Teleport Proxy public address", + }, + { + name: "IDN with subdomains matches proxy host", + app: func() types.Application { + app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "sub.münchen.de"}) + return app + }(), + proxyAddrs: []string{"sub.xn--mnchen-3ya.de:443"}, + wantErr: "conflicts with the Teleport Proxy public address", + }, + { + name: "empty proxy addrs", + app: func() types.Application { + app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "example.com"}) + return app + }(), + proxyAddrs: []string{}, + }, + { + name: "multiple conflicting proxies", + app: func() types.Application { + app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "example.com"}) + return app + }(), + proxyAddrs: []string{"example.com:443", "example.com:80"}, + wantErr: "conflicts with the Teleport Proxy public address", + }, + { + name: "public addr with port", + app: func() types.Application { + app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "app.example.com:8080"}) + return app + }(), + proxyAddrs: []string{"web.example.com:443"}, + wantErr: "invalid public address", + }, + { + name: "public addr with scheme", + app: func() types.Application { + app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "http://app.example.com"}) + return app + }(), + proxyAddrs: []string{"web.example.com:443"}, + wantErr: "invalid public address", + }, + { + name: "public addr with path", + app: func() types.Application { + app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "app.example.com/path"}) + return app + }(), + proxyAddrs: []string{"web.example.com:443"}, + wantErr: "invalid public address", + }, } for _, tt := range tests { diff --git a/lib/services/fuzz_test.go b/lib/services/fuzz_test.go index 0aa3679e5e9e2..418219367480c 100644 --- a/lib/services/fuzz_test.go +++ b/lib/services/fuzz_test.go @@ -19,11 +19,14 @@ package services import ( + "strings" "testing" + "github.com/gravitational/trace" "github.com/stretchr/testify/require" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/utils" ) func FuzzParseRefs(f *testing.F) { @@ -123,3 +126,38 @@ func FuzzParserEvalBoolPredicate(f *testing.F) { }) }) } + +func FuzzValidateApp(f *testing.F) { + // seeds from unit tests + f.Add("http://localhost:8080", "web.example.com:443") // Valid proxy + f.Add("http://localhost:8080", "app.example.com:443") // Another valid proxy + f.Add("http://localhost:8080", "web.example.com:443,other.com:443") // Multiple proxies + f.Add("http://invalid-url", "web.example.com:443") // Invalid app URL + f.Add("http://localhost:8080", "xn--mnchen-3ya.de:443") // IDN in Punycode + f.Add("http://localhost:8080", "münchen.de:443") // IDN in Unicode + f.Add("http://localhost:8080", "example.com:443,example.com:80") // Multiple ports + f.Add("http://localhost:8080", "") // Empty proxy address + f.Add("http://localhost:8080", "example.com") // Proxy without port + + f.Fuzz(func(t *testing.T, appURI string, proxyAddrs string) { + app, err := types.NewAppV3(types.Metadata{Name: "fuzz-app"}, types.AppSpecV3{URI: appURI}) + if err != nil { + // Skip invalid app URIs + return + } + + proxyAddrList := strings.Split(proxyAddrs, ",") + mockProxyGetter := &mockProxyGetter{addrs: proxyAddrList} + + for _, addr := range proxyAddrList { + if _, err := utils.ParseAddr(addr); err != nil { + // Skip invalid proxy addresses + return + } + } + + if err = ValidateApp(app, mockProxyGetter); err != nil { + require.True(t, trace.IsBadParameter(err) || trace.IsAccessDenied(err)) + } + }) +} From 84332b1d63b681fe8e1e15c1666f6405f70bfc87 Mon Sep 17 00:00:00 2001 From: Chris Thach Date: Thu, 6 Nov 2025 19:11:21 -0500 Subject: [PATCH 2/8] Only normalize once. Remove app public_addr validate. Signed-off-by: Chris Thach --- lib/services/app.go | 70 +++++++++++++++------------------------- lib/services/app_test.go | 27 ---------------- 2 files changed, 26 insertions(+), 71 deletions(-) diff --git a/lib/services/app.go b/lib/services/app.go index 312f885b82435..4aca2a0e0db1d 100644 --- a/lib/services/app.go +++ b/lib/services/app.go @@ -77,70 +77,52 @@ func ValidateApp(app types.Application, proxyGetter ProxyGetter) error { return nil } - // Ensure the public address is a valid hostname. It cannot have a port or scheme or path. - appPublicAddrURL, err := utils.ParseAddr(app.GetPublicAddr()) + appPublicAddr, err := utils.ParseAddr(app.GetPublicAddr()) if err != nil { return trace.Wrap(err) } - if appPublicAddrURL.Port(0) != 0 || strings.Contains(app.GetPublicAddr(), "://") || appPublicAddrURL.Path != "" { - return trace.BadParameter( - "Application %q has an invalid public address %q. The public address must be a valid hostname e.g., 'app.example.com'. "+ - "Refer to https://goteleport.com/docs/enroll-resources/application-access/guides/connecting-apps/#customize-public-address.", - app.GetName(), app.GetPublicAddr()) + + // Normalize the application public address to ASCII using IDNA. + asciiAppPublicAddrHost, err := idna.ToASCII(appPublicAddr.Host()) + if err != nil { + return trace.Wrap(err) } - // Prevent routing conflicts and session hijacking by ensuring the application's public address does not match the - // public address of any proxy. If an application shares a public address with a proxy, requests intended for the - // proxy could be misrouted to the application, compromising security. proxyServers, err := proxyGetter.GetProxies() if err != nil { return trace.Wrap(err) } + // Prevent routing conflicts and session hijacking by ensuring the application's public address does not match the + // public address of any proxy. If an application shares a public address with a proxy, requests intended for the + // proxy could be misrouted to the application, compromising security. for _, proxyServer := range proxyServers { proxyAddrs, err := utils.ParseAddrs(proxyServer.GetPublicAddrs()) if err != nil { return trace.Wrap(err) } - matches, err := containsHostname(proxyAddrs, appPublicAddrURL.Host()) - if err != nil { - return trace.Wrap(err) - } - if matches { - return trace.BadParameter( - "Application %q public address %q conflicts with the Teleport Proxy public address. "+ - "Configure the application to use a unique public address that does not match the proxy's public addresses. "+ - "Refer to https://goteleport.com/docs/enroll-resources/application-access/guides/connecting-apps/#customize-public-address.", - app.GetName(), - app.GetPublicAddr(), - ) - } - } - - return nil -} - -// containsHostname returns true if the hostname is contained within the given list, taking into account IDNA encoding -// and case sensitivity. -func containsHostname(addresses []utils.NetAddr, targetHost string) (bool, error) { - asciiTargetHost, err := idna.ToASCII(targetHost) - if err != nil { - return false, trace.BadParameter("invalid IDNA hostname %q: %v", targetHost, err) - } - - for _, addr := range addresses { - asciiAddrHost, err := idna.ToASCII(addr.Host()) - if err != nil { - return false, trace.BadParameter("invalid IDNA hostname %q: %v", addr.Host(), err) - } + for _, proxyAddr := range proxyAddrs { + // Normalize the proxy public address to ASCII using IDNA. + asciiProxyAddr, err := idna.ToASCII(proxyAddr.Host()) + if err != nil { + return trace.BadParameter("invalid IDNA hostname %q: %v", proxyAddr, err) + } - if strings.EqualFold(asciiAddrHost, asciiTargetHost) { - return true, nil + // Compare the ASCII-normalized hostnames for equality, ignoring case. + if strings.EqualFold(asciiProxyAddr, asciiAppPublicAddrHost) { + return trace.BadParameter( + "Application %q public address %q conflicts with the Teleport Proxy public address. "+ + "Configure the application to use a unique public address that does not match the proxy's public addresses. "+ + "Refer to https://goteleport.com/docs/enroll-resources/application-access/guides/connecting-apps/#customize-public-address.", + app.GetName(), + app.GetPublicAddr(), + ) + } } } - return false, nil + return nil } // MarshalApp marshals Application resource to JSON. diff --git a/lib/services/app_test.go b/lib/services/app_test.go index cdac2907819ee..743df8ae0708f 100644 --- a/lib/services/app_test.go +++ b/lib/services/app_test.go @@ -136,33 +136,6 @@ func TestValidateApp(t *testing.T) { proxyAddrs: []string{"example.com:443", "example.com:80"}, wantErr: "conflicts with the Teleport Proxy public address", }, - { - name: "public addr with port", - app: func() types.Application { - app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "app.example.com:8080"}) - return app - }(), - proxyAddrs: []string{"web.example.com:443"}, - wantErr: "invalid public address", - }, - { - name: "public addr with scheme", - app: func() types.Application { - app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "http://app.example.com"}) - return app - }(), - proxyAddrs: []string{"web.example.com:443"}, - wantErr: "invalid public address", - }, - { - name: "public addr with path", - app: func() types.Application { - app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "app.example.com/path"}) - return app - }(), - proxyAddrs: []string{"web.example.com:443"}, - wantErr: "invalid public address", - }, } for _, tt := range tests { From 291929d83851918aa777fadbea3b34553c2e8fd2 Mon Sep 17 00:00:00 2001 From: Chris Thach Date: Thu, 6 Nov 2025 20:41:18 -0500 Subject: [PATCH 3/8] Fix grpc tests Signed-off-by: Chris Thach --- lib/auth/grpcserver_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/auth/grpcserver_test.go b/lib/auth/grpcserver_test.go index 89f4becb47df7..07dbb6c8fcccd 100644 --- a/lib/auth/grpcserver_test.go +++ b/lib/auth/grpcserver_test.go @@ -3813,7 +3813,7 @@ func TestAppsCRUD(t *testing.T) { require.NoError(t, err) err = clt.CreateApp(ctx, misconfiguredApp) - require.ErrorIs(t, err, trace.BadParameter(`Application "misconfigured-app" public address "proxy.example.com" conflicts with the Teleport Proxy public address. Configure the application to use a unique public address that does not match the proxy's public addresses. Refer to https://goteleport.com/docs/enroll-resources/application-access/guides/connecting-apps/.`)) + require.ErrorIs(t, err, trace.BadParameter(`Application "misconfigured-app" public address "proxy.example.com" conflicts with the Teleport Proxy public address. Configure the application to use a unique public address that does not match the proxy's public addresses. Refer to https://goteleport.com/docs/enroll-resources/application-access/guides/connecting-apps/#customize-public-address.`)) }) t.Run("Updating an app with a public address matching a proxy address should fail", func(t *testing.T) { @@ -3827,7 +3827,7 @@ func TestAppsCRUD(t *testing.T) { require.NoError(t, err) err = clt.UpdateApp(ctx, misconfiguredApp) - require.ErrorIs(t, err, trace.BadParameter(`Application "misconfigured-app" public address "proxy.example.com" conflicts with the Teleport Proxy public address. Configure the application to use a unique public address that does not match the proxy's public addresses. Refer to https://goteleport.com/docs/enroll-resources/application-access/guides/connecting-apps/.`)) + require.ErrorIs(t, err, trace.BadParameter(`Application "misconfigured-app" public address "proxy.example.com" conflicts with the Teleport Proxy public address. Configure the application to use a unique public address that does not match the proxy's public addresses. Refer to https://goteleport.com/docs/enroll-resources/application-access/guides/connecting-apps/#customize-public-address.`)) }) } @@ -3953,7 +3953,7 @@ func TestAppServersCRUD(t *testing.T) { require.NoError(t, err) _, err = clt.UpsertApplicationServer(ctx, appServer) - require.ErrorIs(t, err, trace.BadParameter(`Application "misconfigured-app" public address "proxy.example.com" conflicts with the Teleport Proxy public address. Configure the application to use a unique public address that does not match the proxy's public addresses. Refer to https://goteleport.com/docs/enroll-resources/application-access/guides/connecting-apps/.`)) + require.ErrorIs(t, err, trace.BadParameter(`Application "misconfigured-app" public address "proxy.example.com" conflicts with the Teleport Proxy public address. Configure the application to use a unique public address that does not match the proxy's public addresses. Refer to https://goteleport.com/docs/enroll-resources/application-access/guides/connecting-apps/#customize-public-address.`)) }) } From eb1f8e3d9fd911aa2844c05456888c31e9b9df7e Mon Sep 17 00:00:00 2001 From: Chris Thach Date: Fri, 7 Nov 2025 18:26:07 -0500 Subject: [PATCH 4/8] Polish and optimize ValidateApp Signed-off-by: Chris Thach --- lib/services/app.go | 22 ++++++++++------- lib/services/app_test.go | 45 +++++++++++++++++++++++++---------- lib/services/fuzz_test.go | 50 ++++++++++++++++++--------------------- 3 files changed, 69 insertions(+), 48 deletions(-) diff --git a/lib/services/app.go b/lib/services/app.go index 4aca2a0e0db1d..1263e70db6f16 100644 --- a/lib/services/app.go +++ b/lib/services/app.go @@ -72,20 +72,23 @@ type Applications interface { // ValidateApp validates the Application resource. func ValidateApp(app types.Application, proxyGetter ProxyGetter) error { - // If no public address is set, no need to validate further. + // If no public address is set, there's nothing to validate. if app.GetPublicAddr() == "" { return nil } - appPublicAddr, err := utils.ParseAddr(app.GetPublicAddr()) + // It is assumed that the app's public address has already been validated to be a valid address format during app + // resource validation. The remainder of this function focuses on detecting conflicts with proxy public addresses. + appAddr, err := utils.ParseAddr(app.GetPublicAddr()) if err != nil { return trace.Wrap(err) } - // Normalize the application public address to ASCII using IDNA. - asciiAppPublicAddrHost, err := idna.ToASCII(appPublicAddr.Host()) + // Convert the application's public address hostname to its ASCII representation for comparison. Strip any trailing + // dot to ensure consistent comparison. + asciiAppHostname, err := idna.ToASCII(strings.TrimSuffix(appAddr.Host(), ".")) if err != nil { - return trace.Wrap(err) + return trace.WrapWithMessage(err, "app %q has an invalid IDN hostname %q", app.GetName(), appAddr.Host()) } proxyServers, err := proxyGetter.GetProxies() @@ -103,14 +106,15 @@ func ValidateApp(app types.Application, proxyGetter ProxyGetter) error { } for _, proxyAddr := range proxyAddrs { - // Normalize the proxy public address to ASCII using IDNA. - asciiProxyAddr, err := idna.ToASCII(proxyAddr.Host()) + // Also convert the proxy's public address hostname to its ASCII representation for comparison and strip any + // trailing dot. + asciiProxyHostname, err := idna.ToASCII(strings.TrimSuffix(proxyAddr.Host(), ".")) if err != nil { - return trace.BadParameter("invalid IDNA hostname %q: %v", proxyAddr, err) + return trace.WrapWithMessage(err, "proxy %q has an invalid IDN hostname %q", proxyServer.GetName(), proxyAddr) } // Compare the ASCII-normalized hostnames for equality, ignoring case. - if strings.EqualFold(asciiProxyAddr, asciiAppPublicAddrHost) { + if strings.EqualFold(asciiProxyHostname, asciiAppHostname) { return trace.BadParameter( "Application %q public address %q conflicts with the Teleport Proxy public address. "+ "Configure the application to use a unique public address that does not match the proxy's public addresses. "+ diff --git a/lib/services/app_test.go b/lib/services/app_test.go index 743df8ae0708f..8788c071826c8 100644 --- a/lib/services/app_test.go +++ b/lib/services/app_test.go @@ -44,7 +44,8 @@ func TestValidateApp(t *testing.T) { { name: "no public addr, no error", app: func() types.Application { - app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080"}) + app, err := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080"}) + require.NoError(t, err) return app }(), proxyAddrs: []string{"web.example.com:443"}, @@ -52,7 +53,8 @@ func TestValidateApp(t *testing.T) { { name: "public addr does not conflict", app: func() types.Application { - app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "app.example.com"}) + app, err := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "app.example.com"}) + require.NoError(t, err) return app }(), proxyAddrs: []string{"web.example.com:443"}, @@ -60,16 +62,28 @@ func TestValidateApp(t *testing.T) { { name: "public addr matches proxy host", app: func() types.Application { - app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "web.example.com"}) + app, err := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "web.example.com"}) + require.NoError(t, err) return app }(), proxyAddrs: []string{"web.example.com:443"}, wantErr: "conflicts with the Teleport Proxy public address", }, { - name: "public addr with capital letters matches proxy host", + name: "public addr with trailing dot matches proxy host", app: func() types.Application { - app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "WeB.ExAmPle.CoM"}) + app, err := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "web.example.com."}) + require.NoError(t, err) + return app + }(), + proxyAddrs: []string{"web.example.com:443"}, + wantErr: "conflicts with the Teleport Proxy public address", + }, + { + name: "public addr with mixed casing matches proxy host", + app: func() types.Application { + app, err := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "WeB.ExAmPle.CoM"}) + require.NoError(t, err) return app }(), proxyAddrs: []string{"web.example.com:443"}, @@ -78,7 +92,8 @@ func TestValidateApp(t *testing.T) { { name: "multiple proxy addrs, one matches", app: func() types.Application { - app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "web.example.com"}) + app, err := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "web.example.com"}) + require.NoError(t, err) return app }(), proxyAddrs: []string{"other.com:443", "web.example.com:443"}, @@ -87,7 +102,8 @@ func TestValidateApp(t *testing.T) { { name: "public addr with IDN matches proxy host", app: func() types.Application { - app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "例.cn"}) + app, err := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "例.cn"}) + require.NoError(t, err) return app }(), proxyAddrs: []string{"xn--fsq.cn:443"}, @@ -96,7 +112,8 @@ func TestValidateApp(t *testing.T) { { name: "public addr with IDN does not conflict with non-IDN proxy host", app: func() types.Application { - app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "münchen.de"}) + app, err := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "münchen.de"}) + require.NoError(t, err) return app }(), proxyAddrs: []string{"example.com:443"}, @@ -104,7 +121,8 @@ func TestValidateApp(t *testing.T) { { name: "IDN with mixed case matches proxy host", app: func() types.Application { - app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "MünchEn.de"}) + app, err := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "MünchEn.de"}) + require.NoError(t, err) return app }(), proxyAddrs: []string{"münchen.de:443"}, @@ -113,7 +131,8 @@ func TestValidateApp(t *testing.T) { { name: "IDN with subdomains matches proxy host", app: func() types.Application { - app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "sub.münchen.de"}) + app, err := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "sub.münchen.de"}) + require.NoError(t, err) return app }(), proxyAddrs: []string{"sub.xn--mnchen-3ya.de:443"}, @@ -122,7 +141,8 @@ func TestValidateApp(t *testing.T) { { name: "empty proxy addrs", app: func() types.Application { - app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "example.com"}) + app, err := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "example.com"}) + require.NoError(t, err) return app }(), proxyAddrs: []string{}, @@ -130,7 +150,8 @@ func TestValidateApp(t *testing.T) { { name: "multiple conflicting proxies", app: func() types.Application { - app, _ := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "example.com"}) + app, err := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "example.com"}) + require.NoError(t, err) return app }(), proxyAddrs: []string{"example.com:443", "example.com:80"}, diff --git a/lib/services/fuzz_test.go b/lib/services/fuzz_test.go index 418219367480c..34306299c6046 100644 --- a/lib/services/fuzz_test.go +++ b/lib/services/fuzz_test.go @@ -22,11 +22,9 @@ import ( "strings" "testing" - "github.com/gravitational/trace" "github.com/stretchr/testify/require" "github.com/gravitational/teleport/api/types" - "github.com/gravitational/teleport/lib/utils" ) func FuzzParseRefs(f *testing.F) { @@ -128,36 +126,34 @@ func FuzzParserEvalBoolPredicate(f *testing.F) { } func FuzzValidateApp(f *testing.F) { - // seeds from unit tests - f.Add("http://localhost:8080", "web.example.com:443") // Valid proxy - f.Add("http://localhost:8080", "app.example.com:443") // Another valid proxy - f.Add("http://localhost:8080", "web.example.com:443,other.com:443") // Multiple proxies - f.Add("http://invalid-url", "web.example.com:443") // Invalid app URL - f.Add("http://localhost:8080", "xn--mnchen-3ya.de:443") // IDN in Punycode - f.Add("http://localhost:8080", "münchen.de:443") // IDN in Unicode - f.Add("http://localhost:8080", "example.com:443,example.com:80") // Multiple ports - f.Add("http://localhost:8080", "") // Empty proxy address - f.Add("http://localhost:8080", "example.com") // Proxy without port + f.Add("web.example.com:443", "app.example.com") // valid: different addresses + f.Add("", "app.example.com") // valid: empty proxy address + f.Add("proxy.example.com:443", "") // valid: empty app address + f.Add("example.com", "app.example.com") // valid: proxy without port + f.Add("web.example.com:443", "web.example.com") // conflict: same as proxy + f.Add("web.example.com:443", "web.example.com.") // conflict: trailing dot + f.Add("web.example.com:443", "WeB.ExAmPle.CoM") // conflict: case insensitive + f.Add("web.example.com:443,other.com:443", "other.com") // conflict: matches second proxy + f.Add("xn--mnchen-3ya.de:443", "münchen.de") // conflict: IDN + f.Add("münchen.de:443", "MünchEn.de") // conflict: IDN case insensitive + f.Add("example.com:443,example.com:80", "example.com") // conflict: multiple proxy ports - f.Fuzz(func(t *testing.T, appURI string, proxyAddrs string) { - app, err := types.NewAppV3(types.Metadata{Name: "fuzz-app"}, types.AppSpecV3{URI: appURI}) + f.Fuzz(func(t *testing.T, proxyPublicAddrs string, appPublicAddr string) { + // Create app with fuzzy public address. + app, err := types.NewAppV3(types.Metadata{Name: "fuzz-app"}, types.AppSpecV3{ + URI: "http://localhost:8080", + PublicAddr: appPublicAddr, + }) if err != nil { - // Skip invalid app URIs - return + t.Skip() } - proxyAddrList := strings.Split(proxyAddrs, ",") + proxyAddrList := strings.Split(proxyPublicAddrs, ",") mockProxyGetter := &mockProxyGetter{addrs: proxyAddrList} - for _, addr := range proxyAddrList { - if _, err := utils.ParseAddr(addr); err != nil { - // Skip invalid proxy addresses - return - } - } - - if err = ValidateApp(app, mockProxyGetter); err != nil { - require.True(t, trace.IsBadParameter(err) || trace.IsAccessDenied(err)) - } + // ValidateApp should never panic regardless of input. + require.NotPanics(t, func() { + _ = ValidateApp(app, mockProxyGetter) + }) }) } From f474783471e4279b53b4793b93ef6ff1ab34e7e6 Mon Sep 17 00:00:00 2001 From: Chris Thach Date: Tue, 11 Nov 2025 14:26:14 -0500 Subject: [PATCH 5/8] Apply suggestions from code review Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> --- lib/services/app.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/services/app.go b/lib/services/app.go index 1263e70db6f16..814c9c7d4a2ff 100644 --- a/lib/services/app.go +++ b/lib/services/app.go @@ -88,7 +88,7 @@ func ValidateApp(app types.Application, proxyGetter ProxyGetter) error { // dot to ensure consistent comparison. asciiAppHostname, err := idna.ToASCII(strings.TrimSuffix(appAddr.Host(), ".")) if err != nil { - return trace.WrapWithMessage(err, "app %q has an invalid IDN hostname %q", app.GetName(), appAddr.Host()) + return trace.Wrap(err, "app %q has an invalid IDN hostname %q", app.GetName(), appAddr.Host()) } proxyServers, err := proxyGetter.GetProxies() @@ -110,7 +110,7 @@ func ValidateApp(app types.Application, proxyGetter ProxyGetter) error { // trailing dot. asciiProxyHostname, err := idna.ToASCII(strings.TrimSuffix(proxyAddr.Host(), ".")) if err != nil { - return trace.WrapWithMessage(err, "proxy %q has an invalid IDN hostname %q", proxyServer.GetName(), proxyAddr) + return trace.Wrap(err, "proxy %q has an invalid IDN hostname %q", proxyServer.GetName(), proxyAddr) } // Compare the ASCII-normalized hostnames for equality, ignoring case. From 49d6c5500d5f6c7bf9b0b2bdd30b2a3a5949cea0 Mon Sep 17 00:00:00 2001 From: Chris Thach Date: Tue, 11 Nov 2025 14:33:59 -0500 Subject: [PATCH 6/8] Strip all trailing dots Signed-off-by: Chris Thach --- lib/services/app.go | 8 ++++---- lib/services/app_test.go | 10 ++++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/services/app.go b/lib/services/app.go index 814c9c7d4a2ff..10610c9ad8e23 100644 --- a/lib/services/app.go +++ b/lib/services/app.go @@ -85,8 +85,8 @@ func ValidateApp(app types.Application, proxyGetter ProxyGetter) error { } // Convert the application's public address hostname to its ASCII representation for comparison. Strip any trailing - // dot to ensure consistent comparison. - asciiAppHostname, err := idna.ToASCII(strings.TrimSuffix(appAddr.Host(), ".")) + // dots to ensure consistent comparison. + asciiAppHostname, err := idna.ToASCII(strings.TrimRight(appAddr.Host(), ".")) if err != nil { return trace.Wrap(err, "app %q has an invalid IDN hostname %q", app.GetName(), appAddr.Host()) } @@ -107,8 +107,8 @@ func ValidateApp(app types.Application, proxyGetter ProxyGetter) error { for _, proxyAddr := range proxyAddrs { // Also convert the proxy's public address hostname to its ASCII representation for comparison and strip any - // trailing dot. - asciiProxyHostname, err := idna.ToASCII(strings.TrimSuffix(proxyAddr.Host(), ".")) + // trailing dots. + asciiProxyHostname, err := idna.ToASCII(strings.TrimRight(proxyAddr.Host(), ".")) if err != nil { return trace.Wrap(err, "proxy %q has an invalid IDN hostname %q", proxyServer.GetName(), proxyAddr) } diff --git a/lib/services/app_test.go b/lib/services/app_test.go index 8788c071826c8..fafa57d7cd17d 100644 --- a/lib/services/app_test.go +++ b/lib/services/app_test.go @@ -79,6 +79,16 @@ func TestValidateApp(t *testing.T) { proxyAddrs: []string{"web.example.com:443"}, wantErr: "conflicts with the Teleport Proxy public address", }, + { + name: "public addr with multiple trailing dots matches proxy host", + app: func() types.Application { + app, err := types.NewAppV3(types.Metadata{Name: "app"}, types.AppSpecV3{URI: "http://localhost:8080", PublicAddr: "web.example.com..."}) + require.NoError(t, err) + return app + }(), + proxyAddrs: []string{"web.example.com:443"}, + wantErr: "conflicts with the Teleport Proxy public address", + }, { name: "public addr with mixed casing matches proxy host", app: func() types.Application { From bdb86ccd22a357c77c07078eb8b039a3acec6252 Mon Sep 17 00:00:00 2001 From: Chris Thach Date: Tue, 11 Nov 2025 14:46:40 -0500 Subject: [PATCH 7/8] NewAppV3 should never panic during fuzzing. Improve comments in fuzz test. Signed-off-by: Chris Thach --- lib/services/fuzz_test.go | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/lib/services/fuzz_test.go b/lib/services/fuzz_test.go index 34306299c6046..26b7072ae3f9d 100644 --- a/lib/services/fuzz_test.go +++ b/lib/services/fuzz_test.go @@ -132,6 +132,7 @@ func FuzzValidateApp(f *testing.F) { f.Add("example.com", "app.example.com") // valid: proxy without port f.Add("web.example.com:443", "web.example.com") // conflict: same as proxy f.Add("web.example.com:443", "web.example.com.") // conflict: trailing dot + f.Add("web.example.com:443", "web.example.com..") // conflict: multiple trailing dots f.Add("web.example.com:443", "WeB.ExAmPle.CoM") // conflict: case insensitive f.Add("web.example.com:443,other.com:443", "other.com") // conflict: matches second proxy f.Add("xn--mnchen-3ya.de:443", "münchen.de") // conflict: IDN @@ -139,20 +140,23 @@ func FuzzValidateApp(f *testing.F) { f.Add("example.com:443,example.com:80", "example.com") // conflict: multiple proxy ports f.Fuzz(func(t *testing.T, proxyPublicAddrs string, appPublicAddr string) { - // Create app with fuzzy public address. - app, err := types.NewAppV3(types.Metadata{Name: "fuzz-app"}, types.AppSpecV3{ - URI: "http://localhost:8080", - PublicAddr: appPublicAddr, - }) - if err != nil { - t.Skip() - } + // NewAppV3 and ValidateApp should never panic regardless of input. + require.NotPanics(t, func() { + // Create app with a fuzzy public address. + app, err := types.NewAppV3(types.Metadata{Name: "fuzz-app"}, types.AppSpecV3{ + URI: "http://localhost:8080", + PublicAddr: appPublicAddr, + }) + if err != nil { + // Fuzzing may produce invalid values that fail to create an App. + // If so, skip this iteration since the test cannot continue. + t.Skip("skipping invalid app spec") + } - proxyAddrList := strings.Split(proxyPublicAddrs, ",") - mockProxyGetter := &mockProxyGetter{addrs: proxyAddrList} + proxyAddrList := strings.Split(proxyPublicAddrs, ",") + mockProxyGetter := &mockProxyGetter{addrs: proxyAddrList} - // ValidateApp should never panic regardless of input. - require.NotPanics(t, func() { + // Validate the app against the mock proxy addresses. _ = ValidateApp(app, mockProxyGetter) }) }) From e5f3fe03447f0f0a62f9a15722d03ba3b29aea35 Mon Sep 17 00:00:00 2001 From: Chris Thach Date: Tue, 11 Nov 2025 15:15:36 -0500 Subject: [PATCH 8/8] Clarify app spec validated in CheckAndSetDefaults Signed-off-by: Chris Thach --- lib/services/app.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/services/app.go b/lib/services/app.go index 10610c9ad8e23..024ceacda0373 100644 --- a/lib/services/app.go +++ b/lib/services/app.go @@ -77,8 +77,9 @@ func ValidateApp(app types.Application, proxyGetter ProxyGetter) error { return nil } - // It is assumed that the app's public address has already been validated to be a valid address format during app - // resource validation. The remainder of this function focuses on detecting conflicts with proxy public addresses. + // The app's spec has already been validated in CheckAndSetDefaults, so we can assume the public address is a valid + // address. The remainder of this function focuses on detecting conflicts with proxy public addresses because the + // proxy addresses are not part of the app spec and need to be fetched separately. appAddr, err := utils.ParseAddr(app.GetPublicAddr()) if err != nil { return trace.Wrap(err)