From a5f7e4cd608d15e457aec74a472d3c7f1751d10f Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 19 Apr 2021 22:41:37 +0100 Subject: [PATCH 1/4] Encrypt LDAP bind password in db with SECRET_KEY The LDAP source bind password are currently stored in plaintext in the db This PR simply encrypts them with the setting.SECRET_KEY. Fix #15460 Signed-off-by: Andrew Thornton --- models/login_source.go | 17 ++++++++++++++++- modules/auth/ldap/ldap.go | 1 + 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/models/login_source.go b/models/login_source.go index fd977e20a5d7..09405144c3be 100644 --- a/models/login_source.go +++ b/models/login_source.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/auth/oauth2" "code.gitea.io/gitea/modules/auth/pam" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/secret" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" @@ -76,11 +77,25 @@ type LDAPConfig struct { // FromDB fills up a LDAPConfig from serialized format. func (cfg *LDAPConfig) FromDB(bs []byte) error { json := jsoniter.ConfigCompatibleWithStandardLibrary - return json.Unmarshal(bs, &cfg) + err := json.Unmarshal(bs, &cfg) + if err != nil { + return err + } + if cfg.BindPasswordEncrypt != "" { + cfg.BindPassword, err = secret.DecryptSecret(setting.SecretKey, cfg.BindPasswordEncrypt) + cfg.BindPasswordEncrypt = "" + } + return err } // ToDB exports a LDAPConfig to a serialized format. func (cfg *LDAPConfig) ToDB() ([]byte, error) { + var err error + cfg.BindPasswordEncrypt, err = secret.EncryptSecret(setting.SecretKey, cfg.BindPassword) + if err != nil { + return nil, err + } + cfg.BindPassword = "" json := jsoniter.ConfigCompatibleWithStandardLibrary return json.Marshal(cfg) } diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 6c557de018c4..91ad33a60f3a 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -35,6 +35,7 @@ type Source struct { SecurityProtocol SecurityProtocol SkipVerify bool BindDN string // DN to bind with + BindPasswordEncrypt string // Encrypted Bind BN password BindPassword string // Bind DN password UserBase string // Base search path for users UserDN string // Template for the DN of the user for simple auth From bd1c33cfb571e209740bc84586d05f1e4dd9f6ed Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 18 May 2021 14:10:22 +0100 Subject: [PATCH 2/4] update documentation Signed-off-by: Andrew Thornton --- docs/content/doc/features/authentication.en-us.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/content/doc/features/authentication.en-us.md b/docs/content/doc/features/authentication.en-us.md index 0c83fa4d2f8d..004a6b7cab25 100644 --- a/docs/content/doc/features/authentication.en-us.md +++ b/docs/content/doc/features/authentication.en-us.md @@ -88,8 +88,8 @@ Adds the following fields: - Bind Password (optional) - The password for the Bind DN specified above, if any. _Note: The password - is stored in plaintext at the server. As such, ensure that the Bind DN - has as few privileges as possible._ + is stored in encrypted with the SECRET_KEY on the server. It is still recommended + to ensure that the Bind DN has as few privileges as possible._ - User Search Base **(required)** From a1484bacd80068bc3833c82bba6a5f272b53c01f Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 18 May 2021 17:34:26 +0100 Subject: [PATCH 3/4] update doc Signed-off-by: Andrew Thornton --- docs/content/doc/features/authentication.en-us.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/doc/features/authentication.en-us.md b/docs/content/doc/features/authentication.en-us.md index 004a6b7cab25..223d7aa4fb0c 100644 --- a/docs/content/doc/features/authentication.en-us.md +++ b/docs/content/doc/features/authentication.en-us.md @@ -88,7 +88,7 @@ Adds the following fields: - Bind Password (optional) - The password for the Bind DN specified above, if any. _Note: The password - is stored in encrypted with the SECRET_KEY on the server. It is still recommended + is stored encrypted with the SECRET_KEY on the server. It is still recommended to ensure that the Bind DN has as few privileges as possible._ - User Search Base **(required)** From 987f19290f06cc12680014c326175115df838fa2 Mon Sep 17 00:00:00 2001 From: silverwind Date: Wed, 19 May 2021 23:17:14 +0200 Subject: [PATCH 4/4] remove ui warning regarding unencrypted password --- options/locale/locale_en-US.ini | 1 - templates/admin/auth/edit.tmpl | 1 - templates/admin/auth/source/ldap.tmpl | 1 - 3 files changed, 3 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index ac1a0d972639..ab7367ba7ad3 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2283,7 +2283,6 @@ auths.host = Host auths.port = Port auths.bind_dn = Bind DN auths.bind_password = Bind Password -auths.bind_password_helper = Warning: This password is stored in plain text. Use a read-only account if possible. auths.user_base = User Search Base auths.user_dn = User DN auths.attribute_username = Username Attribute diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index e4d7a2e1e1fb..d825cd7d12de 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -53,7 +53,6 @@
-

{{.i18n.Tr "admin.auths.bind_password_helper"}}

{{end}}
diff --git a/templates/admin/auth/source/ldap.tmpl b/templates/admin/auth/source/ldap.tmpl index 584538f53bc9..1cbcb2fd415e 100644 --- a/templates/admin/auth/source/ldap.tmpl +++ b/templates/admin/auth/source/ldap.tmpl @@ -28,7 +28,6 @@
-

{{.i18n.Tr "admin.auths.bind_password_helper"}}