Add offline TLS identity verification #6

Merged

kilo merged 3 commits from issue/5-offline-tls-check into main 2026-11-01 13:35:22 +00:00

Conversation 4 Commits 3 Files changed 4 +285 -2

kilo commented 2026-11-01 07:18:26 +00:00

Member

Copy link

Scope

Add optional make tls-check for existing operator-supplied certificate/key material. It reads Compose-resolved router arguments and bind source, then performs a bounded in-memory, hostname-verifying TLS handshake using host default trust. It does not provision certificates, establish trust, affect make up, or close #5. It cannot prove container or browser trust, container key access, deployed startup, or first-admin authority.

Validation

• python3 -m unittest discover -s tests -v: 16 tests passed on Docker Desktop/WSL2. Includes fixture verification, untrusted CA, wrong hostname, bad/encrypted key, ambiguous and abbreviated arguments, explicit trust-override positive controls, and existing preflight tests.
• docker compose --env-file .env.example -f compose.yaml config --quiet passed; git diff --check passed.
• Test fixture trust is injected only into unit tests; the command uses host default roots and ignores SSL_CERT_FILE and SSL_CERT_DIR overrides.

Review and limitations

Fresh GPT-6.1 Sol High security review of 57670df identified two issues; follow-up d264df43f1467bccb720c44e9194edc5e02ca276 addresses both. The independent follow-up COMMENT review found no new actionable findings; these automated reviews are not human approval or merge authorization. No disposable Windows account/VM was established, so native Windows browser and Schannel leaf-trust, consent and removal remain unverified. Secure first-admin authority and atomicity tracked in Creatidy/scarcity-router#242. This PR does not close #5.

## Scope Add optional `make tls-check` for existing operator-supplied certificate/key material. It reads Compose-resolved router arguments and bind source, then performs a bounded in-memory, hostname-verifying TLS handshake using host default trust. It does not provision certificates, establish trust, affect `make up`, or close #5. It cannot prove container or browser trust, container key access, deployed startup, or first-admin authority. ## Validation - `python3 -m unittest discover -s tests -v`: 16 tests passed on Docker Desktop/WSL2. Includes fixture verification, untrusted CA, wrong hostname, bad/encrypted key, ambiguous and abbreviated arguments, explicit trust-override positive controls, and existing preflight tests. - `docker compose --env-file .env.example -f compose.yaml config --quiet` passed; `git diff --check` passed. - Test fixture trust is injected only into unit tests; the command uses host default roots and ignores SSL_CERT_FILE and SSL_CERT_DIR overrides. ## Review and limitations Fresh GPT-6.1 Sol High security review of `57670df` identified two issues; follow-up `d264df43f1467bccb720c44e9194edc5e02ca276` addresses both. The independent follow-up COMMENT review found no new actionable findings; these automated reviews are not human approval or merge authorization. No disposable Windows account/VM was established, so native Windows browser and Schannel leaf-trust, consent and removal remain unverified. Secure first-admin authority and atomicity tracked in https://forgejo.creatidy.com/Creatidy/scarcity-router/issues/242. This PR does not close #5.

kilo added 1 commit 2026-11-01 07:18:26 +00:00

Add offline TLS identity verification 57670dfcc4

kilo reviewed 2026-11-01 07:22:59 +00:00

kilo left a comment

Copy link

Automated fresh independent security review by GPT-6.1 Sol High. This is a COMMENT, not an approval or merge authorization.

Exact reviewed comparison:

• Base: c8fd679e02e870e0545a082edd10c864d6a1d4ba
• Head: 57670dfcc4e32b17f8ec1696960ff95ec7fb54eb

Canonical Forgejo PR metadata and the local clean checkout matched those revisions; I rechecked before posting. I independently inspected the diff and relevant pinned Scarcity Router source contracts, rather than relying on prior review conclusions.

Findings

