Crypto v0.0.4 by janiejestemja · Pull Request #799 · unyt-org/datex

janiejestemja · 2026-07-20T21:45:00Z

This draft contains:

static crypto keys for (own) com hub
static crypto keys for each (peers) socket (setup via HelloBlock)
per msg random symmetric key
- msg keys are wrapped using key encryption key, derived from shared secret (using pri keys from own com hub and pub keys from peers socket properties)

Additional changes:

fixes surrounding stackoverflows within the com hub on esp (moved code around, removed some clones, lightened stack with heap allocations)

github-actions · 2026-07-20T21:51:01Z

Test Results (datex_macros)

10 tests ±0 10 ✅ ±0 0s ⏱️ ±0s
2 suites ±0 0 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit 425a1cf. ± Comparison against base commit aca6e0c.

…edded

…and per msg wrapped symmetric keys (random)

janiejestemja

if you're ok with the changes in general, i'll go ahead and replace all the .unwrap()'s with more proper error handling. @benStre @jonasstrehle

sadly i failed at my first attempt to implement pqc across the board, i'll try more thoroughly for crypto v0.0.5 i suppose. luckily this means, i didn't had to introduce the HalloBack block type for now.

in general next things i'd approach are these two

wire up rng behind nonce for aes, and salt for hkdf (current version leaks a little 🥹)
add ephemeral x25519 keys (current implementation is not forward secure 😭 )

what i can't wrap my mind around is

add static keys via config file and implement some sort of whitelisting within the interface-section as well? is there really a need for this?

janiejestemja · 2026-08-23T14:52:37Z

    }
 }

+pub trait CryptoSync: Send + Sync {


the idea here is to circumvent the async restrictions given by web crypto api, for now...

since only web crypto is async, i copied the crypto implementation from embedded there. it's kind of a cheat, and i feel like this can be done differently by doing some refactoring around it.

I think it's a good solution to separate the async and sync implementations - we did something similar for the ComInterfaceFactories. But since crypto is just included via the facade, we need a uniform interface for every implementation. So I think you could implement CryptoSync e.g. for native like it is right now, and then auto derive the Crypto trait for every T that implements CryptoSync.
We could then later actually support sync execution with crypto, but for now, only the async Crypto trait will actually be used.

janiejestemja · 2026-08-23T14:57:48Z

    /// * the block signature is validated because the block is for own endpoint and signature is set
    /// * the block needs to be relayed to other endpoints or is a trace block, which requires async handling for the relay/trace logic
    ///   The MaybeAsync returns the own received block if the block is for own endpoint and signature is valid, otherwise None
    // FIXME #732: this seams to generate a very big future which causes heap allocation to fail on embedded targets (!?)


not entirely certain if i fixed this, by fixing a FIXME no-clone, reordering the code, and using pinned boxes for trace blocks.

janiejestemja · 2026-08-23T15:04:30Z

+                    if is_for_own && block_type != BlockType::Hello {
+                        info!("Block is for this endpoint ({})", self.endpoint);
+
+                        Some(block) // FIXME #733: no clone


fixme comment can be removed, left it there as a reference for review....

janiejestemja · 2026-08-23T15:07:02Z

    }

+    /// Updates the sender and timestamp of the block
+    fn update_sender_and_timestamp(


i'm uncertain why this method was nested into the one beneath (prapare_own_block()), as far as i understand nested function in general lead to issues, although i don't know if this would be here the case.

I think it was just an arbitrary decision because this function was only used inside prepare_own_block, and is also just a standalone utitlity function, not a method that needs self. I don't think theres really a downside in this case, but I also think its cleaner to have it as a separate method.

janiejestemja · 2026-08-23T15:13:56Z

            .flags_and_timestamp
            .set_block_type(BlockType::Hello);
        // TODO #182 include fingerprint of the own public key into body
+        // -> done in prepare_own_block (for now?)


actually the prepare body function currently clones own public keys into the body of hello-blocks, not just a fingerprint. without a working certificate authority, where the fingerprint can be used to retrieve a verified key a fingerprint seems rather useless for now.

although if the long term plan is to setup a server acting as certificate authority in this context, it would be unnecessary to implement per-interface-whitelisting of any sorts.

benStre

Thank you - looks good.
I added a view comments - overall, I think you can continue with this approach.
Loading the endpoint keys from the config would be very cool to have, but you can also add this at the end once everything else works.

benStre · 2026-09-02T12:33:39Z

    }
 }

