Altinity / Altinity/altinity-oauth-helper

ADR: Own the ClickHouse LDAP compatibility profile and remove general LDAP dependencies

Open
#32 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

adr
Dominant language
Go
Stars
5
Forks
0
Avg merge
7h 12m
Merged PRs (30d)
31

Description

Status

Accepted.

Implementation is tracked separately in #33.

Decision

ch-oauth-ldap will stop embedding and patching general-purpose LDAP implementations and instead own a small ClickHouse-specific LDAP compatibility profile.

The compatibility target is not LDAP in general. It is the bounded behavior exercised by supported ClickHouse LDAP authentication configurations.

At a high level:

ClickHouse LDAP client
    |
    | LDAPv3 simple Bind
    | bounded role-mapping Search
    | Abandon on search timeout
    | Unbind
    v
ch-oauth-ldap
    |
    | OAuth/OIDC verification
    | role snapshot
    v
ClickHouse RBAC

This does not change ADR #16's product architecture. ch-oauth-ldap remains an OAuth-to-ClickHouse authentication compatibility layer; ClickHouse remains authoritative for RBAC.

It changes ownership of the LDAP compatibility and parsing boundary.


Why this decision is being reconsidered

Issue #19 originally chose github.com/vjeantet/ldapserver with the reasonable premise that an existing LDAP implementation was safer than implementing LDAP BER handling ourselves.

Implementation and adversarial testing changed that premise.

The repository now carries local forks of:

third_party/ldapserver
third_party/goldap

with downstream fixes for issues including:

  • attacker-controlled BER lengths causing very large pre-authentication allocations;
  • short TCP reads being treated as complete messages;
  • unbounded per-client request concurrency;
  • non-reading clients retaining server resources;
  • aggregate connection exhaustion;
  • Abandon/Cancel scheduling complexity;
  • BER INTEGER encoding around MessageID 127/128;
  • pathological recursive LDAP filters;
  • credential-bearing packet logging.

The production path also imports github.com/go-ldap/ldap/v3 for DN parsing/equality/rendering, which brings more general LDAP/authentication/parser code into the production closure, including github.com/go-asn1-ber/asn1-ber and github.com/Azure/go-ntlmssp today.

We are therefore already responsible for substantial security review, patching, regression testing, resource-limit design, and dependency maintenance on an unauthenticated credential-bearing network boundary, while using only a very small fraction of the general LDAP feature set.

LDAP here is a compatibility protocol, not a strategic directory-server feature.

This ADR supersedes issue #19's implementation-level instruction to use github.com/vjeantet/ldapserver and not implement LDAP BER handling directly.


Evidence that the required profile is small

A review of current ClickHouse source shows that the relevant behavior is concentrated in:

src/Access/LDAPClient.h
src/Access/LDAPClient.cpp
src/Access/ExternalAuthenticators.cpp
src/Access/LDAPAccessStorage.cpp

The authentication path is approximately:

LDAPSimpleAuthClient::authenticate()
    |
    +-- openConnection()
    |      +-- LDAPv3 client setup
    |      +-- optional TLS/StartTLS according to config
    |      +-- simple Bind
    |      +-- optional user-DN Search if configured
    |
    +-- zero or more role-mapping Searches
    |
    +-- Unbind / close

For the documented profile used by this product, ClickHouse's Search behavior is correspondingly narrow: configured base DN, scope, filter, one requested attribute, typesOnly = false, configured time/size limits, and no request controls.

ClickHouse delegates encoding and synchronous search behavior to OpenLDAP/libldap. Two library details materially affect the compatibility profile:

  1. OpenLDAP's ldap_create() initializes request BER with LBER_USE_DER. This is strong evidence that ClickHouse requests use DER-compatible primitive encodings, although real supported-version captures remain the final interoperability proof.
  2. OpenLDAP's synchronous Search path sends AbandonRequest when ldap_result() times out. Therefore Abandon is part of the real ClickHouse/libldap profile even though it is not part of the successful authentication path.