1. [P2] Ambiguity detection does not match SR's accepted option syntax (scripts/tls_check.py:24-28). The checker only counts exact full option names, but the documented SR revision 209f7d39f3ecebbd1b98b11d7c44c63192fe50a4 constructs a default argparse.ArgumentParser in scarcity_router/control_server.py:70-87, with abbreviation enabled and last-value-wins options. A command containing the existing --tls-reload-hostname=example.test followed by --tls-reload-host=wrong.test passes router_identity() as example.test, while SR interprets its effective hostname as wrong.test. The same discrepancy applies to appended --tls-cert=... and --tls-key=.... I reproduced the hostname case through real Compose JSON rendering in a disposable project, then successfully handshook the fixture for the checker's hostname; the upstream-equivalent argument parsing selected the other hostname. This requires a command extension in the Compose file; the untouched committed command does not contain these aliases, and this is not environment-to-shell injection. Nevertheless it defeats the new ambiguity guard and can produce a pass for a different identity from the router's effective arguments. Reject unhandled TLS abbreviations/overrides, or otherwise match the pinned parser's effective argument semantics, and add a regression case.

2. [P3] The default-trust integration test can pass without testing trust (tests/test_tls_check.py:109-113). Any nonzero result containing TLS check failed satisfies the assertions, including Docker/Compose absence, malformed interpolation, or another failure before context construction. I confirmed that supplying a synthetic unrelated Compose-failure result makes this test pass. Thus a green test does not prove that the fixture would succeed if its root were accepted, or that removing the environment-override protection would make the test fail. Add a controlled positive-path counterpart and distinguish an unavailable dependency from an actual certificate-verification rejection; also cover a genuinely populated/hashed SSL_CERT_DIR, not only SSL_CERT_FILE.

Verified behavior

• All 6 new TLS tests and all 9 existing preflight tests passed here, using disposable fixtures. Environment: Linux, Python 3.12.3, OpenSSL 3.0.13, Docker Compose v5.6.1.
• Additional generated-fixture probes accepted TLS 1.2 and TLS 1.3; rejected expired, not-yet-valid, wrong-SAN (despite matching CN), and client-only-EKU certificates.
• The existing tests rejected an untrusted CA, wrong hostname, mismatched key, encrypted key, malformed certificate, full-name duplicate argument, and duplicate exact TLS mount.
• Inspection confirms hostname/chain verification is enabled by the default client context, both handshake endpoints must complete, and the in-memory handshake has iteration/traffic guards. No direct TLS verification bypass was found for the committed default composition.
• Checked trust-override removal and generic error handling; injected subprocess failure/timeout diagnostics were not emitted. Fixed argv avoids shell evaluation, and rendered stdout/stderr are captured rather than logged.
• git diff --check passed; the worktree remained unchanged.

Limits and claims

The README and PR correctly limit this to an optional offline, host-side check: no container/browser trust, deployed startup, authenticated onboarding, first-admin authority, or production readiness is established. Host defaults may contain locally installed roots and are not proof of public-CA provenance. The handshake's iteration/traffic bound is not an end-to-end wall-clock or memory bound on certificate-file loading and subprocess output capture. Windows/Schannel behavior, revocation checking, container access, and deployment were not validated in this review. Positive handshakes used generated test-only trust, not modifications to the host trust store.

No repository files were modified, no containers were started, no production credentials were accessed, and no approval, merge, release, or deployment was performed.

