fix(relay): wire issuer_verifying_key to relay's own signing key (bug #123 D3)

[louzt]

Bug #123 (D3) — issuer_verifying_key wired to relay key (was zeroed dummy)

Summary

Replaces the hardcoded zeroed ed25519 key in listener::handle_connection with
the relay's actual long-term signing key. Closes a critical auth gap exposed
during the PR #32 follow-up review.

Root cause

src/relay/listener.rs:138-145 previously constructed the issuer
verifying-key and expected-subject for server_handshake from a hardcoded
zeroed SigningKey::from_bytes(&[0u8; 32]). That zeroed key is a well-known
public key — its verifying-key is the bytes 00..00 (32 of them). Any ticket
whose subject field equals the all-zero ed25519 verifying-key (which is
trivial to forge or set via with_zeroed_subject()) would pass subject
validation against this server.

The intent was to validate that the ticket was issued by this relay.
The implementation accepted any ticket whose subject was the all-zero
verifying-key, defeating the entire trust-on-first-use model.

Fix

1. RelayConfig now owns a signing_key: SigningKey.
2. Relay::signing_key() exposes it; RelayConfig::with_signing_key()
   allows production callers (e.g. main.rs) to pin a deterministic key.
3. RelayConfig::new() defaults to generate_signing_key() — fresh key per
   process, preserving existing test ergonomics (no shared state across
   processes in unit tests).
4. listener::handle_connection now passes relay.signing_key().verifying_key()
   as both the issuer_verifying_key and expected_subject to
   server_handshake.

Tests added

Two regression tests in src/relay/mod.rs:

• relay_exposes_signing_key_and_with_signing_key_overrides — verifies the
  accessor and the builder.
• relay_signing_key_is_not_the_dummy_zeroed_key — locks the negative
  invariant: the relay's signing key MUST NOT be the all-zero key. This
  test would fail loudly if anyone reintroduces the dummy_key pattern.

Validation

• cargo build --lib — clean
• cargo test --lib — 60 passed
• cargo test --test integration_trust_sync --test quic_e2e --test relay_listener_e2e — 6 passed (4+1+1)
• cargo clippy --lib -- -D warnings — clean
• cargo clippy --tests -- -D warnings — clean (selected tests; quic_rebind_observer_e2e excluded — belongs to a separate branch)

Out of scope (carried-over gaps, not part of this PR)

• listener::handle_connection does not yet use the trust store to look up
  the subject's verifying-key for downstream authorization (it currently uses
  the relay's own key as both issuer and subject). This is correct for the
  self-ticket model but does not yet implement a trust-store-based issuer
  resolution. Tracked as bug #124 (D4) — separate PR.
• conn.close(0u8.into(), ...) after handshake failure uses error code 0.
  Should be APPLICATION_ERROR (e.g. 0x01). Tracked as bug #125.

References

• Bug #123 (D3) — discovered during PR #32 review follow-up
• Follow-up to PR #32 (C-001 critical auth bypass fix)
• Plan: ~/.claude/plans/shiny-petting-puppy.md

Summary by CodeRabbit

• New Features

  • Added SnapPipe-gated QUIC connection handling, allowing traffic only after a valid ticket handshake.
  • Added visibility into currently active relay sessions.
  • Relay signing keys can now be configured for secure ticket validation.

• Bug Fixes

  • Corrected ticket verification to accept only tickets issued by the intended relay.
  • Improved connection cleanup and session tracking across rejected or completed connections.

• Documentation

  • Documented the SnapPipe handshake flow, fallback behavior, and direct TLS bypass.

[coderabbitai]

📝 Walkthrough

Walkthrough

The relay now owns an explicit signing key for ticket validation and identity derivation, tracks active connection sessions, and uses that key during SnapPipe handshakes. Deployment documentation describes the SnapPipe-gated QUIC listener and fallback flow.

Changes

Relay identity and lifecycle

Layer / File(s)	Summary
Relay signing-key contract src/relay/mod.rs	RelayConfig accepts a signing key, Relay exposes it, and tests cover generated, pinned, and non-dummy keys.
Connection session tracking src/relay/mod.rs	Connection handling tracks active sessions and uses the internal clock closure consistently across rate-limit and duration checks.
Identity-bound handshake flow src/relay/listener.rs, docs/OPERATIONAL-DEPLOYMENT.md	Handshake validation uses the relay signing key, while deployment documentation describes SnapPipe-gated QUIC acceptance and forwarding behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• LOUST-PRO/SnapPipe#1: Separates ticket issuer and subject claims, which relates to the relay’s signing-key-based handshake validation.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately describes the main relay signing-key handshake fix in the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/d3-issuer-verifying-key-wiring

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/relay/mod.rs (1)

420-475: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a test for active_sessions counter balance.

The new active_sessions tracking is exercised by existing tests indirectly (they call handle_connection), but no test asserts the counter returns to zero after completion or is non-zero during a connection. A simple post-connection assertion would verify the increment/decrement pair is balanced.

🧪 Suggested test for active_sessions balance

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn active_sessions_returns_to_zero_after_connection() {
    let trust = Arc::new(TrustStore::new());
    let limiter = Arc::new(RateLimiter::new(100));
    let config = RelayConfig::new("127.0.0.1:0".parse().unwrap(), trust, limiter);
    let relay = Relay::new(config);

    assert_eq!(relay.active_sessions(), 0, "should start at zero");

    let peer = node();
    let incoming = MemoryStream::with_payload(b"ping".to_vec());
    let outgoing = MemoryStream::default();
    let _log = relay
        .handle_connection(peer, incoming, outgoing, 1_000.0, || 1_000.0)
        .await
        .unwrap();

    assert_eq!(relay.active_sessions(), 0, "should return to zero after completion");
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/relay/mod.rs` around lines 420 - 475, Add an async regression test near
the existing relay tests, using handle_connection to process a MemoryStream
connection and asserting active_sessions() is zero before and after completion.
Follow the suggested setup with RelayConfig, node(), MemoryStream, and
appropriate tokio test attributes to verify the increment/decrement balance.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/relay/mod.rs`:
- Around line 420-475: Add an async regression test near the existing relay
tests, using handle_connection to process a MemoryStream connection and
asserting active_sessions() is zero before and after completion. Follow the
suggested setup with RelayConfig, node(), MemoryStream, and appropriate tokio
test attributes to verify the increment/decrement balance.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 65e9dba0-1e19-4c21-a666-09a9c3e9602d

📥 Commits

Reviewing files that changed from the base of the PR and between ad3df11 and e4338fa.

📒 Files selected for processing (3)

• docs/OPERATIONAL-DEPLOYMENT.md
• src/relay/listener.rs
• src/relay/mod.rs