mirror of
https://github.com/gravitational/teleport.git
synced 2026-08-31 01:50:50 +08:00
005a2aa755
* app: Consolidate app name and public_addr validation Consolidate app `name` and `public_addr` validation to fix six issues where Teleport rejects valid app names, accepts invalid ones, or crashes on startup. Preserve backwards compatibility for dynamic and heartbeat paths: clusters with mixed-case records already in the backend keep working. Strict write validation (admin writes + static config): - App name, static: `IsDNS1123Label` - no dots, max 63 chars. - App name, dynamic: `IsDNS1123Subdomain` - dots OK for AWS-OIDC, max 253 chars. - `public_addr`: `IsDNS1123Subdomain` - dots OK (it is a hostname), max 253 chars. All three k8s validators also enforce lowercase. RFC 1123 itself does not -- lowercase is a Teleport convention for interop with Kubernetes object names. `ValidateApp` is the single write-time validator, called from `CreateApp`, `UpdateApp`, `UpsertApplicationServer`, the inventory control stream, and app service startup. Heartbeats run `NormalizeAppServerForHeartbeat` first, which lowercases both `name` and `public_addr` and strips URL schemes and ports, so heartbeats from older agents end up stored lowercase. Link: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/ * app: Apply review feedback Address review nits from the PR: - Use t.Context() in new tests; add t.Cleanup for client Close. - Drop double Close() in inventory controller tests. - Use proto getters in identitycenter to avoid panic on nil Spec. - Replace multi-hyphen regex with strings.ReplaceAll loop. - Collapse mockRolesAnywhereClient.profiles into pages. - Retitle and flesh out the CHANGELOG entry. - Revert accidental enterprise submodule bump. * app: Apply review feedback Use proto getters for the remaining `acct.Spec.*` accesses in `IdentityCenterAccountToAppServer` so the function stays nil-safe end to end. Fix grammar in the app name doc bullet ("as is a duplicate name" -> "as are duplicate names"). * app: Preserve Identity Center launch URL Identity Center accounts surface in the unified-resource cache as synthetic AppServers and never traverse `ValidateApp` / `ValidateAppServer`, so the DNS-1123 normalization the previous commits introduced in `IdentityCenterAccountToAppServer` had no write path to satisfy. Keep the raw StartUrl in both `URI` and `PublicAddr`: the web Launch button builds the SSO launch href as `${publicAddr}&role_name=...`, so stripping scheme, path, port, or case breaks every Identity Center app launch. Drop the matching lowercase on `metadata.Name` for the same reason, and restore the delete handler in `processEventsAndUpdateCurrent` to its pre-PR shape (rebuild a `KindAppServer` header from the IC account event, no lowercase). Rewrite `TestIdentityCenterAccountToAppServer` to assert StartUrl passes through verbatim; remove the `ValidateApp` round-trip assertion, which was checking a path the synthetic AppServer never takes in real callers. * app: Apply review feedback * app: Apply review feedback Tighten the "Stricter application validation" CHANGELOG entry to focus on the three backwards-incompatible cases: static-config DNS label, unrecoverable public_addr, and duplicate names within one agent's teleport.yaml. Expand the write-path list to include the Terraform provider, Kubernetes operator, and direct API calls. Note that multi-agent same-name heartbeats remain supported for load balancing. * app: Allow underscores in dynamic app names Swap IsDNS1123Subdomain for IsDNS1123SubdomainWithUnderscore in ValidateApp, the required_apps loop, and ValidateAppServer. The strict variant rejected snake_case names that Teleport Cloud and self-hosted clusters both accept today, breaking the Terraform provider tests as the canary. Static yaml (lib/service/servicecfg/app.go) remains strict, since the rule there is a deliberate breaking change. public_addr also stays strict, since it is the actual hostname used for routing. Document the new rule and the strict-TLS-client caveat in the "Application name" section of connecting-apps.mdx.
164 lines
5.3 KiB
Go
164 lines
5.3 KiB
Go
/*
|
|
* Teleport
|
|
* Copyright (C) 2023 Gravitational, Inc.
|
|
*
|
|
* This program is free software: you can redistribute it and/or modify
|
|
* it under the terms of the GNU Affero General Public License as published by
|
|
* the Free Software Foundation, either version 3 of the License, or
|
|
* (at your option) any later version.
|
|
*
|
|
* This program is distributed in the hope that it will be useful,
|
|
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
|
* GNU Affero General Public License for more details.
|
|
*
|
|
* You should have received a copy of the GNU Affero General Public License
|
|
* along with this program. If not, see <http://www.gnu.org/licenses/>.
|
|
*/
|
|
|
|
package services
|
|
|
|
import (
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/require"
|
|
|
|
"github.com/gravitational/teleport/api/types"
|
|
)
|
|
|
|
func FuzzParseRefs(f *testing.F) {
|
|
// seeds from unit test examples
|
|
f.Add("lock")
|
|
f.Add("integration")
|
|
f.Add("integration/00124f1e-d70e-413e-9b20-9b2d4c97e10c")
|
|
f.Add("integration/unknown")
|
|
f.Add("integration/myawsint")
|
|
f.Add("app")
|
|
f.Add("app/appB")
|
|
f.Add("db_server")
|
|
f.Add("db_server/example")
|
|
f.Add("db")
|
|
f.Add("db/example")
|
|
f.Add("db_service")
|
|
f.Add("db_service/7af76d49-b747-4bc1-b43d-c6dd457c229e")
|
|
f.Add("db_service/unknown")
|
|
// other seeds
|
|
f.Add("foo,bar")
|
|
f.Add("foo\\,bar/foobar")
|
|
|
|
f.Fuzz(func(t *testing.T, refs string) {
|
|
require.NotPanics(t, func() {
|
|
ParseRefs(refs)
|
|
})
|
|
})
|
|
}
|
|
|
|
func FuzzParserEvalBoolPredicate(f *testing.F) {
|
|
// seeds from unit tests
|
|
f.Add("name == \"4a6t1q1zcsq97q\"")
|
|
f.Add("labels.env == \"test\"")
|
|
f.Add("contains(reviewer.roles,\"dev\")")
|
|
f.Add("!contains(reviewer.traits[\"teams\"],\"staging-admin\")")
|
|
f.Add("equals(request.reason,review.reason)")
|
|
f.Add("contains(reviewer.roles, \"admin\")")
|
|
f.Add("equals(fully.fake.path,\"should-fail\")")
|
|
f.Add("fakefunc(reviewer.roles,\"some-role\")")
|
|
f.Add("equals(\"too\",\"many\",\"params\")")
|
|
f.Add("contains(\"missing-param\")")
|
|
f.Add("&& missing-left")
|
|
f.Add("labels.env.toomanyfield")
|
|
f.Add("exists(labels.undefined)")
|
|
f.Add("name.toomanyfield")
|
|
f.Add("!name")
|
|
f.Add("name ==")
|
|
f.Add("equals(labels[\"env\"], \"wrong-value\")")
|
|
f.Add("name ||")
|
|
f.Add("&&")
|
|
f.Add("||")
|
|
f.Add("|")
|
|
f.Add("&")
|
|
f.Add("!")
|
|
f.Add(".")
|
|
f.Add("!exists(labels.env)")
|
|
f.Add("name &&")
|
|
f.Add("name &")
|
|
f.Add("name |")
|
|
f.Add("search(\"mac\", \"not-found\")")
|
|
f.Add("hasPrefix(name, \"x\")")
|
|
f.Add("search(\"mac\")")
|
|
f.Add("equals()")
|
|
f.Add("exists()")
|
|
f.Add("search(1,2)")
|
|
f.Add("\"just-string\"")
|
|
f.Add("hasPrefix(1, 2)")
|
|
f.Add("hasPrefix(name, \"too\", \"many\")")
|
|
f.Add("hasPrefix(name, 1)")
|
|
f.Add("search()")
|
|
f.Add("resource.metadata.labels[\"env\"] == \"prod\"")
|
|
f.Add("(exists(labels.env) || exists(labels.os)) && labels.os != \"mac\"")
|
|
f.Add("search(\"does\", \"not\", \"exist\") || resource.spec.addr == \"_\" || labels.version == \"v8\"")
|
|
|
|
f.Fuzz(func(t *testing.T, expr string) {
|
|
resource, err := types.NewServerWithLabels("test-name", types.KindNode, types.ServerSpecV2{
|
|
Hostname: "test-hostname",
|
|
Addr: "test-addr",
|
|
CmdLabels: map[string]types.CommandLabelV2{
|
|
"version": {
|
|
Result: "v8",
|
|
},
|
|
},
|
|
}, map[string]string{
|
|
"env": "prod",
|
|
"os": "mac",
|
|
})
|
|
require.NoError(t, err)
|
|
|
|
require.NotPanics(t, func() {
|
|
parser, err := NewResourceExpression(expr)
|
|
if err != nil {
|
|
return
|
|
}
|
|
|
|
parser.Evaluate(resource)
|
|
})
|
|
})
|
|
}
|
|
|
|
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 stripped before compare
|
|
f.Add("web.example.com:443", "web.example.com..") // rejected by shape check: multiple trailing dots
|
|
f.Add("web.example.com:443", "WeB.ExAmPle.CoM") // rejected by shape check: mixed case
|
|
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") // rejected by shape check: IDN Unicode
|
|
f.Add("münchen.de:443", "MünchEn.de") // rejected by shape check: mixed case + IDN
|
|
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)
|
|
})
|
|
})
|
|
}
|