diff --git a/spiffeid/id.go b/spiffeid/id.go index f4e02eee7..ed4a5e2f6 100644 --- a/spiffeid/id.go +++ b/spiffeid/id.go @@ -52,31 +52,34 @@ func FromString(id string) (ID, error) { switch { case id == "": return ID{}, errEmpty - case !strings.HasPrefix(id, schemePrefix): + case len(id) < schemePrefixLen: + return ID{}, errWrongScheme + case !strings.EqualFold(id[:schemePrefixLen], schemePrefix): return ID{}, errWrongScheme } pathidx := schemePrefixLen - for ; pathidx < len(id); pathidx++ { - c := id[pathidx] - if c == '/' { - break - } - if !isValidTrustDomainChar(c) { - return ID{}, errBadTrustDomainChar - } + for pathidx < len(id) && id[pathidx] != '/' { + pathidx++ } if pathidx == schemePrefixLen { return ID{}, errMissingTrustDomain } + tdNorm := strings.ToLower(id[schemePrefixLen:pathidx]) + for i := 0; i < len(tdNorm); i++ { + if !isValidTrustDomainChar(tdNorm[i]) { + return ID{}, errBadTrustDomainChar + } + } + if err := ValidatePath(id[pathidx:]); err != nil { return ID{}, err } return ID{ - id: id, + id: schemePrefix + tdNorm + id[pathidx:], pathidx: pathidx, }, nil } diff --git a/spiffeid/id_test.go b/spiffeid/id_test.go index 0c528388e..fa70e083e 100644 --- a/spiffeid/id_test.go +++ b/spiffeid/id_test.go @@ -31,7 +31,7 @@ var ( ".", "-", "_", ) - tdChars = mergeSets(lowerAlpha, numbers, special) + tdChars = mergeSets(lowerAlpha, upperAlpha, numbers, special) pathChars = mergeSets(lowerAlpha, upperAlpha, numbers, special) ) @@ -109,7 +109,14 @@ func TestFromString(t *testing.T) { t.Run("reject bad scheme", func(t *testing.T) { assertFail(t, "s", "scheme is missing or invalid") assertFail(t, "spiffe:/", "scheme is missing or invalid") - assertFail(t, "Spiffe://", "scheme is missing or invalid") + }) + t.Run("allow case-insensitive scheme", func(t *testing.T) { + assertOK(t, "SpIfFe://trustdomain/path", td, "/path") + assertOK(t, "SPIFFE://trustdomain/path", td, "/path") + }) + t.Run("normalize trust domain to lowercase", func(t *testing.T) { + assertOK(t, "spiffe://TrUsTdOmAiN/path", td, "/path") + assertOK(t, "SpIfFe://TrUsTdOmAiN/path", td, "/path") }) t.Run("reject missing trust domain", func(t *testing.T) { @@ -147,6 +154,34 @@ func TestFromString(t *testing.T) { assertFail(t, "spiffe://%62%61%64/path", "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") assertFail(t, "spiffe://trustdomain/%62%61%64", "path segment characters are limited to letters, numbers, dots, dashes, and underscores") }) + + t.Run("reject query and fragment", func(t *testing.T) { + assertFail(t, "spiffe://trustdomain/path?x=1", "path segment characters are limited to letters, numbers, dots, dashes, and underscores") + assertFail(t, "spiffe://trustdomain/path#frag", "path segment characters are limited to letters, numbers, dots, dashes, and underscores") + assertFail(t, "spiffe://trustdomain?x=1", "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") + assertFail(t, "spiffe://trustdomain#frag", "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") + }) + + t.Run("reject userinfo and port", func(t *testing.T) { + assertFail(t, "spiffe://user@trustdomain/path", "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") + assertFail(t, "spiffe://user:pass@trustdomain/path", "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") + assertFail(t, "spiffe://trustdomain:8080/path", "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") + assertFail(t, "spiffe://1.2.3.4:443/path", "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") + }) + + t.Run("allow ipv4 and reject ipv6 trust domain authorities", func(t *testing.T) { + assertOK(t, "spiffe://1.2.3.4/service", spiffeid.RequireTrustDomainFromString("1.2.3.4"), "/service") + assertFail(t, "spiffe://[::1]/service", "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") + assertFail(t, "spiffe://[2001:db8::1]/service", "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") + }) + + t.Run("allow non-DNS-shaped trust domains", func(t *testing.T) { + assertOK(t, "spiffe://example..org/service", spiffeid.RequireTrustDomainFromString("example..org"), "/service") + assertOK(t, "spiffe://.example.org/service", spiffeid.RequireTrustDomainFromString(".example.org"), "/service") + assertOK(t, "spiffe://example.org./service", spiffeid.RequireTrustDomainFromString("example.org."), "/service") + assertOK(t, "spiffe://-example.org/service", spiffeid.RequireTrustDomainFromString("-example.org"), "/service") + assertOK(t, "spiffe://example-.org/service", spiffeid.RequireTrustDomainFromString("example-.org"), "/service") + }) } func TestFromURI(t *testing.T) { @@ -168,9 +203,10 @@ func TestFromURI(t *testing.T) { assertOK("spiffe://trustdomain") assertOK("spiffe://trustdomain/path") + assertOK("SpIfFe://trustdomain/path") + assertOK("spiffe://TrUsTdOmAiN/path") assertFail(&url.URL{}, `cannot be empty`) - assertFail(&url.URL{Scheme: "SPIFFE", Host: "trustdomain"}, `scheme is missing or invalid`) assertFail(parseURI("spiffe://trust$domain"), `trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores`) assertFail(parseURI("spiffe://trustdomain/path$"), `path segment characters are limited to letters, numbers, dots, dashes, and underscores`) } @@ -461,6 +497,30 @@ func TestIDTextUnmarshaler(t *testing.T) { require.Equal(t, "spiffe://trustdomain/path", s.ID.String()) } +func TestIDCanonicalizationAndEquality(t *testing.T) { + t.Run("scheme and trust domain are canonicalized", func(t *testing.T) { + mixedCase := spiffeid.RequireFromString("SPIFFE://EXAMPLE.ORG/MyService") + require.Equal(t, "spiffe://example.org/MyService", mixedCase.String()) + }) + + t.Run("scheme and trust domain case variants compare equal", func(t *testing.T) { + base := spiffeid.RequireFromString("spiffe://example.org/service") + for _, variant := range []string{ + "SPIFFE://example.org/service", + "spiffe://EXAMPLE.ORG/service", + "SPIFFE://EXAMPLE.ORG/service", + } { + require.Equal(t, base, spiffeid.RequireFromString(variant)) + } + }) + + t.Run("path remains case sensitive", func(t *testing.T) { + lower := spiffeid.RequireFromString("spiffe://example.org/service") + upper := spiffeid.RequireFromString("spiffe://example.org/Service") + require.NotEqual(t, lower, upper) + }) +} + func BenchmarkIDFromString(b *testing.B) { s := "spiffe://trustdomain/path" for n := 0; n < b.N; n++ { diff --git a/spiffeid/path_test.go b/spiffeid/path_test.go index 6e61c7bd1..31a51bbcb 100644 --- a/spiffeid/path_test.go +++ b/spiffeid/path_test.go @@ -55,4 +55,28 @@ func TestValidatePathSegment(t *testing.T) { t.Run("valid segment", func(t *testing.T) { require.NoError(t, ValidatePathSegment("a")) }) + t.Run("valid mixed segment charset", func(t *testing.T) { + require.NoError(t, ValidatePathSegment("abc-_.Z9")) + }) + t.Run("reject percent-encoded segment text", func(t *testing.T) { + require.ErrorIs(t, ValidatePathSegment("%61pi"), errBadPathSegmentChar) + }) +} + +func TestValidatePath(t *testing.T) { + t.Run("reject root path only", func(t *testing.T) { + require.ErrorIs(t, ValidatePath("/"), errTrailingSlash) + }) + t.Run("reject trailing slash", func(t *testing.T) { + require.ErrorIs(t, ValidatePath("/foo/"), errTrailingSlash) + }) + t.Run("reject empty segment in middle", func(t *testing.T) { + require.ErrorIs(t, ValidatePath("/foo//bar"), errEmptySegment) + }) +} + +func TestJoinPathSegmentsHierarchical(t *testing.T) { + path, err := JoinPathSegments("ns", "default", "sa", "web") + require.NoError(t, err) + require.Equal(t, "/ns/default/sa/web", path) } diff --git a/spiffeid/require_test.go b/spiffeid/require_test.go index e2f3d85b2..3ed70f514 100644 --- a/spiffeid/require_test.go +++ b/spiffeid/require_test.go @@ -73,8 +73,9 @@ func TestRequireTrustDomainFromString(t *testing.T) { td := spiffeid.RequireTrustDomainFromString("spiffe://trustdomain/path") assert.Equal(t, "trustdomain", td.String()) }) - assert.Panics(t, func() { - spiffeid.RequireTrustDomainFromString("spiffe://TRUSTDOMAIN/path") + assert.NotPanics(t, func() { + td := spiffeid.RequireTrustDomainFromString("spiffe://TRUSTDOMAIN/path") + assert.Equal(t, "trustdomain", td.String()) }) } diff --git a/spiffeid/trustdomain.go b/spiffeid/trustdomain.go index 467ed5e6c..085150b30 100644 --- a/spiffeid/trustdomain.go +++ b/spiffeid/trustdomain.go @@ -29,12 +29,13 @@ func TrustDomainFromString(idOrName string) (TrustDomain, error) { } return id.TrustDomain(), nil default: - for i := 0; i < len(idOrName); i++ { - if !isValidTrustDomainChar(idOrName[i]) { + name := strings.ToLower(idOrName) + for i := 0; i < len(name); i++ { + if !isValidTrustDomainChar(name[i]) { return TrustDomain{}, errBadTrustDomainChar } } - return TrustDomain{name: idOrName}, nil + return TrustDomain{name: name}, nil } } diff --git a/spiffeid/trustdomain_test.go b/spiffeid/trustdomain_test.go index 90266a177..6d2e79c23 100644 --- a/spiffeid/trustdomain_test.go +++ b/spiffeid/trustdomain_test.go @@ -53,6 +53,33 @@ func TestTrustDomainFromString(t *testing.T) { assertFail(t, "spiffe://%F0%9F%A4%AF/path", "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") assertFail(t, "spiffe://trustdomain/%F0%9F%A4%AF", "path segment characters are limited to letters, numbers, dots, dashes, and underscores") }) + t.Run("allow case-insensitive scheme and normalize trust domain", func(t *testing.T) { + assertOK(t, "SpIfFe://trustdomain/path", td) + assertOK(t, "spiffe://TrUsTdOmAiN/path", td) + assertOK(t, "TrUsTdOmAiN", td) + }) + + t.Run("allow ipv4 trust domain", func(t *testing.T) { + expected := spiffeid.RequireTrustDomainFromString("1.2.3.4") + assertOK(t, "1.2.3.4", expected) + assertOK(t, "spiffe://1.2.3.4/service", expected) + }) + + t.Run("reject userinfo, port, and ipv6 in authority", func(t *testing.T) { + assertFail(t, "spiffe://user@trustdomain/path", "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") + assertFail(t, "spiffe://user:pass@trustdomain/path", "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") + assertFail(t, "spiffe://trustdomain:8080/path", "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") + assertFail(t, "spiffe://[::1]/service", "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") + assertFail(t, "spiffe://[2001:db8::1]/service", "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") + }) + + t.Run("allow non-DNS-shaped trust domains", func(t *testing.T) { + assertOK(t, "example..org", spiffeid.RequireTrustDomainFromString("example..org")) + assertOK(t, ".example.org", spiffeid.RequireTrustDomainFromString(".example.org")) + assertOK(t, "example.org.", spiffeid.RequireTrustDomainFromString("example.org.")) + assertOK(t, "-example.org", spiffeid.RequireTrustDomainFromString("-example.org")) + assertOK(t, "example-.org", spiffeid.RequireTrustDomainFromString("example-.org")) + }) // Go all the way through 255, which ensures we reject UTF-8 appropriately for i := 0; i < 256; i++ { @@ -91,9 +118,10 @@ func TestTrustDomainFromURI(t *testing.T) { assertOK("spiffe://trustdomain") assertOK("spiffe://trustdomain/path") + assertOK("SpIfFe://trustdomain/path") + assertOK("spiffe://TrUsTdOmAiN/path") assertFail(&url.URL{}, `cannot be empty`) - assertFail(&url.URL{Scheme: "SPIFFE", Host: "trustdomain"}, `scheme is missing or invalid`) assertFail(parseURI("spiffe://trust$domain"), `trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores`) assertFail(parseURI("spiffe://trustdomain/path$"), `path segment characters are limited to letters, numbers, dots, dashes, and underscores`) } @@ -158,7 +186,7 @@ func TestTrustDomainTextUnmarshaler(t *testing.T) { require.NoError(t, err) require.Zero(t, s.TrustDomain) - err = json.Unmarshal([]byte(`{"trustDomain": "BAD"}`), &s) + err = json.Unmarshal([]byte(`{"trustDomain": "bad$"}`), &s) require.EqualError(t, err, "trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores") require.Zero(t, s.TrustDomain)