Skip to content

CTAP 2.3#219

Open
0x0ece wants to merge 2 commits into
trussed-dev:mainfrom
0x0ece:0x0ece/ctap
Open

CTAP 2.3#219
0x0ece wants to merge 2 commits into
trussed-dev:mainfrom
0x0ece:0x0ece/ctap

Conversation

@0x0ece
Copy link
Copy Markdown

@0x0ece 0x0ece commented May 10, 2026

Two changes:

  1. We now need a press and long-press events, so we can support the proper reset according to specs (long-press >= 5s, instead of our custom plug-pressing)
  2. ml-dsa-44, using rust-crypto, it should work on m4/m33. I saw the other PR with definitions for all post quantum algos, I used the same def for features, realistically we won't do more than 128-bit security so to me it doesn't make sense to have code for all 3, but we can add later if needed.

@0x0ece 0x0ece force-pushed the 0x0ece/ctap branch 2 times, most recently from f672cdc to b33c2cf Compare May 10, 2026 07:58
Copy link
Copy Markdown
Member

@robin-nitrokey robin-nitrokey left a comment

Choose a reason for hiding this comment

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

LGTM, I just have some small comments regarding the COSE serialization. Also I want to give @sosthene-nitrokey a chance to review before merging.

Previously we were more reluctant to add experimental features to trussed directly (and would maybe have used a custom backend instead), but with the increased separation due to trussed-core and as this is implemented using a rust-crypto crate, I think it is totally fine.

Comment thread src/mechanisms/mldsa44.rs
Comment on lines +125 to +128
// 01 07 — 1: 7 (kty = AKP)
// 03 38 31 — 3: -50 (alg = ML-DSA-44; -50 = ~49 = 0x31)
// 20 — -1 (key = pub)
// 59 05 20 — bstr(1312)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I find the comment “key = pub” somewhat misleading. I would put it like this:

Suggested change
// 01 07 — 1: 7 (kty = AKP)
// 03 38 31 — 3: -50 (alg = ML-DSA-44; -50 = ~49 = 0x31)
// 20 — -1 (key = pub)
// 59 05 20 — bstr(1312)
// 01 07 — key 1 = kty, value 7 = APK
// 03 38 31 — key 3 = alg, value -50 = ML-DSA-44 (-50 = ~49 = 0x31)
// 20 — key -1 = pub
// 59 05 20 — bstr with length 1312 = 0x0520

Comment thread src/mechanisms/mldsa44.rs
Comment on lines +133 to +135
serialized_key
.extend_from_slice(&[0x20])
.map_err(|_| Error::InternalError)?; // low byte of 1312 (0x0520)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this not part of header?

Comment thread src/mechanisms/mldsa44.rs
Comment on lines +129 to +135
let header: [u8; 9] = [0xa3, 0x01, 0x07, 0x03, 0x38, 0x31, 0x20, 0x59, 0x05];
serialized_key
.extend_from_slice(&header)
.map_err(|_| Error::InternalError)?;
serialized_key
.extend_from_slice(&[0x20])
.map_err(|_| Error::InternalError)?; // low byte of 1312 (0x0520)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe it would be clearer to use PUBLIC_KEY_LEN here instead of duplicating the magic constant?

Comment thread src/mechanisms/mldsa44.rs
Comment on lines +118 to +119
// Hand-encoded CBOR — `cbor-smol` is awkward for negative-int
// map keys and we need exact bytes regardless of crate quirks.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just out of curiosity, what problem did you have with cbor-smol?

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.

2 participants