filter: handle multi-component substring filters

filter.go

@@ -158,13 +158,28 @@ func DecompileFilter(packet *ber.Packet) (ret string, err error) {
 	case FilterSubstrings:
 		ret += ber.DecodeString(packet.Children[0].Data.Bytes())
 		ret += "="
-		switch packet.Children[1].Children[0].Tag {
-		case FilterSubstringsInitial:
-			ret += ber.DecodeString(packet.Children[1].Children[0].Data.Bytes()) + "*"
-		case FilterSubstringsAny:
-			ret += "*" + ber.DecodeString(packet.Children[1].Children[0].Data.Bytes()) + "*"
-		case FilterSubstringsFinal:
-			ret += "*" + ber.DecodeString(packet.Children[1].Children[0].Data.Bytes())
+		// RFC 4511 4.5.1.7: a SubstringFilter holds at most one `initial`
+		// (first), any number of `any`, and at most one `final` (last).
+		// Reading only Children[1].Children[0] discarded every component after
+		// the first, so "(cn=svc-*-prod)" decompiled to "(cn=svc-*)" -- a
+		// strictly BROADER filter. A server that decompiles an incoming request
+		// to route it then answers a different question than the client asked.
+		for i, child := range packet.Children[1].Children {
+			value := ber.DecodeString(child.Data.Bytes())
+			switch child.Tag {
+			case FilterSubstringsInitial:
+				ret += value + "*"
+			case FilterSubstringsAny:
+				if i == 0 {
+					ret += "*"
+				}
+				ret += value + "*"
+			case FilterSubstringsFinal:
+				if i == 0 {
+					ret += "*"
+				}
+				ret += value
+			}
 		}
 	case FilterEqualityMatch:
 		ret += ber.DecodeString(packet.Children[0].Data.Bytes())
@@ -332,26 +347,38 @@ func compileFilter(filter string, pos int) (*ber.Packet, int, error) {
 		}
 		packet.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, attribute, "Attribute"))
 		switch {
-		case packet.Tag == FilterEqualityMatch && condition[0] == '*' && condition[len(condition)-1] == '*':
-			// Any
-			packet.Tag = FilterSubstrings
-			packet.Description = FilterMap[packet.Tag]
-			seq := ber.Encode(ber.ClassUniversal, ber.TypeConstructed, ber.TagSequence, nil, "Substrings")
-			seq.AppendChild(ber.NewString(ber.ClassContext, ber.TypePrimitive, FilterSubstringsAny, condition[1:len(condition)-1], "Any Substring"))
-			packet.AppendChild(seq)
-		case packet.Tag == FilterEqualityMatch && condition[0] == '*':
-			// Final
-			packet.Tag = FilterSubstrings
-			packet.Description = FilterMap[packet.Tag]
-			seq := ber.Encode(ber.ClassUniversal, ber.TypeConstructed, ber.TagSequence, nil, "Substrings")
-			seq.AppendChild(ber.NewString(ber.ClassContext, ber.TypePrimitive, FilterSubstringsFinal, condition[1:], "Final Substring"))
-			packet.AppendChild(seq)
-		case packet.Tag == FilterEqualityMatch && condition[len(condition)-1] == '*':
-			// Initial
+		case packet.Tag == FilterEqualityMatch && strings.Contains(condition, "*"):
+			// RFC 4511 4.5.1.7. The three cases this replaced -- "*x*", "*x"
+			// and "x*" -- covered only single-component patterns. Anything with
+			// an interior '*' ("a*b*c", "svc-*-prod") matched none of them and
+			// fell through to `default`, where it was encoded as an equality
+			// match whose value contained a literal '*'. No entry has a literal
+			// '*' in its value, so those filters silently matched nothing.
+			// Splitting on '*' encodes each component as initial / any / final.
 			packet.Tag = FilterSubstrings
 			packet.Description = FilterMap[packet.Tag]
 			seq := ber.Encode(ber.ClassUniversal, ber.TypeConstructed, ber.TagSequence, nil, "Substrings")
-			seq.AppendChild(ber.NewString(ber.ClassContext, ber.TypePrimitive, FilterSubstringsInitial, condition[:len(condition)-1], "Initial Substring"))
+			parts := strings.Split(condition, "*")
+			for i, part := range parts {
+				if part == "" {
+					continue
+				}
+				switch {
+				case i == 0:
+					seq.AppendChild(ber.NewString(ber.ClassContext, ber.TypePrimitive, FilterSubstringsInitial, part, "Initial Substring"))
+				case i == len(parts)-1:
+					seq.AppendChild(ber.NewString(ber.ClassContext, ber.TypePrimitive, FilterSubstringsFinal, part, "Final Substring"))
+				default:
+					seq.AppendChild(ber.NewString(ber.ClassContext, ber.TypePrimitive, FilterSubstringsAny, part, "Any Substring"))
+				}
+			}
+			if len(seq.Children) == 0 {
+				// A condition of only asterisks, e.g. "**". The previous form
+				// encoded a single empty `any`, which matches every value;
+				// preserved here rather than emitting an empty substrings
+				// sequence, which RFC 4511 forbids.
+				seq.AppendChild(ber.NewString(ber.ClassContext, ber.TypePrimitive, FilterSubstringsAny, "", "Any Substring"))
+			}
 			packet.AppendChild(seq)
 		default:
 			packet.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, condition, "Condition"))
