feat(oidc): disable minimum parameter entropy (#5495)

This allows disabling the minimum parameter entropy checks.

Signed-off-by: James Elliott <james-d-elliott@users.noreply.github.com>
pull/5490/head
James Elliott 2023-05-28 11:50:55 +10:00 committed by GitHub
parent dd7fea68f5
commit 32c68804e0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 101 additions and 25 deletions

View File

@ -305,6 +305,8 @@ make certain scenarios less secure. It is highly encouraged that if your OpenID
these parameters or sends parameters with a lower length than the default that they implement a change rather than these parameters or sends parameters with a lower length than the default that they implement a change rather than
changing this value. changing this value.
This restriction can also be disabled entirely when set to `-1`.
### enforce_pkce ### enforce_pkce
{{< confkey type="string" default="public_clients_only" required="no" >}} {{< confkey type="string" default="public_clients_only" required="no" >}}

View File

@ -147,8 +147,10 @@ const (
errFmtOIDCProviderNoPrivateKey = "identity_providers: oidc: option `issuer_private_keys` or 'issuer_private_key' is required" errFmtOIDCProviderNoPrivateKey = "identity_providers: oidc: option `issuer_private_keys` or 'issuer_private_key' is required"
errFmtOIDCProviderEnforcePKCEInvalidValue = "identity_providers: oidc: option 'enforce_pkce' must be 'never', " + errFmtOIDCProviderEnforcePKCEInvalidValue = "identity_providers: oidc: option 'enforce_pkce' must be 'never', " +
"'public_clients_only' or 'always', but it's configured as '%s'" "'public_clients_only' or 'always', but it's configured as '%s'"
errFmtOIDCProviderInsecureParameterEntropy = "openid connect provider: SECURITY ISSUE - minimum parameter entropy is " + errFmtOIDCProviderInsecureParameterEntropy = "identity_providers: oidc: option 'minimum_parameter_entropy' is " +
"configured to an unsafe value, it should be above 8 but it's configured to %d" "configured to an unsafe and insecure value, it should at least be %d but it's configured to %d"
errFmtOIDCProviderInsecureDisabledParameterEntropy = "identity_providers: oidc: option 'minimum_parameter_entropy' is " +
"disabled which is considered unsafe and insecure"
errFmtOIDCProviderPrivateKeysInvalid = "identity_providers: oidc: issuer_private_keys: key #%d: option 'key' must be a valid private key but the provided data is malformed as it's missing the public key bits" errFmtOIDCProviderPrivateKeysInvalid = "identity_providers: oidc: issuer_private_keys: key #%d: option 'key' must be a valid private key but the provided data is malformed as it's missing the public key bits"
errFmtOIDCProviderPrivateKeysCalcThumbprint = "identity_providers: oidc: issuer_private_keys: key #%d: option 'key' failed to calculate thumbprint to configure key id value: %w" errFmtOIDCProviderPrivateKeysCalcThumbprint = "identity_providers: oidc: issuer_private_keys: key #%d: option 'key' failed to calculate thumbprint to configure key id value: %w"
errFmtOIDCProviderPrivateKeysKeyIDLength = "identity_providers: oidc: issuer_private_keys: key #%d with key id '%s': option `key_id` must be 100 characters or less" errFmtOIDCProviderPrivateKeysKeyIDLength = "identity_providers: oidc: issuer_private_keys: key #%d with key id '%s': option `key_id` must be 100 characters or less"

View File

@ -10,6 +10,8 @@ import (
"strings" "strings"
"time" "time"
"github.com/ory/fosite"
"github.com/authelia/authelia/v4/internal/configuration/schema" "github.com/authelia/authelia/v4/internal/configuration/schema"
"github.com/authelia/authelia/v4/internal/oidc" "github.com/authelia/authelia/v4/internal/oidc"
"github.com/authelia/authelia/v4/internal/utils" "github.com/authelia/authelia/v4/internal/utils"
@ -31,8 +33,13 @@ func validateOIDC(config *schema.OpenIDConnect, val *schema.StructValidator) {
sort.Sort(oidc.SortedSigningAlgs(config.Discovery.ResponseObjectSigningAlgs)) sort.Sort(oidc.SortedSigningAlgs(config.Discovery.ResponseObjectSigningAlgs))
if config.MinimumParameterEntropy != 0 && config.MinimumParameterEntropy < 8 { switch {
val.PushWarning(fmt.Errorf(errFmtOIDCProviderInsecureParameterEntropy, config.MinimumParameterEntropy)) case config.MinimumParameterEntropy == -1:
val.PushWarning(fmt.Errorf(errFmtOIDCProviderInsecureDisabledParameterEntropy))
case config.MinimumParameterEntropy <= 0:
config.MinimumParameterEntropy = fosite.MinParameterEntropy
case config.MinimumParameterEntropy < fosite.MinParameterEntropy:
val.PushWarning(fmt.Errorf(errFmtOIDCProviderInsecureParameterEntropy, fosite.MinParameterEntropy, config.MinimumParameterEntropy))
} }
switch config.EnforcePKCE { switch config.EnforcePKCE {

View File

@ -504,13 +504,59 @@ func TestShouldRaiseErrorOnCertificateNotValid(t *testing.T) {
assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: issuer_private_keys: key #1 with key id 'bf1e10': option 'certificate_chain' does not appear to contain the public key for the private key provided by option 'key'") assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: issuer_private_keys: key #1 with key id 'bf1e10': option 'certificate_chain' does not appear to contain the public key for the private key provided by option 'key'")
} }
func TestValidateIdentityProvidersShouldRaiseWarningOnSecurityIssue(t *testing.T) { func TestValidateIdentityProvidersOpenIDConnectMinimumParameterEntropy(t *testing.T) {
testCases := []struct {
name string
have int
expected int
warnings []string
errors []string
}{
{
"ShouldNotOverrideCustomValue",
20,
20,
nil,
nil,
},
{
"ShouldSetDefault",
0,
8,
nil,
nil,
},
{
"ShouldSetDefaultNegative",
-2,
8,
nil,
nil,
},
{
"ShouldAllowDisabledAndWarn",
-1,
-1,
[]string{"identity_providers: oidc: option 'minimum_parameter_entropy' is disabled which is considered unsafe and insecure"},
nil,
},
{
"ShouldWarnOnTooLow",
2,
2,
[]string{"identity_providers: oidc: option 'minimum_parameter_entropy' is configured to an unsafe and insecure value, it should at least be 8 but it's configured to 2"},
nil,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
validator := schema.NewStructValidator() validator := schema.NewStructValidator()
config := &schema.IdentityProviders{ config := &schema.IdentityProviders{
OIDC: &schema.OpenIDConnect{ OIDC: &schema.OpenIDConnect{
HMACSecret: "abc", HMACSecret: "abc",
IssuerPrivateKey: keyRSA2048, IssuerPrivateKey: keyRSA2048,
MinimumParameterEntropy: 1, MinimumParameterEntropy: tc.have,
Clients: []schema.OpenIDConnectClient{ Clients: []schema.OpenIDConnectClient{
{ {
ID: "good_id", ID: "good_id",
@ -526,10 +572,29 @@ func TestValidateIdentityProvidersShouldRaiseWarningOnSecurityIssue(t *testing.T
ValidateIdentityProviders(config, validator) ValidateIdentityProviders(config, validator)
assert.Len(t, validator.Errors(), 0) assert.Equal(t, tc.expected, config.OIDC.MinimumParameterEntropy)
require.Len(t, validator.Warnings(), 1)
assert.EqualError(t, validator.Warnings()[0], "openid connect provider: SECURITY ISSUE - minimum parameter entropy is configured to an unsafe value, it should be above 8 but it's configured to 1") if n := len(tc.warnings); n == 0 {
assert.Len(t, validator.Warnings(), 0)
} else {
require.Len(t, validator.Warnings(), n)
for i := 0; i < n; i++ {
assert.EqualError(t, validator.Warnings()[i], tc.warnings[i])
}
}
if n := len(tc.errors); n == 0 {
assert.Len(t, validator.Errors(), 0)
} else {
require.Len(t, validator.Errors(), n)
for i := 0; i < n; i++ {
assert.EqualError(t, validator.Errors()[i], tc.errors[i])
}
}
})
}
} }
func TestValidateIdentityProvidersShouldRaiseErrorsOnInvalidClientTypes(t *testing.T) { func TestValidateIdentityProvidersShouldRaiseErrorsOnInvalidClientTypes(t *testing.T) {