cryptohome: Replace abstract AuthFactor interface with a single class
The abstract AuthFactor interface is replaced by single AuthFactor
class. This class would now be a wrapper around abstract
AuthBlock and AuthBlockState.
BUG=b:208834391
TEST=FEATURES=test emerge-strongbad cryptohome
Change-Id: I89e838a4dc0e4348486433fb773af2f28a0c21a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/3313316
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Auto-Submit: Hardik Goyal <hardikgoyal@chromium.org>
Tested-by: Hardik Goyal <hardikgoyal@chromium.org>
Reviewed-by: Greg Kerr <kerrnel@chromium.org>
Commit-Queue: Hardik Goyal <hardikgoyal@chromium.org>
diff --git a/cryptohome/BUILD.gn b/cryptohome/BUILD.gn
index cd1ff90..5072dda 100644
--- a/cryptohome/BUILD.gn
+++ b/cryptohome/BUILD.gn
@@ -426,7 +426,7 @@
"auth_blocks/auth_block_state_unittest.cc",
"auth_blocks/auth_block_unittest.cc",
"auth_blocks/sync_to_async_auth_block_adapter_test.cc",
- "auth_factors/password_auth_factor_unittest.cc",
+ "auth_factor/auth_factor_unittest.cc",
"auth_session_manager_unittest.cc",
"auth_session_unittest.cc",
"challenge_credentials/challenge_credentials_helper_impl_unittest.cc",
diff --git a/cryptohome/auth_factors/password_auth_factor.cc b/cryptohome/auth_factor/auth_factor.cc
similarity index 73%
rename from cryptohome/auth_factors/password_auth_factor.cc
rename to cryptohome/auth_factor/auth_factor.cc
index 965b2cf..6cf35f3 100644
--- a/cryptohome/auth_factors/password_auth_factor.cc
+++ b/cryptohome/auth_factor/auth_factor.cc
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "cryptohome/auth_factors/password_auth_factor.h"
+#include "cryptohome/auth_factor/auth_factor.h"
#include <memory>
#include <utility>
@@ -10,11 +10,11 @@
#include "cryptohome/scrypt_verifier.h"
namespace cryptohome {
-PasswordAuthFactor::PasswordAuthFactor(KeysetManagement* keyset_management)
+AuthFactor::AuthFactor(KeysetManagement* keyset_management)
: keyset_management_(keyset_management) {}
-MountError PasswordAuthFactor::AuthenticateAuthFactor(
- const Credentials& credential, bool is_ephemeral_user) {
+MountError AuthFactor::AuthenticateAuthFactor(const Credentials& credential,
+ bool is_ephemeral_user) {
// Store key data in current auth_factor for future use.
key_data_ = credential.key_data();
@@ -38,16 +38,15 @@
return MOUNT_ERROR_NONE;
}
-std::unique_ptr<CredentialVerifier>
-PasswordAuthFactor::TakeCredentialVerifier() {
+std::unique_ptr<CredentialVerifier> AuthFactor::TakeCredentialVerifier() {
return std::move(credential_verifier_);
}
-const cryptohome::KeyData& PasswordAuthFactor::GetKeyData() const {
+const cryptohome::KeyData& AuthFactor::GetKeyData() const {
return key_data_;
}
-const FileSystemKeyset PasswordAuthFactor::GetFileSystemKeyset() const {
+const FileSystemKeyset AuthFactor::GetFileSystemKeyset() const {
return FileSystemKeyset(*vault_keyset_);
}
diff --git a/cryptohome/auth_factor/auth_factor.h b/cryptohome/auth_factor/auth_factor.h
new file mode 100644
index 0000000..088929f
--- /dev/null
+++ b/cryptohome/auth_factor/auth_factor.h
@@ -0,0 +1,63 @@
+// Copyright 2021 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CRYPTOHOME_AUTH_FACTOR_AUTH_FACTOR_H_
+#define CRYPTOHOME_AUTH_FACTOR_AUTH_FACTOR_H_
+
+#include <memory>
+#include <string>
+
+#include "cryptohome/auth_blocks/auth_block.h"
+#include "cryptohome/credential_verifier.h"
+#include "cryptohome/credentials.h"
+#include "cryptohome/keyset_management.h"
+#include "cryptohome/rpc.pb.h"
+#include "cryptohome/storage/file_system_keyset.h"
+#include "cryptohome/UserDataAuth.pb.h"
+
+namespace cryptohome {
+
+// This is an interface designed to be implemented by the different
+// authentication factors - password, pin, security keys, etc - so that
+// they take handle multiple factors of the same type and know what to do with
+// it.
+class AuthFactor {
+ public:
+ explicit AuthFactor(KeysetManagement* keyset_management);
+ ~AuthFactor() = default;
+ // AuthenticateAuthFactor validates the key should it exist on disk for the
+ // user.
+ MountError AuthenticateAuthFactor(const Credentials& credential,
+ bool is_ephemeral_user);
+
+ // Transfer ownership of password verifier that can be used to verify
+ // credentials during unlock.
+ std::unique_ptr<CredentialVerifier> TakeCredentialVerifier();
+ // -------------------------------------------------------------------------
+ // Temporary functions below as we transition from AuthSession to AuthFactor
+ // -------------------------------------------------------------------------
+ // Returns the key data with which this AuthFactor is authenticated with.
+ const cryptohome::KeyData& GetKeyData() const;
+
+ // Return VaultKeyset of the authenticated user.
+ const VaultKeyset* vault_keyset() const { return vault_keyset_.get(); }
+
+ // Returns FileSystemKeyset of the authenticated user.
+ const FileSystemKeyset GetFileSystemKeyset() const;
+
+ private:
+ // The creator of the AuthFactor object is responsible for the
+ // life of KeysetManagement object.
+ KeysetManagement* keyset_management_;
+ // This is used by User Session to verify users credentials at unlock.
+ std::unique_ptr<CredentialVerifier> credential_verifier_;
+ // Used to decrypt/ encrypt & store credentials.
+ std::unique_ptr<VaultKeyset> vault_keyset_;
+ // Used to store key meta data.
+ cryptohome::KeyData key_data_;
+};
+
+} // namespace cryptohome
+
+#endif // CRYPTOHOME_AUTH_FACTOR_AUTH_FACTOR_H_
diff --git a/cryptohome/auth_factors/password_auth_factor_unittest.cc b/cryptohome/auth_factor/auth_factor_unittest.cc
similarity index 74%
rename from cryptohome/auth_factors/password_auth_factor_unittest.cc
rename to cryptohome/auth_factor/auth_factor_unittest.cc
index 09298da..1c6dba5 100644
--- a/cryptohome/auth_factors/password_auth_factor_unittest.cc
+++ b/cryptohome/auth_factor/auth_factor_unittest.cc
@@ -2,9 +2,9 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-// Unit tests for PasswordAuthFactor.
+// Unit tests for AuthFactor.
-#include "cryptohome/auth_factors/password_auth_factor.h"
+#include "cryptohome/auth_factor/auth_factor.h"
#include <memory>
#include <string>
@@ -30,20 +30,20 @@
const char kFakeUsername[] = "test_username";
const char kFakePassword[] = "test_pass";
-class PasswordAuthFactorTest : public ::testing::Test {
+class AuthFactorTest : public ::testing::Test {
public:
- PasswordAuthFactorTest() = default;
- PasswordAuthFactorTest(const PasswordAuthFactorTest&) = delete;
- PasswordAuthFactorTest& operator=(const PasswordAuthFactorTest&) = delete;
- ~PasswordAuthFactorTest() override = default;
+ AuthFactorTest() = default;
+ AuthFactorTest(const AuthFactorTest&) = delete;
+ AuthFactorTest& operator=(const AuthFactorTest&) = delete;
+ ~AuthFactorTest() override = default;
protected:
- // Mock KeysetManagent object, will be passed to PasswordAuthFactorTest for
+ // Mock KeysetManagent object, will be passed to AuthFactorTest for
// its internal use.
NiceMock<MockKeysetManagement> keyset_management_;
};
-TEST_F(PasswordAuthFactorTest, PersistentAuthenticateAuthFactorTest_Success) {
+TEST_F(AuthFactorTest, PersistentAuthenticateAuthFactorTest_Success) {
// Setup
auto vk = std::make_unique<VaultKeyset>();
Credentials creds(kFakeUsername, brillo::SecureBlob(kFakePassword));
@@ -55,7 +55,7 @@
.WillOnce(Return(true));
std::unique_ptr<AuthFactor> pass_auth_factor =
- std::make_unique<PasswordAuthFactor>(&keyset_management_);
+ std::make_unique<AuthFactor>(&keyset_management_);
// Test
EXPECT_THAT(
@@ -68,14 +68,14 @@
EXPECT_TRUE(verifier->Verify(brillo::SecureBlob(kFakePassword)));
}
-TEST_F(PasswordAuthFactorTest, PersistentAuthenticateAuthFactorTest_Fail) {
+TEST_F(AuthFactorTest, PersistentAuthenticateAuthFactorTest_Fail) {
// Setup
Credentials creds(kFakeUsername, brillo::SecureBlob(kFakePassword));
EXPECT_CALL(keyset_management_, GetValidKeyset(_, _))
.WillOnce(
DoAll(SetArgPointee<1>(MOUNT_ERROR_FATAL), Return(ByMove(nullptr))));
std::unique_ptr<AuthFactor> pass_auth_factor =
- std::make_unique<PasswordAuthFactor>(&keyset_management_);
+ std::make_unique<AuthFactor>(&keyset_management_);
// Test
EXPECT_THAT(
@@ -83,12 +83,12 @@
Eq(MOUNT_ERROR_FATAL));
}
-TEST_F(PasswordAuthFactorTest, EphemeralAuthenticateAuthFactorTest) {
+TEST_F(AuthFactorTest, EphemeralAuthenticateAuthFactorTest) {
// Setup
auto vk = std::make_unique<VaultKeyset>();
Credentials creds(kFakeUsername, brillo::SecureBlob(kFakePassword));
std::unique_ptr<AuthFactor> pass_auth_factor =
- std::make_unique<PasswordAuthFactor>(&keyset_management_);
+ std::make_unique<AuthFactor>(&keyset_management_);
EXPECT_CALL(keyset_management_, GetValidKeyset(_, _)).Times(0);
EXPECT_CALL(keyset_management_, ReSaveKeysetIfNeeded(_, _)).Times(0);
diff --git a/cryptohome/auth_factors/auth_factor.h b/cryptohome/auth_factors/auth_factor.h
deleted file mode 100644
index 4dabea6..0000000
--- a/cryptohome/auth_factors/auth_factor.h
+++ /dev/null
@@ -1,52 +0,0 @@
-// Copyright 2021 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CRYPTOHOME_AUTH_FACTORS_AUTH_FACTOR_H_
-#define CRYPTOHOME_AUTH_FACTORS_AUTH_FACTOR_H_
-
-#include <memory>
-#include <string>
-
-#include "cryptohome/auth_blocks/auth_block.h"
-#include "cryptohome/credential_verifier.h"
-#include "cryptohome/credentials.h"
-#include "cryptohome/keyset_management.h"
-#include "cryptohome/rpc.pb.h"
-#include "cryptohome/storage/file_system_keyset.h"
-#include "cryptohome/UserDataAuth.pb.h"
-
-namespace cryptohome {
-
-// This is a pure virtual interface designed to be implemented by the different
-// authentication factors - password, pin, security keys, etc - so that
-// they take handle multiple factors of the same type and know what to do with
-// it.
-class AuthFactor {
- public:
- AuthFactor() = default;
- virtual ~AuthFactor() = default;
- // AuthenticateAuthFactor validates the key should it exist on disk for the
- // user.
- virtual MountError AuthenticateAuthFactor(const Credentials& credential,
- bool is_ephemeral_user) = 0;
-
- // Transfer ownership of password verifier that can be used to verify
- // credentials during unlock.
- virtual std::unique_ptr<CredentialVerifier> TakeCredentialVerifier() = 0;
- // -------------------------------------------------------------------------
- // Temporary functions below as we transition from AuthSession to AuthFactor
- // -------------------------------------------------------------------------
- // Returns the key data with which this AuthFactor is authenticated with.
- virtual const cryptohome::KeyData& GetKeyData() const = 0;
-
- // Return VaultKeyset of the authenticated user.
- virtual const VaultKeyset* vault_keyset() const = 0;
-
- // Returns FileSystemKeyset of the authenticated user.
- virtual const FileSystemKeyset GetFileSystemKeyset() const = 0;
-};
-
-} // namespace cryptohome
-
-#endif // CRYPTOHOME_AUTH_FACTORS_AUTH_FACTOR_H_
diff --git a/cryptohome/auth_factors/password_auth_factor.h b/cryptohome/auth_factors/password_auth_factor.h
deleted file mode 100644
index caafc84..0000000
--- a/cryptohome/auth_factors/password_auth_factor.h
+++ /dev/null
@@ -1,55 +0,0 @@
-// Copyright 2021 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CRYPTOHOME_AUTH_FACTORS_PASSWORD_AUTH_FACTOR_H_
-#define CRYPTOHOME_AUTH_FACTORS_PASSWORD_AUTH_FACTOR_H_
-
-#include <memory>
-
-#include "cryptohome/auth_factors/auth_factor.h"
-#include "cryptohome/UserDataAuth.pb.h"
-
-namespace cryptohome {
-
-// PasswordAuthFactor defines the behaviour for when AuthSession wants to use
-// password to authenticate.
-class PasswordAuthFactor : public AuthFactor {
- public:
- explicit PasswordAuthFactor(KeysetManagement* keyset_management);
- virtual ~PasswordAuthFactor() = default;
-
- // AuthenticateAuthFactor authenticates user credentials if they exist. This
- // currently uses VaultKeyset, but will eventually use AuthBlocks and USS.
- MountError AuthenticateAuthFactor(const Credentials& credential,
- bool is_ephemeral_user) override;
-
- // Transfer ownership of password verifier that can be used to verify
- // credentials during unlock.
- std::unique_ptr<CredentialVerifier> TakeCredentialVerifier() override;
- // Returns the key data with which this AuthFactor is authenticated with.
- const cryptohome::KeyData& GetKeyData() const override;
-
- // Return VaultKeyset of the authenticated user.
- const VaultKeyset* vault_keyset() const override {
- return vault_keyset_.get();
- };
-
- // Returns FileSystemKeyset of the authenticated user.
- const FileSystemKeyset GetFileSystemKeyset() const override;
-
- private:
- // The creator of the PasswordAuthFactor object is responsible for the
- // life of KeysetManagement object.
- KeysetManagement* keyset_management_;
- // This is used by User Session to verify users credentials at unlock.
- std::unique_ptr<CredentialVerifier> credential_verifier_;
- // Used to decrypt/ encrypt & store credentials.
- std::unique_ptr<VaultKeyset> vault_keyset_;
- // Used to store key meta data.
- cryptohome::KeyData key_data_;
-};
-
-} // namespace cryptohome
-
-#endif // CRYPTOHOME_AUTH_FACTORS_PASSWORD_AUTH_FACTOR_H_
diff --git a/cryptohome/auth_session.cc b/cryptohome/auth_session.cc
index 4e4610b..8e3608a 100644
--- a/cryptohome/auth_session.cc
+++ b/cryptohome/auth_session.cc
@@ -12,7 +12,7 @@
#include <brillo/cryptohome.h>
#include <cryptohome/scrypt_verifier.h>
-#include "cryptohome/auth_factors/password_auth_factor.h"
+#include "cryptohome/auth_factor/auth_factor.h"
#include "cryptohome/keyset_management.h"
#include "cryptohome/storage/mount_utils.h"
#include "cryptohome/vault_keyset.h"
@@ -139,7 +139,7 @@
}
if (authorization_request.key().data().type() == KeyData::KEY_TYPE_PASSWORD ||
authorization_request.key().data().type() == KeyData::KEY_TYPE_KIOSK) {
- auth_factor_ = std::make_unique<PasswordAuthFactor>(keyset_management_);
+ auth_factor_ = std::make_unique<AuthFactor>(keyset_management_);
}
code = auth_factor_->AuthenticateAuthFactor(*credentials, is_ephemeral_user_);
diff --git a/cryptohome/auth_session.h b/cryptohome/auth_session.h
index 94126a8..76256b4 100644
--- a/cryptohome/auth_session.h
+++ b/cryptohome/auth_session.h
@@ -14,7 +14,7 @@
#include <base/unguessable_token.h>
#include <brillo/secure_blob.h>
-#include "cryptohome/auth_factors/auth_factor.h"
+#include "cryptohome/auth_factor/auth_factor.h"
#include "cryptohome/credential_verifier.h"
#include "cryptohome/credentials.h"
#include "cryptohome/keyset_management.h"
diff --git a/cryptohome/libs/BUILD.gn b/cryptohome/libs/BUILD.gn
index 2cc23e0..d5ef514 100644
--- a/cryptohome/libs/BUILD.gn
+++ b/cryptohome/libs/BUILD.gn
@@ -206,7 +206,7 @@
"../auth_blocks/tpm_bound_to_pcr_auth_block.cc",
"../auth_blocks/tpm_ecc_auth_block.cc",
"../auth_blocks/tpm_not_bound_to_pcr_auth_block.cc",
- "../auth_factors/password_auth_factor.cc",
+ "../auth_factor/auth_factor.cc",
"../auth_session.cc",
"../auth_session_manager.cc",
"../crc8.c",
