Commit 48d2aa2c authored by Jerry Lin's avatar Jerry Lin Committed by Commit Bot

Make unittests of password_generator to be deterministic

The tests in components/autofill/c/b/password_generator_fips181_unittest.cc
use a random source during tests and are thus non-deterministic.

This patch adds a method of PasswordGeneratorFips181 called
SetGeneratorForTest. It accepts a parameter generate_func so
PasswordGeneratorFips181 will use this function to generate password to
achieve deterministic unittesting. Unittest code uses this function to
set generator for test to make test deterministic.

R=vabr@chromium.org

Bug: 847200
Change-Id: If2924ed0e59843336f0e642053eefaca4c353163
Reviewed-on: https://chromium-review.googlesource.com/1084392
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: default avatarVaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568782}
parent 776b544c
...@@ -376,6 +376,7 @@ Jeongwoo Park <skeksk91@gmail.com> ...@@ -376,6 +376,7 @@ Jeongwoo Park <skeksk91@gmail.com>
Jeremy Noring <jnoring@hirevue.com> Jeremy Noring <jnoring@hirevue.com>
Jeremy Spiegel <jeremysspiegel@gmail.com> Jeremy Spiegel <jeremysspiegel@gmail.com>
Jeroen Van den Berghe <vandenberghe.jeroen@gmail.com> Jeroen Van den Berghe <vandenberghe.jeroen@gmail.com>
Jerry Lin <wahahab11@gmail.com>
Jesper Storm Bache <jsbache@gmail.com> Jesper Storm Bache <jsbache@gmail.com>
Jesse Miller <jesse@jmiller.biz> Jesse Miller <jesse@jmiller.biz>
Jesus Sanchez-Palencia <jesus.sanchez-palencia.fernandez.fil@intel.com> Jesus Sanchez-Palencia <jesus.sanchez-palencia.fernandez.fil@intel.com>
......
...@@ -50,6 +50,15 @@ bool VerifyPassword(const std::string& password) { ...@@ -50,6 +50,15 @@ bool VerifyPassword(const std::string& password) {
return num_lower_case && num_upper_case && num_digits; return num_lower_case && num_upper_case && num_digits;
} }
// Password generation function for unit testing, default to nullptr.
// If not null, ForceFixPassword() will also always use |kMinDigit| as the digit
// replacement, instead of choosing randomly.
int (*g_test_override_generator)(char* word,
char* hypenated_word,
unsigned short minlen,
unsigned short maxlen,
unsigned int pass_mode) = nullptr;
} // namespace } // namespace
namespace autofill { namespace autofill {
...@@ -66,7 +75,13 @@ void ForceFixPassword(std::string* password) { ...@@ -66,7 +75,13 @@ void ForceFixPassword(std::string* password) {
for (std::string::reverse_iterator iter = password->rbegin(); for (std::string::reverse_iterator iter = password->rbegin();
iter != password->rend(); ++iter) { iter != password->rend(); ++iter) {
if (islower(*iter)) { if (islower(*iter)) {
*iter = base::RandInt(kMinDigit, kMaxDigit); // Tests will use |PasswordGeneratorFips181::SetGeneratorForTest| to put a
// non-random generator in |g_test_override_generator|. To eliminate the
// other source of randomness, always fix the chosen digit to |kMinDigit|
// in such case.
*iter = g_test_override_generator == nullptr
? base::RandInt(kMinDigit, kMaxDigit)
: kMinDigit;
break; break;
} }
} }
...@@ -76,6 +91,15 @@ PasswordGeneratorFips181::PasswordGeneratorFips181(int max_length) ...@@ -76,6 +91,15 @@ PasswordGeneratorFips181::PasswordGeneratorFips181(int max_length)
: password_length_(GetLengthFromHint(max_length, kDefaultPasswordLength)) {} : password_length_(GetLengthFromHint(max_length, kDefaultPasswordLength)) {}
PasswordGeneratorFips181::~PasswordGeneratorFips181() {} PasswordGeneratorFips181::~PasswordGeneratorFips181() {}
void PasswordGeneratorFips181::SetGeneratorForTest(
int (*generator)(char* word,
char* hypenated_word,
unsigned short minlen,
unsigned short maxlen,
unsigned int pass_mode)) {
g_test_override_generator = generator;
}
std::string PasswordGeneratorFips181::Generate() const { std::string PasswordGeneratorFips181::Generate() const {
char password[255]; char password[255];
char unused_hypenated_password[255]; char unused_hypenated_password[255];
...@@ -83,12 +107,13 @@ std::string PasswordGeneratorFips181::Generate() const { ...@@ -83,12 +107,13 @@ std::string PasswordGeneratorFips181::Generate() const {
// No special characters included for now. // No special characters included for now.
unsigned int mode = S_NB | S_CL | S_SL; unsigned int mode = S_NB | S_CL | S_SL;
// Generate the password by gen_pron_pass(), if it is not conforming then // Generate the password and fix afterwards if needed.
// force fix it. auto generator =
gen_pron_pass(password, unused_hypenated_password, password_length_, g_test_override_generator ? g_test_override_generator : gen_pron_pass;
password_length_, mode); generator(password, unused_hypenated_password, password_length_,
password_length_, mode);
std::string str_password(password); std::string str_password(password);
if (VerifyPassword(str_password)) if (VerifyPassword(str_password))
return str_password; return str_password;
......
...@@ -37,6 +37,15 @@ class PasswordGeneratorFips181 { ...@@ -37,6 +37,15 @@ class PasswordGeneratorFips181 {
// Not thread safe. // Not thread safe.
std::string Generate() const; std::string Generate() const;
// This method allows to substitute a replacement for the fips181 method
// gen_pron_pass, used by the generator to generate the password. This is
// useful in tests, for providing a deterministic generator.
static void SetGeneratorForTest(int (*generator)(char* word,
char* hypenated_word,
unsigned short minlen,
unsigned short maxlen,
unsigned int pass_mode));
private: private:
// Unit test also need to access |kDefaultPasswordLength|. // Unit test also need to access |kDefaultPasswordLength|.
static const int kDefaultPasswordLength; static const int kDefaultPasswordLength;
......
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
#include <stddef.h> #include <stddef.h>
#include <string.h>
#include <locale> #include <locale>
...@@ -28,44 +29,82 @@ void CheckPasswordCorrectness(const std::string& password) { ...@@ -28,44 +29,82 @@ void CheckPasswordCorrectness(const std::string& password) {
EXPECT_GT(num_digits, 0) << password; EXPECT_GT(num_digits, 0) << password;
} }
const char* g_password_text = nullptr;
int GenerateForTest(char* word,
char* hypenated_word,
unsigned short minlen,
unsigned short maxlen,
unsigned int pass_mode) {
EXPECT_LE(minlen, maxlen);
EXPECT_TRUE(word);
EXPECT_TRUE(hypenated_word);
EXPECT_TRUE(g_password_text) << "Set g_password_text before every call";
strncpy(word, g_password_text, maxlen);
g_password_text = nullptr;
// Resize password to |maxlen|.
word[maxlen] = '\0';
EXPECT_GE(strlen(word), minlen)
<< "Make sure to provide enough characters in g_password_text";
return static_cast<int>(strlen(word));
}
class PasswordGeneratorFips181Test : public ::testing::Test {
public:
PasswordGeneratorFips181Test() {
autofill::PasswordGeneratorFips181::SetGeneratorForTest(GenerateForTest);
}
~PasswordGeneratorFips181Test() override {
autofill::PasswordGeneratorFips181::SetGeneratorForTest(nullptr);
}
private:
DISALLOW_COPY_AND_ASSIGN(PasswordGeneratorFips181Test);
};
} // namespace } // namespace
namespace autofill { namespace autofill {
TEST(PasswordGeneratorFips181Test, PasswordLength) { TEST_F(PasswordGeneratorFips181Test, PasswordLength) {
PasswordGeneratorFips181 pg1(10); PasswordGeneratorFips181 pg1(10);
g_password_text = "Aa12345678901234567890";
std::string password = pg1.Generate(); std::string password = pg1.Generate();
EXPECT_EQ(password.size(), 10u); EXPECT_EQ(password.size(), 10u);
PasswordGeneratorFips181 pg2(-1); PasswordGeneratorFips181 pg2(-1);
g_password_text = "Aa12345678901234567890";
password = pg2.Generate(); password = pg2.Generate();
EXPECT_EQ( EXPECT_EQ(
password.size(), password.size(),
static_cast<size_t>(PasswordGeneratorFips181::kDefaultPasswordLength)); static_cast<size_t>(PasswordGeneratorFips181::kDefaultPasswordLength));
PasswordGeneratorFips181 pg3(100); PasswordGeneratorFips181 pg3(100);
g_password_text = "Aa12345678901234567890";
password = pg3.Generate(); password = pg3.Generate();
EXPECT_EQ( EXPECT_EQ(
password.size(), password.size(),
static_cast<size_t>(PasswordGeneratorFips181::kDefaultPasswordLength)); static_cast<size_t>(PasswordGeneratorFips181::kDefaultPasswordLength));
} }
TEST(PasswordGeneratorFips181Test, PasswordPattern) { TEST_F(PasswordGeneratorFips181Test, PasswordPattern) {
PasswordGeneratorFips181 pg(12); PasswordGeneratorFips181 pg1(12);
std::string password = pg.Generate(); g_password_text = "012345678jkl";
CheckPasswordCorrectness(password); std::string password1 = pg1.Generate();
} CheckPasswordCorrectness(password1);
TEST(PasswordGeneratorFips181Test, Printable) { PasswordGeneratorFips181 pg2(12);
PasswordGeneratorFips181 pg(12); g_password_text = "abcDEFGHIJKL";
std::string password = pg.Generate(); std::string password2 = pg2.Generate();
for (size_t i = 0; i < password.size(); i++) { CheckPasswordCorrectness(password2);
// Make sure that the character is printable.
EXPECT_TRUE(isgraph(password[i])); PasswordGeneratorFips181 pg3(12);
} g_password_text = "abcdefghijkl";
std::string password3 = pg3.Generate();
CheckPasswordCorrectness(password3);
} }
TEST(PasswordGeneratorFips181Test, ForceFixPasswordTest) { TEST_F(PasswordGeneratorFips181Test, ForceFixPasswordTest) {
std::string passwords_to_fix[] = {"nonumbersoruppercase", std::string passwords_to_fix[] = {"nonumbersoruppercase",
"nonumbersWithuppercase", "nonumbersWithuppercase",
"numbers3Anduppercase", "UmpAwgemHoc"}; "numbers3Anduppercase", "UmpAwgemHoc"};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment