Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1453408, fix FIPS level1 -> level2 transition in C_SetPIN, r=rrelyea
Reviewers: rrelyea

Reviewed By: rrelyea

Bug #: 1453408

Differential Revision: https://phabricator.services.mozilla.com/D38459

--HG--
extra : rebase_source : 78729e28093b8789f266f82af009df9508a4d6b8
extra : amend_source : f542be09b746cd4643487b05487dc0c21c03499a
  • Loading branch information
ueno committed Jul 24, 2019
1 parent 8ff6b47 commit f3fef05
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 5 deletions.
95 changes: 95 additions & 0 deletions gtests/softoken_gtest/softoken_gtest.cc
Expand Up @@ -3,6 +3,7 @@
#include "nspr.h"
#include "nss.h"
#include "pk11pub.h"
#include "secmod.h"
#include "secerr.h"

#include "nss_scoped_ptrs.h"
Expand Down Expand Up @@ -265,6 +266,100 @@ TEST_F(SoftokenNoDBTest, NeedUserInitNoDB) {
ASSERT_EQ(SECSuccess, NSS_Shutdown());
}

#ifndef NSS_FIPS_DISABLED

class SoftokenFipsTest : public SoftokenTest {
protected:
SoftokenFipsTest() : SoftokenTest("SoftokenFipsTest.d-") {}

virtual void SetUp() {
SoftokenTest::SetUp();

// Turn on FIPS mode (code borrowed from FipsMode in modutil/pk11.c)
char *internal_name;
ASSERT_FALSE(PK11_IsFIPS());
internal_name = PR_smprintf("%s", SECMOD_GetInternalModule()->commonName);
ASSERT_EQ(SECSuccess, SECMOD_DeleteInternalModule(internal_name));
PR_smprintf_free(internal_name);
ASSERT_TRUE(PK11_IsFIPS());
}
};

const std::vector<std::string> kFipsPasswordCases[] = {
// FIPS level1 -> level1 -> level1
{"", "", ""},
// FIPS level1 -> level1 -> level2
{"", "", "strong-_123"},
// FIXME: this should work: FIPS level1 -> level2 -> level2
// {"", "strong-_123", "strong-_456"},
// FIPS level2 -> level2 -> level2
{"strong-_123", "strong-_456", "strong-_123"}};

const std::vector<std::string> kFipsPasswordBadCases[] = {
// FIPS level1 -> level2 -> level1
{"", "strong-_123", ""},
// FIPS level2 -> level1 -> level1
{"strong-_123", ""},
// FIPS level2 -> level2 -> level1
{"strong-_123", "strong-_456", ""},
// initialize with a weak password
{"weak"},
// FIPS level1 -> weak password
{"", "weak"},
// FIPS level2 -> weak password
{"strong-_123", "weak"}};

class SoftokenFipsPasswordTest
: public SoftokenFipsTest,
public ::testing::WithParamInterface<std::vector<std::string>> {};

class SoftokenFipsBadPasswordTest
: public SoftokenFipsTest,
public ::testing::WithParamInterface<std::vector<std::string>> {};

TEST_P(SoftokenFipsPasswordTest, SetPassword) {
const std::vector<std::string> &passwords = GetParam();
ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
ASSERT_TRUE(slot);

auto it = passwords.begin();
auto prev_it = it;
EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, (*it).c_str()));
for (it++; it != passwords.end(); it++, prev_it++) {
EXPECT_EQ(SECSuccess,
PK11_ChangePW(slot.get(), (*prev_it).c_str(), (*it).c_str()));
}
}

TEST_P(SoftokenFipsBadPasswordTest, SetBadPassword) {
const std::vector<std::string> &passwords = GetParam();
ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
ASSERT_TRUE(slot);

auto it = passwords.begin();
auto prev_it = it;
SECStatus rv = PK11_InitPin(slot.get(), nullptr, (*it).c_str());
if (it + 1 == passwords.end())
EXPECT_EQ(SECFailure, rv);
else
EXPECT_EQ(SECSuccess, rv);
for (it++; it != passwords.end(); it++, prev_it++) {
rv = PK11_ChangePW(slot.get(), (*prev_it).c_str(), (*it).c_str());
if (it + 1 == passwords.end())
EXPECT_EQ(SECFailure, rv);
else
EXPECT_EQ(SECSuccess, rv);
}
}

INSTANTIATE_TEST_CASE_P(FipsPasswordCases, SoftokenFipsPasswordTest,
::testing::ValuesIn(kFipsPasswordCases));

INSTANTIATE_TEST_CASE_P(BadFipsPasswordCases, SoftokenFipsBadPasswordTest,
::testing::ValuesIn(kFipsPasswordBadCases));

#endif

} // namespace nss_test

int main(int argc, char **argv) {
Expand Down
28 changes: 24 additions & 4 deletions lib/softoken/fipstokn.c
Expand Up @@ -645,17 +645,37 @@ FC_SetPIN(CK_SESSION_HANDLE hSession, CK_CHAR_PTR pOldPin,

CHECK_FORK();

if ((rv = sftk_fipsCheck()) == CKR_OK &&
(rv = sftk_newPinCheck(pNewPin, usNewLen)) == CKR_OK) {
rv = sftk_fipsCheck();
if (rv != CKR_OK) {
goto loser;
}

if (isLevel2 || usNewLen > 0) {
rv = sftk_newPinCheck(pNewPin, usNewLen);
if (rv != CKR_OK) {
goto loser;
}
rv = NSC_SetPIN(hSession, pOldPin, usOldLen, pNewPin, usNewLen);
if ((rv == CKR_OK) &&
(sftk_SlotIDFromSessionHandle(hSession) == FIPS_SLOT_ID)) {
if (rv != CKR_OK) {
goto loser;
}
if (sftk_SlotIDFromSessionHandle(hSession) == FIPS_SLOT_ID) {
/* if we set the password in level1 we now go
* to level2. NOTE: we don't allow the user to
* go from level2 to level1 */
isLevel2 = PR_TRUE;
}
} else {
/* here both old and new passwords are empty, but we need to
* call NSC_SetPIN to force rekey the database entries */
PORT_Assert(usNewLen == 0);
rv = NSC_SetPIN(hSession, pOldPin, usOldLen, pNewPin, usNewLen);
if (rv != CKR_OK) {
goto loser;
}
}

loser:
if (sftk_audit_enabled) {
char msg[128];
NSSAuditSeverity severity = (rv == CKR_OK) ? NSS_AUDIT_INFO : NSS_AUDIT_ERROR;
Expand Down
5 changes: 4 additions & 1 deletion lib/softoken/pkcs11.c
Expand Up @@ -3900,7 +3900,10 @@ NSC_SetPIN(CK_SESSION_HANDLE hSession, CK_CHAR_PTR pOldPin,
crv = CKR_PIN_LEN_RANGE;
goto loser;
}
if (ulNewLen < (CK_ULONG)slot->minimumPinLen) {
/* check the length of new pin, unless both old and new passwords
* are empty */
if ((ulNewLen != 0 || ulOldLen != 0) &&
ulNewLen < (CK_ULONG)slot->minimumPinLen) {
crv = CKR_PIN_LEN_RANGE;
goto loser;
}
Expand Down

0 comments on commit f3fef05

Please sign in to comment.