@@ -431,25 +458,15 @@ func ServerApplyFilter(f *ber.Packet, entry *Entry) (bool, LDAPResultCode) {
 			return false, LDAPResultOperationsError
 		}
 		attribute := f.Children[0].Value.(string)
-		valueBytes := f.Children[1].Children[0].Data.Bytes()
-		valueLower := strings.ToLower(string(valueBytes[:]))
+		// Testing only Children[1].Children[0] meant a multi-component
+		// assertion was satisfied by its FIRST component alone, so
+		// "(cn=svc-*-prod)" matched "svc-door-dev". Every component is matched
+		// below, in order and without overlap.
 		for _, a := range entry.Attributes {
 			if strings.EqualFold(a.Name, attribute) {
 				for _, v := range a.Values {
-					vLower := strings.ToLower(v)
-					switch f.Children[1].Children[0].Tag {
-					case FilterSubstringsInitial:
-						if strings.HasPrefix(vLower, valueLower) {
-							return true, LDAPResultSuccess
-						}
-					case FilterSubstringsAny:
-						if strings.Contains(vLower, valueLower) {
-							return true, LDAPResultSuccess
-						}
-					case FilterSubstringsFinal:
-						if strings.HasSuffix(vLower, valueLower) {
-							return true, LDAPResultSuccess
-						}
+					if substringAssertionMatches(f.Children[1].Children, v) {
+						return true, LDAPResultSuccess
 					}
 				}
 			}
@@ -522,3 +539,33 @@ func parseFilterObjectClass(f *ber.Packet) (string, error) {
 	}
 	return strings.ToLower(objectClass), nil
 }
+
+// substringAssertionMatches reports whether value satisfies every component of
+// an RFC 4511 SubstringFilter. Components match in order and may not overlap:
+// `initial` anchors the head, each `any` must appear after the previously
+// consumed text, and `final` anchors the tail.
+func substringAssertionMatches(components []*ber.Packet, value string) bool {
+	rest := strings.ToLower(value)
+	for _, component := range components {
+		part := strings.ToLower(ber.DecodeString(component.Data.Bytes()))
+		switch component.Tag {
+		case FilterSubstringsInitial:
+			if !strings.HasPrefix(rest, part) {
+				return false
+			}
+			rest = rest[len(part):]
+		case FilterSubstringsAny:
+			idx := strings.Index(rest, part)
+			if idx < 0 {
+				return false
+			}
+			rest = rest[idx+len(part):]
+		case FilterSubstringsFinal:
+			if !strings.HasSuffix(rest, part) {
+				return false
+			}
+			rest = ""
+		}
+	}
+	return true
+}

filter_test.go

@@ -20,6 +20,9 @@ var testFilters = []compileTest{
 	{filterStr: "(sn=Möll*)", filterType: FilterSubstrings},
 	{filterStr: "(sn=*Møll)", filterType: FilterSubstrings},
 	{filterStr: "(sn=*Müll*)", filterType: FilterSubstrings},
+	{filterStr: "(sn=Mö*ller)", filterType: FilterSubstrings},
+	{filterStr: "(sn=M*ö*ller)", filterType: FilterSubstrings},
+	{filterStr: "(sn=*ö*ll*)", filterType: FilterSubstrings},
 	{filterStr: "(sn>=Möller)", filterType: FilterGreaterOrEqual},
 	{filterStr: "(sn<=Møller)", filterType: FilterLessOrEqual},
 	{filterStr: "(sn=*)", filterType: FilterPresent},
@@ -135,3 +138,87 @@ func TestGetFilterObjectClass(t *testing.T) {
 		t.Errorf("GetFilterObjectClass failed")
 	}
 }
+
+// TestSubstringFilterWireFormat decompiles SubstringFilter packets built the way
+// a client puts them on the wire: one child per component. CompileFilter cannot
+// produce this shape for a multi-component pattern, so a compile/decompile round
+// trip does not exercise it.
+func TestSubstringFilterWireFormat(t *testing.T) {
+	type part struct {
+		tag   ber.Tag
+		value string
+	}
+	build := func(attribute string, parts []part) *ber.Packet {
+		packet := ber.Encode(ber.ClassContext, ber.TypeConstructed, FilterSubstrings, nil, FilterMap[FilterSubstrings])
+		packet.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, attribute, "Attribute"))
+		seq := ber.Encode(ber.ClassUniversal, ber.TypeConstructed, ber.TagSequence, nil, "Substrings")
+		for _, p := range parts {
+			seq.AppendChild(ber.NewString(ber.ClassContext, ber.TypePrimitive, p.tag, p.value, "Substring"))
+		}
+		packet.AppendChild(seq)
+		return packet
+	}
+
+	tests := []struct {
+		name     string
+		attr     string
+		parts    []part
+		expected string
+	}{
+		{"initial", "cn", []part{{FilterSubstringsInitial, "svc-"}}, "(cn=svc-*)"},
+		{"any", "cn", []part{{FilterSubstringsAny, "door"}}, "(cn=*door*)"},
+		{"final", "mail", []part{{FilterSubstringsFinal, "@example.com"}}, "(mail=*@example.com)"},
+		{"initial and final", "cn", []part{{FilterSubstringsInitial, "svc-"}, {FilterSubstringsFinal, "-prod"}}, "(cn=svc-*-prod)"},
+		{"initial any final", "cn", []part{{FilterSubstringsInitial, "a"}, {FilterSubstringsAny, "b"}, {FilterSubstringsFinal, "c"}}, "(cn=a*b*c)"},
+		{"two any", "cn", []part{{FilterSubstringsAny, "door"}, {FilterSubstringsAny, "prod"}}, "(cn=*door*prod*)"},
+	}
+	for _, tt := range tests {
+		o, err := DecompileFilter(build(tt.attr, tt.parts))
+		if err != nil {
+			t.Errorf("%s: %s", tt.name, err.Error())
+		} else if o != tt.expected {
+			t.Errorf("%s: %q expected, got %q", tt.name, tt.expected, o)
+		}
+	}
+}
+
+// TestServerApplyFilterSubstrings checks that every component of a substring
+// assertion is matched, in order and without overlap.
+func TestServerApplyFilterSubstrings(t *testing.T) {
+	entry := func(cn string) *Entry {
+		return &Entry{
+			DN:         "cn=" + cn + ",ou=users,dc=example,dc=com",
+			Attributes: []*EntryAttribute{{Name: "cn", Values: []string{cn}}},
+		}
+	}
+	tests := []struct {
+		filterStr string
+		cn        string
+		expected  bool
+	}{
+		{"(cn=svc-*-prod)", "svc-door-prod", true},
+		{"(cn=svc-*-prod)", "svc-door-dev", false},
+		{"(cn=a*b*c)", "axxbyyc", true},
+		{"(cn=a*b*c)", "acb", false},
+		{"(cn=svc-*)", "svc-door-dev", true},
+		{"(cn=*door*)", "svc-door-dev", true},
+		{"(cn=*prod)", "svc-door-prod", true},
+		{"(cn=*prod)", "svc-door-dev", false},
+		// initial and final may not consume the same characters
+		{"(cn=prod*prod)", "prod", false},
+		{"(cn=prod*prod)", "prod-prod", true},
+	}
+	for _, tt := range tests {
+		filter, err := CompileFilter(tt.filterStr)
+		if err != nil {
+			t.Errorf("Problem compiling %s - %s", tt.filterStr, err.Error())
+			continue
+		}
+		keep, code := ServerApplyFilter(filter, entry(tt.cn))
+		if code != LDAPResultSuccess {
+			t.Errorf("%s against %q: unexpected result code %d", tt.filterStr, tt.cn, code)
+		} else if keep != tt.expected {
+			t.Errorf("%s against %q: expected %v, got %v", tt.filterStr, tt.cn, tt.expected, keep)
+		}
+	}
+}