ML-DSA Composite Signature Support - #1109
Conversation
wolfSSL-Fenrir-bot
left a comment
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
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
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
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
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.
wolfSSL-Fenrir-bot
left a comment
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.
2daad5d to
51225eb
Compare
wolfSSL-Fenrir-bot
left a comment
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: 3
3 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
51225eb to
ec83417
Compare
ec83417 to
c820ef1
Compare
wolfSSL-Fenrir-bot
left a comment
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.
c820ef1 to
840e927
Compare
wolfSSL-Fenrir-bot
left a comment
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: 4
4 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
| static int test_CheckAuthKeysLineTypes(void) | ||
| { | ||
| static const char* types[] = { | ||
| "ssh-rsa", |
There was a problem hiding this comment.
🔵 [Low] New test hardcodes key-type names that NameIdMap omits in non-RSA/non-ECDSA builds · Logic errors
types[] lists ssh-rsa and the three ecdsa-sha2-nistp* names unguarded, but CheckAuthKeysLine() now sources valid types from wolfSSH_QueryKey()/NameIdMap, which omits them under WOLFSSH_NO_RSA / WOLFSSH_NO_ECDSA_SHA2_NISTP*. The test then returns WS_FATAL_ERROR on a correct build. ssh-ed25519 is guarded; these are not.
Fix: Wrap each entry in the same guard NameIdMap uses (WOLFSSH_NO_RSA, WOLFSSH_NO_ECDSA_SHA2_NISTP256/384/521, WOLFSSH_NO_SSH_RSA_SHA1 for x509v3-ssh-rsa).
| return BuildAuthKeysLineType("ssh-rsa", key, keySz, lineOut, lineOutSz); | ||
| } | ||
|
|
||
| /* Confirms every key-type string in CheckAuthKeysLine's allowedTypes[] table |
There was a problem hiding this comment.
⚪ [Info] Test comment references allowedTypes[] table removed by this PR · Dead/unreachable code
The comment states the test confirms every string in CheckAuthKeysLine's allowedTypes[] table, but this PR deleted that table in favor of wolfSSH_QueryKey(). The stated invariant no longer exists, obscuring what the test actually asserts.
Fix: Reword the comment to describe the wolfSSH_QueryKey()/NameIdMap lookup the test now exercises.
| static int test_CheckAuthKeysLineTypes(void) | ||
| { | ||
| static const char* types[] = { | ||
| "ssh-rsa", |
There was a problem hiding this comment.
🔵 [Low] test_CheckAuthKeysLineTypes asserts key types that the new registry-driven check gates out · Hardcoded paths, ports, or environment dependencies
CheckAuthKeysLine() now derives accepted types from NameIdMap via wolfSSH_QueryKey(), which gates ssh-rsa on WOLFSSH_NO_RSA, ecdsa-sha2-nistp* on the per-curve macros, and x509v3-ssh-rsa on WOLFSSH_NO_SSH_RSA_SHA1. The test lists those names unconditionally, so it fails on builds with RSA, SHA-1 RSA, or an ECC curve disabled.
Fix: Wrap the RSA, ECDSA-curve, and x509v3 entries in the same #ifndef WOLFSSH_NO_RSA / WOLFSSH_NO_ECDSA_SHA2_NISTP* / WOLFSSH_NO_SSH_RSA_SHA1 guards used by NameIdMap.
| if (ret == WSSHD_AUTH_SUCCESS) { | ||
| for (i = 0; i < NUM_ALLOWED_TYPES; ++i) { | ||
| if (WSTRCMP(type, allowedTypes[i]) == 0) { | ||
| while ((algoName = wolfSSH_QueryKey(&queryIdx)) != NULL) { |
There was a problem hiding this comment.
⚪ [Info] authorized_keys type whitelist widened to every TYPE_KEY name, including certificate algorithms · Authentication bypass
Replacing the hand-maintained allowedTypes[] with wolfSSH_QueryKey() also admits x509v3-* and *-cert-v01@openssh.com names as authorized_keys/TrustedUserCAKeys line types. Impact is contained because the entry still has to byte-match the offered blob, and certificate auth is routed to the CA path before this scanner runs.
Related known finding #6814 (similar but distinct): Both affect public-key authorization through authorized_keys processing, but #6814 removes SearchForPubKey’s no-match rejection gate, whereas this changes CheckAuthKeysLine’s accepted algorithm-name whitelist. The root causes and faulting operations differ, and each requires a separate patch.
Fix: Filter the queried names to non-certificate key types so an authorized_keys line naming a certificate algorithm remains rejected outright.
3 participants
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.