From 0c801e02d5b9804c5444d4e923babc34ee05b61c Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Sun, 2 Feb 2020 13:51:47 +0100 Subject: [PATCH] Remove spaces between dn components, use warnings when necessary --- bottin.hcl.example | 2 +- main.go | 9 ++++++++- read.go | 16 +++++++-------- util.go | 20 +++++++++++++++++-- write.go | 49 +++++++++++++++++++++++----------------------- 5 files changed, 59 insertions(+), 37 deletions(-) diff --git a/bottin.hcl.example b/bottin.hcl.example index 59f7c57..eb29095 100644 --- a/bottin.hcl.example +++ b/bottin.hcl.example @@ -12,7 +12,7 @@ job "directory" { task "server" { driver = "docker" config { - image = "lxpz/bottin_amd64:5" + image = "lxpz/bottin_amd64:6" readonly_rootfs = true port_map { ldap_port = 389 diff --git a/main.go b/main.go index 58fa466..4ee5f10 100644 --- a/main.go +++ b/main.go @@ -377,7 +377,14 @@ func (server *Server) objectExists(dn string) (bool, error) { return len(data) > 0, nil } -func (server *Server) checkSuffix(dn string, allow_extend bool) (string, error) { +func (server *Server) checkDN(dn string, allow_extend bool) (string, error) { + // 1. Canonicalize: remove spaces between things + dn, err := canonicalDN(dn) + if err != nil { + return "", err + } + + // 2. Check suffix (add it if allow_extend is set) suffix := server.config.Suffix if len(dn) < len(suffix) { if dn != suffix[len(suffix)-len(dn):] || !allow_extend { diff --git a/read.go b/read.go index 0b6d345..63a1009 100644 --- a/read.go +++ b/read.go @@ -24,11 +24,10 @@ func (server *Server) handleCompare(s ldap.UserState, w ldap.ResponseWriter, m * } func (server *Server) handleCompareInternal(state *State, r *message.CompareRequest) (int, error) { - dn := string(r.Entry()) attr := string(r.Ava().AttributeDesc()) expected := string(r.Ava().AssertionValue()) - _, err := server.checkSuffix(dn, false) + dn, err := server.checkDN(string(r.Entry()), false) if err != nil { return ldap.LDAPResultInvalidDNSyntax, err } @@ -81,21 +80,22 @@ func (server *Server) handleSearch(s ldap.UserState, w ldap.ResponseWriter, m *l func (server *Server) handleSearchInternal(state *State, w ldap.ResponseWriter, r *message.SearchRequest) (int, error) { + baseObject, err := server.checkDN(string(r.BaseObject()), true) + if err != nil { + return ldap.LDAPResultInvalidDNSyntax, err + } + server.logger.Tracef("-- SEARCH REQUEST: --") - server.logger.Tracef("Request BaseDn=%s", r.BaseObject()) + server.logger.Tracef("Request BaseDn=%s", baseObject) server.logger.Tracef("Request Filter=%s", r.Filter()) server.logger.Tracef("Request FilterString=%s", r.FilterString()) server.logger.Tracef("Request Attributes=%s", r.Attributes()) server.logger.Tracef("Request TimeLimit=%d", r.TimeLimit().Int()) - if !server.config.Acl.Check(&state.login, "read", string(r.BaseObject()), []string{}) { + if !server.config.Acl.Check(&state.login, "read", baseObject, []string{}) { return ldap.LDAPResultInsufficientAccessRights, fmt.Errorf("Please specify a base object on which you have read rights") } - baseObject, err := server.checkSuffix(string(r.BaseObject()), true) - if err != nil { - return ldap.LDAPResultInvalidDNSyntax, err - } basePath, err := dnToConsul(baseObject) if err != nil { return ldap.LDAPResultInvalidDNSyntax, err diff --git a/util.go b/util.go index 4add5ee..96bb00b 100644 --- a/util.go +++ b/util.go @@ -98,13 +98,29 @@ func parseDN(dn string) ([]DNComponent, error) { return nil, fmt.Errorf("Wrong DN component: %s (expected type=value)", rdn) } ret = append(ret, DNComponent{ - Type: splits[0], - Value: splits[1], + Type: strings.TrimSpace(splits[0]), + Value: strings.TrimSpace(splits[1]), }) } return ret, nil } +func canonicalDN(dn string) (string, error) { + path, err := parseDN(dn) + if err != nil { + return "", err + } + + ret := "" + for _, c := range path { + if ret != "" { + ret = ret + "," + } + ret = ret + c.Type + "=" + c.Value + } + return ret, nil +} + func checkRestrictedAttr(attr string) error { RESTRICTED_ATTRS := []string{ ATTR_MEMBEROF, diff --git a/write.go b/write.go index 4da1a53..4874775 100644 --- a/write.go +++ b/write.go @@ -29,9 +29,7 @@ func (server *Server) handleAdd(s ldap.UserState, w ldap.ResponseWriter, m *ldap } func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (int, error) { - dn := string(r.Entry()) - - _, err := server.checkSuffix(dn, false) + dn, err := server.checkDN(string(r.Entry()), false) if err != nil { return ldap.LDAPResultInvalidDNSyntax, err } @@ -80,18 +78,18 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in if strings.EqualFold(key, ATTR_MEMBER) { members = vals_str for _, member := range members { - _, err := server.checkSuffix(member, false) + member_canonical, err := server.checkDN(member, false) if err != nil { return ldap.LDAPResultInvalidDNSyntax, err } - exists, err = server.objectExists(member) + exists, err = server.objectExists(member_canonical) if err != nil { return ldap.LDAPResultOperationsError, err } if !exists { return ldap.LDAPResultNoSuchObject, fmt.Errorf( "Cannot add %s to members, it does not exist!", - member) + member_canonical) } } } @@ -103,7 +101,7 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in entry[ATTR_ENTRYUUID] = []string{genUuid()} entry[dnSplit[0].Type] = []string{dnSplit[0].Value} - // Add our intem in the DB + // Add our item in the DB err = server.addElements(dn, entry) if err != nil { return ldap.LDAPResultOperationsError, err @@ -116,7 +114,7 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in for _, member := range members { memberGroups, err := server.getAttribute(member, ATTR_MEMBEROF) if err != nil { - server.logger.Printf("Could not add %s to memberOf of %s: %s", dn, member, err) + server.logger.Warnf("Could not add %s to memberOf of %s: %s", dn, member, err) continue } if memberGroups == nil { @@ -127,7 +125,7 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in for _, mb := range memberGroups { if mb == dn { alreadyMember = true - server.logger.Printf("Warning: inconsistency detected, %s was memberOf %s at a time when it didn't exist!", + server.logger.Warnf("Warning: inconsistency detected, %s was memberOf %s at a time when it didn't exist!", member, dn) break } @@ -139,7 +137,7 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in ATTR_MEMBEROF: memberGroups, }) if err != nil { - server.logger.Printf("Could not add %s to memberOf of %s: %s", dn, member, err) + server.logger.Warnf("Could not add %s to memberOf of %s: %s", dn, member, err) } } } @@ -169,9 +167,7 @@ func (server *Server) handleDelete(s ldap.UserState, w ldap.ResponseWriter, m *l } func (server *Server) handleDeleteInternal(state *State, r *message.DelRequest) (int, error) { - dn := string(*r) - - _, err := server.checkSuffix(dn, false) + dn, err := server.checkDN(string(*r), false) if err != nil { return ldap.LDAPResultInvalidDNSyntax, err } @@ -229,7 +225,7 @@ func (server *Server) handleDeleteInternal(state *State, r *message.DelRequest) for _, group := range memberOf { groupMembers, err := server.getAttribute(group, ATTR_MEMBER) if err != nil { - server.logger.Printf("Could not remove %s from members of %s: %s", dn, group, err) + server.logger.Warnf("Could not remove %s from members of %s: %s", dn, group, err) continue } @@ -244,7 +240,7 @@ func (server *Server) handleDeleteInternal(state *State, r *message.DelRequest) ATTR_MEMBER: newMembers, }) if err != nil { - server.logger.Printf("Could not remove %s from members of %s: %s", dn, group, err) + server.logger.Warnf("Could not remove %s from members of %s: %s", dn, group, err) } } } @@ -254,7 +250,7 @@ func (server *Server) handleDeleteInternal(state *State, r *message.DelRequest) for _, member := range memberList { memberOf, err := server.getAttribute(member, ATTR_MEMBEROF) if err != nil || memberOf == nil { - server.logger.Printf("Could not remove %s from memberOf of %s: %s", dn, member, err) + server.logger.Warnf("Could not remove %s from memberOf of %s: %s", dn, member, err) continue } @@ -269,7 +265,7 @@ func (server *Server) handleDeleteInternal(state *State, r *message.DelRequest) ATTR_MEMBEROF: newMemberOf, }) if err != nil { - server.logger.Printf("Could not remove %s from memberOf of %s: %s", dn, member, err) + server.logger.Warnf("Could not remove %s from memberOf of %s: %s", dn, member, err) } } } @@ -298,9 +294,7 @@ func (server *Server) handleModify(s ldap.UserState, w ldap.ResponseWriter, m *l } func (server *Server) handleModifyInternal(state *State, r *message.ModifyRequest) (int, error) { - dn := string(r.Object()) - - _, err := server.checkSuffix(dn, false) + dn, err := server.checkDN(string(r.Object()), false) if err != nil { return ldap.LDAPResultInvalidDNSyntax, err } @@ -447,7 +441,11 @@ func (server *Server) handleModifyInternal(state *State, r *message.ModifyReques } // Check that added members actually exist - for _, addMem := range addMembers { + for i := range addMembers { + addMem, err := server.checkDN(addMembers[i], false) + if err != nil { + return ldap.LDAPResultInvalidDNSyntax, err + } exists, err := server.objectExists(addMem) if err != nil { return ldap.LDAPResultOperationsError, err @@ -456,6 +454,7 @@ func (server *Server) handleModifyInternal(state *State, r *message.ModifyReques return ldap.LDAPResultNoSuchObject, fmt.Errorf( "Cannot add member %s, it does not exist", addMem) } + addMembers[i] = addMem } newEntry[ATTR_MODIFIERSNAME] = []string{state.login.user} @@ -470,7 +469,7 @@ func (server *Server) handleModifyInternal(state *State, r *message.ModifyReques for _, addMem := range addMembers { memberOf, err := server.getAttribute(addMem, ATTR_MEMBEROF) if err != nil { - server.logger.Printf("Could not add %s to memberOf of %s: %s", dn, addMem, err) + server.logger.Warnf("Could not add %s to memberOf of %s: %s", dn, addMem, err) continue } if memberOf == nil { @@ -490,7 +489,7 @@ func (server *Server) handleModifyInternal(state *State, r *message.ModifyReques ATTR_MEMBEROF: memberOf, }) if err != nil { - server.logger.Printf("Could not add %s to memberOf of %s: %s", dn, addMem, err) + server.logger.Warnf("Could not add %s to memberOf of %s: %s", dn, addMem, err) } } } @@ -498,7 +497,7 @@ func (server *Server) handleModifyInternal(state *State, r *message.ModifyReques for _, delMem := range delMembers { memberOf, err := server.getAttribute(delMem, ATTR_MEMBEROF) if err != nil { - server.logger.Printf("Could not remove %s from memberOf of %s: %s", dn, delMem, err) + server.logger.Warnf("Could not remove %s from memberOf of %s: %s", dn, delMem, err) continue } if memberOf == nil { @@ -513,7 +512,7 @@ func (server *Server) handleModifyInternal(state *State, r *message.ModifyReques err = server.addElements(delMem, Entry{ATTR_MEMBEROF: newMemberOf}) if err != nil { - server.logger.Printf("Could not remove %s from memberOf of %s: %s", dn, delMem, err) + server.logger.Warnf("Could not remove %s from memberOf of %s: %s", dn, delMem, err) } }