ML-DSA Composite Signature Support#1109
Conversation
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1109
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
9753775 to
b1fec5b
Compare
b1fec5b to
5af710c
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1109
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
5af710c to
1050ef1
Compare
1050ef1 to
a1fddf0
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1109
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
a1fddf0 to
2137136
Compare
2137136 to
145e2cf
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1109
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
145e2cf to
7228798
Compare
7228798 to
47dd078
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1109
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
26c7cfc to
96db1d0
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1109
Scan targets checked: wolfssh-bugs, wolfssh-src
No new issues found in the changed files. ✅
9de220f to
b7d471c
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1109
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
b7d471c to
a75a2ff
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1109
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 2
1 finding(s) posted as inline comments (see file-level comments below)
Medium (1)
wc_MlDsaKey_ImportPubRaw result silently discarded by subsequent wc_MlDsaKey_MakeKeyFromSeed in GetOpenSshKeyMlDsaComposite
Function: GetOpenSshKeyMlDsaComposite
Category: Logic errors
The function calls wc_MlDsaKey_ImportPubRaw to import the ML-DSA public key from the OpenSSH key file, then immediately calls wc_MlDsaKey_MakeKeyFromSeed on the same key object. Per the ML-DSA specification, MakeKeyFromSeed derives a fresh key pair (both private and public halves) from the 32-byte seed, overwriting any previously imported public key material. The result of ImportPubRaw has no lasting effect on the final key state: the derived public key is used unconditionally, and the file-embedded public key is never compared against it. A key file whose stored public key does not match the seed would be silently accepted, and the mismatch would go undetected.
Recommendation: Remove the wc_MlDsaKey_ImportPubRaw call before MakeKeyFromSeed, as it has no effect. To preserve integrity validation, call wc_MlDsaKey_MakeKeyFromSeed first, then export the derived public key with wc_MlDsaKey_ExportPubRaw and compare it byte-for-byte against the pub bytes from the file. If they differ, return WS_KEY_FORMAT_E to flag a corrupted or mismatched key file.
This review was generated automatically by Fenrir. Findings are non-blocking.
a75a2ff to
11a13f2
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1109
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
| if (wc_ecc_sig_to_rs(asnSig, asnSigSz, rBuf, &rSz, sBuf, | ||
| &sSz) != 0) { result = -734; goto done; } | ||
| off = 0; | ||
| MlDsaTest_PutLen(tradSig + off, rSz); off += UINT32_SZ; |
There was a problem hiding this comment.
🔵 [Low] ECC test signature omits r/s pad bytes, diverging from production wire format · Missing edge-case coverage on a function the PR also changed
The test builds the ECC trad-signature as [rSz][rBuf][sSz][sBuf] with no leading-zero pad byte, but CompositeEccSign emits [rSz+rPad][0x00?][rBuf][sSz+sPad][0x00?][sBuf] when r[0]&0x80 or s[0]&0x80. CompositeEccVerify accepts both formats via wc_ecc_rs_raw_to_sig, so the test passes, but the padded case (the production format) is never exercised.
Fix: Mirror CompositeEccSign's padding logic: add rPad = (rBuf[0] & 0x80) ? 1 : 0 and emit [rSz+rPad][0x00?][rBuf] so the test exercises the actual production wire format.
Introduced support for ML-DSA composite signatures for hybrid post-quantum authentication.
Added all sig types as auth options into the test echoservers, new CI tests, and interop tested mldsa44-ed25519 wtih openssh-portable.