The ClickHouse LDAP code is not literally frozen. For example, follow_referrals support was added in 2026. That does not require a general LDAP server: changes are localized and can be reviewed when supported ClickHouse versions change.

The maintenance model is therefore:

derive semantics from ClickHouse source and the documented configuration we support; inspect relevant libldap behavior where ClickHouse delegates to it; verify actual wire encoding with supported-version captures.


Decision boundaries

Acceptance of this ADR means:

  • ClickHouse is the supported LDAP client; ch-oauth-ldap is not a general-purpose LDAP directory server.
  • Support is defined by documented ClickHouse LDAP compatibility profiles, not by every configuration or operation ClickHouse/libldap could theoretically produce.
  • We accept first-party ownership of the protocol and restricted DN security boundary for those profiles.
  • General-purpose LDAP server/client/BER-tree dependencies are removed from the production ch-oauth-ldap closure.
  • The production dependency boundary is enforced automatically in CI rather than left as a review convention.
  • golang.org/x/crypto/cryptobyte is the preferred primitive layer if supported ClickHouse wire evidence is compatible with it; otherwise a tiny bounded first-party BER cursor is acceptable.
  • Small bounded ClickHouse LDAP changes may be added through normal reviewed product work when they preserve this architecture.
  • The ADR is revisited only when a requirement materially invalidates the “small compatibility profile” premise.

Acceptance does not choose exact file names, helper APIs, test names, fuzz target names, PR boundaries, or migration mechanics. Those belong to implementation issue #33.


Supported architectural profile

Bind

Support LDAPv3 simple Bind for documented user-DN shapes.

The Bind password is the OAuth/JWT bearer credential. The credential must be used only for verification and must not be retained after verification.

Connection-local authentication semantics remain:

UNAUTHENTICATED
  -- successful Bind --> AUTHENTICATED

AUTHENTICATED
  -- successful re-Bind --> AUTHENTICATED with replaced state
  -- failed re-Bind     --> UNAUTHENTICATED
  -- close / Unbind     --> CLOSED

A failed re-Bind must not leave the prior identity authenticated.

Search

Support only documented, bounded Search profiles.

For the current role-mapping configuration, the security-relevant filter shape is structurally equivalent to:

AND
 ├── objectClass == groupOfNames
 └── member == currentBoundDN

There is no architectural requirement for a general recursive LDAP filter parser/evaluator.

Role results remain a snapshot of the successfully bound identity. Search size-limit overflow must fail closed: ClickHouse must not silently accept a successfully truncated role set.

Abandon

A valid AbandonRequest is part of the supported ClickHouse/libldap profile because libldap emits it when a synchronous Search times out.

The helper recognizes it and sends no response, as required by LDAP semantics.

This decision does not justify reintroducing generic request registries, per-request goroutines, Cancel/Abandon scheduling capacity, or a concurrent control plane. The helper intentionally retains a simple bounded connection model rather than emulating a general LDAP server's asynchronous operation machinery.

Unbind

Unbind closes the connection.

Unsupported or malformed requests

Malformed framing fails closed by terminating the connection.

For a recognizable out-of-profile operation, a small common LDAP error response is acceptable only if it does not require expanding the decoder into operation-specific generic LDAP handling. Otherwise the connection is closed.

Unsupported requests must never invoke OAuth verification or role logic.


DN and filter security semantics

Removing go-ldap from production means the server owns only the DN behavior required by documented ClickHouse profiles. This is not a commitment to implement arbitrary RFC 4514 normalization.

The important authorization property is that the Search membership assertion identifies the same principal that successfully bound on the connection.

ClickHouse/libldap's escaping pipeline is deterministic:

username
  -> ClickHouse DN escaping
  -> Bind DN text
  -> ClickHouse filter escaping
  -> libldap parses the textual filter escape syntax
  -> BER equalityMatch contains DN text
  -> helper decodes the supported DN form
  -> structural comparison with the authenticated BoundDN

