From 9b6bcca1baa62b6507d7de03941c7faf2a1a55fe Mon Sep 17 00:00:00 2001 From: James Elliott Date: Fri, 8 Apr 2022 09:01:01 +1000 Subject: [PATCH] feat(totp): secret customization (#2681) Allow customizing the shared secrets size specifically for apps which don't support 256bit shared secrets. --- config.template.yml | 3 + docs/configuration/one-time-password.md | 30 +++- internal/commands/storage.go | 4 + internal/commands/storage_run.go | 59 ++++--- internal/configuration/config.template.yml | 3 + internal/configuration/schema/const.go | 8 + internal/configuration/schema/totp.go | 24 +-- internal/configuration/validator/const.go | 10 +- internal/configuration/validator/totp.go | 10 ++ internal/configuration/validator/totp_test.go | 154 ++++++++++++------ internal/mocks/totp.go | 8 +- internal/totp/provider.go | 2 +- internal/totp/totp.go | 15 +- internal/totp/totp_test.go | 146 ++++++++++++----- 14 files changed, 333 insertions(+), 143 deletions(-) diff --git a/config.template.yml b/config.template.yml index f7165a2d1..9e46ed6f3 100644 --- a/config.template.yml +++ b/config.template.yml @@ -127,6 +127,9 @@ totp: skew: 1 ## See: https://www.authelia.com/docs/configuration/one-time-password.html#input-validation to read the documentation. + ## The size of the generated shared secrets. Default is 32 and is sufficient in most use cases, minimum is 20. + secret_size: 32 + ## ## WebAuthn Configuration ## diff --git a/docs/configuration/one-time-password.md b/docs/configuration/one-time-password.md index 48b4a2a86..2b85a3d78 100644 --- a/docs/configuration/one-time-password.md +++ b/docs/configuration/one-time-password.md @@ -7,9 +7,9 @@ nav_order: 16 # Time-based One-Time Password -Authelia uses time-based one-time passwords as the OTP method. You have -the option to tune the settings of the TOTP generation, and you can see a -full example of TOTP configuration below, as well as sections describing them. +The OTP method _Authelia_ uses is the Time-Based One-Time Password Algorithm (TOTP) [RFC6238] which is an extension of +HMAC-Based One-Time Password Algorithm (HOTP) [RFC4226]. You have the option to tune the settings of theTOTP generation, +and you can see a full example of TOTP configuration below, as well as sections describing them. ## Configuration ```yaml @@ -20,6 +20,7 @@ totp: digits: 6 period: 30 skew: 1 + secret_size: 32 ``` ## Options @@ -139,6 +140,21 @@ other. Changing this value affects all TOTP validations, not just newly registered ones. +### secret_size +
+type: integer +{: .label .label-config .label-purple } +default: 32 +{: .label .label-config .label-blue } +required: no +{: .label .label-config .label-green } +
+ +The length in bytes of generated shared secrets. The minimum is 20 (or 160 bits), and the default is 32 (or 256 bits). +In most use cases 32 is sufficient. Though some authenticators may have issues with more than the minimum. Our minimum +is the recommended value in [RFC4226], though technically according to the specification 16 bytes (or 128 bits) is the +minimum. + ## Registration When users register their TOTP device for the first time, the current [issuer](#issuer), [algorithm](#algorithm), and [period](#period) are used to generate the TOTP link and QR code. These values are saved to the database for future @@ -153,9 +169,8 @@ users to register a new device, you can delete the old device for a particular u The period and skew configuration parameters affect each other. The default values are a period of 30 and a skew of 1. It is highly recommended you do not change these unless you wish to set skew to 0. -The way you configure these affects security by changing the length of time a one-time -password is valid for. The formula to calculate the effective validity period is -`period + (period * skew * 2)`. For example period 30 and skew 1 would result in 90 +These options affect security by changing the length of time a one-time password is valid for. The formula to calculate +the effective validity period is `period + (period * skew * 2)`. For example period 30 and skew 1 would result in 90 seconds of validity, and period 30 and skew 2 would result in 150 seconds of validity. ## System time accuracy @@ -192,3 +207,6 @@ Help: ```shell $ authelia storage totp export --help ``` + +[RFC4226]: https://datatracker.ietf.org/doc/html/rfc4226 +[RFC6238]: https://datatracker.ietf.org/doc/html/rfc6238 \ No newline at end of file diff --git a/internal/commands/storage.go b/internal/commands/storage.go index 173abcada..1aa44813f 100644 --- a/internal/commands/storage.go +++ b/internal/commands/storage.go @@ -2,6 +2,8 @@ package commands import ( "github.com/spf13/cobra" + + "github.com/authelia/authelia/v4/internal/configuration/schema" ) // NewStorageCmd returns a new storage *cobra.Command. @@ -107,6 +109,8 @@ func newStorageTOTPGenerateCmd() (cmd *cobra.Command) { Args: cobra.ExactArgs(1), } + cmd.Flags().String("secret", "", "Optionally set the TOTP shared secret as base32 encoded bytes (no padding), it's recommended to not set this option unless you're restoring an TOTP config") + cmd.Flags().Uint("secret-size", schema.TOTPSecretSizeDefault, "set the TOTP secret size") cmd.Flags().Uint("period", 30, "set the TOTP period") cmd.Flags().Uint("digits", 6, "set the TOTP digits") cmd.Flags().String("algorithm", "SHA1", "set the TOTP algorithm") diff --git a/internal/commands/storage_run.go b/internal/commands/storage_run.go index 1193d672c..38ffb85a3 100644 --- a/internal/commands/storage_run.go +++ b/internal/commands/storage_run.go @@ -2,6 +2,7 @@ package commands import ( "context" + "encoding/base32" "errors" "fmt" "image" @@ -11,6 +12,7 @@ import ( "strings" "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/authelia/authelia/v4/internal/configuration" "github.com/authelia/authelia/v4/internal/configuration/schema" @@ -63,10 +65,11 @@ func storagePersistentPreRunE(cmd *cobra.Command, _ []string) (err error) { "postgres.ssl.certificate": "storage.postgres.ssl.certificate", "postgres.ssl.key": "storage.postgres.ssl.key", - "period": "totp.period", - "digits": "totp.digits", - "algorithm": "totp.algorithm", - "issuer": "totp.issuer", + "period": "totp.period", + "digits": "totp.digits", + "algorithm": "totp.algorithm", + "issuer": "totp.issuer", + "secret-size": "totp.secret_size", } sources = append(sources, configuration.NewEnvironmentSource(configuration.DefaultEnvPrefix, configuration.DefaultEnvDelimiter)) @@ -201,13 +204,13 @@ func storageSchemaEncryptionChangeKeyRunE(cmd *cobra.Command, args []string) (er func storageTOTPGenerateRunE(cmd *cobra.Command, args []string) (err error) { var ( - provider storage.Provider - ctx = context.Background() - c *model.TOTPConfiguration - force bool - filename string - file *os.File - img image.Image + provider storage.Provider + ctx = context.Background() + c *model.TOTPConfiguration + force bool + filename, secret string + file *os.File + img image.Image ) provider = getStorageProvider() @@ -216,25 +219,19 @@ func storageTOTPGenerateRunE(cmd *cobra.Command, args []string) (err error) { _ = provider.Close() }() - if force, err = cmd.Flags().GetBool("force"); err != nil { - return err - } - - if filename, err = cmd.Flags().GetString("path"); err != nil { + if force, filename, secret, err = storageTOTPGenerateRunEOptsFromFlags(cmd.Flags()); err != nil { return err } if _, err = provider.LoadTOTPConfiguration(ctx, args[0]); err == nil && !force { return fmt.Errorf("%s already has a TOTP configuration, use --force to overwrite", args[0]) - } - - if err != nil && !errors.Is(err, storage.ErrNoTOTPConfiguration) { + } else if err != nil && !errors.Is(err, storage.ErrNoTOTPConfiguration) { return err } totpProvider := totp.NewTimeBasedProvider(config.TOTP) - if c, err = totpProvider.Generate(args[0]); err != nil { + if c, err = totpProvider.GenerateCustom(args[0], config.TOTP.Algorithm, secret, config.TOTP.Digits, config.TOTP.Period, config.TOTP.SecretSize); err != nil { return err } @@ -271,6 +268,28 @@ func storageTOTPGenerateRunE(cmd *cobra.Command, args []string) (err error) { return nil } +func storageTOTPGenerateRunEOptsFromFlags(flags *pflag.FlagSet) (force bool, filename, secret string, err error) { + if force, err = flags.GetBool("force"); err != nil { + return force, filename, secret, err + } + + if filename, err = flags.GetString("path"); err != nil { + return force, filename, secret, err + } + + if secret, err = flags.GetString("secret"); err != nil { + return force, filename, secret, err + } + + secretLength := base32.StdEncoding.WithPadding(base32.NoPadding).DecodedLen(len(secret)) + if secret != "" && secretLength < schema.TOTPSecretSizeMinimum { + return force, filename, secret, fmt.Errorf("decoded length of the base32 secret must have "+ + "a length of more than %d but '%s' has a decoded length of %d", schema.TOTPSecretSizeMinimum, secret, secretLength) + } + + return force, filename, secret, nil +} + func storageTOTPDeleteRunE(cmd *cobra.Command, args []string) (err error) { var ( provider storage.Provider diff --git a/internal/configuration/config.template.yml b/internal/configuration/config.template.yml index f7165a2d1..9e46ed6f3 100644 --- a/internal/configuration/config.template.yml +++ b/internal/configuration/config.template.yml @@ -127,6 +127,9 @@ totp: skew: 1 ## See: https://www.authelia.com/docs/configuration/one-time-password.html#input-validation to read the documentation. + ## The size of the generated shared secrets. Default is 32 and is sufficient in most use cases, minimum is 20. + secret_size: 32 + ## ## WebAuthn Configuration ## diff --git a/internal/configuration/schema/const.go b/internal/configuration/schema/const.go index 94d72dd50..073712a6c 100644 --- a/internal/configuration/schema/const.go +++ b/internal/configuration/schema/const.go @@ -44,3 +44,11 @@ var ( // TOTPPossibleAlgorithms is a list of valid TOTP Algorithms. TOTPPossibleAlgorithms = []string{TOTPAlgorithmSHA1, TOTPAlgorithmSHA256, TOTPAlgorithmSHA512} ) + +const ( + // TOTPSecretSizeDefault is the default secret size. + TOTPSecretSizeDefault = 32 + + // TOTPSecretSizeMinimum is the minimum secret size. + TOTPSecretSizeMinimum = 20 +) diff --git a/internal/configuration/schema/totp.go b/internal/configuration/schema/totp.go index 4d8be8082..c6efbfed5 100644 --- a/internal/configuration/schema/totp.go +++ b/internal/configuration/schema/totp.go @@ -2,21 +2,23 @@ package schema // TOTPConfiguration represents the configuration related to TOTP options. type TOTPConfiguration struct { - Disable bool `koanf:"disable"` - Issuer string `koanf:"issuer"` - Algorithm string `koanf:"algorithm"` - Digits uint `koanf:"digits"` - Period uint `koanf:"period"` - Skew *uint `koanf:"skew"` + Disable bool `koanf:"disable"` + Issuer string `koanf:"issuer"` + Algorithm string `koanf:"algorithm"` + Digits uint `koanf:"digits"` + Period uint `koanf:"period"` + Skew *uint `koanf:"skew"` + SecretSize uint `koanf:"secret_size"` } var defaultOtpSkew = uint(1) // DefaultTOTPConfiguration represents default configuration parameters for TOTP generation. var DefaultTOTPConfiguration = TOTPConfiguration{ - Issuer: "Authelia", - Algorithm: TOTPAlgorithmSHA1, - Digits: 6, - Period: 30, - Skew: &defaultOtpSkew, + Issuer: "Authelia", + Algorithm: TOTPAlgorithmSHA1, + Digits: 6, + Period: 30, + Skew: &defaultOtpSkew, + SecretSize: TOTPSecretSizeDefault, } diff --git a/internal/configuration/validator/const.go b/internal/configuration/validator/const.go index 4f258161d..cf7390030 100644 --- a/internal/configuration/validator/const.go +++ b/internal/configuration/validator/const.go @@ -104,9 +104,10 @@ const ( // TOTP Error constants. const ( - errFmtTOTPInvalidAlgorithm = "totp: option 'algorithm' must be one of '%s' but it is configured as '%s'" - errFmtTOTPInvalidPeriod = "totp: option 'period' option must be 15 or more but it is configured as '%d'" - errFmtTOTPInvalidDigits = "totp: option 'digits' must be 6 or 8 but it is configured as '%d'" + errFmtTOTPInvalidAlgorithm = "totp: option 'algorithm' must be one of '%s' but it is configured as '%s'" + errFmtTOTPInvalidPeriod = "totp: option 'period' option must be 15 or more but it is configured as '%d'" + errFmtTOTPInvalidDigits = "totp: option 'digits' must be 6 or 8 but it is configured as '%d'" + errFmtTOTPInvalidSecretSize = "totp: option 'secret_size' must be %d or higher but it is configured as '%d'" //nolint:gosec ) // Storage Error constants. @@ -114,7 +115,7 @@ const ( errStrStorage = "storage: configuration for a 'local', 'mysql' or 'postgres' database must be provided" errStrStorageEncryptionKeyMustBeProvided = "storage: option 'encryption_key' must is required" errStrStorageEncryptionKeyTooShort = "storage: option 'encryption_key' must be 20 characters or longer" - errFmtStorageUserPassMustBeProvided = "storage: %s: option 'username' and 'password' are required" //nolint: gosec + errFmtStorageUserPassMustBeProvided = "storage: %s: option 'username' and 'password' are required" //nolint:gosec errFmtStorageOptionMustBeProvided = "storage: %s: option '%s' is required" errFmtStoragePostgreSQLInvalidSSLMode = "storage: postgres: ssl: option 'mode' must be one of '%s' but it is configured as '%s'" ) @@ -325,6 +326,7 @@ var ValidKeys = []string{ "totp.digits", "totp.period", "totp.skew", + "totp.secret_size", // Webauthn Keys. "webauthn.disable", diff --git a/internal/configuration/validator/totp.go b/internal/configuration/validator/totp.go index dbbda4484..0a379a067 100644 --- a/internal/configuration/validator/totp.go +++ b/internal/configuration/validator/totp.go @@ -10,6 +10,10 @@ import ( // ValidateTOTP validates and update TOTP configuration. func ValidateTOTP(config *schema.Configuration, validator *schema.StructValidator) { + if config.TOTP.Disable { + return + } + if config.TOTP.Issuer == "" { config.TOTP.Issuer = schema.DefaultTOTPConfiguration.Issuer } @@ -39,4 +43,10 @@ func ValidateTOTP(config *schema.Configuration, validator *schema.StructValidato if config.TOTP.Skew == nil { config.TOTP.Skew = schema.DefaultTOTPConfiguration.Skew } + + if config.TOTP.SecretSize == 0 { + config.TOTP.SecretSize = schema.DefaultTOTPConfiguration.SecretSize + } else if config.TOTP.SecretSize < schema.TOTPSecretSizeMinimum { + validator.Push(fmt.Errorf(errFmtTOTPInvalidSecretSize, schema.TOTPSecretSizeMinimum, config.TOTP.SecretSize)) + } } diff --git a/internal/configuration/validator/totp_test.go b/internal/configuration/validator/totp_test.go index e430b3a9e..85cd7471f 100644 --- a/internal/configuration/validator/totp_test.go +++ b/internal/configuration/validator/totp_test.go @@ -2,7 +2,6 @@ package validator import ( "fmt" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -11,63 +10,112 @@ import ( "github.com/authelia/authelia/v4/internal/configuration/schema" ) -func TestShouldSetDefaultTOTPValues(t *testing.T) { - validator := schema.NewStructValidator() - config := &schema.Configuration{ - TOTP: schema.TOTPConfiguration{}, - } - - ValidateTOTP(config, validator) - - require.Len(t, validator.Errors(), 0) - assert.Equal(t, "Authelia", config.TOTP.Issuer) - assert.Equal(t, schema.DefaultTOTPConfiguration.Algorithm, config.TOTP.Algorithm) - assert.Equal(t, schema.DefaultTOTPConfiguration.Skew, config.TOTP.Skew) - assert.Equal(t, schema.DefaultTOTPConfiguration.Period, config.TOTP.Period) -} - -func TestShouldNormalizeTOTPAlgorithm(t *testing.T) { - validator := schema.NewStructValidator() - - config := &schema.Configuration{ - TOTP: schema.TOTPConfiguration{ - Algorithm: "sha1", +func TestValidateTOTP(t *testing.T) { + testCases := []struct { + desc string + have schema.TOTPConfiguration + expected schema.TOTPConfiguration + errs []string + warns []string + }{ + { + desc: "ShouldSetDefaultTOTPValues", + expected: schema.DefaultTOTPConfiguration, + }, + { + desc: "ShouldNotSetDefaultTOTPValuesWhenDisabled", + have: schema.TOTPConfiguration{Disable: true}, + expected: schema.TOTPConfiguration{Disable: true}, + }, + { + desc: "ShouldNormalizeTOTPAlgorithm", + have: schema.TOTPConfiguration{ + Algorithm: "sha1", + Digits: 6, + Period: 30, + SecretSize: 32, + Skew: schema.DefaultTOTPConfiguration.Skew, + Issuer: "abc", + }, + expected: schema.TOTPConfiguration{ + Algorithm: "SHA1", + Digits: 6, + Period: 30, + SecretSize: 32, + Skew: schema.DefaultTOTPConfiguration.Skew, + Issuer: "abc", + }, + }, + { + desc: "ShouldRaiseErrorWhenInvalidTOTPAlgorithm", + have: schema.TOTPConfiguration{ + Algorithm: "sha3", + Digits: 6, + Period: 30, + SecretSize: 32, + Skew: schema.DefaultTOTPConfiguration.Skew, + Issuer: "abc", + }, + errs: []string{"totp: option 'algorithm' must be one of 'SHA1', 'SHA256', 'SHA512' but it is configured as 'SHA3'"}, + }, + { + desc: "ShouldRaiseErrorWhenInvalidTOTPValue", + have: schema.TOTPConfiguration{ + Algorithm: "sha3", + Period: 5, + Digits: 20, + SecretSize: 10, + Skew: schema.DefaultTOTPConfiguration.Skew, + Issuer: "abc", + }, + errs: []string{ + "totp: option 'algorithm' must be one of 'SHA1', 'SHA256', 'SHA512' but it is configured as 'SHA3'", + "totp: option 'period' option must be 15 or more but it is configured as '5'", + "totp: option 'digits' must be 6 or 8 but it is configured as '20'", + "totp: option 'secret_size' must be 20 or higher but it is configured as '10'", + }, }, } - ValidateTOTP(config, validator) + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + validator := schema.NewStructValidator() + config := &schema.Configuration{TOTP: tc.have} - assert.Len(t, validator.Errors(), 0) - assert.Equal(t, "SHA1", config.TOTP.Algorithm) -} + ValidateTOTP(config, validator) -func TestShouldRaiseErrorWhenInvalidTOTPAlgorithm(t *testing.T) { - validator := schema.NewStructValidator() + errs := validator.Errors() + warns := validator.Warnings() - config := &schema.Configuration{ - TOTP: schema.TOTPConfiguration{ - Algorithm: "sha3", - }, + if len(tc.errs) == 0 { + assert.Len(t, errs, 0) + assert.Len(t, warns, 0) + assert.Equal(t, tc.expected.Disable, config.TOTP.Disable) + assert.Equal(t, tc.expected.Issuer, config.TOTP.Issuer) + assert.Equal(t, tc.expected.Algorithm, config.TOTP.Algorithm) + assert.Equal(t, tc.expected.Skew, config.TOTP.Skew) + assert.Equal(t, tc.expected.Period, config.TOTP.Period) + assert.Equal(t, tc.expected.SecretSize, config.TOTP.SecretSize) + } else { + expectedErrs := len(tc.errs) + + require.Len(t, errs, expectedErrs) + + for i := 0; i < expectedErrs; i++ { + t.Run(fmt.Sprintf("Err%d", i+1), func(t *testing.T) { + assert.EqualError(t, errs[i], tc.errs[i]) + }) + } + } + + expectedWarns := len(tc.warns) + require.Len(t, warns, expectedWarns) + + for i := 0; i < expectedWarns; i++ { + t.Run(fmt.Sprintf("Err%d", i+1), func(t *testing.T) { + assert.EqualError(t, warns[i], tc.warns[i]) + }) + } + }) } - - ValidateTOTP(config, validator) - - require.Len(t, validator.Errors(), 1) - assert.EqualError(t, validator.Errors()[0], fmt.Sprintf(errFmtTOTPInvalidAlgorithm, strings.Join(schema.TOTPPossibleAlgorithms, "', '"), "SHA3")) -} - -func TestShouldRaiseErrorWhenInvalidTOTPValues(t *testing.T) { - validator := schema.NewStructValidator() - config := &schema.Configuration{ - TOTP: schema.TOTPConfiguration{ - Period: 5, - Digits: 20, - }, - } - - ValidateTOTP(config, validator) - - require.Len(t, validator.Errors(), 2) - assert.EqualError(t, validator.Errors()[0], fmt.Sprintf(errFmtTOTPInvalidPeriod, 5)) - assert.EqualError(t, validator.Errors()[1], fmt.Sprintf(errFmtTOTPInvalidDigits, 20)) } diff --git a/internal/mocks/totp.go b/internal/mocks/totp.go index c87855d07..673cdb8c0 100644 --- a/internal/mocks/totp.go +++ b/internal/mocks/totp.go @@ -51,18 +51,18 @@ func (mr *MockTOTPMockRecorder) Generate(arg0 interface{}) *gomock.Call { } // GenerateCustom mocks base method. -func (m *MockTOTP) GenerateCustom(arg0, arg1 string, arg2, arg3, arg4 uint) (*model.TOTPConfiguration, error) { +func (m *MockTOTP) GenerateCustom(arg0, arg1, arg2 string, arg3, arg4, arg5 uint) (*model.TOTPConfiguration, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GenerateCustom", arg0, arg1, arg2, arg3, arg4) + ret := m.ctrl.Call(m, "GenerateCustom", arg0, arg1, arg2, arg3, arg4, arg5) ret0, _ := ret[0].(*model.TOTPConfiguration) ret1, _ := ret[1].(error) return ret0, ret1 } // GenerateCustom indicates an expected call of GenerateCustom. -func (mr *MockTOTPMockRecorder) GenerateCustom(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { +func (mr *MockTOTPMockRecorder) GenerateCustom(arg0, arg1, arg2, arg3, arg4, arg5 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenerateCustom", reflect.TypeOf((*MockTOTP)(nil).GenerateCustom), arg0, arg1, arg2, arg3, arg4) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenerateCustom", reflect.TypeOf((*MockTOTP)(nil).GenerateCustom), arg0, arg1, arg2, arg3, arg4, arg5) } // Validate mocks base method. diff --git a/internal/totp/provider.go b/internal/totp/provider.go index a7a2f8b76..1280e4163 100644 --- a/internal/totp/provider.go +++ b/internal/totp/provider.go @@ -7,6 +7,6 @@ import ( // Provider for TOTP functionality. type Provider interface { Generate(username string) (config *model.TOTPConfiguration, err error) - GenerateCustom(username string, algorithm string, digits, period, secretSize uint) (config *model.TOTPConfiguration, err error) + GenerateCustom(username string, algorithm, secret string, digits, period, secretSize uint) (config *model.TOTPConfiguration, err error) Validate(token string, config *model.TOTPConfiguration) (valid bool, err error) } diff --git a/internal/totp/totp.go b/internal/totp/totp.go index 6bd7f5ce4..30cfcecc0 100644 --- a/internal/totp/totp.go +++ b/internal/totp/totp.go @@ -1,6 +1,8 @@ package totp import ( + "encoding/base32" + "fmt" "time" "github.com/pquerna/otp" @@ -32,13 +34,22 @@ type TimeBased struct { } // GenerateCustom generates a TOTP with custom options. -func (p TimeBased) GenerateCustom(username, algorithm string, digits, period, secretSize uint) (config *model.TOTPConfiguration, err error) { +func (p TimeBased) GenerateCustom(username, algorithm, secret string, digits, period, secretSize uint) (config *model.TOTPConfiguration, err error) { var key *otp.Key + var secretData []byte + + if secret != "" { + if secretData, err = base32.StdEncoding.WithPadding(base32.NoPadding).DecodeString(secret); err != nil { + return nil, fmt.Errorf("totp generate failed: error decoding base32 string: %w", err) + } + } + opts := totp.GenerateOpts{ Issuer: p.config.Issuer, AccountName: username, Period: period, + Secret: secretData, SecretSize: secretSize, Digits: otp.Digits(digits), Algorithm: otpStringToAlgo(algorithm), @@ -63,7 +74,7 @@ func (p TimeBased) GenerateCustom(username, algorithm string, digits, period, se // Generate generates a TOTP with default options. func (p TimeBased) Generate(username string) (config *model.TOTPConfiguration, err error) { - return p.GenerateCustom(username, p.config.Algorithm, p.config.Digits, p.config.Period, 32) + return p.GenerateCustom(username, p.config.Algorithm, "", p.config.Digits, p.config.Period, p.config.SecretSize) } // Validate the token against the given configuration. diff --git a/internal/totp/totp_test.go b/internal/totp/totp_test.go index 4b774f640..34b421eed 100644 --- a/internal/totp/totp_test.go +++ b/internal/totp/totp_test.go @@ -6,65 +6,127 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/authelia/authelia/v4/internal/configuration/schema" ) func TestTOTPGenerateCustom(t *testing.T) { + testCases := []struct { + desc string + username, algorithm, secret string + digits, period, secretSize uint + err string + }{ + { + desc: "ShouldGenerateSHA1", + username: "john", + algorithm: "SHA1", + digits: 6, + period: 30, + secretSize: 32, + }, + { + desc: "ShouldGenerateLongSecret", + username: "john", + algorithm: "SHA1", + digits: 6, + period: 30, + secretSize: 42, + }, + { + desc: "ShouldGenerateSHA256", + username: "john", + algorithm: "SHA256", + digits: 6, + period: 30, + secretSize: 32, + }, + { + desc: "ShouldGenerateSHA512", + username: "john", + algorithm: "SHA512", + digits: 6, + period: 30, + secretSize: 32, + }, + { + desc: "ShouldGenerateWithSecret", + username: "john", + algorithm: "SHA512", + secret: "ONTGOYLTMZQXGZDBONSGC43EMFZWMZ3BONTWMYLTMRQXGZBSGMYTEMZRMFYXGZDBONSA", + digits: 6, + period: 30, + secretSize: 32, + }, + { + desc: "ShouldGenerateWithBadSecretB32Data", + username: "john", + algorithm: "SHA512", + secret: "@#UNH$IK!J@N#EIKJ@U!NIJKUN@#WIK", + digits: 6, + period: 30, + secretSize: 32, + err: "totp generate failed: error decoding base32 string: illegal base32 data at input byte 0", + }, + { + desc: "ShouldGenerateWithBadSecretLength", + username: "john", + algorithm: "SHA512", + secret: "ONTGOYLTMZQXGZD", + digits: 6, + period: 30, + secretSize: 0, + }, + } + totp := NewTimeBasedProvider(schema.TOTPConfiguration{ - Issuer: "Authelia", - Algorithm: "SHA1", - Digits: 6, - Period: 30, + Issuer: "Authelia", + Algorithm: "SHA1", + Digits: 6, + Period: 30, + SecretSize: 32, }) - assert.Equal(t, uint(1), totp.skew) + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + c, err := totp.GenerateCustom(tc.username, tc.algorithm, tc.secret, tc.digits, tc.period, tc.secretSize) + if tc.err == "" { + assert.NoError(t, err) + require.NotNil(t, c) + assert.Equal(t, tc.period, c.Period) + assert.Equal(t, tc.digits, c.Digits) + assert.Equal(t, tc.algorithm, c.Algorithm) - config, err := totp.GenerateCustom("john", "SHA1", 6, 30, 32) - assert.NoError(t, err) + expectedSecretLen := int(tc.secretSize) + if tc.secret != "" { + expectedSecretLen = base32.StdEncoding.WithPadding(base32.NoPadding).DecodedLen(len(tc.secret)) + } - assert.Equal(t, uint(6), config.Digits) - assert.Equal(t, uint(30), config.Period) - assert.Equal(t, "SHA1", config.Algorithm) + secret := make([]byte, expectedSecretLen) - assert.Less(t, time.Since(config.CreatedAt), time.Second) - assert.Greater(t, time.Since(config.CreatedAt), time.Second*-1) - - secret := make([]byte, base32.StdEncoding.WithPadding(base32.NoPadding).DecodedLen(len(config.Secret))) - - _, err = base32.StdEncoding.WithPadding(base32.NoPadding).Decode(secret, config.Secret) - assert.NoError(t, err) - assert.Len(t, secret, 32) - - config, err = totp.GenerateCustom("john", "SHA1", 6, 30, 42) - assert.NoError(t, err) - - assert.Equal(t, uint(6), config.Digits) - assert.Equal(t, uint(30), config.Period) - assert.Equal(t, "SHA1", config.Algorithm) - - assert.Less(t, time.Since(config.CreatedAt), time.Second) - assert.Greater(t, time.Since(config.CreatedAt), time.Second*-1) - - secret = make([]byte, base32.StdEncoding.WithPadding(base32.NoPadding).DecodedLen(len(config.Secret))) - - _, err = base32.StdEncoding.WithPadding(base32.NoPadding).Decode(secret, config.Secret) - assert.NoError(t, err) - assert.Len(t, secret, 42) - - _, err = totp.GenerateCustom("", "SHA1", 6, 30, 32) - assert.EqualError(t, err, "AccountName must be set") + n, err := base32.StdEncoding.WithPadding(base32.NoPadding).Decode(secret, c.Secret) + assert.NoError(t, err) + assert.Len(t, secret, expectedSecretLen) + assert.Equal(t, expectedSecretLen, n) + } else { + assert.Nil(t, c) + assert.EqualError(t, err, tc.err) + } + }) + } } func TestTOTPGenerate(t *testing.T) { skew := uint(2) totp := NewTimeBasedProvider(schema.TOTPConfiguration{ - Issuer: "Authelia", - Algorithm: "SHA256", - Digits: 8, - Period: 60, - Skew: &skew, + Issuer: "Authelia", + Algorithm: "SHA256", + Digits: 8, + Period: 60, + Skew: &skew, + SecretSize: 32, }) assert.Equal(t, uint(2), totp.skew)