mirror of
https://github.com/versity/versitygw.git
synced 2026-07-21 07:22:32 +00:00
fix: LDAP access key filter injection
LDAP-backed IAM lookups built a search filter using the request access key before signature verification, allowing filter metacharacters to alter query semantics. This made account lookup behavior observable and could be abused as a pre-authentication oracle in affected LDAP deployments. This change escapes access key values with ldap.EscapeFilter when constructing LDAP search filters and adds a regression test that verifies metacharacters are encoded rather than interpreted as filter operators. Also add DN escaping along with this fix too. Reported by GitHub user KrisKennawayDD.
This commit is contained in:
+8
-4
@@ -137,7 +137,7 @@ func (ld *LdapIAMService) CreateAccount(account Account) error {
|
||||
if ld.rootAcc.Access == account.Access {
|
||||
return ErrUserExists
|
||||
}
|
||||
userEntry := ldap.NewAddRequest(fmt.Sprintf("%v=%v,%v", ld.accessAtr, account.Access, ld.queryBase), nil)
|
||||
userEntry := ldap.NewAddRequest(ld.buildUserDN(account.Access), nil)
|
||||
userEntry.Attribute("objectClass", ld.objClasses)
|
||||
userEntry.Attribute(ld.accessAtr, []string{account.Access})
|
||||
userEntry.Attribute(ld.secretAtr, []string{account.Secret})
|
||||
@@ -156,13 +156,17 @@ func (ld *LdapIAMService) CreateAccount(account Account) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (ld *LdapIAMService) buildUserDN(access string) string {
|
||||
return fmt.Sprintf("%v=%v,%v", ld.accessAtr, ldap.EscapeDN(access), ld.queryBase)
|
||||
}
|
||||
|
||||
func (ld *LdapIAMService) buildSearchFilter(access string) string {
|
||||
var searchFilter strings.Builder
|
||||
for _, el := range ld.objClasses {
|
||||
searchFilter.WriteString(fmt.Sprintf("(objectClass=%v)", el))
|
||||
}
|
||||
if access != "" {
|
||||
searchFilter.WriteString(fmt.Sprintf("(%v=%v)", ld.accessAtr, access))
|
||||
searchFilter.WriteString(fmt.Sprintf("(%v=%v)", ld.accessAtr, ldap.EscapeFilter(access)))
|
||||
}
|
||||
return fmt.Sprintf("(&%v)", searchFilter.String())
|
||||
}
|
||||
@@ -236,7 +240,7 @@ func (ld *LdapIAMService) GetUserAccount(access string) (Account, error) {
|
||||
}
|
||||
|
||||
func (ld *LdapIAMService) UpdateUserAccount(access string, props MutableProps) error {
|
||||
req := ldap.NewModifyRequest(fmt.Sprintf("%v=%v, %v", ld.accessAtr, access, ld.queryBase), nil)
|
||||
req := ldap.NewModifyRequest(ld.buildUserDN(access), nil)
|
||||
if props.Secret != nil {
|
||||
req.Replace(ld.secretAtr, []string{*props.Secret})
|
||||
}
|
||||
@@ -264,7 +268,7 @@ func (ld *LdapIAMService) UpdateUserAccount(access string, props MutableProps) e
|
||||
}
|
||||
|
||||
func (ld *LdapIAMService) DeleteUserAccount(access string) error {
|
||||
delReq := ldap.NewDelRequest(fmt.Sprintf("%v=%v, %v", ld.accessAtr, access, ld.queryBase), nil)
|
||||
delReq := ldap.NewDelRequest(ld.buildUserDN(access), nil)
|
||||
|
||||
err := ld.execute(func(c *ldap.Conn) error {
|
||||
return c.Del(delReq)
|
||||
|
||||
@@ -2,6 +2,20 @@ package auth
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestLdapIAMService_BuildUserDN(t *testing.T) {
|
||||
ld := &LdapIAMService{
|
||||
accessAtr: "uid",
|
||||
queryBase: "dc=example,dc=com",
|
||||
}
|
||||
|
||||
access := "admin,+;<>\\value"
|
||||
expected := `uid=admin\,\+\;\<\>\\value,dc=example,dc=com`
|
||||
|
||||
if result := ld.buildUserDN(access); result != expected {
|
||||
t.Fatalf("buildUserDN() = %v, want %v", result, expected)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLdapIAMService_BuildSearchFilter(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
@@ -38,6 +52,13 @@ func TestLdapIAMService_BuildSearchFilter(t *testing.T) {
|
||||
access: "",
|
||||
expected: "(&(objectClass=inetOrgPerson)(objectClass=organizationalPerson)(objectClass=person))",
|
||||
},
|
||||
{
|
||||
name: "escape ldap filter metacharacters in access",
|
||||
objClasses: []string{"inetOrgPerson"},
|
||||
accessAtr: "uid",
|
||||
access: "admin)(s3secret=A*",
|
||||
expected: "(&(objectClass=inetOrgPerson)(uid=admin\\29\\28s3secret=A\\2a))",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
||||
Reference in New Issue
Block a user