mirror of https://github.com/wolfSSL/wolfssh.git
Reject password-change auth requests
- fail userauth when the request sets the password-change flag - do not invoke the userauth callback with the current password - parse the new-password field so the message is fully consumed - add negative unit test asserting USERAUTH_FAILURE and no callback Per RFC 4252 section 8, an expired password MUST NOT be used to authenticate; password changes remain unsupported. Issue: #1047 (6)pull/1086/head
parent
df635a3e16
commit
9e02350aaa
|
|
@ -7950,51 +7950,59 @@ static int DoUserAuthRequestPassword(WOLFSSH* ssh, WS_UserAuthData* authData,
|
|||
|
||||
if (ret == WS_SUCCESS) {
|
||||
if (pw->hasNewPassword) {
|
||||
/* Skip the password change. Maybe error out since we aren't
|
||||
* supporting password changes at this time. */
|
||||
/* Password changes are not supported. Parse the new password
|
||||
* field so the message is fully consumed, then reject the
|
||||
* request rather than authenticating with the current password
|
||||
* (RFC 4252 section 8: an expired password MUST NOT be used for
|
||||
* authentication). The userauth callback is not called. */
|
||||
ret = GetStringRef(&pw->newPasswordSz, &pw->newPassword,
|
||||
buf, len, &begin);
|
||||
if (ret == WS_SUCCESS) {
|
||||
WLOG(WS_LOG_DEBUG,
|
||||
"DUARPW: rejecting unsupported password change request");
|
||||
authFailure = 1;
|
||||
}
|
||||
}
|
||||
else {
|
||||
pw->newPassword = NULL;
|
||||
pw->newPasswordSz = 0;
|
||||
}
|
||||
|
||||
if (ssh->ctx->userAuthCb != NULL) {
|
||||
WLOG(WS_LOG_DEBUG, "DUARPW: Calling the userauth callback");
|
||||
ret = ssh->ctx->userAuthCb(WOLFSSH_USERAUTH_PASSWORD,
|
||||
authData, ssh->userAuthCtx);
|
||||
if (ret == WOLFSSH_USERAUTH_SUCCESS) {
|
||||
WLOG(WS_LOG_DEBUG, "DUARPW: password check success");
|
||||
ret = WS_SUCCESS;
|
||||
}
|
||||
else if (ret == WOLFSSH_USERAUTH_PARTIAL_SUCCESS) {
|
||||
WLOG(WS_LOG_DEBUG, "DUARPW: password check partial success");
|
||||
partialSuccess = 1;
|
||||
ret = WS_SUCCESS;
|
||||
}
|
||||
else if (ret == WOLFSSH_USERAUTH_REJECTED) {
|
||||
WLOG(WS_LOG_DEBUG, "DUARPW: password rejected");
|
||||
#ifndef NO_FAILURE_ON_REJECTED
|
||||
if (ssh->ctx->userAuthCb != NULL) {
|
||||
WLOG(WS_LOG_DEBUG, "DUARPW: Calling the userauth callback");
|
||||
ret = ssh->ctx->userAuthCb(WOLFSSH_USERAUTH_PASSWORD,
|
||||
authData, ssh->userAuthCtx);
|
||||
if (ret == WOLFSSH_USERAUTH_SUCCESS) {
|
||||
WLOG(WS_LOG_DEBUG, "DUARPW: password check success");
|
||||
ret = WS_SUCCESS;
|
||||
}
|
||||
else if (ret == WOLFSSH_USERAUTH_PARTIAL_SUCCESS) {
|
||||
WLOG(WS_LOG_DEBUG, "DUARPW: password check partial success");
|
||||
partialSuccess = 1;
|
||||
ret = WS_SUCCESS;
|
||||
}
|
||||
else if (ret == WOLFSSH_USERAUTH_REJECTED) {
|
||||
WLOG(WS_LOG_DEBUG, "DUARPW: password rejected");
|
||||
#ifndef NO_FAILURE_ON_REJECTED
|
||||
authFailure = 1;
|
||||
#endif
|
||||
authRejected = 1;
|
||||
ret = WS_USER_AUTH_E;
|
||||
}
|
||||
else if (ret == WOLFSSH_USERAUTH_WOULD_BLOCK) {
|
||||
WLOG(WS_LOG_DEBUG, "DUARPW: userauth callback would block");
|
||||
ret = WS_AUTH_PENDING;
|
||||
}
|
||||
else {
|
||||
WLOG(WS_LOG_DEBUG, "DUARPW: password check failed, retry");
|
||||
authFailure = 1;
|
||||
#endif
|
||||
authRejected = 1;
|
||||
ret = WS_USER_AUTH_E;
|
||||
}
|
||||
else if (ret == WOLFSSH_USERAUTH_WOULD_BLOCK) {
|
||||
WLOG(WS_LOG_DEBUG, "DUARPW: userauth callback would block");
|
||||
ret = WS_AUTH_PENDING;
|
||||
ret = WS_SUCCESS;
|
||||
}
|
||||
}
|
||||
else {
|
||||
WLOG(WS_LOG_DEBUG, "DUARPW: password check failed, retry");
|
||||
WLOG(WS_LOG_DEBUG, "DUARPW: No user auth callback");
|
||||
authFailure = 1;
|
||||
ret = WS_SUCCESS;
|
||||
}
|
||||
}
|
||||
else {
|
||||
WLOG(WS_LOG_DEBUG, "DUARPW: No user auth callback");
|
||||
authFailure = 1;
|
||||
}
|
||||
}
|
||||
|
||||
if (ret == WS_SUCCESS)
|
||||
|
|
|
|||
106
tests/unit.c
106
tests/unit.c
|
|
@ -2375,6 +2375,107 @@ done:
|
|||
}
|
||||
|
||||
|
||||
/* userauth callback that records whether it was invoked. Returns SUCCESS so
|
||||
* that, if it were ever reached for a password-change request, the request
|
||||
* would be (incorrectly) authenticated - making a missed rejection visible. */
|
||||
static int s_pwChangeCbCalled = 0;
|
||||
static int UnitAuthAlwaysSucceed(byte authType, WS_UserAuthData* authData,
|
||||
void* ctx)
|
||||
{
|
||||
(void)authData;
|
||||
(void)ctx;
|
||||
if (authType == WOLFSSH_USERAUTH_PASSWORD) {
|
||||
s_pwChangeCbCalled = 1;
|
||||
}
|
||||
return WOLFSSH_USERAUTH_SUCCESS;
|
||||
}
|
||||
|
||||
/* Verify DoUserAuthRequest rejects a password request that sets the
|
||||
* password-change flag (RFC 4252 Section 8: an expired password MUST NOT be
|
||||
* used for authentication). The request is otherwise well-formed and the
|
||||
* userauth callback would return SUCCESS, so a missing rejection would let the
|
||||
* old password authenticate. Asserts:
|
||||
* 1. ret == WS_SUCCESS (connection stays open for retry)
|
||||
* 2. the userauth callback is never invoked
|
||||
* 3. exactly one packet is sent and it is SSH_MSG_USERAUTH_FAILURE
|
||||
* 4. *idx == len (the new-password field is fully consumed) */
|
||||
static int test_DoUserAuthRequest_rejectsPasswordChange(void)
|
||||
{
|
||||
WOLFSSH_CTX* ctx = NULL;
|
||||
WOLFSSH* ssh = NULL;
|
||||
int result = 0;
|
||||
int ret;
|
||||
int capMsgId;
|
||||
byte buf[128];
|
||||
word32 len = 0, idx = 0;
|
||||
|
||||
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
|
||||
if (ctx == NULL)
|
||||
return -660;
|
||||
wolfSSH_SetIOSend(ctx, CaptureIoSendAuthSvc);
|
||||
wolfSSH_SetUserAuth(ctx, UnitAuthAlwaysSucceed);
|
||||
|
||||
ssh = wolfSSH_new(ctx);
|
||||
if (ssh == NULL) {
|
||||
result = -661;
|
||||
goto out;
|
||||
}
|
||||
|
||||
s_pwChangeCbCalled = 0;
|
||||
s_authSvcCaptureSz = 0;
|
||||
s_authSvcSendCount = 0;
|
||||
WMEMSET(s_authSvcCapture, 0, sizeof(s_authSvcCapture));
|
||||
|
||||
/* username: "user" */
|
||||
buf[len++] = 0; buf[len++] = 0; buf[len++] = 0; buf[len++] = 4;
|
||||
WMEMCPY(buf + len, "user", 4); len += 4;
|
||||
/* service name: "ssh-connection" */
|
||||
buf[len++] = 0; buf[len++] = 0; buf[len++] = 0; buf[len++] = 14;
|
||||
WMEMCPY(buf + len, "ssh-connection", 14); len += 14;
|
||||
/* auth method: "password" */
|
||||
buf[len++] = 0; buf[len++] = 0; buf[len++] = 0; buf[len++] = 8;
|
||||
WMEMCPY(buf + len, "password", 8); len += 8;
|
||||
/* password-change flag: TRUE */
|
||||
buf[len++] = 1;
|
||||
/* current password: "oldpass" */
|
||||
buf[len++] = 0; buf[len++] = 0; buf[len++] = 0; buf[len++] = 7;
|
||||
WMEMCPY(buf + len, "oldpass", 7); len += 7;
|
||||
/* new password: "newpass" */
|
||||
buf[len++] = 0; buf[len++] = 0; buf[len++] = 0; buf[len++] = 7;
|
||||
WMEMCPY(buf + len, "newpass", 7); len += 7;
|
||||
|
||||
ret = wolfSSH_TestDoUserAuthRequest(ssh, buf, len, &idx);
|
||||
|
||||
if (ret != WS_SUCCESS) {
|
||||
result = -662;
|
||||
goto out;
|
||||
}
|
||||
if (s_pwChangeCbCalled) {
|
||||
/* The callback must not run for a password-change request. */
|
||||
result = -663;
|
||||
goto out;
|
||||
}
|
||||
if (s_authSvcSendCount != 1) {
|
||||
result = -664;
|
||||
goto out;
|
||||
}
|
||||
capMsgId = CaptureMsgId(s_authSvcCapture, s_authSvcCaptureSz);
|
||||
if (capMsgId != MSGID_USERAUTH_FAILURE) {
|
||||
result = -665;
|
||||
goto out;
|
||||
}
|
||||
if (idx != len) {
|
||||
result = -666;
|
||||
goto out;
|
||||
}
|
||||
|
||||
out:
|
||||
wolfSSH_free(ssh);
|
||||
wolfSSH_CTX_free(ctx);
|
||||
return result;
|
||||
}
|
||||
|
||||
|
||||
/* userAuthTypesCb that advertises no methods (returns mask 0). Mirrors a
|
||||
* wolfsshd configuration with both PasswordAuthentication no and
|
||||
* PubkeyAuthentication no. */
|
||||
|
|
@ -7670,6 +7771,11 @@ int wolfSSH_UnitTest(int argc, char** argv)
|
|||
(unitResult == 0 ? "SUCCESS" : "FAILED"));
|
||||
testResult = testResult || unitResult;
|
||||
|
||||
unitResult = test_DoUserAuthRequest_rejectsPasswordChange();
|
||||
printf("DoUserAuthRequest_rejectsPasswordChange: %s\n",
|
||||
(unitResult == 0 ? "SUCCESS" : "FAILED"));
|
||||
testResult = testResult || unitResult;
|
||||
|
||||
unitResult = test_SendUserAuthFailure_emptyMethods();
|
||||
printf("SendUserAuthFailure_emptyMethods: %s\n",
|
||||
(unitResult == 0 ? "SUCCESS" : "FAILED"));
|
||||
|
|
|
|||
Loading…
Reference in New Issue