diff --git a/lib/api/api_auth.go b/lib/api/api_auth.go index 73560a60d..ee3bb88e6 100644 --- a/lib/api/api_auth.go +++ b/lib/api/api_auth.go @@ -260,7 +260,8 @@ func authLDAP(username string, password string, cfg config.LDAPConfiguration) bo defer connection.Close() - err = connection.Bind(ldapTemplateBindDN(cfg.BindDN, username), password) + bindDN := formatOptionalPercentS(cfg.BindDN, escapeForLDAPDN(username)) + err = connection.Bind(bindDN, password) if err != nil { l.Warnln("LDAP Bind:", err) return false @@ -280,7 +281,7 @@ func authLDAP(username string, password string, cfg config.LDAPConfiguration) bo // the user. If this matches precisely one user then we are good to go. // The search filter uses the same %s interpolation as the bind DN. - searchString := fmt.Sprintf(cfg.SearchFilter, username) + searchString := formatOptionalPercentS(cfg.SearchFilter, escapeForLDAPFilter(username)) const sizeLimit = 2 // we search for up to two users -- we only want to match one, so getting any number >1 is a failure. const timeLimit = 60 // Search for up to a minute... searchReq := ldap.NewSearchRequest(cfg.SearchBaseDN, ldap.ScopeWholeSubtree, ldap.DerefFindingBaseObj, sizeLimit, timeLimit, false, searchString, nil, nil) @@ -298,13 +299,37 @@ func authLDAP(username string, password string, cfg config.LDAPConfiguration) bo return true } -func ldapTemplateBindDN(bindDN string, username string) string { - // Check if formatting directives are included in the ldapTemplateBindDN - if so add username. - // (%%s is a literal %s - unlikely for LDAP, but easy to handle here). - if strings.Count(bindDN, "%s") != strings.Count(bindDN, "%%s") { - bindDN = fmt.Sprintf(bindDN, username) +// escapeForLDAPFilter escapes a value that will be used in a filter clause +func escapeForLDAPFilter(value string) string { + // https://social.technet.microsoft.com/wiki/contents/articles/5392.active-directory-ldap-syntax-filters.aspx#Special_Characters + // Backslash must always be first in the list so we don't double escape them. + return escapeRunes(value, []rune{'\\', '*', '(', ')', 0}) +} + +// escapeForLDAPDN escapes a value that will be used in a bind DN +func escapeForLDAPDN(value string) string { + // https://social.technet.microsoft.com/wiki/contents/articles/5312.active-directory-characters-to-escape.aspx + // Backslash must always be first in the list so we don't double escape them. + return escapeRunes(value, []rune{'\\', ',', '#', '+', '<', '>', ';', '"', '=', ' ', 0}) +} + +func escapeRunes(value string, runes []rune) string { + for _, e := range runes { + value = strings.ReplaceAll(value, string(e), fmt.Sprintf("\\%X", e)) } - return bindDN + return value +} + +func formatOptionalPercentS(template string, username string) string { + var replacements []any + nReps := strings.Count(template, "%s") - strings.Count(template, "%%s") + if nReps < 0 { + nReps = 0 + } + for i := 0; i < nReps; i++ { + replacements = append(replacements, username) + } + return fmt.Sprintf(template, replacements...) } // Convert an ISO-8859-1 encoded byte string to UTF-8. Works by the diff --git a/lib/api/api_auth_test.go b/lib/api/api_auth_test.go index ed3677eeb..e4e207a09 100644 --- a/lib/api/api_auth_test.go +++ b/lib/api/api_auth_test.go @@ -46,22 +46,67 @@ func TestStaticAuthPasswordFail(t *testing.T) { } } -func TestAuthLDAPSendsCorrectBindDNWithTemplate(t *testing.T) { +func TestFormatOptionalPercentS(t *testing.T) { t.Parallel() - templatedDn := ldapTemplateBindDN("cn=%s,dc=some,dc=example,dc=com", "username") - expectedDn := "cn=username,dc=some,dc=example,dc=com" - if expectedDn != templatedDn { - t.Fatalf("ldapTemplateBindDN should be %s != %s", expectedDn, templatedDn) + cases := []struct { + template string + username string + expected string + }{ + {"cn=%s,dc=some,dc=example,dc=com", "username", "cn=username,dc=some,dc=example,dc=com"}, + {"cn=fixedusername,dc=some,dc=example,dc=com", "username", "cn=fixedusername,dc=some,dc=example,dc=com"}, + {"cn=%%s,dc=%s,dc=example,dc=com", "username", "cn=%s,dc=username,dc=example,dc=com"}, + {"cn=%%s,dc=%%s,dc=example,dc=com", "username", "cn=%s,dc=%s,dc=example,dc=com"}, + {"cn=%s,dc=%s,dc=example,dc=com", "username", "cn=username,dc=username,dc=example,dc=com"}, + } + + for _, c := range cases { + templatedDn := formatOptionalPercentS(c.template, c.username) + if c.expected != templatedDn { + t.Fatalf("result should be %s != %s", c.expected, templatedDn) + } } } -func TestAuthLDAPSendsCorrectBindDNWithNoTemplate(t *testing.T) { +func TestEscapeForLDAPFilter(t *testing.T) { t.Parallel() - templatedDn := ldapTemplateBindDN("cn=fixedusername,dc=some,dc=example,dc=com", "username") - expectedDn := "cn=fixedusername,dc=some,dc=example,dc=com" - if expectedDn != templatedDn { - t.Fatalf("ldapTemplateBindDN should be %s != %s", expectedDn, templatedDn) + cases := []struct { + in string + out string + }{ + {"username", `username`}, + {"user(name", `user\28name`}, + {"user)name", `user\29name`}, + {"user\\name", `user\5Cname`}, + {"user*name", `user\2Aname`}, + {"*,CN=asdf", `\2A,CN=asdf`}, + } + + for _, c := range cases { + res := escapeForLDAPFilter(c.in) + if c.out != res { + t.Fatalf("result should be %s != %s", c.out, res) + } + } +} + +func TestEscapeForLDAPDN(t *testing.T) { + t.Parallel() + + cases := []struct { + in string + out string + }{ + {"username", `username`}, + {"* ,CN=asdf", `*\20\2CCN\3Dasdf`}, + } + + for _, c := range cases { + res := escapeForLDAPDN(c.in) + if c.out != res { + t.Fatalf("result should be %s != %s", c.out, res) + } } }