mirror of https://github.com/wolfSSL/wolfssh.git
fix: reject service messages while keying
RFC 4253 section 7.1 bars SERVICE_REQUEST and SERVICE_ACCEPT between a KEXINIT and the matching NEWKEYS. The admission check now consults ssh->isKeying, which acceptState and connectState do not track. It tests WOLFSSH_PEER_IS_KEYING, as the KEXINIT gate below it does: a peer that has not seen our KEXINIT yet may still legally send. - gate MSGID_SERVICE_REQUEST on isKeying in IsMessageAllowedServer() - gate MSGID_SERVICE_ACCEPT on isKeying in IsMessageAllowedClient() - record the refusals as WS_MSGID_NOT_ALLOWED_E, the code DoReceive() stores anyway, rather than WS_REKEYING, a soft retry-later status - add unit and receive-path coverage in tests/regress.c Issue: F-10569pull/1202/head
parent
5fb7be429b
commit
89b8055a0a
|
|
@ -710,6 +710,15 @@ INLINE static int IsMessageAllowedServer(WOLFSSH *ssh, byte msg)
|
|||
}
|
||||
|
||||
if (msg == MSGID_SERVICE_REQUEST) {
|
||||
/* RFC 4253 section 7.1: no service request once the peer's KEXINIT
|
||||
* lands. acceptState doesn't track a rekey. */
|
||||
if (ssh->isKeying & WOLFSSH_PEER_IS_KEYING) {
|
||||
WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
|
||||
msg, "server", "when keying");
|
||||
ssh->error = WS_MSGID_NOT_ALLOWED_E;
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (ssh->acceptState == ACCEPT_KEYED) {
|
||||
return 1;
|
||||
}
|
||||
|
|
@ -732,7 +741,7 @@ INLINE static int IsMessageAllowedServer(WOLFSSH *ssh, byte msg)
|
|||
if (msg == MSGID_KEXINIT) {
|
||||
WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
|
||||
msg, "server", "when keying");
|
||||
ssh->error = WS_REKEYING;
|
||||
ssh->error = WS_MSGID_NOT_ALLOWED_E;
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
|
@ -744,7 +753,7 @@ INLINE static int IsMessageAllowedServer(WOLFSSH *ssh, byte msg)
|
|||
WLOG(WS_LOG_DEBUG,
|
||||
"Message ID %u not the expected message %u",
|
||||
msg, ssh->handshake->expectMsgId);
|
||||
ssh->error = WS_REKEYING;
|
||||
ssh->error = WS_MSGID_NOT_ALLOWED_E;
|
||||
return 0;
|
||||
}
|
||||
else {
|
||||
|
|
@ -823,6 +832,15 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
|
|||
}
|
||||
|
||||
if (msg == MSGID_SERVICE_ACCEPT) {
|
||||
/* RFC 4253 section 7.1: no service accept once the peer's KEXINIT
|
||||
* lands. connectState doesn't track a rekey. */
|
||||
if (ssh->isKeying & WOLFSSH_PEER_IS_KEYING) {
|
||||
WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
|
||||
msg, "client", "when keying");
|
||||
ssh->error = WS_MSGID_NOT_ALLOWED_E;
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (ssh->connectState == CONNECT_CLIENT_USERAUTH_REQUEST_SENT) {
|
||||
return 1;
|
||||
}
|
||||
|
|
@ -845,7 +863,7 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
|
|||
if (msg == MSGID_KEXINIT) {
|
||||
WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
|
||||
msg, "client", "when keying");
|
||||
ssh->error = WS_REKEYING;
|
||||
ssh->error = WS_MSGID_NOT_ALLOWED_E;
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
|
@ -857,7 +875,7 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
|
|||
WLOG(WS_LOG_DEBUG,
|
||||
"Message ID %u not the expected message %u",
|
||||
msg, ssh->handshake->expectMsgId);
|
||||
ssh->error = WS_REKEYING;
|
||||
ssh->error = WS_MSGID_NOT_ALLOWED_E;
|
||||
return 0;
|
||||
}
|
||||
else {
|
||||
|
|
|
|||
|
|
@ -2167,6 +2167,39 @@ static void TestChannelAllowedAfterAuth(WOLFSSH* ssh)
|
|||
}
|
||||
|
||||
|
||||
/* A service accept is allowed only when no key exchange is in flight.
|
||||
* connectState does not see a rekey, so isKeying is checked too. */
|
||||
static void TestClientServiceAcceptBlockedDuringKeying(WOLFSSH* ssh)
|
||||
{
|
||||
int allowed;
|
||||
|
||||
ResetSession(ssh);
|
||||
ssh->connectState = CONNECT_CLIENT_USERAUTH_REQUEST_SENT;
|
||||
|
||||
allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_SERVICE_ACCEPT,
|
||||
WS_MSG_RECV);
|
||||
AssertTrue(allowed);
|
||||
|
||||
/* Same connectState, peer's rekey KEXINIT arrived first. */
|
||||
ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
|
||||
|
||||
allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_SERVICE_ACCEPT,
|
||||
WS_MSG_RECV);
|
||||
AssertFalse(allowed);
|
||||
AssertIntEQ(ssh->error, WS_MSGID_NOT_ALLOWED_E);
|
||||
|
||||
/* Allowed when only this side has started a rekey: RFC 4253 section 7.1
|
||||
* requires tolerating what the peer sent before it saw our KEXINIT. */
|
||||
ResetSession(ssh);
|
||||
ssh->connectState = CONNECT_CLIENT_USERAUTH_REQUEST_SENT;
|
||||
ssh->isKeying = WOLFSSH_SELF_IS_KEYING;
|
||||
|
||||
allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_SERVICE_ACCEPT,
|
||||
WS_MSG_RECV);
|
||||
AssertTrue(allowed);
|
||||
}
|
||||
|
||||
|
||||
/* Drive the whole receive path with a CHANNEL_OPEN sent from a pre-auth
|
||||
* connectState: no channel created, no reply emitted. The connectState
|
||||
* gate does the rejecting; isKeying below is scene-setting only. */
|
||||
|
|
@ -2325,6 +2358,64 @@ static void TestServerServiceRequestStateGated(WOLFSSH* ssh)
|
|||
}
|
||||
|
||||
|
||||
/* Drive the receive path with a SERVICE_REQUEST arriving mid-rekey.
|
||||
* acceptState sits at ACCEPT_KEYED throughout, so only isKeying catches
|
||||
* it. */
|
||||
static void TestServerServiceRequestRejectedDuringKeying(void)
|
||||
{
|
||||
WOLFSSH_CTX* ctx;
|
||||
WOLFSSH* ssh;
|
||||
MemIo io;
|
||||
byte payload[64];
|
||||
byte pkt[128];
|
||||
byte out[128];
|
||||
word32 pktSz;
|
||||
word32 idx = 0;
|
||||
|
||||
idx = AppendString(payload, sizeof(payload), idx, "ssh-userauth");
|
||||
pktSz = WrapPacket(MSGID_SERVICE_REQUEST, payload, idx, pkt, sizeof(pkt));
|
||||
|
||||
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
|
||||
AssertNotNull(ctx);
|
||||
wolfSSH_SetIORecv(ctx, MemRecv);
|
||||
wolfSSH_SetIOSend(ctx, MemSend);
|
||||
|
||||
ssh = wolfSSH_new(ctx);
|
||||
AssertNotNull(ssh);
|
||||
|
||||
MemIoInit(&io, pkt, pktSz, out, sizeof(out));
|
||||
wolfSSH_SetIOReadCtx(ssh, &io);
|
||||
wolfSSH_SetIOWriteCtx(ssh, &io);
|
||||
|
||||
ssh->acceptState = ACCEPT_KEYED;
|
||||
/* The peer's rekey KEXINIT arrived, its NEWKEYS has not. */
|
||||
ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
|
||||
|
||||
AssertIntEQ(wolfSSH_TestDoReceive(ssh), WS_FATAL_ERROR);
|
||||
AssertIntEQ(ssh->error, WS_MSGID_NOT_ALLOWED_E);
|
||||
/* Not dispatched, so acceptState did not advance. */
|
||||
AssertIntEQ(ssh->clientState, CLIENT_BEGIN);
|
||||
AssertIntEQ(ssh->acceptState, ACCEPT_KEYED);
|
||||
/* Nothing emitted in reply. */
|
||||
AssertIntEQ(io.outSz, 0);
|
||||
|
||||
/* Allowed when only this side has started a rekey. */
|
||||
ssh->error = 0;
|
||||
ssh->isKeying = WOLFSSH_SELF_IS_KEYING;
|
||||
AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, MSGID_SERVICE_REQUEST,
|
||||
WS_MSG_RECV));
|
||||
|
||||
/* Allowed once the key exchange is done. */
|
||||
ssh->error = 0;
|
||||
ssh->isKeying = 0;
|
||||
AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, MSGID_SERVICE_REQUEST,
|
||||
WS_MSG_RECV));
|
||||
|
||||
wolfSSH_free(ssh);
|
||||
wolfSSH_CTX_free(ctx);
|
||||
}
|
||||
|
||||
|
||||
static void TestChannelOpenCallbackRejectSendsOpenFail(void)
|
||||
{
|
||||
ChannelOpenHarness harness;
|
||||
|
|
@ -8501,6 +8592,7 @@ int main(int argc, char** argv)
|
|||
TestChannelBlockedBeforeAuth(ssh);
|
||||
TestChannelBlockedEveryPreAuthState(ssh);
|
||||
TestChannelAllowedAfterAuth(ssh);
|
||||
TestClientServiceAcceptBlockedDuringKeying(ssh);
|
||||
TestChannelOpenRejectedBeforeKex(CONNECT_CLIENT_KEXINIT_SENT);
|
||||
TestChannelOpenRejectedBeforeKex(CONNECT_CLIENT_KEXDH_INIT_SENT);
|
||||
#ifndef NO_WOLFSSH_SERVER
|
||||
|
|
@ -8509,6 +8601,7 @@ int main(int argc, char** argv)
|
|||
TestServerUserauthBlockedBeforeKeyed(serverSsh);
|
||||
TestServerOnlyUserauthMsgsBlocked(serverSsh);
|
||||
TestServerServiceRequestStateGated(serverSsh);
|
||||
TestServerServiceRequestRejectedDuringKeying();
|
||||
TestChannelOpenCallbackRejectSendsOpenFail();
|
||||
TestSecondSessionChannelRejected();
|
||||
TestUsernameChangeDisconnects();
|
||||
|
|
|
|||
Loading…
Reference in New Issue