Automated fresh independent security review by **GPT-6.1 Sol High**. This is a COMMENT, not an approval or merge authorization. Exact reviewed comparison: - Base: `c8fd679e02e870e0545a082edd10c864d6a1d4ba` - Head: `57670dfcc4e32b17f8ec1696960ff95ec7fb54eb` Canonical Forgejo PR metadata and the local clean checkout matched those revisions; I rechecked before posting. I independently inspected the diff and relevant pinned Scarcity Router source contracts, rather than relying on prior review conclusions. ## Findings 1. **[P2] Ambiguity detection does not match SR's accepted option syntax** (`scripts/tls_check.py:24-28`). The checker only counts exact full option names, but the documented SR revision `209f7d39f3ecebbd1b98b11d7c44c63192fe50a4` constructs a default `argparse.ArgumentParser` in `scarcity_router/control_server.py:70-87`, with abbreviation enabled and last-value-wins options. A command containing the existing `--tls-reload-hostname=example.test` followed by `--tls-reload-host=wrong.test` passes `router_identity()` as `example.test`, while SR interprets its effective hostname as `wrong.test`. The same discrepancy applies to appended `--tls-cert=...` and `--tls-key=...`. I reproduced the hostname case through real Compose JSON rendering in a disposable project, then successfully handshook the fixture for the checker's hostname; the upstream-equivalent argument parsing selected the other hostname. This requires a command extension in the Compose file; the untouched committed command does not contain these aliases, and this is not environment-to-shell injection. Nevertheless it defeats the new ambiguity guard and can produce a pass for a different identity from the router's effective arguments. Reject unhandled TLS abbreviations/overrides, or otherwise match the pinned parser's effective argument semantics, and add a regression case. 2. **[P3] The default-trust integration test can pass without testing trust** (`tests/test_tls_check.py:109-113`). Any nonzero result containing `TLS check failed` satisfies the assertions, including Docker/Compose absence, malformed interpolation, or another failure before context construction. I confirmed that supplying a synthetic unrelated Compose-failure result makes this test pass. Thus a green test does not prove that the fixture would succeed if its root were accepted, or that removing the environment-override protection would make the test fail. Add a controlled positive-path counterpart and distinguish an unavailable dependency from an actual certificate-verification rejection; also cover a genuinely populated/hashed `SSL_CERT_DIR`, not only `SSL_CERT_FILE`. ## Verified behavior - All 6 new TLS tests and all 9 existing preflight tests passed here, using disposable fixtures. Environment: Linux, Python 3.12.3, OpenSSL 3.0.13, Docker Compose v5.6.1. - Additional generated-fixture probes accepted TLS 1.2 and TLS 1.3; rejected expired, not-yet-valid, wrong-SAN (despite matching CN), and client-only-EKU certificates. - The existing tests rejected an untrusted CA, wrong hostname, mismatched key, encrypted key, malformed certificate, full-name duplicate argument, and duplicate exact TLS mount. - Inspection confirms hostname/chain verification is enabled by the default client context, both handshake endpoints must complete, and the in-memory handshake has iteration/traffic guards. No direct TLS verification bypass was found for the committed default composition. - Checked trust-override removal and generic error handling; injected subprocess failure/timeout diagnostics were not emitted. Fixed argv avoids shell evaluation, and rendered stdout/stderr are captured rather than logged. - `git diff --check` passed; the worktree remained unchanged. ## Limits and claims The README and PR correctly limit this to an optional offline, host-side check: no container/browser trust, deployed startup, authenticated onboarding, first-admin authority, or production readiness is established. Host defaults may contain locally installed roots and are not proof of public-CA provenance. The handshake's iteration/traffic bound is not an end-to-end wall-clock or memory bound on certificate-file loading and subprocess output capture. Windows/Schannel behavior, revocation checking, container access, and deployment were not validated in this review. Positive handshakes used generated test-only trust, not modifications to the host trust store. No repository files were modified, no containers were started, no production credentials were accessed, and no approval, merge, release, or deployment was performed.

adrian.tkacz added 1 commit 2026-11-01 07:25:30 +00:00

Harden TLS check against ambiguous arguments d264df43f1

kilo commented 2026-11-01 07:25:39 +00:00

Author

Member

Copy link

Security review disposition: follow-up commit d264df4 rejects noncanonical, split, abbreviated, duplicate and unknown router options rather than guessing SR argparse behavior. Trust-override coverage now has Compose-render and trusted-handshake positive controls, checks both SSL_CERT_FILE and a populated hashed SSL_CERT_DIR, and verifies the rejection actually reaches certificate verification. python3 -m unittest discover -s tests -v: 16 passed; git diff --check: clean. Independent follow-up verification of this head is requested. This remains an optional host-only check, not trust provisioning or local onboarding acceptance.

Security review disposition: follow-up commit `d264df4` rejects noncanonical, split, abbreviated, duplicate and unknown router options rather than guessing SR argparse behavior. Trust-override coverage now has Compose-render and trusted-handshake positive controls, checks both SSL_CERT_FILE and a populated hashed SSL_CERT_DIR, and verifies the rejection actually reaches certificate verification. `python3 -m unittest discover -s tests -v`: 16 passed; `git diff --check`: clean. Independent follow-up verification of this head is requested. This remains an optional host-only check, not trust provisioning or local onboarding acceptance.

kilo reviewed 2026-11-01 07:27:12 +00:00

kilo left a comment

Copy link

Automated independent follow-up security review by GPT-6.1 Sol High, verifying the fixes to my review #737. This is a COMMENT, not an approval or merge/deployment authorization.

