Merge 1.3.4 to main

CHANGELOG.md

@@ -1,3 +1,5 @@
+- 1.3.4
+  - Upgraded BouncyCastle.Cryptography from 2.0.0 to 2.6.2. The plugin now pins the same BouncyCastle build the AnyCA Gateway uses, so the sync subject guard parses subjects exactly as the gateway does and skips only certificates the gateway genuinely cannot parse (e.g. a CN ending in a dangling `\`, which GCP CAS will issue but RFC 4514 forbids). The 1.3.3 guard ran against the lenient 2.0.0 parser, which never threw on these shapes, so they were admitted and later aborted Command's Full Scan with "badly formatted directory string" (issue #30). Each skipped certificate is logged at error level (`[SYNC-SKIP]`) with its request ID, subject, and reason, and sync continues so a full sync can complete. The upgrade also clears the known BouncyCastle 2.0.0 security advisories.
 - 1.3.3
   - Sync now skips bad/unparseable certificates returned from Google CAS instead of failing the sync.
 - 1.3.2

GCPCAS.Tests/README.md

@@ -0,0 +1,201 @@
+# GCP CAS CA Plugin — Test Suite Reference
+
+## Overview
+
+The `GCPCAS.Tests` project contains the tests for the GCP CAS AnyCA Gateway REST plugin. It holds
+two kinds of tests:
+
+- **Pure unit tests** — no external services. They exercise the sync subject guard and the sync
+  download loop in-process, using self-signed certificates generated at runtime and a fake
+  `CertificateAuthorityServiceClient`. These run under a plain `dotnet test`.
+- **Integration tests** — gated by `[IntegrationTestingFact]`. They hit a **real** GCP CAS project
+  using Application Default Credentials and auto-skip unless the required environment variables are
+  set (see below).
+
+| Class | Layer under test | Isolation technique |
+|---|---|---|
+| `SubjectGuardTests` | `GCPCASClient.SubjectSurvivesGatewayRoundTrip` subject parse guard | Pure unit (in-memory strings + self-signed certs) |
+| `SyncSkipContinuationTests` | `GCPCASClient.DownloadAllIssuedCertificates` skip-and-continue loop | Fake `CertificateAuthorityServiceClient` (no GCP) |
+| `ClientTests` | `GCPCASClient` end-to-end against GCP CAS | `[IntegrationTestingFact]` (real GCP) |
+
+If a test fails in `SubjectGuardTests`, the bug is in the subject-parse decision. If it fails in
+`SyncSkipContinuationTests`, the bug is in how the download loop handles a bad certificate. If it
+fails in `ClientTests`, the bug is in the real GCP interaction (or the environment).
+
+---
+
+## Running the Tests
+
+**Prerequisites:**
+- .NET 8 SDK (test project targets `net8.0`; the plugin targets net6.0/net8.0/net10.0)
+- NuGet packages restored (`dotnet restore`)
+- No external services required for the unit tests
+
+**Run all tests** (integration tests skip automatically without credentials):
+```bash
+dotnet test GCPCAS.sln
+```
+
+**Run only the unit tests / a single class:**
+```bash
+dotnet test --filter "FullyQualifiedName~SubjectGuardTests"
+dotnet test --filter "FullyQualifiedName~SyncSkipContinuationTests"
+```
+
+**Run a specific test by name:**
+```bash
+dotnet test --filter "DisplayName~DownloadAllIssuedCertificates_SkipsBadSubject_AndContinues"
+```
+
+The unit tests reach the plugin's `internal` members via `InternalsVisibleTo("GCPCAS.Tests")`
+declared in `GCPCAS/GCPCAS.csproj`.
+
+---
+
+## Integration test gating
+
+`ClientTests` are decorated with `[IntegrationTestingFact]` (`IntegrationTestingFact.cs`), which
+skips the test unless **all** of these environment variables are set:
+
+- `GCP_PROJECT_ID`
+- `GCP_LOCATION_ID`
+- `GCP_CAS_CAPOOL`
+- `GCP_CAS_CAID`
+
+When set, the tests authenticate with Application Default Credentials and operate against that real
+Enterprise-tier CA pool. With no variables set, `dotnet test` passes trivially (everything skips).
+
+---
+
+## SubjectGuardTests
+
+Regression tests for the sync subject guard (issue #30). GCP CAS will issue certificates whose
+subject is not valid RFC 4514 (e.g. a CN ending in a dangling `\`). The AnyCA Gateway re-parses the
+subject with BouncyCastle's `X509Name` on its `/v2/certificate/search` response; on such a subject
+that parse throws `badly formatted directory string`, the response 500s, and Command's Full Scan
+aborts. The 1.3.3 guard shipped against the lenient BouncyCastle 2.0.0, whose parse never threw on
+these shapes, so they were admitted. The plugin now pins the same BouncyCastle build the gateway
+uses, so `SubjectSurvivesGatewayRoundTrip` is a faithful reproduction of the gateway's accept/reject
+decision — it rejects only what the gateway rejects and accepts everything it accepts.
+
+Subject strings are in .NET's `X509Certificate2.Subject` form.
+
+### SubjectSurvivesGatewayRoundTrip_ClassifiesSubjects (`[Theory]`)
+
+| Subject | Expected | Why |
+|---|---|---|
+| `CN=baseline-app-01.lab.test` | accepted | well-formed |
+| `CN=wellformed.lab.test` | accepted | well-formed |
+| `CN=host.lab.test, OU=PKI, O=Keyfactor Labs, C=US` | accepted | well-formed multi-RDN |
+| `CN=shape1.lab.test\` | **rejected** | CN ends in a dangling backslash — the regression shape |
+| `CN=host.lab.test\, OU=PKI` | **rejected** | dangling backslash right before an RDN separator |
+| `CN=shape2.lab.test\\` | accepted | two backslashes are a valid escaped pair |
+| `CN=shape3.lab.test\\\\, OU=PKI, O=Keyfactor Labs` | accepted | four backslashes are valid escaped pairs |
+| `CN=a\bc` | accepted | `\bc` is a valid RFC 4514 hex escape — **not** a false positive |
+| `CN=a\,b` | accepted | escaped comma is a valid escaped special — **not** a false positive |
+| `CN=a,b` | **rejected** | bare unescaped separator yields a malformed second RDN |
+
+Rejected cases assert a non-empty `failureReason`; accepted cases assert `failureReason == null`.
+The `CN=shape1.lab.test\` → rejected case also serves as a **BouncyCastle-version guard**: if a
+future transitive change reverted to a lenient BouncyCastle, this case would flip and fail.
+
+### RealCertificate_WithTrailingBackslashCn_IsRejected (`[Fact]`)
+
+Builds a real self-signed certificate whose CN ends in a literal backslash byte (the exact shape
+GCP CAS issues), then feeds its `.Subject` through the guard. Asserts .NET renders the backslash
+verbatim (single, not doubled) and that the guard rejects it with a reason.
+
+### RealCertificate_WithWellFormedCn_IsAccepted (`[Fact]`)
+
+Same, with a well-formed CN — asserts the guard accepts it and `failureReason` is null.
+
+### Helper
+
+- `SelfSignedPemWithCommonName(cn)` — builds an in-memory self-signed RSA-2048 certificate with the
+  given CN (via `X500DistinguishedNameBuilder`, so any raw byte including a backslash is accepted)
+  and returns its PEM.
+
+---
+
+## SyncSkipContinuationTests
+
+Regression test for the sync loop's skip-and-continue behaviour (issue #30). It verifies the
+user-facing requirement directly: the download reads the certificates **before** an unparseable one,
+logs and **skips** the bad one, and keeps reading the certificates **after** it, so a full sync
+completes rather than aborting. A fake `CertificateAuthorityServiceClient` streams hand-built pages,
+so no GCP access is required. The `GCPCASClient` is created through an `internal` test-only
+constructor that injects the fake client and starts enabled.
+
+### DownloadAllIssuedCertificates_SkipsBadSubject_AndContinues (`[Fact]`)
+
+| Setup | Assertion |
+|---|---|
+| Page 1 = `good-1`, `good-2`, `bad-1` (CN `broken.lab.test\`); Page 2 = `good-3`, `good-4` | Returns `4`; buffer count `4`; buffered `CARequestID`s are exactly `good-1..good-4` in order; `bad-1` is absent; `buffer.IsAddingCompleted == true` (the `finally`'s `CompleteAdding()` ran) |
+
+This proves the bad certificate is skipped mid-stream (not at the start or end), the surrounding
+good certificates on both pages are still buffered, paging is honoured, and the sync terminates
+cleanly.
+
+### DownloadAllIssuedCertificates_TicketFixture_SkipsOnlyShape1 (`[Fact]`)
+
+Reproduces the exact fixture from the escalation: four well-formed baselines +
+`wellformed.lab.test` + four malformation shapes issued directly on GCP CAS, split across two pages
+(shape1 lands mid-page-2 to prove the loop skips it and keeps reading the shapes after it).
+
+| Certificate | Subject shape | Outcome |
+|---|---|---|
+| `baseline-app-01/02`, `baseline-mq-01`, `baseline-web-01` | well-formed CN | buffered |
+| `wellformed` | well-formed CN | buffered |
+| `shape1` | CN ending in one literal backslash (customer row-301651 shape) | **skipped** |
+| `shape2` | CN ending in two literal backslashes | buffered |
+| `shape3` | four literal backslashes ending the CN value, then `OU=PKI, O=Keyfactor Labs` | buffered |
+| `shape4` | nested DN in the outer CN (`CN=CN=…:oracle_wallet`), then `O`, `C`, `C` | buffered |
+
+| Assertion | Meaning |
+|---|---|
+| Returns `8`; buffered ids are exactly the eight parseable certs in order | shape1 is the only one dropped |
+| `shape1` absent from the buffer | the sole gateway-unparseable subject is skipped |
+| `buffer.IsAddingCompleted == true` | the full sync completed rather than aborting |
+
+This is the regression the fix targets: pre-fix, all nine were admitted and Command's Full Scan then
+aborted on shape1; post-fix, shape1 never enters the buffer and the other eight sync. It also
+confirms the guard is not over-eager — shape2/shape3/shape4 (which the gateway accepts) are **not**
+skipped.
+
+### Helpers and fakes
+
+- `FakeCert(certId, commonName)` — a `Certificate` proto with a resource `CertificateName` and a
+  real self-signed PEM carrying the given CN.
+- `Page(certs…)` — wraps certificates in a `ListCertificatesResponse`.
+- `SelfSignedPem(cn)` — same generator as `SubjectGuardTests`.
+- `FakeCasClient` — subclasses `CertificateAuthorityServiceClient` and overrides
+  `ListCertificatesAsync` to return the supplied pages.
+- `FakePagedAsyncEnumerable` — subclasses `PagedAsyncEnumerable<ListCertificatesResponse, Certificate>`
+  and streams the in-memory pages via `AsRawResponses()` (the method the loop consumes).
+
+---
+
+## ClientTests (integration)
+
+End-to-end tests against a real GCP CAS project. All are `[IntegrationTestingFact]` and skip without
+the four `GCP_*` environment variables.
+
+| Test | What it exercises |
+|---|---|
+| `GCPCASClient_Integration_GetTemplates_ReturnSuccess` | Lists certificate templates (product IDs) from the pool |
+| `GCPCASClient_Integration_DownloadAllCertificates_ReturnSuccess` | Full download of issued certificates into a `BlockingCollection` |
+| `GCPCASClient_Integration_DownloadAllCertificatesAfter_ReturnSuccess` | Incremental download with an `issuedAfter` filter |
+| `GCPCASClient_Integration_EnrollGetRevoke_ReturnSuccess` | Enroll a certificate, fetch it, then revoke it |
+
+---
+
+## Adding New Tests
+
+- **`SubjectGuardTests`** — when changing which subjects are considered parseable/skippable. Prefer
+  a `[Theory]` `[InlineData]` row over a new method. Remember the assertions encode the pinned
+  BouncyCastle's behaviour; if you bump BouncyCastle, re-verify the expected values.
+- **`SyncSkipContinuationTests`** — when changing the download loop's filtering, counting, paging, or
+  buffer-completion behaviour. Build pages with `Page(...)` and certificates with `FakeCert(...)`;
+  do not call `buffer.CompleteAdding()` yourself (the loop does it in a `finally`).
+- **`ClientTests`** — when adding real GCP behaviour that only a live project can validate. Gate it
+  with `[IntegrationTestingFact]` so it skips cleanly in CI without credentials.

GCPCAS.Tests/SubjectGuardTests.cs

@@ -0,0 +1,111 @@
+// Copyright 2025 Keyfactor
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+using System.Security.Cryptography;
+using System.Security.Cryptography.X509Certificates;
+using Keyfactor.Extensions.CAPlugin.GCPCAS.Client;
+
+namespace Keyfactor.Extensions.CAPlugin.GCPCASTests;
+
+/// <summary>
+/// Regression tests for the sync subject guard (issue #30). The 1.3.3 guard shipped against an older, lenient
+/// BouncyCastle whose <c>X509Name</c> parse never threw on a CN ending in a dangling backslash, so such certs
+/// were admitted, persisted, and then aborted Command's Full Scan with "badly formatted directory string" when
+/// the gateway (on a stricter BouncyCastle) re-parsed the subject on its search response. The plugin now pins
+/// the same BouncyCastle build as the gateway, so <see cref="GCPCASClient.SubjectSurvivesGatewayRoundTrip"/> is
+/// a faithful reproduction of the gateway's accept/reject decision - it rejects only what the gateway rejects
+/// and accepts everything it accepts (no structural heuristic).
+///
+/// Subject strings below are in .NET's <see cref="X509Certificate2.Subject"/> form. These are pure unit tests
+/// and run under a plain <c>dotnet test</c>.
+/// </summary>
+public class SubjectGuardTests
+{
+    [Theory]
+    // Well-formed subjects parse.
+    [InlineData("CN=baseline-app-01.lab.test", true)]
+    [InlineData("CN=wellformed.lab.test", true)]
+    [InlineData("CN=host.lab.test, OU=PKI, O=Keyfactor Labs, C=US", true)]
+    // shape1: CN ends in ONE dangling backslash -> BouncyCastle throws. The regression: must be rejected now
+    // (the lenient 1.3.3 BouncyCastle admitted it).
+    [InlineData(@"CN=shape1.lab.test\", false)]
+    // A dangling backslash right before a real RDN separator is rejected the same way.
+    [InlineData(@"CN=host.lab.test\, OU=PKI", false)]
+    // shape2 / shape3: TWO and FOUR backslashes are valid escaped pairs -> accepted, matching the lab
+    // reproduction, which saw only shape1 abort the scan.
+    [InlineData(@"CN=shape2.lab.test\\", true)]
+    [InlineData(@"CN=shape3.lab.test\\\\, OU=PKI, O=Keyfactor Labs", true)]
+    // NOT false positives: a backslash+two-hex is a valid RFC 4514 hex escape, and an escaped comma is a valid
+    // escaped special. These synced fine before and must keep syncing - the old odd-run heuristic wrongly
+    // skipped them.
+    [InlineData(@"CN=a\bc", true)]
+    [InlineData(@"CN=a\,b", true)]
+    // A bare unescaped separator yields a malformed second RDN, which BouncyCastle rejects. (.NET quotes such
+    // values rather than emitting this, but the guard rejects it regardless.)
+    [InlineData("CN=a,b", false)]
+    public void SubjectSurvivesGatewayRoundTrip_ClassifiesSubjects(string dotNetSubject, bool expectedSurvives)
+    {
+        bool survives = GCPCASClient.SubjectSurvivesGatewayRoundTrip(dotNetSubject, out string failureReason);
+
+        Assert.Equal(expectedSurvives, survives);
+        if (expectedSurvives)
+        {
+            Assert.Null(failureReason);
+        }
+        else
+        {
+            Assert.False(string.IsNullOrEmpty(failureReason));
+        }
+    }
+
+    [Fact]
+    public void RealCertificate_WithTrailingBackslashCn_IsRejected()
+    {
+        // End-to-end shape check: build a real cert whose CN ends in a literal backslash byte (as GCP CAS
+        // would accept and issue), then feed the .NET subject string through the guard. This is the exact
+        // shape from the lab reproduction (shape1).
+        string pem = SelfSignedPemWithCommonName(@"shape1.lab.test\");
+        using X509Certificate2 cert = X509Certificate2.CreateFromPem(pem);
+
+        // Sanity: .NET renders the single literal backslash verbatim (not doubled).
+        Assert.EndsWith(@"\", cert.Subject);
+        Assert.DoesNotContain(@"\\", cert.Subject);
+
+        Assert.False(GCPCASClient.SubjectSurvivesGatewayRoundTrip(cert.Subject, out string failureReason));
+        Assert.False(string.IsNullOrEmpty(failureReason));
+    }
+
+    [Fact]
+    public void RealCertificate_WithWellFormedCn_IsAccepted()
+    {
+        string pem = SelfSignedPemWithCommonName("baseline-app-01.lab.test");
+        using X509Certificate2 cert = X509Certificate2.CreateFromPem(pem);
+
+        Assert.True(GCPCASClient.SubjectSurvivesGatewayRoundTrip(cert.Subject, out string failureReason));
+        Assert.Null(failureReason);
+    }
+
+    private static string SelfSignedPemWithCommonName(string commonNameValue)
+    {
+        var builder = new X500DistinguishedNameBuilder();
+        builder.AddCommonName(commonNameValue);
+        X500DistinguishedName subject = builder.Build();
+
+        using RSA rsa = RSA.Create(2048);
+        var request = new CertificateRequest(subject, rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
+        using X509Certificate2 cert = request.CreateSelfSigned(
+            DateTimeOffset.UtcNow.AddDays(-1), DateTimeOffset.UtcNow.AddDays(1));
+        return cert.ExportCertificatePem();
+    }
+}