PEAP-MSCHAPv2: incorrect password expired message when challenge fails

[DouglasSmithLrd]

Issue type

• Defect - Crash or memory corruption.
• Defect - Non compliance with a standards document, or incorrect API usage.
• Defect - Unexpected behaviour (obvious or verified by project member).
• Feature request.

Defect/Feature description

When running PEAP-MSCHAPv2, if the password challenge response from the client fails, and the password is marked as expired, the server responds incorrectly with a password expired message rather than a reattempt with a new challenge.

How to reproduce issue

Use branch origin/v3.0.x
(3.0.11 has other issues that have already been fixed preventing change password operation)
Follow instructions in freeradius documentation for setting up PEAP-MSCHAPv2 change password operation. (In my case using SQL to store the user/password database.)

Mark the password as expired in the SMB-Account-Ctrl-Text.
Try to connect a PEAP-MSCHAPv2 client using an incorrect password.
Observe that freeradius will incorrectly send a message with error 648/password expired.

Clear the password expired status in the SMB-Account-Ctrl-Text.
Try to connect a PEAP-MSCHAPv2 client using an incorrect password.
Observe that freeradius will correctly send a new challenge with error 691/authentication failure.

The issue can be fixed by the following patch, which verifies that the challenge passed before checking if the password has been marked as expired:

diff --git a/src/modules/rlm_mschap/rlm_mschap.c b/src/modules/rlm_mschap/rlm_mschap.c
index 3509bd6..4eb5526 100644
--- a/src/modules/rlm_mschap/rlm_mschap.c
+++ b/src/modules/rlm_mschap/rlm_mschap.c
@@ -1405,7 +1405,8 @@ static rlm_rcode_t mschap_error(rlm_mschap_t *inst, REQUEST *request, unsigned c
    char        *p;

    if ((mschap_result == -648) ||
-       (smb_ctrl && ((smb_ctrl->vp_integer & ACB_PW_EXPIRED) != 0))) {
+       (mschap_result == 0 &&
+        smb_ctrl && ((smb_ctrl->vp_integer & ACB_PW_EXPIRED) != 0))) {
        REDEBUG("Password has expired.  User should retry authentication");
        error = 648;

--

With the patch freeradius will correctly issue a new challenge (if the challenge response fails), and then request a new password once the challenge response succeeds.

Output of [radiusd|freeradius] -X showing issue occurring

Incorrect response when password challenge fails, and password is marked as expired:

(8) eap_mschapv2:   Auth-Type MS-CHAP {
(8) mschap: Found Cleartext-Password, hashing to create NT-Password
(8) mschap: Found Cleartext-Password, hashing to create LM-Password
(8) mschap: Creating challenge hash with username: sqltest
(8) mschap: Client is using MS-CHAPv2
(8) mschap: ERROR: Password has expired.  User should retry authentication
(8)     [mschap] = reject
(8)   } # Auth-Type MS-CHAP = reject
(8) MSCHAP-Error:   E=648 R=0 C=117fb10b722ab1d2b2e5f7e608768213 V=3 M=Password expired
(8) Found new challenge from MS-CHAP-Error: err=648 retry=0 challenge=117fb10b722ab1d2b2e5f7e608768213

Expected behavior when password challenge fails:

(8) eap_mschapv2:   Auth-Type MS-CHAP {
(8) mschap: Found Cleartext-Password, hashing to create NT-Password
(8) mschap: Found Cleartext-Password, hashing to create LM-Password
(8) mschap: Creating challenge hash with username: sqltest
(8) mschap: Client is using MS-CHAPv2
(8) mschap: ERROR: MS-CHAP2-Response is incorrect
(8)     [mschap] = reject
(8)   } # Auth-Type MS-CHAP = reject
(8) MSCHAP-Error:   E=691 R=1 C=91f119c75f9310e52a9508cc2ec363be V=3 M=Authentication failed
(8) Found new challenge from MS-CHAP-Error: err=691 retry=1 challenge=91f119c75f9310e52a9508cc2ec363be

[arr2036]

Thanks for investigating that. The patch looks good, but can you please submit as a PR.

sigh

Looks like the defect template needs some additional edits...