Skip to content

Refactor + demo against updated spec#164

Merged
cfm merged 13 commits intomainfrom
one-crab-to-rule-them-all
Feb 27, 2026
Merged

Refactor + demo against updated spec#164
cfm merged 13 commits intomainfrom
one-crab-to-rule-them-all

Conversation

@rocodes
Copy link
Contributor

@rocodes rocodes commented Jan 28, 2026

Closes #140
Towards #115
Refs #138

(WIP, more explanation tk)

  • Refactor encrypt_decrypt.rs (single file benchmark impl) and merge with jen scaffold impl. Update scaffold tests.

Test Plan

  • CI
  • Visual review
  • ? Code walkthrough
  • Feedback from n >= 1 fpf rustacean

@cfm cfm added this to SecureDrop Jan 28, 2026
@cfm cfm moved this to Ready For Review in SecureDrop Jan 28, 2026
@cfm cfm self-assigned this Jan 28, 2026
@cfm cfm self-requested a review January 28, 2026 22:15
@cfm cfm moved this from Ready For Review to In Progress in SecureDrop Jan 28, 2026
@rocodes rocodes force-pushed the one-crab-to-rule-them-all branch from f4003f7 to 7713e14 Compare February 3, 2026 00:21
mlkem, derive Copy for DHPublicKey.
Refactor our key structs into typed() helper fn.

bugfix: Fixup DH shared secret in primitives/x25519. Add test.
bugfix: Fix mlkem768 deterministic_keygen seed size. Add from_bytes deterministic keygen fn.
…key. Refactor into new types.

Add Api and (restricted) JournalistApi and implement.

Remove source constructor without passphrase.
types, and Client, JournalistClient, and SourceClient (now handled by
api.rs).

Remove unused methods and structs from primitives.rs.
…Deprecate ServerMessageStore and use new wrapped wasm type for de/serializing hashmaps in wasm. Add uuid with js features as a dependency.
…repare for splitting into multiple files. Add passphrase function for source testing purposes.
server.rs: use primitives constant for number of fetch challenges
…and JournalistClient. Use new 0.3 functions.
@rocodes rocodes force-pushed the one-crab-to-rule-them-all branch from 7713e14 to 22a6846 Compare February 3, 2026 00:38
@rocodes rocodes changed the title [wip] Refactor + demo against updated spec Refactor + demo against updated spec Feb 3, 2026
@rocodes rocodes marked this pull request as ready for review February 3, 2026 00:42
@rocodes rocodes requested review from a team as code owners February 3, 2026 00:42
@rocodes
Copy link
Contributor Author

rocodes commented Feb 3, 2026

This is out of draft mode and feedback is welcome. There's a little more I'll do on the setup / core tests, so the goal is feedback, not merge :)

) -> Result<DHSharedSecret, Error> {
let mut shared_secret_bytes = [0u8; 32];
libcrux_curve25519::ecdh(&mut shared_secret_bytes, &private_scalar, &public_key.0)
ecdh(&mut shared_secret_bytes, &public_key.0, &private_scalar)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from https://docs.rs/libcrux-curve25519/latest/libcrux_curve25519/fn.ecdh.html, param order

pub fn ecdh(
    out: &mut [u8; 32],
    pk: &[u8; 32],
    sk: &[u8; 32],
) -> Result<(), Error>

Comment on lines 76 to 85
// Validate key sizes (XWING should have consistent sizes)
if private_key_bytes.len() != XWING_PRIVATE_KEY_LEN
|| public_key_bytes.len() != XWING_PUBLIC_KEY_LEN
{
return Err(anyhow::anyhow!(
"Unexpected XWING key sizes: private={}, public={}",
private_key_bytes.len(),
public_key_bytes.len()
));
}
Copy link
Member

@cfm cfm Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind of logic might be good low-hanging fruit for #154: i.e., proving—

private_key_bytes.len() == XWING_PRIVATE_KEY_LEN
public_key_bytes.len() == XWING_PUBLIC_KEY_LEN

—as invariants of these typed() helpers that they can't violate. Likewise various sanity-checking assertions, if we can make them pre- and post-conditions on function boundaries.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've started this in #167.

Comment on lines 125 to -149
request
.journalist_verifying_key
.verify(&signed_message, &bundle.signature)
.bundles
.iter()
.try_for_each(|k| request.vk.verify(&k.0.as_bytes(), &k.1.as_signature()))
.map_err(|_| anyhow::anyhow!("Invalid signature on ephemeral keys"))?;

// Store the ephemeral keys for the journalist
self.storage
.add_ephemeral_keys(journalist_id, Vec::from([bundle]));
Copy link
Member

@cfm cfm Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably premature overengineering, but I'll mention it just for the sake of starting the discussion early and concretely: This codebase may be a good candidate for the typestate pattern, e.g. "only save keys whose signatures have been verified".

@cfm
Copy link
Member

cfm commented Feb 4, 2026

Exciting to see this! I've read through and left a few comments on possible typing and verifying patterns—more ideas for future reference than feedback per se.

@cfm cfm removed their assignment Feb 4, 2026
cfm added a commit to cfm/securedrop-protocol that referenced this pull request Feb 14, 2026
securedrop_protocol_minimal::primitives::xwing::typed per
<freedomofpress#164 (comment)>.
@cfm
Copy link
Member

cfm commented Feb 17, 2026

Discussed today: @rocodes will rebase this from main, and then I'll review it for merge. No reason not to land this and refine it incrementally.

cfm added a commit to cfm/securedrop-protocol that referenced this pull request Feb 23, 2026
securedrop_protocol_minimal::primitives::xwing::typed per
<freedomofpress#164 (comment)>.
Comment on lines +21 to +22
/// Common client functionality for source and journalist clients
pub trait Api {
Copy link
Member

@cfm cfm Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this pattern, and I think it's a good foundation for exposing the source/journalist asymmetry at the API level. However, in #167 I've just discovered that hax's F* back end doesn't yet support it. We probably don't care for this module, which is high-level enough that we're unlikely to prove much of it in hax, but just FYI.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #169.

fn fetch_keypair(&self) -> (&DHPrivateKey, &DHPublicKey);
fn message_auth_keypair(&self) -> (&DhAkemPrivateKey, &DhAkemPublicKey);
fn build_message(&self, message: Vec<u8>) -> Plaintext;
fn keybundles(&self) -> impl Iterator<Item = &MessageKeyBundle>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (nice!) pattern crashes hax pending cryspen/hax#1965.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and will not be supported. I imagine we very much will want proofs for our types, key bundles among them, and when we do we'll need to use a different pattern here. (But #164 takes an incremental approach precisely so that we can decide when—in this case, when the refactoring work will be worth the proving benefit.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #169.

Copy link
Member

@cfm cfm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving after discussing with @redshiftzero—follow-up refinements welcome!

@cfm cfm added this pull request to the merge queue Feb 27, 2026
Merged via the queue into main with commit d53d8f4 Feb 27, 2026
16 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in SecureDrop Feb 27, 2026
cfm added a commit to cfm/securedrop-protocol that referenced this pull request Feb 27, 2026
securedrop_protocol_minimal::primitives::xwing::typed per
<freedomofpress#164 (comment)>.
@nathandyer nathandyer removed this from SecureDrop Mar 2, 2026
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.

Proposal to reorganize rust components / benchmarks

2 participants