Skip to content

The AD hop follows no LDAP referral, so the bind password cannot leave the anchored LDAPS hop (BACKLOG #2530) - #1877

Merged
wshallwshall merged 16 commits into
mainfrom
builder/ldap3-referral-credentials
Oct 1, 2026
Merged

wshallwshall merged 16 commits into
mainfrom
builder/ldap3-referral-credentials

Conversation

@wshallwshall

Copy link
Copy Markdown
Collaborator

Vault BACKLOG #2530 (P1). Built on PR 1869, which merged as 2679a8d. Head 453990c.

ldap3 2.9.1 followed referrals by default (core/connection.py:194, core/server.py:158-159), and its referral connection (strategy/base.py:751-797) used a plain Tls, with no pinned CA and no narrowing, re-binding with the SERVICE-ACCOUNT credentials on a referred search. The engine now sets auto_referrals=False on every Connection and allowed_referral_hosts=[] on the Server, and refuses a resultCode-10 answer with a specific LdapError that names at most three hosts.
Effects: sign-in is audited as auth.login_error; step-up reports directory_unavailable, which does not count toward lockout; the reconciler reports UNAVAILABLE and never revokes.

Tests: tests/test_ldap_referrals.py runs a real ldap3 stack against two loopback servers. The referred server receives zero connections, each guard alone blocks the follow, and the positive control (both guards reverted) delivers the bind password. Mutation arms by hand: reverting the guards turned 5 tests red; removing the refusal turned 4 red.
Checks: ruff and mypy clean. The full top-level tests suite ran in 7 chunks, all green, on eb78899. After the final commit, the auth/LDAP/TLS slice gave 1016 passed. Test subdirectories and the webconsole tests were not run; CI is the full read.
Docs: ADR 0180 Amendment F, docs/PHI.md section 4, changelog.d/2530.security.md.

Open questions, carried from the Builder:
(1) A search base in another domain makes the reconciler permanently UNAVAILABLE, so offboarded sessions are never revoked, and nothing alerts. Should it get its own alert or a startup probe?
(2) Continuation references (searchResRef) still read as no entry; ldap3 never follows them.
(3) The service connection is never explicitly unbound; this predates the change.
(4) A referral to the service bind surfaces as ldap3's generic message.

wshallwshall added 5 commits September 30, 2026 18:24
… (BACKLOG #2494)

On an interpreter with SSLContext.set_ciphersuites (Python 3.15) the
approved list drops TLS_AES_128_GCM_SHA256. ldap3 2.9.1 builds its own
context and could only narrow TLS 1.2, so assert_ldap3_tls_suites refused
and a first deployment on 3.15 would have lost AD sign-in.

Option 3 of the item, chosen by the Manager: narrow by another route.
assert_ldap3_tls_suites now returns a factory, like the hvac one. It calls
create_default_context(SERVER_AUTH, cadata=...) as ldap3 did, sets
check_hostname and verify_mode in ldap3's order, then adds the posture of
every engine client hop: TLS 1.2 floor, strict X.509 flags, kex pin,
narrow_to_approved_suites (TLS 1.2, TLS 1.3, sigalgs), the assertion, and
the approved-list hold. The new auth/ldap_tls.NarrowedTls wraps each
connection with a fresh context from it and still calls ldap3's own host
name check. Its wrap step is copied from ldap3; a test pins ldap3's
wrap_socket source hash. The bind passes ldap3 no ciphers= any more.

Measured on CPython 3.15.0b3: the bind constructs, offers only the two
approved TLS 1.3 suites, and refuses a TLS 1.3 AES-128-only DC that
ldap3's own Tls accepts. 3.14 is unchanged on the wire.

ADR 0188 amendment, ADR 0180 Amendment E, ADR 0173 and PHI.md corrected.
…erral gap (BACKLOG #2494)

Review round 1 (code-review subagent, xhigh) found no lost TLS guarantee.
Two of its low findings are fixed here. Every handshake test called
wrap_socket directly, so a future ldap3 that bypassed the override would
stay green. test_a_real_ldap3_connection_uses_the_narrowed_context opens
a real ldap3.Connection: a CBC-only DC is refused by NarrowedTls and
accepted by ldap3's own Tls (control). Mutation-checked: restoring
ldap3's wrap_socket turns it red. The NarrowedTls docstring no longer
cites the referral copy as reassurance; it names it as not reached.

Proposed PR title:
LDAPS narrows TLS 1.3 through an engine ldap3.Tls subclass, so AD sign-in survives Python 3.15 (BACKLOG #2494)

Proposed ledger banner:
SHIPPED 2026-09-30 on option 3 (Manager's choice): the engine builds the LDAPS context and NarrowedTls wraps each connection with it; measured on CPython 3.15.0b3, the bind constructs and refuses a TLS 1.3 AES-128-only DC. Referral path still unnarrowed (pre-existing).
…LOG #2530)

ldap3 2.9.1 follows a referral by default: Connection(auto_referrals=True)
and Server(allowed_referral_hosts=None), read as [('*', True)]. On a bound
connection, strategy/base.py create_referral_connection (751-797) opens a
connection to the referred host and binds there with the same user and
password, over a plain ldap3.Tls (no pinned CA bytes, no narrowing) or
none for ldap://. A first deployment would have sent the service-account
password to whatever host one search referral named.

Every Connection in auth/ldap.py now sets auto_referrals=False and the
Server sets allowed_referral_hosts=[]; each alone stops the follow. Every
search goes through _search, and a referral answer to a search or to the
user bind raises LdapError naming only the referred host. So a referral is
audited as auth.login_error at sign-in, read as unavailable by the
reconciler (never revokes), and not counted as a wrong password by the
step-up re-bind. Before this, with following off, it would have read as
"no entries" or "wrong password".

Tests drive a real ldap3 stack against two loopback LDAP servers: the
referred one accepts no connection; with both guards reverted it receives
the bind password (positive control). Static guards pin the kwargs and the
single search path. Docs: ADR 0180 Amendment F, PHI.md section 4, a
changelog.d fragment. #2530 is filed on vault PR 2120, not yet on vault main.
…results (BACKLOG #2530)

Code-review round 1 (code-review subagent, xhigh): no high finding.
Repaired here:
- the refusal names at most three hosts and counts the rest, and admits
  an underscore in a host name;
- docstrings, ADR 0180 Amendment F and the changelog now say the refusal
  covers a referral RESULT (resultCode 10). A searchResRef continuation
  reference is never followed by ldap3 and still reads as no entry; AD
  adds one to every domain-root search, so refusing it would refuse all;
- the refusal's advice no longer assumes a search, and the ADR says a
  global catalog carries universal-group membership only;
- the ldap3 double sets result on every operation, as ldap3 does;
- the single-search-path guard covers async and module scope and lets a
  regular expression's .search through.

Not repaired, left for the Manager (see the PR body): the reconciler
reads a referral as an outage, so a misconfigured base would hold every
pass rather than revoke; the service connection is never unbound
(pre-existing); a single Connection factory instead of three sites.

Proposed PR title:
The AD hop follows no LDAP referral, so the bind password cannot leave the anchored LDAPS hop (BACKLOG #2530)

Proposed ledger banner:
SHIPPED 2026-09-30: every ldap3 Connection sets auto_referrals=False and the Server allowed_referral_hosts=[]; a referral result refuses as LdapError naming only hosts. Loopback test: referred host gets no connection; with both guards reverted it receives the bind password. Stacked on PR 1869.
…binds they cover (BACKLOG #2530)

Code-review round 2 (code-review subagent, xhigh): no new correctness
defect; six low findings, five repaired here:
- an unreadable referral URL no longer takes a named slot ahead of a
  readable host; it is listed last;
- the refusal's global-catalog advice carries the universal-groups
  caveat and points at ADR 0180 Amendment F;
- the changelog says a referral to the service-account bind fails that
  bind as before (ldap3's auto_bind error), not a host-naming refusal;
- the underscore, placeholder-order and empty cases are asserted directly;
- the single-search-path guard drops its dead regex exclusion and maps
  only function bodies. Left: none of substance.

Checks: ruff check and format, mypy strict on messagefoundry and tests,
pytest over every tests/test_*.py file in seven xdist chunks (all green,
before this commit), and the auth/LDAP/TLS/ADR slice after it (1016
passed, 3 skipped).

Proposed PR title:
The AD hop follows no LDAP referral, so the bind password cannot leave the anchored LDAPS hop (BACKLOG #2530)

Proposed ledger banner:
SHIPPED 2026-09-30: every ldap3 Connection sets auto_referrals=False and the Server allowed_referral_hosts=[]; a referral result to a search or the user bind refuses as LdapError naming only hosts. Loopback test: referred host gets no connection; with both guards reverted it receives the bind password. Stacked on PR 1869.
@wshallwshall wshallwshall added the qa Builder QA record posted; not a merge gate label Oct 1, 2026
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA -- korus roles/BUILDER.md step 11
Level: xhigh, from the brief. Tag: xhigh effort → 10 inline angles → dedup (no verify) → sweep → ≤15 findings
Rounds: 2. Findings: 13 confirmed and fixed, 3 rejected (1 contrary to the brief's own design, 2 too costly or too wide for this diff), 4 open.

@wshallwshall
wshallwshall enabled auto-merge October 1, 2026 01:11
Updates PR 1877 onto main. PR 1877 was built on PR 1869's branch.
1869 landed squashed as 2679a8d, with the same patch id as its
original commits.

Three files conflicted: auth/ldap.py, auth/ldap_tls.py and ADR 0180.
Main's copy of each equals 1869's original, so each hunk was 1877's
own edit on top of it. Took the branch side in all three.

The merged tree differs from origin/main by exactly 1877's own diff
(a44d060..453990c): the patch ids match.
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Update onto main (Manager-briefed Builder): head dcdb9ca, a fast-forward from 453990c.

  • 1869's squash 2679a8d and its original commits share patch-id 8b2f4a89; control ec9e7cf alone gives 75df1073.
  • Four conflicted hunks, all 1877's own edits on top of 1869's text; the branch side was taken. The staged diff's patch-id (69d61492) equals 1877's original diff's, so nothing was lost.
  • Checks:
    • ruff and mypy clean (304 and 1046 files)
    • changelog fragments: 11 well-formed
    • crypto-inventory OK, with its positive control firing
    • pytest on 34 ldap, ad, auth, kerberos, tls and crypto files: 1492 passed, 6 skipped
  • Disclosure: the claim gate refused a merge subject naming BACKLOG #2530, because the claim is held by the prior Builder's idle worktree. The merge commit now uses git's default subject. No --no-verify was used.

QA -- korus roles/BUILDER.md step 11
Level: xhigh, from the brief. Tag: none returned.
Rounds: 1. Findings: 0 confirmed in the merge resolution, 0 rejected, 2 open (low, documentation, older than this merge; not fixed, out of scope).

@github-actions github-actions Bot added the ci-red A required check went red. Attribute it before retrying. label Oct 1, 2026
wshallwshall added 2 commits September 30, 2026 23:25
…en on Linux

ldap3 sets SO_RCVTIMEO with struct.pack('LL', receive_timeout, 0) on
every non-Windows host. [auth].ad_receive_timeout is a float (default
10.0), and struct.pack refuses a float, so every ldap3 socket open
raised "struct.error: required argument is not an integer" on Linux
before any byte reached the domain controller. Windows hid it because
ldap3 converts with int(1000 * t) on that branch.

The real-socket referral tests added for BACKLOG #2530 were the first
to open an ldap3 socket on the Linux CI leg, and all five went red
there. The engine was at fault, not the fake DC.

_ldap3_receive_timeout rounds the setting UP to whole seconds: never
shorter than configured, never 0 (ldap3's wait-forever). The POSIX
branch drops sub-second precision anyway. test_ldap_timeouts now
asserts an int equal to ceil(setting) and adds a rounding unit test
whose float arm must raise struct.error.
… E and F

The Unreleased BACKLOG #2494 CHANGELOG entry still said a followed
referral gets a plain ldap3 context. Since BACKLOG #2530 on this branch
the engine refuses referrals, so the entry now says so and points at
changelog.d/2530.security.md. The ADR index row for 0180 now names
Amendments E and F.
@wshallwshall
wshallwshall disabled auto-merge October 1, 2026 04:32
wshallwshall added 6 commits September 30, 2026 23:36
The fix in cd9691a had a CHANGELOG.md edit for the referral entry but no fragment of its own for the struct.error defect. Slug name because the defect has no backlog item. Part of the BACKLOG #2530 pull request.
Renames changelog.d/ldap3-receive-timeout-linux.fixed.md to 2546.fixed.md and cites BACKLOG #2546, the vault item for the Linux struct.error defect (filed on vault PR 2130, not yet on vault main).
…out wrapper at every Connection

A huge finite ad_connect_timeout or ad_receive_timeout overflowed socket.settimeout with OverflowError, which no LdapError handler catches, so sign-in would have failed outside the audited path. The settings validator now refuses anything above 3600 s at config load. The static guard now requires every ldap3.Connection receive_timeout to be a call to _ldap3_receive_timeout, with a control test. The test receive timeout is now 11.25 so its ceiling differs from the default 10. The _ldap3_receive_timeout docstring now says ceil lengthens the timeout on every OS, because ldap3 calls settimeout first. CONFIGURATION.md states the cap and the rounding. Review findings on the BACKLOG #2530 / #2546 pull request.
…0 index row

Reverts the direct CHANGELOG.md edit from c947b5c, because changelog.d/README.md forbids editing CHANGELOG.md in a pull request. The BACKLOG #2530 fragment now says it supersedes the #2494 entry's followed-referral note, in words rather than a fragment path. The #2546 fragment states the 3600 s cap and drops the claim that ldap3 reads 0 as no timeout. The ADR 0180 index row gains Amendment B (BACKLOG #2034) and drops Amendment C's admitted ciphers= value, which Amendment E retired.
…nt reason once

The validator comment said ldap3 reads 0 as wait-forever for both timeouts; that holds only for connect_timeout, while a 0 receive_timeout makes the socket non-blocking. The test now points at the _ldap3_receive_timeout docstring instead of restating the struct.pack reason. Round-one review findings on the BACKLOG #2530 / #2546 pull request.
…e-timeout guard's control drive the guard

Round-two review findings. The over-3600 message no longer claims a larger value overflows (overflow starts near 3e6 s; the cap sits far below it). The <=0 message gives the real reasons. The docstring and the BACKLOG #2546 fragment say struct.error fired after the TCP connect, not before any byte. The static guard's filter is now a function its control test runs on a raw and a wrapped snippet, and the walker sanity check requires all three Connection sites. CONFIGURATION.md says the receive timeout bounds each socket receive. The #2530 fragment sentence is split under 25 words.
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Review round and #2546 fragment (Manager-briefed Builder): head f3f8640, fast-forward from c947b5c.

  • The Linux receive-timeout fix (vault #2546, P1) gets changelog.d/2546.fixed.md.
  • ad_connect_timeout and ad_receive_timeout are now capped at 3600 s, refused at config load.
  • The static guard requires _ldap3_receive_timeout at every ldap3.Connection, and its control arm exercises the filter.
  • The test timeout is 11.25 (ceiling 12), so a hardcoded 10 no longer passes.
  • Docstring corrected: ceil lengthens the timeout by under a second on every OS, and a 0 receive_timeout is non-blocking.
  • The direct CHANGELOG.md edit is REVERTED (byte-identical to dcdb9ca) per changelog.d/README. The correction is in the 2530 fragment instead.
  • The ADR 0180 index row is completed (Amendment B added, Amendment C's admitted ciphers= value dropped).
    Checks:
  • ldap and auth tests: 396 passed, 2 skipped
  • settings, configuration and changelog: 678 passed
  • ruff and mypy clean (304 and 1046 files); 12 fragments well-formed
  • The Linux real-socket arm must be read on the hosted ubuntu leg.
    Open, for follow-up: OverflowError, TypeError and struct.error are not mapped to LdapError (the cap removes the only validated trigger); security_loosenings does not flag raised AD timeouts.

QA -- korus roles/BUILDER.md step 11
Level: xhigh, from the brief. Tag: none returned.
Rounds: 2. Findings: 14 confirmed and fixed (7 Manager-relayed from the prior reviewer plus 7 more from round 2), 1 rejected (unmerged-citation aside: would go stale in a permanent release note), 4 open (overflow class unmapped to LdapError; contradictory #2494 and #2530 release notes, accepted per Manager decision; one-hour worker hold at the cap; security_loosenings does not flag raised AD timeouts).

@wshallwshall
wshallwshall enabled auto-merge October 1, 2026 05:09
wshallwshall added 2 commits October 1, 2026 02:50
…n empty walk

The vacuous-absence lint (tests/test_vacuous_absence_assert_lint.py),
which arrived on main, flags test_every_ldap3_construction_site_turns_referrals_off
on `wrong`: the `not wrong` check walks `sites`, and the existing per-kind
count assertion reads `kinds`, not `sites`, so the lint cannot see it.
Assert the walk found at least four sites (three ldap3.Connection and one
ldap3.Server) before the absence check.

Also merges origin/main in.
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA: code-review subagent xhigh, head 21c948d, verdict PASS; len(sites) >= 4 guard satisfies the vacuous-absence lint, floor matches the four ldap3 sites (1 Server, 3 Connection), test strength unchanged.

@wshallwshall
wshallwshall added this pull request to the merge queue Oct 1, 2026
Merged via the queue into main with commit 552cab7 Oct 1, 2026
45 of 47 checks passed
@wshallwshall
wshallwshall deleted the builder/ldap3-referral-credentials branch October 1, 2026 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-red A required check went red. Attribute it before retrying. qa Builder QA record posted; not a merge gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant