From 825aa770893850fb2626def316c4b0f060ef776f Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Sat, 15 Feb 2020 12:04:06 +0100 Subject: Hopefully, fix most case-sensitivity issues - DNs are always used in canonical form: lowercase, no spaces. This is how they are internally handled and stored in paths and fields such as member and memberof - Attribute names now can have any combination of lower/uppercase and stuff should work - When modifying an attribute with a name that hase a different lower/upper combination than the previously stored value, keep the previous attribute name - Trim spaces from values and do not store empty values --- TODO.md | 4 -- bottin.hcl.example | 2 +- main.go | 59 ++++++++++++++----- util.go | 31 +++++++--- write.go | 166 ++++++++++++++++++++++++++++++----------------------- 5 files changed, 162 insertions(+), 100 deletions(-) diff --git a/TODO.md b/TODO.md index d9fcafa..1dda9b2 100644 --- a/TODO.md +++ b/TODO.md @@ -1,6 +1,2 @@ -- Switch to `go mod` for building Bottin - - Implement missing search filters (in applyFilter) - Add an initial prefix to the consul key value - -- Potential bugs with different combinations of lower/uppercase names diff --git a/bottin.hcl.example b/bottin.hcl.example index 444a5b7..4547a7b 100644 --- a/bottin.hcl.example +++ b/bottin.hcl.example @@ -12,7 +12,7 @@ job "directory" { task "server" { driver = "docker" config { - image = "lxpz/bottin_amd64:13" + image = "lxpz/bottin_amd64:14" readonly_rootfs = true port_map { ldap_port = 389 diff --git a/main.go b/main.go index 9de5f3a..cec68ef 100644 --- a/main.go +++ b/main.go @@ -10,6 +10,7 @@ import ( "io/ioutil" "os" "os/signal" + "strings" "syscall" ldap "bottin/ldapserver" @@ -19,15 +20,18 @@ import ( log "github.com/sirupsen/logrus" ) -const ATTR_USERPASSWORD = "userpassword" -const ATTR_MEMBER = "member" +// System managed attributes (cannot be changed by user, see checkRestrictedAttr) const ATTR_MEMBEROF = "memberof" const ATTR_ENTRYUUID = "entryuuid" const ATTR_CREATORSNAME = "creatorsname" const ATTR_CREATETIMESTAMP = "createtimestamp" const ATTR_MODIFIERSNAME = "modifiersname" const ATTR_MODIFYTIMESTAMP = "modifytimestamp" + +// Attributes that we are interested in at various points const ATTR_OBJECTCLASS = "objectclass" +const ATTR_MEMBER = "member" +const ATTR_USERPASSWORD = "userpassword" type ConfigFile struct { Suffix string `json:"suffix"` @@ -260,20 +264,27 @@ func (server *Server) newUserState() ldap.UserState { } func (server *Server) init() error { - path, err := dnToConsul(server.config.Suffix) + // Check that suffix is in canonical format in config file + suffix_canonical, err := server.checkDN(server.config.Suffix, false) if err != nil { return err } + if suffix_canonical != server.config.Suffix { + return fmt.Errorf("Please write suffix in canonical format: %s", suffix_canonical) + } - pair, _, err := server.kv.Get(path+"/attribute="+ATTR_OBJECTCLASS, nil) + // Check that root object exists. + // If it does, we're done. Otherwise, we have some initialization to do. + exists, err := server.objectExists(server.config.Suffix) if err != nil { return err } - - if pair != nil { + if exists { return nil } + // We have to initialize the server. + // Create a root object and an admin object. base_attributes := Entry{ ATTR_OBJECTCLASS: []string{"top", "dcObject", "organization"}, "structuralobjectclass": []string{"organization"}, @@ -333,16 +344,27 @@ func (server *Server) addElements(dn string, attrs Entry) error { return err } - for k, v := range attrs { + for k, valuesNC := range attrs { path := prefix + "/attribute=" + k - if len(v) == 0 { - // If we have zero values, delete associated k/v pair + + // Trim spaces and remove empty values + values := []string{} + for _, v := range valuesNC { + vv := strings.TrimSpace(v) + if len(vv) > 0 { + values = append(values, vv) + } + } + + // If we have zero values, delete associated k/v pair + // Otherwise, write new values + if len(values) == 0 { _, err := server.kv.Delete(path, nil) if err != nil { return err } } else { - json, err := json.Marshal(v) + json, err := json.Marshal(values) if err != nil { return err } @@ -362,16 +384,23 @@ func (server *Server) getAttribute(dn string, attr string) ([]string, error) { return nil, err } - pair, _, err := server.kv.Get(path+"/attribute="+attr, nil) + pairs, _, err := server.kv.List(path+"/attribute=", nil) if err != nil { return nil, err } - if pair == nil { - return []string{}, nil + values := []string{} + for _, pair := range pairs { + if strings.EqualFold(pair.Key, path+"/attribute="+attr) { + newVals, err := parseValue(pair.Value) + if err != nil { + return nil, err + } + values = append(values, newVals...) + } } - return parseValue(pair.Value) + return values, nil } func (server *Server) objectExists(dn string) (bool, error) { @@ -388,7 +417,7 @@ func (server *Server) objectExists(dn string) (bool, error) { } func (server *Server) checkDN(dn string, allow_extend bool) (string, error) { - // 1. Canonicalize: remove spaces between things + // 1. Canonicalize: remove spaces between things and put all in lower case dn, err := canonicalDN(dn) if err != nil { return "", err diff --git a/util.go b/util.go index 96bb00b..95ad102 100644 --- a/util.go +++ b/util.go @@ -98,19 +98,14 @@ func parseDN(dn string) ([]DNComponent, error) { return nil, fmt.Errorf("Wrong DN component: %s (expected type=value)", rdn) } ret = append(ret, DNComponent{ - Type: strings.TrimSpace(splits[0]), - Value: strings.TrimSpace(splits[1]), + Type: strings.ToLower(strings.TrimSpace(splits[0])), + Value: strings.ToLower(strings.TrimSpace(splits[1])), }) } return ret, nil } -func canonicalDN(dn string) (string, error) { - path, err := parseDN(dn) - if err != nil { - return "", err - } - +func unparseDN(path []DNComponent) string { ret := "" for _, c := range path { if ret != "" { @@ -118,7 +113,16 @@ func canonicalDN(dn string) (string, error) { } ret = ret + c.Type + "=" + c.Value } - return ret, nil + return ret +} + +func canonicalDN(dn string) (string, error) { + path, err := parseDN(dn) + if err != nil { + return "", err + } + + return unparseDN(path), nil } func checkRestrictedAttr(attr string) error { @@ -162,3 +166,12 @@ func valueMatch(attr, val1, val2 string) bool { return strings.EqualFold(val1, val2) } } + +func listContains(list []string, key string) bool { + for _, v := range list { + if key == v { + return true + } + } + return false +} diff --git a/write.go b/write.go index 7a71465..b04cd2c 100644 --- a/write.go +++ b/write.go @@ -58,7 +58,16 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in return ldap.LDAPResultEntryAlreadyExists, nil } - // TODO: check that parent object exists + // Check that parent object exists + parentDn := unparseDN(dnSplit[1:]) + parentExists, err := server.objectExists(parentDn) + if err != nil { + return ldap.LDAPResultOperationsError, err + } + if !parentExists { + return ldap.LDAPResultNoSuchObject, fmt.Errorf( + "Parent object %s does not exist", parentDn) + } // If adding a group, track of who the members will be so that their memberOf field can be updated later members := []string{} @@ -77,8 +86,9 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in if err != nil { return ldap.LDAPResultObjectClassViolation, err } - // If they are writing a member key, we have to check they are adding valid members if strings.EqualFold(key, ATTR_MEMBER) { + // If they are writing a member list, we have to check they are adding valid members + // Also, rewrite member list to use canonical DN syntax (no spaces, all lowercase) for _, member := range vals_str { member_canonical, err := server.checkDN(member, false) if err != nil { @@ -93,19 +103,31 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in "Cannot add %s to members, it does not exist!", member_canonical) } + members = append(members, member_canonical) } - members = append(members, vals_str...) - } - if prev, ok := entry[key]; ok { - entry[key] = append(prev, vals_str...) + entry[key] = members } else { - entry[key] = vals_str + if prev, ok := entry[key]; ok { + entry[key] = append(prev, vals_str...) + } else { + entry[key] = vals_str + } } } - if _, ok := entry[ATTR_OBJECTCLASS]; !ok { + // Ensure object has at least one objectclass value + hasObjectClass := false + for k := range entry { + if strings.EqualFold(k, ATTR_OBJECTCLASS) { + hasObjectClass = true + break + } + } + if !hasObjectClass { entry[ATTR_OBJECTCLASS] = []string{"top"} } + + // Write system attributes entry[ATTR_CREATORSNAME] = []string{state.login.user} entry[ATTR_CREATETIMESTAMP] = []string{genTimestamp()} entry[ATTR_ENTRYUUID] = []string{genUuid()} @@ -306,11 +328,25 @@ func (server *Server) handleModifyInternal(state *State, r *message.ModifyReques addMembers, delMembers := []string{}, []string{} // Produce new entry values to be saved - newEntry := Entry{} + entry := Entry{} + for _, change := range r.Changes() { attr := string(change.Modification().Type_()) - values := change.Modification().Vals() + changeValues := []string{} + for _, v := range change.Modification().Vals() { + changeValues = append(changeValues, string(v)) + } + // If we already had an attribute with this name before, + // make sure we are using the same lowercase/uppercase + for prevAttr := range prevEntry { + if strings.EqualFold(attr, prevAttr) { + attr = prevAttr + break + } + } + + // Check that this attribute is not system-managed thus restricted err = checkRestrictedAttr(attr) if err != nil { return ldap.LDAPResultObjectClassViolation, err @@ -326,112 +362,100 @@ func (server *Server) handleModifyInternal(state *State, r *message.ModifyReques return ldap.LDAPResultInsufficientAccessRights, nil } - if change.Operation() == ldap.ModifyRequestChangeOperationAdd { - newEntry[attr] = prevEntry[attr] - for _, val := range values { - present := false - for _, prevVal := range newEntry[attr] { - if prevVal == string(val) { - present = true - break - } + // If we are changing ATTR_MEMBER, rewrite all values to canonical form + if strings.EqualFold(attr, ATTR_MEMBER) { + for i := range changeValues { + canonical_val, err := server.checkDN(changeValues[i], false) + if err != nil { + return ldap.LDAPResultInvalidDNSyntax, err } - if !present { - newEntry[attr] = append(newEntry[attr], string(val)) + changeValues[i] = canonical_val + } + } + + // If we don't yet have a new value for this attr, + // but one existed before, initialize entry[attr] to the old value + // so that later on what we do is simply modify entry[attr] in place + // (this allows to handle sequences of several changes on the same attr) + if _, ok := entry[attr]; !ok { + if _, ok := prevEntry[attr]; ok { + entry[attr] = prevEntry[attr] + } + } + + // Apply effective modification on entry[attr] + if change.Operation() == ldap.ModifyRequestChangeOperationAdd { + for _, val := range changeValues { + if !listContains(entry[attr], val) { + entry[attr] = append(entry[attr], val) if strings.EqualFold(attr, ATTR_MEMBER) { - addMembers = append(addMembers, string(val)) + addMembers = append(addMembers, val) } } } } else if change.Operation() == ldap.ModifyRequestChangeOperationDelete { - if len(values) == 0 { + if len(changeValues) == 0 { // Delete everything - newEntry[attr] = []string{} if strings.EqualFold(attr, ATTR_MEMBER) { - delMembers = append(delMembers, prevEntry[attr]...) + delMembers = append(delMembers, entry[attr]...) } + entry[attr] = []string{} } else { // Delete only those specified - newEntry[attr] = []string{} - for _, prevVal := range prevEntry[attr] { - keep := true - for _, delVal := range values { - if string(delVal) == prevVal { - keep = false - break - } - } - if keep { - newEntry[attr] = append(newEntry[attr], prevVal) + newList := []string{} + for _, prevVal := range entry[attr] { + if !listContains(changeValues, prevVal) { + newList = append(newList, prevVal) } else { if strings.EqualFold(attr, ATTR_MEMBER) { delMembers = append(delMembers, prevVal) } } } + entry[attr] = newList } } else if change.Operation() == ldap.ModifyRequestChangeOperationReplace { - newEntry[attr] = []string{} - for _, newVal := range values { - newEntry[attr] = append(newEntry[attr], string(newVal)) - } if strings.EqualFold(attr, ATTR_MEMBER) { - for _, newMem := range newEntry[attr] { - mustAdd := true - for _, prevMem := range prevEntry[attr] { - if prevMem == newMem { - mustAdd = false - break - } - } - if mustAdd { + for _, newMem := range changeValues { + if !listContains(entry[attr], newMem) { addMembers = append(addMembers, newMem) } } - for _, prevMem := range prevEntry[attr] { - mustDel := true - for _, newMem := range newEntry[attr] { - if newMem == prevMem { - mustDel = false - break - } - } - if mustDel { + for _, prevMem := range entry[attr] { + if !listContains(changeValues, prevMem) { delMembers = append(delMembers, prevMem) } } } + entry[attr] = changeValues } } // Check that added members actually exist for i := range addMembers { - addMem, err := server.checkDN(addMembers[i], false) - if err != nil { - return ldap.LDAPResultInvalidDNSyntax, err - } - exists, err := server.objectExists(addMem) + exists, err := server.objectExists(addMembers[i]) if err != nil { return ldap.LDAPResultOperationsError, err } if !exists { return ldap.LDAPResultNoSuchObject, fmt.Errorf( - "Cannot add member %s, it does not exist", addMem) + "Cannot add member %s, it does not exist", addMembers[i]) } - addMembers[i] = addMem } - if v, ok := newEntry[ATTR_OBJECTCLASS]; ok && len(v) == 0 { - return ldap.LDAPResultInsufficientAccessRights, fmt.Errorf( - "Cannot remove all objectclass values") + for k, v := range entry { + if strings.EqualFold(k, ATTR_OBJECTCLASS) && len(v) == 0 { + return ldap.LDAPResultInsufficientAccessRights, fmt.Errorf( + "Cannot remove all objectclass values") + } } // Now, the modification has been processed and accepted and we want to commit it - newEntry[ATTR_MODIFIERSNAME] = []string{state.login.user} - newEntry[ATTR_MODIFYTIMESTAMP] = []string{genTimestamp()} + entry[ATTR_MODIFIERSNAME] = []string{state.login.user} + entry[ATTR_MODIFYTIMESTAMP] = []string{genTimestamp()} // Save the edited values - err = server.addElements(dn, newEntry) + err = server.addElements(dn, entry) if err != nil { return ldap.LDAPResultOperationsError, err } -- cgit v1.2.3