Prevent conflicts between service account and jwt issuers
Signed-off-by: Monis Khan <mok@microsoft.com> Kubernetes-commit: 05e1eff7933a440595f4bea322b54054d3c1b153
This commit is contained in:
parent
0a68878666
commit
9432b4df38
|
@ -41,7 +41,7 @@ import (
|
|||
)
|
||||
|
||||
// ValidateAuthenticationConfiguration validates a given AuthenticationConfiguration.
|
||||
func ValidateAuthenticationConfiguration(c *api.AuthenticationConfiguration) field.ErrorList {
|
||||
func ValidateAuthenticationConfiguration(c *api.AuthenticationConfiguration, disallowedIssuers []string) field.ErrorList {
|
||||
root := field.NewPath("jwt")
|
||||
var allErrs field.ErrorList
|
||||
|
||||
|
@ -69,7 +69,7 @@ func ValidateAuthenticationConfiguration(c *api.AuthenticationConfiguration) fie
|
|||
// check and add validation for duplicate issuers.
|
||||
for i, a := range c.JWT {
|
||||
fldPath := root.Index(i)
|
||||
_, errs := validateJWTAuthenticator(a, fldPath, utilfeature.DefaultFeatureGate.Enabled(features.StructuredAuthenticationConfiguration))
|
||||
_, errs := validateJWTAuthenticator(a, fldPath, sets.New(disallowedIssuers...), utilfeature.DefaultFeatureGate.Enabled(features.StructuredAuthenticationConfiguration))
|
||||
allErrs = append(allErrs, errs...)
|
||||
}
|
||||
|
||||
|
@ -79,17 +79,17 @@ func ValidateAuthenticationConfiguration(c *api.AuthenticationConfiguration) fie
|
|||
// CompileAndValidateJWTAuthenticator validates a given JWTAuthenticator and returns a CELMapper with the compiled
|
||||
// CEL expressions for claim mappings and validation rules.
|
||||
// This is exported for use in oidc package.
|
||||
func CompileAndValidateJWTAuthenticator(authenticator api.JWTAuthenticator) (authenticationcel.CELMapper, field.ErrorList) {
|
||||
return validateJWTAuthenticator(authenticator, nil, utilfeature.DefaultFeatureGate.Enabled(features.StructuredAuthenticationConfiguration))
|
||||
func CompileAndValidateJWTAuthenticator(authenticator api.JWTAuthenticator, disallowedIssuers []string) (authenticationcel.CELMapper, field.ErrorList) {
|
||||
return validateJWTAuthenticator(authenticator, nil, sets.New(disallowedIssuers...), utilfeature.DefaultFeatureGate.Enabled(features.StructuredAuthenticationConfiguration))
|
||||
}
|
||||
|
||||
func validateJWTAuthenticator(authenticator api.JWTAuthenticator, fldPath *field.Path, structuredAuthnFeatureEnabled bool) (authenticationcel.CELMapper, field.ErrorList) {
|
||||
func validateJWTAuthenticator(authenticator api.JWTAuthenticator, fldPath *field.Path, disallowedIssuers sets.Set[string], structuredAuthnFeatureEnabled bool) (authenticationcel.CELMapper, field.ErrorList) {
|
||||
var allErrs field.ErrorList
|
||||
|
||||
compiler := authenticationcel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()))
|
||||
mapper := &authenticationcel.CELMapper{}
|
||||
|
||||
allErrs = append(allErrs, validateIssuer(authenticator.Issuer, fldPath.Child("issuer"))...)
|
||||
allErrs = append(allErrs, validateIssuer(authenticator.Issuer, disallowedIssuers, fldPath.Child("issuer"))...)
|
||||
allErrs = append(allErrs, validateClaimValidationRules(compiler, mapper, authenticator.ClaimValidationRules, fldPath.Child("claimValidationRules"), structuredAuthnFeatureEnabled)...)
|
||||
allErrs = append(allErrs, validateClaimMappings(compiler, mapper, authenticator.ClaimMappings, fldPath.Child("claimMappings"), structuredAuthnFeatureEnabled)...)
|
||||
allErrs = append(allErrs, validateUserValidationRules(compiler, mapper, authenticator.UserValidationRules, fldPath.Child("userValidationRules"), structuredAuthnFeatureEnabled)...)
|
||||
|
@ -97,10 +97,10 @@ func validateJWTAuthenticator(authenticator api.JWTAuthenticator, fldPath *field
|
|||
return *mapper, allErrs
|
||||
}
|
||||
|
||||
func validateIssuer(issuer api.Issuer, fldPath *field.Path) field.ErrorList {
|
||||
func validateIssuer(issuer api.Issuer, disallowedIssuers sets.Set[string], fldPath *field.Path) field.ErrorList {
|
||||
var allErrs field.ErrorList
|
||||
|
||||
allErrs = append(allErrs, validateIssuerURL(issuer.URL, fldPath.Child("url"))...)
|
||||
allErrs = append(allErrs, validateIssuerURL(issuer.URL, disallowedIssuers, fldPath.Child("url"))...)
|
||||
allErrs = append(allErrs, validateIssuerDiscoveryURL(issuer.URL, issuer.DiscoveryURL, fldPath.Child("discoveryURL"))...)
|
||||
allErrs = append(allErrs, validateAudiences(issuer.Audiences, issuer.AudienceMatchPolicy, fldPath.Child("audiences"), fldPath.Child("audienceMatchPolicy"))...)
|
||||
allErrs = append(allErrs, validateCertificateAuthority(issuer.CertificateAuthority, fldPath.Child("certificateAuthority"))...)
|
||||
|
@ -108,12 +108,12 @@ func validateIssuer(issuer api.Issuer, fldPath *field.Path) field.ErrorList {
|
|||
return allErrs
|
||||
}
|
||||
|
||||
func validateIssuerURL(issuerURL string, fldPath *field.Path) field.ErrorList {
|
||||
func validateIssuerURL(issuerURL string, disallowedIssuers sets.Set[string], fldPath *field.Path) field.ErrorList {
|
||||
if len(issuerURL) == 0 {
|
||||
return field.ErrorList{field.Required(fldPath, "URL is required")}
|
||||
}
|
||||
|
||||
return validateURL(issuerURL, fldPath)
|
||||
return validateURL(issuerURL, disallowedIssuers, fldPath)
|
||||
}
|
||||
|
||||
func validateIssuerDiscoveryURL(issuerURL, issuerDiscoveryURL string, fldPath *field.Path) field.ErrorList {
|
||||
|
@ -127,13 +127,18 @@ func validateIssuerDiscoveryURL(issuerURL, issuerDiscoveryURL string, fldPath *f
|
|||
allErrs = append(allErrs, field.Invalid(fldPath, issuerDiscoveryURL, "discoveryURL must be different from URL"))
|
||||
}
|
||||
|
||||
allErrs = append(allErrs, validateURL(issuerDiscoveryURL, fldPath)...)
|
||||
// issuerDiscoveryURL is not an issuer URL and does not need to validated against any set of disallowed issuers
|
||||
allErrs = append(allErrs, validateURL(issuerDiscoveryURL, nil, fldPath)...)
|
||||
return allErrs
|
||||
}
|
||||
|
||||
func validateURL(issuerURL string, fldPath *field.Path) field.ErrorList {
|
||||
func validateURL(issuerURL string, disallowedIssuers sets.Set[string], fldPath *field.Path) field.ErrorList {
|
||||
var allErrs field.ErrorList
|
||||
|
||||
if disallowedIssuers.Has(issuerURL) {
|
||||
allErrs = append(allErrs, field.Invalid(fldPath, issuerURL, fmt.Sprintf("URL must not overlap with disallowed issuers: %s", sets.List(disallowedIssuers))))
|
||||
}
|
||||
|
||||
u, err := url.Parse(issuerURL)
|
||||
if err != nil {
|
||||
allErrs = append(allErrs, field.Invalid(fldPath, issuerURL, err.Error()))
|
||||
|
|
|
@ -50,9 +50,10 @@ func TestValidateAuthenticationConfiguration(t *testing.T) {
|
|||
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StructuredAuthenticationConfiguration, true)()
|
||||
|
||||
testCases := []struct {
|
||||
name string
|
||||
in *api.AuthenticationConfiguration
|
||||
want string
|
||||
name string
|
||||
in *api.AuthenticationConfiguration
|
||||
disallowedIssuers []string
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "jwt authenticator is empty",
|
||||
|
@ -174,6 +175,33 @@ func TestValidateAuthenticationConfiguration(t *testing.T) {
|
|||
},
|
||||
want: `jwt[0].userValidationRules[1].expression: Duplicate value: "user.username == 'foo'"`,
|
||||
},
|
||||
{
|
||||
name: "valid authentication configuration with disallowed issuer",
|
||||
in: &api.AuthenticationConfiguration{
|
||||
JWT: []api.JWTAuthenticator{
|
||||
{
|
||||
Issuer: api.Issuer{
|
||||
URL: "https://issuer-url",
|
||||
Audiences: []string{"audience"},
|
||||
},
|
||||
ClaimValidationRules: []api.ClaimValidationRule{
|
||||
{
|
||||
Claim: "foo",
|
||||
RequiredValue: "bar",
|
||||
},
|
||||
},
|
||||
ClaimMappings: api.ClaimMappings{
|
||||
Username: api.PrefixedClaimOrExpression{
|
||||
Claim: "sub",
|
||||
Prefix: pointer.String("prefix"),
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
disallowedIssuers: []string{"a", "b", "https://issuer-url", "c"},
|
||||
want: `jwt[0].issuer.url: Invalid value: "https://issuer-url": URL must not overlap with disallowed issuers: [a b c https://issuer-url]`,
|
||||
},
|
||||
{
|
||||
name: "valid authentication configuration",
|
||||
in: &api.AuthenticationConfiguration{
|
||||
|
@ -204,7 +232,7 @@ func TestValidateAuthenticationConfiguration(t *testing.T) {
|
|||
|
||||
for _, tt := range testCases {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := ValidateAuthenticationConfiguration(tt.in).ToAggregate()
|
||||
got := ValidateAuthenticationConfiguration(tt.in, tt.disallowedIssuers).ToAggregate()
|
||||
if d := cmp.Diff(tt.want, errString(got)); d != "" {
|
||||
t.Fatalf("AuthenticationConfiguration validation mismatch (-want +got):\n%s", d)
|
||||
}
|
||||
|
@ -216,9 +244,10 @@ func TestValidateIssuerURL(t *testing.T) {
|
|||
fldPath := field.NewPath("issuer", "url")
|
||||
|
||||
testCases := []struct {
|
||||
name string
|
||||
in string
|
||||
want string
|
||||
name string
|
||||
in string
|
||||
disallowedIssuers sets.Set[string]
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "url is empty",
|
||||
|
@ -250,6 +279,12 @@ func TestValidateIssuerURL(t *testing.T) {
|
|||
in: "https://issuer-url#fragment",
|
||||
want: `issuer.url: Invalid value: "https://issuer-url#fragment": URL must not contain a fragment`,
|
||||
},
|
||||
{
|
||||
name: "valid url that is disallowed",
|
||||
in: "https://issuer-url",
|
||||
disallowedIssuers: sets.New("https://issuer-url"),
|
||||
want: `issuer.url: Invalid value: "https://issuer-url": URL must not overlap with disallowed issuers: [https://issuer-url]`,
|
||||
},
|
||||
{
|
||||
name: "valid url",
|
||||
in: "https://issuer-url",
|
||||
|
@ -259,7 +294,7 @@ func TestValidateIssuerURL(t *testing.T) {
|
|||
|
||||
for _, tt := range testCases {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := validateIssuerURL(tt.in, fldPath).ToAggregate()
|
||||
got := validateIssuerURL(tt.in, tt.disallowedIssuers, fldPath).ToAggregate()
|
||||
if d := cmp.Diff(tt.want, errString(got)); d != "" {
|
||||
t.Fatalf("URL validation mismatch (-want +got):\n%s", d)
|
||||
}
|
||||
|
|
|
@ -94,6 +94,8 @@ type Options struct {
|
|||
// https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation
|
||||
SupportedSigningAlgs []string
|
||||
|
||||
DisallowedIssuers []string
|
||||
|
||||
// now is used for testing. It defaults to time.Now.
|
||||
now func() time.Time
|
||||
}
|
||||
|
@ -227,7 +229,7 @@ var allowedSigningAlgs = map[string]bool{
|
|||
}
|
||||
|
||||
func New(opts Options) (authenticator.Token, error) {
|
||||
celMapper, fieldErr := apiservervalidation.CompileAndValidateJWTAuthenticator(opts.JWTAuthenticator)
|
||||
celMapper, fieldErr := apiservervalidation.CompileAndValidateJWTAuthenticator(opts.JWTAuthenticator, opts.DisallowedIssuers)
|
||||
if err := fieldErr.ToAggregate(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
|
|
@ -2772,6 +2772,58 @@ func TestToken(t *testing.T) {
|
|||
}`, valid.Unix()),
|
||||
wantInitErr: `claimMappings.extra[2].key: Duplicate value: "example.org/foo"`,
|
||||
},
|
||||
{
|
||||
name: "disallowed issuer via configured value",
|
||||
options: Options{
|
||||
JWTAuthenticator: apiserver.JWTAuthenticator{
|
||||
Issuer: apiserver.Issuer{
|
||||
URL: "https://auth.example.com",
|
||||
Audiences: []string{"my-client"},
|
||||
},
|
||||
ClaimMappings: apiserver.ClaimMappings{
|
||||
Username: apiserver.PrefixedClaimOrExpression{
|
||||
Expression: "claims.username",
|
||||
},
|
||||
Groups: apiserver.PrefixedClaimOrExpression{
|
||||
Expression: "claims.groups",
|
||||
},
|
||||
UID: apiserver.ClaimOrExpression{
|
||||
Expression: "claims.uid",
|
||||
},
|
||||
Extra: []apiserver.ExtraMapping{
|
||||
{
|
||||
Key: "example.org/foo",
|
||||
ValueExpression: "claims.foo",
|
||||
},
|
||||
{
|
||||
Key: "example.org/bar",
|
||||
ValueExpression: "claims.bar",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
DisallowedIssuers: []string{"https://auth.example.com"},
|
||||
now: func() time.Time { return now },
|
||||
},
|
||||
signingKey: loadRSAPrivKey(t, "testdata/rsa_1.pem", jose.RS256),
|
||||
pubKeys: []*jose.JSONWebKey{
|
||||
loadRSAKey(t, "testdata/rsa_1.pem", jose.RS256),
|
||||
},
|
||||
claims: fmt.Sprintf(`{
|
||||
"iss": "https://auth.example.com",
|
||||
"aud": "my-client",
|
||||
"username": "jane",
|
||||
"groups": ["team1", "team2"],
|
||||
"exp": %d,
|
||||
"uid": "1234",
|
||||
"foo": "bar",
|
||||
"bar": [
|
||||
"baz",
|
||||
"qux"
|
||||
]
|
||||
}`, valid.Unix()),
|
||||
wantInitErr: `issuer.url: Invalid value: "https://auth.example.com": URL must not overlap with disallowed issuers: [https://auth.example.com]`,
|
||||
},
|
||||
{
|
||||
name: "extra claim mapping, empty string value for key",
|
||||
options: Options{
|
||||
|
|
Loading…
Reference in New Issue