+pub trait CryptoSync: Send + Sync {


I think it's a good solution to separate the async and sync implementations - we did something similar for the ComInterfaceFactories. But since crypto is just included via the facade, we need a uniform interface for every implementation. So I think you could implement CryptoSync e.g. for native like it is right now, and then auto derive the Crypto trait for every T that implements CryptoSync.
We could then later actually support sync execution with crypto, but for now, only the async Crypto trait will actually be used.

benStre · 2026-09-02T12:35:53Z


        fn gen_ed25519<'a>()
-        -> AsyncCryptoResult<'a, (Vec<u8>, Vec<u8>), Self::Ed25519GenError>
+        -> AsyncCryptoResult<'a, ([u8; 32], [u8; 32]), Self::Ed25519GenError>


I would recommend to define a type alias for the [u8; 32] and others that are used in multiple locations, e.g.

type PrivKey = [u8; 32]

It might even be better to put them into actual newtype wrapper structs, to ensure a private and public key or whatever don't get accidently mixed up.

benStre · 2026-09-02T12:42:43Z

    }

+    /// Updates the sender and timestamp of the block
+    fn update_sender_and_timestamp(


I think it was just an arbitrary decision because this function was only used inside prepare_own_block, and is also just a standalone utitlity function, not a method that needs self. I don't think theres really a downside in this case, but I also think its cleaner to have it as a separate method.

benStre · 2026-09-02T12:49:37Z

    pub direct_endpoint: Option<Endpoint>,
    pub connection_timestamp: u64,
    uuid: ComInterfaceSocketUUID,
+    pub pub_sig_key: Option<[u8; 32]>,


keys should always be associated directly with an Endpoint, not with a socket. An endpoint might be connected via multiple sockets, and multiple endpoints might be connected via a single socket. It's also necessary to keep the keys in memory when a socket disconnects.
The most simple solution would be something like a HashMap<Endpointt,Keys> stored in the ComHub.

janiejestemja added 14 commits August 23, 2026 11:25

made chrono crate optional and implemented unimplemented stub for emb…

2bf474a

…edded

initial work on pqc

26a040f

WIP pqc and static keys

f5d0381

fixed stack overflow for signature verification on esp32

9b214ff

WIP pqc cleanup

e9dff25

set up per socket tunneling initialised by hello block via x/ed25519 …

b5b2145

…and per msg wrapped symmetric keys (random)

sketched out ml-kem usage as x25519 replacement

d64c13e

cleanup pqc attempts

0f5bb88

removed pqc implementations for native and esp32

2ea339e

edit comments for clarity

dccd45b

removed pqc from dependencies and added sync work around web crypto

e0009a4

do we commit cargo lock file as well?

f25b95d

remove redundant import

c972a19

fix unnecessary imports

1bdfb5d

janiejestemja force-pushed the feat/pqc-r0.0.13 branch from c01a23c to 1bdfb5d Compare August 23, 2026 10:06

janiejestemja added 4 commits August 23, 2026 13:01

renamed pqc crypto to crypto sync

a492c94

minor cleanup

bc6144b

fix illegal signed trace block unit test

7ae8f9e

more minor cleanup

2cc7ec6

janiejestemja commented Aug 23, 2026

View reviewed changes

janiejestemja marked this pull request as ready for review August 23, 2026 16:47

jonasstrehle assigned jonasstrehle, benStre and janiejestemja Sep 1, 2026

jonasstrehle added the scope:crypto label Sep 1, 2026

benStre requested review from benStre and jonasstrehle September 1, 2026 18:42

benStre reviewed Sep 2, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

janiejestemja commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 20, 2026

Test Results (datex_macros)

Uh oh!

janiejestemja left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benStre left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

janiejestemja commented Jul 20, 2026 •

edited

Loading