From c2a15d2119dc3189368ee0536b9a09623045665d Mon Sep 17 00:00:00 2001 From: Devon Mizelle Date: Fri, 13 Aug 2021 19:13:05 -0400 Subject: [PATCH] Add Password Complexity Configuration A potential solution to #1963. This commit does the following: 1. Moves the values for minimum and maximum password length into the ClientAPI configuration struct. 2. Introduces a new struct representing the password complexity requirements defined in dendrite-config.yml, with four options. Defaults are compatible with what users probably expect out of synapse. * Minimum length, default of 8 * Maximum length, default of 512 * Minimum number of symbols, default of 0 * Requiring mixed case toggle, default of false 3. Adds tests for the logic of validating passwords. Signed-off-by: Devon Mizelle --- build/docker/config/dendrite-config.yaml | 9 ++ clientapi/routing/password.go | 2 +- clientapi/routing/register.go | 43 ++++++--- clientapi/routing/register_test.go | 109 +++++++++++++++++++++++ clientapi/routing/routing.go | 2 +- dendrite-config.yaml | 9 ++ setup/config/config_clientapi.go | 29 ++++++ setup/config/config_test.go | 5 ++ 8 files changed, 196 insertions(+), 12 deletions(-) diff --git a/build/docker/config/dendrite-config.yaml b/build/docker/config/dendrite-config.yaml index ffcf6a451..6430532b0 100644 --- a/build/docker/config/dendrite-config.yaml +++ b/build/docker/config/dendrite-config.yaml @@ -157,6 +157,15 @@ client_api: threshold: 5 cooloff_ms: 500 + # Settings for requiring complexity out of passwords. + password_requirements: + min_password_length: 8 # synapse default + max_password_length: 512 # synapse default + # minimum number of symbols (non a-z, A-Z to have) + min_number_symbols: 0 + # should passwords have uppercase and lowercase characters? + require_mixed_case: false + # Configuration for the EDU server. edu_server: internal_api: diff --git a/clientapi/routing/password.go b/clientapi/routing/password.go index 87d5f8ff3..af7c15d10 100644 --- a/clientapi/routing/password.go +++ b/clientapi/routing/password.go @@ -78,7 +78,7 @@ func Password( AddCompletedSessionStage(sessionID, authtypes.LoginTypePassword) // Check the new password strength. - if resErr = validatePassword(r.NewPassword); resErr != nil { + if resErr = validatePassword(r.NewPassword, cfg.PasswordRequirements); resErr != nil { return *resErr } diff --git a/clientapi/routing/register.go b/clientapi/routing/register.go index 8823a41e3..975d50c96 100644 --- a/clientapi/routing/register.go +++ b/clientapi/routing/register.go @@ -57,8 +57,6 @@ var ( ) const ( - minPasswordLength = 8 // http://matrix.org/docs/spec/client_server/r0.2.0.html#password-based - maxPasswordLength = 512 // https://github.com/matrix-org/synapse/blob/v0.20.0/synapse/rest/client/v2_alpha/register.py#L161 maxUsernameLength = 254 // http://matrix.org/speculator/spec/HEAD/intro.html#user-identifiers TODO account for domain sessionIDLength = 24 ) @@ -111,6 +109,10 @@ var ( // sessions stores the completed flow stages for all sessions. Referenced using their sessionID. sessions = newSessionsDict() validUsernameRegex = regexp.MustCompile(`^[0-9a-z_\-=./]+$`) + + passwordSymbols = regexp.MustCompile(`[^0-9a-zA-Z]`) + passwordUppercase = regexp.MustCompile(`[A-Z]`) + passwordLowercase = regexp.MustCompile(`[a-z]`) ) // registerRequest represents the submitted registration request. @@ -225,17 +227,38 @@ func validateApplicationServiceUsername(username string) *util.JSONResponse { } // validatePassword returns an error response if the password is invalid -func validatePassword(password string) *util.JSONResponse { +func validatePassword(password string, cfg config.PasswordRequirements) *util.JSONResponse { // https://github.com/matrix-org/synapse/blob/v0.20.0/synapse/rest/client/v2_alpha/register.py#L161 - if len(password) > maxPasswordLength { + if len(password) > cfg.MaxPasswordLength { return &util.JSONResponse{ Code: http.StatusBadRequest, - JSON: jsonerror.BadJSON(fmt.Sprintf("'password' >%d characters", maxPasswordLength)), + JSON: jsonerror.WeakPassword(fmt.Sprintf("'password' >%d characters", cfg.MaxPasswordLength)), } - } else if len(password) > 0 && len(password) < minPasswordLength { + } else if len(password) > 0 && len(password) < cfg.MinPasswordLength { return &util.JSONResponse{ Code: http.StatusBadRequest, - JSON: jsonerror.WeakPassword(fmt.Sprintf("password too weak: min %d chars", minPasswordLength)), + JSON: jsonerror.WeakPassword(fmt.Sprintf("password too weak: min %d chars", cfg.MinPasswordLength)), + } + } + + if cfg.MinNumberSymbols > 0 { + matches := passwordSymbols.FindAllStringIndex(password, -1) + if len(matches) < cfg.MinNumberSymbols { + return &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.WeakPassword(fmt.Sprintf("password too weak: minimum %d symbols", cfg.MinNumberSymbols)), + } + } + } + + if cfg.RequireMixedCase { + lowercase := passwordLowercase.FindAllStringIndex(password, -1) + uppercase := passwordUppercase.FindAllStringIndex(password, -1) + if len(lowercase) == 0 || len(uppercase) == 0 { + return &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.WeakPassword("password must have uppercase and lowercase letters"), + } } } return nil @@ -511,7 +534,7 @@ func Register( return *resErr } } - if resErr = validatePassword(r.Password); resErr != nil { + if resErr = validatePassword(r.Password, cfg.PasswordRequirements); resErr != nil { return *resErr } @@ -935,7 +958,7 @@ func RegisterAvailable( } } -func handleSharedSecretRegistration(userAPI userapi.UserInternalAPI, sr *SharedSecretRegistration, req *http.Request) util.JSONResponse { +func handleSharedSecretRegistration(userAPI userapi.UserInternalAPI, sr *SharedSecretRegistration, passwordRequirements config.PasswordRequirements, req *http.Request) util.JSONResponse { ssrr, err := NewSharedSecretRegistrationRequest(req.Body) if err != nil { return util.JSONResponse{ @@ -959,7 +982,7 @@ func handleSharedSecretRegistration(userAPI userapi.UserInternalAPI, sr *SharedS if resErr := validateUsername(ssrr.User); resErr != nil { return *resErr } - if resErr := validatePassword(ssrr.Password); resErr != nil { + if resErr := validatePassword(ssrr.Password, passwordRequirements); resErr != nil { return *resErr } deviceID := "shared_secret_registration" diff --git a/clientapi/routing/register_test.go b/clientapi/routing/register_test.go index ea07f30be..555a5d93e 100644 --- a/clientapi/routing/register_test.go +++ b/clientapi/routing/register_test.go @@ -15,11 +15,15 @@ package routing import ( + "fmt" + "net/http" "regexp" "testing" "github.com/matrix-org/dendrite/clientapi/auth/authtypes" + "github.com/matrix-org/dendrite/clientapi/jsonerror" "github.com/matrix-org/dendrite/setup/config" + "github.com/matrix-org/util" ) var ( @@ -208,3 +212,108 @@ func TestValidationOfApplicationServices(t *testing.T) { t.Errorf("user_id should not have been valid: @_something_else:localhost") } } + +func TestValidatePassword(t *testing.T) { + t.Parallel() + defaults := &config.PasswordRequirements{} + defaults.Defaults() + + custom := &config.PasswordRequirements{ + MinPasswordLength: 16, + MaxPasswordLength: 32, + RequireMixedCase: true, + MinNumberSymbols: 5, + } + var testCases = []struct { + name string + config config.PasswordRequirements + password string + expected *util.JSONResponse + }{ + { + "default reject too short", + *defaults, + "foobar", + &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.WeakPassword(fmt.Sprintf("password too weak: min %d chars", defaults.MinPasswordLength)), + }}, {"default reject too long", + *defaults, + // len 600 + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.WeakPassword(fmt.Sprintf("'password' >%d characters", defaults.MaxPasswordLength)), + }, + }, + { + "set min too short", + *custom, + "abcd", + &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.WeakPassword(fmt.Sprintf("password too weak: min %d chars", custom.MinPasswordLength)), + }, + }, + { + "set max too long", + *custom, + // len 33 + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", + &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.WeakPassword(fmt.Sprintf("'password' >%d characters", custom.MaxPasswordLength)), + }, + }, + { + "set symbols not enough", + *custom, + "thi$i$apasswordshouldbelong", + &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.WeakPassword(fmt.Sprintf("password too weak: minimum %d symbols", custom.MinNumberSymbols)), + }, + }, + { + "require mixed case but none", + *custom, + "haha_all_lowercase_cant_catch_me", + &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.WeakPassword("password must have uppercase and lowercase letters"), + }, + }, + { + "custom settings but valid", + *custom, + "$0me_$up3r_$trong_P@ass", + nil, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + response := validatePassword(test.password, test.config) + if test.expected == nil && response != nil { + t.Errorf("expected password to be validated. got error: %s", response.JSON.(*jsonerror.MatrixError).Err) + } else if test.expected != nil && response == nil { + t.Errorf("expected password to fail, but was validated") + } else if test.expected != nil && test.expected.Code != response.Code { + t.Errorf("expected error code %d, got %d", test.expected.Code, response.Code) + } else if test.expected != nil && response != nil { + matrixError := response.JSON.(*jsonerror.MatrixError) + expectedError := test.expected.JSON.(*jsonerror.MatrixError) + + if expectedError.Err != matrixError.Err { + t.Errorf("expected error: %s, got error: %s", expectedError.Err, matrixError.Err) + } + } else if test.expected == nil && response == nil { + t.Log("password validation passed") + } else { + t.Errorf("uncaught test result. expected %v, got %v", test.expected, response) + } + }) + } +} diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index c6be8939d..96e658457 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -107,7 +107,7 @@ func Setup( } } if req.Method == http.MethodPost { - return handleSharedSecretRegistration(userAPI, sr, req) + return handleSharedSecretRegistration(userAPI, sr, cfg.PasswordRequirements, req) } return util.JSONResponse{ Code: http.StatusMethodNotAllowed, diff --git a/dendrite-config.yaml b/dendrite-config.yaml index 31b830663..f66ab1b7f 100644 --- a/dendrite-config.yaml +++ b/dendrite-config.yaml @@ -174,6 +174,15 @@ client_api: threshold: 5 cooloff_ms: 500 + # Settings for requiring complexity out of passwords. + password_requirements: + min_password_length: 8 # synapse default + max_password_length: 512 # synapse default + # minimum number of symbols (non a-z, A-Z to have) + min_number_symbols: 0 + # should passwords have uppercase and lowercase characters? + require_mixed_case: false + # Configuration for the EDU server. edu_server: internal_api: diff --git a/setup/config/config_clientapi.go b/setup/config/config_clientapi.go index c7cb9c33e..d31b8afc2 100644 --- a/setup/config/config_clientapi.go +++ b/setup/config/config_clientapi.go @@ -32,6 +32,9 @@ type ClientAPI struct { // was successful RecaptchaSiteVerifyAPI string `yaml:"recaptcha_siteverify_api"` + // Used to enforce standards on password strengths + PasswordRequirements PasswordRequirements `yaml:"password_requirements"` + // TURN options TURN TURN `yaml:"turn"` @@ -53,6 +56,7 @@ func (c *ClientAPI) Defaults() { c.RecaptchaSiteVerifyAPI = "" c.RegistrationDisabled = false c.RateLimiting.Defaults() + c.PasswordRequirements.Defaults() } func (c *ClientAPI) Verify(configErrs *ConfigErrors, isMonolith bool) { @@ -68,6 +72,7 @@ func (c *ClientAPI) Verify(configErrs *ConfigErrors, isMonolith bool) { } c.TURN.Verify(configErrs) c.RateLimiting.Verify(configErrs) + c.PasswordRequirements.Verify(configErrs) } type TURN struct { @@ -123,3 +128,27 @@ func (r *RateLimiting) Defaults() { r.Threshold = 5 r.CooloffMS = 500 } + +type PasswordRequirements struct { + // Minimum number of characters + MinPasswordLength int `yaml:"min_password_length"` + // Maximum number of characters + MaxPasswordLength int `yaml:"max_password_length"` + // Number of symbols required + MinNumberSymbols int `yaml:"minimum_number_of_symbols"` + // Should the password have uppercase and lowercase characters + RequireMixedCase bool `yaml:"require_mixed_case"` +} + +func (p *PasswordRequirements) Verify(configErrs *ConfigErrors) { + checkPositive(configErrs, "client_api.password_requirements.min_password_length", int64(p.MinPasswordLength)) + checkPositive(configErrs, "client_api.password_requirements.max_password_length", int64(p.MaxPasswordLength)) + checkPositive(configErrs, "client_api.password_requirements.min_number_symbols", int64(p.MinNumberSymbols)) +} + +func (p *PasswordRequirements) Defaults() { + p.MinPasswordLength = 8 // http://matrix.org/docs/spec/client_server/r0.2.0.html#password-based + p.MaxPasswordLength = 512 // https://github.com/matrix-org/synapse/blob/v0.20.0/synapse/rest/client/v2_alpha/register.py#L161 + p.MinNumberSymbols = 0 + p.RequireMixedCase = false +} diff --git a/setup/config/config_test.go b/setup/config/config_test.go index 4107b6845..803cb5a98 100644 --- a/setup/config/config_test.go +++ b/setup/config/config_test.go @@ -86,6 +86,11 @@ client_api: turn_shared_secret: "" turn_username: "" turn_password: "" + password_requirements: + min_password_length: 6 + max_password_length: 64 + required_number_symbols: 2 + require_mixed_case: true current_state_server: internal_api: listen: http://localhost:7782