Skip to content

feat(relay): autocert TLS for --domain mode (#9)#13

Merged
ilmoniemi merged 3 commits into
mainfrom
feature/9
May 8, 2026
Merged

feat(relay): autocert TLS for --domain mode (#9)#13
ilmoniemi merged 3 commits into
mainfrom
feature/9

Conversation

@ilmoniemi
Copy link
Copy Markdown
Contributor

Summary

  • Wire cmd/pyrycode-relay/main.go autocert path: :443 (WSS via manager.TLSConfig) and :80 (ACME http-01 via mgr.HTTPHandler(http.NotFoundHandler()) — explicit 404, not 302) with the same timeouts as the insecure path.
  • New internal/relay/tls.go exports NewAutocertManager, EnforceHost (returns 421 Misdirected Request on Host mismatch), TLSConfig (pins MinVersion = TLS 1.2), and the sentinel ErrCacheDirInsecure.
  • Cache dir is created 0700 if missing and rejected (ErrCacheDirInsecure) if an existing dir is group/world-readable — stricter than the AC's "no-op on existing" wording, justified in the spec's security review (TLS keys live there).
  • Adds golang.org/x/crypto/acme/autocert to go.mod (first non-stdlib dep). README's Run section updated with the production command, ACME caveat, and cache-mode note.

Issue

Closes #9.

Testing

  • internal/relay/tls_test.go covers cache-dir creation (0700), no-op on existing secure dir, rejection of insecure dir, non-directory path, missing args, HostPolicy accept/reject (case-mismatch + suffix attack), EnforceHost table (port-tolerant, case-insensitive, 421 on mismatch with no body and next not invoked), and TLSConfig MinVersion pinning.
  • go test -race ./...
  • go vet ./...
  • go build ./cmd/pyrycode-relay
  • Real ACME issuance and :443/:80 listener startup are deliberately not tested (per spec — verified manually on first deploy).

Architecture compliance

  • Public surface, error handling, host-policy semantics, file layout, and MinVersion pinning follow docs/specs/architecture/9-autocert-tls.md exactly.
  • Honoured the spec's departure from the AC literal wording: passes http.NotFoundHandler() to mgr.HTTPHandler (not nil) so non-challenge :80 traffic gets 404 instead of autocert's default 302 redirect — matches the AC's stated intent of explicit-failure semantics. Inline comment in main.go notes the rationale.
  • Single domain only (HostWhitelist(*domain)); no graceful shutdown, no multi-domain, no --acme-email, no metrics — all explicitly out of scope.

🤖 Generated with Claude Code

ilmoniemi added 2 commits May 8, 2026 20:09
Wire production TLS terminus via golang.org/x/crypto/acme/autocert
when --domain is set. Binds :443 for WSS and :80 for the ACME http-01
challenge; non-challenge :80 traffic gets 404 (explicit failure, not
redirect) and :443 requests with a Host header that does not match
--domain get 421 Misdirected Request per the protocol spec.

NewAutocertManager creates the cache dir at 0700 if missing and
refuses to start if an existing dir is group/world-readable
(ErrCacheDirInsecure). TLSConfig pins MinVersion to TLS 1.2.

Closes #9.
@ilmoniemi
Copy link
Copy Markdown
Contributor Author

Code Review: #9

Decision: PASS

Findings

  • [NIT] internal/relay/tls.go:73EnforceHost could include Connection: close on the 421 response so a misdirected client doesn't keep the connection alive trying again. Not required by the spec; leaving as-is is fine.
  • [NIT] cmd/pyrycode-relay/main.go:114defaultCertCache() uses string concatenation for path joining (home + "/.pyrycode-relay/certs"). Pre-existing code, out of scope, but filepath.Join would be more idiomatic if touched later.

Summary

Clean implementation that matches the spec verbatim. The http.NotFoundHandler() choice over nil is correct — the spec note about autocert.HTTPHandler(nil) 302-redirecting GET/HEAD is accurate, and the AC's "explicit failure" intent is honored.

Verified:

  • go vet ./... clean
  • go test -race ./... passes
  • go build ./... succeeds
  • Stricter-than-AC dir-mode policy (ErrCacheDirInsecure) is documented and tested.
  • EnforceHost correctly handles port-tolerant, case-insensitive matching; subtests are race-safe under Go 1.22+ loop semantics (go.mod is 1.26.2).
  • TLSConfig mutates a fresh *tls.Config each call (autocert returns a new one), so no aliasing concern.
  • Two-goroutine startup with os.Exit(1) on listener failure mirrors the insecure path; no shared mutable state.
  • autocert.Manager.HTTPHandler still routes /.well-known/acme-challenge/* to itself when given a non-nil fallback — challenge handling preserved.
  • Security-review section is present in docs/specs/architecture/9-autocert-tls.md with a PASS verdict and explicit findings list, satisfying the security-sensitive label requirement.

Ready to merge.

Add feature doc covering the two-listener wiring (`:443` WSS via
autocert, `:80` ACME http-01), the dual host gates
(`HostWhitelist` for ACME issuance + `EnforceHost` for request-time
421s), the stricter-than-AC cache-dir permission policy, and the TLS
1.2 floor.

ADR-0002 records the explicit-failure 404 on `:80` and why
`HTTPHandler(nil)` is wrong for our posture (autocert redirects
GET/HEAD to HTTPS by default; we want loud failure for misconfigured
clients).

Update PROJECT-MEMORY (autocert listed as built; first non-stdlib dep
noted; loud-failure pattern added) and lessons.md with four new
gotchas: HTTPHandler(nil) behaviour, SNI vs Host divergence, port
stripping on r.Host, and gosec G402 / MinVersion on autocert's
TLSConfig.
@ilmoniemi ilmoniemi merged commit 4d9d02c into main May 8, 2026
2 checks passed
@ilmoniemi ilmoniemi deleted the feature/9 branch May 8, 2026 17:18
ilmoniemi added a commit that referenced this pull request May 8, 2026
Resolves additive doc conflicts from concurrent dispatch of #3 (registry),
#9 (autocert), and #11 (threat-model):

- docs/PROJECT-MEMORY.md: keep registry, autocert, and threat-model rows;
  drop superseded "TLS / autocert | Not started" placeholder; merge all
  Patterns established bullets.
- docs/knowledge/INDEX.md: keep both new feature bullets; renumber the
  registry ADR link to 0003.
- docs/lessons.md: keep all six new lesson sections (registry + autocert).
- docs/knowledge/decisions/0002-connection-registry-passive-store.md ->
  0003-connection-registry-passive-store.md (autocert ADR landed first
  via #9 / PR #13 and owns 0002).
- docs/knowledge/features/connection-registry.md: update internal ADR
  link to 0003.

No code changes. make vet / make test -race / make build all clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

relay: TLS via Let's Encrypt autocert for production --domain mode

1 participant