From 713f8e9ab77babcdc1eed05dd7c91626b771ff24 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Mon, 8 May 2023 13:30:49 +1000 Subject: [PATCH] fix(configuration): fail to parse large int duration (#5408) Large integers used with the duration common syntax failed to parse if they exceeded the ability to fit into an int32. Signed-off-by: James Elliott --- internal/configuration/const.go | 6 + internal/configuration/decode_hooks.go | 44 +++-- internal/configuration/decode_hooks_test.go | 31 ++++ internal/configuration/provider_test.go | 13 ++ .../test_resources/config.durations.yml | 171 ++++++++++++++++++ 5 files changed, 254 insertions(+), 11 deletions(-) create mode 100644 internal/configuration/test_resources/config.durations.yml diff --git a/internal/configuration/const.go b/internal/configuration/const.go index cc9b5a6fb..7ec14716c 100644 --- a/internal/configuration/const.go +++ b/internal/configuration/const.go @@ -2,6 +2,8 @@ package configuration import ( "errors" + "math" + "time" ) // DefaultEnvPrefix is the default environment prefix. @@ -38,6 +40,10 @@ const ( errFmtDecodeHookCouldNotParseEmptyValue = "could not decode an empty value to a %s%s: %w" ) +const ( + durationMax = time.Duration(math.MaxInt64) +) + // IMPORTANT: There is an uppercase copy of this in github.com/authelia/authelia/internal/templates named // envSecretSuffixes. // Make sure you update these at the same time. diff --git a/internal/configuration/decode_hooks.go b/internal/configuration/decode_hooks.go index 1eabcfa7d..c9d4be4f5 100644 --- a/internal/configuration/decode_hooks.go +++ b/internal/configuration/decode_hooks.go @@ -9,6 +9,7 @@ import ( "net/url" "reflect" "regexp" + "strconv" "strings" "time" @@ -112,19 +113,14 @@ func StringToURLHookFunc() mapstructure.DecodeHookFuncType { } // ToTimeDurationHookFunc converts string and integer types to a time.Duration. +// +//nolint:gocyclo // Function is necessarily complex though flows well due to switch statement usage. func ToTimeDurationHookFunc() mapstructure.DecodeHookFuncType { return func(f reflect.Type, t reflect.Type, data any) (value any, err error) { - var ptr bool - - switch f.Kind() { - case reflect.String, reflect.Int, reflect.Int32, reflect.Int64: - // We only allow string and integer from kinds to match. - break - default: - return data, nil - } - - prefixType := "" + var ( + ptr bool + prefixType string + ) if t.Kind() == reflect.Ptr { ptr = true @@ -139,6 +135,14 @@ func ToTimeDurationHookFunc() mapstructure.DecodeHookFuncType { return data, nil } + switch f.Kind() { + case reflect.String, reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Float64: + // We only allow string and integer from kinds to match. + break + default: + return data, nil + } + var result time.Duration switch { @@ -151,11 +155,29 @@ func ToTimeDurationHookFunc() mapstructure.DecodeHookFuncType { case f.Kind() == reflect.Int: seconds := data.(int) + result = time.Second * time.Duration(seconds) + case f.Kind() == reflect.Int8: + seconds := data.(int8) + + result = time.Second * time.Duration(seconds) + case f.Kind() == reflect.Int16: + seconds := data.(int16) + result = time.Second * time.Duration(seconds) case f.Kind() == reflect.Int32: seconds := data.(int32) result = time.Second * time.Duration(seconds) + case f.Kind() == reflect.Float64: + fseconds := data.(float64) + + if fseconds > durationMax.Seconds() { + result = durationMax + } else { + seconds, _ := strconv.Atoi(fmt.Sprintf("%.0f", fseconds)) + + result = time.Second * time.Duration(seconds) + } case f == expectedType: result = data.(time.Duration) case f.Kind() == reflect.Int64: diff --git a/internal/configuration/decode_hooks_test.go b/internal/configuration/decode_hooks_test.go index f9642c170..6a3b9d0df 100644 --- a/internal/configuration/decode_hooks_test.go +++ b/internal/configuration/decode_hooks_test.go @@ -6,6 +6,7 @@ import ( "crypto/rsa" "crypto/x509" "encoding/pem" + "math" "net/mail" "net/url" "reflect" @@ -345,12 +346,36 @@ func TestToTimeDurationHookFunc(t *testing.T) { want: time.Second * 60, decode: true, }, + { + desc: "ShouldDecodeInt8ToSeconds", + have: int8(90), + want: time.Second * 90, + decode: true, + }, + { + desc: "ShouldDecodeInt16ToSeconds", + have: int16(90), + want: time.Second * 90, + decode: true, + }, { desc: "ShouldDecodeInt32ToSeconds", have: int32(90), want: time.Second * 90, decode: true, }, + { + desc: "ShouldDecodeFloat64ToSeconds", + have: float64(90), + want: time.Second * 90, + decode: true, + }, + { + desc: "ShouldDecodeFloat64ToSeconds", + have: math.MaxFloat64, + want: time.Duration(math.MaxInt64), + decode: true, + }, { desc: "ShouldDecodeInt64ToSeconds", have: int64(120), @@ -375,6 +400,12 @@ func TestToTimeDurationHookFunc(t *testing.T) { want: time.Duration(0), decode: true, }, + { + desc: "ShouldSkipParsingBoolean", + have: true, + want: time.Duration(0), + decode: false, + }, { desc: "ShouldNotDecodeFromBool", have: true, diff --git a/internal/configuration/provider_test.go b/internal/configuration/provider_test.go index 98915016b..27f363d4f 100644 --- a/internal/configuration/provider_test.go +++ b/internal/configuration/provider_test.go @@ -7,6 +7,7 @@ import ( "runtime" "sort" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -86,6 +87,18 @@ func TestShouldHaveNotifier(t *testing.T) { assert.NotNil(t, config.Notifier) } +func TestShouldParseLargeIntegerDurations(t *testing.T) { + val := schema.NewStructValidator() + _, config, err := Load(val, NewDefaultSources([]string{"./test_resources/config.durations.yml"}, DefaultEnvPrefix, DefaultEnvDelimiter)...) + + assert.NoError(t, err) + assert.Len(t, val.Errors(), 0) + assert.Len(t, val.Warnings(), 0) + + assert.Equal(t, durationMax, config.Regulation.FindTime) + assert.Equal(t, time.Second*1000, config.Regulation.BanTime) +} + func TestShouldValidateConfigurationWithEnv(t *testing.T) { testSetEnv(t, "SESSION_SECRET", "abc") testSetEnv(t, "STORAGE_MYSQL_PASSWORD", "abc") diff --git a/internal/configuration/test_resources/config.durations.yml b/internal/configuration/test_resources/config.durations.yml new file mode 100644 index 000000000..f20f5d3be --- /dev/null +++ b/internal/configuration/test_resources/config.durations.yml @@ -0,0 +1,171 @@ +--- +default_redirection_url: 'https://home.example.com:8080/' + +server: + address: 'tcp://127.0.0.1:9091' + endpoints: + authz: + forward-auth: + implementation: 'ForwardAuth' + authn_strategies: + - name: 'HeaderProxyAuthorization' + - name: 'CookieSession' + ext-authz: + implementation: 'ExtAuthz' + authn_strategies: + - name: 'HeaderProxyAuthorization' + - name: 'CookieSession' + auth-request: + implementation: 'AuthRequest' + authn_strategies: + - name: 'HeaderAuthRequestProxyAuthorization' + - name: 'CookieSession' + legacy: + implementation: 'Legacy' + +log: + level: 'debug' + +totp: + issuer: 'authelia.com' + +duo_api: + hostname: 'api-123456789.example.com' + integration_key: 'ABCDEF' + +authentication_backend: + ldap: + address: 'ldap://127.0.0.1' + tls: + private_key: | + -----BEGIN RSA PRIVATE KEY----- + MIIEpAIBAAKCAQEA6z1LOg1ZCqb0lytXWZ+MRBpMHEXOoTOLYgfZXt1IYyE3Z758 + cyalk0NYQhY5cZDsXPYWPvAHiPMUxutWkoxFwby56S+AbIMa3/Is+ILrHRJs8Exn + ZkpyrYFxPX12app2kErdmAkHSx0Z5/kuXiz96PHs8S8/ZbyZolLHzdfLtSzjvRm5 + Zue5iFzsf19NJz5CIBfv8g5lRwtE8wNJoRSpn1xq7fqfuA0weDNFPzjlNWRLy6aa + rK7qJexRkmkCs4sLgyl+9NODYJpvmN8E1yhyC27E0joI6rBFVW7Ihv+cSPCdDzGp + EWe81x3AeqAa3mjVqkiq4u4Z2i8JDgBaPboqJwIDAQABAoIBAAFdLZ58jVOefDSU + L8F5R1rtvBs93GDa56f926jNJ6pLewLC+/2+757W+SAI+PRLntM7Kg3bXm/Q2QH+ + Q1Y+MflZmspbWCdI61L5GIGoYKyeers59i+FpvySj5GHtLQRiTZ0+Kv1AXHSDWBm + 9XneUOqU3IbZe0ifu1RRno72/VtjkGXbW8Mkkw+ohyGbIeTx/0/JQ6sSNZTT3Vk7 + 8i4IXptq3HSF0/vqZuah8rShoeNq72pD1YLM9YPdL5by1QkDLnqATDiCpLBTCaNV + I8sqYEun+HYbQzBj8ZACG2JVZpEEidONWQHw5BPWO95DSZYrVnEkuCqeH+u5vYt7 + CHuJ3AECgYEA+W3v5z+j91w1VPHS0VB3SCDMouycAMIUnJPAbt+0LPP0scUFsBGE + hPAKddC54pmMZRQ2KIwBKiyWfCrJ8Xz8Yogn7fJgmwTHidJBr2WQpIEkNGlK3Dzi + jXL2sh0yC7sHvn0DqiQ79l/e7yRbSnv2wrTJEczOOH2haD7/tBRyCYECgYEA8W+q + E9YyGvEltnPFaOxofNZ8LHVcZSsQI5b6fc0iE7fjxFqeXPXEwGSOTwqQLQRiHn9b + CfPmIG4Vhyq0otVmlPvUnfBZ2OK+tl5X2/mQFO3ROMdvpi0KYa994uqfJdSTaqLn + jjoKFB906UFHnDQDLZUNiV1WwnkTglgLc+xrd6cCgYEAqqthyv6NyBTM3Tm2gcio + Ra9Dtntl51LlXZnvwy3IkDXBCd6BHM9vuLKyxZiziGx+Vy90O1xI872cnot8sINQ + Am+dur/tAEVN72zxyv0Y8qb2yfH96iKy9gxi5s75TnOEQgAygLnYWaWR2lorKRUX + bHTdXBOiS58S0UzCFEslGIECgYBqkO4SKWYeTDhoKvuEj2yjRYyzlu28XeCWxOo1 + otiauX0YSyNBRt2cSgYiTzhKFng0m+QUJYp63/wymB/5C5Zmxi0XtWIDADpLhqLj + HmmBQ2Mo26alQ5YkffBju0mZyhVzaQop1eZi8WuKFV1FThPlB7hc3E0SM5zv2Grd + tQnOWwKBgQC40yZY0PcjuILhy+sIc0Wvh7LUA7taSdTye149kRvbvsCDN7Jh75lM + USjhLXY0Nld2zBm9r8wMb81mXH29uvD+tDqqsICvyuKlA/tyzXR+QTr7dCVKVwu0 + 1YjCJ36UpTsLre2f8nOSLtNmRfDPtbOE2mkOoO9dD9UU0XZwnvn9xw== + -----END RSA PRIVATE KEY----- + base_dn: 'dc=example,dc=com' + username_attribute: 'uid' + additional_users_dn: 'ou=users' + users_filter: '(&({username_attribute}={input})(objectCategory=person)(objectClass=user))' + additional_groups_dn: 'ou=groups' + groups_filter: '(&(member={dn})(objectClass=groupOfNames))' + group_name_attribute: 'cn' + mail_attribute: 'mail' + user: 'cn=admin,dc=example,dc=com' + +access_control: + default_policy: 'deny' + + rules: + # Rules applied to everyone + - domain: 'public.example.com' + policy: 'bypass' + + - domain: 'secure.example.com' + policy: 'one_factor' + # Network based rule, if not provided any network matches. + networks: + - '192.168.1.0/24' + - domain: 'secure.example.com' + policy: 'two_factor' + + - domain: ['singlefactor.example.com', 'onefactor.example.com'] + policy: 'one_factor' + + # Rules applied to 'admins' group + - domain: 'mx2.mail.example.com' + subject: 'group:admins' + policy: 'deny' + - domain: '*.example.com' + subject: 'group:admins' + policy: 'two_factor' + + # Rules applied to 'dev' group + - domain: 'dev.example.com' + resources: + - '^/groups/dev/.*$' + subject: 'group:dev' + policy: 'two_factor' + + # Rules applied to user 'john' + - domain: 'dev.example.com' + resources: + - '^/users/john/.*$' + subject: 'user:john' + policy: 'two_factor' + + # Rules applied to 'dev' group and user 'john' + - domain: 'dev.example.com' + resources: + - '^/deny-all.*$' + subject: ['group:dev', 'user:john'] + policy: 'deny' + + # Rules applied to user 'harry' + - domain: 'dev.example.com' + resources: + - '^/users/harry/.*$' + subject: 'user:harry' + policy: 'two_factor' + + # Rules applied to user 'bob' + - domain: '*.mail.example.com' + subject: 'user:bob' + policy: 'two_factor' + - domain: 'dev.example.com' + resources: + - '^/users/bob/.*$' + subject: 'user:bob' + policy: 'two_factor' + +session: + name: 'authelia_session' + expiration: '1h' # 1 hour + inactivity: '5m' # 5 minutes + domain: 'example.com' + redis: + host: '127.0.0.1' + port: 6379 + high_availability: + sentinel_name: 'test' + +regulation: + max_retries: 3 + find_time: 123456789123456789123 + ban_time: 1.0e3 + +storage: + mysql: + address: 'tcp://127.0.0.1:3306' + database: 'authelia' + username: 'authelia' + +notifier: + smtp: + address: 'smtp://127.0.0.1:1025' + username: 'test' + sender: 'admin@example.com' + disable_require_tls: true +...