Skip to content

Fix .NET and mage compatibility in KMS CNG Provider #71

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hlshen wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix .NET and mage compatibility in KMS CNG Provider #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8cd964c
fix: unblock .NET and mage compatibility in CNG provider
hlshen Jun 11, 2026
a62e76c
test: add unit tests for .NET compatibility fixes
hlshen Jun 11, 2026
cb8e8ae
fix: ignore NCRYPT_PERSIST_ONLY_FLAG in OpenKey
hlshen Jun 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions kmscng/main/bridge.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ namespace cloud_kms::kmscng {
namespace {

absl::Status ValidateFlags(uint32_t flags) {
// Ignore NCRYPT_PERSIST_ONLY_FLAG (0x40000000) as we are a read-only provider.
flags &= ~NCRYPT_PERSIST_ONLY_FLAG;

if (flags != 0 && flags != NCRYPT_SILENT_FLAG && flags != BCRYPT_PAD_PKCS1) {
return NewInvalidArgumentError(
absl::StrFormat("unsupported flag specified: %u", flags), NTE_BAD_FLAGS,
Expand Down Expand Up @@ -189,6 +192,7 @@ absl::Status OpenKey(__inout NCRYPT_PROV_HANDLE hProvider,
}
dwFlags = dwFlags & ~NCRYPT_SILENT_FLAG;
dwFlags = dwFlags & ~NCRYPT_MACHINE_KEY_FLAG;
dwFlags = dwFlags & ~NCRYPT_PERSIST_ONLY_FLAG;
if (dwFlags != 0) {
return NewInvalidArgumentError(
absl::StrFormat("unsupported flag specified: %u", dwFlags),
Expand Down
56 changes: 44 additions & 12 deletions kmscng/main/bridge_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
namespace cloud_kms::kmscng {
namespace {

constexpr uint32_t kInvalidFlags = 0xFFFFFFFF;

// TODO(b/270419822): drop these once crypto_utils has been migrated to common.
using cloud_kms::kmsp11::EcdsaVerifyP1363;
using cloud_kms::kmsp11::ParseX509PublicKeyDer;
Expand Down Expand Up @@ -117,6 +119,15 @@ TEST(BridgeTest, OpenProviderSuccess) {
EXPECT_OK(FreeProvider(provider_handle));
}

TEST(BridgeTest, OpenProviderPersistOnlyFlagSuccess) {
NCRYPT_PROV_HANDLE provider_handle;
EXPECT_OK(OpenProvider(&provider_handle, kProviderName.data(),
NCRYPT_PERSIST_ONLY_FLAG));

// Clean up memory.
EXPECT_OK(FreeProvider(provider_handle));
}

TEST(BridgeTest, OpenProviderInvalidHandle) {
EXPECT_THAT(OpenProvider(nullptr, kProviderName.data(), 0),
StatusSsIs(NTE_INVALID_PARAMETER));
Expand All @@ -131,7 +142,7 @@ TEST(BridgeTest, OpenProviderUnexpectedName) {
TEST(BridgeTest, OpenProviderInvalidFlag) {
NCRYPT_PROV_HANDLE provider_handle;
EXPECT_THAT(OpenProvider(&provider_handle, kProviderName.data(),
NCRYPT_PERSIST_ONLY_FLAG),
kInvalidFlags),
StatusSsIs(NTE_BAD_FLAGS));
}

Expand Down Expand Up @@ -237,7 +248,7 @@ TEST(BridgeTest, GetProviderPropertyInvalidFlag) {
DWORD output_size = 0;
EXPECT_THAT(GetProviderProperty(provider_handle, NCRYPT_IMPL_TYPE_PROPERTY,
nullptr, sizeof(DWORD), &output_size,
NCRYPT_PERSIST_ONLY_FLAG),
kInvalidFlags),
StatusSsIs(NTE_BAD_FLAGS));

// Clean up memory.
Expand Down Expand Up @@ -291,7 +302,7 @@ TEST(BridgeTest, SetProviderPropertyInputNull) {

EXPECT_THAT(
SetProviderProperty(provider_handle, NCRYPT_IMPL_TYPE_PROPERTY, nullptr,
sizeof(DWORD), NCRYPT_PERSIST_ONLY_FLAG),
sizeof(DWORD), kInvalidFlags),
StatusSsIs(NTE_INVALID_PARAMETER));

// Clean up memory.
Expand Down Expand Up @@ -333,7 +344,7 @@ TEST(BridgeTest, SetProviderPropertyInvalidFlag) {
DWORD input = NCRYPT_IMPL_SOFTWARE_FLAG;
EXPECT_THAT(SetProviderProperty(provider_handle, NCRYPT_IMPL_TYPE_PROPERTY,
reinterpret_cast<uint8_t*>(&input),
sizeof(DWORD), NCRYPT_PERSIST_ONLY_FLAG),
sizeof(DWORD), kInvalidFlags),
StatusSsIs(NTE_BAD_FLAGS));

// Clean up memory.
Expand All @@ -360,6 +371,27 @@ TEST(BridgeTest, OpenKeySuccess) {
EXPECT_OK(FreeKey(provider_handle, key_handle));
}

TEST(BridgeTest, OpenKeyPersistOnlyFlagSuccess) {
ASSERT_OK_AND_ASSIGN(auto fake_server, fakekms::Server::New());
auto client = fake_server->NewClient();

kms_v1::CryptoKeyVersion ckv = NewCryptoKeyVersion(client.get());

Provider provider;
SetFakeKmsProviderProperties(&provider, fake_server->listen_addr());

NCRYPT_PROV_HANDLE provider_handle =
reinterpret_cast<NCRYPT_PROV_HANDLE>(&provider);
NCRYPT_KEY_HANDLE key_handle;
EXPECT_OK(OpenKey(provider_handle, &key_handle,
StringToWide(ckv.name()).data(), AT_SIGNATURE,
NCRYPT_PERSIST_ONLY_FLAG));
EXPECT_NE(key_handle, 0);

// Clean up memory.
EXPECT_OK(FreeKey(provider_handle, key_handle));
}

TEST(BridgeTest, OpenKeyEc384Success) {
ASSERT_OK_AND_ASSIGN(auto fake_server, fakekms::Server::New());
auto client = fake_server->NewClient();
Expand Down Expand Up @@ -452,7 +484,7 @@ TEST(BridgeTest, OpenKeyInvalidFlag) {

NCRYPT_KEY_HANDLE key_handle;
EXPECT_THAT(OpenKey(provider_handle, &key_handle, L"some_key_name",
AT_SIGNATURE, NCRYPT_PERSIST_ONLY_FLAG),
AT_SIGNATURE, kInvalidFlags),
StatusSsIs(NTE_BAD_FLAGS));

// Clean up memory.
Expand Down Expand Up @@ -765,7 +797,7 @@ TEST(BridgeTest, ExportKeyInvalidFlag) {
DWORD output_size;
EXPECT_THAT(
ExportKey(provider_handle, key_handle, 0, BCRYPT_ECCPUBLIC_BLOB, nullptr,
nullptr, 0, &output_size, NCRYPT_PERSIST_ONLY_FLAG),
nullptr, 0, &output_size, kInvalidFlags),
StatusSsIs(NTE_BAD_FLAGS));

// Clean up memory.
Expand All @@ -790,7 +822,7 @@ TEST(BridgeTest, ExportKeyOutputBufferTooShort) {
DWORD output_size;
EXPECT_THAT(
ExportKey(provider_handle, key_handle, 0, BCRYPT_ECCPUBLIC_BLOB, nullptr,
&output, 1, &output_size, NCRYPT_PERSIST_ONLY_FLAG),
&output, 1, &output_size, kInvalidFlags),
StatusSsIs(NTE_BAD_FLAGS));

// Clean up memory.
Expand Down Expand Up @@ -959,7 +991,7 @@ TEST(BridgeTest, GetKeyPropertyInvalidFlag) {
DWORD output_size;
EXPECT_THAT(GetKeyProperty(provider_handle, key_handle,
NCRYPT_KEY_USAGE_PROPERTY, &output, sizeof(DWORD),
&output_size, NCRYPT_PERSIST_ONLY_FLAG),
&output_size, kInvalidFlags),
StatusSsIs(NTE_BAD_FLAGS));

// Clean up memory.
Expand Down Expand Up @@ -1192,7 +1224,7 @@ TEST(BridgeTest, EnumKeysInvalidFlag) {

NCryptKeyName* output;
EXPECT_THAT(EnumKeys(provider_handle, nullptr, &output, nullptr,
NCRYPT_PERSIST_ONLY_FLAG),
kInvalidFlags),
StatusSsIs(NTE_BAD_FLAGS));

// Clean up memory.
Expand Down Expand Up @@ -1406,7 +1438,7 @@ TEST(BridgeTest, SignHashInvalidFlag) {
std::vector<uint8_t> digest(32, '\1');
EXPECT_THAT(SignHash(provider_handle, key_handle, nullptr, digest.data(),
digest.size(), &output, 0, &output_size,
NCRYPT_PERSIST_ONLY_FLAG),
kInvalidFlags),
StatusSsIs(NTE_BAD_FLAGS));

// Clean up memory.
Expand Down Expand Up @@ -1445,7 +1477,7 @@ TEST(BridgeTest, IsAlgSupportedInvalidFlag) {

DWORD output_size = 0;
EXPECT_THAT(IsAlgSupported(provider_handle, BCRYPT_ECDSA_P256_ALGORITHM,
NCRYPT_PERSIST_ONLY_FLAG),
kInvalidFlags),
StatusSsIs(NTE_BAD_FLAGS));

// Clean up memory.
Expand Down Expand Up @@ -1537,7 +1569,7 @@ TEST(BridgeTest, EnumAlgorithmsInvalidFlag) {
NCryptAlgorithmName* alg_pointer;
EXPECT_THAT(
EnumAlgorithms(provider_handle, NCRYPT_SIGNATURE_OPERATION, &output_size,
&alg_pointer, NCRYPT_PERSIST_ONLY_FLAG),
&alg_pointer, kInvalidFlags),
StatusSsIs(NTE_BAD_FLAGS));

// Clean up memory.
Expand Down
7 changes: 5 additions & 2 deletions kmscng/object.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ absl::StatusOr<kms_v1::PublicKey> GetPublicKey(const KmsClient& client,

absl::flat_hash_map<std::wstring, std::string> BuildInfo(
NCRYPT_PROV_HANDLE prov_handle, std::string key_name,
AlgorithmDetails details) {
AlgorithmDetails details, uint32_t key_size_bits) {
return {
{NCRYPT_ALGORITHM_GROUP_PROPERTY, WideToBytes(details.algorithm_group)},
{NCRYPT_ALGORITHM_PROPERTY, WideToBytes(details.algorithm_property)},
Expand All @@ -71,6 +71,8 @@ absl::flat_hash_map<std::wstring, std::string> BuildInfo(
{NCRYPT_PROVIDER_HANDLE_PROPERTY,
std::string(reinterpret_cast<char*>(&prov_handle),
sizeof(NCRYPT_PROV_HANDLE))},
{NCRYPT_LENGTH_PROPERTY, Uint32ToBytes(key_size_bits)},
{L"PublicKeyLength", Uint32ToBytes(key_size_bits)},
};
}

Expand Down Expand Up @@ -130,7 +132,8 @@ absl::StatusOr<Object*> Object::New(NCRYPT_PROV_HANDLE prov_handle,
}
ASSIGN_OR_RETURN(AlgorithmDetails alg_details,
GetDetails(public_key.algorithm()));
auto info = BuildInfo(prov_handle, key_name, alg_details);
uint32_t key_size_bits = EVP_PKEY_bits(pub->get());
auto info = BuildInfo(prov_handle, key_name, alg_details, key_size_bits);

// using `new` to invoke a private constructor
return new Object(key_name, std::move(client), public_key.algorithm(),
Expand Down
10 changes: 10 additions & 0 deletions kmscng/object_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,5 +79,15 @@ TEST_F(ObjectTest, GetObjectPropertyKeyUsageSuccess) {
IsOkAndHolds(Uint32ToBytes(NCRYPT_ALLOW_SIGNING_FLAG)));
}

TEST_F(ObjectTest, GetObjectPropertyLengthSuccess) {
EXPECT_THAT(object_->GetProperty(NCRYPT_LENGTH_PROPERTY),
IsOkAndHolds(Uint32ToBytes(256)));
}

TEST_F(ObjectTest, GetObjectPropertyPublicKeyLengthSuccess) {
EXPECT_THAT(object_->GetProperty(L"PublicKeyLength"),
IsOkAndHolds(Uint32ToBytes(256)));
}

} // namespace
} // namespace cloud_kms::kmscng