Reviewed-by: Dandan Bi <[email protected]>
Thanks, Dandan > -----Original Message----- > From: Desimone, Nathaniel L <[email protected]> > Sent: Thursday, December 2, 2021 6:52 AM > To: [email protected] > Cc: Bi, Dandan <[email protected]>; Liming Gao > <[email protected]>; Jadhav, Manoj D > <[email protected]>; Chiu, Chasel <[email protected]> > Subject: [edk2-platforms] [PATCH v1] UserAuthFeaturePkg: VerifyPassword() > allows one extra password attempt > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3756 > > If the password provided by the user is incorrect, then the VerifyPassword() > function is supposed to return EFI_SECURITY_VIOLATION if the user has not > exceeded the maximum number of password guesses (currently set to 3). If > the number of password guesses has been exceeded, then VerifyPassword() > shall return EFI_ACCESS_DENIED. UserAuthenticationDxe uses > EFI_ACCESS_DENIED as the signal that the number of guesses has been > exceeded for the purposes of triggering a forced reboot. > > VerifyPassword() checks if the number of password guess attempts has > exceeded the maximum allowed before checking if the current password > guess is correct. If it has, then VerifyPassword() immediately returns > EFI_ACCESS_DENIED. This behavior is correct since it is possible for > VerifyPassword() to be called again after the maximum number of attempts > has been exceeded. However, if the user guesses incorrectly, then > VerifyPassword() will always return EFI_SECURITY_VIOLATION. This is where > the bug is. It is possible that after the current attempt, the maximum allowed > number of attempts is exceeded. Therefore, > VerifyPassword() should check the number of attempts again, after checking > if the password is correct. > > Cc: Dandan Bi <[email protected]> > Cc: Liming Gao <[email protected]> > Cc: Jadhav Manoj D <[email protected]> > Cc: Chasel Chiu <[email protected]> > Signed-off-by: Nate DeSimone <[email protected]> > --- > .../UserAuthenticationSmm.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git > a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDx > eSmm/UserAuthenticationSmm.c > b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDx > eSmm/UserAuthenticationSmm.c > index c24c3c47a5..16e3405a82 100644 > --- > a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDx > eSmm/UserAuthenticationSmm.c > +++ > b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthentication > +++ DxeSmm/UserAuthenticationSmm.c > @@ -504,7 +504,12 @@ SmmPasswordHandler ( > > if (!IsPasswordVerified (UserGuid, > SmmCommunicateSetPassword.OldPassword, PasswordLen + 1)) { > DEBUG ((DEBUG_ERROR, "SmmPasswordHandler: PasswordVerify - > FAIL\n")); > - Status = EFI_SECURITY_VIOLATION; > + if (*PasswordTryCount >= PASSWORD_MAX_TRY_COUNT) { > + DEBUG ((DEBUG_ERROR, "SmmPasswordHandler: SET_PASSWORD try > count reach!\n")); > + Status = EFI_ACCESS_DENIED; > + } else { > + Status = EFI_SECURITY_VIOLATION; > + } > goto EXIT; > } > > @@ -554,7 +559,12 @@ SmmPasswordHandler ( > } > if (!IsPasswordVerified (UserGuid, > SmmCommunicateVerifyPassword.Password, PasswordLen + 1)) { > DEBUG ((DEBUG_ERROR, "SmmPasswordHandler: PasswordVerify - > FAIL\n")); > - Status = EFI_SECURITY_VIOLATION; > + if (*PasswordTryCount >= PASSWORD_MAX_TRY_COUNT) { > + DEBUG ((DEBUG_ERROR, "SmmPasswordHandler: VERIFY_PASSWORD > try count reach!\n")); > + Status = EFI_ACCESS_DENIED; > + } else { > + Status = EFI_SECURITY_VIOLATION; > + } > goto EXIT; > } > mPasswordVerified = TRUE; > -- > 2.27.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#84301): https://edk2.groups.io/g/devel/message/84301 Mute This Topic: https://groups.io/mt/87441032/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