For example, a username containing a comma can produce a Bind DN containing \,; when inserted into the textual Search filter the backslash is represented as \5C, but libldap converts that filter escape before BER encoding. The helper receives the DN assertion value, not the original textual RFC 4515 escape sequence.

Therefore the implementation must preserve structural DN comparison under the supported grammar and must not replace it with loose string suffix/split checks.

If a future documented profile genuinely requires substantially broader DN semantics, that requirement should be reviewed for whether the minimal architecture still holds.


Dependency and supply-chain boundary

The decision is about the actual built production command, not merely go.mod.

After migration, the production closure of ./cmd/ch-oauth-ldap must not contain the current general LDAP stack:

github.com/vjeantet/ldapserver
github.com/vjeantet/goldap
github.com/go-ldap/ldap/v3
github.com/go-asn1-ber/asn1-ber
github.com/Azure/go-ntlmssp

unless a listed transitive module remains for an unrelated production reason that is explicitly reviewed.

Nor should the implementation replace these with another general-purpose LDAP server/client or generic BER packet-tree library under a different name.

This is not a claim that “all external parsing code is unsafe.” A small primitive package such as cryptobyte is acceptable when it materially reduces local binary-parser responsibility without reintroducing a general LDAP stack.

Independent LDAP clients may remain test-only provided they are outside the released command's production dependency closure.


Resource and credential-handling invariants

The replacement must preserve or improve the existing explicit resource bounds:

  • at most 256 admitted LDAP connections;
  • at most 64 KiB LDAP message body before body allocation;
  • 30 second read deadline;
  • 30 second write deadline.

The corresponding named pre-authentication message-body bound is:

256 * 64 KiB = 16 MiB

This is a bound on simultaneously admitted LDAP message-body buffers, not a claim about total process RSS.

The implementation must avoid unbounded parser recursion, attacker-controlled large pre-allocation, unbounded per-client concurrency, and proportional duplicate copies that defeat this model.

Credential-bearing Bind payloads, raw LDAP packets, filter/member values, and other attacker-controlled credential-adjacent data must not be logged. Existing redaction/release security guarantees must remain in force after the dependency rewrite.


Primitive-layer decision

The decision is not “hand-write BER.”

The decision is to own the small LDAP profile and use the smallest safe primitive layer compatible with real ClickHouse traffic.

Preferred path:

supported ClickHouse/libldap captures are accepted by cryptobyte
    -> use cryptobyte + first-party LDAP profile

valid supported capture requires a BER form cryptobyte rejects
    -> use a tiny bounded first-party BER cursor

OpenLDAP's LBER_USE_DER initialization makes the first branch the expected outcome, but the repository must verify that expectation against supported ClickHouse versions before relying on it.

If a local BER cursor is required, it must remain deliberately non-general: bounded definite-length framing and only the primitive forms needed by the supported profile, with no reflection-based generic ASN.1 decoding or arbitrary packet tree.


Product compatibility and future changes

A valid LDAP configuration, or a valid ClickHouse LDAP configuration, is not automatically part of this product's support contract.

New requirements may extend the documented compatibility profile through normal PRs when they remain bounded. Examples might include another fixed Search shape, another requested attribute, another documented scope, or another simple Bind/Search sequence.

Revisit this ADR when a requirement instead pushes toward capabilities such as:

  • arbitrary LDAP clients;
  • a general recursive filter engine;
  • broad SASL support;
  • arbitrary controls/extensions;
  • general Add/Modify/Delete/Compare operations;
  • directory persistence;
  • complex asynchronous request scheduling;
  • general Active Directory / LDAP server emulation.

The distinction is intentional:

new product requirement
    -> review scope and security impact
    -> add narrowly if architecture still holds

valid LDAP/ClickHouse feature exists
    -> does not by itself create a support obligation

