Add ldap filter syntax validation when create search filter (#13008)

Correct ldap search filter is enclosed with '(' and ')'
Search ldap group with the ldap group base DN instead of group DN
Fixes #12613 LDAP Group Filter and Group Base DN have no affect

Signed-off-by: stonezdj <stonezdj@gmail.com>
This commit is contained in:
stonezdj(Daojun Zhang) 2020-09-09 14:37:51 +08:00 committed by GitHub
parent b0877327c7
commit 12f356d1bf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 357 additions and 72 deletions

View File

@ -0,0 +1,75 @@
// Copyright Project Harbor Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package ldap
import (
ber "gopkg.in/asn1-ber.v1"
goldap "gopkg.in/ldap.v2"
"strings"
)
// FilterBuilder build filter for ldap search
type FilterBuilder struct {
packet *ber.Packet
}
// Or ...
func (f *FilterBuilder) Or(filterB *FilterBuilder) *FilterBuilder {
if f.packet == nil {
return filterB
}
if filterB.packet == nil {
return f
}
p := ber.Encode(ber.ClassContext, ber.TypeConstructed, goldap.FilterOr, nil, goldap.FilterMap[goldap.FilterOr])
p.AppendChild(f.packet)
p.AppendChild(filterB.packet)
return &FilterBuilder{packet: p}
}
// And ...
func (f *FilterBuilder) And(filterB *FilterBuilder) *FilterBuilder {
if f.packet == nil {
return filterB
}
if filterB.packet == nil {
return f
}
p := ber.Encode(ber.ClassContext, ber.TypeConstructed, goldap.FilterAnd, nil, goldap.FilterMap[goldap.FilterAnd])
p.AppendChild(f.packet)
p.AppendChild(filterB.packet)
return &FilterBuilder{packet: p}
}
// String ...
func (f *FilterBuilder) String() (string, error) {
if f.packet == nil {
return "", nil
}
return goldap.DecompileFilter(f.packet)
}
// NewFilterBuilder parse FilterBuilder from string
func NewFilterBuilder(filter string) (*FilterBuilder, error) {
f := normalizeFilter(filter)
if len(strings.TrimSpace(f)) == 0 {
return &FilterBuilder{}, nil
}
p, err := goldap.CompileFilter(f)
if err != nil {
return &FilterBuilder{}, ErrInvalidFilter
}
return &FilterBuilder{packet: p}, nil
}

View File

@ -0,0 +1,109 @@
// Copyright Project Harbor Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package ldap
import (
"github.com/stretchr/testify/assert"
"testing"
)
func TestFilterBuilder(t *testing.T) {
fb, err := NewFilterBuilder("(objectclass=groupOfNames)")
if err != nil {
t.Error(err)
}
fb2, err := NewFilterBuilder("(cn=admin)")
if err != nil {
t.Error(err)
}
fb3 := fb.And(fb2)
result, err := fb3.String()
assert.Equal(t, result, "(&(objectclass=groupOfNames)(cn=admin))")
}
func TestFilterBuilder_And(t *testing.T) {
type args struct {
filterA string
filterB string
}
cases := []struct {
name string
in args
want string
wantError error
}{
{name: `normal`, in: args{"(objectclass=groupOfNames)", "(cn=admin)"}, want: "(&(objectclass=groupOfNames)(cn=admin))", wantError: nil},
{name: `empty left`, in: args{"", "(cn=admin)"}, want: "(cn=admin))", wantError: nil},
{name: `empty right`, in: args{"(cn=admin)", ""}, want: "(cn=admin))", wantError: nil},
{name: `both empty`, in: args{"", ""}, want: "", wantError: nil},
}
for _, tt := range cases {
t.Run(tt.name, func(t *testing.T) {
fbA, err := NewFilterBuilder(tt.in.filterA)
if err != nil {
t.Error(err)
}
fbB, err := NewFilterBuilder(tt.in.filterB)
if err != nil {
t.Error(fbB)
}
got, err := fbA.And(fbB).String()
if got != tt.want && err != tt.wantError {
t.Errorf(`(%v) = %v; want "%v"`, tt.in, got, tt.want)
}
})
}
}
func TestFilterBuilder_Or(t *testing.T) {
type args struct {
filterA string
filterB string
}
cases := []struct {
name string
in args
want string
wantError error
}{
{name: `normal`, in: args{"(objectclass=groupOfNames)", "(cn=admin)"}, want: "(|(objectclass=groupOfNames)(cn=admin))", wantError: nil},
{name: `empty left`, in: args{"", "(cn=admin)"}, want: "(cn=admin))", wantError: nil},
{name: `empty right`, in: args{"(cn=admin)", ""}, want: "(cn=admin))", wantError: nil},
{name: `both empty`, in: args{"", ""}, want: "", wantError: nil},
}
for _, tt := range cases {
t.Run(tt.name, func(t *testing.T) {
fbA, err := NewFilterBuilder(tt.in.filterA)
if err != nil {
t.Error(err)
}
fbB, err := NewFilterBuilder(tt.in.filterB)
if err != nil {
t.Error(fbB)
}
got, err := fbA.Or(fbB).String()
if got != tt.want && err != tt.wantError {
t.Errorf(`(%v) = %v; want "%v"`, tt.in, got, tt.want)
}
})
}
}

View File

@ -34,7 +34,10 @@ import (
var ErrNotFound = errors.New("entity not found")
// ErrDNSyntax ...
var ErrDNSyntax = errors.New("Invalid DN syntax")
var ErrDNSyntax = errors.New("invalid DN syntax")
// ErrInvalidFilter ...
var ErrInvalidFilter = errors.New("invalid filter syntax")
// Session - define a LDAP session
type Session struct {
@ -186,9 +189,12 @@ func ConnectionTestWithAllConfig(ldapConfig models.LdapConf, ldapGroupConfig mod
// SearchUser - search LDAP user by name
func (session *Session) SearchUser(username string) ([]models.LdapUser, error) {
var ldapUsers []models.LdapUser
ldapFilter := session.createUserFilter(username)
result, err := session.SearchLdap(ldapFilter)
ldapFilter, err := createUserSearchFilter(session.ldapConfig.LdapFilter, session.ldapConfig.LdapUID, username)
if err != nil {
return nil, err
}
result, err := session.SearchLdap(ldapFilter)
if err != nil {
return nil, err
}
@ -292,6 +298,10 @@ func (session *Session) SearchLdapAttribute(baseDN, filter string, attributes []
if !(strings.HasPrefix(filter, "(") || strings.HasSuffix(filter, ")")) {
filter = "(" + filter + ")"
}
if _, err := goldap.CompileFilter(filter); err != nil {
log.Errorf("Wrong filter format, filter:%v", filter)
return nil, ErrInvalidFilter
}
log.Debugf("Search ldap with filter:%v", filter)
searchRequest := goldap.NewSearchRequest(
baseDN,
@ -321,28 +331,24 @@ func (session *Session) SearchLdapAttribute(baseDN, filter string, attributes []
}
// CreateUserFilter - create filter to search user with specified username
func (session *Session) createUserFilter(username string) string {
// createUserSearchFilter - create filter to search user with specified username
func createUserSearchFilter(origFilter, ldapUID, username string) (string, error) {
oFilter, err := NewFilterBuilder(origFilter)
if err != nil {
return "", err
}
var filterTag string
if username == "" {
filterTag = goldap.EscapeFilter(username)
if len(filterTag) == 0 {
filterTag = "*"
} else {
filterTag = goldap.EscapeFilter(username)
}
ldapFilter := normalizeFilter(session.ldapConfig.LdapFilter)
ldapUID := session.ldapConfig.LdapUID
if ldapFilter == "" {
ldapFilter = "(" + ldapUID + "=" + filterTag + ")"
} else {
ldapFilter = "(&(" + ldapFilter + ")(" + ldapUID + "=" + filterTag + "))"
uFilterStr := fmt.Sprintf("(%v=%v)", ldapUID, filterTag)
uFilter, err := NewFilterBuilder(uFilterStr)
if err != nil {
return "", err
}
log.Debug("ldap filter :", ldapFilter)
return ldapFilter
filter := oFilter.And(uFilter)
return filter.String()
}
// Close - close current session
@ -375,17 +381,31 @@ func (session *Session) SearchGroupByDN(groupDN string) ([]models.LdapGroup, err
return groupList, err
}
func (session *Session) searchGroup(baseDN, filter, groupName, groupNameAttribute string) ([]models.LdapGroup, error) {
func (session *Session) groupBaseDN() string {
if len(session.ldapGroupConfig.LdapGroupBaseDN) == 0 {
return session.ldapConfig.LdapBaseDn
}
return session.ldapGroupConfig.LdapGroupBaseDN
}
func (session *Session) searchGroup(groupDN, filter, groupName, groupNameAttribute string) ([]models.LdapGroup, error) {
ldapGroups := make([]models.LdapGroup, 0)
log.Debugf("Groupname: %v, basedn: %v", groupName, baseDN)
ldapFilter := createGroupSearchFilter(filter, groupName, groupNameAttribute)
log.Debugf("Groupname: %v, groupDN: %v", groupName, groupDN)
ldapFilter, err := createGroupSearchFilter(filter, groupName, groupNameAttribute)
if err != nil {
log.Errorf("wrong filter format: filter:%v, groupName:%v, groupNameAttribute:%v", filter, groupName, groupNameAttribute)
return nil, err
}
attributes := []string{groupNameAttribute}
result, err := session.SearchLdapAttribute(baseDN, ldapFilter, attributes)
result, err := session.SearchLdapAttribute(session.groupBaseDN(), ldapFilter, attributes)
if err != nil {
return nil, err
}
for _, ldapEntry := range result.Entries {
var group models.LdapGroup
if groupDN != ldapEntry.DN {
continue
}
group.GroupDN = ldapEntry.DN
for _, attr := range ldapEntry.Attributes {
// OpenLdap sometimes contain leading space in username
@ -401,40 +421,34 @@ func (session *Session) searchGroup(baseDN, filter, groupName, groupNameAttribut
return ldapGroups, nil
}
func createGroupSearchFilter(oldFilter, groupName, groupNameAttribute string) string {
filter := ""
func createGroupSearchFilter(oldFilterStr, groupName, groupNameAttribute string) (string, error) {
origFilter, err := NewFilterBuilder(oldFilterStr)
if err != nil {
log.Errorf("failed to create group search filter:%v", oldFilterStr)
return "", err
}
groupName = goldap.EscapeFilter(groupName)
groupNameAttribute = goldap.EscapeFilter(groupNameAttribute)
oldFilter = normalizeFilter(oldFilter)
if len(oldFilter) == 0 {
if len(groupName) == 0 {
filter = groupNameAttribute + "=*"
} else {
filter = groupNameAttribute + "=*" + groupName + "*"
}
} else {
if len(groupName) == 0 {
filter = oldFilter
} else {
filter = "(&(" + oldFilter + ")(" + groupNameAttribute + "=*" + groupName + "*))"
}
gFilterStr := ""
if len(groupName) > 0 {
gFilterStr = fmt.Sprintf("(%v=%v)", goldap.EscapeFilter(groupNameAttribute), groupName)
}
return filter
gFilter, err := NewFilterBuilder(gFilterStr)
if err != nil {
log.Errorf("invalid ldap filter:%v", gFilterStr)
return "", err
}
fb := origFilter.And(gFilter)
return fb.String()
}
func contains(s []string, e string) bool {
for _, a := range s {
if a == e {
return true
}
}
return false
}
// normalizeFilter - remove '(' and ')' in ldap filter
// normalizeFilter - add '(' and ')' in ldap filter if it doesn't exist
func normalizeFilter(filter string) string {
norFilter := strings.TrimSpace(filter)
norFilter = strings.TrimPrefix(norFilter, "(")
norFilter = strings.TrimSuffix(norFilter, ")")
return norFilter
if len(norFilter) == 0 {
return norFilter
}
if strings.HasPrefix(norFilter, "(") && strings.HasSuffix(norFilter, ")") {
return norFilter
}
return "(" + norFilter + ")"
}

View File

@ -234,18 +234,21 @@ func Test_createGroupSearchFilter(t *testing.T) {
groupNameAttribute string
}
tests := []struct {
name string
args args
want string
name string
args args
want string
wantErr error
}{
{"Normal Filter", args{oldFilter: "objectclass=groupOfNames", groupName: "harbor_users", groupNameAttribute: "cn"}, "(&(objectclass=groupOfNames)(cn=*harbor_users*))"},
{"Empty Old", args{groupName: "harbor_users", groupNameAttribute: "cn"}, "cn=*harbor_users*"},
{"Empty Both", args{groupNameAttribute: "cn"}, "cn=*"},
{"Empty name", args{oldFilter: "objectclass=groupOfNames", groupNameAttribute: "cn"}, "objectclass=groupOfNames"},
{"Normal Filter", args{oldFilter: "objectclass=groupOfNames", groupName: "harbor_users", groupNameAttribute: "cn"}, "(&(objectclass=groupOfNames)(cn=*harbor_users*))", nil},
{"Empty Old", args{groupName: "harbor_users", groupNameAttribute: "cn"}, "(cn=*harbor_users*)", nil},
{"Empty Both", args{groupNameAttribute: "cn"}, "(cn=*)", nil},
{"Empty name", args{oldFilter: "objectclass=groupOfNames", groupNameAttribute: "cn"}, "(objectclass=groupOfNames)", nil},
{"Empty name with complex filter", args{oldFilter: "(&(objectClass=groupOfNames)(cn=*sample*))", groupNameAttribute: "cn"}, "(&(objectClass=groupOfNames)(cn=*sample*))", nil},
{"Empty name with bad filter", args{oldFilter: "(&(objectClass=groupOfNames),cn=*sample*)", groupNameAttribute: "cn"}, "", ErrInvalidFilter},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := createGroupSearchFilter(tt.args.oldFilter, tt.args.groupName, tt.args.groupNameAttribute); got != tt.want {
if got, err := createGroupSearchFilter(tt.args.oldFilter, tt.args.groupName, tt.args.groupNameAttribute); got != tt.want && err != tt.wantErr {
t.Errorf("createGroupSearchFilter() = %v, want %v", got, tt.want)
}
})
@ -258,7 +261,7 @@ func TestSession_SearchGroup(t *testing.T) {
ldapConn *goldap.Conn
}
type args struct {
baseDN string
groupDN string
filter string
groupName string
groupNameAttribute string
@ -281,7 +284,7 @@ func TestSession_SearchGroup(t *testing.T) {
}{
{"normal search",
fields{ldapConfig: ldapConfig},
args{baseDN: "dc=example,dc=com", filter: "objectClass=groupOfNames", groupName: "harbor_users", groupNameAttribute: "cn"},
args{groupDN: "cn=harbor_users,ou=groups,dc=example,dc=com", filter: "objectClass=groupOfNames", groupName: "harbor_users", groupNameAttribute: "cn"},
[]models.LdapGroup{{GroupName: "harbor_users", GroupDN: "cn=harbor_users,ou=groups,dc=example,dc=com"}}, false},
}
for _, tt := range tests {
@ -292,7 +295,7 @@ func TestSession_SearchGroup(t *testing.T) {
}
session.Open()
defer session.Close()
got, err := session.searchGroup(tt.args.baseDN, tt.args.filter, tt.args.groupName, tt.args.groupNameAttribute)
got, err := session.searchGroup(tt.args.groupDN, tt.args.filter, tt.args.groupName, tt.args.groupNameAttribute)
if (err != nil) != tt.wantErr {
t.Errorf("Session.SearchGroup() error = %v, wantErr %v", err, tt.wantErr)
return
@ -313,17 +316,36 @@ func TestSession_SearchGroupByDN(t *testing.T) {
LdapBaseDn: ldapTestConfig[common.LDAPBaseDN].(string),
}
ldapGroupConfig := models.LdapGroupConf{
LdapGroupBaseDN: "ou=group,dc=example,dc=com",
LdapGroupBaseDN: "dc=example,dc=com",
LdapGroupFilter: "objectclass=groupOfNames",
LdapGroupNameAttribute: "cn",
LdapGroupSearchScope: 2,
}
ldapGroupConfig2 := models.LdapGroupConf{
LdapGroupBaseDN: "ou=group,dc=example,dc=com",
LdapGroupBaseDN: "dc=example,dc=com",
LdapGroupFilter: "objectclass=groupOfNames",
LdapGroupNameAttribute: "o",
LdapGroupSearchScope: 2,
}
groupConfigWithEmptyBaseDN := models.LdapGroupConf{
LdapGroupBaseDN: "",
LdapGroupFilter: "(objectclass=groupOfNames)",
LdapGroupNameAttribute: "cn",
LdapGroupSearchScope: 2,
}
groupConfigWithFilter := models.LdapGroupConf{
LdapGroupBaseDN: "dc=example,dc=com",
LdapGroupFilter: "(cn=*admin*)",
LdapGroupNameAttribute: "cn",
LdapGroupSearchScope: 2,
}
groupConfigWithDifferentGroupDN := models.LdapGroupConf{
LdapGroupBaseDN: "dc=harbor,dc=example,dc=com",
LdapGroupFilter: "(objectclass=groupOfNames)",
LdapGroupNameAttribute: "cn",
LdapGroupSearchScope: 2,
}
type fields struct {
ldapConfig models.LdapConf
ldapGroupConfig models.LdapGroupConf
@ -346,7 +368,7 @@ func TestSession_SearchGroupByDN(t *testing.T) {
{"search non-exist group",
fields{ldapConfig: ldapConfig, ldapGroupConfig: ldapGroupConfig},
args{groupDN: "cn=harbor_non_users,ou=groups,dc=example,dc=com"},
nil, true},
[]models.LdapGroup{}, false},
{"search invalid group dn",
fields{ldapConfig: ldapConfig, ldapGroupConfig: ldapGroupConfig},
args{groupDN: "random string"},
@ -359,6 +381,26 @@ func TestSession_SearchGroupByDN(t *testing.T) {
fields{ldapConfig: ldapConfig, ldapGroupConfig: ldapGroupConfig2},
args{groupDN: "cn=harbor_group,ou=groups,dc=example,dc=com"},
[]models.LdapGroup{{GroupName: "hgroup", GroupDN: "cn=harbor_group,ou=groups,dc=example,dc=com"}}, false},
{"search with empty group base dn",
fields{ldapConfig: ldapConfig, ldapGroupConfig: groupConfigWithEmptyBaseDN},
args{groupDN: "cn=harbor_group,ou=groups,dc=example,dc=com"},
[]models.LdapGroup{{GroupName: "harbor_group", GroupDN: "cn=harbor_group,ou=groups,dc=example,dc=com"}}, false},
{"search with group filter success",
fields{ldapConfig: ldapConfig, ldapGroupConfig: groupConfigWithFilter},
args{groupDN: "cn=harbor_admin,ou=groups,dc=example,dc=com"},
[]models.LdapGroup{{GroupName: "harbor_admin", GroupDN: "cn=harbor_admin,ou=groups,dc=example,dc=com"}}, false},
{"search with group filter fail",
fields{ldapConfig: ldapConfig, ldapGroupConfig: groupConfigWithFilter},
args{groupDN: "cn=harbor_users,ou=groups,dc=example,dc=com"},
[]models.LdapGroup{}, false},
{"search with different group base dn success",
fields{ldapConfig: ldapConfig, ldapGroupConfig: groupConfigWithDifferentGroupDN},
args{groupDN: "cn=harbor_root,dc=harbor,dc=example,dc=com"},
[]models.LdapGroup{{GroupName: "harbor_root", GroupDN: "cn=harbor_root,dc=harbor,dc=example,dc=com"}}, false},
{"search with different group base dn fail",
fields{ldapConfig: ldapConfig, ldapGroupConfig: groupConfigWithDifferentGroupDN},
args{groupDN: "cn=harbor_guest,ou=groups,dc=example,dc=com"},
[]models.LdapGroup{}, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -381,6 +423,35 @@ func TestSession_SearchGroupByDN(t *testing.T) {
}
}
func TestCreateUserSearchFilter(t *testing.T) {
type args struct {
origFilter string
ldapUID string
username string
}
cases := []struct {
name string
in args
want string
wantErr error
}{
{name: `Normal test`, in: args{"(objectclass=inetorgperson)", "cn", "sample"}, want: "(&(objectclass=inetorgperson)(cn=sample)", wantErr: nil},
{name: `Bad original filter`, in: args{"(objectclass=inetorgperson)ldap*", "cn", "sample"}, want: "", wantErr: ErrInvalidFilter},
{name: `Complex original filter`, in: args{"(&(objectclass=inetorgperson)(|(memberof=cn=harbor_users,ou=groups,dc=example,dc=com)(memberof=cn=harbor_admin,ou=groups,dc=example,dc=com)(memberof=cn=harbor_guest,ou=groups,dc=example,dc=com)))", "cn", "sample"}, want: "(&(&(objectclass=inetorgperson)(|(memberof=cn=harbor_users,ou=groups,dc=example,dc=com)(memberof=cn=harbor_admin,ou=groups,dc=example,dc=com)(memberof=cn=harbor_guest,ou=groups,dc=example,dc=com)))(cn=sample)", wantErr: nil},
{name: `Empty original filter`, in: args{"", "cn", "sample"}, want: "(cn=sample)", wantErr: nil},
}
for _, tt := range cases {
t.Run(tt.name, func(t *testing.T) {
got, gotErr := createUserSearchFilter(tt.in.origFilter, tt.in.ldapUID, tt.in.origFilter)
if got != tt.want && gotErr != tt.wantErr {
t.Errorf(`(%v) = %v; want "%v"`, tt.in, got, tt.want)
}
})
}
}
func TestNormalizeFilter(t *testing.T) {
type args struct {
filter string
@ -390,9 +461,11 @@ func TestNormalizeFilter(t *testing.T) {
args args
want string
}{
{"normal test", args{"(objectclass=user)"}, "objectclass=user"},
{"with space", args{" (objectclass=user) "}, "objectclass=user"},
{"nothing", args{"objectclass=user"}, "objectclass=user"},
{"normal test", args{"(objectclass=user)"}, "(objectclass=user)"},
{"with space", args{" (objectclass=user) "}, "(objectclass=user)"},
{"nothing", args{"objectclass=user"}, "(objectclass=user)"},
{"and condition", args{"&(objectclass=user)(cn=admin)"}, "(&(objectclass=user)(cn=admin))"},
{"or condition", args{"|(objectclass=user)(cn=admin)"}, "(|(objectclass=user)(cn=admin))"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

View File

@ -773,6 +773,20 @@ uid: admin_user
uidnumber: 6003
userpassword: {MD5}wb68DeX0CyENafzUADNn9A==
memberof: cn=harbor_admin,ou=groups,dc=example,dc=com
memberof: cn=harbor_root,dc=harbor,dc=example,dc=com
dn: dc=harbor,dc=example,dc=com
associateddomain: harbor
dc: harbor
objectclass: dNSDomain
objectclass: domainRelatedObject
objectclass: top
# Group Entry harbor_admin
dn: cn=harbor_root,dc=harbor,dc=example,dc=com
cn: harbor_root
description: harbor root users
member: cn=admin_user,ou=people,dc=example,dc=com
objectclass: groupOfNames
objectclass: top