From 44abd15162c92bbf52abd2c594abdaad52f239bd Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Wed, 29 Apr 2026 12:06:12 +0200 Subject: [PATCH] chore(api): use ldap package escape functions (#10672) Instead of our own variants, which were mostly but not 100% identical. --------- Signed-off-by: Jakob Borg --- lib/api/api_auth.go | 25 ++---------------------- lib/api/api_auth_test.go | 42 ---------------------------------------- 2 files changed, 2 insertions(+), 65 deletions(-) diff --git a/lib/api/api_auth.go b/lib/api/api_auth.go index 3aaa72bbe..ad2225994 100644 --- a/lib/api/api_auth.go +++ b/lib/api/api_auth.go @@ -270,7 +270,7 @@ func authLDAP(username string, password string, cfg config.LDAPConfiguration) bo defer connection.Close() - bindDN := formatOptionalPercentS(cfg.BindDN, escapeForLDAPDN(username)) + bindDN := formatOptionalPercentS(cfg.BindDN, ldap.EscapeDN(username)) err = connection.Bind(bindDN, password) if err != nil { slog.Error("Failed to bind with LDAP server", slogutil.Error(err)) @@ -291,7 +291,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 := formatOptionalPercentS(cfg.SearchFilter, escapeForLDAPFilter(username)) + searchString := formatOptionalPercentS(cfg.SearchFilter, ldap.EscapeFilter(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) @@ -309,27 +309,6 @@ func authLDAP(username string, password string, cfg config.LDAPConfiguration) bo return true } -// 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 value -} - func formatOptionalPercentS(template string, username string) string { var replacements []any nReps := strings.Count(template, "%s") - strings.Count(template, "%%s") diff --git a/lib/api/api_auth_test.go b/lib/api/api_auth_test.go index 97431b948..ee304d812 100644 --- a/lib/api/api_auth_test.go +++ b/lib/api/api_auth_test.go @@ -73,48 +73,6 @@ func TestFormatOptionalPercentS(t *testing.T) { } } -func TestEscapeForLDAPFilter(t *testing.T) { - t.Parallel() - - 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) - } - } -} - type mockClock struct { now time.Time }