internal: sign Ed25519 user-auth requests through the agent - #1196
Open
yosuke-wolfssl wants to merge 1 commit into
Open
internal: sign Ed25519 user-auth requests through the agent#1196yosuke-wolfssl wants to merge 1 commit into
yosuke-wolfssl wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes agent-backed Ed25519 publickey authentication by ensuring SSH_MSG_USERAUTH_REQUEST includes the required trailing signature when the private key lives in an ssh-agent, and adds regression tests to validate both the success and error-propagation paths.
Changes:
- Implement Ed25519 agent signing in
BuildUserAuthRequestEd25519()by callingwolfSSH_AGENT_SignRequest()and length-prefixing the returned signature blob. - Update the Ed25519 prepare path comment to reflect agent behavior (no private key loaded locally).
- Add two regress tests that (1) assert a signature is emitted and (2) assert agent errors propagate without writing a partial request.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/regress.c | Adds a mock-agent harness and regression tests validating Ed25519 agent-signed USERAUTH_REQUEST packets and error propagation. |
| src/internal.c | Implements the Ed25519 agent signing branch so signed publickey auth requests are well-formed when using an agent. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
yosuke-wolfssl
force-pushed
the
fix/f_11660
branch
from
August 25, 2026 02:06
bbcb822 to
a87f10e
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1196
Scan targets checked: wolfssh-bugs, wolfssh-src
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
- BuildUserAuthRequestEd25519() signs with wolfSSH_AGENT_SignRequest() when the agent is enabled, writing the returned signature blob length-prefixed into the reserved payload and advancing idx past it. The capacity handed to the agent is the room the prepare phase set aside: two lengths plus the signature and public key type sizes. - The buffer the local signing path fills is allocated in that path rather than at the top of the function; sig starts NULL and the small-stack free at the tail already null-checks it. - PrepareUserAuthRequestEd25519() notes that the agent holds the private key and loads none locally. - tests/regress.c gains TestAgentEd25519UserAuthEmitsSignature(), TestAgentEd25519UserAuthPropagatesAgentError() and TestAgentEd25519UserAuthRejectsOversizeSignature(), which drive SendUserAuthRequest() over a mock agent and parse the emitted USERAUTH_REQUEST down to its signature field. - InitAgentEd25519Ctx() takes the signature size the mock agent answers with, so a caller can hand back a blob past the capacity. - ParsePayloadLen() and BuildExtInfoSigAlgs() move to the shared test helper section so the new tests and the existing callers share them. Issue: F-11660
yosuke-wolfssl
force-pushed
the
fix/f_11660
branch
from
August 26, 2026 02:03
a87f10e to
7b9340a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In
WOLFSSH_AGENTbuilds withssh->agentEnabled,BuildUserAuthRequestEd25519()left its agent branch as a/* XXX: Pending */placeholder that wrote nothing and returnedWS_SUCCESS. A client authenticating with an Ed25519 key held by an ssh-agent emitted aSSH_MSG_USERAUTH_REQUESTwhose trailingstring signaturefield was simply absent — a malformed signed publickey request — rather than failing. RSA and ECDSA already implemented this path correctly. Closes 11660.Fix (
src/internal.c)BuildUserAuthRequestEd25519()now callswolfSSH_AGENT_SignRequest()and writes the returned blob length-prefixed into the reserved payload. That isBuildUserAuthRequestRsa()'s shape — signing straight intooutputrather than into a local buffer asBuildUserAuthRequestEcc()does — with the capacity corrected.The capacity handed to the agent is the room the prepare phase actually set aside, not the bare signature size:
PrepareUserAuthRequestEd25519()LENGTH_SZ*3 + 64 + 11= 87ssh-ed25519blob the agent returns4+11+4+64= 83The two terms track each other for any signature and algorithm-name size, not just 64 and 11: the capacity is the reservation minus the one length prefix the builder writes itself.
wolfSSH_AGENT_SignRequest()treats*sigSzas the caller's buffer capacity and returnsWS_BUFFER_Eabove it, so an over-long blob is refused with nothing written.PrepareUserAuthRequestEd25519()needed no functional change —wc_ed25519_sig_size()is constant, so its reservation was already right for the agent case. Only the placeholder comment changed.The signature buffer the local signing path fills now gets allocated in that path instead of at the top of the function, so the agent path no longer takes a
WOLFSSH_SMALL_STACKallocation it never reads.Tests (
tests/regress.c)Three tests, built on the file's existing
MemIotransport capture,Append*builders, and the library's ownGetStringRef()/GetBoolean()parsers:TestAgentEd25519UserAuthEmitsSignature()drivesSendUserAuthRequest()over a mock agent and walks the emitted packet down to its signature field.TestAgentEd25519UserAuthPropagatesAgentError()gives the agent no response and assertsWS_AGENT_NO_KEY_Ereaches the caller with nothing written to the transport.TestAgentEd25519UserAuthRejectsOversizeSignature()has the agent answer one byte past the capacity and assertsWS_BUFFER_Ewith nothing written — pinning the bound from above.All three are mutation-verified: under-declaring the capacity as
keySig->sigSzfails the first, swallowing the sign-request error fails the second, and deleting the size check inwolfSSH_AGENT_SignRequest()fails the third.Verification
tests/regress.test,unit,api,kex,auth,testsuiteall pass.scripts/{sftp,scp}.testflake under parallelmake checkand pass serially.-Werrorclean across 6 build configurations, includingWOLFSSH_SMALL_STACKand the Zephyr defines.regressandunit.Not in this PR
RSA agent user auth is broken independently, and by two separate defects. The first failure is in
PrepareUserAuthRequestRsa(), which handswc_RsaPublicKeyDecode()— an ASN.1 DER parser — the SSH wire blobstring "ssh-rsa", mpint e, mpint n, yielding a rawASN_PARSE_E(-140) out through the wolfSSH API. Only behind that does the second defect bite:BuildUserAuthRequestRsa()andBuildUserAuthRequestRsaCert()pass&keySig->sigSzas the agent capacity, under-declaring it by the algorithm-name prefix (271 vs 256 for a 2048-bit key). A fix needsGetOpenSshPublicKey(), as theWOLFSSH_TPMbranch directly below already uses, and the capacity shape this PR uses for Ed25519. Tracked separately.