From cee19b0332d66851cd7d9a16ea9984d8275a747d Mon Sep 17 00:00:00 2001 From: Scott Bishel Date: Fri, 27 Sep 2019 12:13:31 -0600 Subject: [PATCH] MM-18013 Allow configuration of SAML crypto hashing algorithms (#12362) * MM-18013 Add SAML Algorithms to config. * set defaults to current values, add validation for settings * update to use simplier config entry --- i18n/en.json | 12 ++++++ model/config.go | 40 +++++++++++++++++ model/config_test.go | 100 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+) diff --git a/i18n/en.json b/i18n/en.json index 87fc85f360..3005763f68 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -4706,6 +4706,14 @@ "id": "model.config.is_valid.saml_assertion_consumer_service_url.app_error", "translation": "Service Provider Login URL must be a valid URL and start with http:// or https://." }, + { + "id": "model.config.is_valid.saml_canonical_algorithm.app_error", + "translation": "Invalid Canonical Algorithm." + }, + { + "id": "model.config.is_valid.saml_digest_algorithm.app_error", + "translation": "Invalid Digest Algorithm." + }, { "id": "model.config.is_valid.saml_email_attribute.app_error", "translation": "Invalid Email attribute. Must be set." @@ -4730,6 +4738,10 @@ "id": "model.config.is_valid.saml_public_cert.app_error", "translation": "Service Provider Public Certificate missing. Did you forget to upload it?" }, + { + "id": "model.config.is_valid.saml_signature_algorithm.app_error", + "translation": "Invalid Signature Algorithm." + }, { "id": "model.config.is_valid.saml_username_attribute.app_error", "translation": "Invalid Username attribute. Must be set." diff --git a/model/config.go b/model/config.go index 3e2a2ab9f5..acbd3d6f60 100644 --- a/model/config.go +++ b/model/config.go @@ -138,6 +138,20 @@ const ( SAML_SETTINGS_DEFAULT_LOCALE_ATTRIBUTE = "" SAML_SETTINGS_DEFAULT_POSITION_ATTRIBUTE = "" + SAML_SETTINGS_SIGNATURE_ALGORITHM_SHA1 = "RSAwithSHA1" + SAML_SETTINGS_SIGNATURE_ALGORITHM_SHA256 = "RSAwithSHA256" + SAML_SETTINGS_SIGNATURE_ALGORITHM_SHA384 = "RSAwithSHA384" + SAML_SETTINGS_SIGNATURE_ALGORITHM_SHA512 = "RSAwithSHA512" + SAML_SETTINGS_DEFAULT_SIGNATURE_ALGORITHM = SAML_SETTINGS_SIGNATURE_ALGORITHM_SHA1 + + SAML_SETTINGS_DIGEST_ALGORITHM_SHA1 = "SHA1" + SAML_SETTINGS_DIGEST_ALGORITHM_SHA256 = "SHA256" + SAML_SETTINGS_DEFAULT_DIGEST_ALGORITHM = SAML_SETTINGS_DIGEST_ALGORITHM_SHA1 + + SAML_SETTINGS_CANONICAL_ALGORITHM_C14N = "Canonical1.0" + SAML_SETTINGS_CANONICAL_ALGORITHM_C14N11 = "Canonical1.1" + SAML_SETTINGS_DEFAULT_CANONICAL_ALGORITHM = SAML_SETTINGS_CANONICAL_ALGORITHM_C14N + NATIVEAPP_SETTINGS_DEFAULT_APP_DOWNLOAD_LINK = "https://mattermost.com/download/#mattermostApps" NATIVEAPP_SETTINGS_DEFAULT_ANDROID_APP_DOWNLOAD_LINK = "https://about.mattermost.com/mattermost-android-app/" NATIVEAPP_SETTINGS_DEFAULT_IOS_APP_DOWNLOAD_LINK = "https://about.mattermost.com/mattermost-ios-app/" @@ -1885,6 +1899,10 @@ type SamlSettings struct { IdpDescriptorUrl *string AssertionConsumerServiceURL *string + SignatureAlgorithm *string + DigestAlgorithm *string + CanonicalAlgorithm *string + ScopingIDPProviderId *string ScopingIDPName *string @@ -1934,6 +1952,18 @@ func (s *SamlSettings) SetDefaults() { s.SignRequest = NewBool(false) } + if s.SignatureAlgorithm == nil { + s.SignatureAlgorithm = NewString(SAML_SETTINGS_DEFAULT_SIGNATURE_ALGORITHM) + } + + if s.DigestAlgorithm == nil { + s.DigestAlgorithm = NewString(SAML_SETTINGS_DEFAULT_DIGEST_ALGORITHM) + } + + if s.CanonicalAlgorithm == nil { + s.CanonicalAlgorithm = NewString(SAML_SETTINGS_DEFAULT_CANONICAL_ALGORITHM) + } + if s.IdpUrl == nil { s.IdpUrl = NewString("") } @@ -2800,6 +2830,16 @@ func (ss *SamlSettings) isValid() *AppError { if len(*ss.EmailAttribute) == 0 { return NewAppError("Config.IsValid", "model.config.is_valid.saml_email_attribute.app_error", nil, "", http.StatusBadRequest) } + + if !(*ss.SignatureAlgorithm == SAML_SETTINGS_SIGNATURE_ALGORITHM_SHA1 || *ss.SignatureAlgorithm == SAML_SETTINGS_SIGNATURE_ALGORITHM_SHA256 || *ss.SignatureAlgorithm == SAML_SETTINGS_SIGNATURE_ALGORITHM_SHA384 || *ss.SignatureAlgorithm == SAML_SETTINGS_SIGNATURE_ALGORITHM_SHA512) { + return NewAppError("Config.IsValid", "model.config.is_valid.saml_signature_algorithm.app_error", nil, "", http.StatusBadRequest) + } + if !(*ss.DigestAlgorithm == SAML_SETTINGS_DIGEST_ALGORITHM_SHA1 || *ss.DigestAlgorithm == SAML_SETTINGS_DIGEST_ALGORITHM_SHA256) { + return NewAppError("Config.IsValid", "model.config.is_valid.saml_digest_algorithm.app_error", nil, "", http.StatusBadRequest) + } + if !(*ss.CanonicalAlgorithm == SAML_SETTINGS_CANONICAL_ALGORITHM_C14N || *ss.CanonicalAlgorithm == SAML_SETTINGS_CANONICAL_ALGORITHM_C14N11) { + return NewAppError("Config.IsValid", "model.config.is_valid.saml_canonical_algorithm.app_error", nil, "", http.StatusBadRequest) + } } return nil diff --git a/model/config_test.go b/model/config_test.go index ec6e78de29..a3ed8bc7ff 100644 --- a/model/config_test.go +++ b/model/config_test.go @@ -95,6 +95,106 @@ func TestConfigDefaultFileSettingsS3SSE(t *testing.T) { } } +func TestConfigDefaultSignatureAlgorithm(t *testing.T) { + c1 := Config{} + c1.SetDefaults() + + if *c1.SamlSettings.SignatureAlgorithm != SAML_SETTINGS_DEFAULT_SIGNATURE_ALGORITHM { + t.Fatal("SamlSettings.SignatureAlgorithm default not set") + } + + if *c1.SamlSettings.DigestAlgorithm != SAML_SETTINGS_DEFAULT_DIGEST_ALGORITHM { + t.Fatal("SamlSettings.DigestAlgorithm default not set") + } + if *c1.SamlSettings.CanonicalAlgorithm != SAML_SETTINGS_DEFAULT_CANONICAL_ALGORITHM { + t.Fatal("SamlSettings.CanonicalAlgorithm default not set") + } +} + +func TestConfigOverwriteSignatureAlgorithm(t *testing.T) { + const testAlgorithm = "FakeAlgorithm" + c1 := Config{ + SamlSettings: SamlSettings{ + CanonicalAlgorithm: NewString(testAlgorithm), + SignatureAlgorithm: NewString(testAlgorithm), + DigestAlgorithm: NewString(testAlgorithm), + }, + } + + c1.SetDefaults() + + if *c1.SamlSettings.SignatureAlgorithm != testAlgorithm { + t.Fatal("SamlSettings.SignatureAlgorithm should be overwritten") + } + if *c1.SamlSettings.DigestAlgorithm != testAlgorithm { + t.Fatal("SamlSettings.DigestAlgorithm should be overwritten") + } + if *c1.SamlSettings.CanonicalAlgorithm != testAlgorithm { + t.Fatal("SamlSettings.CanonicalAlgorithm should be overwritten") + } +} + +func TestConfigIsValidDefaultAlgorithms(t *testing.T) { + c1 := Config{} + c1.SetDefaults() + + *c1.SamlSettings.Enable = true + *c1.SamlSettings.Verify = false + *c1.SamlSettings.Encrypt = false + + *c1.SamlSettings.IdpUrl = "http://test.url.com" + *c1.SamlSettings.IdpDescriptorUrl = "http://test.url.com" + *c1.SamlSettings.IdpCertificateFile = "certificatefile" + *c1.SamlSettings.EmailAttribute = "Email" + *c1.SamlSettings.UsernameAttribute = "Username" + + err := c1.SamlSettings.isValid() + if err != nil { + t.Fatal("SAMLSettings validation should pass with default settings") + } +} + +func TestConfigIsValidFakeAlgorithm(t *testing.T) { + c1 := Config{} + c1.SetDefaults() + + *c1.SamlSettings.Enable = true + *c1.SamlSettings.Verify = false + *c1.SamlSettings.Encrypt = false + + *c1.SamlSettings.IdpUrl = "http://test.url.com" + *c1.SamlSettings.IdpDescriptorUrl = "http://test.url.com" + *c1.SamlSettings.IdpCertificateFile = "certificatefile" + *c1.SamlSettings.EmailAttribute = "Email" + *c1.SamlSettings.UsernameAttribute = "Username" + + temp := *c1.SamlSettings.CanonicalAlgorithm + *c1.SamlSettings.CanonicalAlgorithm = "Fake Algorithm" + err := c1.SamlSettings.isValid() + if err == nil { + t.Fatal("SAMLSettings validation should fail with fake Canonical Algorithm") + } + require.Equal(t, "model.config.is_valid.saml_canonical_algorithm.app_error", err.Message) + *c1.SamlSettings.CanonicalAlgorithm = temp + + temp = *c1.SamlSettings.DigestAlgorithm + *c1.SamlSettings.DigestAlgorithm = "Fake Algorithm" + err = c1.SamlSettings.isValid() + if err == nil { + t.Fatal("SAMLSettings validation should pass fake digest Algorithm") + } + require.Equal(t, "model.config.is_valid.saml_digest_algorithm.app_error", err.Message) + *c1.SamlSettings.DigestAlgorithm = temp + + temp = *c1.SamlSettings.SignatureAlgorithm + *c1.SamlSettings.SignatureAlgorithm = "Fake Algorithm" + err = c1.SamlSettings.isValid() + if err == nil { + t.Fatal("SAMLSettings validation should pass with fake signature settings") + } + require.Equal(t, "model.config.is_valid.saml_signature_algorithm.app_error", err.Message) +} + func TestConfigDefaultServiceSettingsExperimentalGroupUnreadChannels(t *testing.T) { c1 := Config{} c1.SetDefaults()