From 7f59f438f8cd6890f4f4bae030487524571a438f Mon Sep 17 00:00:00 2001 From: Sasha Ivanov Date: Sat, 3 Dec 2016 11:06:06 +0200 Subject: [PATCH 1/3] Correction LDAP username validation As https://msdn.microsoft.com/en-us/library/aa366101(v=vs.85).aspx describe spaces should not be in start or at the end of username but they can be inside the username. So please check my solution for it. --- modules/auth/ldap/ldap.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 0cabcb20a084c..7e0d3928717a6 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -60,8 +60,8 @@ func (ls *Source) sanitizedUserQuery(username string) (string, bool) { func (ls *Source) sanitizedUserDN(username string) (string, bool) { // See http://tools.ietf.org/search/rfc4514: "special characters" - badCharacters := "\x00()*\\,='\"#+;<> " - if strings.ContainsAny(username, badCharacters) { + badCharacters := "\x00()*\\,='\"#+;<>" + if strings.ContainsAny(username, badCharacters) || strings.HasPrefix(username, " ") || strings.HasSuffix(username, " ") { log.Debug("'%s' contains invalid DN characters. Aborting.", username) return "", false } From e1216267a56317ed540b20467e609def3a34ad60 Mon Sep 17 00:00:00 2001 From: Mateusz Hromada Date: Sat, 3 Dec 2016 11:13:51 +0200 Subject: [PATCH 2/3] Check for zero length passwords in LDAP module. According to https://tools.ietf.org/search/rfc4513#section-5.1.2 LDAP client should always check before bind whether a password is an empty value. There are at least one LDAP implementation which does not return error if you try to bind with DN set and empty password - AD. --- modules/auth/ldap/ldap.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 7e0d3928717a6..90714392593a7 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -151,6 +151,11 @@ func bindUser(l *ldap.Conn, userDN, passwd string) error { // SearchEntry : search an LDAP source if an entry (name, passwd) is valid and in the specific filter func (ls *Source) SearchEntry(name, passwd string, directBind bool) (string, string, string, string, bool, bool) { + // See https://tools.ietf.org/search/rfc4513#section-5.1.2 + if len(passwd) == 0 { + log.Debug("Auth. failed for %s, password cannot be empty") + return "", "", "", "", false, false + } l, err := dial(ls) if err != nil { log.Error(4, "LDAP Connect error, %s:%v", ls.Host, err) From 8c5866bce2e047ccf5f5fea943962f5e16d0cdfc Mon Sep 17 00:00:00 2001 From: Denis Denisov Date: Sat, 10 Dec 2016 18:56:18 +0200 Subject: [PATCH 3/3] Clearing the login/email spaces at the [start/end] --- models/login_source.go | 4 ++-- modules/auth/ldap/ldap.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/models/login_source.go b/models/login_source.go index 7a5e6083a7cbb..58e0e88b3ea01 100644 --- a/models/login_source.go +++ b/models/login_source.go @@ -548,9 +548,9 @@ func ExternalUserLogin(user *User, login, password string, source *LoginSource, func UserSignIn(username, password string) (*User, error) { var user *User if strings.Contains(username, "@") { - user = &User{Email: strings.ToLower(username)} + user = &User{Email: strings.ToLower(strings.TrimSpace(username))} } else { - user = &User{LowerName: strings.ToLower(username)} + user = &User{LowerName: strings.ToLower(strings.TrimSpace(username))} } hasUser, err := x.Get(user) diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 90714392593a7..3064b319588cc 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -60,8 +60,8 @@ func (ls *Source) sanitizedUserQuery(username string) (string, bool) { func (ls *Source) sanitizedUserDN(username string) (string, bool) { // See http://tools.ietf.org/search/rfc4514: "special characters" - badCharacters := "\x00()*\\,='\"#+;<>" - if strings.ContainsAny(username, badCharacters) || strings.HasPrefix(username, " ") || strings.HasSuffix(username, " ") { + badCharacters := "\x00()*\\,='\"#+;<> " + if strings.ContainsAny(username, badCharacters) { log.Debug("'%s' contains invalid DN characters. Aborting.", username) return "", false }