cryptohome: TpmEccAuthBlock checks inputs
Check in TpmEccAuthBlock's Create() and Derive() operations whether
the auth inputs have all necessary fields, instead of crashing when
they're missing.
BUG=b:219612327
TEST=cros_workon_make --board=$BOARD --test cryptohome
Change-Id: I99db5864936a29eefd9f14c896324e57f7a39337
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/3624299
Tested-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Betul Soysal <betuls@google.com>
Reviewed-by: Greg Kerr <kerrnel@chromium.org>
diff --git a/cryptohome/auth_blocks/auth_block_unittest.cc b/cryptohome/auth_blocks/auth_block_unittest.cc
index cbb0a2d..8c3b6c9 100644
--- a/cryptohome/auth_blocks/auth_block_unittest.cc
+++ b/cryptohome/auth_blocks/auth_block_unittest.cc
@@ -1495,6 +1495,40 @@
->local_crypto_error());
}
+// Test the Create operation fails when there's no user_input provided.
+TEST(TpmEccAuthBlockTest, CreateFailNoUserInput) {
+ // Prepare.
+ std::string obfuscated_username = "OBFUSCATED_USERNAME";
+ NiceMock<MockTpm> tpm;
+ NiceMock<MockCryptohomeKeysManager> cryptohome_keys_manager;
+ TpmEccAuthBlock auth_block(&tpm, &cryptohome_keys_manager);
+ AuthInput auth_input = {.obfuscated_username = obfuscated_username};
+
+ // Test.
+ AuthBlockState auth_state;
+ KeyBlobs vkk_data;
+ EXPECT_EQ(auth_block.Create(auth_input, &auth_state, &vkk_data)
+ ->local_crypto_error(),
+ CryptoError::CE_OTHER_CRYPTO);
+}
+
+// Test the Create operation fails when there's no obfuscated_username provided.
+TEST(TpmEccAuthBlockTest, CreateFailNoObfuscated) {
+ // Prepare.
+ brillo::SecureBlob user_input(20, 'C');
+ NiceMock<MockTpm> tpm;
+ NiceMock<MockCryptohomeKeysManager> cryptohome_keys_manager;
+ TpmBoundToPcrAuthBlock auth_block(&tpm, &cryptohome_keys_manager);
+ AuthInput auth_input = {.user_input = user_input};
+
+ // Test.
+ AuthBlockState auth_state;
+ KeyBlobs vkk_data;
+ EXPECT_EQ(auth_block.Create(auth_input, &auth_state, &vkk_data)
+ ->local_crypto_error(),
+ CryptoError::CE_OTHER_CRYPTO);
+}
+
// Test SealToPcr in TpmEccAuthBlock::Create failed as expected.
TEST(TpmEccAuthBlockTest, CreateSealToPcrFailTest) {
// Set up inputs to the test.
@@ -1679,6 +1713,22 @@
EXPECT_EQ(key_out_data.vkk_iv.value(), key_out_data.chaps_iv.value());
}
+// Test TpmEccAuthBlock::Derive failure when there's no auth_input provided.
+TEST(TpmEccAuthBlockTest, DeriveFailNoAuthInput) {
+ TpmEccAuthBlockState auth_block_state = GetDefaultEccAuthBlockState();
+ AuthBlockState auth_state{.state = std::move(auth_block_state)};
+
+ NiceMock<MockTpm> tpm;
+ NiceMock<MockCryptohomeKeysManager> cryptohome_keys_manager;
+ TpmEccAuthBlock auth_block(&tpm, &cryptohome_keys_manager);
+
+ KeyBlobs key_out_data;
+ AuthInput auth_input;
+ EXPECT_EQ(auth_block.Derive(auth_input, auth_state, &key_out_data)
+ ->local_crypto_error(),
+ CryptoError::CE_OTHER_CRYPTO);
+}
+
// Test GetEccAuthValue in TpmEccAuthBlock::Derive failed as expected.
TEST(TpmEccAuthBlockTest, DeriveGetEccAuthFailTest) {
TpmEccAuthBlockState auth_block_state = GetDefaultEccAuthBlockState();
diff --git a/cryptohome/auth_blocks/tpm_ecc_auth_block.cc b/cryptohome/auth_blocks/tpm_ecc_auth_block.cc
index 9901ac3..2cac66a 100644
--- a/cryptohome/auth_blocks/tpm_ecc_auth_block.cc
+++ b/cryptohome/auth_blocks/tpm_ecc_auth_block.cc
@@ -342,6 +342,21 @@
CryptoStatus TpmEccAuthBlock::Create(const AuthInput& auth_input,
AuthBlockState* auth_block_state,
KeyBlobs* key_blobs) {
+ if (!auth_input.user_input.has_value()) {
+ LOG(ERROR) << "Missing user_input";
+ return MakeStatus<CryptohomeCryptoError>(
+ CRYPTOHOME_ERR_LOC(kLocTpmEccAuthBlockNoUserInputInCreate),
+ ErrorActionSet({ErrorAction::kDevCheckUnexpectedState}),
+ CryptoError::CE_OTHER_CRYPTO);
+ }
+ if (!auth_input.obfuscated_username.has_value()) {
+ LOG(ERROR) << "Missing obfuscated_username";
+ return MakeStatus<CryptohomeCryptoError>(
+ CRYPTOHOME_ERR_LOC(kLocTpmEccAuthBlockNoUsernameInCreate),
+ ErrorActionSet({ErrorAction::kDevCheckUnexpectedState}),
+ CryptoError::CE_OTHER_CRYPTO);
+ }
+
return TryCreate(auth_input, auth_block_state, key_blobs,
kTryCreateMaxRetryCount);
}
@@ -349,6 +364,14 @@
CryptoStatus TpmEccAuthBlock::Derive(const AuthInput& auth_input,
const AuthBlockState& state,
KeyBlobs* key_out_data) {
+ if (!auth_input.user_input.has_value()) {
+ LOG(ERROR) << "Missing user_input";
+ return MakeStatus<CryptohomeCryptoError>(
+ CRYPTOHOME_ERR_LOC(kLocTpmEccAuthBlockNoUserInputInDerive),
+ ErrorActionSet({ErrorAction::kDevCheckUnexpectedState}),
+ CryptoError::CE_OTHER_CRYPTO);
+ }
+
const TpmEccAuthBlockState* auth_state;
if (!(auth_state = std::get_if<TpmEccAuthBlockState>(&state.state))) {
DLOG(FATAL) << "Invalid AuthBlockState";
diff --git a/cryptohome/error/locations.h b/cryptohome/error/locations.h
index 661a42c..6c78d5c 100644
--- a/cryptohome/error/locations.h
+++ b/cryptohome/error/locations.h
@@ -541,6 +541,12 @@
kLocAuthFactorCreateKeyBlobsFailedInCreate = 357,
/* ./auth_factor/auth_factor.cc */
kLocAuthFactorDeriveFailedInAuth = 358,
+ /* ./auth_blocks/tpm_ecc_auth_block.cc */
+ kLocTpmEccAuthBlockNoUserInputInCreate = 359,
+ /* ./auth_blocks/tpm_ecc_auth_block.cc */
+ kLocTpmEccAuthBlockNoUsernameInCreate = 360,
+ /* ./auth_blocks/tpm_ecc_auth_block.cc */
+ kLocTpmEccAuthBlockNoUserInputInDerive = 361,
// End of generated content.
};
// The enum value should not exceed 65535, otherwise we need to adjust the way
