[Server] Fix #3896: send full certificate chain when CA is in the issuer store#3897
Conversation
…uer store CertificateManager.LoadApplicationCertificatesAsync built every CertificateEntry with an empty issuer chain, so SendCertificateChain=true had no effect and servers always sent the leaf certificate only. Resolve the issuer chain from the configured trusted/issuer stores when loading (and rotating) application certificates so the full chain is emitted in both the endpoint ServerCertificate and the secure-channel SenderCertificate. - Add CertificateManager.ResolveIssuerChainAsync: appends every resolved issuer regardless of trust state (GetIssuersAsync's bool reports trust, not resolution); failures are logged and never block startup. - LoadApplicationCertificatesAsync and UpdateApplicationCertificateAsync now populate CertificateEntry.IssuerChain. - De-mask the SendCertificateChain test (no manual issuer injection) and add a leaf-only fallback test.
|
agent seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Companion 1.5.x (master378) fix: #3898. Same symptom, different code path (the 2.0 |
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression where servers configured with SecurityConfiguration.SendCertificateChain = true would still send only the leaf application certificate when the issuing CA was placed in the issuer store (TrustedIssuerCertificates), causing some clients to fail to connect (issue #3896). The change restores issuer-chain resolution during application-certificate load/update so the server can emit the full DER chain blob (leaf || issuers) consistently.
Changes:
- Populate
CertificateEntry.IssuerChainby resolving issuers from trusted + issuer stores duringLoadApplicationCertificatesAsync. - Ensure
UpdateApplicationCertificateAsync(rotation/GDS push) also resolves and stores the issuer chain when the caller doesn’t supply one. - Update and add regression tests to validate full-chain emission from the issuer store and leaf-only fallback when issuers are unavailable.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
Stack/Opc.Ua.Core/Security/Certificates/CertificateManager/CertificateManager.cs |
Adds issuer-chain resolution during app cert load/update to restore correct chain blob emission. |
Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateManager/CertificateManagerTests.cs |
Updates regression coverage for #3896 and adds a negative-path test for missing issuer resolution. |
ResolveIssuerChainAsync caught all exceptions, swallowing OperationCanceledException from GetIssuersAsync and preventing cancellation-aware callers (LoadApplicationCertificatesAsync / UpdateApplicationCertificateAsync) from aborting -- risking unresponsive shutdown. Rethrow OperationCanceledException when the caller's token is cancelled (disposing the empty chain to avoid a leak); only genuine resolution failures are still logged and swallowed. Adds UpdateApplicationCertificatePropagatesCancellation regression test.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3897 +/- ##
==========================================
+ Coverage 72.76% 72.78% +0.01%
==========================================
Files 917 917
Lines 148994 149022 +28
Branches 25814 25816 +2
==========================================
+ Hits 108412 108462 +50
+ Misses 31162 31150 -12
+ Partials 9420 9410 -10
🚀 New features to boost your workflow:
|
|
/azp run OPCFoundation.UA-.NETStandard |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Description
Fixes #3896. A server configured with
SecurityConfiguration.SendCertificateChain = true, whose application instance certificate is issued by a CA placed in the issuer store (TrustedIssuerCertificates), sent only the leaf certificate instead of the full chain. Clients that rely on receiving the chain then failed to connect.Root cause (2.0 line)
The 2.0
CertificateManagerrefactor dropped issuer-chain resolution from the application-certificate load path.CertificateManager.LoadApplicationCertificatesAsyncconstructed everyCertificateEntrywith an empty issuer chain ([]), soCertificateEntry.GetEncodedChainBlob()/LoadCertificateChainRawalways returned the leaf only —SendCertificateChain = truewas effectively ignored regardless of where the CA lived. (The defect was masked by a unit test that manually injected the issuer chain.)This is a regression from
1.5.374, whereServerBaseresolved the chain at startup and appended every resolved issuer unconditionally.Fix
CertificateManager.ResolveIssuerChainAsyncresolves the issuer chain from the configured trusted and issuer stores. It appends every resolved issuer regardless of trust state —GetIssuersAsyncreturnsisTrusted(true only when the issuer is in the trusted store), which reports trust, not resolution success, so it is intentionally ignored. Genuine resolution failures are logged and swallowed so they never block certificate registration / server startup (a leaf-only chain is returned); caller-requested cancellation is propagated so shutdown / abort stays responsive.LoadApplicationCertificatesAsyncandUpdateApplicationCertificateAsync(cert rotation / GDS push) now populateCertificateEntry.IssuerChain.CertificateEntry.IssuerChain— the endpointServerCertificate(ServerBase,TcpTransportListener,HttpsServiceHost) and the secure-channelSenderCertificate(UaSCBinaryChannel) — so the full chain is now emitted everywhere.The companion fix for the
1.5.xmaintenance line (master378) is in a separate PR.Tests
SendCertificateChainBlobResolvesFullChainFromIssuerStore(formerlySendCertificateChainBlobMatchesLeafOrFullChain): the CA is placed in the issuer store only — not the trusted-peer store — and the chain blob is asserted to beleaf || CAbyte-for-byte, resolved entirely by the real load path (no manual injection).LoadApplicationCertificatesProducesLeafOnlyBlobWhenIssuerUnavailable: when the CA is in neither store, the blob falls back to leaf-only without throwing.Security.Certificatessuite (221 passed / 6 skipped),CertificateManager+ cert-rotation categories (35 passed), and theChannelManagerCertRotationIntegrationtest. Build is clean (no new analyzer warnings).Related Issues
Fixes #3896.
Checklist