From 8bb8207808047071f0124a081dbd31e39b7057d7 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Thu, 7 Apr 2022 16:13:01 +1000 Subject: [PATCH] feat(oidc): pairwise subject identifiers (#3116) Allows configuring clients with a sector identifier to allow pairwise subject types. --- config.template.yml | 5 ++ docs/configuration/identity-providers/oidc.md | 59 ++++++++----- internal/configuration/config.template.yml | 5 ++ .../schema/identity_providers.go | 9 +- internal/configuration/validator/const.go | 9 +- .../validator/identity_providers.go | 33 +++++++ .../validator/identity_providers_test.go | 88 +++++++++++++++---- internal/oidc/client.go | 9 +- internal/oidc/provider_test.go | 34 +++++++ internal/oidc/types.go | 2 +- 10 files changed, 207 insertions(+), 46 deletions(-) diff --git a/config.template.yml b/config.template.yml index 8ea8ae488..f7165a2d1 100644 --- a/config.template.yml +++ b/config.template.yml @@ -799,6 +799,11 @@ notifier: ## The client secret is a shared secret between Authelia and the consumer of this client. # secret: this_is_a_secret + ## Sector Identifiers are occasionally used to generate pairwise subject identifiers. In most cases this is not + ## necessary. Read the documentation for more information. + ## The subject identifier must be the host component of a URL, which is a domain name with an optional port. + # sector_identifier: example.com + ## Sets the client to public. This should typically not be set, please see the documentation for usage. # public: false diff --git a/docs/configuration/identity-providers/oidc.md b/docs/configuration/identity-providers/oidc.md index 32f825af2..04b6a7170 100644 --- a/docs/configuration/identity-providers/oidc.md +++ b/docs/configuration/identity-providers/oidc.md @@ -48,6 +48,7 @@ identity_providers: - id: myapp description: My Application secret: this_is_a_secret + sector_identifier: '' public: false authorization_policy: two_factor audience: [] @@ -73,7 +74,6 @@ identity_providers: ## Options ### hmac_secret -
type: string {: .label .label-config .label-purple } @@ -87,7 +87,6 @@ byte string for the purpose of meeting the required format. You must [generate t Should be defined using a [secret](../secrets.md) which is the recommended for containerized deployments. ### issuer_private_key -
type: string {: .label .label-config .label-purple } @@ -104,7 +103,6 @@ private key into your configuration. Should be defined using a [secret](../secrets.md) which is the recommended for containerized deployments. ### access_token_lifespan -
type: duration {: .label .label-config .label-purple } @@ -118,7 +116,6 @@ The maximum lifetime of an access token. It's generally recommended keeping this For more information read these docs about [token lifespan]. ### authorize_code_lifespan -
type: duration {: .label .label-config .label-purple } @@ -132,7 +129,6 @@ The maximum lifetime of an authorize code. This can be rather short, as the auth obtain the other token types. For more information read these docs about [token lifespan]. ### id_token_lifespan -
type: duration {: .label .label-config .label-purple } @@ -145,7 +141,6 @@ required: no The maximum lifetime of an ID token. For more information read these docs about [token lifespan]. ### refresh_token_lifespan -
type: string {: .label .label-config .label-purple } @@ -165,7 +160,6 @@ A good starting point is 50% more or 30 minutes more (which ever is less) time t token lifespan is 90 minutes. ### enable_client_debug_messages -
type: boolean {: .label .label-config .label-purple } @@ -178,7 +172,6 @@ required: no Allows additional debug messages to be sent to the clients. ### minimum_parameter_entropy -
type: integer {: .label .label-config .label-purple } @@ -305,7 +298,6 @@ have the scheme http or https and do not have the hostname of localhost. A list of clients to configure. The options for each client are described below. #### id -
type: string {: .label .label-config .label-purple } @@ -317,7 +309,6 @@ The Client ID for this client. It must exactly match the Client ID configured in consuming this client. #### description -
type: string {: .label .label-config .label-purple } @@ -330,7 +321,6 @@ required: no A friendly description for this client shown in the UI. This defaults to the same as the ID. #### secret -
type: string {: .label .label-config .label-purple } @@ -345,8 +335,45 @@ You must [generate this option yourself](#generating-a-random-secret). This must be provided when the client is a confidential client type, and must be blank when using the public client type. To set the client type to public see the [public](#public) configuration option. -#### public +#### sector_identifier +
+type: string +{: .label .label-config .label-purple } +default: '' +{: .label .label-config .label-blue } +required: no +{: .label .label-config .label-red } +
+_**Important Note:** because adjusting this option will inevitably change the `sub` claim of all tokens generated for +the specified client, changing this should cause the relying party to detect all future authorizations as completely new +users._ + +Must be an empty string or the host component of a URL. This is commonly just the domain name, but may also include a +port. + +Authelia utilizes UUID version 4 subject identifiers. By default the public subject identifier type is utilized for all +clients. This means the subject identifiers will be the same for all clients. This configuration option enables pairwise +for this client, and configures the sector identifier utilized for both the storage and the lookup of the subject +identifier. + +1. All clients who do not have this configured will generate the same subject identifier for a particular user regardless + of which client obtains the ID token. +2. All clients which have the same sector identifier will: + 1. have the same subject identifier for a particular user when compared to clients with the same sector identifier. + 2. have a completely different subject identifier for a particular user whe compared to: + 1. any client with the public subject identifier type. + 2. any client with a differing sector identifier. + +In specific but limited scenarios this option is beneficial for privacy reasons. In particular this is useful when the +party utilizing the _Authelia_ [OpenID Connect] Authorization Server is foreign and not controlled by the user. It would +prevent the third party utilizing the subject identifier with another third party in order to track the user. + +Keep in mind depending on the other claims they may still be able to perform this tracking and it is not a silver bullet. +There are very few benefits when utilizing this in a homelab or business where no third party is utilizing +the server. + +#### public
type: bool {: .label .label-config .label-purple } @@ -364,7 +391,6 @@ blank string. In addition to the standard rules for redirect URIs, public clients can use the `urn:ietf:wg:oauth:2.0:oob` redirect URI. #### authorization_policy -
type: string {: .label .label-config .label-purple } @@ -377,7 +403,6 @@ required: no The authorization policy for this client: either `one_factor` or `two_factor`. #### audience -
type: list(string) {: .label .label-config .label-purple } @@ -388,7 +413,6 @@ required: no A list of audiences this client is allowed to request. #### scopes -
type: list(string) {: .label .label-config .label-purple } @@ -403,7 +427,6 @@ information. The documentation for the application you want to use with Authelia you with the scopes to allow. #### redirect_uris -
type: list(string) {: .label .label-config .label-purple } @@ -425,7 +448,6 @@ their redirect URIs are as follows: 4. The client can ignore rule 3 and use `urn:ietf:wg:oauth:2.0:oob` if it is a [public](#public) client type. #### grant_types -
type: list(string) {: .label .label-config .label-purple } @@ -440,7 +462,6 @@ know what you're doing_. Valid options are: `implicit`, `refresh_token`, `author `client_credentials`. #### response_types -
type: list(string) {: .label .label-config .label-purple } @@ -455,7 +476,6 @@ know what you're doing_. Valid options are: `code`, `code id_token`, `id_token`, `token id_token code`. #### response_modes -
type: list(string) {: .label .label-config .label-purple } @@ -469,7 +489,6 @@ A list of response modes this client can return. It is recommended that this isn know what you're doing. Potential values are `form_post`, `query`, and `fragment`. #### userinfo_signing_algorithm -
type: string {: .label .label-config .label-purple } diff --git a/internal/configuration/config.template.yml b/internal/configuration/config.template.yml index 8ea8ae488..f7165a2d1 100644 --- a/internal/configuration/config.template.yml +++ b/internal/configuration/config.template.yml @@ -799,6 +799,11 @@ notifier: ## The client secret is a shared secret between Authelia and the consumer of this client. # secret: this_is_a_secret + ## Sector Identifiers are occasionally used to generate pairwise subject identifiers. In most cases this is not + ## necessary. Read the documentation for more information. + ## The subject identifier must be the host component of a URL, which is a domain name with an optional port. + # sector_identifier: example.com + ## Sets the client to public. This should typically not be set, please see the documentation for usage. # public: false diff --git a/internal/configuration/schema/identity_providers.go b/internal/configuration/schema/identity_providers.go index bb2c35bd3..69c82a3ea 100644 --- a/internal/configuration/schema/identity_providers.go +++ b/internal/configuration/schema/identity_providers.go @@ -41,10 +41,11 @@ type OpenIDConnectCORSConfiguration struct { // OpenIDConnectClientConfiguration configuration for an OpenID Connect client. type OpenIDConnectClientConfiguration struct { - ID string `koanf:"id"` - Description string `koanf:"description"` - Secret string `koanf:"secret"` - Public bool `koanf:"public"` + ID string `koanf:"id"` + Description string `koanf:"description"` + Secret string `koanf:"secret"` + SectorIdentifier url.URL `koanf:"sector_identifier"` + Public bool `koanf:"public"` Policy string `koanf:"authorization_policy"` diff --git a/internal/configuration/validator/const.go b/internal/configuration/validator/const.go index d597badfc..4f258161d 100644 --- a/internal/configuration/validator/const.go +++ b/internal/configuration/validator/const.go @@ -155,6 +155,12 @@ const ( "'%s' but one option is configured as '%s'" errFmtOIDCClientInvalidUserinfoAlgorithm = "identity_providers: oidc: client '%s': option " + "'userinfo_signing_algorithm' must be one of '%s' but it is configured as '%s'" + errFmtOIDCClientInvalidSectorIdentifier = "identity_providers: oidc: client '%s': option " + + "'sector_identifier' with value '%s': must be a URL with only the host component for example '%s' but it has a %s with the value '%s'" + errFmtOIDCClientInvalidSectorIdentifierWithoutValue = "identity_providers: oidc: client '%s': option " + + "'sector_identifier' with value '%s': must be a URL with only the host component for example '%s' but it has a %s" + errFmtOIDCClientInvalidSectorIdentifierHost = "identity_providers: oidc: client '%s': option " + + "'sector_identifier' with value '%s': must be a URL with only the host component but appears to be invalid" errFmtOIDCServerInsecureParameterEntropy = "openid connect provider: SECURITY ISSUE - minimum parameter entropy is " + "configured to an unsafe value, it should be above 8 but it's configured to %d" ) @@ -482,8 +488,9 @@ var ValidKeys = []string{ "identity_providers.oidc.clients", "identity_providers.oidc.clients[].id", "identity_providers.oidc.clients[].description", - "identity_providers.oidc.clients[].public", "identity_providers.oidc.clients[].secret", + "identity_providers.oidc.clients[].sector_identifier", + "identity_providers.oidc.clients[].public", "identity_providers.oidc.clients[].redirect_uris", "identity_providers.oidc.clients[].authorization_policy", "identity_providers.oidc.clients[].scopes", diff --git a/internal/configuration/validator/identity_providers.go b/internal/configuration/validator/identity_providers.go index f1245bd7b..85f4b9db5 100644 --- a/internal/configuration/validator/identity_providers.go +++ b/internal/configuration/validator/identity_providers.go @@ -151,6 +151,7 @@ func validateOIDCClients(config *schema.OpenIDConnectConfiguration, validator *s validator.Push(fmt.Errorf(errFmtOIDCClientInvalidPolicy, client.ID, client.Policy)) } + validateOIDCClientSectorIdentifier(client, validator) validateOIDCClientScopes(c, config, validator) validateOIDCClientGrantTypes(c, config, validator) validateOIDCClientResponseTypes(c, config, validator) @@ -168,6 +169,38 @@ func validateOIDCClients(config *schema.OpenIDConnectConfiguration, validator *s } } +func validateOIDCClientSectorIdentifier(client schema.OpenIDConnectClientConfiguration, validator *schema.StructValidator) { + if client.SectorIdentifier.String() != "" { + if client.SectorIdentifier.Scheme != "" { + validator.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifier, client.ID, client.SectorIdentifier.String(), client.SectorIdentifier.Host, "scheme", client.SectorIdentifier.Scheme)) + + if client.SectorIdentifier.Path != "" { + validator.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifier, client.ID, client.SectorIdentifier.String(), client.SectorIdentifier.Host, "path", client.SectorIdentifier.Path)) + } + + if client.SectorIdentifier.RawQuery != "" { + validator.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifier, client.ID, client.SectorIdentifier.String(), client.SectorIdentifier.Host, "query", client.SectorIdentifier.RawQuery)) + } + + if client.SectorIdentifier.Fragment != "" { + validator.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifier, client.ID, client.SectorIdentifier.String(), client.SectorIdentifier.Host, "fragment", client.SectorIdentifier.Fragment)) + } + + if client.SectorIdentifier.User != nil { + if client.SectorIdentifier.User.Username() != "" { + validator.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifier, client.ID, client.SectorIdentifier.String(), client.SectorIdentifier.Host, "username", client.SectorIdentifier.User.Username())) + } + + if _, set := client.SectorIdentifier.User.Password(); set { + validator.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifierWithoutValue, client.ID, client.SectorIdentifier.String(), client.SectorIdentifier.Host, "password")) + } + } + } else if client.SectorIdentifier.Host == "" { + validator.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifierHost, client.ID, client.SectorIdentifier.String())) + } + } +} + func validateOIDCClientScopes(c int, configuration *schema.OpenIDConnectConfiguration, validator *schema.StructValidator) { if len(configuration.Clients[c].Scopes) == 0 { configuration.Clients[c].Scopes = schema.DefaultOpenIDConnectClientConfiguration.Scopes diff --git a/internal/configuration/validator/identity_providers_test.go b/internal/configuration/validator/identity_providers_test.go index df1a191dd..1b524ff27 100644 --- a/internal/configuration/validator/identity_providers_test.go +++ b/internal/configuration/validator/identity_providers_test.go @@ -3,6 +3,7 @@ package validator import ( "errors" "fmt" + "net/url" "testing" "time" @@ -151,13 +152,22 @@ func TestShouldRaiseErrorWhenOIDCServerNoClients(t *testing.T) { } func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { + mustParseURL := func(u string) url.URL { + out, err := url.Parse(u) + if err != nil { + panic(err) + } + + return *out + } + testCases := []struct { Name string Clients []schema.OpenIDConnectClientConfiguration - Errors []error + Errors []string }{ { - Name: "empty", + Name: "EmptyIDAndSecret", Clients: []schema.OpenIDConnectClientConfiguration{ { ID: "", @@ -166,13 +176,13 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { RedirectURIs: []string{}, }, }, - Errors: []error{ - fmt.Errorf(errFmtOIDCClientInvalidSecret, ""), - errors.New(errFmtOIDCClientsWithEmptyID), + Errors: []string{ + fmt.Sprintf(errFmtOIDCClientInvalidSecret, ""), + errFmtOIDCClientsWithEmptyID, }, }, { - Name: "client-1", + Name: "InvalidPolicy", Clients: []schema.OpenIDConnectClientConfiguration{ { ID: "client-1", @@ -183,10 +193,10 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { }, }, }, - Errors: []error{fmt.Errorf(errFmtOIDCClientInvalidPolicy, "client-1", "a-policy")}, + Errors: []string{fmt.Sprintf(errFmtOIDCClientInvalidPolicy, "client-1", "a-policy")}, }, { - Name: "client-duplicate", + Name: "ClientIDDuplicated", Clients: []schema.OpenIDConnectClientConfiguration{ { ID: "client-x", @@ -201,10 +211,10 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { RedirectURIs: []string{}, }, }, - Errors: []error{errors.New(errFmtOIDCClientsDuplicateID)}, + Errors: []string{errFmtOIDCClientsDuplicateID}, }, { - Name: "client-check-uri-parse", + Name: "RedirectURIInvalid", Clients: []schema.OpenIDConnectClientConfiguration{ { ID: "client-check-uri-parse", @@ -215,12 +225,12 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { }, }, }, - Errors: []error{ - fmt.Errorf(errFmtOIDCClientRedirectURICantBeParsed, "client-check-uri-parse", "http://abc@%two", errors.New("parse \"http://abc@%two\": invalid URL escape \"%tw\"")), + Errors: []string{ + fmt.Sprintf(errFmtOIDCClientRedirectURICantBeParsed, "client-check-uri-parse", "http://abc@%two", errors.New("parse \"http://abc@%two\": invalid URL escape \"%tw\"")), }, }, { - Name: "client-check-uri-abs", + Name: "RedirectURINotAbsolute", Clients: []schema.OpenIDConnectClientConfiguration{ { ID: "client-check-uri-abs", @@ -231,8 +241,47 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { }, }, }, - Errors: []error{ - fmt.Errorf(errFmtOIDCClientRedirectURIAbsolute, "client-check-uri-abs", "google.com"), + Errors: []string{ + fmt.Sprintf(errFmtOIDCClientRedirectURIAbsolute, "client-check-uri-abs", "google.com"), + }, + }, + { + Name: "InvalidSectorIdentifierInvalidURL", + Clients: []schema.OpenIDConnectClientConfiguration{ + { + ID: "client-invalid-sector", + Secret: "a-secret", + Policy: policyTwoFactor, + RedirectURIs: []string{ + "https://google.com", + }, + SectorIdentifier: mustParseURL("https://user:pass@example.com/path?query=abc#fragment"), + }, + }, + Errors: []string{ + fmt.Sprintf(errFmtOIDCClientInvalidSectorIdentifier, "client-invalid-sector", "https://user:pass@example.com/path?query=abc#fragment", "example.com", "scheme", "https"), + fmt.Sprintf(errFmtOIDCClientInvalidSectorIdentifier, "client-invalid-sector", "https://user:pass@example.com/path?query=abc#fragment", "example.com", "path", "/path"), + fmt.Sprintf(errFmtOIDCClientInvalidSectorIdentifier, "client-invalid-sector", "https://user:pass@example.com/path?query=abc#fragment", "example.com", "query", "query=abc"), + fmt.Sprintf(errFmtOIDCClientInvalidSectorIdentifier, "client-invalid-sector", "https://user:pass@example.com/path?query=abc#fragment", "example.com", "fragment", "fragment"), + fmt.Sprintf(errFmtOIDCClientInvalidSectorIdentifier, "client-invalid-sector", "https://user:pass@example.com/path?query=abc#fragment", "example.com", "username", "user"), + fmt.Sprintf(errFmtOIDCClientInvalidSectorIdentifierWithoutValue, "client-invalid-sector", "https://user:pass@example.com/path?query=abc#fragment", "example.com", "password"), + }, + }, + { + Name: "InvalidSectorIdentifierInvalidHost", + Clients: []schema.OpenIDConnectClientConfiguration{ + { + ID: "client-invalid-sector", + Secret: "a-secret", + Policy: policyTwoFactor, + RedirectURIs: []string{ + "https://google.com", + }, + SectorIdentifier: mustParseURL("example.com/path?query=abc#fragment"), + }, + }, + Errors: []string{ + fmt.Sprintf(errFmtOIDCClientInvalidSectorIdentifierHost, "client-invalid-sector", "example.com/path?query=abc#fragment"), }, }, } @@ -250,7 +299,14 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { ValidateIdentityProviders(config, validator) - assert.ElementsMatch(t, validator.Errors(), tc.Errors) + errs := validator.Errors() + + require.Len(t, errs, len(tc.Errors)) + for i, errStr := range tc.Errors { + t.Run(fmt.Sprintf("Error%d", i+1), func(t *testing.T) { + assert.EqualError(t, errs[i], errStr) + }) + } }) } } diff --git a/internal/oidc/client.go b/internal/oidc/client.go index 28a35af62..2b7a04e46 100644 --- a/internal/oidc/client.go +++ b/internal/oidc/client.go @@ -12,10 +12,11 @@ import ( // NewClient creates a new Client. func NewClient(config schema.OpenIDConnectClientConfiguration) (client *Client) { client = &Client{ - ID: config.ID, - Description: config.Description, - Secret: []byte(config.Secret), - Public: config.Public, + ID: config.ID, + Description: config.Description, + Secret: []byte(config.Secret), + SectorIdentifier: config.SectorIdentifier.String(), + Public: config.Public, Policy: authorization.PolicyToLevel(config.Policy), diff --git a/internal/oidc/provider_test.go b/internal/oidc/provider_test.go index 0a07d78e4..17784eee6 100644 --- a/internal/oidc/provider_test.go +++ b/internal/oidc/provider_test.go @@ -1,6 +1,7 @@ package oidc import ( + "net/url" "testing" "github.com/stretchr/testify/assert" @@ -27,6 +28,39 @@ func TestOpenIDConnectProvider_NewOpenIDConnectProvider_BadIssuerKey(t *testing. assert.Error(t, err, "abc") } +func TestNewOpenIDConnectProvider_ShouldEnableOptionalDiscoveryValues(t *testing.T) { + provider, err := NewOpenIDConnectProvider(&schema.OpenIDConnectConfiguration{ + IssuerPrivateKey: exampleIssuerPrivateKey, + EnablePKCEPlainChallenge: true, + HMACSecret: "asbdhaaskmdlkamdklasmdlkams", + Clients: []schema.OpenIDConnectClientConfiguration{ + { + ID: "a-client", + Secret: "a-client-secret", + SectorIdentifier: url.URL{Host: "google.com"}, + Policy: "one_factor", + RedirectURIs: []string{ + "https://google.com", + }, + }, + }, + }, nil) + + assert.NoError(t, err) + + assert.True(t, provider.Pairwise()) + + disco := provider.GetOpenIDConnectWellKnownConfiguration("https://example.com") + + assert.Len(t, disco.SubjectTypesSupported, 2) + assert.Contains(t, disco.SubjectTypesSupported, "public") + assert.Contains(t, disco.SubjectTypesSupported, "pairwise") + + assert.Len(t, disco.CodeChallengeMethodsSupported, 2) + assert.Contains(t, disco.CodeChallengeMethodsSupported, "S256") + assert.Contains(t, disco.CodeChallengeMethodsSupported, "S256") +} + func TestOpenIDConnectProvider_NewOpenIDConnectProvider_GoodConfiguration(t *testing.T) { provider, err := NewOpenIDConnectProvider(&schema.OpenIDConnectConfiguration{ IssuerPrivateKey: exampleIssuerPrivateKey, diff --git a/internal/oidc/types.go b/internal/oidc/types.go index e8859d75f..a144295e6 100644 --- a/internal/oidc/types.go +++ b/internal/oidc/types.go @@ -97,9 +97,9 @@ type OpenIDConnectStore struct { // Client represents the client internally. type Client struct { ID string - SectorIdentifier string Description string Secret []byte + SectorIdentifier string Public bool Policy authorization.Level