From dc748ca81bb5381f1bef9ccfd866b71f6a955a4f Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Tue, 8 Sep 2026 14:34:17 +0900 Subject: [PATCH] internal: gate auth-agent channel opens on the client's request - DoChannelOpen refuses an auth-agent open on a server endpoint, and on a client with the agent disabled or connectState below CONNECT_CLIENT_CHANNEL_AGENT_REQUEST_SENT. - Both answer OPEN_ADMINISTRATIVELY_PROHIBITED; the ssh->agent check stays as the resource check behind them. - Cover the refusals before the request, with the agent disabled, over an accepting channelOpenCb, and on a server, plus the accepted open past the request; each refusal asserts ssh->error stays clean. - Move TestAgentChannelNullAgentSendsOpenFail to a client harness, and have the agent tests set the agent flag explicitly. Issue: F-13389 --- src/internal.c | 17 ++++- tests/regress.c | 197 +++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 193 insertions(+), 21 deletions(-) diff --git a/src/internal.c b/src/internal.c index b2153e323..ebbe2f28d 100644 --- a/src/internal.c +++ b/src/internal.c @@ -12379,7 +12379,22 @@ static int DoChannelOpen(WOLFSSH* ssh, #ifdef WOLFSSH_AGENT case ID_CHANTYPE_AUTH_AGENT: WLOG(WS_LOG_INFO, "agent = %p", ssh->agent); - if (ssh->agent != NULL) + /* An auth-agent open answers a client's auth-agent-req, so + * only a client takes one */ + if (ssh->ctx->side == WOLFSSH_ENDPOINT_SERVER) { + WLOG(WS_LOG_DEBUG, "Rejecting auth-agent channel open " + "received by a server (wrong direction)"); + ret = WS_INVALID_CHANTYPE; + fail_reason = OPEN_ADMINISTRATIVELY_PROHIBITED; + } + else if (!ssh->agentEnabled || ssh->connectState + < CONNECT_CLIENT_CHANNEL_AGENT_REQUEST_SENT) { + WLOG(WS_LOG_DEBUG, "Rejecting auth-agent channel open " + "before requesting agent forwarding"); + ret = WS_ERROR; + fail_reason = OPEN_ADMINISTRATIVELY_PROHIBITED; + } + else if (ssh->agent != NULL) ssh->agent->channel = peerChannelId; else ret = WS_AGENT_NULL_E; diff --git a/tests/regress.c b/tests/regress.c index c8c3016de..25ed0e793 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -2525,9 +2525,176 @@ static void TestSessionOnClientBeatsOpenCb(void) FreeChannelOpenHarness(&harness); } + +#ifdef WOLFSSH_AGENT +/* ssh->agent is allocated during userauth for agent-backed publickey, so its + * presence alone must not admit the channel. */ +static void TestAgentOpenBeforeRequestFails(void) +{ + ChannelOpenHarness harness; + byte in[128]; + word32 inSz; + int ret; + + inSz = BuildChannelOpenPacket("auth-agent@openssh.com", 9, 0x4000, 0x8000, + NULL, 0, in, sizeof(in)); + + InitChannelOpenHarnessClient(&harness, in, inSz); + AssertNotNull(harness.ssh->agent = + wolfSSH_AGENT_new(harness.ctx->heap)); + AssertIntEQ(wolfSSH_AGENT_enable(harness.ssh, 1), WS_SUCCESS); + + ret = DoReceive(harness.ssh); + AssertChannelOpenFailResponse(&harness, ret); + AssertIntEQ(ParseChannelOpenFailReason(harness.io.out, harness.io.outSz), + OPEN_ADMINISTRATIVELY_PROHIBITED); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); + + FreeChannelOpenHarness(&harness); +} + +/* The request check runs ahead of the open policy hook, so a registered + * channelOpenCb cannot accept an unrequested agent channel. */ +static void TestAgentOpenBeforeRequestBeatsOpenCb(void) +{ + ChannelOpenHarness harness; + byte in[128]; + word32 inSz; + int ret; + + inSz = BuildChannelOpenPacket("auth-agent@openssh.com", 9, 0x4000, 0x8000, + NULL, 0, in, sizeof(in)); + + InitChannelOpenHarnessClient(&harness, in, inSz); + AssertNotNull(harness.ssh->agent = + wolfSSH_AGENT_new(harness.ctx->heap)); + AssertIntEQ(wolfSSH_AGENT_enable(harness.ssh, 1), WS_SUCCESS); + AssertIntEQ(wolfSSH_CTX_SetChannelOpenCb(harness.ctx, AcceptChannelOpenCb), + WS_SUCCESS); + + ret = DoReceive(harness.ssh); + AssertChannelOpenFailResponse(&harness, ret); + AssertIntEQ(ParseChannelOpenFailReason(harness.io.out, harness.io.outSz), + OPEN_ADMINISTRATIVELY_PROHIBITED); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); + + FreeChannelOpenHarness(&harness); +} + +/* A client that never allocated an agent has nothing to hand the channel to, + * so the open fails on the agent check past the request state. */ +static void TestAgentChannelNullAgentSendsOpenFail(void) +{ + ChannelOpenHarness harness; + byte in[128]; + word32 inSz; + int ret; + + inSz = BuildChannelOpenPacket("auth-agent@openssh.com", 11, 0x4000, + 0x8000, NULL, 0, in, sizeof(in)); + + InitChannelOpenHarnessClient(&harness, in, inSz); + AssertIntEQ(wolfSSH_AGENT_enable(harness.ssh, 1), WS_SUCCESS); + harness.ssh->connectState = CONNECT_CLIENT_CHANNEL_AGENT_REQUEST_SENT; + AssertTrue(harness.ssh->agent == NULL); + + ret = DoReceive(harness.ssh); + AssertChannelOpenFailResponse(&harness, ret); + AssertIntEQ(ParseChannelOpenFailReason(harness.io.out, harness.io.outSz), + OPEN_ADMINISTRATIVELY_PROHIBITED); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); + + FreeChannelOpenHarness(&harness); +} + +/* wolfSSH_AGENT_enable(ssh, 0) clears the flag and leaves ssh->agent + * allocated, and connectState advances whether or not the request went out. */ +static void TestAgentOpenWithAgentDisabledFails(void) +{ + ChannelOpenHarness harness; + byte in[128]; + word32 inSz; + int ret; + + inSz = BuildChannelOpenPacket("auth-agent@openssh.com", 9, 0x4000, 0x8000, + NULL, 0, in, sizeof(in)); + + InitChannelOpenHarnessClient(&harness, in, inSz); + AssertNotNull(harness.ssh->agent = + wolfSSH_AGENT_new(harness.ctx->heap)); + AssertIntEQ(wolfSSH_AGENT_enable(harness.ssh, 0), WS_SUCCESS); + harness.ssh->connectState = CONNECT_CLIENT_CHANNEL_AGENT_REQUEST_SENT; + + ret = DoReceive(harness.ssh); + AssertChannelOpenFailResponse(&harness, ret); + AssertIntEQ(ParseChannelOpenFailReason(harness.io.out, harness.io.outSz), + OPEN_ADMINISTRATIVELY_PROHIBITED); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); + + FreeChannelOpenHarness(&harness); +} + +/* Once the request has gone out, the open is confirmed and the agent takes + * the peer's channel id. */ +static void TestAgentOpenAfterRequestSucceeds(void) +{ + ChannelOpenHarness harness; + byte in[128]; + word32 inSz; + int ret; + + inSz = BuildChannelOpenPacket("auth-agent@openssh.com", 9, 0x4000, 0x8000, + NULL, 0, in, sizeof(in)); + + InitChannelOpenHarnessClient(&harness, in, inSz); + AssertNotNull(harness.ssh->agent = + wolfSSH_AGENT_new(harness.ctx->heap)); + AssertIntEQ(wolfSSH_AGENT_enable(harness.ssh, 1), WS_SUCCESS); + harness.ssh->connectState = CONNECT_CLIENT_CHANNEL_AGENT_REQUEST_SENT; + + ret = DoReceive(harness.ssh); + AssertIntEQ(ret, WS_SUCCESS); + AssertIntEQ(ParseMsgId(harness.io.out, harness.io.outSz), + MSGID_CHANNEL_OPEN_CONF); + AssertIntEQ(harness.ssh->channelListSz, 1); + AssertIntEQ(harness.ssh->agent->channel, 9); + + FreeChannelOpenHarness(&harness); +} +#endif /* WOLFSSH_AGENT */ #endif /* !NO_WOLFSSH_CLIENT */ +#if defined(WOLFSSH_AGENT) && !defined(NO_WOLFSSH_SERVER) +/* A server sets ssh->agent when a client asks for agent forwarding. Setting + * connectState too leaves the endpoint side as the only reason to refuse. */ +static void TestAgentOpenOnServerFails(void) +{ + ChannelOpenHarness harness; + byte in[128]; + word32 inSz; + int ret; + + inSz = BuildChannelOpenPacket("auth-agent@openssh.com", 9, 0x4000, 0x8000, + NULL, 0, in, sizeof(in)); + + InitChannelOpenHarness(&harness, in, inSz); + AssertNotNull(harness.ssh->agent = + wolfSSH_AGENT_new(harness.ctx->heap)); + AssertIntEQ(wolfSSH_AGENT_enable(harness.ssh, 1), WS_SUCCESS); + harness.ssh->connectState = CONNECT_CLIENT_CHANNEL_AGENT_REQUEST_SENT; + + ret = DoReceive(harness.ssh); + AssertChannelOpenFailResponse(&harness, ret); + AssertIntEQ(ParseChannelOpenFailReason(harness.io.out, harness.io.outSz), + OPEN_ADMINISTRATIVELY_PROHIBITED); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); + + FreeChannelOpenHarness(&harness); +} +#endif /* WOLFSSH_AGENT && !NO_WOLFSSH_SERVER */ + + #ifndef NO_WOLFSSH_SERVER /* The client gate tests above run against a client endpoint, so the server * branch of IsMessageAllowed was never exercised. The tests below drive a @@ -4419,25 +4586,6 @@ static void TestRequestSuccessWithPortParsesCorrectly(void) #endif #ifdef WOLFSSH_AGENT -static void TestAgentChannelNullAgentSendsOpenFail(void) -{ - ChannelOpenHarness harness; - byte in[128]; - word32 inSz; - int ret; - - inSz = BuildChannelOpenPacket("auth-agent@openssh.com", 11, 0x4000, - 0x8000, NULL, 0, in, sizeof(in)); - - InitChannelOpenHarness(&harness, in, inSz); - AssertTrue(harness.ssh->agent == NULL); - - ret = DoReceive(harness.ssh); - AssertChannelOpenFailResponse(&harness, ret); - - FreeChannelOpenHarness(&harness); -} - /* Nothing asked for forwarding, so the open is refused rather than started. * The refusal is the documented answer to a poll, so it must not land in * ssh->error: wolfSSH_accept() would then abort with WS_INVALID_STATE_E. */ @@ -13552,6 +13700,16 @@ int main(int argc, char** argv) #ifndef NO_WOLFSSH_CLIENT TestSessionOnClientSendsOpenFail(); TestSessionOnClientBeatsOpenCb(); +#ifdef WOLFSSH_AGENT + TestAgentOpenBeforeRequestFails(); + TestAgentOpenBeforeRequestBeatsOpenCb(); + TestAgentChannelNullAgentSendsOpenFail(); + TestAgentOpenWithAgentDisabledFails(); + TestAgentOpenAfterRequestSucceeds(); +#endif +#endif +#if defined(WOLFSSH_AGENT) && !defined(NO_WOLFSSH_SERVER) + TestAgentOpenOnServerFails(); #endif #ifndef NO_WOLFSSH_SERVER TestServerChannelBlockedBeforeAuth(serverSsh); @@ -13614,7 +13772,6 @@ int main(int argc, char** argv) TestRequestSuccessWithPortParsesCorrectly(); #endif #ifdef WOLFSSH_AGENT - TestAgentChannelNullAgentSendsOpenFail(); TestAgentChannelOpenWithoutRequest(); TestAgentChannelOpenFlushesQueuedOpen(); TestAgentChannelOpenAfterDisconnect();