Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 35 additions & 4 deletions service/internal/auth/authz/casbin/casbin.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"errors"
"fmt"
"log/slog"
"net/url"
"sort"
"strings"

Expand Down Expand Up @@ -45,6 +46,12 @@ const (
kvPairParts = 2
)

var dimensionValueEscaper = strings.NewReplacer(
"%", "%25",
"&", "%26",
"*", "%2A",
)

func init() {
// Register the Casbin authorizer factory
authz.RegisterFactory("casbin", NewAuthorizer)
Expand Down Expand Up @@ -555,15 +562,27 @@ func serializeDimensions(ctx *authz.ResolverContext) (string, error) {
}
sort.Strings(keys)

// Build canonical string
// Build canonical string, percent-encoding only the characters that conflict
// with dimension parsing. '%' must be escaped first because it introduces an
// escape sequence, and '&' must be escaped because it separates key-value
// pairs. '*' is escaped because raw '*' is the policy wildcard. '=' can remain
// readable because pair parsing splits on the first '='.
parts := make([]string, 0, len(keys))
for _, k := range keys {
parts = append(parts, fmt.Sprintf("%s=%s", k, allDims[k]))
parts = append(parts, fmt.Sprintf("%s=%s", k, escapeDimensionValue(allDims[k])))
}

return strings.Join(parts, "&"), nil
}

func escapeDimensionValue(value string) string {
return dimensionValueEscaper.Replace(value)
}

func unescapeDimensionValue(value string) (string, error) {
return url.PathUnescape(value)
}

// isValidDimensionKey reports whether a dimension key can be safely serialized.
func isValidDimensionKey(key string) bool {
if key == "" {
Expand Down Expand Up @@ -636,6 +655,14 @@ func dimensionMatch(reqDims, policyDims string) bool {
if !isValidDimensionKey(key) {
return false
}
policyWildcard := policyVal == "*"
if !policyWildcard {
unescapedVal, err := unescapeDimensionValue(policyVal)
if err != nil {
return false
}
policyVal = unescapedVal
}

reqVal, exists := reqMap[key]
if !exists {
Expand All @@ -644,7 +671,7 @@ func dimensionMatch(reqDims, policyDims string) bool {
}

// Wildcard matches any value
if policyVal != "*" && policyVal != reqVal {
if !policyWildcard && policyVal != reqVal {
return false
}
}
Expand All @@ -671,7 +698,11 @@ func parseDimensions(dims string) (map[string]string, bool) {
if !isValidDimensionKey(kv[0]) {
return nil, false
}
result[kv[0]] = kv[1]
unescapedVal, err := unescapeDimensionValue(kv[1])
if err != nil {
return nil, false
}
result[kv[0]] = unescapedVal
}
return result, true
}
220 changes: 220 additions & 0 deletions service/internal/auth/authz/casbin/casbin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,226 @@ func TestSerializeDimensions(t *testing.T) {
}
}

// Test dimension value injection prevention via URL-encoding
func TestSerializeDimensions_InjectionPrevention(t *testing.T) {
tests := []struct {
name string
ctx *authz.ResolverContext
expected string
}{
{
name: "value with ampersand is safely encoded",
ctx: &authz.ResolverContext{
Resources: []*authz.ResolverResource{
{"kas_uri": "https://kas.example.com?foo=bar&second_dim=injected"},
},
},
// The '&' in the value must be percent-encoded so parseDimensions
// sees only one key-value pair, not two.
expected: "kas_uri=https://kas.example.com?foo=bar%26second_dim=injected",
},
{
name: "value with equals sign is unchanged",
ctx: &authz.ResolverContext{
Resources: []*authz.ResolverResource{
{"kas_uri": "https://kas.example.com?key=value"},
},
},
expected: "kas_uri=https://kas.example.com?key=value",
},
{
name: "plain URI with colon and slashes is unchanged",
ctx: &authz.ResolverContext{
Resources: []*authz.ResolverResource{
{"kas_uri": "https://kas.example.com"},
},
},
expected: "kas_uri=https://kas.example.com",
},
{
name: "value with percent sign is safely encoded",
ctx: &authz.ResolverContext{
Resources: []*authz.ResolverResource{
{"label": "done%complete"},
},
},
expected: "label=done%25complete",
},
{
name: "wildcard token value is safely encoded",
ctx: &authz.ResolverContext{
Resources: []*authz.ResolverResource{
{"label": "*"},
},
},
expected: "label=%2A",
},
{
name: "plain value without special chars is unchanged",
ctx: &authz.ResolverContext{
Resources: []*authz.ResolverResource{
{"namespace": "hr"},
},
},
expected: "namespace=hr",
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
result, err := serializeDimensions(tc.ctx)
require.NoError(t, err)
assert.Equal(t, tc.expected, result)
})
}
}

// TestParseDimensions_RoundTrip verifies that values containing special characters
// survive a serialize→parse round-trip without being misinterpreted.
func TestParseDimensions_RoundTrip(t *testing.T) {
tests := []struct {
name string
input map[string]string
}{
{
name: "value with ampersand round-trips correctly",
input: map[string]string{"kas_uri": "https://kas.example.com?foo=bar&second_dim=injected"},
},
{
name: "value with equals sign round-trips correctly",
input: map[string]string{"kas_uri": "https://kas.example.com?key=value"},
},
{
name: "full URI with query string round-trips correctly",
input: map[string]string{"kas_uri": "https://kas.example.com?foo=bar&baz=qux"},
},
{
name: "value with literal plus round-trips correctly",
input: map[string]string{"kas_uri": "https://kas.example.com/path+plus?token=a+b"},
},
{
name: "value with space round-trips correctly",
input: map[string]string{"display_name": "first last"},
},
{
name: "value with percent round-trips correctly",
input: map[string]string{"label": "done%complete"},
},
{
name: "wildcard token value round-trips correctly",
input: map[string]string{"label": "*"},
},
{
name: "plain value round-trips correctly",
input: map[string]string{"namespace": "hr"},
},
{
name: "multiple dimensions including URI round-trip correctly",
input: map[string]string{"kas_uri": "https://kas.example.com?x=1&y=2", "namespace": "hr"},
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Build a ResolverContext from the input map
res := authz.ResolverResource(tc.input)
ctx := &authz.ResolverContext{
Resources: []*authz.ResolverResource{&res},
}

serialized, err := serializeDimensions(ctx)
require.NoError(t, err)

parsed, ok := parseDimensions(serialized)
require.True(t, ok, "parseDimensions must succeed on serialized output")
assert.Equal(t, tc.input, parsed, "round-trip must preserve original values exactly")
})
}
}