Relationship to #31 standalone LDAPS mode

Issue #31's goal is deployment portability: a supported ClickHouse instance outside the current in-cluster environment can use the helper over LDAPS/TLS.

“Any ClickHouse” in that context means any supported ClickHouse deployment using a documented ch-oauth-ldap compatibility profile. It must not be interpreted as a promise to support arbitrary customer-authored ClickHouse LDAP configuration.

Generic tools such as ldapsearch may be useful for best-effort troubleshooting, but they are not supported LDAP clients and may issue operations outside the profile.

#31 should remain consistent with this product boundary.


Consequences

Benefits
  • materially smaller unauthenticated protocol surface;
  • fewer independent LDAP/authentication/parser dependencies in the credential-bearing production path;
  • removal of locally patched generic LDAP forks and their rebase/update burden;
  • resource and credential-handling invariants become reviewable in a small first-party boundary;
  • compatibility work follows a small, inspectable ClickHouse/libldap source path rather than generic LDAP semantics.
Costs and risks
  • the project permanently owns the security of the supported parser/profile boundary;
  • malformed-input testing, fuzzing, interoperability testing, dependency-contract enforcement, and security response become ongoing responsibilities;
  • replacing mature generic code with local parsing can introduce new bugs if scope discipline or testing is weak;
  • supported ClickHouse upgrades require review of relevant ClickHouse/libldap behavior rather than assuming the wire profile is immutable.

The strongest objection to this decision is therefore valid: custom unauthenticated parsing can be more dangerous than a mature dependency. This ADR accepts that responsibility only because the required profile is unusually small, the current generic dependencies already require downstream security ownership, and the replacement is constrained by explicit resource, dependency, compatibility, and testing gates.


Alternatives considered

Keep the current patched forks

Safe fallback and already tested, but preserves a much larger remotely reachable protocol surface and ongoing downstream fork/rebase burden. It also does not remove the production go-ldap dependency chain.

Publish/maintain official Altinity forks

Improves provenance compared with in-tree forks, but adds repositories/releases/credentials without reducing the generic protocol surface.

Return to unmodified upstream libraries

Rejected under the current evidence because it would discard hardening already required by adversarial tests.

Use another general-purpose Go LDAP server

Does not solve the architecture mismatch; it mainly substitutes one broad dependency surface for another.

Put OpenLDAP/389DS in front of the helper

Provides mature LDAP/TLS behavior and process isolation, but introduces a substantially larger daemon/configuration/update/failure surface for a compatibility layer that needs very little LDAP behavior. This remains a reasonable alternative if owning any parser is later judged unacceptable.

Own the profile with local BER primitives

Viable fallback if real libldap traffic is incompatible with cryptobyte, but carries more first-party binary-parser responsibility.

Own the profile with cryptobyte

Preferred if supported captures confirm compatibility: keeps LDAP semantics first-party while delegating low-level bounded ASN.1 primitive handling to a small, well-maintained package rather than a general LDAP stack.


Implementation and acceptance

Implementation, migration phases, concrete test/fuzz targets, fixture layout, dependency-contract mechanics, scope/LOC guardrails, cutover, and rollback are tracked in #33.

#23 (or an equivalent required PR-time gate) is a prerequisite for merging production parser replacement work.

The ADR should be accepted or rejected based on the architecture above, not on approval of particular file names or PR slicing in #33.

Related: #16 (original architecture), #19 (current implementation), #23 (required PR CI), #30 (ClickHouse version compatibility), #31 (LDAPS/TLS deployment mode), #33 (implementation tracker).

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read the accepted ADR and compare its decision boundaries with implementation issue #33. Verify that the documented ClickHouse LDAP compatibility profile, dependency boundary, and resource and credential-handling invariants are recorded consistently; implementation mechanics belong in #33.

Written by the indexing model from the issue text.

Assessment

Tech stack
clickhouse, go
Domain
backend, documentation, security
Issue type
Documentation
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.