Pluggable cryptography and hardware-held private keys (#4190) - #4192
Draft
marcschier wants to merge 22 commits into
Draft
Pluggable cryptography and hardware-held private keys (#4190)#4192marcschier wants to merge 22 commits into
marcschier wants to merge 22 commits into
Conversation
Captures the design research for making cryptographic operations replaceable by an alternative library, an offboard service, or hardware (TPM 2.0 / HSM / PKCS#11 / cloud KMS), without any performance cost in the default all-software configuration, and extending to certificates so private keys need never be materialized in process memory. Key findings recorded in plans/cryptooffboard.md: - .NET's RSA and ECDsa are abstract and the stack already routes every private-key operation through them, so roughly 85% of the stack works unchanged with a hardware-backed key. A bespoke asymmetric crypto interface would reject the ready-made RSACng (TPM), Pkcs11Interop and RSAKeyVault implementations. - ECDH key agreement always uses a freshly generated ephemeral key and never the certificate key, so hardware is only ever asked to Sign and Decrypt. - Hardware offload touches only cold-path operations, so the per-chunk symmetric path needs no change at all; a seam there would cost ~0.018%. - The actual work is twelve certificate persistence and export call sites that assume a PFX round-trip always succeeds, each listed with file and line. - Most required seams already exist: ICertificateStoreProvider, IPushCertificateKeyGenerator, X509SignatureGenerator, and ITokenIssuer as the async offboard-signing precedent. Two decisions are flagged for maintainer sign-off: the synchronous BCL RSA.SignHash contract versus the no-sync-over-async rule for remote KMS, and excluding HTTPS from the hardware-key scope on Windows and macOS. Documentation only; no code changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
Incorporates six additional requirements raised in design review, several of which change the shape of the solution rather than refining it. New in the plan: - Selection and resolution model (A2, A4). Provider choice is now a resolution problem over three discriminators - purpose, security policy URI and certificate type - with a precedence chain modelled directly on the existing IHistorianProviderRegistry. This is what allows an instance key in a TPM, user identity keys in a KMS and everything else in software simultaneously. Resolution happens at binding time, never per operation, so the performance guarantee is unaffected. - Provider capabilities and provenance (A5, A6). Every provider declares what it can serve and how it is validated. Selection and compliance filtering become two queries over one capability declaration, which unifies mix-and-match, missing-profile contribution, FIPS filtering and audit into a single mechanism instead of four bolted-on features. - Contributing missing security policies (A3), scoped as a separate later epic. Two research findings materially change scope: - Registrable security policies are a far larger change than pluggable crypto. Ten interlocking blockers were found, including closed C# enums for every algorithm and policy dictionaries built by reflection over typeof(SecurityPolicies).GetFields(). Isolated as Phase 6 with its own design issue so it does not gate the hardware-key work. The acceptance test is lighting up ECC_curve25519/448, which are fully implemented but dead because the CURVE25519 symbol is defined in no project file. - The current default configuration is not FIPS-clean. ChaCha20-Poly1305 and brainpool policies are advertised by default and neither is FIPS-approved; net472/net48 additionally use the non-validated BouncyCastle NuGet. A FIPS-compliant default therefore requires a compliance profile that filters the advertised policy set, not merely a statement about which library performs the maths. .NET holds no CMVP certificate of its own, so the plan records precisely what may and may not be claimed per platform and per TFM. The audit requirement needs no new infrastructure: roughly thirty Report* audit events, the Part 12 ServerConfiguration node and a deprecated-policy LogLevel.Warning pattern already exist and are reused. The one genuine gap is that no security or crypto metrics exist today. Phases reorganised from six to eight. Seven new risks recorded, including the A3/A5 tension - the profiles .NET cannot do natively are precisely the non-FIPS ones - and three new open questions for maintainers. Documentation only; no code changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
Adds the ability to hold a private key alongside a certificate instead of attached to it, which is how a key resident in a TPM, an HSM, a PKCS#11 token or a remote key service has to be represented. Why this is needed ------------------ An empirical probe on Windows / .NET 10.0.10 disproved the assumption that a hardware key could simply be attached with X509Certificate2.CopyWithPrivateKey: CopyWithPrivateKey(custom non-exportable RSA) -> CryptographicException CopyWithPrivateKey(RSACng, ExportPolicy=None) -> works detached key held beside the certificate -> works The Windows certificate pal only has fast paths for RSACng and RSACryptoServiceProvider. For anything else it falls back to ExportParameters(true), which a non-extractable key refuses by definition. So while the Windows CNG/TPM path works, a PKCS#11 wrapper, an Azure Key Vault RSA or any bespoke offboard provider cannot be bound to a certificate that way at all. CertificateRequest.CreateSelfSigned is affected for the same reason, since it calls CopyWithPrivateKey internally. The detached model sidesteps the platform entirely: the wrapper holds the certificate and the key side by side and never calls CopyWithPrivateKey, so it behaves identically on every platform and target framework. What changed ------------ Certificate gains CopyWithDetachedPrivateKey for RSA and ECDsa, a HasDetachedPrivateKey property, and reports HasPrivateKey true when a detached key is present. GetRSAPrivateKey and GetECDsaPrivateKey return the detached key when one is set. The key participates in the existing reference counted CertificateCore lifetime and is disposed once, with the last handle, unless the caller opts out with ownsPrivateKey false. Callers of GetRSAPrivateKey own the returned object and dispose it, because the platform normally hands out a fresh handle per call. Returning the shared detached key directly would let the first caller destroy it for everyone, so each call returns an independent non-owning view (NonOwningRsa / NonOwningECDsa) that forwards every operation and ignores disposal. Test support ------------ NonExportableRsa and NonExportableECDsa are added to Opc.Ua.Core.TestFramework. They perform every operation correctly but refuse to surrender private key material, mirroring a CNG key created with CngExportPolicies.None, and expose a PrivateKeyExportAttempts counter so a test can assert that a code path never reaches for private material rather than merely tolerating the failure. Only ExportParameters needs to reject the request; the base class funnels the ExportRSAPrivateKey, ExportPkcs8PrivateKey and encrypted variants through it. Opc.Ua.Security.Certificates.Tests now references Opc.Ua.Core.TestFramework, which its own project comment already described as shared with that suite. Verified: 262 tests pass on net10.0 and net48 with no regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
The inner X509Certificate2 of a detached key certificate carries no private key, so a PKCS#12 export would succeed and quietly produce a file without one. That is worse than failing: a caller asking for the key would be handed a useless blob and only discover it later. Export now throws for PKCS#12 when the key is detached, while exporting the public certificate keeps working. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
The stack repeatedly took a PKCS#12 round-trip for granted, so a certificate whose key is not extractable failed in several places that had nothing to do with the key itself. Such keys are the norm once a key lives in a TPM, an HSM, a PKCS#11 token or a remote key service, and they already occur today with a CNG key created with CngExportPolicies.None. This is also the root of the class of bug reported in #2637. Nine call sites are addressed: DefaultCertificateFactory.DetachFromSourceKey and X509Utils.CreateCopyWithPrivateKey both exist to escape an ephemeral key handle owned by a caller that is about to dispose it. A non-extractable key is by definition not such a handle, so there is nothing to detach and the certificate is returned as-is. DirectoryCertificateStore.AddAsync now stores the public certificate and warns, rather than failing. A key that cannot be exported was never going to reach the disk, and it remains reachable through the store it actually lives in. X509CertificateStore.AddAsync on Windows re-imports the key with PersistKeySet so the platform store can persist it. When the key is not extractable it is already held in a key storage provider the store can reach, which is exactly what the re-import was trying to achieve, so the original certificate is added. PEMWriter reported the raw platform error, which says nothing useful. Private key export now fails with NotSupportedException stating the actual constraint. ConfigurationNodeManager carried the previous private key over to a new certificate during the Part 12 UpdateCertificate flow with no error handling at all. It now reports BadSecurityChecksFailed and points at CreateSigningRequest with RegeneratePrivateKey, which is the flow that works for keys that cannot be copied. SharedKeyValuePendingCertificateKeyStore stages a key for another replica to collect. A key that cannot leave its device cannot be staged, so the store now reports that it cannot hold it instead of throwing. Verified: 500 tests in the Core security namespace pass on net10.0 and 492 on net48, plus 263 in Opc.Ua.Security.Certificates.Tests, with no regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
Exercises the operations the secure channel performs during OpenSecureChannel and ActivateSession using a private key that can never be exported: the asymmetric signature over the handshake, verification of the peer signature, the RSA unwrapping of the peer secret, and the key size helpers the channel uses to size its buffers. Each test asserts PrivateKeyExportAttempts is zero, so a path that merely tolerated an export failure would still fail the test. Covers Basic256Sha256, Aes128_Sha256_RsaOaep, ECC_nistP256 and ECC_nistP384. This is the substance of the claim that the stack works unchanged with hardware held keys: if these pass, the key never has to leave its device for a channel to be established. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
The detached key model added earlier is what every provider outside the platform key storage providers has to use: PKCS#11 tokens, cloud key services and anything else that cannot satisfy X509Certificate2.CopyWithPrivateKey. Until now it had unit coverage but nothing exercised it through an actual certificate store. The Windows CNG and TPM store cannot fill that gap. It is Windows only, and it is the one path that already worked before the detached key change, because RSACng hits the CopyWithPrivateKey fast path. A store that reproduces the token contract in memory covers the interesting path on every platform instead. SimulatedHardwareCertificateStore holds certificates whose private keys can be used but never extracted, generates key pairs on request, and refuses to import key material it did not generate, recording the attempt so a test can assert the stack never depends on writing a key back. Its provider hands out handles that share one backing token per store path, since the stack opens and disposes stores freely and a real token is not torn down because one consumer closed its session. Registration needs no new plumbing: CertificateManagerOptions .AddStoreProvider already accepts an ICertificateStoreProvider. The tests drive the full path a channel would: load the certificate from the store, sign the handshake, verify it, and unwrap a peer secret, for Basic256Sha256, Aes256_Sha256_RsaPss and ECC_nistP256. #nullable enable is added to the store file because ICertificateStore is annotated and the test framework project does not enable nullable globally. Verified: 15 non-exportable key tests pass on net48, and 514 tests in the Core security namespace pass on net10.0, up from 500, with no regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
Covers the one hardware path that needs no third party dependency. Keys are created in a Windows key storage provider with CngExportPolicies.None, so the private key is genuinely non extractable, and the resulting certificate is driven through the same channel crypto the secure channel performs. The Platform Crypto Provider is used when a TPM is present, and the software key storage provider stands in otherwise. The fallback still produces a non extractable key, so CI agents without a TPM exercise the same code paths; a test reports which of the two was available so a failure on a TPM equipped machine can be told apart from one without. Two portability details. CngProvider.MicrosoftPlatformCryptoProvider only exists on .NET Core and later, so the provider is named explicitly to keep the factory working on .NET Framework. SupportedOSPlatformAttribute is internal on net48, where it comes from the repo polyfill, so the annotation is gated to .NET 5 and later, which is also the only place the platform compatibility analyzer runs. Note that this path attaches the key with X509Certificate2.CopyWithPrivateKey, which succeeds only because RSACng is one of the two implementations the Windows certificate layer recognises. It is therefore not representative of providers that are not CNG backed; those must use the detached key model, which the simulated hardware store covers on every platform. Verified: 19 non-exportable key tests pass on net10.0 and net48, and 518 tests in the Core security namespace pass, up from 514, with no regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
CertificateStoreIdentifier.DetermineStoreType could only see the built-in store types and the legacy static registry, which is marked obsolete. A store type registered through CertificateManagerOptions.AddStoreProvider was invisible to it, so a configuration that relied on auto-detection silently fell back to Directory and opened the wrong kind of store. An overload taking the providers is added and used by CertificateManager, which already holds them and is where store paths are resolved at runtime. The remaining call sites are in the fluent configuration builder, which has no provider access; configuration that goes through those paths must continue to state the store type explicitly, and the overload documents that. This makes OpenStore probe each provider by path before falling back, which is a new call on the provider contract. One existing test used a strict mock that did not anticipate SupportsStorePath being called; it now stubs it to false, so the provider is still selected by its store type name and the test keeps its original meaning. Verified: 519 tests in the Core security namespace pass on net10.0 and 511 on net48, with no regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
Introduces the model that lets a deployment put its application instance key in a TPM, have user identity tokens signed by a remote key service, narrow certificate issuance to one security policy, and leave everything else to the platform, all at once. A provider does not perform the operations. RSA and ECDsa are already abstract and hardware and cloud implementations of them already exist; a competing signing interface would make those unusable. What a provider supplies instead is the part the platform does not model: which capabilities it can serve, and what may be said about the module behind it. Those two facts drive selection, the advertised security policy set, and the audit trail. CryptoPurpose is a readonly record struct with well known instances rather than an enum. The security constants in this stack are already closed enums, which is the single largest obstacle to contributing a new algorithm; repeating that here would make the provider model equally closed. CryptoValidationStatus records provenance. The levels distinguish a provider that can name a validation certificate, one that merely defers to whatever the platform was configured with, and one that carries no validation at all. The default provider reports the middle case, because whether the underlying module runs in a validated mode is a property of the machine and not something this stack can assert. Unknown is treated as uncertified when filtering. CryptoProviderRegistry resolves from the most specific registration to the least: purpose and policy, then purpose, then the registered default, then the platform. A provider bound to a purpose it never claimed is skipped rather than used, so a configuration mistake fails near its cause instead of deep in a handshake. Resolution is intended for the point where something is bound, not per operation. Verified: 10 new tests covering the precedence matrix, plus 529 tests in the Core security namespace on net10.0 and 521 on net48, with no regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
AddCryptoProvider registers the registry and, in the overload that takes a
configuration action, binds providers to purposes and security policies:
services.AddOpcUa()
.AddCryptoProvider(crypto => crypto
.For(CryptoPurpose.ApplicationInstanceKey).Use(tpmProvider)
.For(CryptoPurpose.KeyAgreement).Use(tpmProvider)
.For(CryptoPurpose.UserIdentityKey).Use(keyVaultProvider));
Bindings are stated explicitly rather than discovered. Scanning assemblies for
providers would be convenient but is incompatible with the trimming and ahead
of time posture of this stack, and it would make the effective security
configuration depend on what happens to be loaded.
Configurations are carried through the container as CryptoProviderConfiguration
instances and applied to the resolved registry, so several independent calls
compose instead of overwriting one another. That lets a library contribute a
binding without knowing what the host already did. The registry itself is
registered with TryAddSingleton, so a consumer that supplied its own keeps it.
ChannelQuotas gains a CryptoProviders property. It already carries
ICertificateValidatorEx, so a service-like reference there is established
practice, and it reaches every channel without touching four constructors. The
channel is expected to resolve once when it opens and hold the result, in the
same way it caches the security policy on its token; nothing on a per message
path consults it.
Registering the model changes no behaviour: with nothing bound the registry
resolves to platform cryptography.
Verified: 15 crypto provider tests, plus 534 tests in the Core security
namespace on net10.0 and 526 on net48, with no regressions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
CryptoProviderAuditor reports which providers are in use and which of them carry no validation, through the surfaces the stack already has: source generated log messages under a new CoreEventIds.CryptoProvider block, and opc.ua.crypto.* metrics, of which there were none before. CryptoCompliancePolicy decides how strictly this is enforced. Permissive is the default and leaves an existing deployment exactly as it was: nothing is warned about and nothing is refused. WarnOnUncertified reports every provider that carries no validation. FipsOnly refuses to start, because a deployment that asked for validated cryptography and did not get it should not run and quietly use something else. The metrics are published regardless of policy. They are pull based and cost nothing when nobody reads them, so the information stays available without changing behaviour. A provider that declines to state its validation is treated as uncertified, so silence is not a way to pass an audit. Verified: 22 crypto provider tests, plus 541 tests in the Core security namespace on net10.0 and 533 on net48, with no regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
IKeyPairGenerator decides how a new application instance certificate gets its key. The builder arrives with the subject, subject alternative names and lifetime already set, so an implementation only chooses where the key comes from and how the certificate is signed. DefaultKeyPairGenerator reproduces the previous behaviour exactly, and ApplicationInstance.KeyPairGenerator defaults to it, so nothing changes unless a host sets it. This was the one remaining place that hard coded software key generation. The GDS push path already had IPushCertificateKeyGenerator; startup did not. The interface documents two constraints a hardware implementation has to respect, both found while building the earlier detached key support: the parameterless CreateForRSA and CreateForECDsa generate a key in software and cannot be used, and CertificateRequest.CreateSelfSigned cannot be used either because it attaches the key with X509Certificate2.CopyWithPrivateKey, which fails for a non extractable key. Such an implementation must supply the public key it generated in the device and sign with an X509SignatureGenerator that calls back into it. Verified: 28 crypto provider tests, 221 configuration tests, and 547 tests in the Core security namespace on net10.0 with 539 on net48, all with no regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
CreateSigningRequest with RegeneratePrivateKey regenerates a key that UpdateCertificate consumes later, possibly in another Session or after a restart. The directory backed store does this by exporting the key to a PKCS#12 file, which a key held in a TPM, an HSM or a PKCS#11 token refuses, so that store declines and the request fails with BadNotSupported. This was the single blocker for hardware backed GDS push. For a device held key there is nothing to export and nothing to protect: the device already is the durable store. HardwarePendingCertificateKeyStore keeps only the association between the pending certificate and its scope, writing the public certificate into the group's own store, and re-attaches the key on the way back by asking that store to load it. A software key is declined so the caller falls back to a store that knows how to protect exportable material. The store can be given a certificate store provider directly. CertificateStoreIdentifier.OpenStore resolves store types through the built-in set and the obsolete static registry, so it cannot see a provider registered through dependency injection; supplying it avoids that path. Also fixes the simulated token: AddAsync was replacing a key bearing entry with the public certificate someone handed it, discarding its own key. A real token does not do that. Verified: 4 new tests, plus 547 tests in the Core security namespace with no regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
CryptoCompliance decides which security policies a compliance posture permits. Several of the policies the stack supports use algorithms that are not approved for validated cryptography, and they are enabled by default because withholding them would break deployments that use them today. FipsOnly withholds them: the two SHA-1 based policies, every ChaCha20-Poly1305 variant, the brainpool curves and curve25519/448. An unknown policy is permitted, because this filter is not the arbiter of which policies exist. The AOT tests exercise registration, resolution, the auditor and the compliance filter in a trimmed ahead of time compiled binary. That is what catches a lookup that only works because metadata happened to survive. docs/CryptoProvider.md documents the whole model and is linked from docs/README.md. It states plainly what can and cannot be claimed about FIPS: .NET holds no validation certificate of its own, so the honest claim is that with FipsOnly the stack performs no cryptography outside the platform modules, and those are validated when the operating system is configured for it. net472 and net48 cannot make the claim at all while BouncyCastle is in the certificate path. The known limitations are listed rather than left to be discovered. Verified: 46 crypto provider tests, 4 AOT tests, and 565 tests in the Core security namespace on net10.0 with 557 on net48, all with no regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
Every other benchmark in the suite measures a full round trip, where the network dominates and a change of a few hundred nanoseconds is invisible. This one measures the symmetric encrypt and sign work on its own, which is what makes a claim about the cost of the per message path checkable. The methods double as NUnit tests so the code stays correct and compiled. BenchmarkDotNet cannot currently build the host from this assembly: STACKGEN001 'Stack generation not supported for Opc.Ua.Gds.Common assembly', which comes from a source generator in an unrelated referenced project and is not caused by this work. That is recorded on the fixture rather than left to be rediscovered. Verified: 49 crypto provider tests pass on net48 and the benchmark methods pass as tests on net10.0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
The per-message symmetric path allocated more than it needed to. On the encrypt side the signature was computed into a fresh array and then copied into the space already reserved for it. On the decrypt side a whole HMAC was built per chunk, even though the channel already keeps one per token and hands it to the encrypt side. Both are now avoided. The signature is written straight into its destination, and the decrypt side takes an optional HMAC so the channel can pass the one it already has. Callers that pass nothing get the previous behaviour, so nothing outside the channel has to change. The benchmark could not run at all before this: BenchmarkDotNet generates a host project, and building it fails with STACKGEN001 from a source generator in an unrelated referenced project. That is pre-existing and repo-wide - the benchmarks in Opc.Ua.Core.Encoders.Tests have the same transitive reference. Rather than change shared tooling, the fixture now runs in process, which generates no host project. It also gained a variant that passes the shared HMAC, so what the channel actually does is what gets measured. Measured on net10.0, 8 KB payload, Basic256Sha256: round trip (channel path) 10.239 us / 2.70 KB -> 9.805 us / 2.17 KB EncryptAndSign 6.793 us / 1.78 KB -> 6.695 us / 1.67 KB SignOnly 2.028 us / 1.24 KB -> 1.971 us / 1.13 KB Allocations fall by 6 to 20 percent and no timing regresses. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
Everything the crypto provider work added so far was exercised against a simulated store. This is the first provider backed by a real device: an optional Opc.Ua.Security.Pkcs11 package with a certificate store over a hardware token, smart card or HSM, addressed by an RFC 7512 pkcs11: URI. The private key is used for signing and decryption and never enters this process. The store binds the token key with CopyWithDetachedPrivateKey rather than X509Certificate2.CopyWithPrivateKey. That is not a preference. On Windows the certificate layer has fast paths only for RSACng and RSACryptoServiceProvider and otherwise falls back to exporting the private parameters, so the obvious approach throws for any token backed key. The detached form works on every platform, and this package is its first real consumer. Built directly on Pkcs11Interop rather than Pkcs11Interop.X509Store: one dependency instead of two, and full control over mechanism parameters, which RSA-PSS needs. CKM_RSA_PKCS_PSS with explicit CK_RSA_PKCS_PSS_PARAMS is supported, so Aes256_Sha256_RsaPss can be served from a token where the device implements it. PKCS#1 v1.5 signing supplies the DigestInfo the mechanism expects, and OAEP decryption passes the matching MGF1. The package is never referenced by Opc.Ua.Core, so applications that do not use a token are unaffected, including their Native AOT support - this package is deliberately not AOT-validated, because Pkcs11Interop resolves the module through native interop and carries no annotations. The token reports its validation status as uncertified unless an operator asserts one. A token may well hold a FIPS certificate, but nothing in the PKCS#11 interface reports one, so the stack must not assume it. That is exactly what the audit surfaces exist to record. CI installs SoftHSM2 and provisions a token so the device path is genuinely covered. Tests skip themselves when no module is present, so a missing SoftHSM2 reduces coverage rather than breaking the build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
CryptoProvider.md was added on its own without the cross-links the repo expects. Certificates, CertificateManager, DependencyInjection, Diagnostics and NativeAoT each now point at it from the place a reader would look, and WhatsNewIn2.0 gains an entry for the new public API. CryptoProvider.md itself documents the PKCS#11 store. MigrationGuide.md needs nothing: none of this obsoletes existing API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
The plan left this to be decided with data rather than up front. The isolated benchmark puts a full 8 KB round trip at roughly 9.8 us and 2.2 KB, nearly all of it inside AES and HMAC, and the one consumer that would justify a public interface there - hardware offload - is already excluded because a device round-trip per message would destroy throughput. So the seam is deliberately not added, and the reason is written down next to the other limitations rather than left as an open question. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
marcschier
changed the title
Code coverage✅ Coverage gate passed.
Coverage is above the recorded baseline - consider ratcheting Thresholds live in |
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (46.27%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #4192 +/- ##
==========================================
+ Coverage 80.23% 80.39% +0.16%
==========================================
Files 1515 1742 +227
Lines 209980 239496 +29516
Branches 36213 41470 +5257
==========================================
+ Hits 168479 192545 +24066
- Misses 28867 32587 +3720
- Partials 12634 14364 +1730
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
CI failed on the Core and Server suites with the certificate leak detector, not with a failing test: all 4212 Core and 4028 Server tests passed, then global teardown found 8 and 3 certificates created but never disposed. CertificateCollection.Add calls AddRef, so it takes its own handle and the caller still owns theirs. Three places built a certificate purely to hand it to Add and then dropped it, which leaks the original every time. The simulated hardware store did it in Enumerate and FindByThumbprint, and the PKCS#11 store had inherited the same shape - that one is a real leak in shipping code, not just in tests. Two more of the same kind: the Windows CNG factory dropped the intermediate public-only certificate that CopyWithDetachedPrivateKey copies from, and the simulated hardware store provider cached a token per path without ever disposing it, so every certificate generated in a test stayed alive. The provider owns those tokens, so it is now IDisposable and the fixtures dispose it. The Windows CNG leak would not have shown up in the failing Linux jobs at all - those tests skip off Windows - so it was on course to fail the Windows legs next. Verified: the leak detector reports nothing for any of the affected fixtures, and the solution still builds with 0 warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
… properly The Security.Pkcs11 job failed for a reason I had built in myself. The skip contract keyed off "is a module installed", not "is the token usable", so on CI - where SoftHSM2 was present but provisioning had silently failed - the token tests ran anyway and asserted. Partial provisioning is the more likely state on a developer machine, so this was the wrong way round. It is now inverted on both sides. The tests skip whenever the token holds no certificate, so a half-configured machine loses coverage instead of breaking the build. CI fails loudly if provisioning does not produce what it should, so the coverage cannot quietly disappear. Provisioning itself was the original failure: openssl req -engine pkcs11 does not work on ubuntu-latest, because OpenSSL 3.x has moved to providers and the legacy engine is not there. The step now builds the key and certificate in software and imports both with pkcs11-tool, then asserts both objects are on the token. Importing is equivalent for what these tests check: SoftHSM stores an imported private key with CKA_SENSITIVE=true and CKA_EXTRACTABLE=false, so it is non-extractable once it is on the token. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
romanett
reviewed
Aug 7, 2026
| /// </remarks> | ||
| private static readonly HashSet<string> s_notApproved = new(StringComparer.Ordinal) | ||
| { | ||
| SecurityPolicies.Basic128Rsa15, |
Contributor
There was a problem hiding this comment.
better add this compliance as a property on SecurityPolicy Info for better maintainability
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Makes the stack's cryptographic operations replaceable — by another library, an offboard service, or hardware (TPM 2.0 / HSM / PKCS#11 token / cloud KMS) — with no performance cost in the default configuration, and extends this to certificates so a private key need never exist in process memory.
This started as the research plan in
plans/cryptooffboard.mdand now carries the implementation. The plan is kept in the branch because it is the design record, and because one of its central assumptions turned out to be wrong in a way worth writing down (below).20 commits, 83 files, ~11.2k insertions.
dotnet build UA.slnx -c Release→ 0 warnings, 0 errors (master currently has 2).The finding that reshaped this work
The design assumed a hardware key could be attached with
X509Certificate2.CopyWithPrivateKey. An empirical probe on Windows / .NET 10 disproved it:CopyWithPrivateKey(custom non-exportable RSA)CryptographicExceptionCopyWithPrivateKey(RSACng, ExportPolicy=None)The Windows certificate pal fast-paths only
RSACngandRSACryptoServiceProvider; for anything else it falls back toExportParameters(true), which a non-extractable key refuses by definition. So a PKCS#11 wrapper,RSAKeyVault, or any offboard provider could not have worked at all — this affects anyone attempting the obvious approach today, not just this PR.CertificateRequest.CreateSelfSignedis affected identically, because it callsCopyWithPrivateKeyinternally.The fix is a detached private key:
Certificateholds the key alongside theX509Certificate2in its ref-counted core rather than inside it, which is platform-independent by construction.GetRSAPrivateKey()returns a non-owning view so the shared device key survives the caller'susing. This is recorded in §3.3a of the plan.What is in here
Detached keys and export hardening.
CopyWithDetachedPrivateKey, plus nine call sites that assumed a PFX round-trip always succeeds — they now fail loudly instead of silently dropping a key.The provider model.
ICryptoProviderdeclares capabilities and validation provenance, not operations —RSA/ECDsaare already the right abstraction for the operations, and competing with them would reject the ready-made implementations. Selection resolves over purpose × security policy × certificate type, so a deployment can put the instance key in a TPM, user identity keys in a KMS and everything else in software simultaneously. Resolution happens at binding time, never per operation.Audit and compliance. Which module performed an operation, and whether it carries any validation, is visible through source-generated logs,
opc.ua.crypto.*metrics and the address space, and can be constrained with a compliance policy. Default isPermissive— zero behavioural change on upgrade.A PKCS#11 package (
OPCFoundation.NetStandard.Opc.Ua.Security.Pkcs11, optional, never referenced byOpc.Ua.Core). A certificate store over a real token addressed by RFC 7512pkcs11:URIs, so an existing configuration moves to hardware by changing only a store path. Built onPkcs11Interopalone rather than also takingPkcs11Interop.X509Store: one dependency, and full mechanism control — which mattered, because RSA-PSS needs explicitCK_RSA_PKCS_PSS_PARAMS. It is the first real consumer of the detached-key seam.A Windows CNG/TPM certificate factory, and a simulated hardware store in the test framework so the device contract is covered on every platform without hardware.
Performance
The "no performance impact" claim was unverifiable until now, because BenchmarkDotNet could not build its generated host:
STACKGEN001from a source generator in an unrelated referenced project. That is pre-existing and repo-wide — the benchmarks inOpc.Ua.Core.Encoders.Testsshare the same transitive reference. Rather than change shared tooling, the fixture runs in-process, which generates no host project. (Worth its own issue.)That made it measurable, and then worth improving. net10.0, 8 KB payload, Basic256Sha256:
EncryptAndSignSignOnlyThe encrypt side now writes the signature straight into the space already reserved for it instead of allocating and copying; the decrypt side accepts the HMAC the channel already keeps per token instead of building one per chunk. Allocations fall 6–20 %, nothing regresses.
ISymmetricCryptoProviderwas deliberately not added, and the measurement is the reason: that path is ~9.8 µs of mostly AES and HMAC, and the only consumer that would justify public API there is hardware offload, which is excluded because a device round-trip per message would destroy throughput. Adding the seam with nothing implementing it would commit the hottest code in the stack to an unused interface. Recorded indocs/CryptoProvider.md.FIPS posture
The plan records what may and may not honestly be claimed. .NET holds no CMVP certificate of its own — it calls through to the OS module — so the default provider reports
FipsCapablePlatform, not "validated". A PKCS#11 token reportsUncertifiedunless an operator asserts a certificate, because nothing in the PKCS#11 interface reports one.net472/net48can make no FIPS claim at all, since theBouncyCastle.CryptographyNuGet is not the validated BC-FNA product.Not in scope
Registrable security policies (Phase 6) have ten interlocking blockers, including algorithm enums closed at compile time and policy dictionaries built by reflection over
typeof(SecurityPolicies).GetFields(). Isolated into its own design issue so it does not gate this work. Its acceptance test would be lighting upECC_curve25519/ECC_curve448, which are implemented in-tree but dead becauseCURVE25519is defined in no project or props file anywhere in the repository.Also out of scope: hot-path symmetric offload (above), HTTPS with a device-held key on Windows/macOS (SChannel and the macOS Security framework require a platform KSP; UA-TCP is unaffected everywhere), and an async asymmetric path —
RSA/ECDsaare synchronous contracts, so a network-backed provider blocks a thread on the cold path.Related Issues
Checklist
Tests
New: detached private keys, non-exportable key stores, channel crypto against a non-exportable key, the simulated hardware store, Windows CNG/TPM certificates, provider registry, DI wiring, auditor, key-pair generator, the Part 12 pending key store, AOT coverage, and the PKCS#11 store, URI parser and token operations.
Verified locally: 569 Core security tests on net10.0 / 561 on net48 · 260 Security.Certificates · 43 channel fixtures · 30 PKCS#11 on both TFMs (+7 correctly skipped without a token). Solution build clean on all six TFMs.
Documentation
New
docs/CryptoProvider.md, cross-linked fromCertificates,CertificateManager,DependencyInjection,Diagnostics,NativeAoTandREADME, plus aWhatsNewIn2.0entry.MigrationGuide.mdneeds nothing — no existing API is obsoleted.CI
The PKCS#11 test project joins the auto-discovered matrix on every OS. CI installs SoftHSM2 and provisions a token on Linux/macOS so the device path is genuinely exercised; the tests skip themselves when no module is present, so a missing SoftHSM2 reduces coverage rather than reddening the build.
Note on scope
This is large because the research and the implementation share a branch. If review would go better with the PKCS#11 package split into a follow-up PR, say so and I will separate it — it is self-contained and touches nothing outside its own two projects, the solution file, the package versions file and the CI workflow.