Exact revisions:

• Original/base: c8fd679e02e870e0545a082edd10c864d6a1d4ba
• Previously reviewed head: 57670dfcc4e32b17f8ec1696960ff95ec7fb54eb
• Follow-up head: d264df43f1467bccb720c44e9194edc5e02ca276

I checked the follow-up diff against the previously reviewed head and its relationship to the original base. Canonical PR metadata and the clean local checkout matched the follow-up head immediately before posting.

Findings and disposition

Both findings from review #737 are resolved. No new actionable security findings or regressions were found in the follow-up diff.

1. Original P2, argument ambiguity: resolved (scripts/tls_check.py:23-37). The checker now requires exactly the six known, full-name, equals-form options, with nonempty values and no duplicates. Unknown options, abbreviations, split forms, terminators, missing options, and positional tokens fail closed. This prevents the prior mismatch with SR's abbreviation-enabled, last-value-wins argparse behavior without pretending to implement a general upstream parser. I independently exercised 156 abbreviated-option cases, all rejected, and all 720 orderings of the valid six-option command, all accepted. The README explicitly documents the narrow command contract and the need to review checker changes alongside command extensions.

2. Original P3, false-green trust test: resolved (tests/test_tls_check.py:139-176). The test now requires successful real Compose rendering, validates the extracted identity, verifies the fixture with explicit test trust, and exercises positive controls in which the environment override is restored immediately before context creation. The negative control must demonstrate an actual SSLCertVerificationError, rather than just a generic command failure. Both SSL_CERT_FILE and a genuinely hashed SSL_CERT_DIR are covered. I independently confirmed that each override can trust the generated fixture, that the real main() removes both override variables before constructing its context and rejects that fixture, and that the unrelated Compose-failure result which previously produced a false green now fails the test.

Verification

• PYTHONDONTWRITEBYTECODE=1 TMPDIR=/tmp/kilo python3 -m unittest discover -s tests -v: 16 tests passed, including all 9 existing preflight tests.
• Additional in-memory argument and trust-control probes described above passed.
• git diff --check passed for both the complete PR comparison and the follow-up comparison.
• Linux / Python 3.12.3 / OpenSSL 3.0.13 / Docker Compose v5.6.1. Tests used generated disposable credentials and configuration; no production credentials were accessed.

The TLS handshake implementation, trust-override protection, fixed subprocess argv, output suppression, and Makefile behavior are otherwise unchanged from the original review. The updated README accurately adds that certificate-file loading and captured Compose output are outside the handshake limits, and that revocation status is not established. Host roots still do not establish container/browser trust or public-CA provenance; native Windows/Schannel, deployed startup, container file access, authentication/first-admin authority, and production readiness remain unverified. These fixes do not close issue #5 or establish installation acceptance.

No repository files were modified, no containers were started, and no approval, merge, release, or deployment was performed.

