From 7c99ae60e5a35d6d919daa55d75a6636b523d96d Mon Sep 17 00:00:00 2001 From: Michael Aldridge Date: Sun, 1 Nov 2026 19:44:11 -0600 Subject: [PATCH] internal/ldap: Support AND, OR and substring search filters --- internal/ldap/bleve_mapper.go | 195 ++++++++++++++++++++++------- internal/ldap/bleve_mapper_test.go | 87 ++++++++++++- internal/ldap/testutil_test.go | 55 +++++++- 3 files changed, 281 insertions(+), 56 deletions(-) diff --git a/internal/ldap/bleve_mapper.go b/internal/ldap/bleve_mapper.go index a72d0fb..881d45d 100644 --- a/internal/ldap/bleve_mapper.go +++ b/internal/ldap/bleve_mapper.go @@ -3,40 +3,154 @@ package ldap import ( "errors" "fmt" + "regexp" "strings" "github.com/ps78674/goldap/message" ) +// bleveWildcardValueRe matches the subset of characters that may appear +// in an unquoted Bleve wildcard term. NetAuth entity and group names +// consist of word characters, so anything outside this set can only +// come from a client trying to smuggle query syntax in. +var bleveWildcardValueRe = regexp.MustCompile(`^[\w.\-*]+$`) + +// buildBleveQuery translates a decoded LDAP search filter into a Bleve +// query string for NetAuth. Only attribute equality, presence and the +// simple substring forms are understood; everything else is refused so +// that a client gets an error rather than wrong results. func (s *server) buildBleveQuery(f message.Filter) (string, error) { - s.l.Trace("Building search expression", "type", fmt.Sprintf("%T", f), "filter", fmt.Sprintf("%#v", f)) - var err error - var etmp string - var expr []string - switch f := f.(type) { - case message.FilterEqualityMatch: - etmp, err = mapToBleveStringQuery(string(f.AttributeDesc()), "=", string(f.AssertionValue())) - expr = append(expr, etmp) - case message.FilterOr: - for _, subf := range f { - s, err := s.buildBleveQuery(subf) - if err != nil { - return "", err - } - expr = append(expr, s) - } - case message.FilterPresent: - etmp, err = mapToBleveStringQuery(string(f), "=", "*") - expr = append(expr, etmp) - default: - s.l.Warn("Unsupported search filter", "filter", fmt.Sprintf("%#v", f)) - err = errors.New("unsupported search filter") - } - return strings.Join(expr, " "), err + return s.bleveQuery(f, false) } -func mapToBleveStringQuery(attr, op, val string) (string, error) { - var predicate, operator string +// bleveQuery builds the query for a filter. The nested flag records +// whether this filter sits inside a boolean group, which determines +// whether a negation is meaningful. +func (s *server) bleveQuery(f message.Filter, nested bool) (string, error) { + s.l.Trace("Building search expression", "type", fmt.Sprintf("%T", f), "filter", fmt.Sprintf("%#v", f)) + + switch f := f.(type) { + case message.FilterEqualityMatch: + return mapToBleveStringQuery(string(f.AttributeDesc()), string(f.AssertionValue())) + case message.FilterPresent: + return mapToBleveStringQuery(string(f), "*") + case message.FilterSubstrings: + return s.bleveSubstringQuery(f) + case message.FilterNot: + // Bleve prohibits a clause with "-", but an expression made + // up entirely of prohibitions matches nothing at all. A + // top-level negation therefore has to be refused rather than + // quietly returning an empty result set. + if !nested { + s.l.Warn("Ignoring a top-level negated filter") + return "", errors.New("cannot negate the entire search") + } + e, err := s.bleveQuery(f.Filter, true) + if err != nil { + return "", err + } + if strings.HasPrefix(e, "-") { + // Double negation would leave Bleve with only + // prohibitions again. + return "", errors.New("cannot negate a negation") + } + if strings.HasPrefix(e, "(") { + return "-" + e, nil + } + return "-(" + e + ")", nil + case message.FilterAnd: + return s.bleveBooleanQuery(f, true) + case message.FilterOr: + return s.bleveBooleanQuery(f, false) + default: + s.l.Warn("Unsupported search filter", "filter", fmt.Sprintf("%#v", f)) + return "", errors.New("unsupported search filter") + } +} + +// bleveBooleanQuery maps an AND or OR filter onto a Bleve boolean +// expression. AND clauses are prefixed with + to make them required, +// while space separated clauses are already optional (OR) in Bleve. +// A group with more than one branch is parenthesized so it can be +// nested without changing its meaning. +func (s *server) bleveBooleanQuery(filters []message.Filter, and bool) (string, error) { + var expr []string + for _, subf := range filters { + e, err := s.bleveQuery(subf, true) + if err != nil { + return "", err + } + if and && !strings.HasPrefix(e, "-") { + // A prohibited clause must not also be marked required. + e = "+" + e + } + expr = append(expr, e) + } + if len(expr) == 0 { + return "", errors.New("empty boolean filter") + } + if !hasPositiveClause(expr) { + // A group made up only of prohibitions leaves Bleve with no + // clause to match on, which would silently return nothing. + return "", errors.New("filter contains no positive clause") + } + if len(expr) == 1 { + return expr[0], nil + } + return "(" + strings.Join(expr, " ") + ")", nil +} + +// hasPositiveClause reports whether any of the compiled subexpressions +// can match on its own. +func hasPositiveClause(expr []string) bool { + for _, e := range expr { + if !strings.HasPrefix(e, "-") { + return true + } + } + return false +} + +// bleveSubstringQuery translates the substring forms clients actually +// send, such as "(uid=js*)" or "(uid=*smith)", into Bleve wildcard +// terms. Mixed inner wildcards like "(uid=a*b*)" are refused since a +// single Bleve wildcard cannot express them faithfully. +func (s *server) bleveSubstringQuery(f message.FilterSubstrings) (string, error) { + var initial, final string + var any []string + for _, sub := range f.Substrings() { + switch v := sub.(type) { + case message.SubstringInitial: + initial = string(v) + case message.SubstringFinal: + final = string(v) + case message.SubstringAny: + any = append(any, string(v)) + } + } + + switch { + case len(any) == 0: + return mapToBleveStringQuery(string(f.Type_()), initial+"*"+final) + case len(any) == 1 && initial == "" && final == "": + // A single inner substring like "(uid=*smith)" is matched + // with wildcards on both ends. + return mapToBleveStringQuery(string(f.Type_()), "*"+any[0]+"*") + default: + s.l.Warn("Unsupported substring filter", "initial", initial, "final", final, "any", any) + return "", errors.New("unsupported substring filter") + } +} + +// mapToBleveStringQuery builds a single Bleve term for the supported +// attributes. A bare "*" keeps its wildcard meaning for +// attribute-present filters; plain values are quoted so they are +// matched as one term and cannot introduce extra query clauses. Values +// containing a wildcard must stay unquoted for the wildcard to work, +// so those are restricted to the character set NetAuth names may +// contain. +func mapToBleveStringQuery(attr, val string) (string, error) { + var predicate string switch attr { case "uid": @@ -47,27 +161,16 @@ func mapToBleveStringQuery(attr, op, val string) (string, error) { return "", errors.New("search attribute is unsupported") } - switch op { - case "=": - operator = ":" - default: - return "", errors.New("search comparison is unsupported") - } - - return predicate + operator + quoteBleveValue(val), nil -} - -// quoteBleveValue makes a value safe to embed in a Bleve query string. -// -// Assertion values come straight from the network and Bleve parses its -// queries, so an unquoted value such as "x Name:y" would add terms the -// client never asked for. Quoting keeps the value as a single term, and -// the wildcard used for attribute-present filters stays outside the -// quotes so that it keeps its meaning. -func quoteBleveValue(val string) string { if val == "*" { - return val + return predicate + ":*", nil } + if strings.Contains(val, "*") { + if !bleveWildcardValueRe.MatchString(val) { + return "", errors.New("search value contains unsupported characters") + } + return predicate + ":" + val, nil + } + escaped := strings.NewReplacer(`\`, `\\`, `"`, `\"`).Replace(val) - return `"` + escaped + `"` + return predicate + `:"` + escaped + `"`, nil } diff --git a/internal/ldap/bleve_mapper_test.go b/internal/ldap/bleve_mapper_test.go index ef92ab5..ddc7288 100644 --- a/internal/ldap/bleve_mapper_test.go +++ b/internal/ldap/bleve_mapper_test.go @@ -35,7 +35,51 @@ func TestBuildBleveQuery(t *testing.T) { berRawEquality("uid", "jsmith"), berRawEquality("uid", "jdoe"), ), - want: `ID:"jsmith" ID:"jdoe"`, + want: `(ID:"jsmith" ID:"jdoe")`, + }, + { + name: "and of two matches", + filter: berAndFilter(t, + berRawEquality("uid", "jsmith"), + berRawPresent("uid"), + ), + want: `(+ID:"jsmith" +ID:*)`, + }, + { + name: "prefix substring", + filter: berSubstringFilter(t, "uid", "js", nil, ""), + want: "ID:js*", + }, + { + name: "suffix substring", + filter: berSubstringFilter(t, "uid", "", nil, "smith"), + want: "ID:*smith", + }, + { + name: "contains substring", + filter: berSubstringFilter(t, "uid", "", []string{"mitt"}, ""), + want: "ID:*mitt*", + }, + { + name: "prefix and suffix", + filter: berSubstringFilter(t, "uid", "jo", nil, "th"), + want: "ID:jo*th", + }, + { + name: "and with a nested negation", + filter: berAndFilter(t, + berRawPresent("uid"), + berElement(0xa2, berRawEquality("uid", "guest")), + ), + want: `(+ID:* -(ID:"guest"))`, + }, + { + name: "nested boolean groups", + filter: berAndFilter(t, + berRawEquality("uid", "jsmith"), + berRawOr(t, berRawEquality("uid", "jdoe"), berRawPresent("uid")), + ), + want: `(+ID:"jsmith" +(ID:"jdoe" ID:*))`, }, } for _, tc := range cases { @@ -61,7 +105,7 @@ func TestBuildBleveQueryEscapesValues(t *testing.T) { want string }{ {"x Name:y", `ID:"x Name:y"`}, - {`x" OR ID:"*`, `ID:"x\" OR ID:\"*"`}, + {`x" OR ID:"y`, `ID:"x\" OR ID:\"y"`}, {`back\slash`, `ID:"back\\slash"`}, {"x AND Name:admin", `ID:"x AND Name:admin"`}, } @@ -74,12 +118,47 @@ func TestBuildBleveQueryEscapesValues(t *testing.T) { t.Errorf("value %q: got %q, want %q", tc.value, got, tc.want) } } + + // Wildcards must stay unquoted to keep their meaning, so values + // that mix a wildcard with query syntax are refused outright + // rather than passed through. + for _, value := range []string{`x" OR ID:*`, `x* OR Name:*`, "*\x00"} { + if got, err := s.buildBleveQuery(berEqualityFilter(t, "uid", value)); err == nil { + t.Errorf("value %q was accepted as %q", value, got) + } + } } func TestBuildBleveQueryUnsupported(t *testing.T) { s := &server{l: testLogger()} - if _, err := s.buildBleveQuery(berPresentFilter(t, "mail")); err == nil { - t.Error("an unsupported attribute should not produce a query") + cases := []struct { + name string + filter message.Filter + }{ + {"unsupported attribute", berPresentFilter(t, "mail")}, + {"unsupported attribute in equality", berEqualityFilter(t, "mail", "x@y.z")}, + { + "top-level negation has nothing to exclude from", + berNotFilter(t, berRawEquality("uid", "jsmith")), + }, + { + "and of only negations", + berAndFilter(t, + berElement(0xa2, berRawEquality("uid", "jsmith")), + berElement(0xa2, berRawEquality("cn", "admins")), + ), + }, + { + "substring with inner wildcards", + berSubstringFilter(t, "uid", "a", []string{"b", "c"}, "d"), + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got, err := s.buildBleveQuery(tc.filter); err == nil { + t.Errorf("filter was accepted as %q", got) + } + }) } } diff --git a/internal/ldap/testutil_test.go b/internal/ldap/testutil_test.go index 5054f1d..8318c00 100644 --- a/internal/ldap/testutil_test.go +++ b/internal/ldap/testutil_test.go @@ -47,12 +47,12 @@ func berSearchRequestWithAttrs(baseDN string, attrs ...string) []byte { selection = append(selection, berOctetString(a)) } search := berElement(0x63, // [APPLICATION 3] constructed - berOctetString(baseDN), // baseObject - berElement(0x0a, []byte{2}), // scope: wholeSubtree - berElement(0x0a, []byte{0}), // derefAliases: never - berInteger(0), // sizeLimit - berInteger(0), // timeLimit - berElement(0x01, []byte{0x00}), // typesOnly: false + berOctetString(baseDN), // baseObject + berElement(0x0a, []byte{2}), // scope: wholeSubtree + berElement(0x0a, []byte{0}), // derefAliases: never + berInteger(0), // sizeLimit + berInteger(0), // timeLimit + berElement(0x01, []byte{0x00}), // typesOnly: false berElement(0x87, []byte("uid")), // filter: (uid=*) berElement(0x30, selection...), // attributes ) @@ -86,12 +86,55 @@ func berOrFilter(t *testing.T, filters ...[]byte) message.Filter { return berDecodeFilter(t, berElement(0xa1, filters...)) } +// berAndFilter encodes an AND [0] of already-encoded filter elements. +func berAndFilter(t *testing.T, filters ...[]byte) message.Filter { + t.Helper() + return berDecodeFilter(t, berElement(0xa0, filters...)) +} + +// berNotFilter encodes a NOT [2] wrapping an already-encoded filter. +func berNotFilter(t *testing.T, filter []byte) message.Filter { + t.Helper() + return berDecodeFilter(t, berElement(0xa2, filter)) +} + // berRawEquality returns the encoded bytes of an equality match so it // can be composed into a nested filter. func berRawEquality(attr, value string) []byte { return berElement(0xa3, berOctetString(attr), berOctetString(value)) } +// berRawPresent returns the encoded bytes of a present filter. +func berRawPresent(attr string) []byte { + return berElement(0x87, []byte(attr)) +} + +// berRawOr returns the encoded bytes of an OR of already-encoded +// filter elements. +func berRawOr(t *testing.T, filters ...[]byte) []byte { + t.Helper() + return berElement(0xa1, filters...) +} + +// berSubstringFilter encodes a substrings [4] filter with the given +// initial, any and final components. +func berSubstringFilter(t *testing.T, attr, initial string, any []string, final string) message.Filter { + t.Helper() + var parts [][]byte + if initial != "" { + parts = append(parts, berElement(0x80, []byte(initial))) + } + for _, a := range any { + parts = append(parts, berElement(0x81, []byte(a))) + } + if final != "" { + parts = append(parts, berElement(0x82, []byte(final))) + } + inner := berElement(0x30, parts...) + ava := berElement(0xa4, berOctetString(attr), inner) + return berDecodeFilter(t, ava) +} + // berDecodeFilter wraps a raw filter element in a minimal search // request, decodes it and returns the parsed filter. func berDecodeFilter(t *testing.T, filterElement []byte) message.Filter {