Skip to content

filter: handle multi-component substring filters - #7

Open
navibodybuilder wants to merge 1 commit into
glauth:masterfrom
navibodybuilder:substring-filter-components
Open

filter: handle multi-component substring filters#7
navibodybuilder wants to merge 1 commit into
glauth:masterfrom
navibodybuilder:substring-filter-components

Conversation

@navibodybuilder

Copy link
Copy Markdown

RFC 4511 §4.5.1.7 allows a SubstringFilter to carry at most one initial
(first), any number of any, and at most one final (last). Three functions in
filter.go assume there is exactly one component, and they interact, so this
fixes them together.

What breaks today

Function Behaviour Effect
DecompileFilter reads only Children[1].Children[0] a wire filter (cn=svc-*-prod) decompiles to (cn=svc-*) — a strictly broader filter
CompileFilter special-cases only x*, *x, *x* a pattern with an interior * (a*b*c) is encoded as an equality match holding a literal *, which matches no entry
ServerApplyFilter tests only Children[1].Children[0] a multi-component assertion is satisfied by its first component alone: (cn=svc-*-prod) matches svc-door-dev

The practical impact depends on which one you hit first. A server that decompiles
an incoming request (to log it, route it, or hand it to a backend as a string)
silently answers a different, broader question than the client asked. A caller
that compiles a multi-component pattern gets no results at all.

Why they must be fixed together

The decompile defect masks the compile defect: a multi-component pattern is
already truncated to a single component before CompileFilter sees it. Fixing
either one alone converts a superset result into an empty one — which, for a
directory sync, is the more damaging failure, because an empty read is what makes
a sync deprovision every entry it did not receive.

A note for reviewers

A compile → decompile round trip does not reproduce any of this, because
CompileFilter did not produce a wire-shaped substring packet either. That is
why the round-trip table in filter_test.go never caught it.
TestSubstringFilterWireFormat builds the packets the way a client actually puts
them on the wire — one child per component — which is what ldapsearch, Active
Directory and JNDI all send.

Tests

  • Three multi-component patterns added to the existing testFilters round-trip
    table.
  • TestSubstringFilterWireFormat — decompiles wire-shaped packets.
  • TestServerApplyFilterSubstrings — matching, including that initial and
    final may not consume the same characters ((cn=prod*prod) matches
    prod-prod but not prod).

All existing tests continue to pass. Against the current code the new tests
produce 8 failures:

initial and final: "(cn=svc-*-prod)" expected, got "(cn=svc-*)"
initial any final: "(cn=a*b*c)" expected, got "(cn=a*)"
two any: "(cn=*door*prod*)" expected, got "(cn=*door*)"
(cn=svc-*-prod) against "svc-door-prod": expected true, got false
(cn=a*b*c) against "axxbyyc": expected true, got false
(cn=prod*prod) against "prod-prod": expected true, got false

(TestConnect, TestSearch, TestSearchWithPaging and
TestMultiGoroutineSearch dial the live public server ldap.itd.umich.edu and
fail wherever outbound 389 is closed. They are unrelated to this change and fail
the same way without it.)

Backwards compatibility

Single-component filters — (cn=x*), (cn=*x), (cn=*x*), (cn=*) — are
unchanged in both encoding and matching. A condition of only asterisks (**)
still encodes a single empty any, preserving the previous match-everything
behaviour rather than emitting an empty substrings sequence, which RFC 4511
forbids.

RFC 4511 4.5.1.7 allows a SubstringFilter to carry at most one `initial`
(first), any number of `any`, and at most one `final` (last). Three
functions assumed there is exactly one component:

  DecompileFilter    emitted only Children[1].Children[0] and discarded
                     the rest, so a wire filter "(cn=svc-*-prod)"
                     decompiled to "(cn=svc-*)" -- a strictly BROADER
                     filter.

  CompileFilter      special-cased only "x*", "*x" and "*x*". A pattern
                     with an interior '*' ("a*b*c") 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.

  ServerApplyFilter  tested only Children[1].Children[0], so a
                     multi-component assertion was satisfied by its first
                     component alone: "(cn=svc-*-prod)" matched
                     "svc-door-dev".

The three interact, so they are fixed together. The decompile defect masks
the compile defect: a multi-component pattern is already truncated to one
component before CompileFilter sees it. Correcting either one alone turns a
superset result into an EMPTY one, which for a directory sync is the more
damaging failure -- an empty read is what makes a sync deprovision every
entry it did not receive.

Note that a compile/decompile round trip does not reproduce any of this,
because CompileFilter did not produce a wire-shaped substring packet
either. TestSubstringFilterWireFormat builds the packets the way a client
puts them on the wire, one child per component.

Tests: three multi-component patterns added to the existing round-trip
table, plus TestSubstringFilterWireFormat and
TestServerApplyFilterSubstrings. All three fail against the current code
(8 failures) and pass with this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant