Skip to content

Commit 1c46f1d

Browse files
Add service-name check and regress test
1 parent fdf621c commit 1c46f1d

3 files changed

Lines changed: 105 additions & 2 deletions

File tree

src/internal.c

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8367,6 +8367,7 @@ static int DoUserAuthRequest(WOLFSSH* ssh,
83678367
word32 begin;
83688368
int ret = WS_SUCCESS;
83698369
byte authNameId;
8370+
byte serviceValid = 1;
83708371
WS_UserAuthData authData;
83718372

83728373
WLOG(WS_LOG_DEBUG, "Entering DoUserAuthRequest()");
@@ -8401,10 +8402,19 @@ static int DoUserAuthRequest(WOLFSSH* ssh,
84018402
authData.serviceName = buf + begin;
84028403
begin += authData.serviceNameSz;
84038404

8404-
ret = GetSize(&authData.authNameSz, buf, len, &begin);
8405+
if (NameToId((const char*)authData.serviceName, authData.serviceNameSz)
8406+
!= ID_SERVICE_CONNECTION) {
8407+
WLOG(WS_LOG_DEBUG, "DUAR: Invalid service name");
8408+
serviceValid = 0;
8409+
ret = SendUserAuthFailure(ssh, 0);
8410+
*idx = len;
8411+
}
8412+
else {
8413+
ret = GetSize(&authData.authNameSz, buf, len, &begin);
8414+
}
84058415
}
84068416

8407-
if (ret == WS_SUCCESS) {
8417+
if (ret == WS_SUCCESS && serviceValid) {
84088418
authData.authName = buf + begin;
84098419
begin += authData.authNameSz;
84108420
authNameId = NameToId((char*)authData.authName, authData.authNameSz);
@@ -10773,6 +10783,12 @@ int wolfSSH_TestChannelPutData(WOLFSSH_CHANNEL* channel, byte* data,
1077310783
{
1077410784
return ChannelPutData(channel, data, dataSz);
1077510785
}
10786+
10787+
int wolfSSH_TestDoUserAuthRequest(WOLFSSH* ssh, byte* buf, word32 len,
10788+
word32* idx)
10789+
{
10790+
return DoUserAuthRequest(ssh, buf, len, idx);
10791+
}
1077610792
#endif
1077710793

1077810794

tests/unit.c

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -918,6 +918,86 @@ static int test_DoChannelRequest(void)
918918
return result;
919919
}
920920

921+
/* IO send callback that discards all output (returns sz as if sent).
922+
* Used by tests that call functions which send packets but have no real
923+
* socket — e.g., SendUserAuthFailure in test_DoUserAuthRequest_serviceName. */
924+
static int discardSend(WOLFSSH* ssh, void* buf, word32 sz, void* ctx)
925+
{
926+
(void)ssh; (void)buf; (void)ctx;
927+
return (int)sz;
928+
}
929+
930+
/* Verify DoUserAuthRequest rejects non-"ssh-connection" service names per
931+
* RFC 4252 Section 5. Invalid names must:
932+
* - send SSH_MSG_USERAUTH_FAILURE (partial_success = false)
933+
* - return WS_SUCCESS so the connection stays open for retry
934+
* - set *idx = len to consume the entire payload (keeps buffer aligned)
935+
* Valid "ssh-connection" must proceed to the auth-method dispatch normally. */
936+
static int test_DoUserAuthRequest_serviceName(void)
937+
{
938+
WOLFSSH_CTX* ctx = NULL;
939+
WOLFSSH* ssh = NULL;
940+
int result = 0;
941+
struct {
942+
const char* svcName;
943+
word32 svcNameSz;
944+
int expectRet;
945+
const char* label;
946+
} cases[] = {
947+
{ "ssh-connection", 14, WS_SUCCESS, "valid service name" },
948+
{ "ssh-agent", 9, WS_SUCCESS, "ssh-agent rejected conn open" },
949+
{ "bad", 3, WS_SUCCESS, "unknown name rejected conn open" },
950+
};
951+
int i;
952+
953+
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
954+
if (ctx == NULL) return -500;
955+
wolfSSH_SetIOSend(ctx, discardSend);
956+
957+
ssh = wolfSSH_new(ctx);
958+
if (ssh == NULL) { wolfSSH_CTX_free(ctx); return -501; }
959+
960+
for (i = 0; i < (int)(sizeof(cases)/sizeof(cases[0])); i++) {
961+
byte buf[128];
962+
word32 len = 0, idx = 0;
963+
word32 snsz = cases[i].svcNameSz;
964+
int ret;
965+
966+
/* username: "user" (4 bytes) */
967+
buf[len++] = 0; buf[len++] = 0; buf[len++] = 0; buf[len++] = 4;
968+
WMEMCPY(buf + len, "user", 4); len += 4;
969+
970+
/* service name */
971+
buf[len++] = (byte)(snsz >> 24); buf[len++] = (byte)(snsz >> 16);
972+
buf[len++] = (byte)(snsz >> 8); buf[len++] = (byte)snsz;
973+
WMEMCPY(buf + len, cases[i].svcName, snsz); len += snsz;
974+
975+
/* auth method: "none" */
976+
buf[len++] = 0; buf[len++] = 0; buf[len++] = 0; buf[len++] = 4;
977+
WMEMCPY(buf + len, "none", 4); len += 4;
978+
979+
ret = wolfSSH_TestDoUserAuthRequest(ssh, buf, len, &idx);
980+
981+
if (ret != cases[i].expectRet) {
982+
printf("DoUserAuthRequest_svcName[%s]: ret=%d expected=%d\n",
983+
cases[i].label, ret, cases[i].expectRet);
984+
result = -502 - i;
985+
break;
986+
}
987+
988+
if (i > 0 && idx != len) {
989+
printf("DoUserAuthRequest_svcName[%s]: idx=%u expected len=%u\n",
990+
cases[i].label, idx, len);
991+
result = -510 - i;
992+
break;
993+
}
994+
}
995+
996+
wolfSSH_free(ssh);
997+
wolfSSH_CTX_free(ctx);
998+
return result;
999+
}
1000+
9211001
#if !defined(WOLFSSH_NO_RSA)
9221002

9231003
/* 2048-bit RSA private key (PKCS#1 DER).
@@ -1210,6 +1290,11 @@ int wolfSSH_UnitTest(int argc, char** argv)
12101290
unitResult = test_ChannelPutData();
12111291
printf("ChannelPutData: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED"));
12121292
testResult = testResult || unitResult;
1293+
1294+
unitResult = test_DoUserAuthRequest_serviceName();
1295+
printf("DoUserAuthRequest_serviceName: %s\n",
1296+
(unitResult == 0 ? "SUCCESS" : "FAILED"));
1297+
testResult = testResult || unitResult;
12131298
#endif
12141299

12151300
#ifdef WOLFSSH_KEYGEN

wolfssh/internal.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,6 +1338,8 @@ enum WS_MessageIdLimits {
13381338
WOLFSSH_API int wolfSSH_TestDoKexDhInit(WOLFSSH* ssh, byte* buf,
13391339
word32 len, word32* idx);
13401340
WOLFSSH_API int wolfSSH_TestChannelPutData(WOLFSSH_CHANNEL*, byte*, word32);
1341+
WOLFSSH_API int wolfSSH_TestDoUserAuthRequest(WOLFSSH* ssh, byte* buf,
1342+
word32 len, word32* idx);
13411343
#ifndef WOLFSSH_NO_DH_GEX_SHA256
13421344
WOLFSSH_API int wolfSSH_TestDoKexDhGexRequest(WOLFSSH* ssh, byte* buf,
13431345
word32 len, word32* idx);

0 commit comments

Comments
 (0)