From 079d98dc9a67d1d4d7aec95848c2b32a80168587 Mon Sep 17 00:00:00 2001 From: Michiel <918128+mvanderlee@users.noreply.github.com> Date: Fri, 5 Sep 2025 23:09:15 +0200 Subject: [PATCH 1/2] Add support for custom group claims --- README.md | 9 ++++- policy/enforcer.go | 78 +++++++++++++++++++++++++++++++++++++---- policy/enforcer_test.go | 35 ++++++++++++++---- 3 files changed, 109 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 2c7c15a7..3831766b 100644 --- a/README.md +++ b/README.md @@ -157,6 +157,12 @@ To allow a group, `ssh-users`, to ssh to your server as `root`, run: sudo opkssh add root oidc:groups:ssh-users google ``` +To allow a group to be read from custom claim `https://acme.com/groups`, `ssh-users`, to ssh to your server as `root`, run: + +```bash +sudo opkssh add root oidc:\"https://acme.com/groups\":ssh-users google +``` + ## How it works We use two features of SSH to make this work. @@ -234,7 +240,7 @@ Linux user accounts are typically referred to in SSH as *principals* and we cont - Subject ID - an unique ID for the user set by the OP. This is the `sub` claim in the ID Token. - Group - the name of the group that the user is part of. This uses the `groups` claim which is presumed to be an array. The group identifier uses a structured identifier. I.e. `oidc:groups:{groupId}`. Replace the `groupId` - with the id of your group. + with the id of your group. If your group contains a colon, escape it `oidc:"https://acme.com/groups":{groupId}`. - Column 3: Issuer URI ```bash @@ -246,6 +252,7 @@ dev bob@microsoft.com https://login.microsoftonline.com/9188040d-6c67-4c5b-b112- # Group identifier dev oidc:groups:developer https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0 +dev oidc:"https://acme.com/groups":developer https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0 ``` To add new rule run: diff --git a/policy/enforcer.go b/policy/enforcer.go index 94f44abe..5caad91d 100644 --- a/policy/enforcer.go +++ b/policy/enforcer.go @@ -30,7 +30,7 @@ import ( ) const ( - OIDC_GROUPS = "oidc:groups:" + OIDC_GROUPS = "oidc:" OIDC_WILDCARD_EMAIL = "oidc-match-end:email:" ) @@ -46,28 +46,94 @@ type Enforcer struct { PolicyLoader Loader } +// type for Identity Token requiredClaims +type requiredClaims struct { + Email string `json:"email"` + Sub string `json:"sub"` +} + // type for Identity Token checkedClaims type checkedClaims struct { - Email string `json:"email"` - Sub string `json:"sub"` - Groups []string `json:"groups"` + Email string `json:"email"` + Sub string `json:"sub"` + StringArrayClaims map[string][]string `json:"stringArrayClaims,omitempty"` +} + +func (s *checkedClaims) UnmarshalJSON(data []byte) error { + // First unmarshal the static fields + var claims requiredClaims + err := json.Unmarshal(data, &claims) + if err != nil { + return err + } + + // Unmarshal the rest + var schema map[string]interface{} + err = json.Unmarshal([]byte(data), &schema) + if err != nil { + return err + } + + s.Email = claims.Email + s.Sub = claims.Sub + s.StringArrayClaims = make(map[string][]string) + // Add all []string types as potential group claims + for key, value := range schema { + valueArray, ok := value.([]interface{}) + if ok { + isStringArrayClaim := true + claimValues := make([]string, len(valueArray)) + for i, v := range valueArray { + str, ok := v.(string) + if !ok { + isStringArrayClaim = false + break + } + claimValues[i] = str + } + if isStringArrayClaim { + s.StringArrayClaims[key] = claimValues + } + } + } + + return nil } // The default location for policy plugins const pluginPolicyDir = "/etc/opk/policy.d" +func escapedSplit(s string, sep rune) []string { + quoted := false + a := strings.FieldsFunc(s, func(r rune) bool { + if r == '"' { + quoted = !quoted + } + return !quoted && r == sep + }) + return a +} + // Validates that the server defined identity attribute matches the // respective claim from the identity token func validateClaim(claims *checkedClaims, user *User) bool { + // Should we match on the email claim? if strings.HasPrefix(claims.Email, OIDC_WILDCARD_EMAIL) { return false } + // Should we match on an oidc claim? if strings.HasPrefix(user.IdentityAttribute, OIDC_GROUPS) { - oidcGroupSections := strings.Split(user.IdentityAttribute, ":") + oidcGroupSections := escapedSplit(user.IdentityAttribute, ':') + oidcGroupsName := strings.Trim(oidcGroupSections[1], "\"") - return slices.Contains(claims.Groups, oidcGroupSections[len(oidcGroupSections)-1]) + return slices.Contains( + claims.StringArrayClaims[oidcGroupsName], + oidcGroupSections[len(oidcGroupSections)-1], + ) } + + // Should we match on the email wildcard claim? wildCardEmailMatch := false if strings.HasPrefix(user.IdentityAttribute, OIDC_WILDCARD_EMAIL) { if strings.HasSuffix(strings.ToLower(claims.Email), strings.ToLower(user.IdentityAttribute[len(OIDC_WILDCARD_EMAIL):len(user.IdentityAttribute)])) { diff --git a/policy/enforcer_test.go b/policy/enforcer_test.go index f80bcd10..9053e179 100644 --- a/policy/enforcer_test.go +++ b/policy/enforcer_test.go @@ -44,11 +44,11 @@ func NewMockOpenIdSubProvider(t *testing.T, sub string) providers.OpenIdProvider return op } -func NewMockOpenIdProviderGroups(t *testing.T, groups []string) providers.OpenIdProvider { +func NewMockOpenIdProviderGroups(t *testing.T, claimName string, groups []string) providers.OpenIdProvider { providerOpts := providers.DefaultMockProviderOpts() op, _, idTokenTemplate, err := providers.NewMockProvider(providerOpts) require.NoError(t, err) - idTokenTemplate.ExtraClaims = map[string]any{"email": "arthur.aardvark@example.com", "groups": groups} + idTokenTemplate.ExtraClaims = map[string]any{"email": "arthur.aardvark@example.com", claimName: groups} return op } @@ -143,6 +143,11 @@ var policyWithOidcGroup = &policy.Policy{ Principals: []string{"test"}, Issuer: "https://accounts.example.com", }, + { + IdentityAttribute: "oidc:\"https://acme.com/groups\":e", + Principals: []string{"test"}, + Issuer: "https://accounts.example.com", + }, { IdentityAttribute: "oidc-match-end:email:@example2.com", Principals: []string{"test"}, @@ -324,7 +329,25 @@ func TestPolicyDeniedWrongIssuer(t *testing.T) { func TestPolicyApprovedOidcGroups(t *testing.T) { t.Parallel() - op := NewMockOpenIdProviderGroups(t, []string{"a", "b", "c"}) + op := NewMockOpenIdProviderGroups(t, "groups", []string{"a", "b", "c"}) + + opkClient, err := client.New(op) + require.NoError(t, err) + pkt, err := opkClient.Auth(context.Background()) + require.NoError(t, err) + + policyEnforcer := &policy.Enforcer{ + PolicyLoader: &MockPolicyLoader{Policy: policyWithOidcGroup}, + } + + err = policyEnforcer.CheckPolicy("test", pkt, "", "example-base64Cert", "ssh-rsa", policy.DenyList{}) + require.NoError(t, err) +} + +func TestPolicyApprovedOidcGroupsUrlClaim(t *testing.T) { + t.Parallel() + + op := NewMockOpenIdProviderGroups(t, "https://acme.com/groups", []string{"e"}) opkClient, err := client.New(op) require.NoError(t, err) @@ -342,7 +365,7 @@ func TestPolicyApprovedOidcGroups(t *testing.T) { func TestPolicyApprovedOidcGroupWithAtSign(t *testing.T) { t.Parallel() - op := NewMockOpenIdProviderGroups(t, []string{"it.infra@my_domain.com"}) + op := NewMockOpenIdProviderGroups(t, "groups", []string{"it.infra@my_domain.com"}) policyLine := &policy.Policy{ Users: []policy.User{ @@ -370,7 +393,7 @@ func TestPolicyApprovedOidcGroupWithAtSign(t *testing.T) { func TestPolicyDeniedOidcGroups(t *testing.T) { t.Parallel() - op := NewMockOpenIdProviderGroups(t, []string{"z"}) + op := NewMockOpenIdProviderGroups(t, "groups", []string{"z"}) opkClient, err := client.New(op) require.NoError(t, err) @@ -426,7 +449,7 @@ func TestEnforcerTableTest(t *testing.T) { }{ { name: "Happy path (No userinfo supplied but ID Token has groups claim)", - op: NewMockOpenIdProviderGroups(t, []string{"group1", "group2"}), + op: NewMockOpenIdProviderGroups(t, "groups", []string{"group1", "group2"}), policyLoader: &MockPolicyLoader{Policy: policyWithOidcGroup}, }, { From a956383e7b43894855d9880700791f2660a77771 Mon Sep 17 00:00:00 2001 From: Ethan Heilman Date: Sun, 21 Sep 2025 16:30:40 -0400 Subject: [PATCH 2/2] Minor change, extra tests --- README.md | 5 ++- docs/config.md | 13 ++++++- policy/enforcer.go | 82 ++++++++++++++++++++++------------------- policy/enforcer_test.go | 17 +++++++++ 4 files changed, 76 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index 3831766b..10c3e17a 100644 --- a/README.md +++ b/README.md @@ -157,12 +157,15 @@ To allow a group, `ssh-users`, to ssh to your server as `root`, run: sudo opkssh add root oidc:groups:ssh-users google ``` -To allow a group to be read from custom claim `https://acme.com/groups`, `ssh-users`, to ssh to your server as `root`, run: +We can also enforce policy on custom claims. +For instance to require that root access is only granted to users whose ID Token has a claim `https://acme.com/groups` with the value `ssh-users` run: ```bash sudo opkssh add root oidc:\"https://acme.com/groups\":ssh-users google ``` +which will add that line to your OPKSSH policy file. + ## How it works We use two features of SSH to make this work. diff --git a/docs/config.md b/docs/config.md index e3f83820..062d567b 100644 --- a/docs/config.md +++ b/docs/config.md @@ -151,14 +151,14 @@ dev oidc-match-end:email:@example.com https://login.microsoftonline.com/9188040d These `auth_id` files can be edited by hand or you can use the add command to add new policies. The add command has the following syntax. -`sudo opkssh add {USER} {EMAIL|SUB|GROUP} {ISSUER}` +`sudo opkssh add {USER} {EMAIL|SUB|CLAIM} {ISSUER}` For convenience you can use the shorthand `google`, `azure`, `gitlab` rather than specifying the entire issuer. This is especially useful in the case of azure where the issuer contains a long and hard to remember random string. The following command will allow `alice@example.com` to ssh in as `root`. -Groups must be prefixed with `oidc:group`. So to allow anyone with the group `admin` to ssh in as root you would run the command: +Claims must be prefixed with `oidc:{CLAIM}` e.g. for the group claim `oidc:group`. To allow anyone with the group `admin` to ssh in as root you would run the command: ```bash sudo opkssh add root oidc:group:admin azure @@ -166,6 +166,15 @@ sudo opkssh add root oidc:group:admin azure Note that currently Google does not put their groups in the ID Token, so groups based auth does not work if you OpenID Provider is Google. +We support policy on claims that are also URIs as this is a common pattern for groups in some systems. +To require that root access is only granted to users whose ID Token has a claim `https://acme.com/groups` with the value `ssh-users` run: + +```bash +sudo opkssh add root oidc:\"https://acme.com/groups\":ssh-users google +``` + +which will add that line to your OPKSSH policy file. + The system authorized identity file requires the following permissions: ```bash diff --git a/policy/enforcer.go b/policy/enforcer.go index 5caad91d..598d62d4 100644 --- a/policy/enforcer.go +++ b/policy/enforcer.go @@ -30,7 +30,7 @@ import ( ) const ( - OIDC_GROUPS = "oidc:" + OIDC_CLAIMS = "oidc:" OIDC_WILDCARD_EMAIL = "oidc-match-end:email:" ) @@ -46,54 +46,57 @@ type Enforcer struct { PolicyLoader Loader } -// type for Identity Token requiredClaims -type requiredClaims struct { - Email string `json:"email"` - Sub string `json:"sub"` -} - // type for Identity Token checkedClaims type checkedClaims struct { - Email string `json:"email"` - Sub string `json:"sub"` - StringArrayClaims map[string][]string `json:"stringArrayClaims,omitempty"` + Email string `json:"email"` + Sub string `json:"sub"` + ExtraClaims map[string][]string `json:"-"` } func (s *checkedClaims) UnmarshalJSON(data []byte) error { - // First unmarshal the static fields - var claims requiredClaims - err := json.Unmarshal(data, &claims) - if err != nil { + + // Avoid infinite recursion + type checkedClaimsAlias checkedClaims + var a checkedClaimsAlias + + // Unmarshal the required claims + if err := json.Unmarshal(data, &a); err != nil { return err } + *s = checkedClaims(a) - // Unmarshal the rest + // Unmarshal everything else var schema map[string]interface{} - err = json.Unmarshal([]byte(data), &schema) + err := json.Unmarshal([]byte(data), &schema) if err != nil { return err } - s.Email = claims.Email - s.Sub = claims.Sub - s.StringArrayClaims = make(map[string][]string) - // Add all []string types as potential group claims - for key, value := range schema { - valueArray, ok := value.([]interface{}) - if ok { - isStringArrayClaim := true - claimValues := make([]string, len(valueArray)) - for i, v := range valueArray { - str, ok := v.(string) - if !ok { - isStringArrayClaim = false - break + var raw map[string]any + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + + s.ExtraClaims = make(map[string][]string, len(raw)) + + for k, v := range raw { + switch t := v.(type) { + case string: + s.ExtraClaims[k] = []string{t} + case []any: + // Turn all elements in a list into a string + out := make([]string, 0, len(t)) + for _, e := range t { + if s, ok := e.(string); ok { + out = append(out, s) + } else { + out = append(out, fmt.Sprint(e)) } - claimValues[i] = str - } - if isStringArrayClaim { - s.StringArrayClaims[key] = claimValues } + s.ExtraClaims[k] = out + default: + // Turn numbers/bools etc into strings + s.ExtraClaims[k] = []string{fmt.Sprint(t)} } } @@ -103,7 +106,10 @@ func (s *checkedClaims) UnmarshalJSON(data []byte) error { // The default location for policy plugins const pluginPolicyDir = "/etc/opk/policy.d" -func escapedSplit(s string, sep rune) []string { +// EscapedSplit splits a string by a separator while ignoring the separator in quoted sections. +// This is useful for strings that may contain the separator character as part of the string +// and not as a delimiter. +func EscapedSplit(s string, sep rune) []string { quoted := false a := strings.FieldsFunc(s, func(r rune) bool { if r == '"' { @@ -123,12 +129,12 @@ func validateClaim(claims *checkedClaims, user *User) bool { } // Should we match on an oidc claim? - if strings.HasPrefix(user.IdentityAttribute, OIDC_GROUPS) { - oidcGroupSections := escapedSplit(user.IdentityAttribute, ':') + if strings.HasPrefix(user.IdentityAttribute, OIDC_CLAIMS) { + oidcGroupSections := EscapedSplit(user.IdentityAttribute, ':') oidcGroupsName := strings.Trim(oidcGroupSections[1], "\"") return slices.Contains( - claims.StringArrayClaims[oidcGroupsName], + claims.ExtraClaims[oidcGroupsName], oidcGroupSections[len(oidcGroupSections)-1], ) } diff --git a/policy/enforcer_test.go b/policy/enforcer_test.go index 9053e179..2b97d8df 100644 --- a/policy/enforcer_test.go +++ b/policy/enforcer_test.go @@ -628,3 +628,20 @@ func TestLocalEmail(t *testing.T) { err = policyEnforcer.CheckPolicy("test", pkt, "", "example-base64Cert", "ssh-rsa", policy.DenyList{}) require.Error(t, err, "user should not have access") } + +func TestEscapedSplit(t *testing.T) { + t.Parallel() + + escaped := policy.EscapedSplit("abc:def:ghi", ':') + require.Equal(t, []string{"abc", "def", "ghi"}, escaped) + + escaped = policy.EscapedSplit(`abc:"xxx:yyy"`, ':') + require.Equal(t, []string{"abc", `"xxx:yyy"`}, escaped) + + escaped = policy.EscapedSplit(`aaa:"bbb:c" zzz:"qqq:www"`, ':') + require.Equal(t, []string{"aaa", "\"bbb:c\" zzz", "\"qqq:www\""}, escaped) + + // Escaped strings which prevent the separator from being recognized + escaped = policy.EscapedSplit(`abc:\"def:ghi\"`, ':') + require.Equal(t, []string{"abc", "\\\"def:ghi\\\""}, escaped) +}