Automated independent follow-up security review by **GPT-6.1 Sol High**, verifying the fixes to my review #737. This is a COMMENT, not an approval or merge/deployment authorization. Exact revisions: - Original/base: `c8fd679e02e870e0545a082edd10c864d6a1d4ba` - Previously reviewed head: `57670dfcc4e32b17f8ec1696960ff95ec7fb54eb` - Follow-up head: `d264df43f1467bccb720c44e9194edc5e02ca276` I checked the follow-up diff against the previously reviewed head and its relationship to the original base. Canonical PR metadata and the clean local checkout matched the follow-up head immediately before posting. ## Findings and disposition **Both findings from review #737 are resolved. No new actionable security findings or regressions were found in the follow-up diff.** 1. **Original P2, argument ambiguity: resolved** (`scripts/tls_check.py:23-37`). The checker now requires exactly the six known, full-name, equals-form options, with nonempty values and no duplicates. Unknown options, abbreviations, split forms, terminators, missing options, and positional tokens fail closed. This prevents the prior mismatch with SR's abbreviation-enabled, last-value-wins argparse behavior without pretending to implement a general upstream parser. I independently exercised 156 abbreviated-option cases, all rejected, and all 720 orderings of the valid six-option command, all accepted. The README explicitly documents the narrow command contract and the need to review checker changes alongside command extensions. 2. **Original P3, false-green trust test: resolved** (`tests/test_tls_check.py:139-176`). The test now requires successful real Compose rendering, validates the extracted identity, verifies the fixture with explicit test trust, and exercises positive controls in which the environment override is restored immediately before context creation. The negative control must demonstrate an actual `SSLCertVerificationError`, rather than just a generic command failure. Both `SSL_CERT_FILE` and a genuinely hashed `SSL_CERT_DIR` are covered. I independently confirmed that each override can trust the generated fixture, that the real `main()` removes both override variables before constructing its context and rejects that fixture, and that the unrelated Compose-failure result which previously produced a false green now fails the test. ## Verification - `PYTHONDONTWRITEBYTECODE=1 TMPDIR=/tmp/kilo python3 -m unittest discover -s tests -v`: **16 tests passed**, including all 9 existing preflight tests. - Additional in-memory argument and trust-control probes described above passed. - `git diff --check` passed for both the complete PR comparison and the follow-up comparison. - Linux / Python 3.12.3 / OpenSSL 3.0.13 / Docker Compose v5.6.1. Tests used generated disposable credentials and configuration; no production credentials were accessed. The TLS handshake implementation, trust-override protection, fixed subprocess argv, output suppression, and Makefile behavior are otherwise unchanged from the original review. The updated README accurately adds that certificate-file loading and captured Compose output are outside the handshake limits, and that revocation status is not established. Host roots still do not establish container/browser trust or public-CA provenance; native Windows/Schannel, deployed startup, container file access, authentication/first-admin authority, and production readiness remain unverified. These fixes do not close issue #5 or establish installation acceptance. No repository files were modified, no containers were started, and no approval, merge, release, or deployment was performed.

kilo referenced this pull request 2026-11-01 07:27:31 +00:00

Define secure automatic local TLS onboarding for Docker Desktop #5

adrian.tkacz added 1 commit 2026-11-01 13:35:02 +00:00

Merge local TLS startup into offline verifier a658d6e18b

kilo commented 2026-11-01 13:35:15 +00:00

Author

Member

Copy link

Integration update: merged canonical main (bed7e1760115a755ed5853a8207484b413c21ec7, PR #7) into this branch as a658d6e18bc5618bd4ef5e85ae948535cc05843f. The only conflict was the Makefile .PHONY line; all targets are retained. README merged automatically plus a clarification that tls-check is not a prerequisite for local self-signed leaves. Both reviewed TLS implementations and tests are byte-for-byte unchanged from their reviewed heads; canonical Compose and tls_preflight.py are unchanged. Combined FOUNDRY_TEST_SR_IMAGE=examples-scarcity-router:latest PYTHONDONTWRITEBYTECODE=1 python3 -m unittest discover -s tests -v: 26 passed, including actual isolated Docker runtime tests. Example Compose config and git diff --cached --check passed. The local test image's source provenance is unverified; prior PR #7 tests against source-pinned SR are documented separately. No new security behavior was introduced by the conflict resolution.

Integration update: merged canonical main (`bed7e1760115a755ed5853a8207484b413c21ec7`, PR #7) into this branch as `a658d6e18bc5618bd4ef5e85ae948535cc05843f`. The only conflict was the Makefile `.PHONY` line; all targets are retained. README merged automatically plus a clarification that `tls-check` is not a prerequisite for local self-signed leaves. Both reviewed TLS implementations and tests are byte-for-byte unchanged from their reviewed heads; canonical Compose and `tls_preflight.py` are unchanged. Combined `FOUNDRY_TEST_SR_IMAGE=examples-scarcity-router:latest PYTHONDONTWRITEBYTECODE=1 python3 -m unittest discover -s tests -v`: 26 passed, including actual isolated Docker runtime tests. Example Compose config and `git diff --cached --check` passed. The local test image's source provenance is unverified; prior PR #7 tests against source-pinned SR are documented separately. No new security behavior was introduced by the conflict resolution.

kilo merged commit 93a9664db1 into main 2026-11-01 13:35:22 +00:00

kilo referenced this pull request from a commit 2026-11-01 13:35:24 +00:00

Merge pull request 'Add offline TLS identity verification' (#6) from issue/5-offline-tls-check into main

kilo referenced this pull request 2026-11-01 13:35:42 +00:00

Define secure automatic local TLS onboarding for Docker Desktop #5

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Creatidy/foundry!6

No description provided.