diff --git a/internal/configuration/const.go b/internal/configuration/const.go index 7b6321784..cc9b5a6fb 100644 --- a/internal/configuration/const.go +++ b/internal/configuration/const.go @@ -28,7 +28,9 @@ var ( const ( errFmtSecretAlreadyDefined = "secrets: error loading secret into key '%s': it's already defined in other " + "configuration sources" - errFmtSecretIOIssue = "secrets: error loading secret path %s into key '%s': %v" + errFmtSecretOSError = "secrets: error loading secret path %s into key '%s': %w" + errFmtSecretOSPermission = "secrets: error loading secret path %s into key '%s': file permission error occurred: %w" + errFmtSecretOSNotExist = "secrets: error loading secret path %s into key '%s': file does not exist error occurred: %w" errFmtGenerateConfiguration = "error occurred generating configuration: %+v" errFmtDecodeHookCouldNotParse = "could not decode '%s' to a %s%s: %w" diff --git a/internal/configuration/koanf_callbacks.go b/internal/configuration/koanf_callbacks.go index d62984eb1..b7218a639 100644 --- a/internal/configuration/koanf_callbacks.go +++ b/internal/configuration/koanf_callbacks.go @@ -2,6 +2,7 @@ package configuration import ( "fmt" + "os" "strings" "github.com/spf13/pflag" @@ -40,13 +41,25 @@ func koanfEnvironmentSecretsCallback(keyMap map[string]string, validator *schema return "", nil } - v, err := loadSecret(value) - if err != nil { - validator.Push(fmt.Errorf(errFmtSecretIOIssue, value, k, err)) - return k, "" - } + switch v, err := loadSecret(value); err { + case nil: + return k, v + default: + switch { + case os.IsNotExist(err): + validator.Push(fmt.Errorf(errFmtSecretOSNotExist, value, k, err)) - return k, v + return "", nil + case os.IsPermission(err): + validator.Push(fmt.Errorf(errFmtSecretOSPermission, value, k, err)) + + return "", nil + default: + validator.Push(fmt.Errorf(errFmtSecretOSError, value, k, err)) + + return "", nil + } + } } } diff --git a/internal/configuration/koanf_callbacks_test.go b/internal/configuration/koanf_callbacks_test.go index 561038efd..039f18637 100644 --- a/internal/configuration/koanf_callbacks_test.go +++ b/internal/configuration/koanf_callbacks_test.go @@ -117,10 +117,10 @@ func TestKoanfSecretCallbackShouldErrorOnFSError(t *testing.T) { callback := koanfEnvironmentSecretsCallback(keyMap, val) key, value := callback("AUTHELIA_THEME", secret) - assert.Equal(t, "theme", key) - assert.Equal(t, "", value) + assert.Equal(t, "", key) + assert.Equal(t, nil, value) require.Len(t, val.Errors(), 1) assert.Len(t, val.Warnings(), 0) - assert.EqualError(t, val.Errors()[0], fmt.Sprintf(errFmtSecretIOIssue, secret, "theme", fmt.Sprintf("open %s: permission denied", secret))) + assert.EqualError(t, val.Errors()[0], fmt.Sprintf("secrets: error loading secret path %s into key 'theme': file permission error occurred: open %s: permission denied", secret, secret)) } diff --git a/internal/configuration/provider_test.go b/internal/configuration/provider_test.go index f60d68651..f91b9629b 100644 --- a/internal/configuration/provider_test.go +++ b/internal/configuration/provider_test.go @@ -22,7 +22,7 @@ func TestShouldErrorSecretNotExist(t *testing.T) { testSetEnv(t, "JWT_SECRET_FILE", filepath.Join(dir, "jwt")) testSetEnv(t, "DUO_API_SECRET_KEY_FILE", filepath.Join(dir, "duo")) testSetEnv(t, "SESSION_SECRET_FILE", filepath.Join(dir, "session")) - testSetEnv(t, "AUTHENTICATION_BACKEND_LDAP_PASSWORD_FILE", filepath.Join(dir, "authentication")) + testSetEnv(t, "AUTHENTICATION_BACKEND_LDAP_PASSWORD_FILE", dir) testSetEnv(t, "NOTIFIER_SMTP_PASSWORD_FILE", filepath.Join(dir, "notifier")) testSetEnv(t, "SESSION_REDIS_PASSWORD_FILE", filepath.Join(dir, "redis")) testSetEnv(t, "SESSION_REDIS_HIGH_AVAILABILITY_SENTINEL_PASSWORD_FILE", filepath.Join(dir, "redis-sentinel")) @@ -44,20 +44,21 @@ func TestShouldErrorSecretNotExist(t *testing.T) { sort.Sort(utils.ErrSliceSortAlphabetical(errs)) errFmt := utils.GetExpectedErrTxt("filenotfound") + errFmtDir := utils.GetExpectedErrTxt("isdir") // ignore the errors before this as they are checked by the validator. - assert.EqualError(t, errs[0], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "authentication"), "authentication_backend.ldap.password", fmt.Sprintf(errFmt, filepath.Join(dir, "authentication")))) - assert.EqualError(t, errs[1], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "duo"), "duo_api.secret_key", fmt.Sprintf(errFmt, filepath.Join(dir, "duo")))) - assert.EqualError(t, errs[2], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "jwt"), "jwt_secret", fmt.Sprintf(errFmt, filepath.Join(dir, "jwt")))) - assert.EqualError(t, errs[3], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "mysql"), "storage.mysql.password", fmt.Sprintf(errFmt, filepath.Join(dir, "mysql")))) - assert.EqualError(t, errs[4], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "notifier"), "notifier.smtp.password", fmt.Sprintf(errFmt, filepath.Join(dir, "notifier")))) - assert.EqualError(t, errs[5], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "oidc-hmac"), "identity_providers.oidc.hmac_secret", fmt.Sprintf(errFmt, filepath.Join(dir, "oidc-hmac")))) - assert.EqualError(t, errs[6], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "oidc-key"), "identity_providers.oidc.issuer_private_key", fmt.Sprintf(errFmt, filepath.Join(dir, "oidc-key")))) - assert.EqualError(t, errs[7], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "postgres"), "storage.postgres.password", fmt.Sprintf(errFmt, filepath.Join(dir, "postgres")))) - assert.EqualError(t, errs[8], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "redis"), "session.redis.password", fmt.Sprintf(errFmt, filepath.Join(dir, "redis")))) - assert.EqualError(t, errs[9], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "redis-sentinel"), "session.redis.high_availability.sentinel_password", fmt.Sprintf(errFmt, filepath.Join(dir, "redis-sentinel")))) - assert.EqualError(t, errs[10], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "session"), "session.secret", fmt.Sprintf(errFmt, filepath.Join(dir, "session")))) - assert.EqualError(t, errs[11], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "tls"), "server.tls.key", fmt.Sprintf(errFmt, filepath.Join(dir, "tls")))) + assert.EqualError(t, errs[0], fmt.Sprintf("secrets: error loading secret path %s into key 'authentication_backend.ldap.password': %s", dir, fmt.Sprintf(errFmtDir, dir))) + assert.EqualError(t, errs[1], fmt.Sprintf("secrets: error loading secret path %s into key 'duo_api.secret_key': file does not exist error occurred: %s", filepath.Join(dir, "duo"), fmt.Sprintf(errFmt, filepath.Join(dir, "duo")))) + assert.EqualError(t, errs[2], fmt.Sprintf("secrets: error loading secret path %s into key 'jwt_secret': file does not exist error occurred: %s", filepath.Join(dir, "jwt"), fmt.Sprintf(errFmt, filepath.Join(dir, "jwt")))) + assert.EqualError(t, errs[3], fmt.Sprintf("secrets: error loading secret path %s into key 'storage.mysql.password': file does not exist error occurred: %s", filepath.Join(dir, "mysql"), fmt.Sprintf(errFmt, filepath.Join(dir, "mysql")))) + assert.EqualError(t, errs[4], fmt.Sprintf("secrets: error loading secret path %s into key 'notifier.smtp.password': file does not exist error occurred: %s", filepath.Join(dir, "notifier"), fmt.Sprintf(errFmt, filepath.Join(dir, "notifier")))) + assert.EqualError(t, errs[5], fmt.Sprintf("secrets: error loading secret path %s into key 'identity_providers.oidc.hmac_secret': file does not exist error occurred: %s", filepath.Join(dir, "oidc-hmac"), fmt.Sprintf(errFmt, filepath.Join(dir, "oidc-hmac")))) + assert.EqualError(t, errs[6], fmt.Sprintf("secrets: error loading secret path %s into key 'identity_providers.oidc.issuer_private_key': file does not exist error occurred: %s", filepath.Join(dir, "oidc-key"), fmt.Sprintf(errFmt, filepath.Join(dir, "oidc-key")))) + assert.EqualError(t, errs[7], fmt.Sprintf("secrets: error loading secret path %s into key 'storage.postgres.password': file does not exist error occurred: %s", filepath.Join(dir, "postgres"), fmt.Sprintf(errFmt, filepath.Join(dir, "postgres")))) + assert.EqualError(t, errs[8], fmt.Sprintf("secrets: error loading secret path %s into key 'session.redis.password': file does not exist error occurred: %s", filepath.Join(dir, "redis"), fmt.Sprintf(errFmt, filepath.Join(dir, "redis")))) + assert.EqualError(t, errs[9], fmt.Sprintf("secrets: error loading secret path %s into key 'session.redis.high_availability.sentinel_password': file does not exist error occurred: %s", filepath.Join(dir, "redis-sentinel"), fmt.Sprintf(errFmt, filepath.Join(dir, "redis-sentinel")))) + assert.EqualError(t, errs[10], fmt.Sprintf("secrets: error loading secret path %s into key 'session.secret': file does not exist error occurred: %s", filepath.Join(dir, "session"), fmt.Sprintf(errFmt, filepath.Join(dir, "session")))) + assert.EqualError(t, errs[11], fmt.Sprintf("secrets: error loading secret path %s into key 'server.tls.key': file does not exist error occurred: %s", filepath.Join(dir, "tls"), fmt.Sprintf(errFmt, filepath.Join(dir, "tls")))) } func TestLoadShouldReturnErrWithoutValidator(t *testing.T) { diff --git a/internal/configuration/validator/const.go b/internal/configuration/validator/const.go index 0a91a138f..44ac2622b 100644 --- a/internal/configuration/validator/const.go +++ b/internal/configuration/validator/const.go @@ -142,11 +142,12 @@ const ( const ( errFmtOIDCNoClientsConfigured = "identity_providers: oidc: option 'clients' must have one or " + "more clients configured" - errFmtOIDCNoPrivateKey = "identity_providers: oidc: option 'issuer_private_key' is required" - errFmtOIDCInvalidPrivateKeyBitSize = "identity_providers: oidc: option 'issuer_private_key' must be an RSA private key with %d bits or more but it only has %d bits" - errFmtOIDCCertificateMismatch = "identity_providers: oidc: option 'issuer_private_key' does not appear to be the private key the certificate provided by option 'issuer_certificate_chain'" - errFmtOIDCCertificateChain = "identity_providers: oidc: option 'issuer_certificate_chain' produced an error during validation of the chain: %w" - errFmtOIDCEnforcePKCEInvalidValue = "identity_providers: oidc: option 'enforce_pkce' must be 'never', " + + errFmtOIDCNoPrivateKey = "identity_providers: oidc: option 'issuer_private_key' is required" + errFmtOIDCInvalidPrivateKeyBitSize = "identity_providers: oidc: option 'issuer_private_key' must be an RSA private key with %d bits or more but it only has %d bits" + errFmtOIDCInvalidPrivateKeyMalformedMissingPublicKey = "identity_providers: oidc: option 'issuer_private_key' must be a valid RSA private key but the provided data is missing the public key bits" + errFmtOIDCCertificateMismatch = "identity_providers: oidc: option 'issuer_private_key' does not appear to be the private key the certificate provided by option 'issuer_certificate_chain'" + errFmtOIDCCertificateChain = "identity_providers: oidc: option 'issuer_certificate_chain' produced an error during validation of the chain: %w" + errFmtOIDCEnforcePKCEInvalidValue = "identity_providers: oidc: option 'enforce_pkce' must be 'never', " + "'public_clients_only' or 'always', but it is configured as '%s'" errFmtOIDCCORSInvalidOrigin = "identity_providers: oidc: cors: option 'allowed_origins' contains an invalid value '%s' as it has a %s: origins must only be scheme, hostname, and an optional port" diff --git a/internal/configuration/validator/identity_providers.go b/internal/configuration/validator/identity_providers.go index 95f7c2707..b9ea7e9b9 100644 --- a/internal/configuration/validator/identity_providers.go +++ b/internal/configuration/validator/identity_providers.go @@ -37,7 +37,9 @@ func validateOIDC(config *schema.OpenIDConnectConfiguration, val *schema.StructV } } - if config.IssuerPrivateKey.Size()*8 < 2048 { + if config.IssuerPrivateKey.PublicKey.N == nil { + val.Push(fmt.Errorf(errFmtOIDCInvalidPrivateKeyMalformedMissingPublicKey)) + } else if config.IssuerPrivateKey.Size()*8 < 2048 { val.Push(fmt.Errorf(errFmtOIDCInvalidPrivateKeyBitSize, 2048, config.IssuerPrivateKey.Size()*8)) } } diff --git a/internal/configuration/validator/identity_providers_test.go b/internal/configuration/validator/identity_providers_test.go index d77844046..8957c61f8 100644 --- a/internal/configuration/validator/identity_providers_test.go +++ b/internal/configuration/validator/identity_providers_test.go @@ -525,6 +525,35 @@ func TestShouldRaiseErrorOnKeySizeTooSmall(t *testing.T) { assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: option 'issuer_private_key' must be an RSA private key with 2048 bits or more but it only has 1024 bits") } +func TestShouldRaiseErrorOnKeyInvalidPublicKey(t *testing.T) { + validator := schema.NewStructValidator() + config := &schema.IdentityProvidersConfiguration{ + OIDC: &schema.OpenIDConnectConfiguration{ + HMACSecret: "rLABDrx87et5KvRHVUgTm3pezWWd8LMN", + IssuerPrivateKey: MustParseRSAPrivateKey(testKey3), + Clients: []schema.OpenIDConnectClientConfiguration{ + { + ID: "good_id", + Secret: MustDecodeSecret(goodOpenIDConnectClientSecret), + Policy: "two_factor", + RedirectURIs: []string{ + "https://google.com/callback", + }, + }, + }, + }, + } + + config.OIDC.IssuerPrivateKey.PublicKey.N = nil + + ValidateIdentityProviders(config, validator) + + assert.Len(t, validator.Warnings(), 0) + require.Len(t, validator.Errors(), 1) + + assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: option 'issuer_private_key' must be a valid RSA private key but the provided data is missing the public key bits") +} + func TestShouldRaiseErrorWhenOIDCClientConfiguredWithBadResponseModes(t *testing.T) { validator := schema.NewStructValidator() config := &schema.IdentityProvidersConfiguration{ diff --git a/internal/utils/const.go b/internal/utils/const.go index 3ce72025f..eb53c2d93 100644 --- a/internal/utils/const.go +++ b/internal/utils/const.go @@ -122,6 +122,15 @@ var htmlEscaper = strings.NewReplacer( var ErrTimeoutReached = errors.New("timeout reached") const ( - windows = "windows" - errFmtLinuxNotFound = "open %s: no such file or directory" + windows = "windows" + errFmtLinuxNotFound = "%s %%s: no such file or directory" + errFmtWindowsNotFound = "%s %%s: The system cannot find the %s specified." + + strStat = "stat" + strOpen = "open" + strFile = "file" + strPath = "path" + strIsDir = "isdir" + strPathNotFound = "pathnotfound" + strFileNotFound = "filenotfound" ) diff --git a/internal/utils/errs.go b/internal/utils/errs.go index 7dc0f828e..a96b9e1ea 100644 --- a/internal/utils/errs.go +++ b/internal/utils/errs.go @@ -1,6 +1,9 @@ package utils -import "runtime" +import ( + "fmt" + "runtime" +) // ErrSliceSortAlphabetical is a helper type that can be used with sort.Sort to sort a slice of errors in alphabetical // order. Usage is simple just do sort.Sort(ErrSliceSortAlphabetical([]error{})). @@ -12,28 +15,29 @@ func (s ErrSliceSortAlphabetical) Less(i, j int) bool { return s[i].Error() < s[ func (s ErrSliceSortAlphabetical) Swap(i, j int) { s[i], s[j] = s[j], s[i] } -// GetExpectedErrTxt returns error text for expected errs. +// GetExpectedErrTxt returns error text for expected errs. THIS IS A TEST UTILITY FUNCTION. func GetExpectedErrTxt(err string) string { - switch err { - case "pathnotfound": - switch runtime.GOOS { - case windows: - return "open %s: The system cannot find the path specified." - default: - return errFmtLinuxNotFound - } - case "filenotfound": - switch runtime.GOOS { - case windows: - return "open %s: The system cannot find the file specified." - default: - return errFmtLinuxNotFound - } - case "yamlisdir": - switch runtime.GOOS { - case windows: + switch runtime.GOOS { + case windows: + switch err { + case strPathNotFound: + return fmt.Sprintf(errFmtWindowsNotFound, strOpen, strPath) + case strStat + strPathNotFound: + return fmt.Sprintf(errFmtWindowsNotFound, strStat, strPath) + case strFileNotFound: + return fmt.Sprintf(errFmtWindowsNotFound, strOpen, strFile) + case strStat + strFileNotFound: + return fmt.Sprintf(errFmtWindowsNotFound, strStat, strFile) + case strIsDir: return "read %s: The handle is invalid." - default: + } + default: + switch err { + case strPathNotFound, strFileNotFound: + return fmt.Sprintf(errFmtLinuxNotFound, strOpen) + case strStat + strPathNotFound, strStat + strFileNotFound: + return fmt.Sprintf(errFmtLinuxNotFound, strStat) + case strIsDir: return "read %s: is a directory" } }