diff --git a/lib/auth/grpcserver_test.go b/lib/auth/grpcserver_test.go index 1d54edbd39eda..0141b800bc823 100644 --- a/lib/auth/grpcserver_test.go +++ b/lib/auth/grpcserver_test.go @@ -3637,7 +3637,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) { @@ -3651,7 +3651,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.`)) }) } @@ -3777,7 +3777,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.`)) }) } diff --git a/lib/services/app.go b/lib/services/app.go index 6bb83bf40d8e5..240414b4bb939 100644 --- a/lib/services/app.go +++ b/lib/services/app.go @@ -24,11 +24,11 @@ import ( "fmt" "net/url" "os" - "slices" "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" @@ -63,31 +63,54 @@ type Applications interface { // ValidateApp validates the Application resource. func ValidateApp(app types.Application, proxyGetter ProxyGetter) error { + // If no public address is set, there's nothing to validate. + if app.GetPublicAddr() == "" { + return nil + } + + // 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) + } + + // Convert the application's public address hostname to its ASCII representation for comparison. Strip any trailing + // 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()) + } + + 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. - if app.GetPublicAddr() != "" { - proxyServers, err := proxyGetter.GetProxies() + 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()) + for _, proxyAddr := range proxyAddrs { + // Also convert the proxy's public address hostname to its ASCII representation for comparison and strip any + // trailing dots. + asciiProxyHostname, err := idna.ToASCII(strings.TrimRight(proxyAddr.Host(), ".")) if err != nil { - return trace.Wrap(err) + return trace.Wrap(err, "proxy %q has an invalid IDN hostname %q", proxyServer.GetName(), proxyAddr) } - if slices.ContainsFunc( - proxyAddrs, - func(proxyAddr utils.NetAddr) bool { - return app.GetPublicAddr() == proxyAddr.Host() - }, - ) { + // Compare the ASCII-normalized hostnames for equality, ignoring case. + 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. "+ - "Refer to https://goteleport.com/docs/enroll-resources/application-access/guides/connecting-apps/.", + "Refer to https://goteleport.com/docs/enroll-resources/application-access/guides/connecting-apps/#customize-public-address.", app.GetName(), app.GetPublicAddr(), ) diff --git a/lib/services/app_test.go b/lib/services/app_test.go index 65b58133f1282..1835d0db61471 100644 --- a/lib/services/app_test.go +++ b/lib/services/app_test.go @@ -39,7 +39,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"}, @@ -47,7 +48,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"}, @@ -55,7 +57,38 @@ 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 trailing dot 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 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 { + 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"}, @@ -64,12 +97,71 @@ 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"}, wantErr: "conflicts with the Teleport Proxy public address", }, + { + name: "public addr with IDN matches proxy host", + app: func() types.Application { + 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"}, + 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, 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"}, + }, + { + name: "IDN with mixed case matches proxy host", + app: func() types.Application { + 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"}, + wantErr: "conflicts with the Teleport Proxy public address", + }, + { + name: "IDN with subdomains matches proxy host", + app: func() types.Application { + 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"}, + wantErr: "conflicts with the Teleport Proxy public address", + }, + { + name: "empty proxy addrs", + app: func() types.Application { + 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{}, + }, + { + name: "multiple conflicting proxies", + app: func() types.Application { + 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"}, + wantErr: "conflicts with the Teleport Proxy public address", + }, } for _, tt := range tests { diff --git a/lib/services/fuzz_test.go b/lib/services/fuzz_test.go index 0aa3679e5e9e2..26b7072ae3f9d 100644 --- a/lib/services/fuzz_test.go +++ b/lib/services/fuzz_test.go @@ -19,6 +19,7 @@ package services import ( + "strings" "testing" "github.com/stretchr/testify/require" @@ -123,3 +124,40 @@ func FuzzParserEvalBoolPredicate(f *testing.F) { }) }) } + +func FuzzValidateApp(f *testing.F) { + 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: 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 + 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, proxyPublicAddrs string, appPublicAddr string) { + // 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} + + // Validate the app against the mock proxy addresses. + _ = ValidateApp(app, mockProxyGetter) + }) + }) +}