From 9e02350aaa9eb73c32c84e262dfbee5a471f7570 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 22 Jun 2026 12:04:17 -0700 Subject: [PATCH] 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) --- src/internal.c | 72 ++++++++++++++++++--------------- tests/unit.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 32 deletions(-) diff --git a/src/internal.c b/src/internal.c index 5a4f4779..4ad959d3 100644 --- a/src/internal.c +++ b/src/internal.c @@ -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) diff --git a/tests/unit.c b/tests/unit.c index 2c5e082c..536999c1 100644 --- a/tests/unit.c +++ b/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"));