// TestDimensionMatch_WithURIValues verifies that dimensionMatch works correctly
// when dimension values are URIs (which get URL-encoded by serializeDimensions).
func TestDimensionMatch_WithURIValues(t *testing.T) {
tests := []struct {
name string
input map[string]string
policyDims string
expected bool
}{
{
name: "serialized URI matches policy with same URI",
input: map[string]string{"kas_uri": "https://kas.example.com"},
policyDims: "kas_uri=https://kas.example.com",
expected: true,
},
Comment thread
c-r33d marked this conversation as resolved.
{
name: "serialized URI with query string matches escaped policy URI",
input: map[string]string{"kas_uri": "https://kas.example.com?foo=bar&baz=qux"},
policyDims: "kas_uri=" + escapeDimensionValue("https://kas.example.com?foo=bar&baz=qux"),
expected: true,
},
{
name: "serialized URI with literal plus matches escaped policy URI",
input: map[string]string{"kas_uri": "https://kas.example.com/path+plus?token=a+b"},
policyDims: "kas_uri=" + escapeDimensionValue("https://kas.example.com/path+plus?token=a+b"),
expected: true,
},
{
name: "URI value with query string does not match policy for base URI only",
input: map[string]string{"kas_uri": "https://kas.example.com?foo=bar"},
policyDims: "kas_uri=https://kas.example.com",
// The query string makes the URIs different; policy requires exact match.
expected: false,
},
{
name: "escaped literal star policy value does not act as wildcard",
input: map[string]string{"kas_uri": "https://kas.example.com"},
policyDims: "kas_uri=" + escapeDimensionValue("*"),
expected: false,
},
{
name: "serialized literal star matches escaped policy value",
input: map[string]string{"label": "*"},
policyDims: "label=" + escapeDimensionValue("*"),
expected: true,
},
{
name: "serialized literal star matches raw wildcard policy value",
input: map[string]string{"label": "*"},
policyDims: "label=*",
expected: true,
},
{
name: "injected extra dimension does not satisfy a different policy key",
input: map[string]string{"kas_uri": "https://kas.example.com?foo=bar&second_dim=injected"},
policyDims: "second_dim=injected",
// The injected 'second_dim' must NOT appear as a separate dimension.
expected: false,
},
{
name: "wildcard policy matches URI dimension",
input: map[string]string{"kas_uri": "https://kas.example.com"},
policyDims: "*",
expected: true,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Serialize the input dimensions as the authorizer would
res := authz.ResolverResource(tc.input)
ctx := &authz.ResolverContext{
Resources: []*authz.ResolverResource{&res},
}
serialized, err := serializeDimensions(ctx)
require.NoError(t, err)

result := dimensionMatch(serialized, tc.policyDims)
assert.Equal(t, tc.expected, result, "dimensionMatch(%q, %q)", serialized, tc.policyDims)
})
}
}

// Test NewAuthorizer factory function via authz.New
func TestNewAuthorizer(t *testing.T) {
log := logger.CreateTestLogger()
Expand Down
15 changes: 15 additions & 0 deletions tests-bdd/features/authz-v2.feature
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,18 @@ Feature: Authz v2 default policy authorization
Then the response should be permission denied
When I send a request to list KAS keys for URI "https://kas-a.example.com"
Then the response should be permission denied

# Security: URL values containing '&' and '=' must not be mis-parsed as
# extra dimensions. A kas-a scoped role must not gain access to a different
# KAS key whose URI injects a trailing "kas_uri=https://kas-a.example.com".
Scenario: KAS URI with injected dimension characters is rejected
Given I use the platform as "opentdf-admin"
And I create KAS keys:
| kas_uri | key_id |
| https://kas-b.example.com?foo=bar&kas_uri=https://kas-a.example.com | inject-query-kid |
| https://kas-b.example.com/a?x=y&kas_uri=https://kas-a.example.com | inject-path-kid |
Given I use the platform as "kas-a"
When I send a request to get KAS key "inject-query-kid"
Then the response should be permission denied
When I send a request to get KAS key "inject-path-kid"
Then the response should be permission denied
Loading