Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
f3815f8
feat(#33 phase2): add internal/ldap/profile restricted DN grammar
BorisTyshkevich Aug 30, 2026
f357b55
feat(#33 phase2): add internal/ldap/profile framing, envelope, and Co…
BorisTyshkevich Aug 30, 2026
0cd4206
test(#33): add profile dependency contracts (production-absence, cryp…
BorisTyshkevich Aug 30, 2026
7f4fd5d
feat(#33 phase2): add internal/ldap/profile response encoders and dia…
BorisTyshkevich Aug 30, 2026
1e3e066
merge: p2-03-encode-diagnostics (#33)
BorisTyshkevich Aug 30, 2026
fe7a8ce
merge: p2-15-dependency-contracts (#33)
BorisTyshkevich Aug 30, 2026
7765969
feat(#33): add internal/ldap/profile logging (correlation_id, reason …
BorisTyshkevich Aug 30, 2026
b47526c
feat(#33 phase2): add internal/ldap/profile config, session state, an…
BorisTyshkevich Aug 30, 2026
710e865
feat(#33 phase2): add internal/ldap/profile Bind decoder and state po…
BorisTyshkevich Aug 30, 2026
92b50e4
feat(#33 phase2): add internal/ldap/profile Search decoder, fixed fil…
BorisTyshkevich Aug 30, 2026
828d928
feat(#33 phase2): add internal/ldap/profile server lifecycle and disp…
BorisTyshkevich Aug 30, 2026
9585d44
test(#33): add mechanical architecture contract for internal/ldap/pro…
BorisTyshkevich Aug 30, 2026
57e244b
test(#33): add shared old/new profile compatibility tests
BorisTyshkevich Aug 30, 2026
d61cc0e
test(#33): add internal/ldap/profile real-TCP protocol test suite
BorisTyshkevich Aug 30, 2026
3a91fc1
merge: p2-09-tcp-protocol-port (#33)
BorisTyshkevich Aug 30, 2026
fe866d5
merge: p2-14-shared-compat (#33)
BorisTyshkevich Aug 30, 2026
5edce04
merge: p2-16-architecture-contract (#33)
BorisTyshkevich Aug 30, 2026
b14dec6
test(#33 phase2): add internal/ldap/profile adversarial and mid-Searc…
BorisTyshkevich Aug 30, 2026
9b00613
test(#33 phase2): add internal/ldap/profile Search limits, hostile-DN…
BorisTyshkevich Aug 30, 2026
36f3475
test(#33): add real-TCP replay of committed ClickHouse wire fixtures
BorisTyshkevich Aug 30, 2026
cfa3b9a
test(#33): register internal/ldap/profile in the redaction inventory
BorisTyshkevich Aug 30, 2026
b564af6
merge: p2-12-wire-replay (#33)
BorisTyshkevich Aug 30, 2026
d50a95c
merge: p2-17-redaction-inventory (#33)
BorisTyshkevich Aug 30, 2026
0693fe1
test(#33): add profile package's differential oracle against vendored…
BorisTyshkevich Aug 30, 2026
736f573
docs(#33): document Phase 2 compatibility profile and the ten Phase-3…
BorisTyshkevich Aug 30, 2026
d273414
fix(#33 phase 2): correct write-stall classification, harden DN error…
BorisTyshkevich Aug 30, 2026
ba8862d
fix(#33 phase 2): address review pass 1 findings
BorisTyshkevich Aug 30, 2026
1dbaa59
fix(##33 phase 2): address review pass 2 findings
BorisTyshkevich Aug 30, 2026
685fdf0
fix(##33 phase 2): address review pass 3 findings
BorisTyshkevich Aug 30, 2026
f3bf13b
test(#33): enforce profile type-semantic invariants with go/types ins…
BorisTyshkevich Aug 30, 2026
aef938b
fix(##33 phase 2): address review pass 1 findings
BorisTyshkevich Aug 30, 2026
78570aa
fix(##33 phase 2): address review pass 2 findings
BorisTyshkevich Aug 30, 2026
b62c1c2
fix(#33 phase 2): address review pass 3 findings
BorisTyshkevich Aug 31, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ no native Bearer/JWT auth path; Antalya does).
|---|---|
| `cmd/ch-jwt-verify/` | the sidecar binary — `main.go` (CLI/server wiring), `config.go` (YAML config), `settings.go`, `verify.go` (the `/verify` handler: JWT validation, cache-key binding, identity policy), `verify_test.go` |
| `cmd/ch-oauth-ldap/` | the standalone LDAPv3 server binary — `main.go` (CLI/signal-context/lifecycle wiring: verifier + role pipeline + `internal/ldap.Server`), `config.go` (YAML config, defaults, fail-startup validation, conversion into `verification.Config`/`identity.Config`/`roles.Config`/`internal/ldap.Config`), `config_test.go` (incl. `testdata/operator-guide.yaml` loaded through production `LoadConfig` and strict-decoded with `KnownFields(true)` — the proof backing `docs/ch-oauth-ldap-operator-guide.md`'s YAML fences), `main_test.go` |
| `internal/ldap/` | LDAP protocol package consumed by `cmd/ch-oauth-ldap` — `server.go` (lifecycle/handler-source wiring, dependency-logger suppression, connection cap, timeouts), `controls.go` (unsupported-critical-control guard), `session.go` (per-connection authenticated state), `dn.go` (RFC 4514 Bind-DN parsing/validation), `filter.go` (structural Search-filter authorization), `bind.go`/`search.go`/`unsupported.go` (the LDAP operation handlers, incl. Search sizeLimit/timeLimit/typesOnly enforcement and safe numeric-only Search telemetry), `entry.go` (synthetic `groupOfNames` rendering), plus `*_test.go` including a real-TCP `protocol_test.go` and `adversarial_test.go` |
| `internal/ldap/` | LDAP protocol package consumed by `cmd/ch-oauth-ldap` — `server.go` (lifecycle/handler-source wiring, dependency-logger suppression, connection cap, timeouts), `controls.go` (unsupported-critical-control guard), `session.go` (per-connection authenticated state), `dn.go` (RFC 4514 Bind-DN parsing/validation), `filter.go` (structural Search-filter authorization), `bind.go`/`search.go`/`unsupported.go` (the LDAP operation handlers, incl. Search sizeLimit/timeLimit/typesOnly enforcement and safe numeric-only Search telemetry), `entry.go` (synthetic `groupOfNames` rendering), plus `*_test.go` including a real-TCP `protocol_test.go` and `adversarial_test.go`; also `profile_compat_test.go` (`package ldap_test`, issue #33 phase 2 — shared old/new compatibility tests driving both `internal/ldap` and `internal/ldap/profile` over real TCP, asserting exact parity, incl. the `correlation_id` value itself, only where the plan actually claims parity) |
| `internal/ldap/profile/` | issue #33 phase 2 — a first-party, bounded ClickHouse LDAP compatibility profile (package `profile`) meant to eventually replace `internal/ldap` above; its ASN.1/protocol implementation (`frame.go`/`protocol.go`) is stdlib plus `golang.org/x/crypto/cryptobyte` only — the package's own dependency closure is wider by design, since `config.go` directly depends on `internal/verification` and the upstream OAuth SDK's types for `Verifier`/`RoleResolver` (a deliberate, legitimate transitive dependency `internal/securitytest/profile_dependency_contract_test.go` documents rather than pins away); no `go-ldap`/vendored-goldap/vendored-ldapserver import either way (enforced below). **Phase 2 status: implementation and tests exist; `cmd/ch-oauth-ldap` still runs the legacy `internal/ldap` path in production — nothing production-reachable imports this package yet (Phase 4 cuts over).** Production files: `config.go` (`Config`/`Verifier`/`RoleResolver`/`ValidateConfig`; deliberately no `Listen` — the command keeps owning the listener), `dn.go` (restricted RFC-4514-subset DN grammar, structural equality, synthetic-DN rendering), `frame.go` (bounded-before-allocation BER framing, `cryptobyte` LDAPMessage envelope, minimal Controls-criticality scanner), `protocol.go` (application tags, result-code constants, the shared minimal-positive-INTEGER validator Abandon's implicit target integer reuses), `encode.go` (`cryptobyte.Builder` response encoders, the closed `diagnostic` enum + `addLDAPResultFields`, 64 KiB response-PDU preflight), `logging.go` (byte-identical `correlationID` re-derivation, the closed `reason` enum, log helpers), `session.go` (connection-owned `authState`/`connection`, no password/frame field retained), `bind.go` (LDAPv3-only simple-Bind decoder, clear-before-validate state policy, the sole `Verify`/`Roles` call sites), `search.go` (fixed ClickHouse Search shape, nonrecursive two-child membership-filter decode, sizeLimit/timeLimit, `cn`-only entries), `server.go` (lifecycle, 256-connection admission, the package's one production `go` statement, dispatch), `doc.go` (package-status doc comment). Test files: a `*_test.go` beside each production file, plus `protocol_test.go` (real-TCP black-box suite ported from legacy `internal/ldap/protocol_test.go`), `adversarial_test.go`/`midsearch_test.go` (framing-cap/deadline-stall/malformed-BER/partial-write adversarial suite), `conncap_test.go` (256/257-connection admission), `search_limits_test.go`/`hostile_dn_test.go`/`redaction_boundary_test.go` (Search-limit telemetry, hostile-Bind-DN, and marker-redaction proofs), `replay_test.go` (`TestProfileReplay` — real-TCP replay of every committed `internal/ldap/testdata/clickhouse-wire/**` session through this server), `differential_test.go` (`TestProfile_DifferentialDecoder`, `package profile`, differential oracle against vendored goldap imported only from `_test.go` — Phase 4 **must delete this file**, see `docs/clickhouse-ldap-wire-profile.md` §11.4), `fakes_test.go` (the canonical in-package `fakeVerifier`/`fakeResolver`/`fakeClock`, per Amendment 7 duplicated — not shared via a non-test helper package — in `internal/ldap/profile_compat_test.go`), and five native fuzz targets: `frame_fuzz_test.go` (`FuzzLDAPFrame`), `bind_fuzz_test.go` (`FuzzBindRequest`), `search_fuzz_test.go` (`FuzzSearchRequest`), `dn_fuzz_test.go` (`FuzzRestrictedDN`, `FuzzMemberAssertionDN`) — every committed seed runs under ordinary `go test`; a short fuzz smoke beyond seeds is `go test ./internal/ldap/profile -run '^$' -fuzz=Fuzz<Name> -fuzztime=20s`, one target at a time (never wait for a real 20/30s production deadline in an ordinary unit test). Registered in `internal/securitytest`'s redaction scope (`scopeDirs`, plus sink kind `ldap-profile-diagnostic` for direct `addLDAPResultFields(..., diagConstant)` calls) and covered by the general `internal/ldap/**` nested-package guard (`TestRedactionInventory_NestedLDAPPackagesAreRegistered`, registered outright rather than exempted). `internal/securitytest/profile_architecture_contract_test.go` mechanically enforces the syntactic invariants: exactly one production `go` statement (the admission spawn); no `sync.Map`; no old-LDAP/BER/`unsafe` import; `decodeMembershipFilter` nonrecursive with exactly two `decodeEquality` calls; `diagnostic`/`reason` bytes reachable only through their closed enums (no dynamic conversion). Its companion `internal/securitytest/profile_types_contract_test.go` enforces the four type-semantic invariants over the fully type-checked package (go/types on gc export data from a deterministic `go list -deps -export`, via stdlib `importer.ForCompiler` — not AST shape enumeration, which three review passes plus an architecture consultation showed is bypassable by ordinary Go): no map- or channel-typed declaration, signature, or value expression beyond `Server.active` (`map[net.Conn]struct{}`) and `serveDone`/`stopDone` (`chan struct{}`), and `Verifier.Verify`/`RoleResolver.Roles` each referenced — call, method value, or method expression — exactly once, both in `bind.go` (struct fields named `Roles` are excluded by selection kind, not naming convention); reflection/`unsafe` laundering is outside what the type checks see and is covered by the import bans. `internal/securitytest/profile_dependency_contract_test.go` mechanically enforces this package's absence from `./cmd/ch-oauth-ldap`'s live dependency closure during Phase 2, and that this package's own closure requires `golang.org/x/crypto/cryptobyte` while excluding the vendored/general LDAP stack (`vjeantet/ldapserver`, `vjeantet/goldap`, `go-ldap/ldap/v3`, `go-asn1-ber/asn1-ber`, `Azure/go-ntlmssp`, `internal/wirefixture`) |
| `internal/securitytest/` | phase-5 automated security verification, not itself security-relevant production code (see its `doc.go`) — `redaction_inventory_test.go` (AST-enumerates every log/error/response-construction sink across the explicit audited scope list named by `doc.go`'s `scopeDirs` — `cmd/ch-oauth-ldap`, `cmd/ch-jwt-verify`, `internal/ldap`, `internal/verification`, `internal/identity`, `internal/roles`, `internal/wirefixture`, `integration/clickhouse/wirecapture` — plus the vendored `third_party/ldapserver` fork, and cross-checks `testdata/redaction-sites.tsv`; `TestRedactionInventory_Phase1AuditedScopesRemainFlat` mechanically guards the two issue-#33 scopes staying flat, since `discoverSites` only reads each scope directory's direct, non-test `.go` files and never recurses), `sdk_contract_test.go` (pinned `go-mcp-oauth-sdk` version + no-replace + strict-entrypoint AST checks), `release_gate_test.go` (`-tags phase5release` only: fails while any manifest row is `blocked_external`, per amendment A1 — see "Build & test" below), `pr_gate_contract_test.go` (untagged, so it runs in both `go test ./...` and the gate's own `go test -race ./...` step: parses `.github/workflows/pr-gate.yml` and asserts the required PR gate's structure against literal expectations — unfiltered triggers, no path filters, `contents: read` only, exactly one unconditional job named `Required PR gate`, the five commands exactly once in order, SHA-pinned actions; an accidental-drift detector, not tamper resistance, since it lives in the same PR-controlled tree as the workflow), `docs_contract_test.go` (every `config-source`-marked YAML/XML fence in `README.md`/`docs/ch-oauth-ldap-operator-guide.md` — this repo's only two operator-facing narrative documents — is an exact contiguous excerpt of its real source file, with both files' `<clickhouse>` fence additionally required to equal the ClickHouse fixture's `<clickhouse>` element exactly, not just be a substring of the whole fixture file; required/forbidden HA-and-trust-boundary wording; the stale-1-MiB-comment guard; `docs/clickhouse-ldap-wire-profile.md` is deliberately NOT in its scope — see `wire_profile_contract_test.go` below), `dependency_contract_test.go` (issue #33 phase 1, plan §5/§6/§31: deterministically invokes `go list -deps` against `./cmd/ch-oauth-ldap` and asserts the live non-standard closure exactly matches `testdata/production-nonstdlib-deps.txt`, that non-standard `golang.org/x/crypto/cryptobyte` is absent, and that `internal/wirefixture` is absent — the mutable *live* expectation, distinct from the immutable historical snapshot at `internal/ldap/testdata/phase1-baseline/production-nonstdlib-deps.txt`; Phase 3 extends this same file with the general-LDAP denylist rather than adding a competing gate), `wire_profile_contract_test.go` (issue #33 phase 1, plan §30: Docker-free anti-drift contract over the wire evidence — static ClickHouse/OpenLDAP source-provenance matrix cross-checked against `run-all-builds.sh`/the wire-profile doc/every `profile.json`, four-way tracked-line equality, the `<clickhouse>` config-hash + semantic contract, fixture inventory (orphan/sequence/SHA/`connection_count`/operation-coverage checks), the `*.ber binary` `.gitattributes` rule, an all-file JWT-shape scanner plus its own AST-derived positive control, the wire doc's exactly-one-decision-marker/no-XML-YAML-fence boundary, the constructed-MessageID-127/128 regenerate-and-compare proof, and the AST/`go list -deps` check that `integration/clickhouse/wirecapture` writes metadata only through `internal/wirefixture`) |
| `internal/wirefixture/` | issue #33 phase 1 (plan §9, §25–§29) — the sole owner of the committed ClickHouse-LDAP-wire fixture schema: `schema.go` (`Profile`/`Session`/`PDU`, strict JSON decode, deterministic encode, `StableProfile`/`StableSession`/`StablePDU` verify-mode comparison projections), `root.go` (`ModuleRoot`, fixture-path helpers, `ValidateFixtureRoot`), `constructed.go` (`BuildConstructedSimpleBind`/`BuildConstructedMessageIDBoundarySession`, the deterministic MessageID-127/128 boundary generator), `confighash.go` (`ClickHouseConfigElementSHA256`, hashing only the trimmed `<clickhouse>...</clickhouse>` element). Test/tooling support only: `internal/securitytest/dependency_contract_test.go` asserts it is mechanically absent from `./cmd/ch-oauth-ldap`'s production closure. `integration/clickhouse/wirecapture` writes `profile.json`/`session.json` only through this package's `WriteProfile`/`WriteSession`; `internal/ldap/clickhouse_wire_cryptobyte_test.go` and `internal/securitytest/wire_profile_contract_test.go` read through it too — see plan §9's "writer/reader ownership" |
| `docs/` | `ch-oauth-ldap-operator-guide.md` — consolidates (does not restate) the working ClickHouse config, OIDC/Auth0/JWKS field reference, identity/role-pipeline behavior, the cache invariant, measured ClickHouse Search-limit behavior (scenario G'), the plaintext-bearer trust boundary, and HA/capacity incl. the Kubernetes runbook. Every fence in it is enforced against its real source by `internal/securitytest/docs_contract_test.go`. `clickhouse-ldap-wire-profile.md` — issue #33 phase-1 engineering evidence (source-cited ClickHouse/OpenLDAP behavior, the wire corpus, the `cryptobyte` primitive decision) for Phase 2/3's implementer, not a second operator-facing guide; owned by `internal/securitytest/wire_profile_contract_test.go`, not `docs_contract_test.go` |
Expand Down Expand Up @@ -130,6 +131,16 @@ fork that logic — extend the SDK instead when the need is generic (not
- `go test -race ./internal/ldap` — the LDAP package's race gate; run it
whenever touching connection-local session state or the critical-control
guard.
- `go test -race ./internal/ldap/profile` — the issue #33 phase 2 compatibility
profile's own race gate (this package is not yet in the production closure,
so it is not covered by `go test -race ./...`'s pass over `internal/ldap`
in spirit — it's a separate package, run separately). Its five native fuzz
targets' committed seed corpora already run under ordinary `go test`; a
short documented smoke beyond seeds, run one target at a time, is
`go test ./internal/ldap/profile -run '^$' -fuzz=Fuzz<Name> -fuzztime=20s`
for each of `FuzzLDAPFrame`, `FuzzBindRequest`, `FuzzSearchRequest`,
`FuzzRestrictedDN`, `FuzzMemberAssertionDN` — never wait for a real
20/30-second production deadline in an ordinary unit test.
- `(cd third_party/goldap && go test ./...)` — the vendored, patched
`github.com/vjeantet/goldap` fork's own test suite (BER integer
sign-disambiguation, `ModifyDNResponse.SetResultCode`, and the Filter
Expand Down
55 changes: 55 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,61 @@ What the fixture proves, and what you should expect in production:
lands, keep `ch-oauth-ldap` on a trusted internal network and never
expose it publicly.

### Compatibility profile (issue #33 phase 2 — implementation only, not yet in production)

Issue #33 is replacing the vendored `third_party/goldap`/`third_party/ldapserver`
LDAP stack above with a first-party, bounded ClickHouse compatibility profile at
`internal/ldap/profile/`. **This is a development-status note, not a cutover
announcement**: `cmd/ch-oauth-ldap` still runs the legacy server described above
in production; the profile package exists with its own real-TCP tests, native
fuzzing, real-TCP replay of every committed wire fixture, and
dependency/architecture/redaction contracts (see `CLAUDE.md`'s
`internal/ldap/profile/` repo-map row), but nothing production-reachable imports
it yet — that cutover is Phase 4.

Two ClickHouse-configured values stay genuinely variable, not hard-coded, in the
replacement:

- **`search_limit`** stays a client/operator-controlled `N` (the fixture default
above is `256`);
- **client `timeLimit`** is honored as sent (the currently tracked ClickHouse
value is `20` seconds).

Cutover also brings several **deliberate narrowings** — behavior this legacy
server tolerates today that the replacement will not, each one requiring
explicit Phase 3 acceptance before Phase 4:

- **Search-shape narrowing.** Only subtree scope, `derefAliases=0`,
`typesOnly=false`, and exactly one requested attribute (case-insensitive
`cn`) are accepted; an empty attribute list, `*`, `1.1`, or any other/
multiple attribute selection is rejected (result 50) instead of today's
generic projection.
- **LDAPv3-only narrowing.** Only LDAPv3 simple Bind is accepted; the current
server's incidental Bind-version-2 acceptance is not retained (result 2
instead).
- **DN-grammar narrowing.** Multi-valued RDNs, `;` RDN separators, `#`
BER-hexstring values, dotted-decimal/OID attribute types, and arbitrary
escaped attribute-type names are rejected; supported whitespace handling
and `\XX` value escapes remain.
- **Control-plane narrowing.** Ordinary Abandon remains a no-response
compatibility no-op but no longer cancels an in-flight operation; ordinary
RFC 3909 Cancel becomes an unsupported Extended request (result 53) instead
of today's vendored Cancel scheduler; a peer disconnect no longer
asynchronously cancels an already-running verification call.
- **New response-PDU cap.** Every outbound LDAPMessage is capped at 64 KiB;
an oversized `SearchResultEntry` is dropped in favor of `SearchResultDone`
result 11 (`adminLimitExceeded`), with the already-emitted entry count
preserved. This is a genuinely new client-visible behavior, not parity —
today's write path has no outbound size bound at all.
- **New `UserRDNAttribute` startup validation.** The replacement requires the
configured attribute descriptor to match `[A-Za-z][A-Za-z0-9-]*`; current
production only rejects an empty/whitespace value.

See [`docs/clickhouse-ldap-wire-profile.md`](docs/clickhouse-ldap-wire-profile.md)
§11 for the full engineering-evidence writeup of all ten narrowings and
[`docs/ch-oauth-ldap-operator-guide.md`](docs/ch-oauth-ldap-operator-guide.md#10-compatibility-profile-issue-33-phase-2--in-development)
§10 for the equivalent operator-facing note.

[ch-ldap]: https://clickhouse.com/docs/operations/external-authenticators/ldap
[ch-79099]: https://github.com/ClickHouse/ClickHouse/pull/79099
[ch-78791]: https://github.com/ClickHouse/ClickHouse/issues/78791
Expand Down
Loading