diff --git a/changelog.d/3010.security.md b/changelog.d/3010.security.md index 705de7254..d7f916ba1 100644 --- a/changelog.d/3010.security.md +++ b/changelog.d/3010.security.md @@ -15,10 +15,11 @@ **The nested group search is paged**, at 500 entries per page. So an account in more groups than a directory's unpaged cap no longer fails sign-in with sizeLimitExceeded. AD's cap, MaxPageSize, is 1000 by default. A page that does not end with success refuses the whole search. So does a - later page that drops the paging control, and a search still paging after 20 pages. The pages - already read are never used as the whole answer. A refused paged search is abandoned (RFC 2696) - once the directory has sent a cookie, at the cost of one more round trip. Paging was measured - against a loopback server through ldap3, not against a domain controller. + later page that drops the paging control, and a search past its paging bounds + (`BACKLOG #3696`). The pages already read are never used as the whole answer. A refused paged + search is abandoned (RFC 2696) once the directory has sent a cookie, at the cost of one more + round trip. Paging was measured against a loopback server through ldap3, not against a domain + controller. **A response ldap3 cannot decode is a directory error, not an unhandled error.** ldap3's default decoder raises `KeyError`, `TypeError`, `IndexError`, `AttributeError` or `UnicodeError` on a malformed response. Cases include at least a result code outside its table, a malformed paged diff --git a/changelog.d/3696.changed.md b/changelog.d/3696.changed.md new file mode 100644 index 000000000..837b4f762 --- /dev/null +++ b/changelog.d/3696.changed.md @@ -0,0 +1,13 @@ +- **A paged LDAP group search has one overall deadline, and its cap counts groups, not pages.** + All the pages of the nested group search now share one deadline, the new + `[auth].ad_group_search_deadline`, 120 s by default. Each page's reads stay bounded by + `[auth].ad_receive_timeout`. Before, each page could take the whole receive timeout. It is not + a hard wall-clock bound. A search still paging at the deadline is refused as the directory not + answering. The search is now refused past 10,000 groups at any page size. Before, it was + refused past 20 pages, which is 2,000 groups at 100 a page. Both refusals warn. **The session + reconciler pages, and audits `auth.ad_reconcile_paging_refused`, for an account either refusal + leaves unjudged**, once as the account enters that state rather than on every pass. A disable or + deletion still revokes such an account; its role and scope re-diff does not run. A plain + directory outage still pages nobody. A deadline above its default is a named loosening. The + limits are in [CONNECTIONS.md](../docs/CONNECTIONS.md), the Active Directory rows of Table B. + (vault `BACKLOG #3696` items 3 and 4) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index c6846e7fe..d96423001 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -755,13 +755,14 @@ document ([SECURITY-DOCS-POLICY.md](SECURITY-DOCS-POLICY.md)). | `ad_bind_dn` | str | — | service-account DN used for lookups. It must be able to read each user's `userAccountControl`: an account whose value it cannot read is refused at sign-in (BACKLOG #1639). The session reconciler reads it as **undetermined**, not absent. One undetermined answer strikes the way a disabled account does, but only when it is the only one the reconciler knows of across its rotation, the same pass read the attribute on another account, and no earlier wave has latched the hold. Otherwise the undetermined accounts are held: their sessions are not revoked and their strikes reset. Two or more known at once latch the hold, which then holds even a single one until none remains. The rest of the pass goes ahead, subject to the mass-revoke breaker ([ADR 0195](adr/0195-brake-the-ad-session-reconciler-on-an-undetermined-useraccountcontrol-wave.md)). | | `ad_bind_password` | secret | — | supply via env (`MEFOR_AUTH_AD_BIND_PASSWORD`), or use `ad_bind_password_secret`. A value in the file is still accepted, with a WARNING at load; keep it out of the file | | `ad_bind_password_secret` | str | — | connector `SecretProvider` reference (ADR 0019 §5) — when set and `[secrets].provider` is configured, the bind password is resolved from that backend (e.g. a Vault KV `path#field`) instead of `ad_bind_password`. A reference, not a secret. | -| `ad_use_nested_groups` | bool | `true` | resolve nested groups (`LDAP_MATCHING_RULE_IN_CHAIN`) | +| `ad_use_nested_groups` | bool | `true` | resolve nested groups (`LDAP_MATCHING_RULE_IN_CHAIN`). The search is paged, with a deadline and a cap on groups; [CONNECTIONS.md](CONNECTIONS.md#table-b--per-service-resource-strategy-asvs-1313), the Active Directory row, gives its bounds | | `ad_tls_verify` | bool | `true` | validate the LDAPS certificate. `false` is refused at start, with no escape, because every bind carries a password ([why](SECURITY-LOOSENING.md#no-escape-sends-a-credential-to-an-unverified-peer)). For a private CA, set `ad_tls_ca_cert_file` | | `ad_tls_ca_cert_file` | str | — | trust an internal CA for LDAPS without disabling verification. Read once at start, and every LDAPS bind trusts those bytes. **A changed file takes effect at the next restart.** The PEM rules, and what a reload does, are under `[api].tls_client_ca_file` (BACKLOG #2185) | | `ad_tls_ca_cert_pin` | str | — | optional lowercase-hex SHA-256 pin over the corresponding CA anchor PEM (`ad_tls_ca_cert_file`); a mismatch refuses at load + reload (ASVS 6.7.1); unset = no pin (dormant); set but empty or whitespace refuses at load | | `ad_allow_insecure_ldap` | bool | `false` | explicit opt-in to a non-`ldaps://` bind (trusted-network dev only). Inert under `[security].enforcement = enforce`; see its [SECURITY-LOOSENING.md](SECURITY-LOOSENING.md) entry | | `ad_connect_timeout` | float | `10.0` | seconds — bounds the LDAP/LDAPS **TCP connect** on every `ldap3` `Server` the authenticator builds (ASVS 13.1.3). Must be finite and `> 0`; `0`, negative, `inf` and `NaN` are refused at config load, and so is anything above `3600`. `ldap3`'s own default is `None` (wait forever), so without this an unresponsive DC pinned a thread-pool worker indefinitely | | `ad_receive_timeout` | float | `10.0` | seconds — bounds **each socket receive** during an LDAP response (both binds and every search) on every `ldap3` `Connection`. Same validation, up to `3600`. The engine rounds it up to whole seconds before handing it to `ldap3`, so `9.25` acts as `10` | +| `ad_group_search_deadline` | float | `120.0` | seconds — the one deadline all the pages of the nested group search share (vault BACKLOG #3696). Each page's reads are still bounded by `ad_receive_timeout`, so a directory that stops answering fails at that, unless less than that is left of the deadline; this refuses a search whose pages keep arriving. The first page is never cut short by it. Same validation, up to `3600`, and a value above `120` is a [loosening](SECURITY-LOOSENING.md#authad_connect_timeout-ad_receive_timeout-or-ad_group_search_deadline-above-its-default-with-ad_enabled--a-stalled-directory-call-holds-a-thread-longer). It is not a hard wall-clock bound. A search refused at it fails that directory call, at least a sign-in, a step-up or a session recheck, and the session reconciler pages once for that account; [CONNECTIONS.md](CONNECTIONS.md#table-b--per-service-resource-strategy-asvs-1313), the Active Directory rows, says what it covers and what it does not | | `ad_session_recheck_seconds` | int | `300` | **Directory session reconciliation** ([ADR 0079](adr/0079-kerberos-idp-session-coordination.md) mechanism 2). How often to re-resolve directory principals holding **live** sessions and revoke those AD has disabled or deleted — without it, an AD disable does not take effect until the `[security].max_session_hours` cap (12 h). **`300` (five minutes) is the default** (ADR 0148 GIVEN 1 — the hardened path is the shipped path), floored at **60 s** (a pass costs one LDAP bind per signed-in directory user). `0` disables the loop and is a **loosening** once AD is on — `security_loosenings()` names it. The default is **inert without AD** (`should_reconcile()` also needs an LDAP client), so a non-AD deployment is unaffected; an **explicit** non-zero value without `ad_enabled` is still refused rather than left silently dead. | | `ad_session_recheck_strikes` | int | `2` | Consecutive passes a principal must fail to resolve before its sessions are revoked. *The search matched nothing* cannot tell *deleted* from *moved out of the search base*, so a single ambiguous result must never revoke; a set disabled bit and an unreadable `userAccountControl` strike the same way. A wave of unreadable answers is held instead of revoked, with no setting ([ADR 0195](adr/0195-brake-the-ad-session-reconciler-on-an-undetermined-useraccountcontrol-wave.md)). Range 1–10. | | `ad_session_recheck_max_users` | int | `200` | Per-pass bind budget. Beyond this, remaining users are picked up by later passes (least-recently-probed first), so a large estate degrades to a longer effective interval instead of a bind storm. A pass revokes no more accounts than it probed. So at or below `ad_session_revoke_max`, the breaker can never trip. With AD on and a non-zero interval, the engine then logs a **WARNING** at start that names both settings. It still loads, and revocation still works. To arm the breaker, raise this budget above the cap. | @@ -862,6 +863,11 @@ source `directory-reconciler-probe-fault` (BACKLOG #3100). The reconciler raises which a probe fails to read an entry. Nothing resolves it automatically. Resolve it by hand once the WARNING lines that name each faulting account stop. +A paging refusal opens one `ad_reconcile_aborted` instance per account, under the source +`directory-reconciler-paging:` (vault BACKLOG #3696). Nothing resolves it automatically. +Resolve it by hand once that account no longer hits the bound, for example once its sign-in +succeeds again. + A pass judges only accounts that hold a session, and nothing reads an account again once it has left. An account leaves at least when it signs out, reaches the session cap, is revoked by the reconciler, is disabled locally, or is deleted, and when the hourly session reaper deletes an idle diff --git a/docs/CONNECTIONS.md b/docs/CONNECTIONS.md index 6fcb5238e..ae68d0656 100644 --- a/docs/CONNECTIONS.md +++ b/docs/CONNECTIONS.md @@ -4006,8 +4006,8 @@ reading this page already applies to a file the scan never opened. | Store — SQLite (`[store].backend = sqlite`) | `PRAGMA busy_timeout` 5000 ms on the writer and on every read-pool connection; **the pool borrow itself carries no timeout**; no network timeout applies | connections are closed on store close | a busy database retries inside the store layer | n/a | | Store — SQL Server (`[store].backend = sqlserver`) | three distinct bounds: `[store].connect_timeout` 15 s (**login**, passed as pyodbc `timeout=`), `[store].command_timeout` 30 s (**statement**, applied per acquire as the pyodbc connection attribute) and `[store].acquire_timeout` 30 s (the **pool borrow**); `[store].warm_pool_timeout` 15 s bounds the warm-up | every acquire releases back to the pool; the pool is closed on shutdown; a borrow the pool satisfies after the borrower gave up is released back rather than stranded | a driver error — or a `StoreAcquireTimeout` — propagates to the stage worker and the row stays claimable | stage handoffs re-run idempotently; `reset_stale_inflight` recovers on restart | | Store — Postgres (`[store].backend = postgres`) | `[store].connect_timeout` 15 s (`create_pool(timeout=…)`), `[store].command_timeout` 30 s as `asyncpg`'s per-statement bound, and `[store].acquire_timeout` 30 s on the message-pipeline pool borrows (see the coverage note above) | as SQL Server | as SQL Server | as SQL Server | -| Active Directory — login binds (`[auth].ad_server`) | `[auth].ad_connect_timeout` 10 s on the LDAP TCP connect and `[auth].ad_receive_timeout` 10 s on every LDAP response read — threaded into **every** `ldap3` `Server`/`Connection` construction | the service-account connection is context-managed; the user bind is unbound in a `finally`, so a **rejected** password releases it too (the common adversarial case) | fails closed with `LdapError`; the login is rejected and audited | **single-shot** — two binds, no retry loop | -| Active Directory — session reconciler (`ad_session_recheck_seconds`) | the same `ad_connect_timeout` / `ad_receive_timeout` | as the login path — context-managed connections | a pass that fails is retried on the next interval; strike state is process-local | **single-shot per pass**; `ad_session_recheck_strikes` (2) required before a revoke | +| Active Directory — login binds (`[auth].ad_server`) | `[auth].ad_connect_timeout` 10 s on the LDAP TCP connect and `[auth].ad_receive_timeout` 10 s on every LDAP response read — threaded into **every** `ldap3` `Server`/`Connection` construction. The nested group search is paged, and **all its pages share one `[auth].ad_group_search_deadline`** 120 s: each socket read of a page after the first waits at most the shorter of `ad_receive_timeout` and what was left when the page was asked for, no page is asked for once it is spent, and the abandon of a refused search gets only what is left (vault BACKLOG #3696). **It is not a hard wall-clock bound**: the cap is set once per page and applies to each read, so a page that arrives over several reads, each inside that cap, can hold the search up to the number of reads times what was left. The page size is the directory's choice, since it may ignore the 500 the engine asks for | the service-account connection is context-managed; the user bind is unbound in a `finally`, so a **rejected** password releases it too (the common adversarial case) | fails closed with `LdapError`; the login is rejected and audited, and so is a group search past its deadline or past 10,000 groups, whatever page size the directory returns (one WARNING per process for each) | **single-shot** — two binds, no retry loop; the group search's pages are one search, not retries | +| Active Directory — session reconciler (`ad_session_recheck_seconds`) | the same `ad_connect_timeout` / `ad_receive_timeout` / `ad_group_search_deadline` | as the login path — context-managed connections | a pass that fails is retried on the next interval; strike state is process-local. A group search refused for its deadline, its 10,000-group cap or its page backstop reads that account as unavailable on every pass. A disable or deletion still revokes it, because the user search runs first, but its roles and channel scope are not re-diffed while that lasts. So it pages, as `ad_reconcile_aborted` under the source `directory-reconciler-paging:`, and audits `auth.ad_reconcile_paging_refused`, once as the account enters that state, never on every pass. It pages again only after a pass judges the account, which an outage, a referral or a probe fault does not, or after a restart (vault BACKLOG #3696). A plain outage still pages nobody | **single-shot per pass**; `ad_session_recheck_strikes` (2) required before a revoke | | Kerberos / SPNEGO SSO (`kerberos_spn`) | **none engine-owned** — the OS provider owns any KDC timeout | the SPNEGO context is per-request | a failed step raises `LdapError` and audits a login reject | **single-shot**, single-leg — no NTLM fallback, no multi-leg handshake | | OIDC IdP — token endpoint (`oidc_token_endpoint`) | 10 s — `auth/oidc/flow.py`'s **own** `exchange_code(timeout=…)` default (`AuthService._oidc_exchange` passes none). The JWKS leg's 10 s is a **separate** literal, `oidc_http.DEFAULT_IDP_TIMEOUT_SECONDS`; the two coincide but are independent | the response is context-managed; the body is size-capped | a non-200 or oversize body fails the login closed | **single-shot** — one POST per login | | OIDC IdP — JWKS fetch (`oidc_jwks_uri`) | 10 s — `oidc_http.DEFAULT_IDP_TIMEOUT_SECONDS`, the constant `jwks_fetcher` is the only consumer of | the response is context-managed; the size cap is enforced on the socket read | a fetch failure fails verification closed | **single-shot**, further damped by the refetch floor | diff --git a/docs/PHI.md b/docs/PHI.md index 0d7209a0c..b655daf34 100644 --- a/docs/PHI.md +++ b/docs/PHI.md @@ -1820,7 +1820,7 @@ with materially different PHI profiles, so they get their own rows; stream 4 is | **5. `audit_log` table** (SQLite, Postgres, SQL Server) | who / what / **where-from** / when of auth + PHI *access* and admin actions — plus, while `[security].audit_all_authorization_decisions` is on (**default `true`** since BACKLOG #1277, 2026-09-02, which reversed the ADR 0118 §5 `false`; the internal field it desugars to is `audit_all_authz`, whose old `[diagnostics]` TOML spelling is **refused at load** — ADR 0118), an `authz` row for **every** authorization decision including successes. That is the shipped volume of this stream, not an opt-in addition to it: one row per authenticated request on each `require()`-gated route, and this row's retention cell records that **nothing prunes the table** — `actor`, `action`, `channel_id`, `client`, `detail`, `row_hash` | JSON `detail`; **tamper-evident hash chain** over `prev_hash` + the row (the `client` address is **inside** the chained payload — ADR 0150) | the store database | HIPAA §164.312(b) audit controls; incident response; `verify_audit_chain` integrity checks | `GET /audit` requires **`audit:read`**; `GET /audit/export` requires the separate **`audit:export`** and streams CSV with formula-injection neutralisation, recording its own `audit.export` row *before* streaming; `GET /me/security-events` is a per-user view of the same table | **`[retention].audit_days` is reserved and NOT enforced — keep-forever by design** (the audit-retention requirement, ~6 years — **not** chain-breakage; [§8](#8-retention--purge) states the position and cites its source of record) | `detail` is stored **in the clear** (it is not a cipher-covered column): its protection is that writers only ever store filter shapes, counts and ids — never bodies or credentials — plus the store ACL and the volume layer | | **6. `message_events` table** | the per-message disposition timeline — the **complete** vocabulary is `received`, `routed`, `unrouted`, `filtered`, `transformed`, `delivered`, `failed`, `dead`, `error`, `replayed`, `resent`, `reingressed`, `passthrough`, `passthrough_dropped`, `cancelled`, `edit_resend`, `edit_resubmit`, `viewed`, `not_deployed`, the ADR 0154 synchronous-reply pair `reply_returned` / `reply_timeout` (names, counts and `waited_ms` only — **never** a fragment of the partner's reply body), and `reply_dropped`, a delivery whose 2xx reply body was over the byte bound and dropped (the status and the bound only, vault BACKLOG #3040) (CI asserts this list against the engine's own `MESSAGE_EVENT_KINDS`). `[diagnostics].message_events` can thin the set, but never below the compliance floor `viewed` / `dead` / `error` / `failed` / `not_deployed` / `reply_timeout` / `reply_dropped` | rows: `message_id`, `ts`, `event`, `destination`, `detail` | the store database | operator timeline on the message-detail view; the `viewed` row is the HIPAA PHI-access record | `GET /messages/{id}` under **`messages:view_raw`** + `require_phi_read`; the read itself writes a `viewed` event **and** a `message_view` audit row | no dedicated window — `purge_message_bodies` sets `message_events.detail` to `NULL` in the same transaction that blanks the body, so it inherits `[retention].messages_days` | `detail` is `safe_text()`-scrubbed **then** cipher-encrypted (AAD `("message_events","detail",message_id,ts,event)`). Verbosity gate `[diagnostics].message_events` = `all` (default) / `errors` / `off`, with a **compliance floor that can never be thinned**: `viewed`, `dead`, `error`, `failed`, `not_deployed`, `reply_timeout`, `reply_dropped` are retained at every level (`reply_timeout` is the one row that explains a "we called you and got a 504" complaint, so an instance that thinned its logs would lose exactly the record it is later asked for; `reply_dropped` is the one row that answers "you ignored our reply", for the same reason) | | **7. `connection_event` table — DEFAULT ON** (`[diagnostics].connection_events = true`) | transport/lifecycle events per connection: `established`, `closed` (reason `eof`, `idle_timeout` or `frame_deadline` — a fixed vocabulary chosen by the listener, never free text), `idle_timeout`, `at_capacity`, `peer_not_allowlisted`, `frame_oversize`, `framing_error`, `peer_reset`, the MLLP listener's `handler_error` (BACKLOG #1619 -- the inbound handler faulted on a frame the listener read cleanly, such as a store outage at the ingress commit; the reason is scrubbed at the source like every other), the MLLP and HTTP listeners' `tls_handshake_failed` (vault BACKLOG #2613 and #3302 -- an inbound TLS handshake failed, on any event loop; the first at once, then one per listener per minute carrying the count since the last. The reason is a count, an exception class name and OpenSSL's reason mnemonic, never the peer's bytes or certificate. The event names a peer only where the MLLP listener runs its own handshakes, which is the standard asyncio loop; where the loop runs them, which is uvloop and every HTTP listener, the peer is not known, and a handshake that only runs past its bound is not counted), the DATABASE poll source's `row_undecodable` (BACKLOG #1662 — a polled row the source cannot turn into a body; the reason is scrubbed at the source and names the exception type or the operator's own configured column, never a row value), the FILE poll source's quarantine kinds `file_oversize` / `file_decompress_failed` / `file_content_mismatch` / `file_scan_rejected` (BACKLOG #1621 — a drop moved to `.error`; the reason is a size, a content type or a scrubbed codec or scanner message, never the file name), the inbound-HTTP intake-auth refusals `intake_auth_failed` / `auth_subject_denied` / `auth_rate_limited` (ADR 0154 D6 — peer address and mode only; **never** the credential, a prefix of it, or its length. Each of these also writes a tamper-evident audit-log row — the copy that survives an operator turning this diagnostics stream off), plus the runner's `connection_lost` / `connection_restored`. That is the whole vocabulary, asserted in CI against the literal emit call sites in `transports/` and the pipeline runner **and** cross-checked against the console's own filter tuple. The MLLP, raw-TCP, X12 and HTTP listeners emit these; the **DATABASE poll source** emits `row_undecodable` and nothing else (it dials out, so there is no lifecycle to report and its rows carry a **NULL peer column**); the **FILE poll source** emits only its four quarantine kinds, also with a NULL peer column; the **DICOM inbound C-STORE SCP** emits none — the runner injects the sink onto **every** source (`wiring_runner.py`, over the base-class `on_connection_event` field), so that connector *has* the wiring and simply never calls it — so this stream covers those four listeners plus those poll-source events and the runner's outbound-lane transitions — not literally every connection. A DICOM association's connects and refusals are therefore **absent** from this stream. The **`ISA`/`IEA`-framed X12 inbound** was equally absent until BACKLOG #1665 and is not any more: it emits the same seven kinds as the raw-TCP listener | rows: `ts`, `connection`, `transport`, `direction`, `kind`, `peer_host`, `message_id` (correlation hint), `reason` | the store database, **all three backends** | Corepoint-style transport diagnostics — "did the sender connect, and why did it drop" | `GET /events` and `GET /connections/{name}/events` under **`monitoring:read`** (**not** a PHI permission) with per-channel RBAC — an out-of-scope `connection=` is 403'd *and* audited — server-clamped to ≤1000 rows. `reason` alone is gated on `messages:view_summary` and masked as `****` until the audited per-event `reveal=` act (BACKLOG #2443; the `connection_event.reason` row of §3's PL-2 block says how) | `[retention].connection_event_retention_hours` (its own **hours** window); 0 inherits `[retention].messages_days`; both 0 = keep forever. Plain age `DELETE` (metadata-only) | **`reason` is free text that can carry sensitive fragments.** Defended twice — `safe_exc()` at the source, `safe_text(reason, limit=200)` at the store (the bound §3's PL-2 block states) — then cipher-encrypted (AAD `("connection_event","reason",connection,ts,kind)`). Every other column is config metadata; the table is documented **metadata-only** — never a frame, body or HL7 field value. Writes are a pure side observer: a bounded in-memory queue drained by a background task outside any handoff transaction, so a flood can never block a listener or pin a message disposition | -| **8. `alert_instance` table — default on wherever an `[alerts]` notifier exists** | resolvable operator alerts: `connection_stopped`, `queue_buildup`, `lane_stuck`, `message_stall`, `saturation`, `connection_error`, `storage_threshold`, `cert_expiry`, `secret_rotation`, `initial_credential_expiring` (an UNCLAIMED admin-issued temporary password nearing the instant the login gate stops accepting it -- ASVS 6.4.5, BACKLOG #1141; keyed on `user:`, and the payload is the ISO deadline plus whole hours remaining, repeated in the reason column, never the password), `integrity_drift`, `update_available`, `backup_failed`, `store_privilege_warning` (the store privilege preflight found the store principal over-granted or could not read it -- BACKLOG #305; the payload is the finding, a count and the preflight's summary line, which names the login, the database and role names, never a password or message content), `leadership_acquired`, `dr_activated`, `log_write_failed` (an application-log sink was rolled after a write failure, or is UNWRITABLE and this process's connections were stopped -- BACKLOG #122, ADR 0162; the payload is the sink LABEL, the stage, a `safe_exc` reason and a count of connections stopped, never the record whose write failed), `log_forward_failed` (the off-box log forwarder is absent, losing records or not sending -- BACKLOG #2612; keyed on `forwarder:`, with `@node:` or `@shard:` appended when several engine processes share the store, and the payload is that key, fixed reason words and a count, never a record, the collector's address or an error text; an unpinned cluster node's `node_id` holds this engine's own host name and process id, so the key carries them), `gcm_invocations` (the per-key AES-GCM invocation bound crossing its 2^31 soft warn — ASVS 11.3.4; its payload carries a one-way `key_id` fingerprint plus counters, never key bytes), `approval_stale_requester` (a dual-control release refused because the requester no longer holds the authority the operation needs -- ASVS 8.3.2, BACKLOG #289; keyed on the approval id, and the payload is that id, the operation key and a closed-set reason slug, never the requester's name or the captured params), `approval_too_early` (a dual-control release refused because it arrived before `[approvals].min_dwell_seconds` -- ASVS 2.4.2, BACKLOG #287; keyed on `approval:`, and the payload is that key, the operation key and a fixed reason string, never the approver's or requester's name or the captured params), `audit_write_failed` (the approval gate could not write one of its audit rows -- vault BACKLOG #2255; keyed on `approval:`, or on `config_reload:inline` when an ungated config reload could not write its `config_reload_attempted` row -- vault BACKLOG #2254; the payload is that key and the lost row's audit action name, never the row's detail, a username or the captured params; the ERROR log line beside it holds the detail on some paths and only the exception on others), `ad_reconcile_aborted` (a directory reconciler pass left signed-in accounts unrevoked and needs an operator: the mass-revoke breaker tripped and nothing was revoked, or one or more probes were answered with an LDAP referral and those accounts were left unjudged while the rest of the pass was applied, BACKLOG #2538, or one or more probes raised a fault the engine could not read and those accounts were left unjudged the same way, BACKLOG #3100; the payload is a fixed source label, `directory-reconciler` for the breaker, `directory-reconciler-referral` for a referral and `directory-reconciler-probe-fault` for a probe fault so each is its own instance, the closed-set reason slug `mass_revoke_breaker`, `directory_referral` or `probe_fault`, the probed count and the latched operator explanation, never a username), `ad_reconcile_held` (the reconciler is holding accounts whose `userAccountControl` it cannot read, ADR 0195; the payload is the same fixed source label, a closed-set reason slug, the count of undetermined accounts and the latched operator explanation, never a username), `ad_session_revoked` (the reconciler revoked a directory principal's sessions; keyed on that operator account's username, with a `directory_absent`, `directory_disabled`, `directory_undetermined`, `roles_changed` or `scope_changed` reason), `approval_approver_provenance` (a dual-control release, or the resolution of an interrupted one, went ahead with an approver or resolver account created, re-passworded or TOTP-enrolled after the request -- BACKLOG #315, #2125; keyed on `approval:`, or `approval::resolve` for a resolution, and the payload is the operation key and the closed-set change slugs, never a username or the captured params), `administrator_granted` (the built-in Administrator role was granted by account creation, a role change, a directory group newly mapped to it -- BACKLOG #315 -- or a directory sign-in whose role sync newly gave the account the role -- vault BACKLOG #2610; keyed on `user:` or `ad-group:`, with the grant route and the granting administrator's username, or `` for a directory sign-in), `no_enabled_administrator` (a directory sign-in's role sync or the session reconciler took Administrator from its last enabled holder -- vault BACKLOG #3229; keyed on `user:`, and the payload is `via`, `ad-sync` or `ad-reconcile`, and a fixed reason sentence, never message content), `intake_paused` (the engine paused intake because the staged backlog went over `[inbound].max_staged_depth` or the SQLite volume fell below `[retention].min_free_disk_mb` -- BACKLOG #290; keyed on `intake:staged_depth` or `intake:disk_floor`, with `@node:` or `@shard:` appended when several engine processes share the store (BACKLOG #2272), and the payload is the reason, a count or a MiB figure, its limit and the store backend's name, never message content), `config_changed` (a start loaded a config whose fingerprint differs from the store's baseline -- vault BACKLOG #2597; keyed on `config:`, and the payload is the two digests, the node and engine shard labels, and the action, actor username and time of the row the older digest came from, never the config directory, a git commit or message content), `crl_reload_refused` (the CRL reload refused a replaced CRL file, so running TLS contexts keep the copy they hold -- vault BACKLOG #2904; keyed on `crl_reload:`, with `@node:` or `@shard:` appended when several engine processes share the store, and the payload is that key, the file's path, a short class token for the remedy, a count of contexts, the engine's own fixed remedy sentence and, when a start would refuse the file too, a `start_refuses` flag, never the refusal's reason, which quotes CRL issuer names and dates; that reason is in the application log, and in the `detail` of the CRL expiry alert once the held copy nears its `nextUpdate`), `remote_debug_refused` (the engine's remote-debugging guard refused scripts that another process injected into the engine process -- vault BACKLOG #2742; keyed on `remote_debug:refused`, with `@node:` or `@shard:` appended when several engine processes share the store, and the payload is that key, the count of scripts refused since the alert was last raised, the count since the process started and a fixed sentence built from the two, never a script's file name, which the injecting process chooses; that name is in the application log, in the guard's WARNING line), `sandbox_worker_refused` (a `[sandbox].mode = "subprocess"` worker was refused because the graph it loaded differs from the engine's, most often a config file edited on disk with no reload -- vault BACKLOG #2724; keyed on the inbound's name, raised once per refusal, and the payload is that name and a fixed sentence naming the cause and the remedy, never a message body or the refusal text, which names config names and digests and is in the application log), and the six security signals of the audit-stream rule layer -- vault BACKLOG #2613: `signin_failure_burst` (keyed on `signin:`, never the typed username), `access_denied_burst`, `body_view_burst` and `bulk_export` (each keyed on `account:`), `log_level_debug` (keyed on `logging:debug`) and `posture_loosened` (keyed on `posture:start`); the payload is a count and a fixed sentence holding a level name, switch names or a username, never a message body, a message id or an audit row's detail. The reachable **inverse** signals — `connection_restored`, `leadership_lost`, `dr_released`, `store_privilege_clean`, `intake_resumed`, `crl_reload_applied` (it resolves `crl_reload_refused` under the same key, so for the same file and the same engine identity, at least once the reload's record no longer holds the refusal, once no hop holds any CRL, and when the reload stops -- vault BACKLOG #2904; it is also raised once per held file outside a CA bundle, at the first pass that judges it with no refusal standing, whether or not an alert is open, so it can resolve nothing, and it then clears one an earlier run under that identity left open -- vault BACKLOG #3543. An alert for a file no hop holds now stays open, and so does one an unpinned cluster node raised, because that node's identity changes on every start; [CONFIGURATION.md](CONFIGURATION.md), *The alert resolves itself*, is the source of record), `ad_reconcile_breaker_cleared`, `ad_reconcile_hold_released` (the last two resolve `ad_reconcile_aborted`, under the breaker's or the referral's source but never the probe fault's, which has no inverse, and `ad_reconcile_held`; when they do, and where they never do, is stated once in [CONFIGURATION.md](CONFIGURATION.md#when-the-reconcilers-alerts-resolve-themselves) -- BACKLOG #2136) — are never rows here: `_record_state` routes an inverse through `_AUTO_RESOLVE` to `resolve_alert_instances_for`, never to `upsert_alert_instance`. (One more mapped key, `connection_started`, is emitted by no code path today.) | rows: `event_type`, `connection`, `severity`, `status`, `first_seen`, `last_seen`, `count`, `reason`, `acked_by`, `acked_at`, `resolved_at`, `suspended_until`, `escalation_tier` | the store database, **all three backends** | the operator alert list — acknowledge / resolve / suspend. Durable state is recorded **before** any suppression or throttle return, so a muted alert still leaves a record | `GET /alerts/active` under **`monitoring:diagnose`** (**not** a PHI permission) with the same per-channel scope, and `reason` alone gated on `messages:view_summary` and masked as `****` until the audited per-alert `reveal=` act (BACKLOG #2443; the `alert_instance.reason` row of §3's PL-2 block says how); ack/resolve/suspend/**resume** are POSTs on the same tier, and the separate read-only `GET /alerts/rules` view sits on its own gate | shares the connection-event window; **only RESOLVED instances are DELETEd**, by `resolved_at` — an open or acknowledged condition is never aged out from under an operator | **`reason` is free text** taken from the event's `detail`/`reason`: `safe_text(reason, limit=200)` (the bound §3's PL-2 block states) then cipher-encrypted (AAD `("alert_instance","reason",event_type,connection)` — the de-dup grain, so one AAD covers both the INSERT and the re-fire UPDATE). | +| **8. `alert_instance` table — default on wherever an `[alerts]` notifier exists** | resolvable operator alerts: `connection_stopped`, `queue_buildup`, `lane_stuck`, `message_stall`, `saturation`, `connection_error`, `storage_threshold`, `cert_expiry`, `secret_rotation`, `initial_credential_expiring` (an UNCLAIMED admin-issued temporary password nearing the instant the login gate stops accepting it -- ASVS 6.4.5, BACKLOG #1141; keyed on `user:`, and the payload is the ISO deadline plus whole hours remaining, repeated in the reason column, never the password), `integrity_drift`, `update_available`, `backup_failed`, `store_privilege_warning` (the store privilege preflight found the store principal over-granted or could not read it -- BACKLOG #305; the payload is the finding, a count and the preflight's summary line, which names the login, the database and role names, never a password or message content), `leadership_acquired`, `dr_activated`, `log_write_failed` (an application-log sink was rolled after a write failure, or is UNWRITABLE and this process's connections were stopped -- BACKLOG #122, ADR 0162; the payload is the sink LABEL, the stage, a `safe_exc` reason and a count of connections stopped, never the record whose write failed), `log_forward_failed` (the off-box log forwarder is absent, losing records or not sending -- BACKLOG #2612; keyed on `forwarder:`, with `@node:` or `@shard:` appended when several engine processes share the store, and the payload is that key, fixed reason words and a count, never a record, the collector's address or an error text; an unpinned cluster node's `node_id` holds this engine's own host name and process id, so the key carries them), `gcm_invocations` (the per-key AES-GCM invocation bound crossing its 2^31 soft warn — ASVS 11.3.4; its payload carries a one-way `key_id` fingerprint plus counters, never key bytes), `approval_stale_requester` (a dual-control release refused because the requester no longer holds the authority the operation needs -- ASVS 8.3.2, BACKLOG #289; keyed on the approval id, and the payload is that id, the operation key and a closed-set reason slug, never the requester's name or the captured params), `approval_too_early` (a dual-control release refused because it arrived before `[approvals].min_dwell_seconds` -- ASVS 2.4.2, BACKLOG #287; keyed on `approval:`, and the payload is that key, the operation key and a fixed reason string, never the approver's or requester's name or the captured params), `audit_write_failed` (the approval gate could not write one of its audit rows -- vault BACKLOG #2255; keyed on `approval:`, or on `config_reload:inline` when an ungated config reload could not write its `config_reload_attempted` row -- vault BACKLOG #2254; the payload is that key and the lost row's audit action name, never the row's detail, a username or the captured params; the ERROR log line beside it holds the detail on some paths and only the exception on others), `ad_reconcile_aborted` (a directory reconciler pass left signed-in accounts unrevoked and needs an operator: the mass-revoke breaker tripped and nothing was revoked, or one or more probes were answered with an LDAP referral and those accounts were left unjudged while the rest of the pass was applied, BACKLOG #2538, or one or more probes raised a fault the engine could not read and those accounts were left unjudged the same way, BACKLOG #3100, or an account's group search was refused for the engine's own deadline or cap, so a role or scope change in the directory does not revoke that account's sessions, though a disable or deletion still does, vault BACKLOG #3696; for the breaker, a referral and a probe fault the payload is a fixed source label, `directory-reconciler` for the breaker, `directory-reconciler-referral` for a referral and `directory-reconciler-probe-fault` for a probe fault so each is its own instance, the closed-set reason slug `mass_revoke_breaker`, `directory_referral` or `probe_fault`, the probed count and the latched operator explanation, never a username; a paging refusal is instead keyed on that operator account's username, as `directory-reconciler-paging:`, so each account is its own instance, raised as the account enters that state and again after a restart while it stands, and the payload is that key, the closed-set reason slug `group_search_deadline` or `group_search_cap`, the probed count and a fixed sentence naming the cause, never anything else about the account), `ad_reconcile_held` (the reconciler is holding accounts whose `userAccountControl` it cannot read, ADR 0195; the payload is the same fixed source label, a closed-set reason slug, the count of undetermined accounts and the latched operator explanation, never a username), `ad_session_revoked` (the reconciler revoked a directory principal's sessions; keyed on that operator account's username, with a `directory_absent`, `directory_disabled`, `directory_undetermined`, `roles_changed` or `scope_changed` reason), `approval_approver_provenance` (a dual-control release, or the resolution of an interrupted one, went ahead with an approver or resolver account created, re-passworded or TOTP-enrolled after the request -- BACKLOG #315, #2125; keyed on `approval:`, or `approval::resolve` for a resolution, and the payload is the operation key and the closed-set change slugs, never a username or the captured params), `administrator_granted` (the built-in Administrator role was granted by account creation, a role change, a directory group newly mapped to it -- BACKLOG #315 -- or a directory sign-in whose role sync newly gave the account the role -- vault BACKLOG #2610; keyed on `user:` or `ad-group:`, with the grant route and the granting administrator's username, or `` for a directory sign-in), `no_enabled_administrator` (a directory sign-in's role sync or the session reconciler took Administrator from its last enabled holder -- vault BACKLOG #3229; keyed on `user:`, and the payload is `via`, `ad-sync` or `ad-reconcile`, and a fixed reason sentence, never message content), `intake_paused` (the engine paused intake because the staged backlog went over `[inbound].max_staged_depth` or the SQLite volume fell below `[retention].min_free_disk_mb` -- BACKLOG #290; keyed on `intake:staged_depth` or `intake:disk_floor`, with `@node:` or `@shard:` appended when several engine processes share the store (BACKLOG #2272), and the payload is the reason, a count or a MiB figure, its limit and the store backend's name, never message content), `config_changed` (a start loaded a config whose fingerprint differs from the store's baseline -- vault BACKLOG #2597; keyed on `config:`, and the payload is the two digests, the node and engine shard labels, and the action, actor username and time of the row the older digest came from, never the config directory, a git commit or message content), `crl_reload_refused` (the CRL reload refused a replaced CRL file, so running TLS contexts keep the copy they hold -- vault BACKLOG #2904; keyed on `crl_reload:`, with `@node:` or `@shard:` appended when several engine processes share the store, and the payload is that key, the file's path, a short class token for the remedy, a count of contexts, the engine's own fixed remedy sentence and, when a start would refuse the file too, a `start_refuses` flag, never the refusal's reason, which quotes CRL issuer names and dates; that reason is in the application log, and in the `detail` of the CRL expiry alert once the held copy nears its `nextUpdate`), `remote_debug_refused` (the engine's remote-debugging guard refused scripts that another process injected into the engine process -- vault BACKLOG #2742; keyed on `remote_debug:refused`, with `@node:` or `@shard:` appended when several engine processes share the store, and the payload is that key, the count of scripts refused since the alert was last raised, the count since the process started and a fixed sentence built from the two, never a script's file name, which the injecting process chooses; that name is in the application log, in the guard's WARNING line), `sandbox_worker_refused` (a `[sandbox].mode = "subprocess"` worker was refused because the graph it loaded differs from the engine's, most often a config file edited on disk with no reload -- vault BACKLOG #2724; keyed on the inbound's name, raised once per refusal, and the payload is that name and a fixed sentence naming the cause and the remedy, never a message body or the refusal text, which names config names and digests and is in the application log), and the six security signals of the audit-stream rule layer -- vault BACKLOG #2613: `signin_failure_burst` (keyed on `signin:`, never the typed username), `access_denied_burst`, `body_view_burst` and `bulk_export` (each keyed on `account:`), `log_level_debug` (keyed on `logging:debug`) and `posture_loosened` (keyed on `posture:start`); the payload is a count and a fixed sentence holding a level name, switch names or a username, never a message body, a message id or an audit row's detail. The reachable **inverse** signals — `connection_restored`, `leadership_lost`, `dr_released`, `store_privilege_clean`, `intake_resumed`, `crl_reload_applied` (it resolves `crl_reload_refused` under the same key, so for the same file and the same engine identity, at least once the reload's record no longer holds the refusal, once no hop holds any CRL, and when the reload stops -- vault BACKLOG #2904; it is also raised once per held file outside a CA bundle, at the first pass that judges it with no refusal standing, whether or not an alert is open, so it can resolve nothing, and it then clears one an earlier run under that identity left open -- vault BACKLOG #3543. An alert for a file no hop holds now stays open, and so does one an unpinned cluster node raised, because that node's identity changes on every start; [CONFIGURATION.md](CONFIGURATION.md), *The alert resolves itself*, is the source of record), `ad_reconcile_breaker_cleared`, `ad_reconcile_hold_released` (the last two resolve `ad_reconcile_held` and `ad_reconcile_aborted`, the latter only under the breaker's or the referral's source: the probe fault's and a paging refusal's instances have no inverse; when they do, and where they never do, is stated once in [CONFIGURATION.md](CONFIGURATION.md#when-the-reconcilers-alerts-resolve-themselves) -- BACKLOG #2136) — are never rows here: `_record_state` routes an inverse through `_AUTO_RESOLVE` to `resolve_alert_instances_for`, never to `upsert_alert_instance`. (One more mapped key, `connection_started`, is emitted by no code path today.) | rows: `event_type`, `connection`, `severity`, `status`, `first_seen`, `last_seen`, `count`, `reason`, `acked_by`, `acked_at`, `resolved_at`, `suspended_until`, `escalation_tier` | the store database, **all three backends** | the operator alert list — acknowledge / resolve / suspend. Durable state is recorded **before** any suppression or throttle return, so a muted alert still leaves a record | `GET /alerts/active` under **`monitoring:diagnose`** (**not** a PHI permission) with the same per-channel scope, and `reason` alone gated on `messages:view_summary` and masked as `****` until the audited per-alert `reveal=` act (BACKLOG #2443; the `alert_instance.reason` row of §3's PL-2 block says how); ack/resolve/suspend/**resume** are POSTs on the same tier, and the separate read-only `GET /alerts/rules` view sits on its own gate | shares the connection-event window; **only RESOLVED instances are DELETEd**, by `resolved_at` — an open or acknowledged condition is never aged out from under an operator | **`reason` is free text** taken from the event's `detail`/`reason`: `safe_text(reason, limit=200)` (the bound §3's PL-2 block states) then cipher-encrypted (AAD `("alert_instance","reason",event_type,connection)` — the de-dup grain, so one AAD covers both the INSERT and the re-fire UPDATE). | | **9. `response` rows with `kind='ack_sent'` — DEFAULT ON** (`[diagnostics].response_sent = true`) | the ACK/NAK the engine returned to an inbound sender, under a sentinel destination `\x1fack:` | rows: `ack_code` (`AA`/`AE`/`AR`/`CA`/`CE`/`CR`), `ack_phase` (`decode`/`parse`/`strict`/`ingest`), `outcome`, `body`, `detail` | the store database | "what did we actually reply, and why" — the operator's answer to a sender disputing an ACK | `GET /messages/{id}/responses` under `messages:read` + `require_phi_read`; the `body` only for a caller who also holds `messages:view_raw` and `messages:view_summary`; every read writes a `response.read` audit row | `body`, `detail` and `resp_headers` are set to `NULL` in place by `purge_message_bodies` on the message-body window, on all three backends | **PHI fail-safe:** the ACK **body** is stored **only when the store cipher is active** — on a keyless store it is `NULL` rather than plaintext — and every NAK passes no body at all, so the offending field value is never persisted. The disposition metadata (`ack_code`/`ack_phase`/`outcome`) is non-PHI and always captured; `detail` is `safe_text`-scrubbed, held to the 200-character bound §3's PL-2 block states, and encrypted | | **10. `[alerts]` webhook transport** (off by default — `webhook_url` unset) | one HTTPS POST per alert, carrying every non-underscore event key as JSON | JSON | the operator's webhook endpoint (Slack/Teams/PagerDuty/custom) | operator notification | **`https` only** — a plaintext `http://` webhook URL is refused at construction unless the `MEFOR_ALLOW_INSECURE_TLS` escape is set (and then a warning is logged); since #329 this path routes that escape through the clamped `weakened_tls_escape_permitted(posture)` (the instance posture threaded from the API lifespan), so on an enforcing-PHI instance the escape is inert and a cleartext webhook POST stays refused — the same clamp as the connectors, no longer the raw escape. Redirects are refused; an optional `webhook_allowed_hosts` egress allowlist gates the host | the endpoint's | **carries the alert's `detail`/`reason` free text** (`safe_exc()`-scrubbed at the emit sites, but **not** re-run through `safe_text` on this path). Internal `_`-prefixed keys (per-rule recipients, rule id, cooldown) are stripped before send, so recipient addresses never cross the wire | | **11. `[alerts]` SMTP transport — operator alert list** (off unless `email_smtp_host` + `email_from` + ≥1 `email_to`) | one email per alert; default subject `[MessageFoundry] — `, default body every non-underscore event key as `k: v` | plain text (always kept — never HTML-only); optional HTML alternative | the operators' mailboxes | operator notification | `smtp_allowed_hosts` egress allowlist; the SMTP password comes from `MEFOR_ALERTS_EMAIL_PASSWORD` or a `[secrets]` provider, never the config file; per-send timeout `email_timeout` | the mail system's | carries the same `detail`/`reason` free text as the webhook. #138 operator templates are constrained to a **closed non-PHI variable allowlist** validated fail-closed at config load. **Transport posture:** `send_plain_email` builds an explicit **verifying** context (chain + hostname + strict RFC 5280, TLS 1.2 floor) via `tls_policy.build_smtp_tls_context()` and passes it to `starttls()`, anchored to the OS roots, `[alerts].email_tls_ca_file`, or `[tls].internal_ca_file` — the same factory the EMAIL and DIRECT *message destinations* use, so all three SMTP cells now share one policy ([#323](BACKLOG.md), closed 2026-08-02). Before that this call passed **no** context and Python's stdlib default applied (`ssl._create_stdlib_context` **is** `ssl._create_unverified_context` — `CERT_NONE`, `check_hostname = False`), leaving the hop encrypted but unauthenticated. There is still **no hop gradient or attestation on this path** — unlike the connectors, this cell is constructed outside the `active_hop_posture` scope, so its deviations (`email_use_tls = false`, or `email_tls_verify = false`) are gated by a `[security].allow_unverified_alert_smtp_tls` **acknowledgment switch at the serve gate** rather than by the clamped escape: on an enforcing PHI instance `serve` refuses to start without it, and permits + `AUDIT`-logs the start with it. Both deviations are named by `security_loosenings()` and reported by `messagefoundry check`'s `alert-smtp-tls` advisory | diff --git a/docs/SECURITY-LOOSENING.md b/docs/SECURITY-LOOSENING.md index 087034370..8aa3804c2 100644 --- a/docs/SECURITY-LOOSENING.md +++ b/docs/SECURITY-LOOSENING.md @@ -136,7 +136,7 @@ leaves the engine. | | `[store].secure_delete` | `true` (*conditional* — a loosening only on a file-backed SQLite store; `false` leaves a purged body's bytes in the file) | | | `[auth].ad_session_recheck_seconds` | `300` s (*conditional* — a loosening only once `ad_enabled`) | | | `[auth].ad_allow_insecure_ldap` | `false` (*conditional* — a loosening only while a plain bind is live; loads only under `enforcement = warn`. See its entry below) | -| | `[auth].ad_connect_timeout`, `ad_receive_timeout` | `10` s / `10` s (*conditional* — a loosening only once `ad_enabled`; a timeout above `10` s is named. The load refuses `0`, a negative, `inf`, `NaN` and anything above `3600`) | +| | `[auth].ad_connect_timeout`, `ad_receive_timeout`, `ad_group_search_deadline` | `10` s / `10` s / `120` s (*conditional* — a loosening only once `ad_enabled`; a value above its default is named. The load refuses `0`, a negative, `inf`, `NaN` and anything above `3600`) | | | `[auth].admin_new_ip_step_up` | `true` | | | `[auth].require_action_step_up` | `true` (`false` lets the action-bound step-up routes accept the session-wide step-up window) | | | `[auth].password_check_breached`, `password_check_context`, `password_check_username` | `true` (each `false` turns off one screen of a local password) | @@ -174,7 +174,7 @@ leaves the engine. **At least thirty of these do not live in `[security]`.** `[store].aad_bind`, `[store].allow_unmarked_ciphertext`, `[store].secure_delete`, `[auth].ad_session_recheck_seconds`, `[auth].ad_allow_insecure_ldap`, `[auth].ad_connect_timeout`, `[auth].ad_receive_timeout`, -`[auth].admin_new_ip_step_up`, `[auth].require_action_step_up`, the three +`[auth].ad_group_search_deadline`, `[auth].admin_new_ip_step_up`, `[auth].require_action_step_up`, the three `[auth].password_check_*` keys, the four `[auth].login_rate_limit_*` keys, the three `[auth].lockout_*` keys, the three named `[auth].phi_read_rate_limit_*` keys, the four `[auth].admin_write_*` keys, `[auth].mfa_verify_min_elapsed_seconds`, `[auth].oidc_callback_min_elapsed_seconds`, @@ -756,22 +756,27 @@ Named as `alerts.rules`, with each rule's position, its `id`, and the reminder e is written, as for the other settings-scoped loosenings. - **Reversible:** yes, immediately — point `ad_server` at `ldaps://`, or delete the line, and restart. -### `[auth].ad_connect_timeout` or `ad_receive_timeout` above `10` s **with `ad_enabled`** — a stalled directory call holds a thread longer +### `[auth].ad_connect_timeout`, `ad_receive_timeout` or `ad_group_search_deadline` above its default **with `ad_enabled`** — a stalled directory call holds a thread longer > **Conditional** on `[auth].ad_enabled`, since nothing reads the timeouts with no directory (vault -> BACKLOG #2567, ASVS 13.1.3). Each is named on its own once it is above `10` s. A shorter timeout -> fails a stalled call sooner, which is stricter, and is not named. Neither has an off value: the +> BACKLOG #2567, ASVS 13.1.3). Each is named on its own once it is above its default: `10` s for the two timeouts, `120` s for the deadline (vault BACKLOG #3696). A shorter timeout +> fails a stalled call sooner, which is stricter, and is not named. None has an off value: the > row in the table above gives what the load refuses. - **What you lose:** each directory call runs in a worker thread until the domain controller answers or a timeout ends. That covers at least a sign-in, a step-up and a session recheck. One call can wait on several connects and reads, each bounded on its own. With a longer timeout, a controller that has stopped answering holds each thread longer, so stalled calls tie up the thread pool that - sign-in shares for longer. + sign-in shares for longer. A longer `ad_group_search_deadline` holds the thread while a slow + controller keeps sending pages of the nested group search, since each page is still bounded by + `ad_receive_timeout`. - **When acceptable:** a directory reached over a slow link, where `10` s is measured to be too short - for a healthy round trip. + for a healthy round trip, or `120` s too short for a healthy nested group search of the largest + account. A search refused at the deadline fails that directory call, a sign-in for example, and + the reconciler pages for that account rather than re-reading its roles and scope. - **Compensating controls:** none bounds how many calls stall at once. The sign-in rate limit paces how many start per window, not how many are held, and a session recheck does not pass it. So - raise the timeout only as far as the measured round trip needs. -- **Reversible:** yes, immediately — restore `10` (or delete the line) and restart. + raise the timeout only as far as the measured round trip, or the measured group search, needs. +- **Reversible:** yes, immediately — restore the default, `10` for the two timeouts and `120` for the + deadline (or delete the line), and restart. ### `[auth].require_action_step_up = false` — the action-bound step-up falls back to the session window > Reported whenever it is `false` (vault BACKLOG #2600). The default is `true` @@ -2054,7 +2059,7 @@ chapter was not part of the verification above. | `[store].allow_unmarked_ciphertext` (unmarked-value refusal) | V11 Cryptography | **SC-28(1)** Cryptographic Protection · **SI-7** Software, Firmware, and Information Integrity | §164.312(c)(1) Integrity · §164.312(c)(2) Mechanism to Authenticate ePHI | | `[auth].ad_session_recheck_seconds` (directory revocation propagation) | V7 Session Management · V6 Authentication | **AC-2(3)** Disable Accounts · **AC-12** Session Termination | §164.312(a)(2)(i) Unique User Identification · §164.308(a)(3)(ii)(C) Termination Procedures | | `[auth].ad_allow_insecure_ldap` (plain `ldap://` AD bind) | V12 Secure Communication | **SC-8** Transmission Confidentiality and Integrity · **SC-8(1)** Cryptographic Protection | §164.312(e)(1) Transmission Security · §164.312(d) Person or Entity Authentication | -| `[auth].ad_connect_timeout`, `[auth].ad_receive_timeout` (bound on one directory call) | V13 Configuration (13.1.3) | **SC-5** Denial-of-Service Protection | §164.312(d) Person or Entity Authentication | +| `[auth].ad_connect_timeout`, `[auth].ad_receive_timeout`, `[auth].ad_group_search_deadline` (bound on one directory call) | V13 Configuration (13.1.3) | **SC-5** Denial-of-Service Protection | §164.312(d) Person or Entity Authentication | | `MEFOR_ALLOW_INSECURE_CONFIG_SOURCE` (config-source check downgraded to a warning) | V13 Configuration | **CM-5** Access Restrictions for Change · **SI-7** Software, Firmware, and Information Integrity | §164.312(c)(1) Integrity | | `[auth].admin_new_ip_step_up` (mid-session new-address step-up) | V8 Authorization (adaptive, 8.2.4) · V6 Authentication | **AC-2(12)** Account Monitoring for Atypical Usage · **IA-11** Re-authentication | §164.312(d) Person or Entity Authentication · §164.308(a)(5)(ii)(C) Log-in Monitoring | | `[auth].login_rate_limit_*`, `[auth].lockout_minutes`, `[auth].lockout_threshold`, `[auth].lockout_max_minutes` (sign-in limits and account lockout) | V6 Authentication (6.1.1) | **AC-7** Unsuccessful Logon Attempts | §164.312(d) Person or Entity Authentication · §164.308(a)(5)(ii)(C) Log-in Monitoring | diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 260036015..a23cba247 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -4632,6 +4632,14 @@ visible and limits nothing. A pass with a fault marks no alert clear. When it re [CONFIGURATION.md](CONFIGURATION.md#when-the-reconcilers-alerts-resolve-themselves). Step-up refuses the account with outcome `probe_fault`. +A group search the engine refuses for its own bound, past `[auth].ad_group_search_deadline` or +past its cap on groups or pages, reads that account as unreachable (vault BACKLOG #3696). A disable +or deletion still revokes it, because the user search runs first, but a role or channel-scope +change in the directory does not reach its sessions while that lasts. Unlike an outage it pages, once per account as the account enters +that state, and audits `auth.ad_reconcile_paging_refused` naming the account. The bound and the +page are stated in [CONNECTIONS.md](CONNECTIONS.md#table-b--per-service-resource-strategy-asvs-1313), +the Active Directory rows. + ### The engine session is independent of the IdP session (ASVS 7.6.1) The engine mints its own session at sign-in. That session does not follow the identity provider diff --git a/messagefoundry/api/app.py b/messagefoundry/api/app.py index fb4cb84a5..fc80cc606 100644 --- a/messagefoundry/api/app.py +++ b/messagefoundry/api/app.py @@ -282,6 +282,8 @@ from messagefoundry.auth.audit_visibility import reads_audit_copies_in_the_log from messagefoundry.auth.reconcile import ( HOLD_REASON, + PAGING_CAP, + PAGING_DEADLINE, PROBE_FAULT_ABORT, REFERRAL_ABORT, ReconcilePlan, @@ -10158,6 +10160,18 @@ async def _directory_reconciler( #: The source label of the probe fault's own ``ad_reconcile_aborted`` instance (BACKLOG #3100), apart #: from the breaker's and the referral's for the same reason. _PROBE_FAULT_ALERT_SOURCE = "directory-reconciler-probe-fault" +#: The source label prefix of a paging refusal's ``ad_reconcile_aborted`` instance (vault BACKLOG +#: #3696). The account name follows it, so each account is its own instance and throttle, as +#: ``ad_session_revoked`` is keyed on the account: a second account entering inside the first's +#: cooldown still pages. +_PAGING_ALERT_SOURCE = "directory-reconciler-paging" +#: What each paging refusal cause tells the operator. Short, and the consequence first, because +#: the store keeps only the first 200 characters of an instance's reason; the account is in the +#: source. +_PAGING_REFUSAL_TEXT: Final[Mapping[str, str]] = { + PAGING_DEADLINE: "past [auth].ad_group_search_deadline", + PAGING_CAP: "past the engine's cap on groups or pages", +} def _alert_reconcile_plan(plan: ReconcilePlan, auth: AuthService, sink: AlertSink) -> None: @@ -10191,6 +10205,10 @@ def _alert_reconcile_plan(plan: ReconcilePlan, auth: AuthService, sink: AlertSin alert with reason ``probe_fault``, under its own source label (:data:`_PROBE_FAULT_ALERT_SOURCE`). It has no inverse: an operator resolves it. + A paging refusal (vault BACKLOG #3696) pages once per account as it enters that state, after + the revocations and on an outage-shaped pass too, matching its ``auth.ad_reconcile_paging_refused`` row; + :func:`_alert_paging_refusals` says how. + Reads the RETURNED plan, so it sees only a pass that finished. A failed write of the pass's own held, aborted, skipped, referred, faulted or unkeyed-binding row no longer ends the pass (BACKLOG #2137): the service logs it at ERROR and returns the plan, so these alerts still fire. @@ -10212,6 +10230,9 @@ def _alert_reconcile_plan(plan: ReconcilePlan, auth: AuthService, sink: AlertSin earlier run left open (:func:`_without_inherited_clears` drops that clear); ``_mark_reconcile_clears`` states that rule once.""" if plan.directory_outage: + # A paging refusal is not the outage, so an outage-shaped pass still pages for it (vault + # BACKLOG #3696). An outage applies no revocation, so nothing here pages ahead of it. + _alert_paging_refusals(plan, sink) return if plan.aborted is not None and not plan.directory_referral and not plan.probe_fault: # A breaker trip. A pass of referrals only is not one: its one alert is the referral's below. @@ -10225,6 +10246,9 @@ def _alert_reconcile_plan(plan: ReconcilePlan, auth: AuthService, sink: AlertSin elif plan.aborted is None: for revocation in plan.revocations: sink.ad_session_revoked(revocation.username, reason=revocation.reason) + # After the breaker and the revocations, which do not recur either, and before the referral, + # the fault and the hold, which page again on the next pass (vault BACKLOG #3696). + _alert_paging_refusals(plan, sink) if plan.referred: # BACKLOG #2538. After the breaker and the revocations, matching the service's order. sink.ad_reconcile_aborted( @@ -10259,6 +10283,29 @@ def _alert_reconcile_plan(plan: ReconcilePlan, auth: AuthService, sink: AlertSin sink.ad_reconcile_breaker_cleared(_REFERRAL_ALERT_SOURCE) +def _alert_paging_refusals(plan: ReconcilePlan, sink: AlertSink) -> None: + """Page each account that has just entered the paging-refused state (vault BACKLOG #3696). + + Its group search was refused for the engine's own bound, so the reconciler reads it as + unavailable: a disable or deletion still revokes it, but a role or scope change in the + directory does not reach its sessions while that lasts. The auth service latches the state per + account (``AuthService._note_paging_refusals``), so this pages once on entry, not every pass. + It has no inverse, like the probe fault's: an operator resolves it. The account name, in the + source only, is the one ``ad_session_revoked`` already carries; nothing else about the account + is.""" + for _user_id, username, cause in plan.paging_refused: + what = _PAGING_REFUSAL_TEXT.get(cause, cause) + sink.ad_reconcile_aborted( + f"{_PAGING_ALERT_SOURCE}:{username}", + reason=cause, + probed=plan.probed, + detail=( + "Directory role and scope changes do not reach this account's sessions: its group " + f"search was refused, {what}. A disable or deletion still revokes them." + ), + ) + + def _without_clears(plan: ReconcilePlan) -> ReconcilePlan: """The plan with every ``*_clear`` flag set False, so it resolves no alert instance. diff --git a/messagefoundry/auth/ldap.py b/messagefoundry/auth/ldap.py index 1daea38ab..c1db8e337 100644 --- a/messagefoundry/auth/ldap.py +++ b/messagefoundry/auth/ldap.py @@ -12,8 +12,9 @@ so the only bound on an unresponsive domain controller is the pair of finite ``ldap3`` timeouts every ``Server``/``Connection`` here is constructed with — ``[auth].ad_connect_timeout`` (the TCP connect) and ``[auth].ad_receive_timeout`` (each LDAP response read), both defaulting to 10 s (ASVS 13.1.3). ldap3's -own defaults are ``None`` on both, i.e. wait forever. ``ldap3``/``spnego`` are imported lazily so a -local-only deployment never touches them. +own defaults are ``None`` on both, i.e. wait forever. All the pages of the paged nested group search +also share one ``[auth].ad_group_search_deadline`` (:func:`_search_paged`, BACKLOG #3696). +``ldap3``/``spnego`` are imported lazily so a local-only deployment never touches them. **No referral is followed, and one is refused** (BACKLOG #2530); :func:`_refuse_referral` says why. **A search that ends with any result code but success is refused too** (BACKLOG #3010), never read @@ -28,6 +29,7 @@ import re import ssl import struct +import time import uuid from collections.abc import Iterable, Iterator from dataclasses import dataclass @@ -1101,11 +1103,30 @@ def _undecodable(operation: str, exc: BaseException) -> LdapUndecodableError: #: returns smaller pages, which is still the whole answer. _PAGE_SIZE = 500 -#: Pages read before a paged search is refused as unfinished. A count of pages, not of entries: at -#: :data:`_PAGE_SIZE` it is 10,000 entries, and fewer where a directory returns shorter pages. Each -#: page may take up to ``[auth].ad_receive_timeout``, so a directory that keeps returning a cookie -#: would otherwise hold the sign-in or reconcile worker for as long as it keeps answering. -_MAX_PAGES = 20 +#: Entries a paged search may return before it is refused as unfinished (BACKLOG #3696 item 4). +#: A count of entries, not of pages, so the cap does not move with the page size a directory +#: returns: a directory capping a page at 100 (``MaxPageSize = 100``) gets the same 10,000 as one +#: returning :data:`_PAGE_SIZE`. It was 20 pages, which at 100 a page refused an account in more +#: than 2,000 groups. +_MAX_PAGED_ENTRIES = 10_000 + +#: Pages read before a paged search is refused, a backstop against a directory that keeps +#: returning a cookie with empty or tiny pages. The overall deadline already bounds the time +#: (:func:`_search_paged`); this bounds the round trips inside it. Set where it cannot bind before +#: :data:`_MAX_PAGED_ENTRIES` on any page of 10 entries or more. +_MAX_PAGES = _MAX_PAGED_ENTRIES // 10 + 1 + +#: The ``code_name`` of a paged search refused for its bounds rather than for its answer: past its +#: overall deadline, past :data:`_MAX_PAGED_ENTRIES`, or past the :data:`_MAX_PAGES` backstop +#: (BACKLOG #3696). Public, because the directory reconciler pages on them where it reads every +#: other unanswered search as an outage (``AuthService._probe_principal``). +PAGING_DEADLINE_SPENT = "deadline spent" +PAGING_TOO_MANY_ENTRIES = "too many entries" +PAGING_TOO_MANY_PAGES = "too many pages" + +#: The clock a paged search's overall deadline reads. Monotonic, so a wall-clock step cannot +#: stretch or cut the budget; a module name so a test can drive it. +_monotonic = time.monotonic def _paged_cookie(conn: Any) -> bytes | None: @@ -1122,7 +1143,7 @@ def _paged_cookie(conn: Any) -> bytes | None: return cookie if isinstance(cookie, bytes) else None -def _search_paged(conn: Any, operation: str, **kwargs: Any) -> list[Any]: +def _search_paged(conn: Any, operation: str, *, budget: float, **kwargs: Any) -> list[Any]: """Every entry of a search that can outrun a directory's unpaged cap, read page by page. BACKLOG #3010. Each page goes through :func:`_search`, so a page that does not end with success @@ -1130,38 +1151,142 @@ def _search_paged(conn: Any, operation: str, **kwargs: Any) -> list[Any]: is never used as the whole. Unpaged, an account in more groups than the cap would end the nested group search with sizeLimitExceeded, which that refusal turns into a failed sign-in. - **A first page with no readable control is the whole answer**: the control is sent - non-critical, so a directory may ignore it, and that answer either is complete or ends with a - code :func:`_refuse_unanswered` refuses. **A later page with none is refused**: the directory - was paging, and a page that does not say whether it is the last cannot be read as the last. + **A first page with no readable control is the whole answer**, under the entry cap below: the + control is sent non-critical, so a directory may ignore it, and that answer either is complete + or ends with a code :func:`_refuse_unanswered` refuses. **A later page with none is refused**: + the directory was paging, and a page that does not say whether it is the last cannot be read + as the last. + + **All the pages share one deadline, ``budget`` seconds** (BACKLOG #3696 item 3). Each socket + read of a page after the first waits at most what was left when the page was asked for + (:class:`_SocketTimeoutCap`), and no page is asked for once it is spent. A deadline spent + between pages, or a read or send of a later page that waits out what was left, is refused as + ``deadline spent``. The first page keeps the socket's own timeout, so a first page that times + out is one slow read, as any other search's, and a deadline set below + ``[auth].ad_receive_timeout`` refuses a search past its first page rather than failing every + read as an outage. Without it, every page could take the whole + ``[auth].ad_receive_timeout``, so the bound was that times the page count. What the bound + does not cover is in ``docs/CONNECTIONS.md``, the Active Directory row of the timeout table. + + **More than :data:`_MAX_PAGED_ENTRIES` entries refuses the search** (BACKLOG #3696 item 4), + whatever size the directory's pages are, one page included; :data:`_MAX_PAGES` is a backstop. **A refused search is abandoned** (:func:`_abandon_paging`) once the directory has sent a cookie, so it need not hold the paging state until its own timeout. Not after a response ldap3 could not decode: the stream may be out of step after one, so no abandon is sent on it, and - the directory holds the paging state until its own timeout or the connection ends. + the directory holds the paging state until its own timeout or the connection ends. **Nor once + the deadline is spent**: an abandon is one more round trip, so it gets only what is left. The + caller unbinds the connection as its block ends (``_service_session``), and that drops the + state too. """ + import ldap3 + entries: list[Any] = [] cookie: bytes | None = None # what the next page is asked with newest: bytes | None = None # the newest non-empty cookie the directory sent + deadline = _monotonic() + budget + timeout = _SocketTimeoutCap(conn) + out_of_time = False + + def spent() -> LdapUnansweredError: + nonlocal out_of_time + out_of_time = True + return _paging_refused( + operation, + PAGING_DEADLINE_SPENT, + f"did not finish paging within {budget:g} s", + "BACKLOG #3696", + ) + try: for page in range(_MAX_PAGES): - _search(conn, operation, paged_size=_PAGE_SIZE, paged_cookie=cookie, **kwargs) + remaining = deadline - _monotonic() + if remaining <= 0: + raise spent() + # The first page is never capped: see the docstring. + shortened = bool(page) and timeout.cap(remaining) + try: + _search(conn, operation, paged_size=_PAGE_SIZE, paged_cookie=cookie, **kwargs) + except ( + ldap3.core.exceptions.LDAPSocketReceiveError, + ldap3.core.exceptions.LDAPSocketSendError, + ) as exc: + # ldap3 merges the socket's own exception into its class, so a read or send that + # waited out the deadline rather than the socket's timeout is a TimeoutError here. + if shortened and isinstance(exc, TimeoutError): + raise spent() from exc + raise entries.extend(conn.entries) cookie = _paged_cookie(conn) if cookie is None and page: raise _paging_refused( operation, "paging control lost", "dropped the paging control" ) + if len(entries) > _MAX_PAGED_ENTRIES: + # Only this page's cookie is live: after a last page the directory holds no + # state, and an older cookie would ask it to drop state it already dropped. + newest = None + raise _paging_refused( + operation, + PAGING_TOO_MANY_ENTRIES, + f"returned more than {_MAX_PAGED_ENTRIES:,} entries", + "BACKLOG #3696", + ) if not cookie: return entries newest = cookie raise _paging_refused( - operation, "too many pages", f"was still paging after {_MAX_PAGES} pages" + operation, PAGING_TOO_MANY_PAGES, f"was still paging after {_MAX_PAGES} pages" ) except (LdapUnansweredError, LdapReferralError): - # The refusing page was read, so the stream is in step and a cookie in it is readable. - _abandon_paging(conn, operation, _paged_cookie(conn) or newest, kwargs) + remaining = deadline - _monotonic() + if not out_of_time and remaining > 0: + # The refusing page was read, so the stream is in step and a cookie in it is readable. + timeout.cap(remaining) + _abandon_paging(conn, operation, _paged_cookie(conn) or newest, kwargs) raise + finally: + timeout.restore() + + +class _SocketTimeoutCap: + """Shortens ``conn``'s socket receive timeout for a paged search, and restores it after. + + BACKLOG #3696 item 3. ldap3 applies ``receive_timeout`` once, when it opens the socket, through + ``socket.settimeout``, which leaves the socket non-blocking under Python's own poll, so a later + ``settimeout`` governs every read that follows on every OS; the ``SO_RCVTIMEO`` ldap3 also sets + applies only to a blocking socket. A connection with no socket, such as a test double, is left + alone. + """ + + def __init__(self, conn: Any) -> None: + self._sock: Any = getattr(conn, "socket", None) + self._original: float | None = self._sock.gettimeout() if self._sock is not None else None + + def cap(self, seconds: float) -> bool: + """Bound each read to at most ``seconds``, never longer than the socket's own timeout. + Returns whether ``seconds`` is now the bound, rather than that timeout.""" + if self._sock is None: + return False + shortened = self._original is None or seconds < self._original + try: + self._sock.settimeout(seconds if shortened else self._original) + except OSError as failed: + # A socket ldap3 already closed: nothing reads it, and the refusal or the next + # search's own error is the answer, so this must not replace either. + logger.debug("AD socket timeout cap on a paged search: %s", _type_name(failed)) + return False + return shortened + + def restore(self) -> None: + """Put the socket's own timeout back. ldap3 closes the socket when a read fails, and + ``settimeout`` on a closed one raises, which is logged and is no fault: nothing reads it.""" + if self._sock is None: + return + try: + self._sock.settimeout(self._original) + except OSError as failed: + logger.debug("AD socket timeout restore after a paged search: %s", _type_name(failed)) def _abandon_paging( @@ -1174,7 +1299,9 @@ def _abandon_paging( A finding of BACKLOG #3010's review. ``cookie`` is the newest non-empty one the directory sent; with none, it never held state for this search and nothing is sent. Best effort: the search is refused whatever this answers, so a failed abandon is not raised: a refusal of it is warned once - by :func:`_search`, and an ldap3 error from it is logged at debug level. It costs one more round trip, which can take up to ``[auth].ad_receive_timeout``. + by :func:`_search`, and an ldap3 error from it is logged at debug level. It costs one more + round trip, whose reads wait at most what is left of the paged search's deadline, and it is + not sent once that is spent (BACKLOG #3696). """ import ldap3 @@ -1187,12 +1314,15 @@ def _abandon_paging( logger.debug("AD abandon of a refused %s failed: %s", operation, _type_name(failed)) -def _paging_refused(operation: str, case: str, what: str) -> LdapUnansweredError: - """The refusal for a paged search whose pages each ended with success but whose paging did not - end, warned once per operation and case.""" +def _paging_refused( + operation: str, case: str, what: str, item: str = "BACKLOG #3010" +) -> LdapUnansweredError: + """The refusal for a paged search refused for its paging rather than for a result code: the + paging did not end, or ran past its deadline or entry cap. Warned once per operation and case; + ``item`` is the backlog item that added the case.""" message = ( f"AD {what} on the {operation}; the engine reads it as the directory not answering " - "(BACKLOG #3010)." + f"({item})." ) _warn_once(f"unanswered: {operation}: {case}", "%s Reported once.", message) return LdapUnansweredError(message, code_name=case) @@ -1803,9 +1933,12 @@ def _resolve_groups(self, conn: Any, user_dn: str, member_of: list[str]) -> froz # A search the directory did not finish raises here (BACKLOG #3010), never a shorter # group list. # Paged, so an account in more groups than the directory's unpaged cap still resolves. + # All the pages together get one [auth].ad_group_search_deadline, so paging no longer + # multiplies the per-read ad_receive_timeout (BACKLOG #3696 item 3). groups = _search_paged( conn, "group search", + budget=self._s.ad_group_search_deadline, search_base=self._s.ad_group_search_base, search_filter=f"(member:{_MATCHING_RULE_IN_CHAIN}:={_escape_filter(user_dn)})", search_scope=ldap3.SUBTREE, diff --git a/messagefoundry/auth/reconcile.py b/messagefoundry/auth/reconcile.py index 621a7da0f..3e92c0861 100644 --- a/messagefoundry/auth/reconcile.py +++ b/messagefoundry/auth/reconcile.py @@ -134,7 +134,9 @@ class Probe: #: For :attr:`ProbeOutcome.REFERRED`: the refusal's text, which names the referred search and #: the referred hosts and nothing else (BACKLOG #2530). The pass logs the first one. For #: :attr:`ProbeOutcome.FAULT`: the exception's type name, never its message (BACKLOG #3100). - #: None for every other outcome. + #: For :attr:`ProbeOutcome.UNAVAILABLE`: :data:`PAGING_DEADLINE` or :data:`PAGING_CAP` when the + #: group search was refused for its bounds rather than unanswered (vault BACKLOG #3696), else + #: None. None for every other outcome. detail: str | None = None @@ -182,6 +184,14 @@ class UsernameRefresh: #: with a fault. The same string as :attr:`ProbeOutcome.FAULT`'s value. PROBE_FAULT_ABORT = ProbeOutcome.FAULT.value +#: The causes of an :attr:`ProbeOutcome.UNAVAILABLE` probe whose group search the engine refused +#: for its own bounds (vault BACKLOG #3696): past ``[auth].ad_group_search_deadline``, or past the +#: cap on groups or its page backstop. It pages, once per account as it enters that state +#: (``AuthService._note_paging_refusals``): unlike an outage it does not pass on its own, and the +#: enabled account's roles and scope are not re-read while it lasts. Also the alert's ``reason``. +PAGING_DEADLINE = "group_search_deadline" +PAGING_CAP = "group_search_cap" + #: The outcomes that judge nothing about the account: no strike, no recorded outcome (rule item 4). _UNJUDGED = frozenset({ProbeOutcome.UNAVAILABLE, ProbeOutcome.REFERRED, ProbeOutcome.FAULT}) @@ -265,6 +275,12 @@ class ReconcilePlan: breaker_clear: bool = False hold_clear: bool = False referral_clear: bool = False + #: Set by the auth service after the pass, never by :func:`plan_pass` (vault BACKLOG #3696): + #: ``(user_id, username, cause)`` of each account that has just ENTERED the state of reading + #: UNAVAILABLE because its group search was refused for its bounds, ``cause`` being + #: :data:`PAGING_DEADLINE` or :data:`PAGING_CAP`. An account already in that state is not + #: here, so each pages once per state change. Set on every pass, an outage-shaped one included. + paging_refused: tuple[tuple[str, str, str], ...] = () @property def directory_outage(self) -> bool: diff --git a/messagefoundry/auth/service.py b/messagefoundry/auth/service.py index 7af33f502..e9da3367f 100644 --- a/messagefoundry/auth/service.py +++ b/messagefoundry/auth/service.py @@ -55,12 +55,16 @@ ) from messagefoundry.auth.identity import ALL_CHANNELS, AuthProvider, Identity, SessionMechanism from messagefoundry.auth.ldap import ( + PAGING_DEADLINE_SPENT, + PAGING_TOO_MANY_ENTRIES, + PAGING_TOO_MANY_PAGES, AdPrincipal, DirectoryAnswer, KerberosTokenRefused, LdapAuthenticator, LdapError, LdapReferralError, + LdapUnansweredError, LdapUndecodableError, kerberos_principal, normalise_object_guid, @@ -1792,6 +1796,17 @@ def _holds_lockout_state(user: UserRecord) -> bool: DirectoryAnswer.AMBIGUOUS: reconcile.ProbeOutcome.ABSENT, } +#: The paged group search refusals that are the engine's own bounds, and the cause each pages as +#: (vault BACKLOG #3696). The reconciler reads them as UNAVAILABLE like any unanswered search, and +#: then pages (`AuthService._note_paging_refusals`), because unlike an outage they recur. +_PAGING_BOUND_CAUSES: Final[Mapping[str, str]] = MappingProxyType( + { + PAGING_DEADLINE_SPENT: reconcile.PAGING_DEADLINE, + PAGING_TOO_MANY_ENTRIES: reconcile.PAGING_CAP, + PAGING_TOO_MANY_PAGES: reconcile.PAGING_CAP, + } +) + #: The ``auth.reauth`` reason for each step-up re-bind whose lookup judged no password (BACKLOG #: #2434). None is counted toward the lockout. ``None`` means no lookup ran. ``_reauth_ad`` refuses #: an empty password before it asks, so ``None`` from a directory reads as one that could not be @@ -2952,6 +2967,11 @@ def __init__( #: 0195 rule item 3). The undetermined-wave hold counts UNDETERMINED entries here across the #: probe rotation, so two unreadable accounts sampled on different passes are still two. self._reconcile_outcomes: dict[str, reconcile.ProbeOutcome] = {} + #: user_id -> the bound that first refused that account's group search in its current + #: paging-refused state (vault BACKLOG #3696). The latch that makes the page fire once per + #: account per state change: `_record_paging_refusals` sets it, `_note_paging_refusals` + #: re-arms it, and the prune drops an account that left. + self._reconcile_paging_refused: dict[str, str] = {} #: The undetermined-wave hold's operator message (ADR 0195). Its own latch, because a held #: pass is not an aborted one and so clears `_reconcile_alert`. Set on a pass that holds, #: cleared on the next pass that judges and does not. A pass with no candidates, or a @@ -6948,7 +6968,16 @@ async def _probe_principal(self, user: UserRecord) -> reconcile.Probe: # need the console. Debug-level — a flapping DC must not flood the log at one line per # user per pass; the pass-level summary below reports the count at WARNING. _log.debug("directory probe failed for %s: %s", user.username, exc) - return reconcile.Probe(user.id, user.username, reconcile.ProbeOutcome.UNAVAILABLE) + # Vault BACKLOG #3696. A group search past its deadline or its entry cap is still + # unjudged, so still UNAVAILABLE, but it says which bound refused it, so it pages. + cause = ( + _PAGING_BOUND_CAUSES.get(exc.code_name) + if isinstance(exc, LdapUnansweredError) + else None + ) + return reconcile.Probe( + user.id, user.username, reconcile.ProbeOutcome.UNAVAILABLE, detail=cause + ) except Exception as exc: if _unreached_directory(exc): # BACKLOG #3100. ldap.py wraps the socket faults it knows of as LdapError (BACKLOG @@ -7015,6 +7044,62 @@ async def _probe_principal(self, user: UserRecord) -> reconcile.Probe: directory_username=principal.username if user.directory_object_id is not None else None, ) + def _note_paging_refusals( + self, probes: Iterable[reconcile.Probe], judged: Iterable[str] + ) -> tuple[tuple[str, str, str], ...]: + """``(user_id, username, cause)`` of each probed account that ENTERS the paging-refused + state on this pass (vault BACKLOG #3696). + + Its group search was refused for the engine's own bound, past + ``[auth].ad_group_search_deadline`` or past the cap on groups or pages, so it reads + UNAVAILABLE. A disable or deletion still strikes, because the user search runs first; what + stops is the role and scope re-diff of an enabled account. Unlike an outage it recurs on + every pass, so it pages, once per state change: an account already latched is not + returned again, whichever bound refused it this time. An account the pass judged + (``judged``, the plan's ``outcomes``) re-arms here; an outage, a referral or a fault judged + nothing, says nothing about the bound, and leaves the latch alone. **The latch is set only once the entry + is recorded** (:meth:`_record_paging_refusals`), so a pass that raises in the service before that + point leaves the account to enter again on the next pass. A sink that raises in the + lifespan task after it can still cost the page; the row stands. An account that + left the candidate set is pruned with the other records, and the latch is process-local, so + a restart pages a standing one once more. + """ + latch = self._reconcile_paging_refused + for user_id in judged: + latch.pop(user_id, None) + entered: list[tuple[str, str, str]] = [] + for probe in probes: + if ( + probe.outcome is reconcile.ProbeOutcome.UNAVAILABLE + and probe.detail is not None # REFERRED and FAULT carry a detail of their own + and probe.user_id not in latch + ): + entered.append((probe.user_id, probe.username, probe.detail)) + return tuple(entered) + + async def _record_paging_refusals(self, plan: reconcile.ReconcilePlan) -> None: + """Log, audit and latch each account entering the paging-refused state (vault BACKLOG + #3696). + + One WARNING and one ``auth.ad_reconcile_paging_refused`` row per entry, naming the account + as ``actor``, so the page the lifespan task raises for it matches a row, as every + reconcile alert does (ASVS 8.3.2). Written after the revocations, like the referral's and + the fault's rows, and through the same soft write, so a refused row costs only its row. + """ + for user_id, username, cause in plan.paging_refused: + _log.warning( + "directory reconcile: the group search for %s was refused for its bound (%s), so " + "its roles and channel scope are not re-read while this lasts", + scrub_log_argument(username), + cause, + ) + await self._audit_reconciler_row( + "auth.ad_reconcile_paging_refused", + actor=username, + detail=_json({"reason": cause, "probed": plan.probed, "aborted": plan.aborted}), + ) + self._reconcile_paging_refused[user_id] = cause + async def reconcile_directory_sessions(self) -> reconcile.ReconcilePlan: """Run one reconciliation pass and apply it; return what it did (for the loop + tests). @@ -7099,6 +7184,7 @@ async def reconcile_directory_sessions(self) -> reconcile.ReconcilePlan: reconcile.prune_ledger(self._reconcile_strikes, users, rank=float) reconcile.prune_ledger(self._reconcile_last_probed, users, rank=float) reconcile.prune_ledger(self._reconcile_outcomes, users, rank=reconcile.outcome_rank) + reconcile.prune_ledger(self._reconcile_paging_refused, users, rank=lambda _cause: 0.0) if reconcile.ProbeOutcome.UNDETERMINED not in self._reconcile_outcomes.values(): # u is 0: every held account has left the record, so the hysteresis has nothing to hold. self._reconcile_hold_latched = False @@ -7190,6 +7276,8 @@ async def reconcile_directory_sessions(self) -> reconcile.ReconcilePlan: self._reconcile_strikes.update(plan.strikes) self._reconcile_outcomes.update(plan.outcomes) self._reconcile_hold_latched = plan.latched + # Vault BACKLOG #3696. Before any return below, so an outage-shaped pass pages too. + plan = replace(plan, paging_refused=self._note_paging_refusals(probes, plan.outcomes)) if plan.hold: self._advance_hold_standing("held") # BACKLOG #2136. A trip makes every candidate unconfirmed until a pass that is not aborted @@ -7234,6 +7322,8 @@ async def reconcile_directory_sessions(self) -> reconcile.ReconcilePlan: # a pass of faults only as a fault and nothing else (#3100). await self._record_reconcile_referral(plan, first=referral) await self._record_reconcile_fault(plan) + # Vault BACKLOG #3696. On every abort, an outage-shaped one included, as its page is. + await self._record_paging_refusals(plan) if not plan.judged_nothing: # A held pass writes its own row even when the breaker also aborts it (ADR 0195 rule # item 9). An outage or a pass of referrals only judged nothing, so it leaves the @@ -7300,6 +7390,8 @@ async def reconcile_directory_sessions(self) -> reconcile.ReconcilePlan: await self._record_reconcile_referral(plan, first=referral) # BACKLOG #3100. After the revocations too, for the same reason as the referral's. await self._record_reconcile_fault(plan) + # Vault BACKLOG #3696. After the revocations too, for the same reason. + await self._record_paging_refusals(plan) # ADR 0195. After the revocations, so a held-row audit write that fails cannot stop a # genuine disable or demotion in the same pass from being applied. And that failure is # logged rather than raised (BACKLOG #2137), so the pass still returns its plan and the @@ -8305,7 +8397,9 @@ async def _audit_reconciler_row( BACKLOG #2137. These rows are the held row, the breaker's aborted row, the outage's skipped row, the referral's row (BACKLOG #2538), the probe fault's row (BACKLOG #3100) and the - unkeyed-binding report. Since BACKLOG #3018 they also include each account's + unkeyed-binding report. They include each paging refusal's + ``auth.ad_reconcile_paging_refused`` row (vault BACKLOG #3696), which names the account as + ``actor``. Since BACKLOG #3018 they also include each account's ``auth.ad_session_revoked`` row and, on the reconciler's call only, a rename's ``auth.ad_username_refreshed`` and ``auth.ad_username_refresh_conflict`` rows; those name the account as ``actor``. **A failed write here does not end the pass.** If it raised, the diff --git a/messagefoundry/config/settings.py b/messagefoundry/config/settings.py index e827f4bf3..c197b9699 100644 --- a/messagefoundry/config/settings.py +++ b/messagefoundry/config/settings.py @@ -3083,7 +3083,8 @@ def split_kerberos_spn(spn: str) -> tuple[str, str]: #: and ``tests/test_site_context_words.py`` holds the two equal. EXTRA_CONTEXT_WORD_MIN_LENGTH = 3 -#: Upper bound for ``[auth].ad_connect_timeout`` and ``ad_receive_timeout``. An hour is far past any +#: Upper bound for ``[auth].ad_connect_timeout``, ``ad_receive_timeout`` and +#: ``ad_group_search_deadline``. An hour is far past any #: real directory round trip and far below the point where a socket timeout overflows. _AD_TIMEOUT_MAX_SECONDS = 3600.0 @@ -3342,6 +3343,16 @@ class AuthSettings(_Section): # above 10 s is a LOOSENING that security_loosenings() names while AD is on (vault BACKLOG #2567). ad_connect_timeout: float = 10.0 # seconds — bound the LDAP/LDAPS TCP connect ad_receive_timeout: float = 10.0 # seconds — bound each LDAP response read (bind + search) + # seconds — the one deadline all the pages of the nested group search share (auth/ldap.py + # _search_paged, vault BACKLOG #3696). Each page's reads are still bounded by + # ad_receive_timeout, so a directory that stops answering fails at that, unless less than + # that is left of this, when it is refused as past this. The first page is never capped. Well + # above it on purpose: a search past it refuses the account, whose roles and scope the + # reconciler then cannot re-read. 120 s is twelve per-read stalls. NOT a hard wall-clock bound: + # it caps each read of a page at what is left, so a page that arrives over several reads can + # run past it (docs/CONNECTIONS.md, the Active Directory row). Validated and named as a + # loosening with the two timeouts above. + ad_group_search_deadline: float = 120.0 # --- directory session reconciliation (ADR 0079 mechanism 2) ------------------------------- # Disabling an account in AD does NOT terminate its live engine session: once an opaque token is @@ -3846,16 +3857,16 @@ def _check_totp_skew(cls, value: int) -> int: ) return value - @field_validator("ad_connect_timeout", "ad_receive_timeout") + @field_validator("ad_connect_timeout", "ad_receive_timeout", "ad_group_search_deadline") @classmethod - def _check_ad_timeout(cls, value: float) -> float: + def _check_ad_timeout(cls, value: float, info: ValidationInfo) -> float: # Must stay FINITE and positive (ASVS 13.1.3): ldap3 treats a None, or a 0 connect_timeout, as # "wait forever", which is exactly the unbounded wait these settings exist to remove, and # inf/NaN are the same hole by another spelling. (A 0 receive_timeout instead makes the socket # non-blocking, so every read fails at once.) Rejected at config load, not at bind time. if not value > 0 or value == float("inf"): raise ValueError( - "ad_connect_timeout / ad_receive_timeout must be a finite number of seconds > 0 " + f"{info.field_name} must be a finite number of seconds > 0 " "(inf, NaN or a None connect timeout would mean an unbounded LDAP wait; 0 or a " "negative value would fail every LDAP read or connect)" ) @@ -3865,7 +3876,7 @@ def _check_ad_timeout(cls, value: float) -> float: # every time. The cap refuses the value at load instead, far below that point. if value > _AD_TIMEOUT_MAX_SECONDS: raise ValueError( - f"ad_connect_timeout / ad_receive_timeout must be at most {_AD_TIMEOUT_MAX_SECONDS:g} " + f"{info.field_name} must be at most {_AD_TIMEOUT_MAX_SECONDS:g} " f"seconds (got {value:g}); the cap keeps the value far below where a socket " "timeout overflows" ) @@ -8258,7 +8269,8 @@ def _auth_limit_loosenings(auth: AuthSettings) -> list[tuple[str, str]]: * ``ad_connect_timeout`` and ``ad_receive_timeout`` (vault BACKLOG #2567) bound how long one directory call may hold a worker thread, so a value above its default is looser, named while AD is on. They are timeouts and not rate limits; they sit here because they are read the same - way, against the shipped default. + way, against the shipped default. So is ``ad_group_search_deadline`` (vault BACKLOG #3696), + the deadline the pages of the nested group search share. * ``step_up_max_age_seconds``, ``totp_skew_steps`` and ``initial_password_expiry_hours`` (vault BACKLOG #2600 step 4) are read the same way too. A step-up window or a temporary-password life above its default is looser, and so is any TOTP skew above the strict 0. A @@ -8628,25 +8640,40 @@ def _window( # default is looser. The load refuses 0, a negative, inf and NaN, so there is no off value. # Named only with AD on, as ad_session_recheck_seconds is: with no directory nothing reads them. if auth.ad_enabled: - for field, what in ( - ("ad_connect_timeout", "the LDAP connect timeout"), + stalled = ( + "a directory call (at least a sign-in, a step-up and a session recheck) to a domain " + "controller that has stopped answering holds a worker thread longer before it fails" + ) + for field, what, so in ( + ("ad_connect_timeout", "the LDAP connect timeout", stalled), # auth/ldap.py hands ldap3 this value rounded UP to a whole second. ( "ad_receive_timeout", "the LDAP response-read timeout (applied rounded up to a whole second)", + stalled, + ), + # Vault BACKLOG #3696. Each page is still bounded by ad_receive_timeout, so this one + # holds the thread only while a slow directory keeps answering. + ( + "ad_group_search_deadline", + "the nested group search's overall deadline", + "the paged group search of a directory call (at least a sign-in, a step-up and " + "a session recheck), against a domain controller that keeps answering slowly, " + "holds a worker thread longer before it is refused", ), ): + if field == "ad_group_search_deadline" and not ( + auth.ad_use_nested_groups and auth.ad_group_search_base + ): + continue # no paged group search runs, so nothing reads it value, default = getattr(auth, field), _auth_default(field) if value > default: out.append( ( field, # repr round-trips, so a value one step past the default never prints as it. - f"{what} is {value!r} s, above the default of {default:g} s, so a " - "directory call (at least a sign-in, a step-up and a session recheck) to " - "a domain controller that has stopped answering holds a worker thread " - "longer before it fails -- stalled calls then tie up the thread pool that " - "sign-in shares for longer", + f"{what} is {value!r} s, above the default of {default:g} s, so {so} -- " + "stalled calls then tie up the thread pool that sign-in shares for longer", ) ) diff --git a/messagefoundry/pipeline/alerts.py b/messagefoundry/pipeline/alerts.py index 621afdf1f..975a7dda2 100644 --- a/messagefoundry/pipeline/alerts.py +++ b/messagefoundry/pipeline/alerts.py @@ -591,13 +591,16 @@ def ad_reconcile_aborted(self, name: str, *, reason: str, probed: int, detail: s which leaves the referred accounts unjudged and may sit beside revocations the pass applied; or one or more probes raised a fault the engine could not read (``probe_fault``, BACKLOG #3100, the ``auth.ad_reconcile_faulted`` row), which leaves those accounts unjudged the same - way. The type's name predates the second and third cases. Read ``reason`` before naming the - cause. + way; or one account's group search was refused for the engine's own bound + (``group_search_deadline`` or ``group_search_cap``, vault BACKLOG #3696), raised once as + the account enters that state, which leaves it unjudged too. The type's name predates all + but the first case. Read ``reason`` before naming the cause. ``name`` labels the source: ``"directory-reconciler"`` for the breaker, - ``"directory-reconciler-referral"`` for a referral, and - ``"directory-reconciler-probe-fault"`` for a probe fault, so each is its own instance and - throttle. ``reason`` is one of those three closed-set slugs. ``probed`` is how many + ``"directory-reconciler-referral"`` for a referral, + ``"directory-reconciler-probe-fault"`` for a probe fault, and + ``"directory-reconciler-paging:"`` for a paging refusal, so each is its own + instance and throttle. ``reason`` is one of those five closed-set slugs. ``probed`` is how many principals the pass probed. ``detail`` is the operator-facing explanation the auth service latches. No PHI. Emitted by the API-lifespan reconciler task, never from ``auth/``. A whole-directory outage is NOT this event: it is diff --git a/tests/test_ad_reconcile_paging_refusal.py b/tests/test_ad_reconcile_paging_refusal.py new file mode 100644 index 000000000..8bcfef537 --- /dev/null +++ b/tests/test_ad_reconcile_paging_refusal.py @@ -0,0 +1,224 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Foundation, LLC and contributors +"""Vault BACKLOG #3696, the Lander's send-back on PR 2431: a revoke that cannot run must page. + +A group search refused for the engine's own bound, past ``[auth].ad_group_search_deadline`` or +past the entry cap, reads that account as UNAVAILABLE, so the reconciler never revokes it while it +lasts. Unlike an outage it recurs on every pass. So it pages through the AlertSink, and audits +``auth.ad_reconcile_paging_refused``, once per account as it enters that state, and again only +after the account reads otherwise. A plain outage still pages nobody. All directory data here is +synthetic. +""" + +from __future__ import annotations + +import json +from typing import Any + +import pytest + +from messagefoundry.api.app import _alert_reconcile_plan +from messagefoundry.auth import reconcile +from messagefoundry.auth.ldap import ( + PAGING_DEADLINE_SPENT, + PAGING_TOO_MANY_ENTRIES, + PAGING_TOO_MANY_PAGES, + LdapError, + LdapReferralError, + LdapUnansweredError, +) +from messagefoundry.auth.service import AuthService +from messagefoundry.store.store import MessageStore +from tests.test_ad_session_reconcile import _ad_settings, _FakeLdap, _principal, _signed_in_ad_user +from tests.test_approval_requester_recheck import _Sink + +_SOURCE = "directory-reconciler-paging:" +_ACTION = "auth.ad_reconcile_paging_refused" + + +def _bounded(code_name: str) -> LdapUnansweredError: + """What ``_search_paged`` raises for a search refused for its bound.""" + return LdapUnansweredError("synthetic: the group search was refused", code_name=code_name) + + +#: The two plain outages: a directory that is down, and one that answers with a code that is not +#: an answer. Neither names a bound. +_OUTAGES: dict[str, LdapError] = { + "unreachable": LdapError("synthetic: LDAP socket closed"), + "unanswered": LdapUnansweredError( + "synthetic: result code 51 (busy)", code=51, code_name="busy" + ), +} + + +async def _service(store: MessageStore, *names: str) -> tuple[AuthService, _FakeLdap, list[str]]: + ldap = _FakeLdap({name: _principal(name) for name in names}) + service = AuthService(store, _ad_settings(), ldap=ldap) # type: ignore[arg-type] + await service.initialize() + tokens = [await _signed_in_ad_user(service, store, name) for name in names] + return service, ldap, [t for t in tokens if t is not None] + + +async def _pages( + service: AuthService, +) -> tuple[reconcile.ReconcilePlan, list[tuple[str, dict[str, Any]]]]: + """One pass, and the ``(source, fields)`` of each paging page the lifespan task raised.""" + plan = await service.reconcile_directory_sessions() + sink = _Sink() + _alert_reconcile_plan(plan, service, sink) + pages = [ + (name, fields) + for kind, name, fields in sink.events + if kind == "ad_reconcile_aborted" and name.startswith(_SOURCE) + ] + return plan, pages + + +async def _rows(store: MessageStore) -> list[tuple[str, str]]: + """``(actor, reason)`` of each paging refusal's audit row.""" + rows = [dict(r) for r in await store.list_audit(action=_ACTION)] + return [(str(r["actor"]), json.loads(str(r["detail"]))["reason"]) for r in rows] + + +@pytest.mark.parametrize( + ("code_name", "cause"), + [ + (PAGING_DEADLINE_SPENT, reconcile.PAGING_DEADLINE), + (PAGING_TOO_MANY_ENTRIES, reconcile.PAGING_CAP), + (PAGING_TOO_MANY_PAGES, reconcile.PAGING_CAP), + ], + ids=["deadline", "entry-cap", "page-backstop"], +) +@pytest.mark.parametrize("alone", [False, True], ids=["beside-an-answer", "outage-shaped-pass"]) +async def test_a_bounded_group_search_pages_once_per_state_change( + code_name: str, cause: str, alone: bool +) -> None: + """The page and its audit row come as the account enters the state, not on the next pass, and + again after the account reads normally and then enters it once more. Alone, the pass is + outage-shaped (``directory_unavailable``), which pages nobody for the outage, and still pages + for this. The enabled account's sessions are not revoked meanwhile: it still reads + UNAVAILABLE.""" + store = await MessageStore.open(":memory:") + try: + names = ("jdoe",) if alone else ("jdoe", "asmith") + service, ldap, tokens = await _service(store, *names) + ldap.broken["jdoe"] = _bounded(code_name) + + plan, pages = await _pages(service) + assert plan.directory_outage is alone + assert [(name, why) for _uid, name, why in plan.paging_refused] == [("jdoe", cause)] + assert [(name, fields["reason"]) for name, fields in pages] == [(f"{_SOURCE}jdoe", cause)] + assert await _rows(store) == [("jdoe", cause)] + + plan, pages = await _pages(service) + assert (plan.paging_refused, pages) == ((), []), "paged again on the next pass" + assert len(await _rows(store)) == 1, "audited again on the next pass" + + del ldap.broken["jdoe"] # the account reads normally again + assert (await _pages(service))[1] == [] + ldap.broken["jdoe"] = _bounded(code_name) + assert len((await _pages(service))[1]) == 1, "did not re-arm" + assert await _rows(store) == [("jdoe", cause), ("jdoe", cause)] + + for token in tokens: + assert await service.identity_for_token(token) is not None + finally: + await store.close() + + +async def test_a_change_of_bound_is_not_a_new_state_and_the_text_names_no_directory_value() -> None: + """An account that alternates between the deadline and the cap stays in one state, so it + pages once. The detail names the setting, not the account, which is in the source only, and + nothing the directory sent.""" + store = await MessageStore.open(":memory:") + try: + service, ldap, _tokens = await _service(store, "jdoe", "asmith") + ldap.broken["jdoe"] = _bounded(PAGING_DEADLINE_SPENT) + [(name, fields)] = (await _pages(service))[1] + assert (name, fields["reason"]) == (f"{_SOURCE}jdoe", reconcile.PAGING_DEADLINE) + assert "refused, past [auth].ad_group_search_deadline" in fields["detail"] + assert len(fields["detail"]) <= 200, "the store keeps only 200 characters of a reason" + assert "synthetic" not in fields["detail"] and "jdoe" not in fields["detail"] + ldap.broken["jdoe"] = _bounded(PAGING_TOO_MANY_ENTRIES) + assert (await _pages(service))[1] == [] + ldap.broken["jdoe"] = _bounded(PAGING_DEADLINE_SPENT) + assert (await _pages(service))[1] == [] + finally: + await store.close() + + +@pytest.mark.parametrize( + "unjudged", + [LdapReferralError("synthetic: referred"), KeyError("synthetic")], + ids=["referral", "probe-fault"], +) +async def test_a_referral_or_fault_does_not_re_arm(unjudged: Exception) -> None: + """A referral or a probe fault judged nothing about the account, so, like an outage, it says + nothing about the bound: the account stays latched and does not page again. Its own detail is + not a bound either, so it raises no paging page of its own.""" + store = await MessageStore.open(":memory:") + try: + service, ldap, _tokens = await _service(store, "jdoe", "asmith") + ldap.broken["jdoe"] = _bounded(PAGING_TOO_MANY_ENTRIES) + assert len((await _pages(service))[1]) == 1 + ldap.broken["jdoe"] = unjudged + assert (await _pages(service))[1] == [], "a referral or fault paged as a paging refusal" + ldap.broken["jdoe"] = _bounded(PAGING_TOO_MANY_ENTRIES) + assert (await _pages(service))[1] == [], "an unjudged answer re-armed the latch" + finally: + await store.close() + + +async def test_a_pass_that_raises_before_the_row_pages_on_the_next_pass( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The latch is set only once the entry is recorded. A pass that raises before then leaves + the account to enter again on the next pass, so its page and row are delayed, never lost.""" + store = await MessageStore.open(":memory:") + try: + service, ldap, _tokens = await _service(store, "jdoe", "asmith") + ldap.broken["jdoe"] = _bounded(PAGING_DEADLINE_SPENT) + calls = 0 + real = service._record_reconcile_fault + + async def raise_once(plan: reconcile.ReconcilePlan) -> None: + nonlocal calls + calls += 1 + if calls == 1: + raise RuntimeError("synthetic: the pass fails part-way") + await real(plan) + + monkeypatch.setattr(service, "_record_reconcile_fault", raise_once) + with pytest.raises(RuntimeError, match="part-way"): + await service.reconcile_directory_sessions() + assert await _rows(store) == [] + assert len((await _pages(service))[1]) == 1, "the entry was lost with the failed pass" + assert await _rows(store) == [("jdoe", reconcile.PAGING_DEADLINE)] + finally: + await store.close() + + +@pytest.mark.parametrize("alone", [False, True], ids=["beside-an-answer", "outage-shaped-pass"]) +@pytest.mark.parametrize("how", sorted(_OUTAGES)) +async def test_a_plain_outage_raises_no_paging_page(alone: bool, how: str) -> None: + """A directory that is down, or answers with a code that is not an answer, keeps its existing + behaviour: UNAVAILABLE, no page or row of this kind. Nor does an outage re-arm a latched + account, so nothing pages again when the outage passes and the bound still refuses.""" + store = await MessageStore.open(":memory:") + try: + names = ("jdoe",) if alone else ("jdoe", "asmith") + service, ldap, _tokens = await _service(store, *names) + ldap.broken["jdoe"] = _OUTAGES[how] + plan, pages = await _pages(service) + assert plan.directory_outage is alone + assert (plan.unavailable, plan.paging_refused, pages) == (1, (), []) + assert await _rows(store) == [] + + ldap.broken["jdoe"] = _bounded(PAGING_DEADLINE_SPENT) + assert len((await _pages(service))[1]) == 1 + ldap.broken["jdoe"] = _OUTAGES[how] + assert (await _pages(service))[1] == [] + ldap.broken["jdoe"] = _bounded(PAGING_DEADLINE_SPENT) + assert (await _pages(service))[1] == [], "an outage re-armed the latch" + finally: + await store.close() diff --git a/tests/test_ad_session_reconcile.py b/tests/test_ad_session_reconcile.py index 8b90835dd..688bcb48f 100644 --- a/tests/test_ad_session_reconcile.py +++ b/tests/test_ad_session_reconcile.py @@ -136,7 +136,8 @@ def __init__( self.unreachable = unreachable or set() #: Usernames whose lookup the directory answers with a referral (BACKLOG #2538). self.referring: set[str] = set() - #: Usernames whose lookup raises something that is not an LdapError (BACKLOG #2241). + #: Usernames whose lookup raises this exception: one that is not an LdapError (BACKLOG + #: #2241), or an LdapError the probe reads by its fields (vault BACKLOG #3696). self.broken: dict[str, Exception] = {} self.probes: list[str] = [] self.probe_keys: list[tuple[str, str]] = [] diff --git a/tests/test_communications_inventory.py b/tests/test_communications_inventory.py index b6dfe83ea..46ee7c560 100644 --- a/tests/test_communications_inventory.py +++ b/tests/test_communications_inventory.py @@ -398,6 +398,11 @@ def _default(model: type, field: str) -> float | int: ), ("ad_connect_timeout", _default(AuthSettings, "ad_connect_timeout"), "config.settings"), ("ad_receive_timeout", _default(AuthSettings, "ad_receive_timeout"), "config.settings"), + ( + "ad_group_search_deadline", + _default(AuthSettings, "ad_group_search_deadline"), + "config.settings", + ), ("forward_host", logging_setup._FORWARD_TCP_TIMEOUT, "logging_setup"), ("ntp_peer", logging_setup._SNTP_TIMEOUT, "logging_setup"), # The token POST does NOT use the shared IdP constant: AuthService._oidc_exchange passes no diff --git a/tests/test_ldap_search_result_codes.py b/tests/test_ldap_search_result_codes.py index 11d7881cd..710538194 100644 --- a/tests/test_ldap_search_result_codes.py +++ b/tests/test_ldap_search_result_codes.py @@ -34,6 +34,7 @@ import pickle import socket import threading +import time from collections.abc import Callable, Iterator from types import SimpleNamespace from typing import Any @@ -66,6 +67,7 @@ from messagefoundry.auth.permissions import Role from messagefoundry.auth.reconcile import ProbeOutcome from messagefoundry.auth.service import AuthService +from messagefoundry.config.settings import AuthSettings from messagefoundry.store.store import MessageStore from tests.test_ad_user_account_control import _alive, _audited, _pass, _signed_in_estate from tests.test_ldap_referrals import ( @@ -445,18 +447,43 @@ def _replay(conn: Any, page: dict[str, Any], wrap: Callable[[Any], object] = lam """What a directory answers the RFC 2696 abandon with: success, no entries, an empty cookie.""" +#: The overall deadline the bare-double tests give a paged search: far past anything they take. +_BUDGET = 60.0 + + +class _FakeSocket: + """The two socket methods :class:`ldap_module._SocketTimeoutCap` calls, recording each timeout + set.""" + + def __init__(self, timeout: float | None) -> None: + self.timeout = timeout + self.set: list[float | None] = [] + + def gettimeout(self) -> float | None: + return self.timeout + + def settimeout(self, value: float | None) -> None: + self.set.append(value) + self.timeout = value + + class _PagedConn: """A bare double that answers its n-th search with ``pages[n]``, recording each search. A search - with a page size of zero is the abandon: it is recorded in ``abandons`` instead.""" + with a page size of zero is the abandon: it is recorded in ``abandons`` instead. ``on_page``, + when given, runs before each page is answered, so a test can move a clock.""" def __init__( - self, pages: list[dict[str, Any]], abandon_raises: Exception | None = None + self, + pages: list[dict[str, Any]], + abandon_raises: Exception | None = None, + on_page: Callable[[], None] | None = None, ) -> None: - self._pages, self._abandon_raises = pages, abandon_raises + self._pages, self._abandon_raises, self._on_page = pages, abandon_raises, on_page self.calls: list[dict[str, Any]] = [] self.abandons: list[dict[str, Any]] = [] self.result: dict[str, Any] | None = None self.entries: list[object] = [] + self.socket: _FakeSocket | None = None def search(self, **kwargs: Any) -> bool: if kwargs.get("paged_size") == 0: @@ -465,6 +492,8 @@ def search(self, **kwargs: Any) -> bool: raise self._abandon_raises return _replay(self, _ABANDONED) self.calls.append(kwargs) + if self._on_page is not None: + self._on_page() return _replay(self, self._pages[min(len(self.calls), len(self._pages)) - 1]) @@ -477,7 +506,11 @@ def test_a_paged_search_reads_every_page_and_sends_each_cookie_back() -> None: _page(RESULT_SUCCESS, [c], b""), # an empty cookie marks the last page (RFC 2696) ] ) - assert ldap_module._search_paged(conn, "group search", search_base=_GROUPS) == [a, b, c] + assert ldap_module._search_paged(conn, "group search", budget=_BUDGET, search_base=_GROUPS) == [ + a, + b, + c, + ] assert [call["paged_cookie"] for call in conn.calls] == [ None, b"synthetic-cookie-1", @@ -493,7 +526,7 @@ def test_a_directory_that_ignores_the_page_request_is_read_once() -> None: with success, so it is the whole answer.""" entry = object() conn = _PagedConn([_page(RESULT_SUCCESS, [entry], None)]) - assert ldap_module._search_paged(conn, "group search") == [entry] + assert ldap_module._search_paged(conn, "group search", budget=_BUDGET) == [entry] assert len(conn.calls) == 1 assert conn.abandons == [] @@ -531,7 +564,7 @@ def test_a_later_page_that_did_not_end_with_success_refuses_the_whole_search( ] ) with pytest.raises(LdapUnansweredError, match="group search") as refused: - ldap_module._search_paged(conn, "group search", search_base=_GROUPS) + ldap_module._search_paged(conn, "group search", budget=_BUDGET, search_base=_GROUPS) assert refused.value.code == code assert len(conn.calls) == 2 assert _abandoned_with(conn) == [abandoned_with] @@ -541,7 +574,7 @@ def test_a_refused_first_page_is_not_abandoned() -> None: """The directory sent no cookie, so it holds no paging state to drop.""" conn = _PagedConn([_page(RESULT_BUSY, [], None)]) with pytest.raises(LdapUnansweredError, match=r"result code 51 \(busy\)"): - ldap_module._search_paged(conn, "group search") + ldap_module._search_paged(conn, "group search", budget=_BUDGET) assert conn.abandons == [] @@ -549,7 +582,7 @@ def test_a_later_page_answered_with_a_referral_is_refused_and_abandoned() -> Non referred = {**_page(10, [], None), "referrals": ["ldaps://dc9.other.invalid/DC=other"]} conn = _PagedConn([_page(RESULT_SUCCESS, [object()], b"synthetic-cookie-1"), referred]) with pytest.raises(LdapReferralError): - ldap_module._search_paged(conn, "group search") + ldap_module._search_paged(conn, "group search", budget=_BUDGET) assert _abandoned_with(conn) == [b"synthetic-cookie-1"] @@ -574,7 +607,7 @@ def test_an_abandon_that_fails_leaves_the_original_refusal( abandon_raises=fault, ) with pytest.raises(LdapUnansweredError, match=r"result code 51 \(busy\)") as refused: - ldap_module._search_paged(conn, "group search") + ldap_module._search_paged(conn, "group search", budget=_BUDGET) assert refused.value is not fault assert len(conn.abandons) == 1 if not isinstance(fault, LdapError): # a refusal is warned by _search; an ldap3 error is not @@ -587,12 +620,250 @@ def test_a_directory_that_never_stops_paging_is_refused(monkeypatch: pytest.Monk monkeypatch.setattr(ldap_module, "_MAX_PAGES", 3) conn = _PagedConn([_page(RESULT_SUCCESS, [object()], b"synthetic-again")]) with pytest.raises(LdapUnansweredError, match="still paging after 3 pages") as refused: - ldap_module._search_paged(conn, "group search", search_base=_GROUPS) + ldap_module._search_paged(conn, "group search", budget=_BUDGET, search_base=_GROUPS) assert len(conn.calls) == 3 assert (refused.value.code, refused.value.code_name) == (None, "too many pages") assert _abandoned_with(conn) == [b"synthetic-again"] +# --- BACKLOG #3696 items 3 and 4: one deadline across the pages, a cap on entries ---------------- + + +def test_a_deadline_below_the_receive_timeout_never_caps_the_first_page( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Review round 2 on PR 2431. A first page slower than the deadline, but inside the socket's + own timeout, is read in full: the deadline refuses the search before its second page, as a + bound the reconciler pages on, not as a timed-out read it would take for an outage.""" + clock = _FakeClock() + monkeypatch.setattr(ldap_module, "_monotonic", clock) + + def slow() -> None: + clock.now += 0.6 + + conn = _PagedConn([_page(RESULT_SUCCESS, [object()], b"synthetic-again")], on_page=slow) + conn.socket = _FakeSocket(10.0) + with pytest.raises(LdapUnansweredError) as refused: + ldap_module._search_paged(conn, "group search", budget=0.5) + assert refused.value.code_name == "deadline spent" + assert len(conn.calls) == 1 + assert conn.socket.set == [10.0], "the first page was capped" + + +def test_the_group_search_deadline_is_its_own_setting(monkeypatch: pytest.MonkeyPatch) -> None: + """The Lander's send-back on PR 2431. The pages share ``[auth].ad_group_search_deadline``, + whole and unrounded, not the per-read ``ad_receive_timeout``, which ships at 10 s.""" + budgets: list[float] = [] + + def record(_conn: Any, _operation: str, *, budget: float, **_kwargs: Any) -> list[Any]: + budgets.append(budget) + return [] + + monkeypatch.setattr(ldap_module, "_search_paged", record) + settings = _ad_settings(ad_receive_timeout=10.0, ad_group_search_deadline=37.5) + LdapAuthenticator(settings)._resolve_groups(object(), _USER_DN, []) + assert budgets == [37.5] + assert AuthSettings().ad_group_search_deadline > AuthSettings().ad_receive_timeout + + +class _FakeClock: + """A monotonic clock that moves only when a test moves it.""" + + def __init__(self) -> None: + self.now = 1000.0 + + def __call__(self) -> float: + return self.now + + +def test_pages_each_inside_the_receive_timeout_but_past_the_deadline_together_are_refused( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """Item 3. Each page takes 6 s, inside a 10 s receive timeout, so no read times out; two pages + spend the 10 s deadline, and the third is never asked for. Each page's reads may wait only + what is left of the deadline, and the socket gets its own timeout back after. The search is + not abandoned: that is one more round trip past the deadline.""" + clock = _FakeClock() + monkeypatch.setattr(ldap_module, "_monotonic", clock) + + def six_seconds() -> None: + clock.now += 6.0 + + conn = _PagedConn([_page(RESULT_SUCCESS, [object()], b"synthetic-again")], on_page=six_seconds) + conn.socket = _FakeSocket(10.0) + caplog.set_level(logging.WARNING, logger=ldap_module.__name__) + with pytest.raises(LdapUnansweredError, match="did not finish paging within 10 s") as refused: + ldap_module._search_paged(conn, "group search", budget=10.0, search_base=_GROUPS) + assert (refused.value.code, refused.value.code_name) == (None, "deadline spent") + assert len(conn.calls) == 2 + assert conn.socket.set == [4.0, 10.0], "capped to what is left, then restored" + assert conn.abandons == [] + assert [r.levelname for r in caplog.records if "within 10 s" in r.getMessage()] == ["WARNING"] + + +@pytest.mark.parametrize( + ("own", "bound"), + [(120.0, _BUDGET), (10.0, 10.0)], + ids=["deadline-shorter-than-the-socket", "never-lengthened"], +) +def test_a_search_that_ends_inside_the_deadline_restores_the_socket_timeout( + own: float, bound: float +) -> None: + """Each later page's reads are capped at the shorter of the deadline and the socket's own + timeout, the first page keeps the socket's own, and the socket gets its own back when the + search ends.""" + conn = _PagedConn( + [ + _page(RESULT_SUCCESS, [object()], b"synthetic-cookie-1"), + _page(RESULT_SUCCESS, [object()], b""), + ] + ) + conn.socket = _FakeSocket(own) + assert len(ldap_module._search_paged(conn, "group search", budget=_BUDGET)) == 2 + *capped, restored = conn.socket.set + assert len(capped) == 1 + assert all(value is not None and bound - 1 < value <= bound for value in capped) + assert restored == own == conn.socket.timeout + + +@pytest.mark.parametrize( + ("page_seconds", "abandoned"), + [(6.0, [b"synthetic-cookie-1"]), (11.0, [])], + ids=["refused-inside-the-deadline-is-abandoned", "refused-past-it-is-not"], +) +def test_a_refused_page_is_abandoned_only_inside_the_deadline( + monkeypatch: pytest.MonkeyPatch, page_seconds: float, abandoned: list[bytes] +) -> None: + """The abandon is one more round trip, so it gets only what is left of the deadline, and is + not sent once the deadline is spent.""" + clock = _FakeClock() + monkeypatch.setattr(ldap_module, "_monotonic", clock) + conn = _PagedConn( + [ + _page(RESULT_SUCCESS, [object()], b"synthetic-cookie-1"), + _page(RESULT_BUSY, [], None), + ], + on_page=lambda: setattr(clock, "now", clock.now + page_seconds / 2), + ) + conn.socket = _FakeSocket(20.0) + with pytest.raises(LdapUnansweredError, match=r"result code 51 \(busy\)"): + ldap_module._search_paged(conn, "group search", budget=10.0) + assert _abandoned_with(conn) == abandoned + if abandoned: # the abandon's reads got what was left: 10 - 6 + assert conn.socket.set[-2] == pytest.approx(4.0) + assert conn.socket.timeout == 20.0 + + +_RECEIVE_ERROR = ldap3.core.exceptions.LDAPSocketReceiveError +_SEND_ERROR = ldap3.core.exceptions.LDAPSocketSendError + + +@pytest.mark.parametrize("kind", [_RECEIVE_ERROR, _SEND_ERROR], ids=["read", "send"]) +@pytest.mark.parametrize( + ("on_call", "own", "as_deadline"), + [ + pytest.param(1, 60.0, False, id="first-page-is-one-slow-read"), + pytest.param(2, 5.0, False, id="later-page-bounded-by-the-socket-timeout"), + pytest.param(2, 60.0, True, id="later-page-bounded-by-the-deadline"), + ], +) +def test_only_a_later_page_timing_out_against_the_deadline_is_refused_as_the_deadline( + monkeypatch: pytest.MonkeyPatch, + kind: type[Exception], + on_call: int, + own: float, + as_deadline: bool, +) -> None: + """ldap3 merges the socket's TimeoutError into its own class. Only a timeout the deadline set + is the deadline: a first page's, or one the socket's own shorter timeout set, passes through + as ldap3's error, as any slow read does.""" + monkeypatch.setattr(ldap_module, "_monotonic", _FakeClock()) + calls = 0 + + def time_out() -> None: + nonlocal calls + calls += 1 + if calls == on_call: + raise ldap3.core.exceptions.communication_exception_factory( + kind, TimeoutError("synthetic") + )("synthetic") + + conn = _PagedConn([_page(RESULT_SUCCESS, [object()], b"synthetic-again")], on_page=time_out) + conn.socket = _FakeSocket(own) + with pytest.raises(LdapUnansweredError if as_deadline else kind) as raised: + ldap_module._search_paged(conn, "group search", budget=10.0) + if as_deadline: + assert isinstance(raised.value, LdapUnansweredError) + assert raised.value.code_name == "deadline spent" + assert isinstance(raised.value.__cause__, kind) + assert conn.abandons == [] + assert conn.socket.timeout == own + + +_ENTRY = object() +"""One entry, repeated: the entry-cap tests count entries and never read one.""" + + +def _pages_of(total: int, size: int) -> list[dict[str, Any]]: + """``total`` entries in pages of ``size``, as a directory with that MaxPageSize returns them: + a cookie on every page but the last, which carries an empty one.""" + counts = [size] * (total // size) + ([total % size] if total % size else []) + return [ + _page( + RESULT_SUCCESS, + [_ENTRY] * count, + b"" if n == len(counts) - 1 else f"synthetic-cookie-{n}".encode(), + ) + for n, count in enumerate(counts) + ] + + +@pytest.mark.parametrize("size", [100, 500]) +@pytest.mark.parametrize("total", [2_500, 10_000]) +def test_an_account_up_to_the_entry_cap_resolves_whatever_the_page_size( + size: int, total: int +) -> None: + """Item 4. Counted in pages, a directory capping a page at 100 refused an account in more than + 2,000 groups, where one returning 500 a page allowed 10,000.""" + conn = _PagedConn(_pages_of(total, size)) + assert len(ldap_module._search_paged(conn, "group search", budget=_BUDGET)) == total + assert conn.abandons == [] + + +@pytest.mark.parametrize( + ("size", "total", "abandoned"), + [ + # The page that crosses the cap is the last, so the directory holds no state to drop. + pytest.param(100, 10_001, [], id="100-last-page"), + pytest.param(500, 10_001, [], id="500-last-page"), + # It is not, so its cookie is the live one. + pytest.param(100, 10_150, [b"synthetic-cookie-100"], id="100-mid-search"), + pytest.param(500, 10_600, [b"synthetic-cookie-20"], id="500-mid-search"), + ], +) +def test_an_account_past_the_entry_cap_is_refused_with_a_warning_whatever_the_page_size( + size: int, total: int, abandoned: list[bytes], caplog: pytest.LogCaptureFixture +) -> None: + caplog.set_level(logging.WARNING, logger=ldap_module.__name__) + conn = _PagedConn(_pages_of(total, size)) + with pytest.raises(LdapUnansweredError, match="returned more than 10,000 entries") as refused: + ldap_module._search_paged(conn, "group search", budget=_BUDGET) + assert (refused.value.code, refused.value.code_name) == (None, "too many entries") + assert len(conn.calls) == -(-10_001 // size), "refused on the page that crossed the cap" + assert _abandoned_with(conn) == abandoned + warned = [r for r in caplog.records if "more than 10,000 entries" in r.getMessage()] + assert [r.levelname for r in warned] == ["WARNING"] + + +def test_the_page_backstop_cannot_bind_before_the_entry_cap() -> None: + """Item 4's loop guard. A directory returning 10 entries a page still reaches the entry cap + before the page count refuses it; the deadline bounds the time either way.""" + conn = _PagedConn(_pages_of(10_001, 10)) + with pytest.raises(LdapUnansweredError) as refused: + ldap_module._search_paged(conn, "group search", budget=_BUDGET) + assert refused.value.code_name == "too many entries" + + _UNREADABLE_CONTROLS = [ pytest.param(None, id="no-result"), pytest.param({"result": 0}, id="no-controls"), @@ -627,7 +898,7 @@ def test_a_later_page_that_drops_the_paging_control_refuses_the_whole_search( later = {**result, "_entries": [object()]} conn = _PagedConn([_page(RESULT_SUCCESS, [object()], b"synthetic-cookie-1"), later]) with pytest.raises(LdapUnansweredError, match="dropped the paging control") as refused: - ldap_module._search_paged(conn, "group search") + ldap_module._search_paged(conn, "group search", budget=_BUDGET) assert (refused.value.code, refused.value.code_name) == (None, "paging control lost") assert len(conn.calls) == 2 assert _abandoned_with(conn) == [b"synthetic-cookie-1"] @@ -845,13 +1116,14 @@ def _scripted_dc( extended_code: int = RESULT_SUCCESS, bind_reply: Callable[[bytes], _Reply | None] | None = None, extended_reply: _Reply | None = None, + **settings_over: Any, ) -> Iterator[tuple[_ScriptedDc, LdapAuthenticator]]: """A loopback server, and an authenticator pointed at it over plain ``ldap://``: the property - under test is the decode, not the hop's TLS.""" + under test is the decode, not the hop's TLS. ``settings_over`` overrides further settings.""" dc = _ScriptedDc(answer, extended_code, bind_reply, extended_reply) try: settings = _ad_settings( - ad_server=f"ldap://127.0.0.1:{dc.port}", ad_allow_insecure_ldap=True + ad_server=f"ldap://127.0.0.1:{dc.port}", ad_allow_insecure_ldap=True, **settings_over ) yield dc, LdapAuthenticator(settings, enforcing=False) finally: @@ -883,7 +1155,9 @@ def _two_pages(raw: bytes) -> list[_Reply]: def test_real_ldap3_pages_the_group_search_and_sends_the_cookie_back() -> None: """ldap3 encodes the request control and decodes the cookie where ``_paged_cookie`` reads it.""" with _scripted_dc(_two_pages) as (dc, authenticator), _service_conn(authenticator) as conn: + before = conn.socket.gettimeout() groups = authenticator._resolve_groups(conn, _USER_DN, []) + assert conn.socket.gettimeout() == before, "the receive timeout is restored (#3696)" assert groups == {ldap_module.canonical_group_dn(_group_dn(n)) for n in (1, 2, 3)} assert len(dc.searches) == 2 assert all(ldap_module._PAGED_RESULTS_OID.encode() in raw for raw in dc.searches) @@ -909,6 +1183,38 @@ def answer(raw: bytes) -> list[_Reply]: assert b"synthetic-cookie-1" in dc.searches[2] +def test_real_ldap3_refuses_a_page_read_that_waits_out_the_deadline( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """BACKLOG #3696 item 3 through a real socket. The second page takes 0.6 s, inside the 2 s + receive timeout, so before the deadline it was read. The first page spends all but 0.2 s of + the 3 s ``ad_group_search_deadline`` on the engine's clock, so the second page's read may wait + only that, and times out. The timeout is refused as the deadline, not passed on as one slow + read. The deadline is its own setting: had the receive timeout been the deadline, the first + page would have spent it, and the second never been asked.""" + clock = _FakeClock() + monkeypatch.setattr(ldap_module, "_monotonic", clock) + + def answer(raw: bytes) -> list[_Reply]: + if b"synthetic-cookie-1" in raw: + time.sleep(0.6) + else: + clock.now += 2.8 # before the reply is sent, so before the engine reads it + return _two_pages(raw) + + with ( + _scripted_dc(answer, ad_receive_timeout=2.0, ad_group_search_deadline=3.0) as ( + dc, + authenticator, + ), + _service_conn(authenticator) as conn, + pytest.raises(LdapUnansweredError, match="did not finish paging within 3 s") as refused, + ): + authenticator._resolve_groups(conn, _USER_DN, []) + assert isinstance(refused.value.__cause__, ldap3.core.exceptions.LDAPSocketReceiveError) + assert len(dc.searches) == 2, "no abandon past the deadline" + + # --- sign-in: a directory error, not an unknown user --------------------------------------------- diff --git a/tests/test_ldap_timeouts.py b/tests/test_ldap_timeouts.py index 9961c22df..8f460c356 100644 --- a/tests/test_ldap_timeouts.py +++ b/tests/test_ldap_timeouts.py @@ -606,6 +606,7 @@ def test_ad_timeout_defaults_are_finite_and_positive() -> None: s = AuthSettings() assert 0 < s.ad_connect_timeout < float("inf") assert 0 < s.ad_receive_timeout < float("inf") + assert 0 < s.ad_group_search_deadline < float("inf") def test_ad_timeouts_are_documented_in_the_operator_configuration_reference() -> None: @@ -614,7 +615,7 @@ def test_ad_timeouts_are_documented_in_the_operator_configuration_reference() -> root = Path(__file__).resolve().parent.parent config = (root / "docs" / "CONFIGURATION.md").read_text(encoding="utf-8") settings = AuthSettings() - for field in ("ad_connect_timeout", "ad_receive_timeout"): + for field in ("ad_connect_timeout", "ad_receive_timeout", "ad_group_search_deadline"): row = [line for line in config.splitlines() if line.strip().startswith(f"| `{field}`")] assert row, ( f"[auth].{field} has no row in docs/CONFIGURATION.md's `[auth]` table; it ships as an " @@ -634,9 +635,13 @@ def test_ad_timeout_rejects_an_unbounded_value(bad: float) -> None: _ad_settings(ad_connect_timeout=bad) with pytest.raises(ValueError, match="finite number of seconds"): _ad_settings(ad_receive_timeout=bad) + with pytest.raises(ValueError, match="ad_group_search_deadline must be a finite number"): + _ad_settings(ad_group_search_deadline=bad) -@pytest.mark.parametrize("field", ["ad_connect_timeout", "ad_receive_timeout"]) +@pytest.mark.parametrize( + "field", ["ad_connect_timeout", "ad_receive_timeout", "ad_group_search_deadline"] +) def test_ad_timeout_rejects_a_value_that_overflows_the_socket(field: str) -> None: """A huge finite value raises OverflowError in socket.settimeout. That now maps to LdapError (BACKLOG #2566, tests/test_ldap_error_mapping.py), but every sign-in would still fail, so the diff --git a/tests/test_security_posture_defaults.py b/tests/test_security_posture_defaults.py index b3fdca613..7a3bd55c6 100644 --- a/tests/test_security_posture_defaults.py +++ b/tests/test_security_posture_defaults.py @@ -305,6 +305,31 @@ def test_an_ad_timeout_without_ad_is_not_a_loosening(field: str) -> None: assert field in _names(auth=_ad(**{field: 3600.0})) +# --- [auth].ad_group_search_deadline (vault BACKLOG #3696) ------------------------------------- + + +def test_a_group_search_deadline_above_its_default_is_a_named_loosening() -> None: + """Named alone, against its own 120 s default rather than the timeouts' 10 s, and only while a + paged group search runs: AD on, nested groups on and a group search base set. At or below the + default it is stricter and is not named.""" + field = "ad_group_search_deadline" + + def grouped(**over: object) -> AuthSettings: + return _ad(ad_group_search_base="OU=Groups,DC=test,DC=invalid", **over) + + risks = dict(_pairs(auth=grouped(**{field: 3600.0}))) + assert "3600.0 s, above the default of 120 s" in risks[field] + assert "worker thread" in risks[field] and "keeps answering slowly" in risks[field] + assert [n for n in risks if n.startswith("ad_")] == [field] + assert "is 120.5 s" in dict(_pairs(auth=grouped(**{field: 120.5})))[field] + for value in (120.0, 30.0): + assert field not in _names(auth=grouped(**{field: value})) + assert field not in _names(auth=AuthSettings(**{field: 3600.0})) # type: ignore[arg-type] + # Nor where no paged group search runs: nested groups off, or no group search base. + assert field not in _names(auth=grouped(**{field: 3600.0, "ad_use_nested_groups": False})) + assert field not in _names(auth=_ad(**{field: 3600.0})) + + # --- [auth].admin_new_ip_step_up (BACKLOG #288) ----------------------------------------------