0

[crypto] Fix UMA recording TPM key creation from a wrapped key on Win

Crypto.TPMOperation.Win.WrappedKeyCreation{RSA,ECDSA}.Error UMA records
wrapped key export operation whereas it's supposed to record wrapped key
import (see Crypto.TPMOperation.WrappedKeyCreation{RSA,ECDSA} as an
example).

This CL reworks TPM error histograms around wrapped keys:
- Adds `Crypto.TPMOperation.Win.WrappedKeyCreation.Error` histogram that
  records the wrapped key import. This histogram doesn't have an
  algorithm suffix since the selected algorithm is unknown until the key
  is successfully imported.
- Renames `Crypto.TPMOperation.Win.WrappedKeyCreation{RSA,ECDSA}.Error`
  as `Crypto.TPMOperation.Win.WrappedKeyExport{RSA,ECDSA}.Error` to
  avoid the confusion with the new histogram above and also other
  histograms with the "WrappedKeyCreation" suffix.

OBSOLETE_HISTOGRAM[Crypto.TPMOperation.Win.WrappedKeyCreationECDSA.Error]=Replaced by Crypto.TPMOperation.Win.WrappedKeyExportECDSA.Error
OBSOLETE_HISTOGRAM[Crypto.TPMOperation.Win.WrappedKeyCreationRSA.Error]=Replaced by Crypto.TPMOperation.Win.WrappedKeyExportRSA.Error

Bug: 347025280
Change-Id: Ib165e9849b5799a8152ca2dea2964d73dffa7a12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6167112
Reviewed-by: David Schinazi <dschinazi@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1405982}
This commit is contained in:
Alex Ilin
2025-01-14 02:25:12 -08:00
committed by Chromium LUCI CQ
parent fd8642e3e2
commit ac03cf4cff
4 changed files with 47 additions and 16 deletions

@ -287,6 +287,8 @@ std::string OperationToString(TPMOperation operation) {
return "NewKeyCreation";
case TPMOperation::kWrappedKeyCreation:
return "WrappedKeyCreation";
case TPMOperation::kWrappedKeyExport:
return "WrappedKeyExport";
}
}

@ -13,10 +13,18 @@
namespace crypto {
enum class TPMOperation {
// An operation to sign data with a TPM key.
kMessageSigning,
// An operation to verify a TPM signature.
kMessageVerify,
// An operation to create a TPM key from a wrapped key or a similar
// representation identifying a TPM key.
kWrappedKeyCreation,
// An operation to create a new TPM-protected key.
kNewKeyCreation,
// An operation to export a wrapped key (or a similar representation
// identifying a TPM key) from an existing TPM key.
kWrappedKeyExport,
};
// Converts the given `operation` to a string representation.

@ -54,13 +54,19 @@ const char kMetricVirtualOpenStorageError[] =
void LogTPMOperationError(
TPMOperation operation,
SECURITY_STATUS status,
SignatureVerifier::SignatureAlgorithm selected_algorithm) {
std::optional<SignatureVerifier::SignatureAlgorithm> selected_algorithm) {
static constexpr char kCreateKeyErrorStatusHistogramFormat[] =
"Crypto.TPMOperation.Win.%s%s.Error";
// Only `kWrappedKeyCreation` could and should be recorded without
// `selected_algorithm`.
CHECK_EQ(!selected_algorithm.has_value(),
operation == TPMOperation::kWrappedKeyCreation);
std::string algorithm_string =
selected_algorithm ? AlgorithmToString(*selected_algorithm) : "";
base::UmaHistogramSparse(
base::StringPrintf(kCreateKeyErrorStatusHistogramFormat,
OperationToString(operation).c_str(),
AlgorithmToString(selected_algorithm).c_str()),
algorithm_string.c_str()),
status);
}
@ -456,7 +462,7 @@ class UnexportableKeyProviderWin : public UnexportableKeyProvider {
/*dwLegacyKeySpec=*/0, /*dwFlags=*/0);
if (FAILED(creation_status)) {
LogTPMOperationError(TPMOperation::kNewKeyCreation, creation_status,
algo.value());
algo);
return nullptr;
}
@ -468,8 +474,8 @@ class UnexportableKeyProviderWin : public UnexportableKeyProvider {
const base::expected<std::vector<uint8_t>, SECURITY_STATUS> wrapped_key =
ExportKey(key.get(), BCRYPT_OPAQUE_KEY_BLOB);
if (!wrapped_key.has_value()) {
LogTPMOperationError(TPMOperation::kWrappedKeyCreation,
wrapped_key.error(), algo.value());
LogTPMOperationError(TPMOperation::kWrappedKeyExport, wrapped_key.error(),
algo);
return nullptr;
}
@ -807,11 +813,14 @@ bool LoadWrappedTPMKey(base::span<const uint8_t> wrapped,
return false;
}
if (FAILED(NCryptImportKey(
provider.get(), /*hImportKey=*/NULL, BCRYPT_OPAQUE_KEY_BLOB,
/*pParameterList=*/nullptr, ScopedNCryptKey::Receiver(key).get(),
const_cast<PBYTE>(wrapped.data()), wrapped.size(),
/*dwFlags=*/NCRYPT_SILENT_FLAG))) {
SECURITY_STATUS import_status = NCryptImportKey(
provider.get(), /*hImportKey=*/NULL, BCRYPT_OPAQUE_KEY_BLOB,
/*pParameterList=*/nullptr, ScopedNCryptKey::Receiver(key).get(),
const_cast<PBYTE>(wrapped.data()), wrapped.size(),
/*dwFlags=*/NCRYPT_SILENT_FLAG);
if (FAILED(import_status)) {
LogTPMOperationError(TPMOperation::kWrappedKeyCreation, import_status,
std::nullopt);
return false;
}
return true;

@ -190,20 +190,32 @@ chromium-metrics-reviews@google.com.
</token>
</histogram>
<histogram name="Crypto.TPMOperation.Win.WrappedKeyCreation.Error"
enum="Win32TpmErrorCodes" expires_after="2025-08-01">
<owner>seblalancette@chromium.org</owner>
<owner>kristianm@chromium.org</owner>
<owner>src/crypto/OWNERS</owner>
<summary>
Collected on Windows when an operation to create a TPM-protected key from a
wrapped key failed. Represents the Win32 status returned by the API
corresponding with that operation.
</summary>
</histogram>
<histogram name="Crypto.TPMOperation.Win.{Operation}{SignatureAlgorithm}.Error"
enum="Win32TpmErrorCodes" expires_after="2025-08-01">
<owner>seblalancette@chromium.org</owner>
<owner>kristianm@chromium.org</owner>
<owner>src/crypto/OWNERS</owner>
<summary>
Collected on Windows when an operation failed while using a TPM-protected
key. Represents the Win32 status returned by the API corresponding with that
operation.
Collected on Windows when a {Operation} operation failed while using a
TPM-protected {SignatureAlgorithm} key. Represents the Win32 status returned
by the API corresponding with that operation.
</summary>
<token key="Operation">
<variant name="MessageSigning"/>
<variant name="NewKeyCreation"/>
<variant name="WrappedKeyCreation"/>
<variant name="MessageSigning" summary="signing"/>
<variant name="NewKeyCreation" summary="new key creation"/>
<variant name="WrappedKeyExport" summary="wrapped key export"/>
</token>
<token key="SignatureAlgorithm">
<variant name="ECDSA"/>