Add ldap filter syntax validation when create search filter

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 2020-09-09 10:07:07 +08:00
parent bc3433194b
commit b9752f3112
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