Skip to content

keep-desktop: add opt-in strict certificate pinning toggle#826

Merged
kwsantiago merged 2 commits into
mainfrom
feat/desktop-strict-cert-pinning
Jul 16, 2026
Merged

keep-desktop: add opt-in strict certificate pinning toggle#826
kwsantiago merged 2 commits into
mainfrom
feat/desktop-strict-cert-pinning

Conversation

@kwsantiago

@kwsantiago kwsantiago commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Closes the remaining desktop half of #786. keep-desktop hardcoded require_pinned = false in verify_relay_certificates, so both the relay-connect path and the bunker path always trusted an un-pinned wss:// relay on first use. keep-mobile already ships the persisted opt-in toggle; this brings desktop to parity and documents out-of-band pin provisioning.

Changes

  • Settings.strict_cert_pinning (persisted in settings.json, #[serde(default)], off by default): strict pinning stays opt-in, and a settings file written before the field existed keeps parsing with strict mode off.
  • verify_relay_certificates takes a require_pinned policy parameter; NetworkConfig carries it from App::network_config() (relay-connect path), and the bunker start path captures it from settings. When enabled, a wss:// relay with no recorded pin is rejected fail-closed instead of TOFU-pinned.
  • Settings UI: a "Strict pinning" toggle in the TLS certificate pinning card, following the existing toggle pattern (screen event bubbles up; the app handler persists via save_settings).
  • docs/SECURITY.md: documents the first-use TOFU window, how to enable strict mode on desktop/mobile, and how to provision relay SPKI pins out-of-band (cert-pins.json hostname → SPKI SHA-256 hex, with an openssl one-liner that matches the pin computation: SHA-256 over the DER-encoded SubjectPublicKeyInfo).

Verification

  • New tests: legacy settings.json without the field parses with strict mode off; the setting round-trips through serde; the toggle message updates both the persisted settings and the screen state (and asserts the opt-in default).
  • cargo test -p keep-desktop: 68 passed. cargo clippy -p keep-desktop --all-targets: clean.

Summary by CodeRabbit

  • New Features

    • Added an optional strict TLS certificate pinning setting for relay connections.
    • When enabled, relays without a pre-recorded certificate pin are rejected instead of trusted on first connection.
    • Added a settings toggle to enable or disable strict pinning.
  • Documentation

    • Documented strict pinning behavior, certificate pin provisioning, storage locations, and pin mismatch handling.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@kwsantiago, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 48 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 07a38778-a7fe-44b1-b1fb-04b2ece71a48

📥 Commits

Reviewing files that changed from the base of the PR and between 8c4be69 and a113987.

📒 Files selected for processing (5)
  • docs/SECURITY.md
  • keep-desktop/src/screen/settings.rs
  • keep-frost-net/src/cert_pin.rs
  • keep-frost-net/src/error.rs
  • keep-mobile/src/lib.rs

Walkthrough

A new opt-in desktop setting controls strict TLS certificate pinning. The value is persisted, exposed in the settings UI, propagated into relay verification, and documented with pin provisioning instructions. Strict mode rejects unpinned relays while retaining pin-mismatch errors.

Changes

Strict certificate pinning

Layer / File(s) Summary
Settings and UI control
keep-desktop/src/app/config.rs, keep-desktop/src/screen/settings.rs, keep-desktop/src/app/mod.rs
Adds the persisted strict_cert_pinning setting, UI toggle, message/event handling, default-off behavior, serialization compatibility, and state tests.
Policy propagation
keep-desktop/src/app/helpers.rs, keep-desktop/src/app/navigation.rs, keep-desktop/src/app/settings.rs, keep-desktop/src/app/mod.rs, keep-desktop/src/bunker_service.rs
Passes the setting through screen state, network configuration, settings updates, and bunker startup.
Certificate verification enforcement
keep-desktop/src/frost.rs, docs/SECURITY.md
Uses require_pinned to select strict pinning or TOFU behavior and documents out-of-band pin provisioning and storage.

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

Possibly related issues

Possibly related PRs

  • privkeyio/keep#783 — Adds matching strict-versus-TOFU enforcement in the underlying certificate evaluation path.
  • privkeyio/keep#805 — Implements related SPKI pinning behavior using the require_pinned policy.
  • privkeyio/keep#812 — Covers the same persisted strict-pinning setting and verification propagation.

Poem

A rabbit flips the pinning switch,
No unknown relay gets a stitch.
Pins are stored, the trust path’s clear,
Mismatch bounces from the ear.
Strict and safe, the tunnels gleam—
Hop, hop, secure the stream!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: an opt-in strict certificate pinning toggle for keep-desktop.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/desktop-strict-cert-pinning

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

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

@kwsantiago kwsantiago merged commit 89ee825 into main Jul 16, 2026
9 checks passed
@kwsantiago kwsantiago deleted the feat/desktop-strict-cert-pinning branch July 16, 2026 15:38
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.

1 participant