mirror of
https://gitlab.com/openconnect/ocserv.git
synced 2026-08-09 09:51:49 +08:00
Move to a requirements-first approach
Add doc/requirements/, a structured set of normative requirements extracted from the current ocserv implementation (internal/*.md, generated with the requirements-from-implementation protocol) and from the OpenConnect/AnyConnect protocol sources, reconciled into protocol/unified.md. Update AGENTS.md so that new features and bug fixes are documented as requirements first: find or add the relevant REQ-* entry (with acceptance criteria) and update the implied tests before changing code, and confirm in merge requests that existing requirements and use-cases still hold. This follows partially https://github.com/microsoft/PromptKit Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
This commit is contained in:
+1
-1
@@ -1,4 +1,4 @@
|
|||||||
[codespell]
|
[codespell]
|
||||||
skip = ./.git,aclocal.m4,./autom4te.cache,./build-aux,config.log,ccan,pcl,*.pem,./src/ocserv-fw-nftables,./tests/test-fw-script
|
skip = ./.git,aclocal.m4,./autom4te.cache,./build-aux,config.log,ccan,pcl,*.pem,./src/ocserv-fw-nftables,./tests/test-fw-script
|
||||||
ignore-words-list = alloced,ans,anull,bu,gir,siz,te
|
ignore-words-list = alloced,ans,anull,bu,gir,siz,te,chage
|
||||||
uri-ignore-words-list = gord,ist
|
uri-ignore-words-list = gord,ist
|
||||||
|
|||||||
@@ -56,6 +56,62 @@ solely because they do not work on BSD.
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## Requirements-First Workflow
|
||||||
|
|
||||||
|
`doc/requirements/` is the normative description of what ocserv must do (see
|
||||||
|
`doc/requirements/README.md` for the document map, ID scheme, and per-requirement
|
||||||
|
format). For any change that alters observable behavior — new feature, bug fix, or
|
||||||
|
behavior-changing refactor — work in this order:
|
||||||
|
|
||||||
|
1. **Requirement first.**
|
||||||
|
- Find the requirement(s) covering the area you're changing (search
|
||||||
|
`doc/requirements/` for the relevant `REQ-*` IDs, or use the document map to
|
||||||
|
find the right file by process/subsystem).
|
||||||
|
- If your change alters what an existing requirement describes, **update that
|
||||||
|
requirement first**, re-applying the protocol in `contrib/ai/protocols/` that
|
||||||
|
generated its document so the new entry matches the rest of the file. Do not
|
||||||
|
leave a requirement describing the old behavior once the code changes — but do
|
||||||
|
not break other requirements or use-cases in the process (see below). If a
|
||||||
|
requirement is independently wrong and needs revising for reasons unrelated to
|
||||||
|
this change, do that in its own dedicated merge request instead.
|
||||||
|
- If no requirement covers the new behavior, add one in the appropriate document,
|
||||||
|
following its existing ID prefix, category tags, and per-requirement format.
|
||||||
|
- For bug fixes: if the bug is a violation of an existing requirement, cite its ID
|
||||||
|
in the commit/MR. If the bug reveals a gap in the requirements, extend or add a
|
||||||
|
requirement describing the *correct* behavior before fixing the code.
|
||||||
|
2. **Tests second.** Write or update the positive and negative tests implied by the
|
||||||
|
requirement's **Acceptance** criteria (see "Testing New Functionality" below)
|
||||||
|
*before* touching implementation code. For bug fixes, confirm the new test
|
||||||
|
reproduces the bug and fails against the unmodified code.
|
||||||
|
3. **Code last.** Implement the change so the new/updated tests pass, and so the
|
||||||
|
code, the requirement, and `doc/ocserv.8.md`/`doc/sample.config` (where
|
||||||
|
applicable) all agree.
|
||||||
|
|
||||||
|
Do not jump straight to step 3 — a code change with no corresponding requirement
|
||||||
|
update is incomplete, even if it builds and passes existing tests.
|
||||||
|
|
||||||
|
### Do not break existing requirements or use-cases
|
||||||
|
|
||||||
|
Before submitting a merge request, check that the change does not silently
|
||||||
|
invalidate requirements or use-cases other than the one you set out to change:
|
||||||
|
|
||||||
|
- Search `doc/requirements/` for `REQ-*`/`AC-*`/`OC-*` entries citing the files,
|
||||||
|
functions, or config options you touched, and confirm each still holds. If one no
|
||||||
|
longer holds, either your change is wrong, or that requirement was wrong to begin
|
||||||
|
with (mark it `REVIEW` if unsure which) — never leave code and a `DERIVED`
|
||||||
|
requirement contradicting each other.
|
||||||
|
- A requirement that's wrong for reasons unrelated to your change belongs in its own
|
||||||
|
dedicated merge request — with rationale and evidence it doesn't break other
|
||||||
|
use-cases — not bundled into this one.
|
||||||
|
- Run the full local test suite (`ninja -C build test`), not just the test for the
|
||||||
|
area you changed — requirements frequently span files (e.g. config scope/
|
||||||
|
inheritance, IPC message contracts) and a regression often shows up as a failure
|
||||||
|
in a test you didn't expect to touch.
|
||||||
|
- If your change affects config scope, inheritance, or vhost behavior, also run
|
||||||
|
`tests/check-config-scope.py` (see `doc/requirements/internal/config.md`).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## Build System
|
## Build System
|
||||||
|
|
||||||
The project uses **meson**.
|
The project uses **meson**.
|
||||||
@@ -378,9 +434,14 @@ Before working on a subsystem, read the relevant doc:
|
|||||||
|
|
||||||
### Agent-runnable — verify before declaring a change complete
|
### Agent-runnable — verify before declaring a change complete
|
||||||
|
|
||||||
|
- [ ] Relevant `doc/requirements/` entries added or updated *before* the code
|
||||||
|
change (see "Requirements-First Workflow" above)
|
||||||
- [ ] `clang-format --dry-run -Werror` passes on every modified file under `src/` and `tests/`
|
- [ ] `clang-format --dry-run -Werror` passes on every modified file under `src/` and `tests/`
|
||||||
- [ ] `ninja -C build` succeeds
|
- [ ] `ninja -C build` succeeds
|
||||||
- [ ] Relevant test passes: `meson test -C build <test-name>`
|
- [ ] Relevant test passes: `meson test -C build <test-name>`
|
||||||
|
- [ ] Full suite passes: `ninja -C build test` (no regressions in unrelated tests)
|
||||||
|
- [ ] Existing `doc/requirements/` entries touching the changed files/functions
|
||||||
|
still hold (none silently contradicted)
|
||||||
- [ ] Every commit has `Signed-off-by:`
|
- [ ] Every commit has `Signed-off-by:`
|
||||||
|
|
||||||
### Human-judgment required — flag in the MR, do not decide unilaterally
|
### Human-judgment required — flag in the MR, do not decide unilaterally
|
||||||
|
|||||||
@@ -13,6 +13,15 @@ See [`AGENTS.md`](../../AGENTS.md) in the repository root for the full agent gui
|
|||||||
|
|
||||||
Load the appropriate file as a system prompt prefix in your AI tool before starting work.
|
Load the appropriate file as a system prompt prefix in your AI tool before starting work.
|
||||||
|
|
||||||
|
## Requirements
|
||||||
|
|
||||||
|
The `protocols/requirements-*.md` files in this directory generate the
|
||||||
|
structured requirements documents under
|
||||||
|
[`doc/requirements/`](../../doc/requirements/README.md). When extending those
|
||||||
|
documents, re-apply the same protocol that originally generated the target
|
||||||
|
file (see the document map in `doc/requirements/README.md`) rather than
|
||||||
|
hand-writing requirements in a different style.
|
||||||
|
|
||||||
## Acknowledgment
|
## Acknowledgment
|
||||||
|
|
||||||
The structure of this framework — composable persona and protocol files that can be
|
The structure of this framework — composable persona and protocol files that can be
|
||||||
|
|||||||
@@ -6,6 +6,11 @@ the worker processes. The following sections describe the purpose and tasks
|
|||||||
assigned to each component, and the last section describes the communication
|
assigned to each component, and the last section describes the communication
|
||||||
protocol between them.
|
protocol between them.
|
||||||
|
|
||||||
|
For normative, testable requirements derived from this design and from the
|
||||||
|
current implementation, see [`doc/requirements/`](requirements/README.md).
|
||||||
|
This document explains *how* ocserv works; `doc/requirements/` states *what
|
||||||
|
must hold*.
|
||||||
|
|
||||||
|
|
||||||
# VPN overview
|
# VPN overview
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,134 @@
|
|||||||
|
# ocserv Requirements
|
||||||
|
|
||||||
|
This directory contains structured, testable requirements derived from the
|
||||||
|
ocserv implementation and from the protocols ocserv must interoperate with
|
||||||
|
(OpenConnect, Cisco AnyConnect). It complements `doc/design.md`:
|
||||||
|
|
||||||
|
- **`doc/design.md`** — narrative description of *how* ocserv works
|
||||||
|
(process model, IPC sequences, cookie lifecycle).
|
||||||
|
- **`doc/requirements/`** — normative description of *what must hold*,
|
||||||
|
in atomic, testable statements with RFC 2119 keywords, source citations,
|
||||||
|
and acceptance criteria.
|
||||||
|
|
||||||
|
Each requirement links back to the `design.md` section that explains the
|
||||||
|
context, and forward to the test (if any) that verifies it. When code,
|
||||||
|
design.md, and a requirement disagree, treat it as a `[REVIEW]` item, not
|
||||||
|
as an automatic override — resolve by reading the cited source and, if
|
||||||
|
still unclear, ask a maintainer.
|
||||||
|
|
||||||
|
## Generation protocols
|
||||||
|
|
||||||
|
These documents are generated and maintained using the reasoning protocols
|
||||||
|
in `contrib/ai/protocols/`:
|
||||||
|
|
||||||
|
| Directory | Protocol | Produces |
|
||||||
|
|-----------|----------|----------|
|
||||||
|
| `internal/` | `requirements-from-implementation.md` | Requirements derived from current ocserv source: what each process (main, sec-mod, worker) and the IPC layer between them actually guarantee. |
|
||||||
|
| `protocol/sources/` | `requirements-elicitation.md` | Per-source requirement extractions (OpenConnect protocol draft, observed AnyConnect behavior). Working artifacts, not the final spec. |
|
||||||
|
| `protocol/unified.md` | `requirements-reconciliation.md` | A single reconciled wire-protocol spec, merging `protocol/sources/*` with the `OCSERV` implementation and relevant TLS/DTLS RFCs, classifying agreement (UNIVERSAL/MAJORITY/DIVERGENT/EXTENSION). |
|
||||||
|
|
||||||
|
When adding to or updating a document, re-apply the protocol that generated
|
||||||
|
it — do not hand-write requirements in a different style than the rest of
|
||||||
|
the file.
|
||||||
|
|
||||||
|
## Document map
|
||||||
|
|
||||||
|
| Document | ID prefix | Process(es) | Sources |
|
||||||
|
|----------|-----------|-------------|---------|
|
||||||
|
| `internal/ipc.md` | `REQ-IPC-` | all (cross-process) | `src/ipc.proto`, `src/ctl.proto`, `doc/design.md#ipc-communication*` |
|
||||||
|
| `internal/config.md` | `REQ-CONFIG-` | all (cross-process) | `src/config.c`, `src/config-ports.c`, `src/config-kkdcp.c`, `src/subconfig.c`, `src/sup-config/file.c`, `src/cfg.proto`, `src/vpn.h`, `src/vhost.h`, `doc/sample.config`, `tests/check-config-scope.py`, `tests/config-inherit.c` |
|
||||||
|
| `internal/sec-mod.md` | `REQ-SECMOD-` | sec-mod | `src/sec-mod*.c`, `src/sec-mod-auth.h`, `src/auth/*`, `src/acct/*` |
|
||||||
|
| `internal/authentication.md` | `REQ-AUTH-` | sec-mod (primary), worker, main | `src/sec-mod-auth.{c,h}`, `src/auth/*`, `src/acct/*`, `src/config.c`, `src/subconfig.c`, `src/worker-auth.c`, `doc/sample.config` |
|
||||||
|
| `internal/main.md` | `REQ-MAIN-` | main | `src/main.c`, `src/main-*.c` |
|
||||||
|
| `internal/worker.md` | `REQ-WORKER-` | worker | `src/worker.c`, `src/worker-*.c` |
|
||||||
|
| `protocol/sources/openconnect.md` | `OC-PROTO-` | n/a (external spec) | `~/projects/openconnect/protocol/draft-openconnect.xml` |
|
||||||
|
| `protocol/sources/anyconnect.md` | `AC-CLIENT-` | n/a (observed client behavior) | `doc/README-cisco-svc.md`, worker CSTP/HTTSP handling |
|
||||||
|
| `protocol/unified.md` | `REQ-PROTO-` | worker (mostly) | reconciles `OC-PROTO-*`, `AC-CLIENT-*`, RFC-TLS/RFC-DTLS, `OCSERV` (= `internal/worker.md` + code) |
|
||||||
|
|
||||||
|
`internal/ipc.md` is generated first — the other `internal/*` documents cite
|
||||||
|
its `REQ-IPC-*` entries wherever a behavior crosses a process boundary,
|
||||||
|
instead of restating the IPC contract.
|
||||||
|
|
||||||
|
## ID scheme
|
||||||
|
|
||||||
|
```
|
||||||
|
REQ-<PREFIX><CATEGORY>-<NNN>
|
||||||
|
```
|
||||||
|
|
||||||
|
- `<PREFIX>` identifies the document (table above); `IPC` has no
|
||||||
|
further category — IDs are `REQ-IPC-NNN`, grouped by message name in
|
||||||
|
the document body.
|
||||||
|
- `<CATEGORY>` for `internal/*` documents uses the tags from
|
||||||
|
`requirements-from-implementation.md`: `INIT`, `AUTH`, `ACCT`,
|
||||||
|
`SESSION`, `CFG`, `NET`, `SEC`, `ERR`, `TEARDOWN`.
|
||||||
|
- `<CATEGORY>` for `protocol/unified.md` uses the tags from
|
||||||
|
`requirements-reconciliation.md`: `CONN`, `AUTH`, `SESSION`, `DATA`,
|
||||||
|
`CTRL`, `CFG`, `COMPAT`, `SEC`, `EXT`.
|
||||||
|
- `<NNN>` is a 3-digit sequence number, unique within
|
||||||
|
`<PREFIX><CATEGORY>` and never reused (if a requirement is removed,
|
||||||
|
mark it `WITHDRAWN`, do not renumber).
|
||||||
|
|
||||||
|
Examples: `REQ-AUTH-AUTH-001`, `REQ-WORKER-NET-003`, `REQ-IPC-014`,
|
||||||
|
`REQ-PROTO-COMPAT-002`.
|
||||||
|
|
||||||
|
`protocol/sources/*.md` use their own non-normative working IDs
|
||||||
|
(`OC-PROTO-<CAT>-<NNN>`, `AC-CLIENT-<CAT>-<NNN>`) — these are inputs to
|
||||||
|
`unified.md` and are not cited from `internal/*`.
|
||||||
|
|
||||||
|
## Status legend
|
||||||
|
|
||||||
|
Every requirement carries a `Status`:
|
||||||
|
|
||||||
|
| Status | Meaning |
|
||||||
|
|--------|---------|
|
||||||
|
| `DERIVED` | Directly supported by current code/spec; no open questions. |
|
||||||
|
| `REVIEW` | Behavior observed but contradicts documentation, another requirement, or looks like a possible defect — needs a maintainer decision. |
|
||||||
|
| `AMBIGUOUS` | Cannot be classified as essential/incidental without domain knowledge; two interpretations given. |
|
||||||
|
| `UNDOCUMENTED` | Behavior exists in code with no doc, test, or evident purpose. |
|
||||||
|
| `WITHDRAWN` | Previously published requirement no longer applies; kept for ID stability, with a note explaining why. |
|
||||||
|
|
||||||
|
`protocol/unified.md` additionally carries a `Class` per
|
||||||
|
`requirements-reconciliation.md`: `UNIVERSAL`, `MAJORITY`, `DIVERGENT`,
|
||||||
|
`EXTENSION`.
|
||||||
|
|
||||||
|
## Per-requirement format
|
||||||
|
|
||||||
|
```markdown
|
||||||
|
### REQ-<PREFIX><CAT>-<NNN>
|
||||||
|
**Requirement:** <system/process> MUST/SHOULD/MAY <behavior> when
|
||||||
|
<condition>, so that <rationale>.
|
||||||
|
**Strength:** MUST | SHOULD | MAY | MUST NOT | SHOULD NOT
|
||||||
|
**Status:** DERIVED | REVIEW | AMBIGUOUS | UNDOCUMENTED | WITHDRAWN
|
||||||
|
**Source:** <file>:<line> [, ...] ; doc/design.md#<section>
|
||||||
|
**Acceptance:** <test path or description> — positive|negative|unit ;
|
||||||
|
local | CI (root/full stack)
|
||||||
|
**Links:** <other REQ-IDs this depends on or relates to>
|
||||||
|
```
|
||||||
|
|
||||||
|
For `protocol/unified.md`, add **Class** and, for non-UNIVERSAL entries,
|
||||||
|
**Divergence:** describing what differs between sources and why.
|
||||||
|
|
||||||
|
## Document frontmatter
|
||||||
|
|
||||||
|
Each requirements document opens with:
|
||||||
|
|
||||||
|
```yaml
|
||||||
|
---
|
||||||
|
title: <short title>
|
||||||
|
generator: requirements-from-implementation | requirements-elicitation | requirements-reconciliation
|
||||||
|
process: main | sec-mod | worker | ipc | n/a
|
||||||
|
id-prefix: REQ-<PREFIX>
|
||||||
|
sources:
|
||||||
|
- <file or glob>
|
||||||
|
- doc/design.md#<section>
|
||||||
|
---
|
||||||
|
```
|
||||||
|
|
||||||
|
## Conventions carried over from `contrib/ai/protocols/`
|
||||||
|
|
||||||
|
- **Negative requirements are mandatory for `SEC`, `AUTH`, and `IPC`**
|
||||||
|
categories — write the MUST NOT before the MUST.
|
||||||
|
- **Privilege boundary violations are never `[UNDOCUMENTED]` or
|
||||||
|
incidental** — they are `SEC` requirements, always essential.
|
||||||
|
- **IPC acceptance criteria must cite protobuf field names** from
|
||||||
|
`src/ipc.proto` / `src/ctl.proto`, not vague descriptions.
|
||||||
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,470 @@
|
|||||||
|
---
|
||||||
|
title: configuration subsystem requirements
|
||||||
|
generator: requirements-from-implementation
|
||||||
|
process: main, sec-mod, worker (shared)
|
||||||
|
id-prefix: REQ-CONFIG
|
||||||
|
sources:
|
||||||
|
- src/config.c
|
||||||
|
- src/config-ports.c
|
||||||
|
- src/config-kkdcp.c
|
||||||
|
- src/subconfig.c
|
||||||
|
- src/sup-config/file.c
|
||||||
|
- src/cfg.proto
|
||||||
|
- src/vpn.h
|
||||||
|
- src/vhost.h
|
||||||
|
- doc/sample.config
|
||||||
|
- tests/check-config-scope.py
|
||||||
|
- tests/config-inherit.c
|
||||||
|
---
|
||||||
|
|
||||||
|
# Configuration Subsystem Requirements
|
||||||
|
|
||||||
|
`src/config.c` (plus `src/config-ports.c`, `src/config-kkdcp.c`,
|
||||||
|
`src/subconfig.c`, `src/sup-config/file.c`) implements `ocserv.conf` parsing
|
||||||
|
via `inih`. It is linked into all three processes — main, sec-mod (each
|
||||||
|
sec-mod instance reloads its own copy), and the worker (which re-parses a
|
||||||
|
root-created snapshot rather than the live file, REQ-CONFIG-SEC-001) — so
|
||||||
|
this document is cross-process, like `internal/ipc.md`.
|
||||||
|
|
||||||
|
The central design event covered here is commit `139ff827` ("config:
|
||||||
|
restructure per-vhost configuration for clarity and maintainability",
|
||||||
|
resolves #705): it split per-vhost configuration into a protobuf-generated
|
||||||
|
`ReloadableConfig` (`src/cfg.proto`, SIGHUP-reloadable) and a `static_cfg_st`
|
||||||
|
(`src/vpn.h`, restart-only), and introduced named-vhost inheritance from the
|
||||||
|
default vhost via a pack/unpack round-trip for the former and explicit
|
||||||
|
field-by-field copy for the latter.
|
||||||
|
|
||||||
|
## INIT
|
||||||
|
|
||||||
|
### REQ-CONFIG-INIT-001 — `ocserv.conf` is parsed as INI with `[vhost:<name>]` sections; the unnamed/first vhost is the default
|
||||||
|
|
||||||
|
**Requirement:** `parse_cfg_file()` MUST parse the configuration file with
|
||||||
|
`ini_parse()` and `cfg_ini_handler()`. A line with no enclosing section
|
||||||
|
applies to the **default vhost** (`vhost->name == NULL`, always the list
|
||||||
|
tail per `default_vhost()`). A section header `[vhost:<name>]` MUST select
|
||||||
|
or create (`vhost_add()`) a named vhost for all subsequent lines until the
|
||||||
|
next section header; any other non-empty section name MUST be rejected with
|
||||||
|
a `skipping unknown section` warning (non-fatal, line ignored) unless
|
||||||
|
`reload` or `is_worker` is set (in which case the warning is suppressed).
|
||||||
|
Virtual host names are canonicalized via `sanitize_name()`/`idna_map()`; a
|
||||||
|
canonicalization that changes the name MUST print a `note:` (suppressed
|
||||||
|
under reload/worker).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/config.c:943-1030 (`cfg_ini_handler`), src/vhost.h:115-122
|
||||||
|
(`GETVHOST`/`default_vhost`)
|
||||||
|
**Acceptance:** unit, local — a config with one `[vhost:example]` section
|
||||||
|
and no other vhost sections MUST produce exactly two `vhost_cfg_st` entries
|
||||||
|
(default + `example`); `find_vhost(head, "example")` MUST return the named
|
||||||
|
one, `find_vhost(head, NULL)` and `find_vhost(head, "unknown")` MUST both
|
||||||
|
return the default. Negative: a config containing `[bogus]` MUST NOT abort
|
||||||
|
parsing (line ignored, warning printed on first parse only).
|
||||||
|
**Links:** REQ-CONFIG-CFG-002, REQ-CONFIG-CFG-003
|
||||||
|
|
||||||
|
### REQ-CONFIG-INIT-002 — Two-tier per-vhost config: `ReloadableConfig` (SIGHUP-reloadable) vs `static_cfg_st` (restart-only)
|
||||||
|
|
||||||
|
**Requirement:** Every `vhost_cfg_st` MUST carry exactly two configuration
|
||||||
|
sub-objects with disjoint lifetimes: `config` (`ReloadableConfig *`,
|
||||||
|
protobuf-generated from `src/cfg.proto`, replaced wholesale on every
|
||||||
|
`reload_cfg_file()`) and `static_config` (`struct static_cfg_st`, `src/vpn.h`,
|
||||||
|
populated once at startup and never replaced by reload). Code MUST access
|
||||||
|
these only via `GETRCONFIG(s)` / `GETSCONFIG(s)` (`src/vhost.h:116-117`),
|
||||||
|
never by reaching into `vhost_cfg_st` fields directly, so that the
|
||||||
|
reload/no-reload distinction stays enforced at a single point. Adding a new
|
||||||
|
*reloadable* field requires only editing `cfg.proto` (regenerate with
|
||||||
|
`protoc-c` per `AGENTS.md`); adding a new *restart-only* field requires
|
||||||
|
editing `static_cfg_st` in `src/vpn.h` AND (per REQ-CONFIG-CFG-003) deciding
|
||||||
|
its inheritance behavior in `vhost_inherit_static_config()`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/vhost.h:35-78, 101-117; src/cfg.proto; src/vpn.h:224-275
|
||||||
|
**Acceptance:** `grep -rn '\->config\.\|->static_config\.' src/*.c` (outside
|
||||||
|
`config.c`/`vhost.h`) MUST find no direct struct-field access bypassing
|
||||||
|
`GETRCONFIG`/`GETSCONFIG` — `[OPEN]`, not run in this pass; recommended as a
|
||||||
|
lint/CI check given how easy it is to reintroduce direct access after #705.
|
||||||
|
**Links:** REQ-CONFIG-CFG-001, REQ-CONFIG-CFG-002, REQ-CONFIG-CFG-003
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## CFG — scope, inheritance, defaults, reload
|
||||||
|
|
||||||
|
### REQ-CONFIG-CFG-001 — Every config option has exactly one of five `[scope: ...]` annotations, enforced by `tests/check-config-scope.py`
|
||||||
|
|
||||||
|
**Requirement:** Every option documented in `doc/sample.config` MUST carry a
|
||||||
|
`# [scope: ...]` annotation from a closed vocabulary of five values:
|
||||||
|
`global (non-reloadable)`, `vhost (non-reloadable)`, `global`, `vhost`,
|
||||||
|
`vhost user`. Every field of `struct cfg_st`/`struct static_cfg_st` in
|
||||||
|
`src/vpn.h` MUST carry a matching inline `[scope: ...]` comment.
|
||||||
|
`tests/check-config-scope.py` MUST pass, enforcing the following
|
||||||
|
cross-references (its checks a–h):
|
||||||
|
- (a) every `sample.config` option has a scope annotation;
|
||||||
|
- (b) every reloadable `[... global]` option has a matching
|
||||||
|
`error_on_vhost()` call in `config.c` (REQ-CONFIG-CFG-004);
|
||||||
|
- (c) every `error_on_vhost()` call in `config.c` is annotated
|
||||||
|
`[... global]` in `sample.config` (modulo
|
||||||
|
`DEPRECATED_GLOBAL_ALIASES`);
|
||||||
|
- (d)/(e) every `[...user...]`-annotated option is handled in
|
||||||
|
`src/sup-config/file.c` and vice versa (REQ-CONFIG-CFG-006);
|
||||||
|
- (f) every `[...user...]` option is also `[...vhost...]` (per-user
|
||||||
|
overrides require a per-vhost base);
|
||||||
|
- (g) every `cfg_st`/`static_cfg_st` field in `src/vpn.h` has a
|
||||||
|
`[scope:]` comment;
|
||||||
|
- (h) every `[scope: vhost (non-reloadable)]` field in `static_cfg_st` is
|
||||||
|
referenced in `vhost_inherit_static_config()` or listed in
|
||||||
|
`STATIC_VHOST_INHERIT_EXCEPTIONS` (REQ-CONFIG-CFG-003).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** tests/check-config-scope.py (full file); src/vpn.h:224-275
|
||||||
|
(`static_cfg_st` annotations)
|
||||||
|
**Acceptance:** `meson test -C build check-config-scope` (or
|
||||||
|
`tests/check-config-scope.py` directly) exits 0. Negative: adding a new
|
||||||
|
option to `sample.config` without a `[scope:]` line, or a new
|
||||||
|
`static_cfg_st` field without a `[scope:]` comment, MUST fail check (a) or
|
||||||
|
(g) respectively — this is the project's primary safeguard against
|
||||||
|
config/doc/code drift for scope semantics.
|
||||||
|
**Links:** REQ-CONFIG-CFG-002, REQ-CONFIG-CFG-003, REQ-CONFIG-CFG-004,
|
||||||
|
REQ-CONFIG-CFG-006
|
||||||
|
|
||||||
|
### REQ-CONFIG-CFG-002 — Named vhosts inherit `ReloadableConfig` from the default vhost via a protobuf pack/unpack deep copy, taken on first entry into the `[vhost:NAME]` section
|
||||||
|
|
||||||
|
**Requirement:** The first time `cfg_ini_handler()` encounters a
|
||||||
|
`[vhost:NAME]` section for a given vhost (`vhost->cfg_inherited == 0`), and
|
||||||
|
**before** parsing any option in that section, it MUST replace
|
||||||
|
`vhost->config` with `cfg_copy_from_default(defvhost->config, vhost->pool)`
|
||||||
|
— a full protobuf `pack()`/`unpack()` round-trip of the default vhost's
|
||||||
|
*current* `ReloadableConfig` (including all scalars, strings, repeated
|
||||||
|
fields, and sub-messages such as `NetworkConfig`), reallocated under
|
||||||
|
`vhost->pool` via a talloc-backed `ProtobufCAllocator` so a single
|
||||||
|
`talloc_free(vhost->config)` releases the entire copy. It MUST then set
|
||||||
|
`vhost->cfg_inherited = 1` so the copy happens at most once per
|
||||||
|
parse/reload pass. Options subsequently parsed in the `[vhost:NAME]` section
|
||||||
|
overlay this inherited snapshot field-by-field.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/config.c:540-568 (`cfg_copy_from_default`), 1014-1029
|
||||||
|
(call site); tests/config-inherit.c (phases 1–3)
|
||||||
|
**Acceptance:** tests/config-inherit.c phase 1 — a `[vhost:named]` section
|
||||||
|
that sets only `auth`/`server-cert`/`server-key`/`ipv4-network`/
|
||||||
|
`ipv4-netmask` MUST end up with `keepalive`, `dpd`, `idle_timeout`,
|
||||||
|
`cookie_timeout`, `deny_roaming`, `banner`, `default_domain`, `rx_per_sec`,
|
||||||
|
and `default_mtu` all equal to the default vhost's values
|
||||||
|
(`check_inherited()`). Phase 2 — if the named vhost additionally sets
|
||||||
|
`keepalive = 55555`, only `keepalive` differs; all other inherited fields
|
||||||
|
are unchanged. Phase 3 — both properties survive `reload_cfg_file()`.
|
||||||
|
**Divergence / ordering caveat `[REVIEW]`**: the snapshot is of the default
|
||||||
|
vhost's `ReloadableConfig` **as parsed up to that point in the file** — if
|
||||||
|
`doc/sample.config`'s convention of "default vhost options first, named
|
||||||
|
`[vhost:...]` sections last" is violated (a default-vhost option appears
|
||||||
|
*after* a `[vhost:NAME]` section in the file), that option will **not** be
|
||||||
|
inherited by vhosts whose `[vhost:NAME]` section already triggered the
|
||||||
|
copy. This ordering dependency is not validated by `check_cfg()` or
|
||||||
|
documented in `doc/ocserv.8.md`/`doc/sample.config`. `[CANDIDATE for doc
|
||||||
|
addition]`: document the required section ordering, or `[OPEN]`: consider
|
||||||
|
deferring the inheritance copy to the post-parse loop
|
||||||
|
(`src/config.c:1700-1799`, where `vhost_inherit_static_config()` already
|
||||||
|
runs) so it is order-independent like REQ-CONFIG-CFG-003.
|
||||||
|
**Links:** REQ-CONFIG-CFG-001, REQ-CONFIG-CFG-003, REQ-CONFIG-CFG-005,
|
||||||
|
REQ-CONFIG-CFG-007
|
||||||
|
|
||||||
|
### REQ-CONFIG-CFG-003 — Named vhosts inherit `static_cfg_st` fields via `vhost_inherit_static_config()`, run once after the full file is parsed
|
||||||
|
|
||||||
|
**Requirement:** After `ini_parse()` completes, `parse_cfg_file()` MUST walk
|
||||||
|
all vhosts in reverse-add order (default vhost first, per
|
||||||
|
`list_for_each_rev`) and, for every named vhost, call
|
||||||
|
`vhost_inherit_static_config(vhost, defvhost)`. This function MUST:
|
||||||
|
- **unconditionally** copy every `[scope: global (non-reloadable)]` field
|
||||||
|
(`port`, `udp_port`, `uid`, `gid`, `sec_mod_scale`, `stats_reset_time`,
|
||||||
|
`socket_file_prefix`, `occtl_socket_file`, `chroot_dir`) from the default
|
||||||
|
vhost — named vhosts cannot override these regardless of what they set;
|
||||||
|
- **conditionally** copy every `[scope: vhost (non-reloadable)]` field
|
||||||
|
(`cert`/`cert_size`, `key`/`key_size`, `ca`, `dh_params_file`,
|
||||||
|
`cert_hash` (`ANYCONNECT_CLIENT_COMPAT`), `pin_file`, `srk_pin_file`,
|
||||||
|
`key_pin`, `srk_pin`, `auth`/`auth_methods`, `acct`) **only if** the
|
||||||
|
named vhost left that field unset (NULL / zero-size / zero-count) —
|
||||||
|
"inherit if not set, else keep the vhost's own value";
|
||||||
|
- **exclude** `sup_config_type`: `cfg_alloc_vhost()` pre-sets it to
|
||||||
|
`SUP_CONFIG_FILE` (non-zero) for every vhost, so there is no zero
|
||||||
|
sentinel to distinguish "not set" from "explicitly set to the default" —
|
||||||
|
a named vhost needing a different `sup-config` MUST set it explicitly.
|
||||||
|
This exclusion is enumerated in `STATIC_VHOST_INHERIT_EXCEPTIONS`
|
||||||
|
(REQ-CONFIG-CFG-001(h)).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/config.c:1766-1768 (call site, post-parse loop),
|
||||||
|
1820-1915 (`vhost_inherit_static_config`, `VHOST_INHERIT*` macros);
|
||||||
|
tests/config-inherit.c phase 4
|
||||||
|
**Acceptance:** tests/config-inherit.c phase 4 — a `[vhost:named]` section
|
||||||
|
that sets only `ipv4-network`/`ipv4-netmask` (omitting `auth`,
|
||||||
|
`server-cert`, `server-key` entirely) MUST end up with
|
||||||
|
`named->static_config.cert`/`cert_size`, `key`/`key_size`, and
|
||||||
|
`auth`/`auth_methods` all equal to the default vhost's (deep-copied
|
||||||
|
strings, not shared pointers, except `acct`/`auth[].auth_ctx`/`dl_ctx` which
|
||||||
|
are intentionally shared read-only module state per the function's
|
||||||
|
docstring). Negative: a named vhost that DOES set its own `auth`/cert/key
|
||||||
|
MUST keep them — `vhost_inherit_static_config` must not clobber
|
||||||
|
explicitly-set values.
|
||||||
|
**Links:** REQ-CONFIG-CFG-001, REQ-CONFIG-CFG-002
|
||||||
|
|
||||||
|
### REQ-CONFIG-CFG-004 — `[scope: ... global]` reloadable options MUST be rejected inside `[vhost:NAME]` sections via `error_on_vhost()`
|
||||||
|
|
||||||
|
**Requirement:** When `cfg_ini_handler()` parses an option annotated
|
||||||
|
`[scope: global]` or `[scope: global (non-reloadable)]` in `sample.config`
|
||||||
|
(e.g. `listen-host`, `udp-listen-host`, `tcp-port`, `udp-port`,
|
||||||
|
`run-as-user`, `run-as-group`, `socket-file`, `occtl-socket-file`,
|
||||||
|
`chroot-dir`, `server-stats-reset-time` and others — see
|
||||||
|
`tests/check-config-scope.py` checks (b)/(c) for the full set) while
|
||||||
|
`vhost->name != NULL` (i.e. inside a `[vhost:NAME]` section), it MUST call
|
||||||
|
`error_on_vhost(vhost->name, "<option-name>")`, which prints `'<option-name>'
|
||||||
|
cannot be set inside a virtual host section` to stderr and returns `true`;
|
||||||
|
the calling code path MUST then skip applying the value (the option is a
|
||||||
|
no-op inside the vhost section, not a fatal error — parsing continues).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/config.c:893-904 (`error_on_vhost`), 1047-1320 (call sites)
|
||||||
|
**Acceptance:** [SEC/CFG] negative — a config with `tcp-port = 9000` inside
|
||||||
|
a `[vhost:example]` section MUST print the `error_on_vhost` message, MUST
|
||||||
|
NOT change `example`'s `static_config.port` (it remains inherited from the
|
||||||
|
default per REQ-CONFIG-CFG-003), and parsing MUST continue (not
|
||||||
|
`exit(EXIT_FAILURE)`) — confirm via `check-config-scope.py` test (b)/(c) plus
|
||||||
|
a targeted parse test.
|
||||||
|
**Links:** REQ-CONFIG-CFG-001, REQ-CONFIG-CFG-003
|
||||||
|
|
||||||
|
### REQ-CONFIG-CFG-005 — Reloadable scalar defaults are applied via `apply_default_conf()` with explicit `has_<field>=1`, before parsing and before any inheritance copy
|
||||||
|
|
||||||
|
**Requirement:** `cfg_new()` (called for every vhost at `vhost_add()` time,
|
||||||
|
and again per-vhost on every reload via `reload_cfg_file()`) MUST call
|
||||||
|
`apply_default_conf()` immediately after `reloadable_config__init()`. For
|
||||||
|
every `ReloadableConfig` optional scalar with a non-zero/non-empty default
|
||||||
|
(`rekey_time`, `cookie_timeout`, `auth_timeout`, `ban_time`,
|
||||||
|
`ban_reset_time`, `max_ban_score`, `ban_points_wrong_password`,
|
||||||
|
`ban_points_connect`, `ban_points_kkdcp`, `dpd`, `mobile_dpd`, `keepalive`,
|
||||||
|
`switch_to_tcp_timeout`, `mobile_idle_timeout`,
|
||||||
|
`network->ipv6_subnet_prefix`, plus boolean-with-default-true fields
|
||||||
|
`dtls_legacy`, `dtls_psk`, `predictable_ips`, `use_utmp`, and (if
|
||||||
|
`ENABLE_COMPRESSION`) `no_compress_limit`), `apply_default_conf()` MUST set
|
||||||
|
both the value AND `has_<field> = 1`. Setting `has_<field>` is what makes
|
||||||
|
the default participate in protobuf pack/unpack (REQ-CONFIG-CFG-002): a
|
||||||
|
named vhost that never overlays one of these fields inherits the **default
|
||||||
|
vhost's effective value** (its own default, or whatever the default vhost
|
||||||
|
section explicitly set), not a protobuf-zero/absent value. Permanent
|
||||||
|
(`static_cfg_st`) defaults (`stats_reset_time = 1 week`, `log_level`,
|
||||||
|
`syslog_facility = LOG_DAEMON`, `occtl_socket_file`) are applied only when
|
||||||
|
`!reload`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/config.c:766-824 (`apply_default_conf`), 826-845 (`cfg_new`)
|
||||||
|
**Acceptance:** unit, local — a minimal config with no `keepalive`/`dpd`/
|
||||||
|
`rekey-time`/etc. directives MUST produce `DEFAULT_KEEPALIVE_TIME` /
|
||||||
|
`DEFAULT_DPD_TIME` / `DEFAULT_REKEY_TIME` (etc., from `defaults.h` or
|
||||||
|
equivalent) in the default vhost's `ReloadableConfig`, with the
|
||||||
|
corresponding `has_*` flags set to 1; a named vhost in the same config MUST
|
||||||
|
inherit these exact values (REQ-CONFIG-CFG-002), not protobuf defaults
|
||||||
|
(0/NULL).
|
||||||
|
**Links:** REQ-CONFIG-CFG-002
|
||||||
|
|
||||||
|
### REQ-CONFIG-CFG-006 — `[scope: vhost user]` options are additionally parsed per-user/per-group via `src/sup-config/file.c`, transit IPC as `GroupCfgSt`, and are merged onto the vhost default by main in `apply_default_config()`
|
||||||
|
|
||||||
|
**Requirement:** Options annotated `[scope: vhost user]` in
|
||||||
|
`sample.config` MUST be both (a) settable at vhost level (subject to
|
||||||
|
REQ-CONFIG-CFG-002/003 inheritance) as a default, AND (b) individually
|
||||||
|
overridable per-user or per-group via the supplemental-config mechanism
|
||||||
|
(`sup-config = file`, `per-user-dir`/`per-group-dir`), parsed by
|
||||||
|
`src/sup-config/file.c`'s INI handler using the `READ_RAW_*`/`READ_TF`
|
||||||
|
macro family. These macros write directly into a `GroupCfgSt` (`config`
|
||||||
|
field of `SecmSessionReplyMsg`, `src/ipc.proto`) — a real protobuf message
|
||||||
|
— setting its generated `has_<field>=1` flags (e.g.
|
||||||
|
`msg->config->has_no_udp`, src/sup-config/file.c:112) for every field
|
||||||
|
present in the per-user/per-group file. This is the **same** `has_<field>`
|
||||||
|
mechanism as REQ-CONFIG-CFG-005, not a distinct `is_set` convention: a
|
||||||
|
`GroupCfgSt` with no per-user overrides is `GROUP_CFG_ST__INIT`-initialized
|
||||||
|
(all `has_*` false, all pointers NULL) by `handle_secm_session_open_cmd()`
|
||||||
|
(src/sec-mod-auth.c:504-505), and `get_sup_config()`
|
||||||
|
(src/sec-mod-auth.c:593-594) only sets the fields the per-user/group file
|
||||||
|
actually overrides. `iroute` directives are additionally collected across
|
||||||
|
all per-user config files into `config->known_iroutes` via
|
||||||
|
`load_iroutes()`/`append_iroutes_from_file()` when `expose_iroutes` is set,
|
||||||
|
for `occtl show iroutes`.
|
||||||
|
|
||||||
|
The populated `GroupCfgSt` travels from sec-mod to main as
|
||||||
|
`SecmSessionReplyMsg.config` over `CMD_SECM_SESSION_REPLY`
|
||||||
|
(src/main-sec-mod-cmd.c:537/recv at 537-538) — i.e. per-user overrides DO
|
||||||
|
cross the sec-mod->main IPC boundary as part of this message, same as the
|
||||||
|
rest of the session-open reply. The merge with the vhost-level default
|
||||||
|
happens entirely in **main**: `session_open()`
|
||||||
|
(src/main-sec-mod-cmd.c:494) sets `proc->config = msg->config` (line ~593)
|
||||||
|
and then calls `apply_default_config(sec_mod_instance, proc, proc->config)`
|
||||||
|
(src/main-sec-mod-cmd.c:336-491, called at line 597). For each
|
||||||
|
`[scope: vhost user]` field, `apply_default_config()` checks
|
||||||
|
`!gc->has_<field>` (scalars/bools) or `gc-><field> == NULL`
|
||||||
|
(strings/repeated fields); if unset — meaning no per-user/per-group override
|
||||||
|
was found — it copies the vhost-level value (`vhost->config->...`) into
|
||||||
|
`gc` and, for the `has_*` cases, sets `has_<field> = 1`. If the field was
|
||||||
|
already set by `get_sup_config()`, the per-user/group value is left
|
||||||
|
untouched. The resulting merged `GroupCfgSt` (`proc->config`) is what main
|
||||||
|
forwards to the worker as the session's effective configuration.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sup-config/file.c:44-80 (`READ_RAW_*`/`READ_TF` macros),
|
||||||
|
112 (`has_no_udp` example); src/ipc.proto:30-69 (`group_cfg_st`), 342-347
|
||||||
|
(`secm_session_reply_msg`); src/sec-mod-auth.c:474-505, 593-594
|
||||||
|
(`GROUP_CFG_ST__INIT`, `get_sup_config()`); src/main-sec-mod-cmd.c:336-491
|
||||||
|
(`apply_default_config`), 494-598 (`session_open`); src/config.c:686-764
|
||||||
|
(`iroutes_handler`, `append_iroutes_from_file`, `load_iroutes`);
|
||||||
|
tests/check-config-scope.py checks (d)/(e)/(f)
|
||||||
|
**Acceptance:** `tests/check-config-scope.py` checks (d)–(f) pass (every
|
||||||
|
`[...user...]` option is handled in `sup-config/file.c` and vice versa,
|
||||||
|
modulo `PER_USER_ONLY`; every `[...user...]` option is also
|
||||||
|
`[...vhost...]`). Functional: a per-user config file setting `rx-data-per-sec`
|
||||||
|
for user `alice` MUST override the vhost-level `rx-data-per-sec` for
|
||||||
|
`alice`'s session only — confirm by setting different vhost-default and
|
||||||
|
per-user values, and observing `apply_default_config()` leaves
|
||||||
|
`gc->has_rx_per_sec`/`gc->rx_per_sec` at the per-user value (does not
|
||||||
|
overwrite it from `vhost->config`), while a second user with no per-user
|
||||||
|
override receives the vhost default.
|
||||||
|
`[LIMITATION (gitlab#265/!154): there is no per-RADIUS-class sup-config
|
||||||
|
source — config-per-group/config-per-user and `radius_get_sup_config()`'s
|
||||||
|
attribute mapping (REQ-AUTH-AUTH-026) are the only contributors to
|
||||||
|
`GroupCfgSt`. gitlab!154 (closed, unmerged) sketched a `per-radius-class-dir`
|
||||||
|
source for this; see the related limitation note on REQ-AUTH-AUTH-029.]`
|
||||||
|
**Links:** REQ-CONFIG-CFG-001, REQ-CONFIG-CFG-005, REQ-AUTH-AUTH-026,
|
||||||
|
REQ-AUTH-AUTH-029
|
||||||
|
|
||||||
|
### REQ-CONFIG-CFG-007 — SIGHUP reload replaces each vhost's `ReloadableConfig` via archive-and-reparse (main) or immediate-free-and-reparse (sec-mod); in-flight references are kept alive via an attic + usage-count until drained
|
||||||
|
|
||||||
|
**Requirement:** `reload_cfg_file()` MUST, for each vhost: (a) if called for
|
||||||
|
**main** (`sec_mod == 0`), call `archive_cfg()`, which moves the vhost's
|
||||||
|
current `config` and `usage_count` into a new `attic_entry_st`
|
||||||
|
(`talloc_steal`'d so they survive the parent's lifetime), adds the entry to
|
||||||
|
`vhost->attic` **only if** `*usage_count != 0` (otherwise frees it
|
||||||
|
immediately), sets `vhost->config = NULL` and `vhost->usage_count = NULL`,
|
||||||
|
and resets `vhost->cfg_inherited = 0` so the next parse re-triggers
|
||||||
|
REQ-CONFIG-CFG-002's inheritance copy against the *new* default-vhost
|
||||||
|
config; (b) if called for **sec-mod** (`sec_mod == 1`), call `clear_cfg()`,
|
||||||
|
which immediately `talloc_free()`s the old `config` (sec-mod holds no
|
||||||
|
long-lived references that survive a reload, unlike main's in-flight
|
||||||
|
`proc_st` workers) and likewise resets `cfg_inherited = 0`; (c) for every
|
||||||
|
vhost with `config == NULL`, call `cfg_new()` (re-applying defaults, REQ-
|
||||||
|
CONFIG-CFG-005); (d) re-run `parse_cfg_file()` with `CFG_FLAG_RELOAD` (and
|
||||||
|
`CFG_FLAG_SECMOD` if `sec_mod`). Separately, `clear_old_configs()` MUST be
|
||||||
|
called periodically (or after reload) to walk each vhost's `attic` and
|
||||||
|
`talloc_free()` any entry whose `*usage_count` has dropped to 0, releasing
|
||||||
|
the old `ReloadableConfig` once no in-flight worker still references it.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/config.c:2384-2432 (`archive_cfg`, `clear_cfg`),
|
||||||
|
2501-2525 (`reload_cfg_file`), 2579-2595 (`clear_old_configs`);
|
||||||
|
src/vhost.h:41-43 (`usage_count`, `attic`); tests/config-inherit.c phase 3
|
||||||
|
**Acceptance:** tests/config-inherit.c phase 3 — after `reload_cfg_file()`
|
||||||
|
with an unchanged config, `check_inherited(named)` MUST still pass (inherited
|
||||||
|
fields survive reload because `cfg_inherited` was reset and the inheritance
|
||||||
|
copy re-ran against the freshly-reparsed default vhost). `[OPEN]`: this pass
|
||||||
|
did not add a test for the attic/usage_count drain path itself (a worker
|
||||||
|
holding a reference across a reload, then exiting, should cause its vhost's
|
||||||
|
attic entry to be freed by a subsequent `clear_old_configs()`).
|
||||||
|
**Links:** REQ-CONFIG-CFG-002, REQ-CONFIG-CFG-003, REQ-CONFIG-CFG-005,
|
||||||
|
REQ-MAIN-CFG-001 (cross-process ordering: main calls `reload_cfg_file()`
|
||||||
|
only after all sec-mod instances have completed their own reload)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## SEC
|
||||||
|
|
||||||
|
### REQ-CONFIG-SEC-001 — The worker process parses configuration from a root-created snapshot, never the live config file path
|
||||||
|
|
||||||
|
**Requirement:** When `parse_cfg_file()` is called with `CFG_FLAG_WORKER`
|
||||||
|
set (`PROC_FS_SUPPORTED` builds), it MUST NOT call `ini_parse()` on the
|
||||||
|
configured file path directly. Instead it MUST resolve the path via
|
||||||
|
`snapshot_lookup_filename(config_snapshot, file, ...)` (falling back to
|
||||||
|
`OLD_DEFAULT_CFG_FILE`'s snapshot), parse that snapshot file, and then
|
||||||
|
rewrite `dh_params_file`, `ocsp_response`, and every `cert[]` entry to their
|
||||||
|
snapshot equivalents via `replace_file_with_snapshot()`. The non-worker path
|
||||||
|
(main/sec-mod) parses the live file directly and additionally calls
|
||||||
|
`snapshot_create()` for the config file itself plus the same
|
||||||
|
cert/DH-params/OCSP-response files, so the snapshots the worker later reads
|
||||||
|
are root-controlled copies taken at main/sec-mod's privilege level, not
|
||||||
|
paths the unprivileged worker resolves itself.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/config.c:1592-1673 (`PROC_FS_SUPPORTED` branch of
|
||||||
|
`parse_cfg_file`)
|
||||||
|
**Divergence**: on non-`PROC_FS_SUPPORTED` (BSD) builds, this snapshot
|
||||||
|
indirection does not exist — `src/config.c:1674-1694` has the worker parse
|
||||||
|
the live file path directly, same as main/sec-mod. Per `AGENTS.md`'s BSD
|
||||||
|
best-effort policy, this is an accepted platform difference, not a defect,
|
||||||
|
but it does mean the SEC property above is Linux-only — `[REVIEW]`: confirm
|
||||||
|
whether worker-side TLS/cert paths on BSD builds are otherwise protected by
|
||||||
|
`chroot`/seccomp such that direct file access doesn't reintroduce a
|
||||||
|
privilege-boundary concern (`AGENTS.md`'s "no direct filesystem access
|
||||||
|
outside its seccomp profile" rule).
|
||||||
|
**Acceptance:** [SEC] — on Linux, confirm the worker process's open file
|
||||||
|
descriptors for cert/key/CRL/config files resolve under the snapshot
|
||||||
|
directory (e.g. `/proc/<pid>/fd`), not the configured `server-cert`/
|
||||||
|
`ocserv.conf` paths.
|
||||||
|
**Links:** —
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## ERR
|
||||||
|
|
||||||
|
### REQ-CONFIG-ERR-001 — `check_cfg()` enforces mandatory options and fails closed (`exit(EXIT_FAILURE)`) at parse/reload time, per-vhost
|
||||||
|
|
||||||
|
**Requirement:** After inheritance (REQ-CONFIG-CFG-002/003) and auth/acct
|
||||||
|
setup, `check_cfg(vhost, defvhost, silent)` MUST `exit(EXIT_FAILURE)` (after
|
||||||
|
printing an `error:` message, prefixed with the vhost name if named) if, for
|
||||||
|
that vhost: no authentication method was configured
|
||||||
|
(`static_config.auth[0].enabled == 0`); `socket_file_prefix` is unset
|
||||||
|
(default vhost only — "the 'socket-file' configuration option must be
|
||||||
|
specified"); `static_config.port == 0` ("the tcp-port option is mandatory");
|
||||||
|
`cert_size == 0 || key_size == 0`; or neither `ipv4-network` nor
|
||||||
|
`ipv6-network` is set, or either is set without its corresponding
|
||||||
|
netmask/prefix. These checks run for **every** vhost (default and named)
|
||||||
|
after inheritance has had a chance to fill in missing values from the
|
||||||
|
default vhost — i.e. a named vhost satisfies these checks via inheritance,
|
||||||
|
not by repeating every mandatory option.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/config.c:1917-1976 (`check_cfg`)
|
||||||
|
**Acceptance:** [CFG] negative — a `[vhost:example]` section that sets
|
||||||
|
`ipv4-network` without `ipv4-netmask`, with no default-vhost
|
||||||
|
`ipv4-netmask` to inherit, MUST cause `ocserv -c <file> -t` (or worker/main
|
||||||
|
startup) to print `no mask found for IPv4 network` (prefixed with
|
||||||
|
`example: `) and `exit(EXIT_FAILURE)` — process does not start with a
|
||||||
|
partially-valid config. Positive: the same config with the netmask present
|
||||||
|
(either set directly or inherited from the default vhost) MUST pass.
|
||||||
|
**Links:** REQ-CONFIG-CFG-002, REQ-CONFIG-CFG-003
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Completeness notes
|
||||||
|
|
||||||
|
- **`src/config-ports.c` / `src/config-kkdcp.c`**: port-range and KKDCP
|
||||||
|
(Kerberos KDC proxy, `HAVE_GSSAPI`) parsing helpers are included by
|
||||||
|
`config.c` but not separately analyzed in this pass —
|
||||||
|
`[UNDOCUMENTED: candidate REQ-CONFIG-CFG-* for port-list/range syntax and
|
||||||
|
KKDCP URL-forwarding config (parse_kkdcp, REQ-CONFIG-CFG-005's
|
||||||
|
`urlfw`/`urlfw_size` handling), if found to have non-obvious validation
|
||||||
|
rules.]`
|
||||||
|
- **`src/subconfig.c`** (`auth = pam[...]`/`radius[...]`/etc. bracketed
|
||||||
|
sub-option syntax, `expand_brackets_string`, `MAX_SUBOPTIONS`): the
|
||||||
|
bracket-expansion mechanism itself is generic INI-within-INI parsing with
|
||||||
|
no MUST/MUST NOT beyond "the configured module's `get_brackets_string`
|
||||||
|
receives the expanded suboptions" — per-module option semantics belong to
|
||||||
|
`internal/sec-mod.md` (auth/acct module config), not here.
|
||||||
|
`[UNDOCUMENTED: candidate REQ-CONFIG-CFG-* if a malformed `[...]` bracket
|
||||||
|
(unterminated, nested) is found to have a non-obvious failure mode rather
|
||||||
|
than a parse error.]`
|
||||||
|
- **`camouflage`/`camouflage_secret`/`camouflage_realm`**: these are
|
||||||
|
`[scope: vhost]` `ReloadableConfig` fields parsed by the ordinary
|
||||||
|
mechanisms covered by REQ-CONFIG-CFG-002/004; their *behavioral* contract
|
||||||
|
(URL-secret gate, 401/404/405 responses) is fully specified in
|
||||||
|
`protocol/unified.md` REQ-PROTO-COMPAT-006 and `internal/worker.md`
|
||||||
|
REQ-WORKER-AUTH-004 — not duplicated here.
|
||||||
|
- **`occtl reload`** (`CMD_SECM_RELOAD` / SIGHUP from `occtl`): triggers the
|
||||||
|
same `reload_cfg_file()` path as `SIGHUP`; the IPC framing is covered by
|
||||||
|
`internal/ipc.md`, the cross-process ordering by `REQ-MAIN-CFG-001`. Not
|
||||||
|
re-derived here.
|
||||||
|
- **Command-line flags** (`-c`, `-f`/`--foreground`, `-d`/`--debug`, `-t`/
|
||||||
|
`--test`, `--pid-file`, etc., `src/config.c` `getopt` handling): out of
|
||||||
|
scope for this pass — these select *which* file is parsed and how the
|
||||||
|
process behaves around parsing (foreground vs daemonize, test-and-exit),
|
||||||
|
not configuration *content* semantics.
|
||||||
@@ -0,0 +1,596 @@
|
|||||||
|
---
|
||||||
|
title: IPC requirements
|
||||||
|
generator: requirements-from-implementation
|
||||||
|
process: ipc
|
||||||
|
id-prefix: REQ-IPC
|
||||||
|
sources:
|
||||||
|
- src/ipc.proto
|
||||||
|
- src/ctl.proto
|
||||||
|
- src/defs.h
|
||||||
|
- src/sec-mod.c
|
||||||
|
- src/sec-mod-auth.c
|
||||||
|
- src/main-auth.c
|
||||||
|
- src/main-worker-cmd.c
|
||||||
|
- src/main-sec-mod-cmd.c
|
||||||
|
- src/worker-auth.c
|
||||||
|
- src/tlslib.c
|
||||||
|
- doc/design.md#ipc-communication
|
||||||
|
- doc/design.md#ipc-communication-for-sid-assignment
|
||||||
|
- doc/design.md#ipc-communication-for-session-termination
|
||||||
|
---
|
||||||
|
|
||||||
|
# IPC Requirements
|
||||||
|
|
||||||
|
This document covers the Unix-socket, protobuf-c (`src/ipc.proto`,
|
||||||
|
`src/ctl.proto`) messages exchanged between the three ocserv processes
|
||||||
|
(main, sec-mod, worker) and `occtl`. It is the spine cited by
|
||||||
|
`internal/main.md`, `internal/sec-mod.md`, and `internal/worker.md` for any
|
||||||
|
behavior that crosses a process boundary.
|
||||||
|
|
||||||
|
`internal/*` documents MUST cite these IDs instead of restating the IPC
|
||||||
|
contract.
|
||||||
|
|
||||||
|
## REQ-IPC-001 — General framing
|
||||||
|
|
||||||
|
**Requirement:** All cross-process communication MUST use the
|
||||||
|
`cmd_request_t` framing (`src/defs.h`) over Unix domain sockets, with the
|
||||||
|
payload (if any) serialized as the protobuf-c message named in the comment
|
||||||
|
preceding each `cmd_request_t` value in `src/ipc.proto`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/defs.h:60-110; src/ipc.proto (per-message comments)
|
||||||
|
**Acceptance:** unit — `tests/` IPC round-trip tests pack/unpack each
|
||||||
|
message type and confirm `cmd_request_to_str()` covers every enum value.
|
||||||
|
**Links:** —
|
||||||
|
|
||||||
|
### REQ-IPC-002 — Unknown command rejection
|
||||||
|
|
||||||
|
**Requirement:** A process receiving a `cmd_request_t` value it does not
|
||||||
|
recognize for its role MUST log the value and return an error without
|
||||||
|
acting on the payload.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod.c:468-470 (`default: ... return -1`)
|
||||||
|
**Acceptance:** negative, local — send an out-of-range `cmd_request_t` to
|
||||||
|
sec-mod's socket and confirm it logs `unknown type` and closes/errors
|
||||||
|
without crashing.
|
||||||
|
**Links:** REQ-IPC-001
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## SEC_AUTH_INIT / SEC_AUTH_REP / SEC_AUTH_CONT (worker <-> sec-mod)
|
||||||
|
|
||||||
|
```
|
||||||
|
worker -> sec-mod: SEC_AUTH_INIT (sec_auth_init_msg)
|
||||||
|
sec-mod -> worker: SEC_AUTH_REP (sec_auth_reply_msg, new SID)
|
||||||
|
worker -> sec-mod: SEC_AUTH_CONT (sec_auth_cont_msg, SID + password)
|
||||||
|
sec-mod -> worker: SEC_AUTH_REP (OK or FAILED)
|
||||||
|
```
|
||||||
|
|
||||||
|
### REQ-IPC-010 — SEC_AUTH_INIT HMAC anti-replay
|
||||||
|
|
||||||
|
**Requirement:** sec-mod MUST reject a `sec_auth_init_msg` whose `hmac`
|
||||||
|
field is not exactly `HMAC_DIGEST_SIZE` bytes, or whose value does not equal
|
||||||
|
`HMAC(orig_remote_ip || our_ip || session_start_time)` computed with
|
||||||
|
`sec->hmac_key`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-auth.c:886-914
|
||||||
|
**Acceptance:** negative, local — send `SEC_AUTH_INIT` with a wrong-length
|
||||||
|
or mismatched `hmac` field and confirm sec-mod logs `hmac is the wrong
|
||||||
|
size` / `hmac presented by client doesn't match parameters provided` and
|
||||||
|
does not create a `client_entry_st`.
|
||||||
|
**Links:** REQ-IPC-011, REQ-WORKER-AUTH-* (worker side: HMAC is computed by
|
||||||
|
main at `WORKER_STARTUP` time, see REQ-IPC-040)
|
||||||
|
|
||||||
|
### REQ-IPC-011 — SEC_AUTH_INIT replay window
|
||||||
|
|
||||||
|
**Requirement:** sec-mod MUST reject a `sec_auth_init_msg` whose
|
||||||
|
`session_start_time` is older than `vhost->config->auth_timeout` seconds
|
||||||
|
relative to sec-mod's current time, even if the HMAC is valid.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-auth.c:916-922
|
||||||
|
**Acceptance:** negative, local — replay a captured, HMAC-valid
|
||||||
|
`SEC_AUTH_INIT` after `auth-timeout` has elapsed; confirm sec-mod logs `hmac
|
||||||
|
presented by client expired - possible replay` and rejects.
|
||||||
|
**Links:** REQ-IPC-010
|
||||||
|
|
||||||
|
### REQ-IPC-012 — orig_remote_ip is required
|
||||||
|
|
||||||
|
**Requirement:** sec-mod MUST reject a `sec_auth_init_msg` with no
|
||||||
|
`orig_remote_ip` field, before computing the HMAC.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-auth.c:895-898
|
||||||
|
**Acceptance:** negative, local — confirm sec-mod logs `missing remote IP in
|
||||||
|
auth init` and rejects a message that omits `orig_remote_ip`.
|
||||||
|
**Links:** REQ-IPC-010
|
||||||
|
|
||||||
|
### REQ-IPC-013 — auth_type determines auth module selection
|
||||||
|
|
||||||
|
**Requirement:** sec-mod MUST select the first enabled auth module on the
|
||||||
|
target vhost whose configured `type` bitmask contains all bits set in
|
||||||
|
`sec_auth_init_msg.auth_type`, and MUST reject the request (without creating
|
||||||
|
a session) if no such module exists.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-auth.c:841-871 (`set_module`), 930-935
|
||||||
|
**Acceptance:** negative, local — send `SEC_AUTH_INIT` with `auth_type=0` or
|
||||||
|
a type not configured on the vhost; confirm sec-mod logs `no module found
|
||||||
|
for auth type` and returns failure.
|
||||||
|
**Links:** REQ-IPC-010
|
||||||
|
|
||||||
|
### REQ-IPC-014 — confidential username types are not echoed from SEC_AUTH_INIT
|
||||||
|
|
||||||
|
**Requirement:** When `sec_auth_init_msg.auth_type` has any bit set in
|
||||||
|
`CONFIDENTIAL_USER_NAME_AUTH_TYPES`, sec-mod MUST NOT copy
|
||||||
|
`sec_auth_init_msg.user_name` into the session's accounting username — the
|
||||||
|
real username is established only after authentication completes.
|
||||||
|
**Strength:** MUST NOT
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-auth.c:971-977
|
||||||
|
**Acceptance:** unit, local — for a cert-based auth type, send `SEC_AUTH_INIT`
|
||||||
|
with a `user_name` set and confirm `e->acct_info.username` remains empty (or
|
||||||
|
is set only after `auth_init`/`auth_pass` completes) rather than the
|
||||||
|
submitted value.
|
||||||
|
**Links:** REQ-SECMOD-AUTH-*
|
||||||
|
|
||||||
|
### REQ-IPC-015 — SEC_AUTH_CONT requires valid SID and matching state
|
||||||
|
|
||||||
|
**Requirement:** sec-mod MUST reject a `sec_auth_cont_msg` if `sid.len !=
|
||||||
|
SID_SIZE`, if no `client_entry_st` exists for that SID, or if the entry's
|
||||||
|
state is not `PS_AUTH_INIT` or `PS_AUTH_CONT`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-auth.c:784-805
|
||||||
|
**Acceptance:** negative, local — (a) send `SEC_AUTH_CONT` with a
|
||||||
|
wrong-length SID; (b) send it with a random 8-byte SID that was never issued
|
||||||
|
by `SEC_AUTH_REP`; (c) send a second `SEC_AUTH_CONT` after the session
|
||||||
|
reached `PS_AUTH_COMPLETED`. Each MUST be rejected.
|
||||||
|
**Links:** REQ-IPC-010
|
||||||
|
|
||||||
|
### REQ-IPC-016 — SEC_AUTH_CONT requires a password
|
||||||
|
|
||||||
|
**Requirement:** sec-mod MUST reject a `sec_auth_cont_msg` whose `password`
|
||||||
|
field is absent.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-auth.c:810-816
|
||||||
|
**Acceptance:** negative, local — send `SEC_AUTH_CONT` without `password`
|
||||||
|
set; confirm rejection with `no password given in auth cont`.
|
||||||
|
**Links:** REQ-IPC-015
|
||||||
|
|
||||||
|
### REQ-IPC-017 — SID is assigned only by sec-mod
|
||||||
|
|
||||||
|
**Requirement:** A worker MUST NOT construct its own SID; the SID used in
|
||||||
|
`SEC_AUTH_CONT`, `AUTH_COOKIE_REQ`, `SECM_SESSION_OPEN/CLOSE`, and
|
||||||
|
`SEC_CLI_STATS` MUST be the `sid` value sec-mod returned in `SEC_AUTH_REP`
|
||||||
|
for that authentication attempt.
|
||||||
|
**Strength:** MUST NOT
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** doc/design.md#ipc-communication-for-sid-assignment;
|
||||||
|
src/sec-mod-auth.c:174-217 (sid assigned in `handle_sec_auth_res` /
|
||||||
|
`SEC_AUTH_REP` reply path)
|
||||||
|
**Acceptance:** [REVIEW: no automated negative test found — add one that
|
||||||
|
sends `AUTH_COOKIE_REQ`/`SEC_AUTH_CONT` with a SID the worker fabricated
|
||||||
|
rather than one received via `SEC_AUTH_REP`, and confirm
|
||||||
|
`find_client_entry()` rejects it (covered structurally by REQ-IPC-015 /
|
||||||
|
REQ-IPC-021, but no test isolates "worker-fabricated SID" specifically).]
|
||||||
|
**Links:** REQ-IPC-015, REQ-IPC-021
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## AUTH_COOKIE_REQ / AUTH_COOKIE_REP (worker <-> main, main <-> sec-mod)
|
||||||
|
|
||||||
|
```
|
||||||
|
worker -> main: AUTH_COOKIE_REQ (auth_cookie_request_msg: cookie)
|
||||||
|
main -> sec-mod: SECM_SESSION_OPEN (secm_session_open_msg: sid)
|
||||||
|
sec-mod -> main: SECM_SESSION_REPLY (secm_session_reply_msg: config, OK/FAILED)
|
||||||
|
main -> worker: AUTH_COOKIE_REP (auth_cookie_reply_msg: OK + tun device, or FAILED)
|
||||||
|
```
|
||||||
|
|
||||||
|
### REQ-IPC-020 — AUTH_COOKIE_REQ only from PS_AUTH_INACTIVE
|
||||||
|
|
||||||
|
**Requirement:** main MUST reject an `AUTH_COOKIE_REQ` received from a
|
||||||
|
worker whose `proc->status != PS_AUTH_INACTIVE`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main-worker-cmd.c:421-427
|
||||||
|
**Acceptance:** negative, local — after a worker has already completed
|
||||||
|
`AUTH_COOKIE_REQ` once (status advanced past `PS_AUTH_INACTIVE`), send a
|
||||||
|
second `AUTH_COOKIE_REQ` on the same connection and confirm main returns
|
||||||
|
`ERR_BAD_COMMAND`.
|
||||||
|
**Links:** REQ-IPC-021
|
||||||
|
|
||||||
|
### REQ-IPC-021 — SECM_SESSION_OPEN requires SID of a completed, unexpired session
|
||||||
|
|
||||||
|
**Requirement:** sec-mod MUST reply `FAILED` to `SECM_SESSION_OPEN` if
|
||||||
|
`sid.len != SID_SIZE`, if no `client_entry_st` exists for that SID, if the
|
||||||
|
entry's status is not `PS_AUTH_COMPLETED`, or if the entry is expired
|
||||||
|
(`IS_CLIENT_ENTRY_EXPIRED`).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-auth.c:508-536
|
||||||
|
**Acceptance:** negative, local — (a) send `SECM_SESSION_OPEN` with a
|
||||||
|
wrong-length or unknown SID; (b) send it for a SID still in
|
||||||
|
`PS_AUTH_INIT`/`PS_AUTH_CONT`; (c) send it after the cookie's expiry time
|
||||||
|
has passed. Each MUST yield `secm_session_reply_msg.reply = FAILED` (via
|
||||||
|
`send_failed_session_open_reply`).
|
||||||
|
**Links:** REQ-IPC-015, REQ-IPC-022
|
||||||
|
|
||||||
|
### REQ-IPC-022 — SECM_SESSION_OPEN refreshes cookie expiry
|
||||||
|
|
||||||
|
**Requirement:** On a successful `SECM_SESSION_OPEN`, sec-mod MUST update
|
||||||
|
the client entry's `exptime` to `now + cookie_timeout + AUTH_SLACK_TIME` and
|
||||||
|
increment `e->in_use`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-auth.c:617-620
|
||||||
|
**Acceptance:** unit, local — open a session via `SECM_SESSION_OPEN`,
|
||||||
|
inspect `e->exptime` before/after, confirm it advances by
|
||||||
|
`cookie_timeout + AUTH_SLACK_TIME`. Cross-reference `doc/sample.config`
|
||||||
|
`cookie-timeout` documentation.
|
||||||
|
**Links:** REQ-IPC-021
|
||||||
|
|
||||||
|
### REQ-IPC-023 — AUTH_COOKIE_REP carries the TUN device only on OK
|
||||||
|
|
||||||
|
**Requirement:** main MUST send `auth_cookie_reply_msg.reply = OK` together
|
||||||
|
with the allocated TUN device fd (via `send_socket_msg_to_worker`) only when
|
||||||
|
`proc->tun_lease.name` is set; otherwise it MUST send `reply = FAILED` with
|
||||||
|
no fd, via `send_msg_to_worker` (no socket transfer).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main-auth.c:48-115
|
||||||
|
**Acceptance:** negative, local — force `SECM_SESSION_OPEN` to fail (e.g.
|
||||||
|
expired cookie) and confirm the worker receives `AUTH_COOKIE_REP` with
|
||||||
|
`reply = FAILED` and no fd is transferred (worker does not gain a TUN
|
||||||
|
device).
|
||||||
|
**Links:** REQ-IPC-021, REQ-MAIN-NET-* (TUN lease allocation)
|
||||||
|
|
||||||
|
### REQ-IPC-024 — Worker MUST NOT have a TUN device before AUTH_COOKIE_REP OK
|
||||||
|
|
||||||
|
**Requirement:** A worker process MUST NOT possess an open TUN device file
|
||||||
|
descriptor before it has received `AUTH_COOKIE_REP` with `reply = OK`.
|
||||||
|
**Strength:** MUST NOT
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main-auth.c:53,103-106; doc/design.md#the-worker-processes
|
||||||
|
(TUN device "forwarded from main to worker after successful
|
||||||
|
authentication")
|
||||||
|
**Acceptance:** [REVIEW: this is a privilege-boundary invariant
|
||||||
|
(`AGENTS.md` "Architecture" table) but no test directly observes the
|
||||||
|
worker's fd table pre-/post-`AUTH_COOKIE_REP`. Add a negative test, e.g. via
|
||||||
|
seccomp audit or `/proc/<worker-pid>/fd` inspection in a CI harness, that
|
||||||
|
confirms the TUN fd does not exist before `AUTH_COOKIE_REP` OK.]
|
||||||
|
**Links:** REQ-IPC-023, REQ-SEC-* (privilege boundary)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## SECM_SESSION_CLOSE / SECM_CLI_STATS, SEC_CLI_STATS (session teardown)
|
||||||
|
|
||||||
|
```
|
||||||
|
worker -> sec-mod: SEC_CLI_STATS (cli_stats_msg, SID)
|
||||||
|
main -> sec-mod: SECM_SESSION_CLOSE (secm_session_close_msg, SID)
|
||||||
|
sec-mod -> main: SECM_CLI_STATS (cli_stats_msg)
|
||||||
|
```
|
||||||
|
|
||||||
|
### REQ-IPC-030 — SECM_SESSION_CLOSE requires a valid SID
|
||||||
|
|
||||||
|
**Requirement:** sec-mod MUST reject `SECM_SESSION_CLOSE` with
|
||||||
|
`ERR_BAD_COMMAND` if `sid.len != SID_SIZE`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-auth.c:632-637
|
||||||
|
**Acceptance:** negative, local — send `SECM_SESSION_CLOSE` with a
|
||||||
|
malformed `sid`; confirm `ERR_BAD_COMMAND`.
|
||||||
|
**Links:** REQ-IPC-015
|
||||||
|
|
||||||
|
### REQ-IPC-031 — SECM_SESSION_CLOSE for unknown or unauthenticated SID still replies CLI_STATS
|
||||||
|
|
||||||
|
**Requirement:** If `SECM_SESSION_CLOSE` references a SID with no
|
||||||
|
`client_entry_st`, or one whose status is `< PS_AUTH_COMPLETED`, sec-mod
|
||||||
|
MUST still reply with `CMD_SECM_CLI_STATS` (a zeroed `cli_stats_msg`) rather
|
||||||
|
than an error, so main's session-close path does not block.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-auth.c:639-656
|
||||||
|
**Acceptance:** negative, local — send `SECM_SESSION_CLOSE` for a SID that
|
||||||
|
was never opened (or only reached `PS_AUTH_INIT`); confirm sec-mod replies
|
||||||
|
with `CMD_SECM_CLI_STATS` and main does not hang waiting for a reply.
|
||||||
|
**Links:** REQ-IPC-030
|
||||||
|
|
||||||
|
### REQ-IPC-032 — Stats counters are monotonic, taking the maximum
|
||||||
|
|
||||||
|
**Requirement:** On `SECM_SESSION_CLOSE`, sec-mod MUST update
|
||||||
|
`e->stats.{uptime,bytes_in,bytes_out}` only if the incoming value is greater
|
||||||
|
than the stored value (`>` comparison), never overwriting with a smaller
|
||||||
|
number.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-auth.c:658-666
|
||||||
|
**Acceptance:** unit, local — send `SECM_SESSION_CLOSE` twice for the same
|
||||||
|
SID, second time with smaller `bytes_in`/`bytes_out`/`uptime`; confirm
|
||||||
|
`e->stats` values are unchanged after the second message.
|
||||||
|
**Links:** REQ-IPC-031
|
||||||
|
|
||||||
|
### REQ-IPC-033 — server_disconnected sets discon_reason
|
||||||
|
|
||||||
|
**Requirement:** When `secm_session_close_msg.server_disconnected` is true,
|
||||||
|
sec-mod MUST set `e->discon_reason = REASON_SERVER_DISCONNECT` and report it
|
||||||
|
in the `cli_stats_msg.discon_reason` reply field.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-auth.c:668-676
|
||||||
|
**Acceptance:** unit, local — send `SECM_SESSION_CLOSE` with
|
||||||
|
`server_disconnected = true`; confirm `CMD_SECM_CLI_STATS` reply has
|
||||||
|
`discon_reason = REASON_SERVER_DISCONNECT`. Cross-check `occtl show events`
|
||||||
|
output, which surfaces `discon_reason`.
|
||||||
|
**Links:** REQ-IPC-031
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## WORKER_STARTUP (main -> worker)
|
||||||
|
|
||||||
|
### REQ-IPC-040 — sec_auth_init_hmac is computed by main, not the worker
|
||||||
|
|
||||||
|
**Requirement:** The `sec_auth_init_hmac` field used by the worker in
|
||||||
|
`SEC_AUTH_INIT` MUST be computed by main (using `sec->hmac_key`, which the
|
||||||
|
worker does not have) and forwarded to the worker via
|
||||||
|
`worker_startup_msg.sec_auth_init_hmac`; the worker MUST treat this value as
|
||||||
|
opaque and copy it verbatim into `sec_auth_init_msg.hmac`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main.c:1857-1858 (main computes and sends);
|
||||||
|
src/worker.c:289-307 (worker copies into `ws->sec_auth_init_hmac`,
|
||||||
|
bounds-checked against `sizeof(ws->sec_auth_init_hmac)`);
|
||||||
|
src/worker-auth.c:1762-1763, src/worker-svc.c:127-128 (worker forwards
|
||||||
|
verbatim in `SEC_AUTH_INIT`)
|
||||||
|
**Acceptance:** unit, local — confirm `worker_startup_msg.sec_auth_init_hmac`
|
||||||
|
has length `HMAC_DIGEST_SIZE`; confirm worker rejects/truncates a
|
||||||
|
`WORKER_STARTUP` message whose `sec_auth_init_hmac` exceeds
|
||||||
|
`sizeof(ws->sec_auth_init_hmac)` (src/worker.c:289-290).
|
||||||
|
**Links:** REQ-IPC-010, REQ-SEC-001 (worker has no access to `sec->hmac_key`)
|
||||||
|
|
||||||
|
### REQ-IPC-041 — WORKER_STARTUP transfers pre-opened fds, not raw keys
|
||||||
|
|
||||||
|
**Requirement:** `worker_startup_msg` MUST transfer only file descriptors
|
||||||
|
(`cmd_fd`, `conn_fd`, `snapshot_entries[].file_descriptor`) and
|
||||||
|
non-cryptographic identifiers (`secmod_addr`, IPs, group lists); it MUST NOT
|
||||||
|
contain private key material or HMAC secrets.
|
||||||
|
**Strength:** MUST NOT
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/ipc.proto (`worker_startup_msg` field list)
|
||||||
|
**Acceptance:** [REVIEW: this is a privilege-boundary assertion derivable
|
||||||
|
from the schema (no `bytes` field plausibly sized/named for key material),
|
||||||
|
but there is no automated check that a future field addition to
|
||||||
|
`worker_startup_msg` doesn't violate it. Consider a schema-diff check or
|
||||||
|
explicit comment in `ipc.proto` flagging this constraint for reviewers of
|
||||||
|
future `.proto` edits.]
|
||||||
|
**Links:** REQ-SEC-001
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## SEC_SIGN / SEC_SIGN_DATA / SEC_SIGN_HASH / SEC_DECRYPT / SEC_GET_PK (worker <-> sec-mod, via main relay)
|
||||||
|
|
||||||
|
### REQ-IPC-050 — Private key operations are delegated, never performed by the worker
|
||||||
|
|
||||||
|
**Requirement:** All TLS private-key operations (signing, hash-signing,
|
||||||
|
decryption, and public-key retrieval) for a vhost's certificate key MUST be
|
||||||
|
performed by sec-mod in response to `CMD_SEC_SIGN`, `CMD_SEC_SIGN_DATA`,
|
||||||
|
`CMD_SEC_SIGN_HASH`, `CMD_SEC_DECRYPT`, and `CMD_SEC_GET_PK`; the worker's
|
||||||
|
GnuTLS `gnutls_privkey_t` callbacks (`key_cb_sign_data_func`,
|
||||||
|
`key_cb_sign_hash_func`, `key_cb_decrypt_func`) MUST only marshal requests
|
||||||
|
to sec-mod and return its response — they MUST NOT hold or derive key
|
||||||
|
material locally.
|
||||||
|
**Strength:** MUST NOT
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/tlslib.c:793-832 (`key_cb_*_func` -> `key_cb_common_func` ->
|
||||||
|
`CMD_SEC_*`); src/sec-mod.c:219-338 (sec-mod performs
|
||||||
|
`gnutls_privkey_sign_data`/`sign_hash`/`decrypt_data2` using
|
||||||
|
`vhost->key[i]`)
|
||||||
|
**Acceptance:** [SEC] negative, local — this is the core
|
||||||
|
worker/private-key privilege boundary from `AGENTS.md`. Verify (a) the
|
||||||
|
worker process never opens the vhost private key file (seccomp file-open
|
||||||
|
denial, or `strace`/audit showing no `open()` of the key path from the
|
||||||
|
worker PID), and (b) `sec_op_msg` / `sec_get_pk_msg` round trips correctly
|
||||||
|
for each of `CMD_SEC_SIGN`, `CMD_SEC_SIGN_DATA`, `CMD_SEC_SIGN_HASH`,
|
||||||
|
`CMD_SEC_DECRYPT`, `CMD_SEC_GET_PK`.
|
||||||
|
**Links:** REQ-SEC-001 (architecture invariant in AGENTS.md)
|
||||||
|
|
||||||
|
### REQ-IPC-051 — sec_op_msg.key_idx and vhost select the key
|
||||||
|
|
||||||
|
**Requirement:** sec-mod MUST select the private key for a `sec_op_msg` /
|
||||||
|
`sec_get_pk_msg` using the requesting worker's `vhost` field and
|
||||||
|
`key_idx`/`pk` fields, and MUST NOT use a key belonging to a different vhost
|
||||||
|
than the one the requesting worker was started for.
|
||||||
|
**Strength:** MUST NOT
|
||||||
|
**Status:** REVIEW
|
||||||
|
**Source:** src/sec-mod.c:219-338 (`vhost->key[i]` indexed by `key_idx`);
|
||||||
|
src/sec-mod.c:1160-1201 (accept loop — `check_upeer_id()` validates only the
|
||||||
|
peer process's uid/gid/pid, recording no vhost binding for `cfd`);
|
||||||
|
src/vhost.h:137-152 (`find_vhost()` — case-insensitive string match over
|
||||||
|
*all* configured vhosts on the message-supplied `name`, falling back to
|
||||||
|
`default_vhost()`, never failing)
|
||||||
|
**Acceptance:** [REVIEW: confirmed interpretation (b) — sec-mod's
|
||||||
|
connection-accept path (`accept()` + `check_upeer_id()` in
|
||||||
|
`src/sec-mod.c`'s main loop) establishes no per-connection vhost binding; it
|
||||||
|
checks only that the peer is a legitimate ocserv worker process (uid/gid/pid),
|
||||||
|
not which vhost it was started for. `process_worker_packet()` then resolves
|
||||||
|
the vhost for `CMD_SEC_GET_PK`/`CMD_SEC_SIGN_DATA`/`CMD_SEC_SIGN_HASH`/
|
||||||
|
`CMD_SEC_SIGN`/`CMD_SEC_DECRYPT` purely via `find_vhost(sec->vconfig,
|
||||||
|
op->vhost)` — a string supplied in the message itself — and then indexes
|
||||||
|
`vhost->key[key_idx]` (bounds-checked against that *resolved* vhost's
|
||||||
|
`key_size`, but not against the requesting worker's own vhost). A worker
|
||||||
|
that supplies a different, validly-configured vhost's name in
|
||||||
|
`op->vhost`/`pkm->vhost` causes sec-mod to perform the signing/decryption
|
||||||
|
operation with that other vhost's private key and return the result to the
|
||||||
|
requester. This appears to violate the stated MUST NOT and needs maintainer
|
||||||
|
review: either (i) sec-mod must record the vhost a worker was started for
|
||||||
|
(e.g. at `WORKER_STARTUP`/first request) and reject `sec_op_msg`/
|
||||||
|
`sec_get_pk_msg` whose `vhost` field does not match it, or (ii) the
|
||||||
|
requirement's premise that vhosts are mutually-distrusting boundaries needs
|
||||||
|
revising if cross-vhost key use is intentional in this deployment model.]
|
||||||
|
**Links:** REQ-IPC-050
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## RESUME_STORE_REQ / RESUME_FETCH_REQ / RESUME_DELETE_REQ / RESUME_FETCH_REP (worker <-> sec-mod)
|
||||||
|
|
||||||
|
### REQ-IPC-060 — Resumption data is zeroized after use
|
||||||
|
|
||||||
|
**Requirement:** sec-mod MUST zero the raw IPC buffer and the unpacked
|
||||||
|
`session_resume_store_req_msg.session_data` with `safe_memset()` after
|
||||||
|
processing `RESUME_STORE_REQ`, regardless of success or failure.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod.c:393-397
|
||||||
|
**Acceptance:** unit, local — after `handle_resume_store_req()` returns,
|
||||||
|
inspect the buffer passed to `recv_msg` and `smsg->session_data.data`;
|
||||||
|
confirm both are zero-filled. This protects TLS session resumption secrets
|
||||||
|
from being left in process memory longer than necessary.
|
||||||
|
**Links:** —
|
||||||
|
|
||||||
|
### REQ-IPC-061 — RESUME_FETCH_REQ always replies, OK or FAILED
|
||||||
|
|
||||||
|
**Requirement:** sec-mod MUST always send a `RESUME_FETCH_REP`
|
||||||
|
(`session_resume_reply_msg`) in response to `RESUME_FETCH_REQ`, with
|
||||||
|
`reply = OK` and `session_data` populated on success, or `reply = FAILED`
|
||||||
|
(no `session_data`) if `handle_resume_fetch_req()` returns an error.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod.c:428-463
|
||||||
|
**Acceptance:** negative, local — request resumption for a `session_id`
|
||||||
|
that was never stored (or was deleted via `RESUME_DELETE_REQ`); confirm the
|
||||||
|
worker receives `RESUME_FETCH_REP` with `reply = FAILED` rather than no
|
||||||
|
reply or a connection error.
|
||||||
|
**Links:** —
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## SECM_TERMINATE_USER_SESSIONS / SECM_TERMINATE_SESSION (occtl -> main -> sec-mod)
|
||||||
|
|
||||||
|
```
|
||||||
|
occtl -> main: CTL_CMD_LIST_COOKIES (resolve short SID -> safe_id)
|
||||||
|
main -> occtl: cookie list
|
||||||
|
occtl -> main: CTL_CMD_TERMINATE_USER / ID / SESSION
|
||||||
|
main: disconnect worker process(es)
|
||||||
|
main -> sec-mod: SECM_TERMINATE_USER_SESSIONS (username) | SECM_TERMINATE_SESSION (safe_id)
|
||||||
|
sec-mod -> main: SECM_TERMINATE_SESSION_REPLY
|
||||||
|
main -> occtl: CTL_CMD_TERMINATE_*_REP
|
||||||
|
```
|
||||||
|
|
||||||
|
### REQ-IPC-070 — Session ID prefix ambiguity is rejected, not guessed
|
||||||
|
|
||||||
|
**Requirement:** When `occtl terminate session` is given a session ID
|
||||||
|
prefix that matches more than one entry in the cookie list returned by
|
||||||
|
`CTL_CMD_LIST_COOKIES`, main/occtl MUST refuse the operation and list the
|
||||||
|
matching sessions rather than acting on the first or any arbitrary match.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** doc/design.md#ipc-communication-for-session-termination
|
||||||
|
("If the prefix is ambiguous (matches multiple sessions), occtl refuses the
|
||||||
|
operation and lists the matching sessions.")
|
||||||
|
**Acceptance:** negative, local — create two sessions whose `safe_id`
|
||||||
|
values share a common prefix; run `occtl terminate session <prefix>` and
|
||||||
|
confirm it refuses with a list of matches, and that neither session is
|
||||||
|
terminated. Scripts MUST instead use the full session ID from `occtl --json
|
||||||
|
show sessions valid` (`Full session` field).
|
||||||
|
**Links:** REQ-IPC-071
|
||||||
|
|
||||||
|
### REQ-IPC-071 — SECM_TERMINATE_* always elicits SECM_TERMINATE_SESSION_REPLY
|
||||||
|
|
||||||
|
**Requirement:** sec-mod MUST reply to both `SECM_TERMINATE_USER_SESSIONS`
|
||||||
|
and `SECM_TERMINATE_SESSION` with `secm_terminate_session_reply_msg`, with
|
||||||
|
`result = true` only if at least one matching session's cookie was
|
||||||
|
invalidated.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/ipc.proto (`secm_terminate_session_reply_msg`); src/sec-mod.c
|
||||||
|
(CMD_SECM_TERMINATE_* handlers near line 549+)
|
||||||
|
**Acceptance:** negative, local — issue `SECM_TERMINATE_USER_SESSIONS` for a
|
||||||
|
username with no active sessions; confirm `result = false` is returned (not
|
||||||
|
an error/timeout), and confirm main relays this via `CTL_CMD_TERMINATE_*_REP`
|
||||||
|
to occtl.
|
||||||
|
**Links:** REQ-IPC-070
|
||||||
|
|
||||||
|
### REQ-IPC-072 — Termination invalidates the cookie, not just the live connection
|
||||||
|
|
||||||
|
**Requirement:** A successful `SECM_TERMINATE_USER_SESSIONS` /
|
||||||
|
`SECM_TERMINATE_SESSION` MUST invalidate the session's cookie in sec-mod
|
||||||
|
(so a subsequent `AUTH_COOKIE_REQ` / `SECM_SESSION_OPEN` with that SID fails
|
||||||
|
per REQ-IPC-021), in addition to main disconnecting any live worker for that
|
||||||
|
session.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** doc/design.md#ipc-communication-for-session-termination ("main
|
||||||
|
process disconnects the active worker (if any) and forwards the request to
|
||||||
|
sec-mod to invalidate the session cookie, preventing automatic
|
||||||
|
reconnection")
|
||||||
|
**Acceptance:** negative, local — terminate a session via `occtl`, then
|
||||||
|
attempt to reconnect a client using the previously-issued cookie; confirm
|
||||||
|
`AUTH_COOKIE_REQ` is rejected (`AUTH_COOKIE_REP` `reply = FAILED`) rather
|
||||||
|
than resuming the terminated session.
|
||||||
|
**Links:** REQ-IPC-021, REQ-IPC-071
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## WORKER_BAN_IP / SECM_BAN_IP (worker -> main, sec-mod -> main)
|
||||||
|
|
||||||
|
### REQ-IPC-080 — ban_ip_reply_msg.sid is only meaningful from sec-mod
|
||||||
|
|
||||||
|
**Requirement:** The `sid` field of `ban_ip_reply_msg` (used by main to tell
|
||||||
|
sec-mod whether to disconnect a user, per the `ban_ip_msg` comment "sec-mod
|
||||||
|
sends it") MUST be populated only on the sec-mod -> main direction; main
|
||||||
|
MUST NOT depend on a worker-supplied `ban_ip_msg.sid` to disconnect a
|
||||||
|
different session than the one reporting the ban.
|
||||||
|
**Strength:** MUST NOT
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/ipc.proto (`ban_ip_msg.sid` comment "sec-mod sends it";
|
||||||
|
`ban_ip_reply_msg.sid` comment "sec-mod needs it");
|
||||||
|
src/worker-vpn.c:499-509 (`CMD_BAN_IP` send, `has_sid` never set);
|
||||||
|
src/main-worker-cmd.c:282-322 (`CMD_BAN_IP` handler, never reads `tmsg->sid`,
|
||||||
|
replies with `BAN_IP_REPLY_MSG__INIT` so `has_sid == 0`);
|
||||||
|
src/sec-mod-auth.c:103-105 (`CMD_SECM_BAN_IP` send, `sid = e->sid`,
|
||||||
|
`has_sid = 1`); src/main-sec-mod-cmd.c:143-181 (`CMD_SECM_BAN_IP` handler
|
||||||
|
echoes `tmsg->sid` into the reply); src/sec-mod.c:506-517
|
||||||
|
(`CMD_SECM_BAN_IP_REPLY` dispatches to `handle_sec_auth_ban_ip_reply`)
|
||||||
|
**Acceptance:** positive, local — confirm `BanIpMsg msg = BAN_IP_MSG__INIT`
|
||||||
|
in `src/worker-vpn.c` is never followed by an assignment to `msg.sid` or
|
||||||
|
`msg.has_sid`, so every `CMD_BAN_IP` message from a worker has `has_sid ==
|
||||||
|
0`. Confirm the `CMD_BAN_IP` case in `src/main-worker-cmd.c` has no
|
||||||
|
reference to `tmsg->sid` and constructs its reply from
|
||||||
|
`BAN_IP_REPLY_MSG__INIT` without setting `reply.sid`/`reply.has_sid`.
|
||||||
|
Negative — confirm `handle_sec_auth_ban_ip_reply()` (the only consumer of
|
||||||
|
`ban_ip_reply_msg.sid` for session lookup, via `find_client_entry`) is
|
||||||
|
reachable only from the `CMD_SECM_BAN_IP_REPLY` case in `src/sec-mod.c`,
|
||||||
|
never from `CMD_BAN_IP_REPLY` (the worker-facing reply, handled in
|
||||||
|
`src/worker-vpn.c` without inspecting `reply->sid`). Together these confirm
|
||||||
|
that even a compromised worker sending a crafted `ban_ip_msg` with
|
||||||
|
`has_sid=1` and an arbitrary `sid` would have that field silently ignored by
|
||||||
|
main — `sid` is meaningful only on the sec-mod <-> main `CMD_SECM_BAN_IP` /
|
||||||
|
`CMD_SECM_BAN_IP_REPLY` round trip.
|
||||||
|
**Links:** REQ-SEC-*
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Completeness notes
|
||||||
|
|
||||||
|
- **Coverage**: every `cmd_request_t` value in `src/defs.h` with a
|
||||||
|
corresponding message in `src/ipc.proto` has at least one requirement
|
||||||
|
above, except the latency-stats path (`CMD_LATENCY_STATS_DELTA`,
|
||||||
|
`latency_stats_delta`) and `CMD_SECM_STATS`/`CMD_SECM_RELOAD`, which are
|
||||||
|
periodic/administrative and have no security-relevant validation beyond
|
||||||
|
REQ-IPC-001/002. `[UNDOCUMENTED: if these messages gain validation logic
|
||||||
|
in the future, add requirements here.]`
|
||||||
|
- **ctl.proto**: only the termination flow (REQ-IPC-070..072) is covered in
|
||||||
|
detail, since it is the security-relevant occtl path (it can end a user's
|
||||||
|
VPN session). Read-only `occtl` queries (`status_rep`, `user_list_rep`,
|
||||||
|
`ban_list_rep`, etc.) are reporting-only and have no MUST/MUST NOT
|
||||||
|
contract beyond "reflect server state accurately" —
|
||||||
|
`[UNDOCUMENTED: candidate for a future REQ-IPC-LOG-* style requirement if
|
||||||
|
occtl output is found to diverge from server state]`.
|
||||||
|
- **Field coverage gap**: `group_cfg_st` (the per-user/group supplemental
|
||||||
|
config forwarded in `auth_cookie_reply_msg.config` and
|
||||||
|
`secm_session_reply_msg.config`) is not covered field-by-field here; its
|
||||||
|
contract is primarily a `CFG` concern and is covered in
|
||||||
|
`internal/sec-mod.md` (population) and `internal/worker.md` (consumption).
|
||||||
@@ -0,0 +1,468 @@
|
|||||||
|
---
|
||||||
|
title: main process requirements
|
||||||
|
generator: requirements-from-implementation
|
||||||
|
process: main
|
||||||
|
id-prefix: REQ-MAIN
|
||||||
|
sources:
|
||||||
|
- src/main.c
|
||||||
|
- src/main-ban.c
|
||||||
|
- src/main-auth.c
|
||||||
|
- src/main-worker-cmd.c
|
||||||
|
- src/main-sec-mod-cmd.c
|
||||||
|
- src/main-proc.c
|
||||||
|
- src/ip-lease.c
|
||||||
|
- src/vpn.h
|
||||||
|
- doc/design.md#the-main-process
|
||||||
|
- doc/requirements/internal/ipc.md
|
||||||
|
---
|
||||||
|
|
||||||
|
# main Process Requirements
|
||||||
|
|
||||||
|
main runs as root and owns the TCP/UDP listeners, TUN device allocation, IP
|
||||||
|
leasing, the per-connection ban list, and worker process lifecycle. It
|
||||||
|
delegates authentication and credential handling to sec-mod entirely (see
|
||||||
|
`internal/ipc.md` and `internal/sec-mod.md`). This document covers main's
|
||||||
|
own state: listener setup, the fork/worker-spawn sequence, IP lease
|
||||||
|
allocation, and the ban-score database (`src/main-ban.c`).
|
||||||
|
|
||||||
|
## INIT
|
||||||
|
|
||||||
|
### REQ-MAIN-INIT-001 — Worker fork clears parent listener and sec-mod fds before exec-equivalent setup
|
||||||
|
|
||||||
|
**Requirement:** Immediately after `fork()` for a new worker, the child
|
||||||
|
MUST: reset signal mask to `sig_default_set`; close the unused end of the
|
||||||
|
command socketpair (`cmd_fd[0]`); call `clear_lists(s)` to release the
|
||||||
|
parent's listener/proc lists; close `s->top_fd` if open; and close every
|
||||||
|
`sec_mod_instances[i].sec_mod_fd` / `sec_mod_fd_sync` — so the worker
|
||||||
|
inherits no listening sockets and no direct sec-mod control channel.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main.c:1197-1211
|
||||||
|
**Acceptance:** [SEC] negative, local — after a worker is spawned, inspect
|
||||||
|
its open file descriptors (`/proc/<pid>/fd`); confirm no listener socket fds
|
||||||
|
and no `sec_mod_fd`/`sec_mod_fd_sync` fds are present, only `cmd_fd[1]` and
|
||||||
|
the accepted client connection.
|
||||||
|
**Links:** REQ-IPC-040, REQ-SEC-001
|
||||||
|
|
||||||
|
### REQ-MAIN-INIT-002 — sec_auth_init_hmac is computed by main and the HMAC key is zeroized in the child
|
||||||
|
|
||||||
|
**Requirement:** The worker child MUST receive `ws->sec_auth_init_hmac`
|
||||||
|
computed by main from `s->hmac_key` plus the worker's
|
||||||
|
`remote_ip_str`/`our_ip_str`/`session_start_time` (REQ-IPC-040), and main
|
||||||
|
MUST `safe_memset()` its copy of `s->hmac_key` to zero in the child
|
||||||
|
immediately after computing this HMAC, so the worker process never holds
|
||||||
|
the key used to validate `SEC_AUTH_INIT` HMACs.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main.c:1254-1269
|
||||||
|
**Acceptance:** [SEC] negative, local — after worker spawn, confirm
|
||||||
|
`ws->sec_auth_init_hmac` is set (`HMAC_DIGEST_SIZE` bytes, non-zero) but
|
||||||
|
`s->hmac_key` is all-zero in the worker's memory image (e.g. via core dump
|
||||||
|
inspection in a test harness, or a debug build assertion). A worker that
|
||||||
|
recovered `hmac_key` could forge `SEC_AUTH_INIT` HMACs for sec-mod
|
||||||
|
(REQ-IPC-010).
|
||||||
|
**Links:** REQ-IPC-010, REQ-IPC-040, REQ-MAIN-SEC-001
|
||||||
|
|
||||||
|
### REQ-MAIN-INIT-003 — sec-mod instance for a session is selected by client IP hash, fixed at fork time
|
||||||
|
|
||||||
|
**Requirement:** The worker's `sec_mod_instance_index` (and thus which
|
||||||
|
sec-mod instance will own its `client_entry_st`/SID) MUST be computed once,
|
||||||
|
at fork time, as `hash(remote_addr) % sec_mod_instance_count`, and copied
|
||||||
|
into `ws->secmod_addr`/`secmod_addr_len`. This selection MUST NOT change for
|
||||||
|
the lifetime of the worker, because "each cookie is valid for its IP address
|
||||||
|
and when resuming it must reach the same sec-mod process that contains the
|
||||||
|
corresponding session information under the SID."
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main.c:1217-1234 (comment explains rationale)
|
||||||
|
**Acceptance:** unit, local — for `sec_mod_instance_count > 1`, confirm two
|
||||||
|
connections from the same client IP always select the same
|
||||||
|
`sec_mod_instance_index`, and that a resumed cookie (REQ-IPC-021) is routed
|
||||||
|
to the same instance that issued it (`AUTH_COOKIE_REQ` ->
|
||||||
|
`secmod_addr`/`secmod_addr_len` from `worker_startup_msg`, not
|
||||||
|
recomputed).
|
||||||
|
**Links:** REQ-IPC-017, REQ-IPC-021
|
||||||
|
|
||||||
|
### REQ-MAIN-INIT-004 — Connection limit and TCP-wrapper/ban checks precede fork
|
||||||
|
|
||||||
|
**Requirement:** main MUST reject a new connection — closing the accepted
|
||||||
|
fd without forking a worker — if (a) `max_clients > 0` and
|
||||||
|
`s->stats.active_clients >= max_clients`, (b) `check_tcp_wrapper(fd) < 0`
|
||||||
|
(`/etc/hosts.{allow,deny}`), or (c) for non-Unix, non-proxy-protocol
|
||||||
|
listeners, `check_if_banned()` is true for `ws->remote_addr`. These checks
|
||||||
|
MUST run before `socketpair()`/`fork()`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main.c:1157-1186
|
||||||
|
**Acceptance:** negative, local — (a) set `max-clients` to current active
|
||||||
|
count and attempt a new connection, confirm it is closed and no worker
|
||||||
|
process appears; (b) configure `hosts.deny` to block the test client IP,
|
||||||
|
confirm rejection; (c) drive `check_if_banned` true (REQ-MAIN-SEC-002),
|
||||||
|
confirm rejection. In each case confirm no new PID is added to
|
||||||
|
`s->proc_list`.
|
||||||
|
**Links:** REQ-MAIN-SEC-002, REQ-MAIN-SEC-003
|
||||||
|
|
||||||
|
### REQ-MAIN-INIT-005 — Proxy-protocol listeners skip the connect-time ban check
|
||||||
|
|
||||||
|
**Requirement:** When `listen_proxy_proto` is enabled, main MUST NOT call
|
||||||
|
`check_if_banned()` using the immediate peer address at accept time (that
|
||||||
|
address is the load balancer's, not the client's) — ban enforcement for
|
||||||
|
proxy-protocol connections happens later once the real client address is
|
||||||
|
known from the PROXY protocol header.
|
||||||
|
**Strength:** MUST NOT
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main.c:1173-1186 (`if (ws->conn_type != SOCK_TYPE_UNIX &&
|
||||||
|
!GETRCONFIG(s)->listen_proxy_proto)`)
|
||||||
|
**Acceptance:** [SEC] Confirmed — the post-PROXY-header ban check exists at
|
||||||
|
src/main-worker-cmd.c:387-405, in the `CMD_SESSION_INFO` handler. Sequencing:
|
||||||
|
the worker calls `parse_proxy_proto_header()` (src/worker-vpn.c:895) to
|
||||||
|
populate `ws->remote_addr` with the real client address *before* the TLS
|
||||||
|
handshake, then calls `session_info_send()` (src/worker-vpn.c:968, right
|
||||||
|
after the handshake completes) which, when `listen_proxy_proto` is set,
|
||||||
|
includes `remote_addr`/`our_addr` in `SessionInfoMsg` (src/worker-vpn.c:1146-1153).
|
||||||
|
Main's `CMD_SESSION_INFO` handler, when `GETRCONFIG(s)->listen_proxy_proto`
|
||||||
|
and `tmsg->has_remote_addr`, updates `proc->remote_addr` via
|
||||||
|
`proc_table_update_ip()` and calls `check_if_banned(s, &proc->remote_addr,
|
||||||
|
proc->remote_addr_len)` (main-worker-cmd.c:391-400) — which itself adds
|
||||||
|
`ban_points_connect` per REQ-MAIN-SEC-005 — and if the result is non-zero,
|
||||||
|
calls `kill_proc(proc)` (`SIGTERM`, main.h:189-193) to tear down the worker.
|
||||||
|
Ban enforcement is therefore **deferred, not bypassed**, for proxy-protocol
|
||||||
|
connections: a banned client's connection is terminated after the TLS
|
||||||
|
handshake rather than at accept time. Positive test: configure
|
||||||
|
`listen-proxy-proto`, ban an IP, send a PROXY-protocol-prefixed connection
|
||||||
|
from that IP; confirm the worker completes the TLS handshake but is then
|
||||||
|
`SIGTERM`'d (no CONNECT response). `[NOTE: unlike the non-proxy path (banned
|
||||||
|
before accept-time work begins), a banned client behind a proxy still
|
||||||
|
consumes one TLS handshake's worth of work before being killed — a minor
|
||||||
|
resource-amplification difference, not a bypass. Not flagged as
|
||||||
|
`[SEC-RISK]`; informational only.]`
|
||||||
|
**Links:** REQ-MAIN-SEC-002, REQ-MAIN-SEC-005, doc/design.md#load-balancer-integration
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## NET — IP lease allocation
|
||||||
|
|
||||||
|
### REQ-MAIN-NET-001 — Leased IPs must not collide with existing leases, the TUN address, network, or broadcast address
|
||||||
|
|
||||||
|
**Requirement:** `get_ipv4_lease()` MUST reject a candidate address if (a)
|
||||||
|
an identical `/32` lease already exists (`ip_lease_exists`), (b) the
|
||||||
|
candidate equals the network address, or (c) the candidate equals the
|
||||||
|
computed broadcast address (`network | ~mask`). `get_ipv6_lease()` MUST
|
||||||
|
additionally reject a candidate `/prefix` subnet if it equals the TUN
|
||||||
|
device's own subnet (`ip_cmp(subnet, tun) == 0`) or if that subnet is
|
||||||
|
already leased.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/ip-lease.c:150-184 (`is_ipv6_ok`, `is_ipv4_ok`)
|
||||||
|
**Acceptance:** unit, local — exhaust the configured IPv4 pool to 2 free
|
||||||
|
addresses: the network and broadcast addresses of the subnet; confirm
|
||||||
|
`get_ipv4_lease` does not return either and instead fails with no lease
|
||||||
|
available after `MAX_IP_TRIES`. For IPv6, configure a lease subnet equal to
|
||||||
|
the TUN device's subnet and confirm `get_ipv6_lease` rejects it.
|
||||||
|
**Links:** —
|
||||||
|
|
||||||
|
### REQ-MAIN-NET-002 — IP lease allocation gives up after MAX_IP_TRIES random attempts
|
||||||
|
|
||||||
|
**Requirement:** `get_ipv4_lease()`/`get_ipv6_lease()` MUST attempt at most
|
||||||
|
`MAX_IP_TRIES` (16) randomly-generated candidate addresses before reporting
|
||||||
|
allocation failure; it MUST NOT loop indefinitely searching for a free
|
||||||
|
address in an exhausted pool.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/ip-lease.c:186-193 (`#define MAX_IP_TRIES 16`,
|
||||||
|
`max_loops = MAX_IP_TRIES`)
|
||||||
|
**Acceptance:** unit, local — configure an address pool with 0 free
|
||||||
|
addresses; confirm `get_ipv4_lease` returns an error within `MAX_IP_TRIES`
|
||||||
|
iterations (bounded time), not a hang. With `predictable-ips` enabled,
|
||||||
|
`proc->ipv4_seed` is set once at session creation to
|
||||||
|
`hash_any(username, ...)` (src/sec-mod-auth.c:571-573) and consumed only on
|
||||||
|
the *first* loop iteration (`max_loops == MAX_IP_TRIES`,
|
||||||
|
src/ip-lease.c:300-301). If that single deterministic candidate is rejected
|
||||||
|
by `is_ipv4_ok()` (already leased, or equal to the network/broadcast
|
||||||
|
address), the function does **not** retry other deterministic addresses
|
||||||
|
derived from the seed — it falls through to the same candidates used in the
|
||||||
|
non-predictable case: up to 5 further attempts via `gnutls_rnd()`
|
||||||
|
(src/ip-lease.c:303-313, true random) followed by up to 10 attempts via
|
||||||
|
`ip_from_seed()` chained off the last random value (src/ip-lease.c:314-319),
|
||||||
|
all under the same `max_loops`/`MAX_IP_TRIES` counter. Fallback behavior: a
|
||||||
|
colliding predictable seed silently degrades to random IP assignment for
|
||||||
|
that session, matching the "IP stays the same for the same user when
|
||||||
|
possible" wording in `doc/sample.config` (predictable-ips); after all 16
|
||||||
|
attempts fail, `ERR_NO_IP` is returned regardless of `predictable_ips`.
|
||||||
|
Positive test: with `predictable-ips = true` and the hashed candidate for a
|
||||||
|
test username already leased to another session, confirm the new session
|
||||||
|
receives a different (non-deterministic) IP within `MAX_IP_TRIES`
|
||||||
|
iterations rather than hanging or erroring.
|
||||||
|
**Links:** REQ-SECMOD-SESSION (predictable_ips / ipv4_seed, see
|
||||||
|
src/sec-mod-auth.c:570-579)
|
||||||
|
|
||||||
|
### REQ-MAIN-NET-003 — Reconnecting client (steal) transfers IP leases without re-fetching from the pool
|
||||||
|
|
||||||
|
**Requirement:** `steal_ip_leases(proc, thief)` (used when a client
|
||||||
|
reconnects and supersedes an existing session for the same SID/user) MUST
|
||||||
|
`talloc_move` the existing `ipv4`/`ipv6` lease structures from `proc` to
|
||||||
|
`thief`, MUST call `reset_tun(proc)` on the old process's TUN device (since
|
||||||
|
its fd is only valid in the old worker), and MUST leave `proc` with its own
|
||||||
|
*copy* of the lease addressing info (for disconnect scripts/accounting) —
|
||||||
|
not a shared pointer — by allocating a fresh `ip_lease_st` and `memcpy`-ing
|
||||||
|
`rip`/`lip`/`sig` fields.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/ip-lease.c:105-148
|
||||||
|
**Acceptance:** unit, local — simulate a reconnect (second `AUTH_COOKIE_REQ`
|
||||||
|
for a SID with an existing `proc_st`); confirm the new worker's `proc_st`
|
||||||
|
gets the same IPv4/IPv6 addresses as the old one, the old worker's TUN
|
||||||
|
device is reset, and disconnect accounting for the *old* session still
|
||||||
|
reports the correct (copied) IP — i.e., freeing `thief->ipv4` does not
|
||||||
|
corrupt `proc->ipv4`.
|
||||||
|
**Links:** —
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## SEC — ban list
|
||||||
|
|
||||||
|
### REQ-MAIN-SEC-001 — hmac_key is generated once at startup, never traverses worker IPC, and is zeroed in worker children
|
||||||
|
|
||||||
|
**Requirement:** `s->hmac_key` MUST be generated exactly once, at main
|
||||||
|
startup, via `hmac_init_key()` (src/common/hmac.c:34-37 —
|
||||||
|
`gnutls_rnd(GNUTLS_RND_RANDOM, ...)`), called from src/main.c:1520 — before
|
||||||
|
`run_sec_mod()` forks any sec-mod instance (src/main.c:1658) and long before
|
||||||
|
any worker is forked per connection (src/main.c:1197). `s->hmac_key` MUST
|
||||||
|
NOT appear in any `*.proto` message exchanged over a worker's
|
||||||
|
`cmd_fd`/`cmd_fd_sync` sockets (confirmed by `internal/ipc.md` REQ-IPC-041:
|
||||||
|
no key material in `worker_startup_msg` or any other IPC message). The only
|
||||||
|
processes that ever hold a copy of `hmac_key` are main itself and each
|
||||||
|
sec-mod instance — sec-mod receives it as a direct function argument across
|
||||||
|
`fork()` (src/main-sec-mod-cmd.c:908-909 -> `sec_mod_server()` ->
|
||||||
|
`memcpy` into `sec->hmac_key` at src/sec-mod.c:1029), which it needs to
|
||||||
|
validate `SEC_AUTH_INIT` HMACs via `generate_hmac()`
|
||||||
|
(src/sec-mod-auth.c:906). For each worker, immediately after main computes
|
||||||
|
that worker's `ws->sec_auth_init_hmac = HMAC(s->hmac_key, remote_ip_str ||
|
||||||
|
our_ip_str || session_start_time)` (src/main.c:1262-1266), main MUST
|
||||||
|
`safe_memset()` its own `s->hmac_key` to zero **in the worker child
|
||||||
|
process** (src/main.c:1268-1269), so the unprivileged worker never observes
|
||||||
|
the key itself — only the single derived per-session HMAC.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/common/hmac.c:34-37; src/main.c:1520, 1197-1269, 1658;
|
||||||
|
src/main-sec-mod-cmd.c:908-909; src/sec-mod.c:1029; src/sec-mod-auth.c:906;
|
||||||
|
REQ-IPC-041
|
||||||
|
**Acceptance:** [SEC] (a) unit, local — confirm `hmac_init_key()` is called
|
||||||
|
exactly once per main process lifetime, using `GNUTLS_RND_RANDOM`, before
|
||||||
|
`run_sec_mod()`; (b) static, local — grep-confirm no `*.proto` message field
|
||||||
|
carries `hmac_key` or raw key bytes (REQ-IPC-041); (c) negative, local —
|
||||||
|
after a worker is spawned, inspect its memory image (debug build / core
|
||||||
|
dump) and confirm `s->hmac_key` is all-zero while `ws->sec_auth_init_hmac`
|
||||||
|
is a non-zero `HMAC_DIGEST_SIZE`-byte digest (REQ-MAIN-INIT-002); (d) unit,
|
||||||
|
local — confirm each sec-mod instance's `sec->hmac_key` equals main's
|
||||||
|
original (pre-zeroing) `hmac_key`, since sec-mod uses it to validate
|
||||||
|
`SEC_AUTH_INIT` HMACs.
|
||||||
|
**Links:** REQ-MAIN-INIT-002, REQ-IPC-040, REQ-IPC-041
|
||||||
|
|
||||||
|
### REQ-MAIN-SEC-002 — Ban score is additive, saturating, and IP-prefix-aware for IPv6
|
||||||
|
|
||||||
|
**Requirement:** `add_ip_to_ban_list()` MUST: treat an IPv6 address as its
|
||||||
|
`/64` prefix (zero the low 8 bytes via `massage_ipv6_address`) for banning
|
||||||
|
purposes; add `score` to the existing entry's score using saturating
|
||||||
|
arithmetic (`(e->score + score) > e->score ? ... : e->score` — i.e. never
|
||||||
|
wrap around on overflow); and MUST NOT extend `e->expires` on repeated
|
||||||
|
violations once the entry is already banned (`e->score >= max_ban_score`),
|
||||||
|
"or the user will never be unbanned if he periodically polls the server."
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main-ban.c:118-224
|
||||||
|
**Acceptance:** unit, local — (a) two IPv6 addresses differing only in the
|
||||||
|
low 64 bits MUST share one ban entry; (b) drive `e->score` to
|
||||||
|
`UINT_MAX - 1` then add more points, confirm `e->score` does not wrap to a
|
||||||
|
small value; (c) once banned, send further violations and confirm
|
||||||
|
`e->expires` does not move further into the future.
|
||||||
|
**Links:** REQ-SECMOD-SEC-003
|
||||||
|
|
||||||
|
### REQ-MAIN-SEC-003 — Ban score reset rules: time-based reset only while not currently banned, or after a completed ban expires
|
||||||
|
|
||||||
|
**Requirement:** `add_ip_to_ban_list()` MUST reset `e->score = 0` and
|
||||||
|
`e->last_reset = now` if EITHER (a) the entry's previous ban has expired
|
||||||
|
(`now > e->expires`) while it was still marked banned (`IS_BANNED`), OR (b)
|
||||||
|
`ban_reset_time` has elapsed since `last_reset` AND the entry is currently
|
||||||
|
*not* banned. It MUST NOT reset the score for an entry that is currently
|
||||||
|
banned and not yet expired, even if `ban_reset_time` has elapsed — an active
|
||||||
|
ban cannot be prematurely lifted by the reset-time check.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main-ban.c:164-176 (comment: "to avoid prematurely lifting
|
||||||
|
an active ban")
|
||||||
|
**Acceptance:** unit, local — (1) accumulate score to reach `max_ban_score`
|
||||||
|
(now banned, `expires = now + ban_time`); (2) advance time past
|
||||||
|
`ban_reset_time` but before `expires`; send another violation; confirm
|
||||||
|
`e->score` is NOT reset (still >= `max_ban_score`, ban continues). (3)
|
||||||
|
advance time past `expires`; send another violation; confirm `e->score` IS
|
||||||
|
reset to 0 (+ the new violation's points).
|
||||||
|
**Links:** REQ-MAIN-SEC-002
|
||||||
|
|
||||||
|
### REQ-MAIN-SEC-004 — Local interface addresses are exempt from ban checks
|
||||||
|
|
||||||
|
**Requirement:** `check_if_banned()` MUST return 0 (not banned) without
|
||||||
|
consulting `s->ban_db` for any address that matches a local interface
|
||||||
|
address/netmask in `s->if_addresses` (`if_address_test_local`).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main-ban.c:287-306
|
||||||
|
**Acceptance:** unit, local — add a local interface's address to the ban
|
||||||
|
list directly (e.g. via repeated `add_str_ip_to_ban_list`); confirm
|
||||||
|
`check_if_banned()` for that address still returns 0. This prevents a
|
||||||
|
misconfiguration (or an attacker spoofing a server-local source address)
|
||||||
|
from causing the server to ban itself / a trusted load-balancer IP.
|
||||||
|
**Links:** REQ-MAIN-INIT-005
|
||||||
|
|
||||||
|
### REQ-MAIN-SEC-005 — check_if_banned itself contributes ban_points_connect
|
||||||
|
|
||||||
|
**Requirement:** Every call to `check_if_banned()` for a non-exempt,
|
||||||
|
non-malformed address MUST itself call `add_ip_to_ban_list(...,
|
||||||
|
ban_points_connect)` — i.e., merely attempting a connection contributes
|
||||||
|
points toward a future ban, independent of whether this particular
|
||||||
|
connection is accepted or rejected.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main-ban.c:322-325
|
||||||
|
**Acceptance:** unit, local — make `ban_points_connect` repeated connection
|
||||||
|
attempts (each individually allowed); confirm the `(connect_count *
|
||||||
|
ban_points_connect) >= max_ban_score` threshold results in the next
|
||||||
|
connection being rejected by `check_if_banned`, even though each prior
|
||||||
|
connection was itself accepted.
|
||||||
|
**Links:** REQ-MAIN-SEC-002, REQ-IPC-080
|
||||||
|
|
||||||
|
### REQ-MAIN-SEC-006 — Unban clears score and expiry but does not delete the entry
|
||||||
|
|
||||||
|
**Requirement:** `remove_ip_from_ban_list()` MUST set `e->score = 0` and
|
||||||
|
`e->expires = 0` for a matching entry (after applying the same `/64`
|
||||||
|
IPv6-prefix massaging as REQ-MAIN-SEC-002), and return non-zero, but MUST
|
||||||
|
NOT remove the entry from `s->ban_db` — the entry remains for
|
||||||
|
`last_reset`/future scoring bookkeeping. Returns 0 if no entry matches or
|
||||||
|
`size` is not 4 or 16.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main-ban.c:253-285
|
||||||
|
**Acceptance:** unit, local — `occtl unban <ip>` on a banned IP; confirm
|
||||||
|
`check_if_banned` now returns 0 for that IP, but the entry still exists in
|
||||||
|
`s->ban_db` (e.g. confirmed via a subsequent `occtl show ban points` if
|
||||||
|
such exists, or by re-violating and confirming `last_reset` carries over
|
||||||
|
rather than being treated as a brand-new entry).
|
||||||
|
**Links:** REQ-MAIN-SEC-002, REQ-MAIN-SEC-003
|
||||||
|
|
||||||
|
### REQ-MAIN-SEC-007 — cleanup_banned_entries removes only fully-expired-and-stale entries
|
||||||
|
|
||||||
|
**Requirement:** `cleanup_banned_entries()` MUST delete a ban entry only if
|
||||||
|
BOTH `now >= e->expires` AND `now > e->last_reset + ban_reset_time` — an
|
||||||
|
entry whose ban has expired but whose reset window has not yet elapsed MUST
|
||||||
|
be retained (so its score history is not lost prematurely).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main-ban.c:344-363
|
||||||
|
**Acceptance:** unit, local — create an entry with `expires` in the past but
|
||||||
|
`last_reset + ban_reset_time` in the future; run
|
||||||
|
`cleanup_banned_entries()`; confirm the entry is retained. Advance time past
|
||||||
|
`last_reset + ban_reset_time`; confirm it is now removed.
|
||||||
|
**Links:** REQ-MAIN-SEC-003
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## TEARDOWN
|
||||||
|
|
||||||
|
### REQ-MAIN-TEARDOWN-001 — Workers exceeding auth_timeout without completing authentication are killed
|
||||||
|
|
||||||
|
**Requirement:** `kill_children_auth_timeout()` MUST terminate
|
||||||
|
(`remove_proc(..., RPROC_KILL)`) any `proc_st` with `status <
|
||||||
|
PS_AUTH_COMPLETED` whose `conn_time` is older than `now - auth_timeout`,
|
||||||
|
during periodic maintenance.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main.c:982-998
|
||||||
|
**Acceptance:** negative, local — start a connection, stall before
|
||||||
|
completing `SEC_AUTH_CONT`/`AUTH_COOKIE_REQ` past `auth-timeout`; confirm
|
||||||
|
the worker process is killed by the next maintenance tick. Cross-reference
|
||||||
|
REQ-IPC-011 (sec-mod's independent `auth_timeout` replay-window check —
|
||||||
|
both checks use the same configuration value but enforce it in different
|
||||||
|
processes).
|
||||||
|
**Links:** REQ-IPC-011
|
||||||
|
|
||||||
|
### REQ-MAIN-TEARDOWN-002 — SIGTERM with server-drain-ms stops listeners before terminating sessions
|
||||||
|
|
||||||
|
**Requirement:** On `SIGTERM`, if `server_drain_ms > 0`, main MUST first
|
||||||
|
stop and close all listening sockets (no new connections accepted) and
|
||||||
|
start a `server_drain_ms` timer, deferring `terminate_server()` (which kills
|
||||||
|
existing worker sessions) until that timer fires. If `server_drain_ms == 0`,
|
||||||
|
`terminate_server()` MUST run immediately on `SIGTERM`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main.c:1037-1068
|
||||||
|
**Acceptance:** unit, local — with `server-drain-ms > 0`, send `SIGTERM`;
|
||||||
|
confirm new connection attempts fail (listener closed) while existing
|
||||||
|
sessions continue for up to `server_drain_ms`, then are terminated. With
|
||||||
|
`server-drain-ms = 0`, confirm immediate termination. Cross-reference
|
||||||
|
`doc/sample.config` `server-drain-ms`.
|
||||||
|
**Links:** REQ-MAIN-TEARDOWN-003
|
||||||
|
|
||||||
|
### REQ-MAIN-TEARDOWN-003 — terminate_server force-kills remaining workers after a 5s grace period
|
||||||
|
|
||||||
|
**Requirement:** `terminate_server()` MUST send termination signals to all
|
||||||
|
children via `kill_children()`, then wait up to 5000ms (polling
|
||||||
|
`waitpid(-1, NULL, WNOHANG)`) for them to exit; if children remain after
|
||||||
|
5000ms, it MUST `kill(0, SIGKILL)` (signal the entire process group) rather
|
||||||
|
than waiting indefinitely.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main.c:1000-1028
|
||||||
|
**Acceptance:** unit, local — spawn a worker that ignores `SIGTERM`
|
||||||
|
(test-only signal handler); trigger server termination; confirm the worker
|
||||||
|
is force-killed via `SIGKILL` within ~5 seconds and `ev_break` is reached
|
||||||
|
(main exits).
|
||||||
|
**Links:** REQ-MAIN-TEARDOWN-002
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## CFG — reload
|
||||||
|
|
||||||
|
### REQ-MAIN-CFG-001 — SIGHUP reloads sec-mod before main's own config
|
||||||
|
|
||||||
|
**Requirement:** On `SIGHUP`, main MUST, for each sec-mod instance: send
|
||||||
|
`SIGHUP` to the sec-mod process AND send `CMD_SECM_RELOAD` (via
|
||||||
|
`secmod_reload()`) — and only after all sec-mod instances have been signaled
|
||||||
|
does main call `reload_cfg_file()` for its own configuration. If
|
||||||
|
`secmod_reload()` fails for any instance, main MUST trigger a full
|
||||||
|
`SIGTERM` (`ev_feed_signal_event(loop, SIGTERM)`) rather than continuing
|
||||||
|
with an inconsistent reload.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/main.c:1070-1091 (comment: "Reload on main needs to happen
|
||||||
|
later than sec-mod. That's because of a test that the certificate matches
|
||||||
|
the used key.")
|
||||||
|
**Acceptance:** unit, local — trigger `SIGHUP` and confirm ordering via
|
||||||
|
log timestamps: sec-mod's `CMD_SECM_RELOAD_REPLY` (REQ-SECMOD, see
|
||||||
|
src/sec-mod.c:491-501) is observed before main's `reload_cfg_file` log
|
||||||
|
entry. Negative: make `secmod_reload()` fail (e.g. stop one sec-mod
|
||||||
|
instance before SIGHUP); confirm main initiates full shutdown rather than a
|
||||||
|
partial reload.
|
||||||
|
**Links:** REQ-SECMOD-INIT-001
|
||||||
|
|
||||||
|
## Completeness notes
|
||||||
|
|
||||||
|
- **Listener setup** (`_listen_ports`, `listen_ports`,
|
||||||
|
`set_udp_socket_options`, `set_common_socket_options`) is configuration
|
||||||
|
plumbing with no MUST/MUST NOT contract beyond "bind what's configured in
|
||||||
|
`doc/sample.config` (`listen-host`, `tcp-port`, `udp-port`, etc.)" —
|
||||||
|
`[UNDOCUMENTED: candidate REQ-MAIN-INIT-* if specific socket-option
|
||||||
|
requirements (e.g. SO_REUSEADDR, IPV6_V6ONLY) are found to matter for
|
||||||
|
correctness rather than just performance.]`
|
||||||
|
- **UDP session forwarding** (`forward_udp_to_owner`,
|
||||||
|
`CMD_UDP_FD`/`udp_fd_msg`): not covered here. `[UNDOCUMENTED: candidate
|
||||||
|
REQ-MAIN-NET-* + REQ-IPC-* covering how main demultiplexes UDP packets to
|
||||||
|
the owning worker by source address, and what happens on a DTLS
|
||||||
|
client-hello from an address with no matching worker.]`
|
||||||
|
- **occtl control socket** (`main-ctl-unix.c`): only the termination flow is
|
||||||
|
covered (`internal/ipc.md` REQ-IPC-070..072). Other `occtl` commands
|
||||||
|
(`show status`, `show ip bans`, `reload`, `show iroutes`) are
|
||||||
|
reporting/administrative with no further requirements in this pass.
|
||||||
|
- **chroot** (`src/main.c:1675-1691`): sec-mod's chdir-to-chroot behavior is
|
||||||
|
noted but not analyzed for completeness — `[UNDOCUMENTED: candidate
|
||||||
|
REQ-MAIN-SEC-* covering what happens if chroot_dir is misconfigured
|
||||||
|
(relative vs absolute socket paths).]`
|
||||||
@@ -0,0 +1,321 @@
|
|||||||
|
---
|
||||||
|
title: sec-mod requirements
|
||||||
|
generator: requirements-from-implementation
|
||||||
|
process: sec-mod
|
||||||
|
id-prefix: REQ-SECMOD
|
||||||
|
sources:
|
||||||
|
- src/sec-mod.c
|
||||||
|
- src/sec-mod-auth.c
|
||||||
|
- src/sec-mod-auth.h
|
||||||
|
- src/sec-mod-db.c
|
||||||
|
- src/sec-mod-cookies.c
|
||||||
|
- src/sec-mod-resume.c
|
||||||
|
- src/sec-mod.h
|
||||||
|
- src/defs.h
|
||||||
|
- src/vpn.h
|
||||||
|
- doc/design.md#the-security-module-process
|
||||||
|
- doc/requirements/internal/ipc.md
|
||||||
|
---
|
||||||
|
|
||||||
|
# sec-mod Requirements
|
||||||
|
|
||||||
|
sec-mod runs as root, holds private keys, session state (`client_db`), and
|
||||||
|
performs authentication and accounting. It is the only process that may
|
||||||
|
access credentials and the only process trusted to assign SIDs and issue
|
||||||
|
session cookies. See `internal/ipc.md` for the message-level contracts;
|
||||||
|
this document covers sec-mod's internal state machine, the auth module
|
||||||
|
vtable (`auth_mod_st`, `src/sec-mod-auth.h`), and session/cookie lifecycle.
|
||||||
|
|
||||||
|
## INIT
|
||||||
|
|
||||||
|
### REQ-SECMOD-INIT-001 — Per-vhost module initialization is idempotent
|
||||||
|
|
||||||
|
**Requirement:** `sec_auth_init()` MUST initialize each enabled auth
|
||||||
|
module's `vhost_init` and the accounting module's `vhost_init` for a vhost
|
||||||
|
at most once: it MUST only call `vhost_init` when `auth_ctx`/`acct_ctx` is
|
||||||
|
still `NULL`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-auth.c:63-85
|
||||||
|
**Acceptance:** unit, local — call `sec_auth_init(vhost)` twice; confirm
|
||||||
|
`vhost_init` is invoked exactly once per configured auth/acct module
|
||||||
|
(e.g. via a counting mock module).
|
||||||
|
**Links:** —
|
||||||
|
|
||||||
|
## AUTH — auth_mod_st vtable contract
|
||||||
|
|
||||||
|
The auth-module vtable (`src/sec-mod-auth.h`), per-method authentication
|
||||||
|
requirements, and the certificate defense-in-depth/group-selection logic
|
||||||
|
formerly numbered `REQ-SECMOD-AUTH-001..006` have moved to
|
||||||
|
`internal/authentication.md` as `REQ-AUTH-AUTH-001..006`, alongside a full
|
||||||
|
per-method (plain/PAM/RADIUS/GSSAPI/OIDC/certificate) requirements pass. The
|
||||||
|
old IDs are kept reserved (not reused) for citation stability:
|
||||||
|
|
||||||
|
| Old ID | New ID |
|
||||||
|
|--------|--------|
|
||||||
|
| `REQ-SECMOD-AUTH-001` | `REQ-AUTH-AUTH-001` |
|
||||||
|
| `REQ-SECMOD-AUTH-002` | `REQ-AUTH-AUTH-002` |
|
||||||
|
| `REQ-SECMOD-AUTH-003` | `REQ-AUTH-AUTH-003` |
|
||||||
|
| `REQ-SECMOD-AUTH-004` | `REQ-AUTH-AUTH-004` |
|
||||||
|
| `REQ-SECMOD-AUTH-005` | `REQ-AUTH-AUTH-005` |
|
||||||
|
| `REQ-SECMOD-AUTH-006` | `REQ-AUTH-AUTH-006` |
|
||||||
|
|
||||||
|
`REQ-SECMOD-AUTH-001..006` are `WITHDRAWN` (relocated; see table above).
|
||||||
|
|
||||||
|
## SEC — anti-replay, banning, key isolation
|
||||||
|
|
||||||
|
### REQ-SECMOD-SEC-001 — SID is generated with a CSPRNG and is per-instance-tagged
|
||||||
|
|
||||||
|
**Requirement:** `new_client_entry()` MUST generate `e->sid` using
|
||||||
|
`gnutls_rnd(GNUTLS_RND_RANDOM, ...)` (a cryptographically secure RNG), and
|
||||||
|
MUST overwrite `sid[0]` with `sec->sec_mod_instance_id` so that SIDs from
|
||||||
|
different sec-mod instances (in multi-instance deployments) are
|
||||||
|
distinguishable. If after 3 retries a collision still exists in
|
||||||
|
`client_db`, entry creation MUST fail rather than reuse a SID.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-db.c:98-156
|
||||||
|
**Acceptance:** [SEC] unit, local — generate many client entries and
|
||||||
|
confirm no two share a `sid`; confirm `sid[0]` always equals the
|
||||||
|
configured `sec_mod_instance_id`. Negative: simulate `find_client_entry`
|
||||||
|
always returning non-NULL (collision) and confirm `new_client_entry`
|
||||||
|
returns NULL after 3 retries rather than looping forever or reusing.
|
||||||
|
**Links:** REQ-IPC-017
|
||||||
|
|
||||||
|
### REQ-SECMOD-SEC-002 — safe_id is derived from SID, not independently random
|
||||||
|
|
||||||
|
**Requirement:** `e->acct_info.safe_id` (the value exposed to `occtl` and
|
||||||
|
external accounting, `SAFE_ID_SIZE` bytes printable) MUST be computed by
|
||||||
|
`calc_safe_id(e->sid, SID_SIZE, ...)` — a deterministic derivation from the
|
||||||
|
SID — not a separately generated random value, so that `safe_id` can be used
|
||||||
|
to look up sessions (REQ-IPC-070) without sec-mod maintaining a second
|
||||||
|
index.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-db.c:139-140; src/common/common.c:54-79
|
||||||
|
(`calc_safe_id` calls `safe_hash`, which runs `sid` through SHA-1 via
|
||||||
|
nettle's `sha1_init`/`sha1_update`/`sha1_digest`, then `oc_base64_encode`s
|
||||||
|
the 20-byte digest; comment at common.c:53 states "The goal is one-wayness")
|
||||||
|
**Acceptance:** unit, local — confirm `calc_safe_id(sid, ...)` is
|
||||||
|
deterministic (same `sid` -> same `safe_id`, since SHA-1 + base64 are
|
||||||
|
deterministic) and that `safe_id` does not trivially reveal `sid`: the
|
||||||
|
derivation is SHA-1(`sid`) (not a simple encoding/truncation), so recovering
|
||||||
|
`sid` from `safe_id` requires inverting SHA-1, which is computationally
|
||||||
|
infeasible. `safe_id` MUST nonetheless continue to be treated as sensitive:
|
||||||
|
since it is a deterministic function of `sid` and terminate-by-prefix
|
||||||
|
(REQ-IPC-070) accepts a `safe_id` prefix to select a session, possession of
|
||||||
|
`safe_id` is sufficient to identify/terminate that session even though `sid`
|
||||||
|
itself cannot be recovered from it.
|
||||||
|
**Links:** REQ-SECMOD-SEC-001, REQ-IPC-070
|
||||||
|
|
||||||
|
### REQ-SECMOD-SEC-003 — Wrong-password and connection scoring feed the IP ban list, gated by max-ban-score
|
||||||
|
|
||||||
|
**Requirement:** sec-mod MUST send `CMD_SECM_BAN_IP` with
|
||||||
|
`score = ban_points_wrong_password` on a qualifying failed/retried
|
||||||
|
authentication (REQ-AUTH-AUTH-003, and on final auth failure per
|
||||||
|
src/sec-mod-auth.c:443-445), but MUST NOT send any `CMD_SECM_BAN_IP`
|
||||||
|
message at all if `vhost->config->max_ban_score == 0` (banning disabled for
|
||||||
|
that vhost).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-auth.c:89-119 (`sec_mod_add_score_to_ip`, early
|
||||||
|
return on `max_ban_score == 0`); src/config.c:796-805
|
||||||
|
(`DEFAULT_MAX_BAN_SCORE`, `DEFAULT_PASSWORD_POINTS`)
|
||||||
|
**Acceptance:** unit, local — set `max-ban-score = 0` in vhost config; drive
|
||||||
|
repeated failed authentications; confirm no `CMD_SECM_BAN_IP` is sent
|
||||||
|
(main's ban table for that IP remains empty). With `max-ban-score > 0`,
|
||||||
|
confirm `CMD_SECM_BAN_IP.score` matches the configured
|
||||||
|
`ban-points-wrong-password`. Cross-reference `doc/sample.config` `max-ban-score` /
|
||||||
|
`ban-points-wrong-password` documentation.
|
||||||
|
**Links:** REQ-AUTH-AUTH-003, REQ-IPC-080
|
||||||
|
|
||||||
|
### REQ-SECMOD-SEC-004 — sec-mod never serves a private-key operation for an unbound vhost
|
||||||
|
|
||||||
|
**Requirement:** sec-mod's connection-acceptance path (`accept()` plus
|
||||||
|
`check_upeer_id()` in its main loop) MUST establish which vhost a connecting
|
||||||
|
worker (`cfd`) was started for, and `process_worker_packet()` MUST use that
|
||||||
|
server-recorded vhost — not the `vhost` field of the incoming
|
||||||
|
`sec_op_msg`/`sec_get_pk_msg` — to select the vhost whose `key[key_idx]` is
|
||||||
|
used for `CMD_SEC_SIGN`, `CMD_SEC_DECRYPT`, `CMD_SEC_SIGN_DATA`,
|
||||||
|
`CMD_SEC_SIGN_HASH`, and `CMD_SEC_GET_PK`.
|
||||||
|
**Strength:** MUST NOT
|
||||||
|
**Status:** REVIEW
|
||||||
|
**Source:** src/sec-mod.c:219-338 (`process_worker_packet`, all five
|
||||||
|
key-operation cases call `find_vhost(sec->vconfig, op->vhost)` /
|
||||||
|
`find_vhost(sec->vconfig, pkm->vhost)` — the message-supplied string — then
|
||||||
|
bounds-check and index `vhost->key[key_idx]`); src/sec-mod.c:1160-1201
|
||||||
|
(accept loop — `check_upeer_id()` validates only the peer's uid/gid/pid, and
|
||||||
|
records no vhost for `cfd`); src/vhost.h:137-152 (`find_vhost()` — matches
|
||||||
|
any configured vhost by name, case-insensitively, falling back to
|
||||||
|
`default_vhost()`; never returns NULL); src/ipc.proto:284-300 (`sec_op_msg`,
|
||||||
|
`sec_get_pk_msg` — `vhost` is an optional string in the message, with no
|
||||||
|
`sid` or other connection-identity field); REQ-IPC-051
|
||||||
|
**Acceptance:** [REVIEW: as implemented, this requirement does NOT hold.
|
||||||
|
sec-mod's accept loop performs only OS-level peer-credential validation
|
||||||
|
(`check_upeer_id`) and records no per-`cfd` vhost binding. Each of the five
|
||||||
|
key-operation handlers in `process_worker_packet()` resolves the vhost
|
||||||
|
solely via `find_vhost(sec->vconfig, <message>->vhost)` — a string supplied
|
||||||
|
by the requesting worker — then indexes that vhost's `key[key_idx]`
|
||||||
|
(bounds-checked against `vhost->key_size`, but only for the
|
||||||
|
*message-resolved* vhost, not the requester's own). Concretely: a worker
|
||||||
|
process started for vhost A can send `sec_op_msg{vhost="B", key_idx=0,
|
||||||
|
sig=...}` over its existing sec-mod socket, and sec-mod will perform the
|
||||||
|
signing/decryption with vhost B's private key and return the result — there
|
||||||
|
is no check that the requester is actually serving vhost B. Negative test
|
||||||
|
(once fixed): a worker socket established for vhost A sends a
|
||||||
|
`sec_op_msg`/`sec_get_pk_msg` naming vhost B; sec-mod MUST reject it (e.g.
|
||||||
|
`ERR_AUTH_FAIL`/connection close) rather than performing the operation with
|
||||||
|
vhost B's key. This needs maintainer review per AGENTS.md's "Changes to
|
||||||
|
cookie or SID handling" / privilege-boundary criteria — flagging here rather
|
||||||
|
than resolving unilaterally.]
|
||||||
|
**Links:** REQ-IPC-050, REQ-IPC-051
|
||||||
|
|
||||||
|
## SESSION — client_db lifecycle, cookies, expiry
|
||||||
|
|
||||||
|
### REQ-SECMOD-SESSION-001 — A client_entry_st is keyed solely by SID
|
||||||
|
|
||||||
|
**Requirement:** `find_client_entry()` MUST locate entries by exact 32-byte
|
||||||
|
(`SID_SIZE`) match on `e->sid` only; sec-mod MUST NOT accept a partial or
|
||||||
|
prefix match for `SEC_AUTH_CONT`, `SECM_SESSION_OPEN`, or
|
||||||
|
`SECM_SESSION_CLOSE` (contrast with `terminate_session_by_sid`,
|
||||||
|
REQ-SECMOD-SESSION-005, which intentionally allows a `safe_id` prefix for
|
||||||
|
the human-facing `occtl terminate` command only).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-db.c:158-176
|
||||||
|
**Acceptance:** negative, local — send `SEC_AUTH_CONT` with a `sid` that is
|
||||||
|
a correct prefix of a valid SID but padded/truncated to `SID_SIZE`;
|
||||||
|
confirm `find_client_entry` returns NULL (no match) — i.e., this is a
|
||||||
|
full-value comparison, covered structurally by REQ-IPC-015.
|
||||||
|
**Links:** REQ-IPC-015, REQ-SECMOD-SESSION-005
|
||||||
|
|
||||||
|
### REQ-SECMOD-SESSION-002 — Expiry requires in_use == 0
|
||||||
|
|
||||||
|
**Requirement:** A `client_entry_st` MUST NOT be treated as expired
|
||||||
|
(`IS_CLIENT_ENTRY_EXPIRED`) while `e->in_use > 0`, regardless of
|
||||||
|
`e->exptime`, so that an active session (one with an open
|
||||||
|
`SECM_SESSION_OPEN` reference) is never reaped by `cleanup_client_entries()`
|
||||||
|
mid-use.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod.h:84-87 (`IS_CLIENT_ENTRY_EXPIRED_FULL`:
|
||||||
|
`e->exptime != -1 && now >= e->exptime && e->in_use == 0`)
|
||||||
|
**Acceptance:** unit, local — set `e->exptime` in the past while
|
||||||
|
`e->in_use == 1`; confirm `cleanup_client_entries()` does not delete the
|
||||||
|
entry. Decrement `in_use` to 0 (via `expire_client_entry`); confirm it is
|
||||||
|
now eligible for cleanup once `exptime` passes.
|
||||||
|
**Links:** REQ-IPC-022, REQ-SECMOD-SESSION-003
|
||||||
|
|
||||||
|
### REQ-SECMOD-SESSION-003 — expire_client_entry: persistent-cookies and disconnect-reason determine immediate delete vs. temporary close
|
||||||
|
|
||||||
|
**Requirement:** On `expire_client_entry()` with `e->in_use` reaching 0:
|
||||||
|
- If `persistent_cookies == 0` AND `discon_reason` is one of
|
||||||
|
`REASON_SERVER_DISCONNECT`, `REASON_SESSION_TIMEOUT`, or
|
||||||
|
(`REASON_USER_DISCONNECT` AND `session_is_open`), the entry MUST be
|
||||||
|
deleted immediately (`del_client_entry`) — no cookie reuse is possible.
|
||||||
|
- Otherwise, the entry MUST be kept with a refreshed `exptime`:
|
||||||
|
- For `REASON_USER_DISCONNECT`, `exptime` MUST only be shortened to
|
||||||
|
`now + AUTH_SLACK_TIME` if not `persistent_cookies` or the entry would
|
||||||
|
otherwise outlive `now + AUTH_SLACK_TIME`'s lower bound (i.e. never
|
||||||
|
*extend* expiry on user disconnect).
|
||||||
|
- For all other reasons, `exptime = now + cookie_timeout +
|
||||||
|
AUTH_SLACK_TIME`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-db.c:210-247
|
||||||
|
**Acceptance:** unit, local — for each `discon_reason` value and each
|
||||||
|
`persistent_cookies` setting (0/1), call `expire_client_entry` and assert
|
||||||
|
the resulting state (deleted vs. kept) and, if kept, `exptime`. Cross-check
|
||||||
|
`doc/sample.config` for `persistent-cookies`.
|
||||||
|
**Links:** REQ-SECMOD-SESSION-002, REQ-IPC-031
|
||||||
|
|
||||||
|
### REQ-SECMOD-SESSION-004 — terminate_user_sessions deletes all matching entries unconditionally
|
||||||
|
|
||||||
|
**Requirement:** `terminate_user_sessions(sec, username)` MUST delete every
|
||||||
|
`client_entry_st` with `acct_info.username == username`, regardless of
|
||||||
|
`in_use`, `status`, or `exptime` — administrative termination overrides
|
||||||
|
normal expiry rules (REQ-SECMOD-SESSION-002 does not apply to this path). It
|
||||||
|
MUST return 1 if at least one entry was deleted, 0 if `username` is
|
||||||
|
NULL/empty or no entry matched.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-db.c:252-284
|
||||||
|
**Acceptance:** negative, local — terminate a user with `in_use > 0` (an
|
||||||
|
open session); confirm the entry is deleted immediately and a subsequent
|
||||||
|
`SECM_SESSION_OPEN`/`AUTH_COOKIE_REQ` for that SID fails (REQ-IPC-021,
|
||||||
|
REQ-IPC-072). Confirm return value 0 for an empty username.
|
||||||
|
**Links:** REQ-IPC-071, REQ-IPC-072, REQ-SECMOD-SESSION-002
|
||||||
|
|
||||||
|
### REQ-SECMOD-SESSION-005 — terminate_session_by_sid requires an exact-length safe_id and stops at first match
|
||||||
|
|
||||||
|
**Requirement:** `terminate_session_by_sid()` MUST reject a `safe_id` whose
|
||||||
|
length is not exactly `SAFE_ID_SIZE - 1`, MUST match by prefix
|
||||||
|
(`memcmp` over `safe_id_len` bytes) against `t->acct_info.safe_id`, and MUST
|
||||||
|
stop at the first match (session IDs are assumed unique at this length).
|
||||||
|
Disambiguation of *shorter* prefixes is occtl's responsibility
|
||||||
|
(REQ-IPC-070), not sec-mod's — by the time sec-mod receives
|
||||||
|
`SECM_TERMINATE_SESSION`, the `safe_id` MUST already be full-length.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-db.c:289-331
|
||||||
|
**Acceptance:** negative, local — send `SECM_TERMINATE_SESSION` with a
|
||||||
|
`safe_id` shorter than `SAFE_ID_SIZE - 1`; confirm rejection with log
|
||||||
|
`invalid session ID length` and `terminated == 0`. This is the
|
||||||
|
defense-in-depth backstop for REQ-IPC-070 (occtl-side prefix
|
||||||
|
disambiguation) — even if occtl's check were bypassed, sec-mod will not
|
||||||
|
act on a short prefix.
|
||||||
|
**Links:** REQ-IPC-070, REQ-IPC-071
|
||||||
|
|
||||||
|
### REQ-SECMOD-SESSION-006 — list-cookies omits expired entries and never-expiring in-use sessions report expires=0
|
||||||
|
|
||||||
|
**Requirement:** `handle_secm_list_cookies_reply()` MUST skip entries for
|
||||||
|
which `IS_CLIENT_ENTRY_EXPIRED` is true, and MUST report
|
||||||
|
`CookieIntMsg.expires = 0` (meaning "does not expire") for any entry with
|
||||||
|
`e->in_use > 0`, even if `e->exptime` is set to a finite value.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-cookies.c:74-114
|
||||||
|
**Acceptance:** unit, local — create entries in states {expired,
|
||||||
|
unexpired+in_use=0, unexpired+in_use>0}; request `SECM_LIST_COOKIES`;
|
||||||
|
confirm the expired entry is absent, and the `in_use>0` entry has
|
||||||
|
`expires=0` in the reply (`occtl show sessions valid` reflects this).
|
||||||
|
**Links:** REQ-SECMOD-SESSION-002, REQ-IPC-070
|
||||||
|
|
||||||
|
## TEARDOWN
|
||||||
|
|
||||||
|
### REQ-SECMOD-TEARDOWN-001 — db deinit calls auth_deinit for every remaining entry
|
||||||
|
|
||||||
|
**Requirement:** `sec_mod_client_db_deinit()` MUST call
|
||||||
|
`sec_auth_user_deinit()` (which invokes `auth_mod_st.auth_deinit` if the
|
||||||
|
entry has a module/`auth_ctx`) for every remaining `client_entry_st` before
|
||||||
|
freeing the hash table, so auth modules can release per-session resources
|
||||||
|
(e.g. PAM handles) on sec-mod shutdown — not only on normal session
|
||||||
|
completion.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/sec-mod-db.c:71-85
|
||||||
|
**Acceptance:** unit, local — populate `client_db` with entries that have a
|
||||||
|
mock module with a counting `auth_deinit`; call
|
||||||
|
`sec_mod_client_db_deinit()`; confirm `auth_deinit` was called once per
|
||||||
|
entry, including entries that never reached `PS_AUTH_COMPLETED`.
|
||||||
|
**Links:** —
|
||||||
|
|
||||||
|
## Completeness notes
|
||||||
|
|
||||||
|
- **Vtable gap**: `auth_mod_st` fields `vhost_deinit` and `group_list` have
|
||||||
|
no requirement above — `[UNDOCUMENTED: vhost_deinit's call site was not
|
||||||
|
located in this pass; group_list appears used only by occtl-facing group
|
||||||
|
enumeration, not the auth state machine. Add requirements once call sites
|
||||||
|
are confirmed.]`
|
||||||
|
- **PAM/RADIUS/GSSAPI/OIDC module-specific behaviors** (e.g. PAM
|
||||||
|
conversation function quirks, RADIUS Access-Challenge mapping to
|
||||||
|
`ERR_AUTH_CONTINUE`) are intentionally out of scope here — they implement
|
||||||
|
the `auth_mod_st` contract above and should get their own
|
||||||
|
`internal/auth-<module>.md` if/when needed; this document covers the
|
||||||
|
vtable contract every module must satisfy.
|
||||||
|
- **Reload (`CMD_SECM_RELOAD`)**: sec-mod calls `reload_server(sec)` and
|
||||||
|
replies `CMD_SECM_RELOAD_REPLY` (src/sec-mod.c:491-501) but this document
|
||||||
|
does not enumerate which parts of `sec_mod_st` state survive reload vs.
|
||||||
|
reset — `[UNDOCUMENTED: candidate REQ-SECMOD-INIT-* once reload semantics
|
||||||
|
for client_db / vhost config are confirmed against doc/sample.config
|
||||||
|
reload annotations.]`
|
||||||
@@ -0,0 +1,465 @@
|
|||||||
|
---
|
||||||
|
title: worker process requirements
|
||||||
|
generator: requirements-from-implementation
|
||||||
|
process: worker
|
||||||
|
id-prefix: REQ-WORKER
|
||||||
|
sources:
|
||||||
|
- src/worker.c
|
||||||
|
- src/worker-vpn.c
|
||||||
|
- src/worker-auth.c
|
||||||
|
- src/worker-resume.c
|
||||||
|
- src/worker-privs.c
|
||||||
|
- src/isolate.c
|
||||||
|
- src/tlslib.c
|
||||||
|
- src/worker.h
|
||||||
|
- doc/design.md#the-worker-processes
|
||||||
|
- doc/requirements/internal/ipc.md
|
||||||
|
- doc/requirements/internal/sec-mod.md
|
||||||
|
---
|
||||||
|
|
||||||
|
# worker Process Requirements
|
||||||
|
|
||||||
|
The worker is unprivileged, seccomp-confined, and handles exactly one
|
||||||
|
client's TLS/DTLS session and HTTP(S) authentication exchange. It has no
|
||||||
|
direct access to credentials, private keys, or the ban database — these are
|
||||||
|
delegated to sec-mod (REQ-IPC-050) and main (`ws_add_score_to_ip` /
|
||||||
|
`WORKER_BAN_IP`, REQ-IPC-080) respectively. This document covers the
|
||||||
|
worker's own state machine: startup/config receipt, privilege drop, seccomp
|
||||||
|
filter, TLS/DTLS session setup, the HTTP authentication handlers, and cookie
|
||||||
|
finalization.
|
||||||
|
|
||||||
|
## INIT
|
||||||
|
|
||||||
|
### REQ-WORKER-INIT-001 — Worker refuses to run without OCSERV_ENV_WORKER_STARTUP_MSG
|
||||||
|
|
||||||
|
**Requirement:** `main()` MUST check for the `OCSERV_ENV_WORKER_STARTUP_MSG`
|
||||||
|
environment variable and `exit(EXIT_FAILURE)` with a diagnostic if absent,
|
||||||
|
before any other initialization — the worker binary MUST NOT be runnable
|
||||||
|
standalone.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/worker.c:71-75
|
||||||
|
**Acceptance:** negative, local — run `ocserv-worker` directly (no env var
|
||||||
|
set); confirm it prints "This application is part of ocserv and should not
|
||||||
|
be run in isolation" and exits non-zero without opening any sockets.
|
||||||
|
**Links:** —
|
||||||
|
|
||||||
|
### REQ-WORKER-INIT-002 — set_ws_from_env bounds-checks every variable-length field before memcpy
|
||||||
|
|
||||||
|
**Requirement:** `set_ws_from_env()` MUST validate
|
||||||
|
`msg->secmod_addr.len <= sizeof(ws->secmod_addr)`,
|
||||||
|
`msg->remote_addr.len <= sizeof(ws->remote_addr)`,
|
||||||
|
`msg->our_addr.len <= sizeof(ws->our_addr)`, and
|
||||||
|
`msg->sec_auth_init_hmac.len <= sizeof(ws->sec_auth_init_hmac)` —
|
||||||
|
returning failure (and the worker exiting) if any check fails — before
|
||||||
|
copying the corresponding field with `memcpy`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/worker.c:277-307
|
||||||
|
**Acceptance:** [SEC] negative, unit — construct a `WorkerStartupMsg` with
|
||||||
|
`secmod_addr.len` (or each of the other three fields) larger than the
|
||||||
|
destination struct field; confirm `set_ws_from_env` returns 0 and the
|
||||||
|
worker exits via `return 1` in `main()`, rather than overflowing
|
||||||
|
`ws->secmod_addr`/etc. This is the worker-side mirror of REQ-IPC-041's
|
||||||
|
schema-diff concern — even if main never sends an oversized field, the
|
||||||
|
worker MUST NOT trust that invariant blindly, since `WorkerStartupMsg` is
|
||||||
|
attacker-influenced indirectly (fields like `remote_addr`/`our_addr`
|
||||||
|
ultimately derive from the client's connection).
|
||||||
|
**Links:** REQ-IPC-040, REQ-IPC-041
|
||||||
|
|
||||||
|
### REQ-WORKER-INIT-003 — drop_privileges performs chroot, then chdir("/"), then setgid/setuid, in that order
|
||||||
|
|
||||||
|
**Requirement:** If `chroot_dir` is configured, `drop_privileges()` MUST
|
||||||
|
`chdir(chroot_dir)`, then `chroot(chroot_dir)`, then `chdir("/")` — in this
|
||||||
|
order — each step fatal (`exit(EXIT_FAILURE)`) on failure. `setgid`/
|
||||||
|
`setgroups` (if `gid != -1` and currently root) MUST be performed before
|
||||||
|
`setuid` (if `uid != -1` and currently root) — reversing this order would
|
||||||
|
leave the process unable to change its group membership after dropping
|
||||||
|
root.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/isolate.c:127-184
|
||||||
|
**Acceptance:** [SEC] negative, local — configure `chroot`, `run-as-user`,
|
||||||
|
`run-as-group`; after worker startup, confirm (a) the worker's filesystem
|
||||||
|
root is the configured chroot directory (e.g. via `/proc/<pid>/root`), (b)
|
||||||
|
`getuid()`/`getgid()` in the worker match the configured non-root
|
||||||
|
user/group, and (c) `getgroups()` returns exactly the configured group
|
||||||
|
(supplementary groups dropped via `setgroups(1, &gid)`).
|
||||||
|
**Links:** —
|
||||||
|
|
||||||
|
### REQ-WORKER-INIT-004 — drop_privileges sets RLIMIT_NPROC to 0 after privilege drop
|
||||||
|
|
||||||
|
**Requirement:** `drop_privileges()` MUST call
|
||||||
|
`setrlimit(RLIMIT_NPROC, {0,0})` as its final step, preventing the worker
|
||||||
|
(now unprivileged) from forking any child process. A failure to set this
|
||||||
|
limit MUST be logged but is non-fatal (the worker continues — some
|
||||||
|
container/seccomp environments deny `setrlimit` itself).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/isolate.c:186-193
|
||||||
|
**Acceptance:** [SEC] negative, local — after worker startup, attempt
|
||||||
|
`fork()` from within the worker (e.g. a debug build hook); confirm it fails
|
||||||
|
with `EAGAIN`/`ENOMEM` due to `RLIMIT_NPROC=0`. This is defense-in-depth: a
|
||||||
|
worker that achieved arbitrary code execution still cannot spawn a shell via
|
||||||
|
`fork`+`exec`.
|
||||||
|
**Links:** REQ-WORKER-INIT-005
|
||||||
|
|
||||||
|
### REQ-WORKER-INIT-005 — Worker syscall filter is a fixed allowlist; unlisted syscalls terminate the process
|
||||||
|
|
||||||
|
**Requirement:** When built with `HAVE_LIBSECCOMP`, `disable_system_calls()`
|
||||||
|
MUST install a seccomp filter whose default action is
|
||||||
|
`SCMP_ACT_ERRNO(ENOSYS)` (or `SCMP_ACT_TRAP` under
|
||||||
|
`USE_SECCOMP_TRAP`, which logs via `oc_syslog` and calls
|
||||||
|
`exit(EXIT_FAILURE)` from `sigsys_action`) for any syscall not on the
|
||||||
|
explicit allowlist. The allowlist MUST be installed (`seccomp_load`)
|
||||||
|
before the worker processes any client-supplied data. `ioctl` is
|
||||||
|
allowed ONLY for `SIOCGIFMTU` (`SCMP_A1(SCMP_CMP_EQ, SIOCGIFMTU)`) — no
|
||||||
|
other `ioctl` request codes are permitted.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/worker-privs.c:71-237
|
||||||
|
**Acceptance:** [SEC] negative, local — with seccomp enabled
|
||||||
|
(`ISOLATE_WORKERS=true`, not ASAN/coverage build), attempt to trigger a
|
||||||
|
disallowed syscall from the worker (e.g. `socketpair`, `execve`, `ptrace`,
|
||||||
|
or `ioctl` with a request other than `SIOCGIFMTU`); confirm the worker
|
||||||
|
terminates (under `SCMP_ACT_ERRNO`: the call fails with `ENOSYS` and
|
||||||
|
whatever error handling follows; under `SCMP_ACT_TRAP`: the process exits
|
||||||
|
via `SIGSYS`/`sigsys_action`). A worker that gained code execution via a
|
||||||
|
parsing bug MUST NOT be able to `execve()` a shell or `ptrace()` another
|
||||||
|
process.
|
||||||
|
**Links:** REQ-WORKER-INIT-004
|
||||||
|
|
||||||
|
### REQ-WORKER-INIT-006 — seccomp filter is conditionally relaxed only for socket_wrapper test environments
|
||||||
|
|
||||||
|
**Requirement:** `readlink`/`readlinkat` MUST be added to the seccomp
|
||||||
|
allowlist ONLY when `SOCKET_WRAPPER_DIR` is set in the environment (test
|
||||||
|
harness). Production deployments (no `SOCKET_WRAPPER_DIR`) MUST NOT have
|
||||||
|
these syscalls available to the worker.
|
||||||
|
**Strength:** MUST NOT
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/worker-privs.c:107-112
|
||||||
|
**Acceptance:** [SEC] negative, local — run a production-configured worker
|
||||||
|
(no `SOCKET_WRAPPER_DIR`) and confirm `readlink`/`readlinkat` are rejected
|
||||||
|
by the seccomp filter. Confirmed no other env-var-gated relaxations exist in
|
||||||
|
`disable_system_calls()` (src/worker-privs.c:71-220, the only function
|
||||||
|
adding `seccomp_rule_add(ctx, SCMP_ACT_ALLOW, ...)` rules, called from
|
||||||
|
src/worker-vpn.c:881): every other `ADD_SYSCALL(...)` invocation in that
|
||||||
|
function is unconditional — `SOCKET_WRAPPER_DIR` is the only `getenv()` check
|
||||||
|
in the file, gating exactly the `readlink`/`readlinkat` pair.
|
||||||
|
**Links:** REQ-WORKER-INIT-005
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## AUTH
|
||||||
|
|
||||||
|
The credential-delegation principle and certificate-username-extraction
|
||||||
|
requirements formerly numbered `REQ-WORKER-AUTH-001..003` have moved to
|
||||||
|
`internal/authentication.md`, alongside the full per-auth-method requirements
|
||||||
|
pass. The old IDs are kept reserved (not reused) for citation stability:
|
||||||
|
|
||||||
|
| Old ID | New ID |
|
||||||
|
|--------|--------|
|
||||||
|
| `REQ-WORKER-AUTH-001` | `REQ-AUTH-AUTH-007` |
|
||||||
|
| `REQ-WORKER-AUTH-002` | `REQ-AUTH-AUTH-008` |
|
||||||
|
| `REQ-WORKER-AUTH-003` | `REQ-AUTH-AUTH-009` |
|
||||||
|
|
||||||
|
`REQ-WORKER-AUTH-001..003` are `WITHDRAWN` (relocated; see table above). The
|
||||||
|
cookie/camouflage/IPC-validation requirements below
|
||||||
|
(`REQ-WORKER-AUTH-004..006`) remain here — they concern worker session/cookie
|
||||||
|
handling rather than authentication-method semantics.
|
||||||
|
|
||||||
|
### REQ-WORKER-AUTH-004 — Camouflage gate: failed cookie auth under camouflage returns 405, not 401/503, until the secret is matched
|
||||||
|
|
||||||
|
**Requirement:** When `camouflage` is configured and
|
||||||
|
`ws->camouflage_check_passed == 0`, a failed `cookie_authenticate_or_exit()`
|
||||||
|
MUST respond `405 Method Not Allowed` (indistinguishable from a non-VPN web
|
||||||
|
server's response to an unexpected method) rather than `401`/`503`, which
|
||||||
|
would reveal the presence of an ocserv endpoint to an unauthenticated
|
||||||
|
prober. `check_camouflage_url()` sets `camouflage_check_passed = 1` only
|
||||||
|
when the request URL's query string exactly equals
|
||||||
|
`camouflage_secret`, OR when `ws->auth_state >= S_AUTH_COOKIE` (i.e. the
|
||||||
|
client has already gotten past the camouflage gate once this session).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/worker-auth.c:1044-1057, src/worker-vpn.c:819-836
|
||||||
|
**Acceptance:** [SEC] negative — with `camouflage` configured and a secret
|
||||||
|
set, send a CONNECT request with an invalid cookie and no/incorrect
|
||||||
|
camouflage query string; confirm the response is `405`, not `401` or
|
||||||
|
`503`. Positive: append `?<camouflage-secret>` to the URL with an invalid
|
||||||
|
cookie; confirm the response reverts to `401`/`503` (camouflage passed,
|
||||||
|
real error now shown).
|
||||||
|
**Links:** —
|
||||||
|
|
||||||
|
### REQ-WORKER-AUTH-005 — Cookie auth reply from main is strictly validated before TUN claim and config acceptance
|
||||||
|
|
||||||
|
**Requirement:** `recv_cookie_auth_reply()`, on `AUTH__REP__OK`, MUST
|
||||||
|
validate (failing with `ERR_AUTH_FAIL` and NOT installing `ws->user_config`
|
||||||
|
or `ws->tun_fd` otherwise): `msg->vname`, `msg->config`, `msg->user_name`
|
||||||
|
all non-NULL; `msg->sid.len == sizeof(ws->sid)`;
|
||||||
|
`msg->session_id.len == sizeof(ws->session_id)`;
|
||||||
|
`msg->secmod_addr.len <= sizeof(ws->secmod_addr)`; and that
|
||||||
|
`tun_claim(ws->tun_fd)` succeeds for the fd received via
|
||||||
|
`send_socket_msg_to_worker` (REQ-IPC-023). Any failure MUST result in
|
||||||
|
`ERR_AUTH_FAIL` and the worker exiting via `cookie_authenticate_or_exit()`'s
|
||||||
|
`exit_worker(ws)` path — never falling through to `tun_mainloop` with a
|
||||||
|
partially-populated `ws->user_config`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/worker-auth.c:721-867, src/worker-auth.c:1024-1060
|
||||||
|
**Acceptance:** [SEC] negative, local — (requires fault injection in main or
|
||||||
|
a test double for `AUTH_COOKIE_REP`) send `AUTH__REP__OK` with `sid.len`
|
||||||
|
one byte short of `SID_SIZE`; confirm the worker treats this as
|
||||||
|
`ERR_AUTH_FAIL`, never sets `ws->sid_set = 1`, and exits rather than
|
||||||
|
proceeding into `connect_handler`.
|
||||||
|
**Links:** REQ-IPC-023, REQ-IPC-024
|
||||||
|
|
||||||
|
### REQ-WORKER-AUTH-006 — user_config from AuthCookieReplyMsg is a borrowed pointer; freed only on the error path
|
||||||
|
|
||||||
|
**Requirement:** On `AUTH__REP__OK`, `ws->user_config = msg->config` MUST
|
||||||
|
alias memory owned by the unpacked `AuthCookieReplyMsg` (`msg`) for the
|
||||||
|
remainder of the worker's lifetime — `recv_cookie_auth_reply()` MUST NOT
|
||||||
|
call `auth_cookie_reply_msg__free_unpacked(msg, &pa)` on the success path.
|
||||||
|
On any error path (`ret < 0`), it MUST free `msg` and set
|
||||||
|
`ws->user_config = NULL` to avoid a dangling pointer.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/worker-auth.c:856-866 (comment explicitly documents this
|
||||||
|
lifetime contract)
|
||||||
|
**Acceptance:** [SEC] This is a memory-safety invariant rather than a
|
||||||
|
behaviorally-observable one, and is covered by the existing `asan/clang/Fedora`
|
||||||
|
CI job (.gitlab-ci.yml:403-414) rather than a dedicated test. That job runs
|
||||||
|
with `DISABLE_ASAN_BROKEN_TESTS=1`, which `tests/common.sh:33-36` translates
|
||||||
|
into `ISOLATE_WORKERS=false` — i.e. worker processes run under ASAN/LSAN
|
||||||
|
instrumentation instead of seccomp isolation. `recv_cookie_auth_reply()` is
|
||||||
|
called from `auth_cookie_request()` (src/worker-auth.c:1102) on every
|
||||||
|
successful connection, and `ws->user_config` is dereferenced throughout
|
||||||
|
`worker-vpn.c` for the life of the worker, so the `AUTH__REP__OK` success
|
||||||
|
path (and the subsequent free of `msg`/`ws->user_config` at worker exit) is
|
||||||
|
exercised by every CONNECT-flow `root_scripts` test (e.g.
|
||||||
|
`test-cookie-timeout`, `traffic`) that job runs. A pass of `asan/clang/Fedora`
|
||||||
|
with no LeakSanitizer/ASAN report against `recv_cookie_auth_reply` or
|
||||||
|
`ws->user_config` is sufficient acceptance; no new test required.
|
||||||
|
**Links:** —
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## SEC
|
||||||
|
|
||||||
|
### REQ-WORKER-SEC-001 — Private-key operations are marshaled to sec-mod; the worker never holds the private key
|
||||||
|
|
||||||
|
**Requirement:** `key_cb_sign_data_func`, `key_cb_sign_hash_func`,
|
||||||
|
`key_cb_sign_func`, and `key_cb_decrypt_func` (installed via
|
||||||
|
`gnutls_privkey_import_ext4`/`_ext2` with
|
||||||
|
`GNUTLS_PRIVKEY_IMPORT_AUTO_RELEASE`) MUST each call
|
||||||
|
`key_cb_common_func()`, which sends `CMD_SEC_SIGN_DATA` /
|
||||||
|
`CMD_SEC_SIGN_HASH` / `CMD_SEC_SIGN` / `CMD_SEC_DECRYPT` to sec-mod over
|
||||||
|
`cdata->sa` (the sec-mod socket address recorded at certificate-load time)
|
||||||
|
and returns sec-mod's result — the `gnutls_privkey_t` registered in the
|
||||||
|
worker's TLS credentials holds no exploitable key material, only `cdata`
|
||||||
|
(vhost name, key index, socket address).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/tlslib.c:793-836, src/tlslib.c:893-930; REQ-IPC-050
|
||||||
|
**Acceptance:** [SEC] negative, local — after worker startup (post
|
||||||
|
`drop_privileges`+seccomp), search the worker's heap/memory for the PEM
|
||||||
|
private key's byte pattern (e.g. via a debug build that logs the key,
|
||||||
|
compared against `/proc/<pid>/maps` + `gcore`); confirm it is absent — only
|
||||||
|
sec-mod's process memory contains it. This is the core privilege-boundary
|
||||||
|
invariant restated from the worker's perspective; REQ-IPC-050 covers the
|
||||||
|
IPC framing.
|
||||||
|
**Links:** REQ-IPC-050
|
||||||
|
|
||||||
|
### REQ-WORKER-SEC-002 — TLS session resumption data is size-capped before being sent to sec-mod
|
||||||
|
|
||||||
|
**Requirement:** `resume_db_store()` MUST reject (return
|
||||||
|
`GNUTLS_E_DB_ERROR` without contacting sec-mod) any resumption `data.size >
|
||||||
|
MAX_SESSION_DATA_SIZE` or `key.size > GNUTLS_MAX_SESSION_ID`.
|
||||||
|
`resume_db_fetch()`/`resume_db_delete()` MUST similarly reject
|
||||||
|
`key.size > GNUTLS_MAX_SESSION_ID` before sending `RESUME_FETCH_REQ`/
|
||||||
|
`RESUME_DELETE_REQ`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/worker-resume.c:82-206
|
||||||
|
**Acceptance:** [SEC] negative, unit — call `resume_db_store` with a
|
||||||
|
`gnutls_datum_t` larger than `MAX_SESSION_DATA_SIZE`; confirm it returns
|
||||||
|
`GNUTLS_E_DB_ERROR` and `connect_to_secmod()` is never called (no socket
|
||||||
|
opened). This bounds the size of data sec-mod must store per session
|
||||||
|
(REQ-IPC-060) and prevents a malicious/buggy TLS stack from using session
|
||||||
|
tickets as an amplification vector against sec-mod's memory.
|
||||||
|
**Links:** REQ-IPC-060, REQ-IPC-061
|
||||||
|
|
||||||
|
### REQ-WORKER-SEC-003 — DTLS-PSK key is derived per-session via gnutls_prf from the already-authenticated TLS master secret
|
||||||
|
|
||||||
|
**Requirement:** `setup_dtls_psk_keys()` MUST derive the DTLS-PSK key via
|
||||||
|
`gnutls_prf(ws->session, ..., PSK_LABEL, ...)` over the existing
|
||||||
|
(authenticated) CSTP TLS session — it MUST NOT proceed
|
||||||
|
(`oclog(...); return -1`) if `ws->session == NULL` (no CSTP session to
|
||||||
|
derive from). The legacy DTLS path (`setup_legacy_dtls_keys`) similarly
|
||||||
|
requires `ws->req.selected_ciphersuite != NULL`, failing otherwise.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/worker-vpn.c:273-381
|
||||||
|
**Acceptance:** unit, local — attempt DTLS setup before CSTP TLS handshake
|
||||||
|
completes (`ws->session == NULL`); confirm `setup_dtls_psk_keys` returns -1
|
||||||
|
and no DTLS session is established. This ensures the DTLS channel is always
|
||||||
|
cryptographically bound to an already-authenticated TLS session — a DTLS
|
||||||
|
channel cannot be the *first* authenticated channel for a worker.
|
||||||
|
**Links:** —
|
||||||
|
|
||||||
|
### REQ-WORKER-SEC-004 — Ban-score reports to main are advisory for final reports; main's verdict on non-final reports is enforced by immediate worker exit
|
||||||
|
|
||||||
|
**Requirement:** `ws_add_score_to_ip()` MUST send a `WORKER_BAN_IP`
|
||||||
|
(`BanIpMsg`) to main and act on `BanIpReplyMsg` (REQ-IPC-080) as follows:
|
||||||
|
the worker holds no ban database and cannot itself decide whether an IP is
|
||||||
|
banned — that decision is made by main (`add_str_ip_to_ban_list`,
|
||||||
|
`check_if_banned`, REQ-MAIN-SEC-002..007). For a **final** report
|
||||||
|
(`final == 1`, sent only from `exit_worker_reason()` when the worker has
|
||||||
|
already decided to exit for `reason`/`discon_reason`), the worker MUST NOT
|
||||||
|
let `reply->reply` alter that already-decided exit. For a **non-final**
|
||||||
|
report (`final == 0`, e.g. on a scoring event during an active session),
|
||||||
|
the worker MUST treat `reply->reply != AUTH__REP__OK` as main's
|
||||||
|
instruction to terminate the *current* connection immediately
|
||||||
|
(`exit(EXIT_FAILURE)`) — main is the sole authority on whether the ban
|
||||||
|
threshold (`max_ban_score`) has been exceeded, but the worker enforces that
|
||||||
|
single verdict locally.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/worker-vpn.c:479-531 (`ws_add_score_to_ip`: `if (final == 0
|
||||||
|
&& reply->reply != AUTH__REP__OK) exit(EXIT_FAILURE);`); src/worker-vpn.c:600-618
|
||||||
|
(`exit_worker_reason` — calls `ws_add_score_to_ip(ws, 0, 1, reason)` only
|
||||||
|
after unconditionally committing to `talloc_free(ws->main_pool)` +
|
||||||
|
`worker_exit(EXIT_FAILURE)`, which execute regardless of `reply->reply`);
|
||||||
|
src/worker-kkdcp.c:153 (`ws_add_score_to_ip(ws,
|
||||||
|
WSRCONFIG(ws)->ban_points_kkdcp, 0, 0)` — the only `final == 0` call site);
|
||||||
|
src/ipc.proto:209-214 (`ban_ip_reply_msg.reply` comment: "whether to
|
||||||
|
disconnect the user"); src/main-worker-cmd.c:298-310 (`CMD_BAN_IP` handler —
|
||||||
|
`reply.reply = AUTH__REP__FAILED` iff `add_str_ip_to_ban_list()` reports the
|
||||||
|
score now exceeds `max_ban_score`, else `AUTH__REP__OK`)
|
||||||
|
**Acceptance:** [SEC] Confirmed by tracing both call sites of
|
||||||
|
`ws_add_score_to_ip`:
|
||||||
|
- **Final report** (`exit_worker_reason`, `final=1`): the `exit()` check
|
||||||
|
at worker-vpn.c:525 is gated on `final == 0`, so `reply->reply` has no
|
||||||
|
effect — the worker exits for the reason it already had, independent of
|
||||||
|
the ban reply. Positive test: drive a session to `exit_worker_reason`
|
||||||
|
with `ws->ban_points > 0` while main's reply is `AUTH__REP__FAILED`;
|
||||||
|
confirm the worker's exit code/log reflects `discon_reason`, not the
|
||||||
|
ban reply.
|
||||||
|
- **Non-final report** (`worker-kkdcp.c`, `final=0`): if main replies
|
||||||
|
`AUTH__REP__FAILED` (max-ban-score exceeded for this IP), the worker
|
||||||
|
calls `exit(EXIT_FAILURE)` immediately, tearing down the current
|
||||||
|
connection. Positive test: configure a low `max-ban-score`, trigger
|
||||||
|
repeated `ban_points_kkdcp`-scoring KKDCP requests from one IP; confirm
|
||||||
|
the worker exits once main reports `AUTH__REP__FAILED`, and that the
|
||||||
|
*next* connection attempt from that IP is rejected by main per
|
||||||
|
REQ-MAIN-SEC-002..007 (the ban now applies to future connections too,
|
||||||
|
not just this one).
|
||||||
|
- Negative: confirm a worker cannot use `BanIpReplyMsg` to learn ban-list
|
||||||
|
state for *other* IPs — `ban_ip_msg.ip = ws->remote_ip_str` is always the
|
||||||
|
worker's own peer address (worker-vpn.c:500), so the reply is scoped to
|
||||||
|
that single IP only.
|
||||||
|
**Links:** REQ-IPC-080, REQ-MAIN-SEC-002, REQ-MAIN-SEC-005
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## NET
|
||||||
|
|
||||||
|
### REQ-WORKER-NET-001 — Virtual host selection from TLS SNI happens before credentials are set, and falls back to default vhost
|
||||||
|
|
||||||
|
**Requirement:** `hello_hook_func()` (or its `peek_client_hello` fallback
|
||||||
|
for GnuTLS < 3.4) MUST call `find_vhost(ws->vconfig, hostname)` using the
|
||||||
|
TLS ClientHello's `server_name` extension (parsed via manual
|
||||||
|
`SKIP8`/`SKIP16` traversal of the handshake message, bounds-checked against
|
||||||
|
`msg->size` and `sizeof(ws->buffer) - 1` at each step), and MUST call
|
||||||
|
`SET_VHOST_CREDS` (selecting the matching vhost's certificate/key
|
||||||
|
credentials for the handshake) regardless of whether a matching vhost was
|
||||||
|
found — falling back to the default vhost. A client requesting an unknown
|
||||||
|
hostname MUST NOT cause the handshake to abort; it proceeds with the
|
||||||
|
default vhost's credentials (logged at `LOG_INFO`).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/worker-vpn.c:671-836
|
||||||
|
**Acceptance:** unit, local — connect with SNI set to a hostname not
|
||||||
|
matching any configured vhost; confirm the handshake completes using the
|
||||||
|
default vhost's certificate, and the server logs "client requested hostname
|
||||||
|
... does not match known vhost". [SEC] negative — send a ClientHello with a
|
||||||
|
server_name extension whose declared length (`hsize`) would read past
|
||||||
|
`msg->size` or past `sizeof(ws->buffer)`; confirm `hello_hook_func` detects
|
||||||
|
this (`hsize == 0 || hsize + pos > msg->size || hsize > sizeof(ws->buffer) -
|
||||||
|
1`) and aborts SNI parsing (`goto finish`) without an out-of-bounds
|
||||||
|
`memcpy`.
|
||||||
|
**Links:** —
|
||||||
|
|
||||||
|
### REQ-WORKER-NET-002 — connect_handler only accepts the literal CONNECT target /CSCOSSLC/tunnel
|
||||||
|
|
||||||
|
**Requirement:** `connect_handler()` MUST respond `404` and close the
|
||||||
|
connection (`cstp_fatal_close` + `exit_worker`) for any CONNECT request
|
||||||
|
whose `req->url` is neither `/CSCOSSLC/tunnel` nor `CSCOSSLC/tunnel`
|
||||||
|
(the latter accommodating a known Clavister Android client defect). This
|
||||||
|
check MUST happen after `cookie_authenticate_or_exit()` — i.e. an
|
||||||
|
unauthenticated client gets the cookie-auth error response, not a 404,
|
||||||
|
regardless of the URL it requested.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/worker-vpn.c:2075-2085
|
||||||
|
**Acceptance:** negative, local — with a valid cookie, send
|
||||||
|
`CONNECT /not-the-tunnel-path HTTP/1.1`; confirm `404` and connection
|
||||||
|
close. With an invalid/absent cookie and the same bad path, confirm the
|
||||||
|
cookie-auth error (REQ-WORKER-AUTH-005) is returned, not `404` — ordering
|
||||||
|
matters for camouflage (REQ-WORKER-AUTH-004).
|
||||||
|
**Links:** REQ-WORKER-AUTH-004, REQ-WORKER-AUTH-005
|
||||||
|
|
||||||
|
### REQ-WORKER-NET-003 — DTLS is enabled only if UDP is configured, not disabled by user config, and a TLS master secret was captured
|
||||||
|
|
||||||
|
**Requirement:** `connect_handler()` MUST set
|
||||||
|
`DTLS_ACTIVE(ws)->udp_state = DTLS_INACTIVE(ws)->udp_state = UP_WAIT_FD`
|
||||||
|
(enabling DTLS) only if ALL of: `WSSCONFIG(ws)->udp_port != 0`,
|
||||||
|
`!WSRCONFIG(ws)->no_udp` (REQ-IPC AUTH_COOKIE_REP `config.no_udp`,
|
||||||
|
REQ-WORKER-AUTH-005), and `req->master_secret_set != 0` (the TLS handshake
|
||||||
|
exposed a master secret via the `SSLKEYLOGFILE`-style hook). If any
|
||||||
|
condition fails, DTLS MUST remain `UP_DISABLED` and `ws->master_secret`
|
||||||
|
MUST NOT be populated from `req->master_secret`.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Status:** DERIVED
|
||||||
|
**Source:** src/worker-vpn.c:2157-2168
|
||||||
|
**Acceptance:** unit, local — (a) set `no_udp` for a user (via sec-mod
|
||||||
|
per-user config) and confirm `X-DTLS-*` headers are absent / DTLS never
|
||||||
|
activates for that session even though `udp-port` is globally configured;
|
||||||
|
(b) configure `udp-port = 0` and confirm the same.
|
||||||
|
**Links:** REQ-WORKER-SEC-003
|
||||||
|
|
||||||
|
## Completeness notes
|
||||||
|
|
||||||
|
- **`worker-vpn.c` main loops** (`tls_mainloop`, `dtls_mainloop`,
|
||||||
|
`tun_mainloop`, `parse_data`/`parse_cstp_data`/`parse_dtls_data`,
|
||||||
|
~2600 lines combined): the per-packet CSTP/DTLS framing and tun-bridging
|
||||||
|
protocol is largely *protocol*-level (what bytes mean) rather than
|
||||||
|
process-security — those requirements belong in
|
||||||
|
`protocol/sources/anyconnect.md` (AC-CLIENT-*) and `protocol/unified.md`
|
||||||
|
(REQ-PROTO-DATA-*), not here. `[UNDOCUMENTED: candidate
|
||||||
|
REQ-WORKER-NET-* if a process-boundary-relevant invariant is found in
|
||||||
|
these loops, e.g. bounds-checking of attacker-controlled length fields
|
||||||
|
before `tun_enqueue_write`.]`
|
||||||
|
- **`worker-kkdcp.c`** (GSSAPI KKDCP proxying, 334 lines): not examined in
|
||||||
|
this pass. `[UNDOCUMENTED: candidate REQ-WORKER-AUTH-* / REQ-WORKER-NET-*
|
||||||
|
covering how KKDCP requests are validated/forwarded — this is a
|
||||||
|
worker-to-KDC proxy path and warrants its own security review given it
|
||||||
|
involves the worker making outbound network connections.]`
|
||||||
|
- **`worker-proxyproto.c`** (492 lines, PROXY protocol v1/v2 parsing): not
|
||||||
|
examined. `[UNDOCUMENTED: candidate REQ-WORKER-NET-* — this is the
|
||||||
|
worker-side counterpart to REQ-MAIN-INIT-005's open question about
|
||||||
|
post-PROXY-header ban enforcement; also a natural place for
|
||||||
|
length/bounds-checking requirements on attacker-controlled proxy headers.]`
|
||||||
|
- **`worker-svc.c`** (Cisco SVC/AnyConnect-specific binary protocol
|
||||||
|
handling, 304 lines): deferred to `protocol/sources/anyconnect.md`
|
||||||
|
(AC-CLIENT-*) per the project layout — this is client-compatibility
|
||||||
|
protocol surface, not a process-isolation concern.
|
||||||
|
- **`worker-http.c`/`worker-http-handlers.c`** (HTTP parsing via llhttp,
|
||||||
|
~1187 lines combined): the HTTP request/response framing itself
|
||||||
|
(header limits, method routing) is protocol-level;
|
||||||
|
`MAX_HTTP_REQUESTS`/`requests_left` (src/worker-vpn.c:860) bounds the
|
||||||
|
number of pipelined requests per TLS connection — `[UNDOCUMENTED:
|
||||||
|
candidate REQ-WORKER-NET-* if this bound has security significance
|
||||||
|
beyond resource exhaustion, e.g. preventing auth-state confusion across
|
||||||
|
pipelined requests.]`
|
||||||
|
- **GSSAPI/KKDCP ASN.1 init** (`asn1_array2tree(kkdcp_asn1_tab, ...)`,
|
||||||
|
src/worker.c:175-182): fatal on failure at startup; not analyzed further.
|
||||||
@@ -0,0 +1,473 @@
|
|||||||
|
---
|
||||||
|
title: AnyConnect / Cisco-compatibility client protocol requirements (working set)
|
||||||
|
generator: requirements-elicitation
|
||||||
|
process: n/a
|
||||||
|
id-prefix: AC-CLIENT
|
||||||
|
sources:
|
||||||
|
- doc/README-cisco-svc.md
|
||||||
|
- src/worker-svc.c
|
||||||
|
- src/worker-http.c (user-agent detection, known_urls table)
|
||||||
|
- src/worker-auth.c (cisco_client_compat, ANYCONNECT_CLIENT_COMPAT, webvpnc cookie, profile delivery)
|
||||||
|
- src/worker-vpn.c (per-agent IPv6/DNS/route behavior)
|
||||||
|
- src/worker.h (user_agent_type enum, ws->req.is_ios)
|
||||||
|
---
|
||||||
|
|
||||||
|
# Scope
|
||||||
|
|
||||||
|
This document captures requirements that are specific to **Cisco AnyConnect** and
|
||||||
|
the broader family of clients/devices that identify themselves via a
|
||||||
|
Cisco-compatible `User-Agent` string, as distinct from the generic OpenConnect
|
||||||
|
protocol covered in `protocol/sources/openconnect.md` (OC-PROTO-*).
|
||||||
|
|
||||||
|
**Core objective** [from doc/README-cisco-svc.md and code]: allow ocserv to
|
||||||
|
interoperate with clients and devices that were built against Cisco's AnyConnect
|
||||||
|
server (ASA) rather than against the OpenConnect protocol draft, by recognizing
|
||||||
|
their identifying strings and adjusting behavior (URL routing, cookie format,
|
||||||
|
config delivery, IPv6/DNS handling, TLS cipher selection) to match what those
|
||||||
|
clients expect.
|
||||||
|
|
||||||
|
**Explicit constraints**:
|
||||||
|
- The `/svc` endpoint and its behavior are gated by `cisco-svc-client-compat = true`
|
||||||
|
(per-vhost config option) — `[PROC: worker]`.
|
||||||
|
- The `ANYCONNECT_CLIENT_COMPAT` build-time option gates an entire family of
|
||||||
|
URL routes and cookie/profile-delivery extensions (`/profiles/*`,
|
||||||
|
`/VPNManifest.xml`, `/1/*`, `/+CSCOT+/*`, `/logout`, `webvpnc` cookie,
|
||||||
|
`<vpn-profile-manifest>` XML fragment) — `[PROC: worker]`.
|
||||||
|
- `cisco_client_compat` (a separate, runtime per-vhost config flag) changes the
|
||||||
|
certificate-presence requirement at cookie-auth time — `[PROC: worker, IPC]`
|
||||||
|
(affects what is sent to main in `AUTH_COOKIE_REQ`).
|
||||||
|
|
||||||
|
**Implicit constraints** `[IMPLICIT]`:
|
||||||
|
- `[IMPLICIT]` All AnyConnect-compatibility behavior must remain additive: it must
|
||||||
|
not change the wire behavior observed by clients that identify as
|
||||||
|
`AGENT_OPENCONNECT` / `AGENT_OPENCONNECT_V3`, except where the code explicitly
|
||||||
|
branches on `user_agent_type`.
|
||||||
|
- `[IMPLICIT]` User-Agent string matching is the *only* signal used to select
|
||||||
|
per-client behavior; there is no protocol-level capability negotiation for these
|
||||||
|
extensions (cross-ref OC-PROTO note on "no real version negotiation").
|
||||||
|
|
||||||
|
**Out of scope**: the generic config-auth XML handshake, CSTP/DTLS framing, and
|
||||||
|
DPD/keepalive/rekey semantics, which are common to all agent types and already
|
||||||
|
covered by `openconnect.md`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
# COMPAT — Client identification
|
||||||
|
|
||||||
|
## AC-CLIENT-COMPAT-001
|
||||||
|
**Requirement**: The worker MUST classify each connecting client into one of the
|
||||||
|
`user_agent_type` values (`AGENT_UNKNOWN`, `AGENT_OPENCONNECT_V3`,
|
||||||
|
`AGENT_OPENCONNECT`, `AGENT_ANYCONNECT`, `AGENT_OPENCONNECT_CLAVISTER`,
|
||||||
|
`AGENT_ANYLINK`, `AGENT_SVC_IPPHONE`) based on a case-insensitive prefix match of
|
||||||
|
the `User-Agent` HTTP header against a fixed, ordered list of strings.
|
||||||
|
- Strength: MUST (this is the sole dispatch mechanism for all AC-CLIENT-* behavior)
|
||||||
|
- Source: `src/worker-http.c` (`HEADER_USER_AGENT` case, ~lines 410-477)
|
||||||
|
- Acceptance: connecting with each of the recognized `User-Agent` strings (e.g.
|
||||||
|
`"Cisco AnyConnect VPN Agent for Apple..."`, `"Cisco AnyConnect..."`,
|
||||||
|
`"AnyConnect-compatible OpenConnect..."`, `"AnyConnect..."`,
|
||||||
|
`"Clavister OneConnect VPN..."`, `"AnyLink Secure Client..."`,
|
||||||
|
`"Cisco SVC IPPhone Client..."`, `"Open AnyConnect VPN Agent v<N>"`,
|
||||||
|
`"OpenConnect VPN Agent..."`) results in the expected `user_agent_type`; an
|
||||||
|
unrecognized string results in `AGENT_UNKNOWN` and is logged at `LOG_DEBUG`
|
||||||
|
without rejecting the connection.
|
||||||
|
- Links: REQ-WORKER-NET-* (worker.md), OC-PROTO-CONN-005 (capability headers)
|
||||||
|
|
||||||
|
## AC-CLIENT-COMPAT-002 `[AMBIGUOUS]`
|
||||||
|
**Requirement**: The order of the `User-Agent` prefix checks SHALL be significant:
|
||||||
|
`"AnyConnect-compatible OpenConnect"` (33 chars) is checked before the shorter
|
||||||
|
`"AnyConnect"` (10 chars) prefix, so OpenConnect builds that self-identify with
|
||||||
|
the compatibility string are classified as `AGENT_OPENCONNECT`, not
|
||||||
|
`AGENT_ANYCONNECT`.
|
||||||
|
- Strength: MUST (de facto — reordering would silently reclassify a whole class of
|
||||||
|
OpenConnect clients as AnyConnect, changing DNS/IPv6 header formatting per
|
||||||
|
AC-CLIENT-CONN-002)
|
||||||
|
- Status: `[UNDOCUMENTED]` — this ordering dependency is not commented in the code
|
||||||
|
and is easy to break with future additions to the table.
|
||||||
|
- Source: `src/worker-http.c` lines 434-459
|
||||||
|
- Acceptance: a `User-Agent` of `"AnyConnect-compatible OpenConnect vX"` MUST yield
|
||||||
|
`AGENT_OPENCONNECT`, not `AGENT_ANYCONNECT`.
|
||||||
|
- `[CANDIDATE for unified.md]`: recommend adding a comment documenting the ordering
|
||||||
|
invariant, and/or a unit test enumerating one representative string per branch
|
||||||
|
with its expected `user_agent_type`, to guard against future reordering.
|
||||||
|
|
||||||
|
## AC-CLIENT-COMPAT-003
|
||||||
|
**Requirement**: Detection of `"Open AnyConnect VPN Agent v<N>"` (the historical
|
||||||
|
OpenConnect self-identification string) MUST further branch on the numeric
|
||||||
|
version `N`: `N <= 3` yields `AGENT_OPENCONNECT_V3` (legacy XML reply format, see
|
||||||
|
AC-CLIENT-CONN-001), `N > 3` yields `AGENT_OPENCONNECT`.
|
||||||
|
- Strength: MUST
|
||||||
|
- Source: `src/worker-http.c` lines 421-433 (`atoi(&req->user_agent[27])`)
|
||||||
|
- Acceptance: `"Open AnyConnect VPN Agent v3"` -> `AGENT_OPENCONNECT_V3`;
|
||||||
|
`"Open AnyConnect VPN Agent v4"` -> `AGENT_OPENCONNECT`.
|
||||||
|
- `[REVIEW]`: `atoi` on attacker-controlled input is bounds-safe here (result only
|
||||||
|
drives an enum branch, no allocation/indexing), but a non-numeric suffix (e.g.
|
||||||
|
`"Open AnyConnect VPN Agent vX"`) yields `atoi() == 0 <= 3`, i.e.
|
||||||
|
`AGENT_OPENCONNECT_V3`. This degrades gracefully (more conservative feature set)
|
||||||
|
and is not flagged as a defect, but is worth a one-line comment.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
# CONN — AnyConnect-specific connection/response shaping
|
||||||
|
|
||||||
|
## AC-CLIENT-CONN-001
|
||||||
|
**Requirement**: For `AGENT_OPENCONNECT_V3` clients, the successful-authentication
|
||||||
|
response body MUST use the legacy XML wrapper (`ocv3_success_msg_head` /
|
||||||
|
`ocv3_success_msg_foot`, i.e. a bare `<auth id="success">...</auth>` without the
|
||||||
|
enclosing `<config-auth>` element), instead of the standard
|
||||||
|
`oc_success_msg_head` / `*_FOOT` wrapper used for all other agent types.
|
||||||
|
- Strength: MUST
|
||||||
|
- Source: `src/worker-auth.c` lines 50-76, 1124-1151 (`post_common_handler`)
|
||||||
|
- Acceptance: a client identifying as `"Open AnyConnect VPN Agent v3"` and
|
||||||
|
successfully authenticating receives a response body beginning with
|
||||||
|
`<auth id="success">` (no `<?xml...?><config-auth...>` wrapper element around
|
||||||
|
it) and ending with `</auth>`.
|
||||||
|
- Links: OC-PROTO-AUTH-008 (cert auth success -> complete flow), `[CANDIDATE for
|
||||||
|
unified.md]`: classify as **EXTENSION** (backward-compatibility shim for a
|
||||||
|
client population predating the `config-auth` XML schema described in
|
||||||
|
draft-openconnect).
|
||||||
|
|
||||||
|
## AC-CLIENT-CONN-002
|
||||||
|
**Requirement**: For DNS server delivery, when `user_agent_type == AGENT_ANYCONNECT`
|
||||||
|
and a DNS entry is IPv6, the worker MUST send `X-CSTP-DNS-IP6: <addr>`; for an IPv4
|
||||||
|
DNS entry (any agent) or for any DNS entry from a non-AnyConnect agent, it MUST
|
||||||
|
send `X-CSTP-DNS: <addr>` (the comment in the source states "openconnect does not
|
||||||
|
require the split of DNS and DNS-IP6 and only recent versions understand the IP6
|
||||||
|
variant").
|
||||||
|
- Strength: MUST
|
||||||
|
- Source: `src/worker-vpn.c` lines 2291-2314
|
||||||
|
- Acceptance: with `full_ipv6` enabled and an IPv6 DNS server configured, an
|
||||||
|
`AGENT_ANYCONNECT` session receives `X-CSTP-DNS-IP6: <addr>`; an
|
||||||
|
`AGENT_OPENCONNECT` session receives `X-CSTP-DNS: <addr>` for the same address.
|
||||||
|
- `[CANDIDATE for unified.md]`: classify as **DIVERGENT** — same logical
|
||||||
|
information (DNS server address), different header name depending on detected
|
||||||
|
client family. Not a security concern but a compatibility-surface item worth
|
||||||
|
tracking if `X-CSTP-DNS-IP6` is ever standardized.
|
||||||
|
|
||||||
|
## AC-CLIENT-CONN-003
|
||||||
|
**Requirement**: IPv6 route/DNS advertisement MUST be gated per `user_agent_type`
|
||||||
|
when `ws->full_ipv6` is set: `AGENT_OPENCONNECT_V3` -> disabled (logged as "agent
|
||||||
|
known not to support them"); `AGENT_OPENCONNECT`, `AGENT_ANYCONNECT`,
|
||||||
|
`AGENT_OPENCONNECT_CLAVISTER`, `AGENT_ANYLINK` -> enabled; `AGENT_UNKNOWN` (and any
|
||||||
|
future/unhandled value via `default`) -> enabled, with a `LOG_NOTICE` warning
|
||||||
|
("Enabling IPv6 routes/DNS although the agent is unknown").
|
||||||
|
- Strength: MUST
|
||||||
|
- Source: `src/worker-vpn.c` lines 2265-2289
|
||||||
|
- Acceptance: with IPv6 enabled server-side, a session whose `User-Agent` matches
|
||||||
|
none of the known strings still receives IPv6 routes/DNS, and the server log
|
||||||
|
contains the `LOG_NOTICE` line.
|
||||||
|
- `[REVIEW]`: the `default:` branch silently enabling IPv6 for unknown agents is a
|
||||||
|
permissive default; flagged for maintainer awareness but not classified as a
|
||||||
|
defect since failing to advertise IPv6 to a capable-but-unrecognized client would
|
||||||
|
be a worse failure mode (broken connectivity vs. unused advertised routes).
|
||||||
|
|
||||||
|
## AC-CLIENT-CONN-004
|
||||||
|
**Requirement**: When `ws->full_ipv6` is set and `req->is_ios` is set (set only
|
||||||
|
when the `User-Agent` matches `"Cisco AnyConnect VPN Agent for Apple..."`, see
|
||||||
|
AC-CLIENT-COMPAT-001) and the session has no configured routes or
|
||||||
|
`ws->default_route == 0`, the worker MUST additionally send
|
||||||
|
`X-CSTP-Split-Include-IP6: 2000::/3`.
|
||||||
|
- Strength: MUST
|
||||||
|
- Source: `src/worker-vpn.c` lines 2351-2357 (comment: "Anyconnect on IOS requires
|
||||||
|
this route in order to use IPv6")
|
||||||
|
- Acceptance: an iOS AnyConnect session (`User-Agent` starting with
|
||||||
|
`"Cisco AnyConnect VPN Agent for Apple"`) with IPv6 enabled and no
|
||||||
|
split-tunnel/default route receives `X-CSTP-Split-Include-IP6: 2000::/3` in the
|
||||||
|
config-auth response headers.
|
||||||
|
- `[CANDIDATE for unified.md]`: classify as **EXTENSION** — a platform-specific
|
||||||
|
(iOS) workaround with no analogue in `openconnect.md`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
# COMPAT — `/svc` endpoint (Cisco IP-Phone clients)
|
||||||
|
|
||||||
|
## AC-CLIENT-COMPAT-010
|
||||||
|
**Requirement**: The `/svc` URL (GET and POST) MUST be registered and reachable
|
||||||
|
regardless of `ANYCONNECT_CLIENT_COMPAT` build configuration (it is the one entry
|
||||||
|
in `known_urls[]` outside the `#ifdef ANYCONNECT_CLIENT_COMPAT` block), but both
|
||||||
|
`get_svc_handler` and `post_svc_handler` MUST reject the request unless
|
||||||
|
**both** of the following hold: (a) `WSRCONFIG(ws)->cisco_svc_client_compat` is
|
||||||
|
true, AND (b) `ws->req.user_agent_type == AGENT_SVC_IPPHONE`.
|
||||||
|
- Strength: MUST
|
||||||
|
- Source: `src/worker-http.c` line 81 (`LL("/svc", get_svc_handler, post_svc_handler)`);
|
||||||
|
`src/worker-svc.c` lines 40-44 and 216-220
|
||||||
|
- Acceptance (negative): a request to `/svc` (GET or POST) when
|
||||||
|
`cisco-svc-client-compat = false` (the documented default per
|
||||||
|
`doc/README-cisco-svc.md`), or when `cisco-svc-client-compat = true` but the
|
||||||
|
`User-Agent` is not `"Cisco SVC IPPhone Client..."`, MUST be rejected (the
|
||||||
|
handler does not proceed to the IP-Phone-specific logic).
|
||||||
|
- `[REVIEW]`: confirm the exact rejection response (404 vs. falling through to
|
||||||
|
default handler behavior) — `get_svc_handler`/`post_svc_handler` early-return,
|
||||||
|
but the precise HTTP status returned to a non-IP-Phone client hitting `/svc`
|
||||||
|
should be verified against `response_404`/`response_401` call sites for a
|
||||||
|
complete negative-test specification.
|
||||||
|
- Links: REQ-WORKER-AUTH-004 (camouflage), `[CANDIDATE for unified.md]`: classify
|
||||||
|
as **EXTENSION** (additional entry point beyond OC-PROTO-CONN-003/004's
|
||||||
|
`/` and `/CSCOSSLC/tunnel`).
|
||||||
|
|
||||||
|
## AC-CLIENT-AUTH-010
|
||||||
|
**Requirement**: `get_svc_handler` (the initial GET to `/svc`) MUST respond with
|
||||||
|
HTTP 200 and a `Set-Cookie: webvpnlogin=1; secure` header, with no body, to signal
|
||||||
|
the IP-Phone client to proceed with a credential POST. It MUST also log a warning
|
||||||
|
(but still proceed) if `cisco_svc_client_compat` is false or the `User-Agent` does
|
||||||
|
not match `AGENT_SVC_IPPHONE` — `[AMBIGUOUS]` this appears to overlap with
|
||||||
|
AC-CLIENT-COMPAT-010's hard gate; the relationship between the warning-and-proceed
|
||||||
|
path and the reject path needs clarification from source re-inspection.
|
||||||
|
- Strength: MUST
|
||||||
|
- Status: `[REVIEW]` — possible duplication/overlap between the gating check (which
|
||||||
|
appears twice, at lines 40-44 and again implicitly) and a separate
|
||||||
|
warn-but-continue check; needs a side-by-side reading of `get_svc_handler` and
|
||||||
|
`post_svc_handler` to resolve which check is authoritative.
|
||||||
|
- Source: `src/worker-svc.c` (`get_svc_handler`)
|
||||||
|
- Acceptance: a `Cisco SVC IPPhone Client` issuing `GET /svc` with
|
||||||
|
`cisco-svc-client-compat = true` receives `HTTP/1.1 200` with header
|
||||||
|
`Set-Cookie: webvpnlogin=1; secure` and an empty body.
|
||||||
|
- Links: AC-CLIENT-COMPAT-010
|
||||||
|
|
||||||
|
## AC-CLIENT-AUTH-011
|
||||||
|
**Requirement**: `post_svc_handler` MUST parse `username` and `password` fields
|
||||||
|
from the POST body (via `parse_reply`), then perform a full
|
||||||
|
`SEC_AUTH_INIT` / `SEC_AUTH_CONT` exchange with sec-mod via `client_auth()`
|
||||||
|
(populating `SecAuthInitMsg`/`SecAuthContMsg` with `hmac`, `remote_ip`,
|
||||||
|
`orig_remote_ip`, `our_ip`, `session_start_time`, `user_agent`, `device_type`,
|
||||||
|
`device_platform`), identical in IPC shape to the worker-driven password
|
||||||
|
authentication path used by `post_auth_handler`.
|
||||||
|
- Strength: MUST
|
||||||
|
- Source: `src/worker-svc.c` (`post_svc_handler`, `client_auth`)
|
||||||
|
- Acceptance: posting valid `username`/`password` form fields to `/svc` as a
|
||||||
|
`Cisco SVC IPPhone Client` results in the same `SEC_AUTH_INIT`/`SEC_AUTH_CONT`
|
||||||
|
IPC traffic to sec-mod as an equivalent password login via `/auth`.
|
||||||
|
- Links: REQ-IPC-015/016 (per internal/ipc.md), REQ-AUTH-AUTH-002,
|
||||||
|
OC-PROTO-AUTH-004 (password auth-request/auth-reply flow)
|
||||||
|
- `[CANDIDATE for unified.md]`: classify as **MAJORITY** for the underlying
|
||||||
|
auth-exchange shape (shared with OC-PROTO-AUTH-004), **EXTENSION** for the
|
||||||
|
`/svc`-specific transport (form POST instead of config-auth XML).
|
||||||
|
|
||||||
|
## AC-CLIENT-AUTH-012
|
||||||
|
**Requirement**: On successful authentication, `post_svc_handler` MUST set
|
||||||
|
`ws->auth_state = S_AUTH_COOKIE` and respond HTTP 200 with
|
||||||
|
`Set-Cookie: webvpn=<base64 cookie>; secure`. On authentication failure, it MUST
|
||||||
|
respond `HTTP/1.1 401 Authentication failed`, then call `cstp_fatal_close` and
|
||||||
|
`exit_worker` — i.e. the worker process MUST terminate on a failed `/svc` login,
|
||||||
|
unlike the `/auth` path which may permit further auth rounds.
|
||||||
|
- Strength: MUST
|
||||||
|
- Source: `src/worker-svc.c` (`post_svc_handler`)
|
||||||
|
- Acceptance (positive): successful `/svc` POST with valid credentials yields
|
||||||
|
`HTTP/1.1 200` + `Set-Cookie: webvpn=<cookie>; secure`, and the worker remains
|
||||||
|
alive in `S_AUTH_COOKIE` state for a subsequent `CONNECT` (or `/svc`
|
||||||
|
cookie-validated request, see AC-CLIENT-AUTH-013).
|
||||||
|
- Acceptance (negative): `/svc` POST with invalid credentials yields
|
||||||
|
`HTTP/1.1 401 Authentication failed`, and the worker process exits — a
|
||||||
|
subsequent request on the same TCP connection MUST fail (connection closed).
|
||||||
|
- Links: OC-PROTO-AUTH-005 (401 on auth failure — note OC-PROTO-AUTH-005 already
|
||||||
|
flags ocserv's camouflage-405 behavior as a DIVERGENT/EXTENSION candidate;
|
||||||
|
`/svc`'s unconditional-401-then-exit is a *third* variant worth folding into
|
||||||
|
that same unified entry), REQ-WORKER-AUTH-004 (camouflage 405 gate — note
|
||||||
|
`cookie_authenticate_or_exit`'s camouflage branch is NOT shown to apply inside
|
||||||
|
`post_svc_handler`'s own 401 path; `[REVIEW]` whether `/svc` login failures are
|
||||||
|
exempt from camouflage masking by design or by oversight).
|
||||||
|
- `[CANDIDATE for unified.md]`: the immediate `exit_worker()` on `/svc` auth
|
||||||
|
failure (vs. allowing retries on `/auth`) should be evaluated as a
|
||||||
|
**negative-test requirement**: confirm no information beyond "401
|
||||||
|
Authentication failed" is leaked, and that the worker exit does not leave
|
||||||
|
sec-mod-side session/ban-score state inconsistent (cross-ref REQ-MAIN-SEC-005,
|
||||||
|
REQ-AUTH-AUTH-* ban accounting).
|
||||||
|
|
||||||
|
## AC-CLIENT-AUTH-013 `[UNDOCUMENTED]`
|
||||||
|
**Requirement**: After a successful `/svc` login (`S_AUTH_COOKIE`), the IP-Phone
|
||||||
|
client is expected to use the returned `webvpn` cookie for subsequent requests —
|
||||||
|
the exact follow-on flow (does the phone then issue `CONNECT /CSCOSSLC/tunnel`
|
||||||
|
like a standard OpenConnect client, or does it use `/svc` again, or a different
|
||||||
|
endpoint?) is not stated in `doc/README-cisco-svc.md` and was not traced in
|
||||||
|
`worker-svc.c` beyond `post_svc_handler`.
|
||||||
|
- Strength: SHOULD (informational gap, not a normative gap in ocserv's own
|
||||||
|
behavior — ocserv's obligations end at issuing the cookie)
|
||||||
|
- Status: `[UNDOCUMENTED]`
|
||||||
|
- Source: `doc/README-cisco-svc.md` (56 lines, does not describe post-login flow)
|
||||||
|
- `[CANDIDATE for unified.md]`: if the IP-Phone reuses `CONNECT /CSCOSSLC/tunnel`
|
||||||
|
with the `webvpn` cookie, AC-CLIENT-AUTH-013 collapses into OC-PROTO-CONN-004
|
||||||
|
(classify **MAJORITY**); if it uses a distinct mechanism, this needs its own
|
||||||
|
unified entry. Recommend tracing an actual IP-Phone packet capture or asking
|
||||||
|
maintainers before reconciliation.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
# COMPAT — TLS cipher / transport requirements for IP-Phones
|
||||||
|
|
||||||
|
## AC-CLIENT-SEC-010
|
||||||
|
**Requirement**: Per `doc/README-cisco-svc.md`, when serving Cisco IP-Phone
|
||||||
|
(7800/8800/8900/9900 Enterprise-firmware) clients, the administrator MUST
|
||||||
|
configure `tls-priorities` to force AES256-CBC or AES128-CBC, because these
|
||||||
|
phones fail with an "old session cipher not returned" error if the negotiated
|
||||||
|
cipher is outside this set.
|
||||||
|
- Strength: MUST (documented operational requirement; not enforced by ocserv code
|
||||||
|
— it is a configuration constraint on the administrator)
|
||||||
|
- Source: `doc/README-cisco-svc.md`
|
||||||
|
- Status: this is a **documentation-only** requirement — no corresponding code
|
||||||
|
enforces or validates it. `[CANDIDATE for unified.md]`: classify as
|
||||||
|
**EXTENSION**, and consider whether ocserv should emit a configuration-time
|
||||||
|
warning when `cisco-svc-client-compat = true` is set without a compatible
|
||||||
|
`tls-priorities` override (a `[CANDIDATE]` for a future config-validation
|
||||||
|
requirement, not in scope for this elicitation pass).
|
||||||
|
|
||||||
|
## AC-CLIENT-SEC-011
|
||||||
|
**Requirement**: Per `doc/README-cisco-svc.md`, DTLS for these IP-Phones requires
|
||||||
|
the server to listen on **port 443** for UDP, regardless of the configured HTTPS
|
||||||
|
TCP port.
|
||||||
|
- Strength: MUST (documented operational requirement)
|
||||||
|
- Source: `doc/README-cisco-svc.md`
|
||||||
|
- Status: documentation-only, as AC-CLIENT-SEC-010.
|
||||||
|
- `[CANDIDATE for unified.md]`: classify as **EXTENSION**; cross-ref
|
||||||
|
REQ-WORKER-NET-003 (DTLS enablement conditions) — confirm whether ocserv's UDP
|
||||||
|
listener port is independently configurable from the TCP port, or whether this
|
||||||
|
is purely an administrator-side firewall/NAT requirement.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
# AUTH — `cisco_client_compat` cookie-authentication relaxation
|
||||||
|
|
||||||
|
## AC-CLIENT-AUTH-020
|
||||||
|
**Requirement**: In `auth_cookie()`, when the selected auth type includes
|
||||||
|
`AUTH_TYPE_CERTIFICATE` AND `WSRCONFIG(ws)->cisco_client_compat == 0` (the
|
||||||
|
default), the worker MUST require `ws->cert_auth_ok != 0` and MUST call
|
||||||
|
`get_cert_info(ws)` before sending `AUTH_COOKIE_REQ` to main, failing the cookie
|
||||||
|
auth (`return -1`) if either check fails. When `cisco_client_compat == 1`, this
|
||||||
|
entire certificate-presence check is **skipped** — `AUTH_COOKIE_REQ` is sent
|
||||||
|
without re-validating `cert_auth_ok`/`get_cert_info`, even if certificate auth was
|
||||||
|
the selected method.
|
||||||
|
- Strength: MUST (both branches are unconditional given the flag's value)
|
||||||
|
- Source: `src/worker-auth.c` lines 1066-1085
|
||||||
|
- Acceptance (negative, default config): with `cisco_client_compat = false`
|
||||||
|
(default) and certificate-based auth selected, a cookie-auth attempt where
|
||||||
|
`ws->cert_auth_ok == 0` MUST fail (`auth_cookie` returns -1, leading to the
|
||||||
|
401/405/503 handling in `cookie_authenticate_or_exit`, REQ-WORKER-AUTH-004).
|
||||||
|
- Acceptance (positive, compat config): with `cisco_client_compat = true` and
|
||||||
|
certificate-based auth selected, a cookie-auth attempt proceeds to
|
||||||
|
`AUTH_COOKIE_REQ` even when `ws->cert_auth_ok == 0` — relying entirely on
|
||||||
|
sec-mod's independent verification (REQ-AUTH-AUTH-005) for the actual
|
||||||
|
security decision.
|
||||||
|
- `[SEC-RISK / REVIEW]`: this flag removes a worker-side defense-in-depth check
|
||||||
|
(cross-ref REQ-AUTH-AUTH-008, which already notes sec-mod independently
|
||||||
|
re-derives cert identity). Because sec-mod's check is authoritative, this is
|
||||||
|
*not* a privilege-boundary violation per se, but it widens the set of requests
|
||||||
|
that reach sec-mod without a worker-side pre-filter. Flag explicitly for
|
||||||
|
maintainer review per AGENTS.md's "Human-judgment required" list (cookie/SID
|
||||||
|
handling changes) if this flag is ever touched.
|
||||||
|
- Links: REQ-AUTH-AUTH-007, REQ-AUTH-AUTH-008, REQ-AUTH-AUTH-005,
|
||||||
|
`[CANDIDATE for unified.md]`: classify as **DIVERGENT** (config-gated relaxation
|
||||||
|
of a check present in the default/MAJORITY path).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
# EXT — `ANYCONNECT_CLIENT_COMPAT` build-time extensions
|
||||||
|
|
||||||
|
## AC-CLIENT-EXT-030
|
||||||
|
**Requirement**: When built `WITH ANYCONNECT_CLIENT_COMPAT`, the `known_urls[]`
|
||||||
|
table MUST additionally register: `/profiles/*` (directory handler ->
|
||||||
|
`get_config_handler`), `/VPNManifest.xml`, `/1/index.html`, `/1/Linux`,
|
||||||
|
`/1/Linux_64`, `/1/Windows`, `/1/Windows_ARM64`, `/1/Darwin_i386`,
|
||||||
|
`/1/binaries/vpndownloader.sh`, `/1/VPNManifest.xml`,
|
||||||
|
`/1/binaries/update.txt`, `/+CSCOT+/translation-table`,
|
||||||
|
`/+CSCOT+/oem-customization`, and `/logout`.
|
||||||
|
- Strength: MUST (build-conditional, but unconditional within that build)
|
||||||
|
- Source: `src/worker-http.c` lines 64-80
|
||||||
|
- Acceptance: a build with `ANYCONNECT_CLIENT_COMPAT` defined serves a non-404
|
||||||
|
response for `GET /profiles/<file>` (subject to AC-CLIENT-EXT-031) and for each
|
||||||
|
literal path above; a build without it returns 404 (the default
|
||||||
|
GET-URL-not-found handling) for all of these paths.
|
||||||
|
- `[CANDIDATE for unified.md]`: classify as **EXTENSION**. Several of these paths
|
||||||
|
(`/1/binaries/vpndownloader.sh`, `/1/Linux*`, `/1/Windows*`, `/1/Darwin_i386`)
|
||||||
|
appear to mimic Cisco ASA's webvpn client-download portal; `[UNDOCUMENTED]`
|
||||||
|
whether ocserv actually serves meaningful content for these vs. stub/empty
|
||||||
|
responses (`get_empty_handler` for most, `get_dl_handler`/`get_string_handler`
|
||||||
|
for a few) — worth a one-line note in `doc/README-cisco-svc.md` or
|
||||||
|
`ocserv.8.md` if administrators are expected to populate any of these.
|
||||||
|
|
||||||
|
## AC-CLIENT-EXT-031
|
||||||
|
**Requirement**: `get_config_handler` (registered for `/profiles/*` under
|
||||||
|
`ANYCONNECT_CLIENT_COMPAT`) MUST: (1) require a valid session cookie via
|
||||||
|
`cookie_authenticate_or_exit`; (2) respond 404 if
|
||||||
|
`ws->user_config->xml_config_file` is unset, or if `stat()` on that path fails;
|
||||||
|
(3) otherwise stream the file with `Content-Type: text/xml` and
|
||||||
|
`Content-Length` set to the file's size via `cstp_send_file`.
|
||||||
|
- Strength: MUST
|
||||||
|
- Source: `src/worker-http-handlers.c` lines 94-130
|
||||||
|
- Acceptance (positive): with `xml_config_file` configured for the authenticated
|
||||||
|
user and the file present, `GET /profiles/<anything>` (cookie-authenticated)
|
||||||
|
returns `200` with `Content-Type: text/xml` and the file's bytes.
|
||||||
|
- Acceptance (negative): without a valid cookie, `cookie_authenticate_or_exit`
|
||||||
|
enforces the same 401/405/503 rules as REQ-WORKER-AUTH-004; with a valid cookie
|
||||||
|
but no `xml_config_file` configured, or a configured-but-missing file, the
|
||||||
|
response is 404.
|
||||||
|
- `[REVIEW]`: `ws->req.url` (the requested sub-path under `/profiles/`) does not
|
||||||
|
appear to be used to select *which* file to serve — `get_config_handler` always
|
||||||
|
serves `ws->user_config->xml_config_file` regardless of the URL suffix. This
|
||||||
|
means any path under `/profiles/` (e.g. `/profiles/../../etc/passwd`) maps to
|
||||||
|
the same single configured file — i.e. **the URL suffix is not used for file
|
||||||
|
selection, so path traversal via the URL is not applicable here**. Confirmed by
|
||||||
|
reading `get_config_handler`'s body (no `ws->req.url` reference after the
|
||||||
|
initial debug log). Recorded as a negative-requirement *confirmation*, not a
|
||||||
|
gap: `[CANDIDATE for unified.md negative requirements]` — "the `/profiles/<x>`
|
||||||
|
URL suffix MUST NOT influence which file is served."
|
||||||
|
- Links: REQ-WORKER-AUTH-004, AC-CLIENT-AUTH-021 (webvpnc cookie below, which
|
||||||
|
advertises the `/profiles/%s` URI to the client)
|
||||||
|
|
||||||
|
## AC-CLIENT-AUTH-021
|
||||||
|
**Requirement**: When built `WITH ANYCONNECT_CLIENT_COMPAT` and
|
||||||
|
`WSRCONFIG(ws)->xml_config_file` is set, the successful-authentication response
|
||||||
|
(`post_common_handler`) MUST:
|
||||||
|
(a) append an `OC_SUCCESS_MSG_FOOT_PROFILE` XML fragment (a
|
||||||
|
`<config type="private"><vpn-profile-manifest><vpn rev="1.0"><file
|
||||||
|
type="profile" service-type="user"><uri>/profiles/%s</uri><hash
|
||||||
|
type="sha1">%s</hash></file></vpn></vpn-profile-manifest></config>`,
|
||||||
|
with `%s` = `xml_config_file` name and `xml_config_hash`) instead of the
|
||||||
|
plain `OC_SUCCESS_MSG_FOOT`; and
|
||||||
|
(b) set an additional `Set-Cookie: webvpnc=bu:/&p:t&iu:1/&sh:<cert_hash>&...
|
||||||
|
&fu:profiles%2F<xml_config_file>&fh:<xml_config_hash>; path=/; Secure;
|
||||||
|
HttpOnly` header (or, if `xml_config_file` is unset, the shorter
|
||||||
|
`webvpnc=bu:/&p:t&iu:1/&sh:<cert_hash>; path=/; Secure; HttpOnly` form).
|
||||||
|
Both forms are sent in addition to, and after, the unconditional
|
||||||
|
`Set-Cookie: webvpn=<cookie>; Secure; HttpOnly` and the `webvpnc=` *clearing*
|
||||||
|
cookie (`expires=Thu, 01 Jan 1970...`) that precede them.
|
||||||
|
- Strength: MUST (build-conditional)
|
||||||
|
- Source: `src/worker-auth.c` lines 1132-1151 (XML foot), 1219-1249 (cookies)
|
||||||
|
- Acceptance: under `ANYCONNECT_CLIENT_COMPAT` with `xml_config_file` configured,
|
||||||
|
a successful auth response contains, in order: a `webvpn=` cookie, a clearing
|
||||||
|
`webvpnc=; expires=...1970...` cookie, OWASP headers, then a non-expiring
|
||||||
|
`webvpnc=bu:/&p:t&iu:1/&sh:<hash>&...&fu:profiles%2F<file>&fh:<hash>` cookie, then
|
||||||
|
the XML body with the `<vpn-profile-manifest>` fragment referencing
|
||||||
|
`/profiles/<xml_config_file>`.
|
||||||
|
- `[CANDIDATE for unified.md]`: classify as **EXTENSION** (config-profile delivery
|
||||||
|
has no analogue in `openconnect.md`). `[REVIEW]`: the two `webvpnc=` `Set-Cookie`
|
||||||
|
headers (one clearing, one setting) sent in the same response is unusual;
|
||||||
|
confirm this ordering is intentional (clear-then-reset pattern, perhaps for
|
||||||
|
clients that cache the first `Set-Cookie` for a given name) rather than a
|
||||||
|
leftover from incremental development.
|
||||||
|
- Links: AC-CLIENT-EXT-031 (the `/profiles/<file>` URI this cookie/XML advertises)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
# Completeness notes
|
||||||
|
|
||||||
|
- **`src/worker-svc.c` `client_auth()` internals** (HMAC construction, exact field
|
||||||
|
population of `SecAuthInitMsg`/`SecAuthContMsg`): covered at the level of "same
|
||||||
|
IPC shape as `/auth`" (AC-CLIENT-AUTH-011); a field-by-field diff against
|
||||||
|
`post_auth_handler`'s equivalent call was not performed in this pass.
|
||||||
|
`[CANDIDATE]` for a follow-up pass if sec-mod-side AC-CLIENT requirements are
|
||||||
|
ever needed (currently sec-mod.md treats all `SEC_AUTH_INIT`/`CONT` sources
|
||||||
|
uniformly).
|
||||||
|
- **`get_dl_handler` / `get_string_handler` / `get_empty_handler`** content:
|
||||||
|
not inspected; assumed to serve static/stub content per AC-CLIENT-EXT-030's
|
||||||
|
`[UNDOCUMENTED]` note.
|
||||||
|
- **DTLS cipher negotiation table (`ciphersuites[]` in `worker-http.c`)**: this
|
||||||
|
table implements the `X-DTLS-CipherSuite` / `X-DTLS12-CipherSuite` HTTP-header
|
||||||
|
based DTLS cipher negotiation referenced generically in `openconnect.md`
|
||||||
|
(OC-PROTO-CTRL-* rekey/DTLS area) — it is **not** AnyConnect-specific (both
|
||||||
|
OpenConnect and AnyConnect clients use this header-based negotiation), so it is
|
||||||
|
intentionally excluded from this document and left to `openconnect.md` /
|
||||||
|
`unified.md` to cover under CTRL or CONN.
|
||||||
|
- **`AGENT_OPENCONNECT_CLAVISTER` and `AGENT_ANYLINK`**: recognized for
|
||||||
|
classification (AC-CLIENT-COMPAT-001) and IPv6 enablement
|
||||||
|
(AC-CLIENT-CONN-003), but no further Clavister- or AnyLink-specific branches
|
||||||
|
were found in the files read for this pass. `[UNDOCUMENTED]` whether these
|
||||||
|
agent types have any other special-cased behavior elsewhere in the worker
|
||||||
|
(not searched exhaustively).
|
||||||
@@ -0,0 +1,475 @@
|
|||||||
|
---
|
||||||
|
title: OpenConnect protocol (draft) requirement extraction
|
||||||
|
generator: requirements-elicitation
|
||||||
|
process: n/a
|
||||||
|
id-prefix: OC-PROTO
|
||||||
|
sources:
|
||||||
|
- ~/projects/openconnect/protocol/draft-openconnect.xml (draft-mavrogiannopoulos-openconnect-04/05, "The OpenConnect VPN Protocol Version 1.2", 2023)
|
||||||
|
---
|
||||||
|
|
||||||
|
# OpenConnect Protocol — Extracted Requirements
|
||||||
|
|
||||||
|
This is a **working extraction**, not a normative spec for ocserv. It applies
|
||||||
|
`requirements-elicitation.md` to the OpenConnect protocol draft
|
||||||
|
(`draft-mavrogiannopoulos-openconnect-04`/`-05`), which describes the
|
||||||
|
**generic, client-agnostic** wire protocol that both the `openconnect` client
|
||||||
|
and `ocserv` implement, and which AnyConnect is "believed to be compatible
|
||||||
|
with."
|
||||||
|
|
||||||
|
IDs here are `OC-PROTO-<CAT>-<NNN>` using the `protocol/unified.md` category
|
||||||
|
tags (`CONN`, `AUTH`, `DATA`, `CTRL`, `SEC`, `EXT`, `COMPAT`) so that
|
||||||
|
`requirements-reconciliation.md` can align them directly against
|
||||||
|
`AC-CLIENT-*` (observed AnyConnect behavior) and `OCSERV` (current
|
||||||
|
`internal/worker.md` + code) entries. These IDs are **not** cited from
|
||||||
|
`internal/*` — only `protocol/unified.md` may reference them.
|
||||||
|
|
||||||
|
Per Phase 1 of the elicitation protocol: **in scope** is the wire protocol
|
||||||
|
(TLS/HTTP/CONNECT handshake sequence, CSTP/DTLS framing, rekey/DPD/keepalive,
|
||||||
|
compression). **Out of scope**: the specific XML schema validation rules
|
||||||
|
beyond the DTD given, and any ocserv-side IPC — those belong to `internal/*`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## CONN — Tunnel establishment
|
||||||
|
|
||||||
|
### OC-PROTO-CONN-001
|
||||||
|
**Requirement:** The client SHOULD negotiate TLS 1.2 or later for the initial
|
||||||
|
HTTPS connection to the server's well-known port (conventionally 443).
|
||||||
|
**Strength:** SHOULD
|
||||||
|
**Source:** §"VPN tunnel establishment" (tunnel-establishment)
|
||||||
|
**Notes:** "SHOULD" not "MUST" — a server MAY be configured to accept earlier
|
||||||
|
TLS versions for legacy clients. `[CANDIDATE for unified.md: ocserv's
|
||||||
|
`min-tls-version` / `default-priorities` config interacts with this — check
|
||||||
|
default `priorities` string for the effective floor.]`
|
||||||
|
|
||||||
|
### OC-PROTO-CONN-002
|
||||||
|
**Requirement:** The client SHOULD include the Server Name Indication (SNI,
|
||||||
|
RFC 6066) extension in its initial TLS ClientHello, carrying the DNS name of
|
||||||
|
the server it intends to reach.
|
||||||
|
**Strength:** SHOULD
|
||||||
|
**Source:** §"Tunnel initiation" (tunnel-initiation)
|
||||||
|
**Notes:** Maps directly to `internal/worker.md` REQ-WORKER-NET-001
|
||||||
|
(`hello_hook_func` vhost selection by SNI). `[CANDIDATE: unified.md should
|
||||||
|
note ocserv's behavior for a client that omits SNI entirely — falls back to
|
||||||
|
default vhost, per REQ-WORKER-NET-001.]`
|
||||||
|
|
||||||
|
### OC-PROTO-CONN-003
|
||||||
|
**Requirement:** After the TLS session is established, the client MUST send
|
||||||
|
an HTTP POST to `/` with a `config-auth` XML body of `type="init"` and
|
||||||
|
`Content-Type: text/xml`, regardless of which authentication method(s) it
|
||||||
|
supports.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Source:** §"Tunnel initiation" (tunnel-initiation), example XML
|
||||||
|
**Notes:** This is the protocol's single entry point — all authentication
|
||||||
|
flows (password, certificate, SPNEGO) begin from the server's response to
|
||||||
|
this POST.
|
||||||
|
|
||||||
|
### OC-PROTO-CONN-004
|
||||||
|
**Requirement:** Upon receipt of a `config-auth` reply of `type="complete"`
|
||||||
|
(with `<auth id="success">`), the client MUST issue an HTTP `CONNECT
|
||||||
|
/CSCOSSLC/tunnel HTTP/1.1` request over the same TLS connection to initiate
|
||||||
|
the VPN tunnel.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Source:** §"Tunnel and channels establishment" (params-exchange)
|
||||||
|
**Notes:** The literal path `/CSCOSSLC/tunnel` is a fixed, Cisco-derived
|
||||||
|
string in the *generic* protocol — not an AnyConnect-only extension. Maps to
|
||||||
|
`internal/worker.md` REQ-WORKER-NET-002 (`connect_handler` accepts only this
|
||||||
|
path, plus a documented Clavister-client variant).
|
||||||
|
|
||||||
|
### OC-PROTO-CONN-005
|
||||||
|
**Requirement:** The CONNECT request MUST advertise client capabilities via
|
||||||
|
HTTP headers: `X-CSTP-Address-Type` (comma-separated `IPv4`/`IPv6`/both),
|
||||||
|
`X-CSTP-Base-MTU`, and optionally `X-CSTP-Accept-Encoding` and `User-Agent`.
|
||||||
|
**Strength:** MUST (Address-Type, Base-MTU) / MAY (Accept-Encoding,
|
||||||
|
User-Agent)
|
||||||
|
**Source:** §"Client capabilities" (capabilities)
|
||||||
|
|
||||||
|
### OC-PROTO-CONN-006
|
||||||
|
**Requirement:** After a successful CONNECT, the server's response MUST be
|
||||||
|
the last HTTP message on this connection — the TCP connection then becomes
|
||||||
|
the CSTP channel, transporting framed IP packets (§"primary-channel-protocol")
|
||||||
|
for the remainder of the session.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Source:** §"The primary CSTP channel - TCP" (primary-channel)
|
||||||
|
**Notes:** **[Missing conditional branch — Phase 3]** The draft does not
|
||||||
|
specify what a client should do if the server sends *additional* HTTP
|
||||||
|
headers/data after the CONNECT response but before the first CSTP frame, nor
|
||||||
|
what a server should do if it receives non-CSTP-framed bytes immediately
|
||||||
|
after sending its CONNECT response. `[CANDIDATE: unified.md should record
|
||||||
|
ocserv's actual behavior here as `OCSERV`-side, likely DIVERGENT or
|
||||||
|
UNIVERSAL-by-silence.]`
|
||||||
|
|
||||||
|
### OC-PROTO-CONN-007
|
||||||
|
**Requirement:** The server's CONNECT response configuration headers
|
||||||
|
(`X-CSTP-Address`, `X-CSTP-Netmask`, `X-CSTP-Address-IP6`, `X-CSTP-DNS`,
|
||||||
|
`X-CSTP-Default-Domain`, `X-CSTP-Split-DNS`, `X-CSTP-Split-Include`,
|
||||||
|
`X-CSTP-Split-Exclude`, `X-CSTP-Base-MTU`, `X-CSTP-DynDNS`,
|
||||||
|
`X-CSTP-Content-Encoding`, `X-DTLS-Content-Encoding`) constitute the client's
|
||||||
|
tunnel networking configuration; absence of any `X-CSTP-Split-Include` header
|
||||||
|
means the client MUST route its default route through the VPN.
|
||||||
|
**Strength:** MUST (default-route inference) / the headers themselves are
|
||||||
|
each individually optional depending on what's been requested/negotiated
|
||||||
|
**Source:** §"Server response and tunnel configuration" (server-response)
|
||||||
|
**Notes:** "X-CSTP-Address-IP6 ... prefix length is RECOMMENDED to be set to
|
||||||
|
127-bits" (RFC 6164) and "X-CSTP-Netmask ... RECOMMENDED the server address to
|
||||||
|
be the first in defined network" are both SHOULD-strength conventions, not
|
||||||
|
MUST — `[CANDIDATE: check whether ocserv's `ip-lease.c` (REQ-MAIN-NET-001)
|
||||||
|
follows the /127 and "server address first" recommendations for IPv6/IPv4
|
||||||
|
respectively.]`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## AUTH — Authentication
|
||||||
|
|
||||||
|
### OC-PROTO-AUTH-001
|
||||||
|
**Requirement:** The server MUST always be authenticated to the client via
|
||||||
|
its X.509 certificate during the TLS handshake; the server's identity SHOULD
|
||||||
|
be carried in the certificate's `SubjectAlternativeName` (type `dNSName`).
|
||||||
|
**Strength:** MUST (server cert auth) / SHOULD (SAN dNSName placement)
|
||||||
|
**Source:** §"Server authentication" (server-authentication)
|
||||||
|
|
||||||
|
### OC-PROTO-AUTH-002
|
||||||
|
**Requirement:** The protocol allows client authentication via password,
|
||||||
|
X.509 client certificate, or HTTP SPNEGO (GSSAPI/Kerberos) — singly or in
|
||||||
|
combination ("or combinations of them").
|
||||||
|
**Strength:** MAY (the protocol permits all three; deployment-specific which
|
||||||
|
are enabled)
|
||||||
|
**Source:** §"Client authentication" (client-authentication)
|
||||||
|
**Notes:** **[Open-ended enumeration — Phase 3, resolved]** The enumeration
|
||||||
|
is closed: exactly {password, certificate, SPNEGO} and combinations thereof.
|
||||||
|
Any ocserv auth backend (PAM, RADIUS, OIDC, plain, GSSAPI) is an
|
||||||
|
*implementation* of "password" or "SPNEGO" from the protocol's perspective —
|
||||||
|
`internal/sec-mod.md`'s `auth_mod_st` vtable is the OCSERV-side
|
||||||
|
implementation point, not a protocol extension. `[CANDIDATE: unified.md
|
||||||
|
should classify ocserv's OIDC auth as EXTENSION if it doesn't map cleanly
|
||||||
|
onto "password" — it likely uses a browser redirect flow not described
|
||||||
|
here.]`
|
||||||
|
|
||||||
|
### OC-PROTO-AUTH-003
|
||||||
|
**Requirement:** It is RECOMMENDED that clients complete authentication
|
||||||
|
within a single TLS session and rely on TLS session resumption for
|
||||||
|
reconnection, because splitting username/password across separate TLS/HTTP
|
||||||
|
connections (a legacy pattern some clients use) prevents the server from
|
||||||
|
binding the TLS channel to the VPN session (RFC 5056 channel binding).
|
||||||
|
**Strength:** SHOULD NOT (split-connection auth) / SHOULD (single-session +
|
||||||
|
resumption)
|
||||||
|
**Source:** §"Client authentication" (client-authentication), penultimate
|
||||||
|
paragraph
|
||||||
|
**Notes:** This directly motivates `internal/ipc.md` REQ-IPC-010
|
||||||
|
(`sec_auth_init_hmac` anti-replay) and `internal/worker.md`
|
||||||
|
REQ-WORKER-SEC-003 (DTLS-PSK derived from the CSTP TLS session) — ocserv's
|
||||||
|
design assumes the RECOMMENDED single-session model and adds its own binding
|
||||||
|
(HMAC over `remote_ip`/`our_ip`/`session_start_time`) as defense for clients
|
||||||
|
that still split connections. `[CANDIDATE: unified.md SEC entry — does
|
||||||
|
ocserv's HMAC scheme provide equivalent channel binding to RFC 5056 for the
|
||||||
|
legacy split-connection case, or only mitigate replay?]`
|
||||||
|
|
||||||
|
### OC-PROTO-AUTH-004
|
||||||
|
**Requirement:** During password authentication, the server presents one or
|
||||||
|
more `config-auth` `type="auth-request"` forms (each potentially requesting
|
||||||
|
multiple fields, e.g. username+password, or a second-factor token in a
|
||||||
|
follow-up form); the client responds via HTTP POST to the form's `action`
|
||||||
|
URL with a `type="auth-reply"` body. The server MAY repeat this exchange
|
||||||
|
(HTTP 200 + new `auth-request`) an arbitrary number of times before either
|
||||||
|
`type="complete"` (success) or HTTP 401 (failure).
|
||||||
|
**Strength:** MUST (the request/reply/complete shapes) / MAY (number of
|
||||||
|
rounds)
|
||||||
|
**Source:** §"Tunnel authentication using passwords"
|
||||||
|
(authentication_pass)
|
||||||
|
**Notes:** Maps to `internal/ipc.md` REQ-IPC-015/016 (`SEC_AUTH_CONT`
|
||||||
|
multi-round) and `internal/authentication.md` REQ-AUTH-AUTH-002 (`auth_msg`
|
||||||
|
always called when continuing).
|
||||||
|
|
||||||
|
### OC-PROTO-AUTH-005
|
||||||
|
**Requirement:** If client authentication fails (password or SPNEGO), the
|
||||||
|
server MUST respond with HTTP 401 Unauthorized.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Source:** §"Tunnel authentication using passwords" (authentication_pass),
|
||||||
|
§"Tunnel authentication using SPNEGO" (authentication_gssapi)
|
||||||
|
**Notes:** **[Missing negative requirement — Phase 3, resolved by spec
|
||||||
|
itself]** This *is* the negative requirement; pairs with OC-PROTO-AUTH-004's
|
||||||
|
success case. `[CANDIDATE: unified.md should check whether ocserv ever
|
||||||
|
returns a non-401 status (e.g. 403, 503) on auth failure for any auth
|
||||||
|
backend — `internal/worker.md` REQ-WORKER-AUTH-004 shows camouflage mode
|
||||||
|
deliberately returns 405 instead, which would be a documented DIVERGENT/
|
||||||
|
EXTENSION case.]`
|
||||||
|
|
||||||
|
### OC-PROTO-AUTH-006
|
||||||
|
**Requirement:** It is RECOMMENDED that clients pad authentication XML
|
||||||
|
bodies containing username/password to a multiple of 64 bytes (via an
|
||||||
|
`X-Pad` HTTP header with arbitrary printable data) to reduce the information
|
||||||
|
a passive eavesdropper gains from observing encrypted message lengths.
|
||||||
|
**Strength:** SHOULD
|
||||||
|
**Source:** §"Tunnel authentication using passwords" (authentication_pass),
|
||||||
|
final paragraph
|
||||||
|
**Notes:** Client-side mitigation; no corresponding server requirement is
|
||||||
|
stated. `[CANDIDATE: unified.md — does ocserv's worker do anything with an
|
||||||
|
`X-Pad` header if present (e.g. ignore it safely), or could an oversized
|
||||||
|
`X-Pad` header interact badly with HTTP header size limits
|
||||||
|
(`internal/worker.md` "Completeness notes" re: `worker-http.c`)?]`
|
||||||
|
|
||||||
|
### OC-PROTO-AUTH-007
|
||||||
|
**Requirement:** For certificate authentication, the TLS server MUST request
|
||||||
|
a client certificate during the handshake; because under TLS 1.2 client
|
||||||
|
certificates are sent unencrypted, certificates used with this protocol
|
||||||
|
SHOULD NOT contain identifying information beyond a username or pseudonymous
|
||||||
|
identifier, RECOMMENDED to be placed in the certificate DN's `UID` attribute
|
||||||
|
(OID 0.9.2342.19200300.100.1.1).
|
||||||
|
**Strength:** MUST (request cert) / SHOULD NOT (extraneous identifying info
|
||||||
|
in cert) / RECOMMENDED (UID OID placement)
|
||||||
|
**Source:** §"Tunnel authentication using certificates" (authentication_cert)
|
||||||
|
**Notes:** Maps to `internal/authentication.md` REQ-AUTH-AUTH-008/009
|
||||||
|
(`get_cert_username`, `cert-user-oid` config) and
|
||||||
|
REQ-AUTH-AUTH-005 (defense-in-depth re-verification).
|
||||||
|
`cert-user-oid` is configurable in ocserv (not hardcoded to the RFC 4519 UID
|
||||||
|
OID) — `[CANDIDATE: unified.md EXTENSION — ocserv generalizes "RECOMMENDED
|
||||||
|
UID OID" into a configurable OID, including the special value
|
||||||
|
`SAN(rfc822name)` not mentioned in the draft at all.]`
|
||||||
|
|
||||||
|
### OC-PROTO-AUTH-008
|
||||||
|
**Requirement:** On successful certificate validation (after the `init`
|
||||||
|
POST), the server replies HTTP 200 with `config-auth type="complete"`,
|
||||||
|
identical in shape to the password-success case; the client then proceeds
|
||||||
|
directly to CONNECT (OC-PROTO-CONN-004) — there is no certificate-specific
|
||||||
|
`auth-request`/`auth-reply` round trip.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Source:** §"Tunnel authentication using certificates" (authentication_cert)
|
||||||
|
|
||||||
|
### OC-PROTO-AUTH-009
|
||||||
|
**Requirement:** A client supporting HTTP SPNEGO (RFC 4559 / GSSAPI, RFC
|
||||||
|
2743) MUST signal this in its `init` POST via the header
|
||||||
|
`X-Support-HTTP-Auth: true`. The server then responds HTTP 401 and SPNEGO
|
||||||
|
negotiation proceeds per RFC 4559. The server MAY additionally send
|
||||||
|
`X-Support-HTTP-Auth: fallback` to indicate alternative methods (e.g.
|
||||||
|
password) are available if SPNEGO fails — a client receiving this header
|
||||||
|
after a SPNEGO failure SHOULD retry without `X-Support-HTTP-Auth: true`.
|
||||||
|
**Strength:** MUST (signaling header) / MAY (fallback header) / SHOULD
|
||||||
|
(client retry behavior)
|
||||||
|
**Source:** §"Tunnel authentication using SPNEGO" (authentication_gssapi)
|
||||||
|
**Notes:** **[Missing conditional branch — Phase 3, resolved]** The
|
||||||
|
"fallback" branch is explicitly the else-case for "SPNEGO advertised but
|
||||||
|
failed." Maps to `internal/sec-mod.md` GSSAPI auth module (out of detailed
|
||||||
|
scope per that doc's completeness notes) — `[CANDIDATE: unified.md COMPAT —
|
||||||
|
does ocserv's GSSAPI module emit `X-Support-HTTP-Auth: fallback` correctly
|
||||||
|
when GSSAPI auth fails and another module is configured?]`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## DATA — CSTP and DTLS channel framing
|
||||||
|
|
||||||
|
### OC-PROTO-DATA-001
|
||||||
|
**Requirement:** Every CSTP channel packet MUST begin with an 8-byte header:
|
||||||
|
bytes 0-3 fixed to `0x53 0x54 0x46 0x01` ("STF" + 0x01), bytes 4-5 the
|
||||||
|
big-endian length of the following payload, byte 6 the payload type (see
|
||||||
|
OC-PROTO-DATA-002), byte 7 fixed `0x00`. The entire 8-byte-header-plus-payload
|
||||||
|
unit MUST be carried inside a single TLS record.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Source:** §"The CSTP Channel Protocol" (primary-channel-protocol), table
|
||||||
|
`cstp_table`
|
||||||
|
**Notes:** **[Vague/undefined term — Phase 3]** "fixed to 0x53/0x54/0x46/0x01"
|
||||||
|
— a receiver encountering a different magic should presumably treat the
|
||||||
|
connection as protocol-violating, but the draft doesn't say so explicitly.
|
||||||
|
`[CANDIDATE: unified.md negative requirement — OCSERV's `worker-vpn.c`
|
||||||
|
`parse_cstp_data` behavior on a bad magic: does it terminate the connection,
|
||||||
|
log, or silently ignore?]`
|
||||||
|
|
||||||
|
### OC-PROTO-DATA-002
|
||||||
|
**Requirement:** The CSTP/DTLS payload type byte MUST be one of: `0x00`
|
||||||
|
DATA (IPv4/IPv6 packet), `0x03` DPD-REQ, `0x04` DPD-RESP, `0x05` DISCONNECT
|
||||||
|
(followed by 1 reason byte), `0x07` KEEPALIVE (no payload), `0x08` COMPRESSED
|
||||||
|
DATA, `0x09` TERMINATE (server→client only, no payload). A receiver MUST be
|
||||||
|
able to process both `0x00` and `0x08` regardless of whether compression was
|
||||||
|
negotiated for *outbound* data (OC-PROTO-EXT-002).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Source:** §"The CSTP Channel Protocol" (primary-channel-protocol), table
|
||||||
|
`packet_table`
|
||||||
|
**Notes:** **[Open-ended enumeration — Phase 3, resolved]** The set of
|
||||||
|
payload types is closed (7 values). **[Missing negative requirement —
|
||||||
|
Phase 3]** No explicit statement of receiver behavior for an *unrecognized*
|
||||||
|
payload type byte (e.g. `0x01`, `0x02`, `0x06`, or `>=0x0a`).
|
||||||
|
`[CANDIDATE: unified.md — record OCSERV's actual behavior for unknown CSTP
|
||||||
|
type bytes as either DIVERGENT (if it errors/disconnects) or an
|
||||||
|
UNDOCUMENTED gap if it's silently ignored; this is security-relevant as a
|
||||||
|
potential DoS or parser-confusion vector.]`
|
||||||
|
|
||||||
|
### OC-PROTO-DATA-003
|
||||||
|
**Requirement:** DISCONNECT packets (type `0x05`) MUST carry exactly one
|
||||||
|
reason byte: `0x70` LOCAL ERROR (session invalidated), `0x91` VPN RECONNECT
|
||||||
|
(session preserved, client intends immediate reconnect), `0xb0` USER
|
||||||
|
DISCONNECT (session invalidated), `0xd1` VPN PAUSE (session preserved).
|
||||||
|
Reason codes not in this set MUST be treated by the receiver as equivalent
|
||||||
|
to `0x91` (temporary disconnect, session preserved).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Source:** §"The CSTP Channel Protocol" (primary-channel-protocol), table
|
||||||
|
`disconnect_reason_table`
|
||||||
|
**Notes:** Directly maps to `internal/ipc.md` REQ-IPC-033
|
||||||
|
(`server_disconnected` → `REASON_SERVER_DISCONNECT`) and
|
||||||
|
`internal/sec-mod.md` REQ-SECMOD-SESSION-003 (persistent-cookies +
|
||||||
|
discon_reason). `REASON_USER_DISCONNECT` in ocserv corresponds to `0xb0`;
|
||||||
|
`[CANDIDATE: unified.md — verify ocserv's internal `discon_reason` enum
|
||||||
|
(referenced in REQ-SECMOD-SESSION-003) maps 1:1 onto these 4 wire values plus
|
||||||
|
the "unknown→0x91" fallback rule, including the "session should be
|
||||||
|
invalidated" vs "preserved" distinction matching
|
||||||
|
`IS_CLIENT_ENTRY_EXPIRED`/`expire_client_entry` logic.]`
|
||||||
|
|
||||||
|
### OC-PROTO-DATA-004
|
||||||
|
**Requirement:** The DTLS channel packet format consists of a 1-byte header
|
||||||
|
(same type values as OC-PROTO-DATA-002, from `packet_table`) followed by
|
||||||
|
data, the whole encapsulated in a single DTLS record (RFC 6347).
|
||||||
|
**Strength:** MUST
|
||||||
|
**Source:** §"The DTLS Channel Protocol" (secondary-channel-protocol)
|
||||||
|
**Notes:** **[Unanchored comparative / underspecified — Phase 3]** The DTLS
|
||||||
|
header is 1 byte vs. CSTP's 8 bytes — the draft doesn't restate which of the
|
||||||
|
7 payload types are meaningful over DTLS (e.g. is DISCONNECT/TERMINATE valid
|
||||||
|
over DTLS, or only DATA/DPD/KEEPALIVE since DTLS is the "secondary,
|
||||||
|
optional" channel and CSTP remains the control channel?). `[CANDIDATE:
|
||||||
|
unified.md AMBIGUOUS — needs OCSERV source inspection
|
||||||
|
(`parse_dtls_data` in worker-vpn.c) to determine which type bytes the
|
||||||
|
implementation actually accepts on the DTLS channel.]`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## CTRL — Rekey, DPD, keepalive
|
||||||
|
|
||||||
|
### OC-PROTO-CTRL-001
|
||||||
|
**Requirement:** The server advertises the rekey method for each channel via
|
||||||
|
`X-CSTP-Rekey-Method`/`X-DTLS-Rekey-Method`, one of: `none` (no rekey until
|
||||||
|
2^48 DTLS records / 2^64 TLS records), `ssl` (periodic TLS/DTLS
|
||||||
|
rehandshake-or-rekey), or `new-tunnel` (periodic full session teardown +
|
||||||
|
client reconnect). When not `none`, `X-CSTP-Rekey-Time`/`X-DTLS-Rekey-Time`
|
||||||
|
(seconds) gives the period.
|
||||||
|
**Strength:** MUST (header semantics) / the chosen method is server policy
|
||||||
|
**Source:** §"The Channel Re-Key Protocol" (rekey-protocol)
|
||||||
|
**Notes:** **[Open-ended enumeration — Phase 3, resolved]** Exactly 3 rekey
|
||||||
|
methods. `[CANDIDATE: unified.md — ocserv's `rekey-method`/`rekey-time`
|
||||||
|
config options (check `doc/sample.config`) map onto which of these 3? Is
|
||||||
|
`new-tunnel` actually implemented, or is it client-only behavior ocserv
|
||||||
|
merely advertises?]`
|
||||||
|
|
||||||
|
### OC-PROTO-CTRL-002
|
||||||
|
**Requirement:** When the `ssl` rekey method is used under TLS/DTLS 1.2, both
|
||||||
|
peers MUST ensure either safe renegotiation (RFC 5746) is used, or that the
|
||||||
|
peer's identity (certificate) remains unchanged across the rekey — to prevent
|
||||||
|
a renegotiation-based identity-substitution attack.
|
||||||
|
**Strength:** MUST
|
||||||
|
**Source:** §"The Channel Re-Key Protocol" (rekey-protocol), final paragraph
|
||||||
|
**Notes:** This is a `SEC`-flavored requirement embedded in the CTRL section.
|
||||||
|
`[CANDIDATE: unified.md SEC — does GnuTLS's RFC 5746 support make this
|
||||||
|
automatic for ocserv (i.e. is this requirement satisfied "for free" by the
|
||||||
|
TLS library and thus UNIVERSAL/no-action), or does ocserv need explicit
|
||||||
|
identity-pinning logic across a TLS 1.2 rehandshake?]`
|
||||||
|
|
||||||
|
### OC-PROTO-CTRL-003
|
||||||
|
**Requirement:** Any peer receiving a DPD-REQ packet MUST respond with a
|
||||||
|
DPD-RESP packet carrying identical contents to the request (enabling its use
|
||||||
|
for Path MTU detection via arbitrary attached data); any peer receiving a
|
||||||
|
KEEPALIVE packet MUST respond with another KEEPALIVE packet. DPD timing is
|
||||||
|
advisory to clients but the response obligation is unconditional ("MUST
|
||||||
|
respond").
|
||||||
|
**Strength:** MUST
|
||||||
|
**Source:** §"The Keepalive and Dead Peer Detection Protocols"
|
||||||
|
(dead-peer-detection)
|
||||||
|
**Notes:** Server-advertised timing via `X-CSTP-DPD`, `X-CSTP-Keepalive`,
|
||||||
|
`X-DTLS-DPD`, `X-DTLS-Keepalive` (relative seconds, per-channel).
|
||||||
|
`[CANDIDATE: unified.md — verify OCSERV worker responds to DPD-REQ with
|
||||||
|
byte-identical DPD-RESP payload, not just an empty/fixed DPD-RESP — the
|
||||||
|
"suitable for Path MTU detection" property depends on this.]`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## SEC — Security considerations
|
||||||
|
|
||||||
|
### OC-PROTO-SEC-001
|
||||||
|
**Requirement:** Implementations MUST NOT enable payload compression
|
||||||
|
(`oc-lz4`, `lzs`) by default, because compression before encryption can leak
|
||||||
|
information about plaintext length/content (CRIME/BREACH-style attacks);
|
||||||
|
each side MAY still choose to compress individual packets using the
|
||||||
|
COMPRESSED DATA payload type if compression was explicitly negotiated, and
|
||||||
|
MUST be able to *receive* both compressed and uncompressed payloads
|
||||||
|
regardless of its own send-side choice.
|
||||||
|
**Strength:** MUST NOT (default-on) / MAY (opt-in compression) / MUST
|
||||||
|
(bidirectional decode capability)
|
||||||
|
**Source:** §"Compression" (compression), §security-analysis
|
||||||
|
**Notes:** This is the spec's one explicit `MUST NOT` outside the auth
|
||||||
|
sections — a strong signal for `protocol/unified.md`. `[CANDIDATE: unified.md
|
||||||
|
SEC — confirm ocserv's default config has compression disabled
|
||||||
|
(`doc/sample.config` `compression` option default), and that enabling it
|
||||||
|
requires an explicit administrator opt-in, not just client advertisement via
|
||||||
|
`X-CSTP-Accept-Encoding`.]`
|
||||||
|
|
||||||
|
### OC-PROTO-SEC-002
|
||||||
|
**Requirement:** All security considerations of the referenced TLS (RFC 8446)
|
||||||
|
and DTLS (RFC 6347) specifications apply to this protocol; additionally,
|
||||||
|
while payload contents are encrypted, payload *lengths* remain visible and
|
||||||
|
may in some scenarios reveal information about the transferred data, an
|
||||||
|
effect compression can exacerbate.
|
||||||
|
**Strength:** MUST (RFC 8446/6347 considerations apply) / informational
|
||||||
|
(length-leakage caveat)
|
||||||
|
**Source:** §"Security Considerations" (security-analysis)
|
||||||
|
**Notes:** This is a blanket incorporation-by-reference — `[CANDIDATE:
|
||||||
|
unified.md should not attempt to re-derive RFC 8446/6347 requirements
|
||||||
|
verbatim, but should reference them as the "RFC-TLS/RFC-DTLS" source
|
||||||
|
category mentioned in the document map, and focus reconciliation on the
|
||||||
|
OpenConnect-specific deltas in this document.]`
|
||||||
|
|
||||||
|
### OC-PROTO-SEC-003
|
||||||
|
**Requirement:** Because TLS 1.2 and earlier do not encrypt client/server
|
||||||
|
certificates during the handshake, certificates used with this protocol
|
||||||
|
SHOULD contain the minimum possible identifying information (reiterates
|
||||||
|
OC-PROTO-AUTH-007's SHOULD NOT for client certs, extended to server certs).
|
||||||
|
**Strength:** SHOULD
|
||||||
|
**Source:** §"Security Considerations" (security-analysis), final paragraph
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## EXT — Extensibility / compression algorithms
|
||||||
|
|
||||||
|
### OC-PROTO-EXT-001
|
||||||
|
**Requirement:** The set of named compression algorithms for CSTP/DTLS
|
||||||
|
channels is `oc-lz4` (stateless LZ4) and `lzs` (stateless LZS/Stacker); both
|
||||||
|
MUST be stateless, specifically so that compression context is not shared
|
||||||
|
across packets from different sources.
|
||||||
|
**Strength:** MUST (statelessness) — the algorithm *names* are a closed
|
||||||
|
enumeration in this draft version
|
||||||
|
**Source:** §"Compression" (compression), table `compression_table`
|
||||||
|
**Notes:** **[Open-ended enumeration — Phase 3, resolved for this draft
|
||||||
|
version]** Exactly 2 algorithms named. `[CANDIDATE: unified.md EXTENSION —
|
||||||
|
does ocserv support both, one, or additional algorithms beyond this list
|
||||||
|
(e.g. zlib)? Check `WSRCONFIG(ws)->...` compression config and GnuTLS
|
||||||
|
capabilities.]`
|
||||||
|
|
||||||
|
### OC-PROTO-EXT-002
|
||||||
|
**Requirement:** `X-CSTP-Content-Encoding` / `X-DTLS-Content-Encoding`
|
||||||
|
response headers, when present, MUST be set to one of the algorithm names the
|
||||||
|
client offered via `X-CSTP-Accept-Encoding` / `X-DTLS-Accept-Encoding` —
|
||||||
|
i.e. the server MUST NOT select a compression algorithm the client did not
|
||||||
|
advertise support for.
|
||||||
|
**Strength:** MUST NOT (select unadvertised algorithm)
|
||||||
|
**Source:** §"Server response and tunnel configuration" (server-response)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Completeness notes
|
||||||
|
|
||||||
|
- **`example-session1.atxt`/`.uml`** (sibling files in the protocol repo, not
|
||||||
|
read in this pass): likely a worked example consistent with the
|
||||||
|
"Overview of the tunnel establishment" ASCII diagram already extracted
|
||||||
|
inline (§example-session) — `[CANDIDATE: skip; the inline diagram already
|
||||||
|
covers the same sequence as OC-PROTO-CONN-001..006.]`
|
||||||
|
- **DTD (`config-auth.dtd`, §dtd-decl)**: defines the XML schema for
|
||||||
|
`config-auth`/`auth`/`form`/`input`/`select`/`option` elements, including a
|
||||||
|
`select name="group_list"` element not otherwise discussed in the prose.
|
||||||
|
`[UNDOCUMENTED in the prose: candidate OC-PROTO-AUTH-* for group selection
|
||||||
|
via a `<select name="group_list">` form — this likely corresponds to
|
||||||
|
ocserv's `auth-group`/multi-group selection UI (`internal/worker.md`
|
||||||
|
`resolve_selected_group`/`append_group_idx` functions, referenced but not
|
||||||
|
detailed in worker.md). Should be elicited as its own requirement before
|
||||||
|
reconciliation, since `internal/worker.md` has direct code for this.]`
|
||||||
|
- **Versioning**: the draft is "Version 1.2" with no discussion of how a
|
||||||
|
client/server negotiate or detect protocol version, beyond the
|
||||||
|
`<version who="vpn">v5.01</version>` / `<version who="sg">0.1(1)</version>`
|
||||||
|
free-form strings in the XML, which appear to be client/server *software*
|
||||||
|
versions, not protocol versions. `[CANDIDATE: unified.md AMBIGUOUS — is
|
||||||
|
there any protocol-level version negotiation at all, or is compatibility
|
||||||
|
determined entirely by which optional headers/features are present
|
||||||
|
("feature detection" rather than version negotiation)?]`
|
||||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user