From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mx.groups.io with SMTP id smtpd.web11.820.1638399107341949845 for ; Wed, 01 Dec 2021 14:51:47 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.120, mailfrom: nathaniel.l.desimone@intel.com) X-IronPort-AV: E=McAfee;i="6200,9189,10185"; a="235308096" X-IronPort-AV: E=Sophos;i="5.87,280,1631602800"; d="scan'208";a="235308096" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Dec 2021 14:51:46 -0800 X-IronPort-AV: E=Sophos;i="5.87,280,1631602800"; d="scan'208";a="576534365" Received: from nldesimo-desk1.amr.corp.intel.com ([10.212.177.220]) by fmsmga004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Dec 2021 14:51:46 -0800 From: "Nate DeSimone" To: devel@edk2.groups.io Cc: Dandan Bi , Liming Gao , Jadhav Manoj D , Chasel Chiu Subject: [edk2-platforms] [PATCH v1] UserAuthFeaturePkg: VerifyPassword() allows one extra password attempt Date: Wed, 1 Dec 2021 14:51:32 -0800 Message-Id: <20211201225132.1211-1-nathaniel.l.desimone@intel.com> X-Mailer: git-send-email 2.27.0.windows.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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 Cc: Liming Gao Cc: Jadhav Manoj D Cc: Chasel Chiu Signed-off-by: Nate DeSimone --- .../UserAuthenticationSmm.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.c b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.c index c24c3c47a5..16e3405a82 100644 --- a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.c +++ b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/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