diff --git a/CoseSign1.Abstractions/Interfaces/ISupportsScittCompliance.cs b/CoseSign1.Abstractions/Interfaces/ISupportsScittCompliance.cs new file mode 100644 index 00000000..5c793097 --- /dev/null +++ b/CoseSign1.Abstractions/Interfaces/ISupportsScittCompliance.cs @@ -0,0 +1,42 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace CoseSign1.Abstractions.Interfaces; + +/// +/// Optional capability interface implemented by implementations +/// that support toggling SCITT (Supply Chain Integrity, Transparency, and Trust) compliance behavior. +/// +/// +/// +/// Hosts that need to enable or disable automatic CWT (CBOR Web Token) claim emission on a +/// signing-key provider should test for this interface rather than depending on a specific concrete +/// type. This keeps the cross-boundary contract in CoseSign1.Abstractions, allowing +/// implementing assemblies (e.g. CoseSign1.Certificates) to remain plugin-local in +/// host/plugin isolation scenarios without +/// triggering type-identity mismatches. +/// +/// +/// Example host pattern: +/// +/// ICoseSigningKeyProvider provider = plugin.CreateProvider(configuration); +/// if (provider is ISupportsScittCompliance scittProvider) +/// { +/// scittProvider.EnableScittCompliance = enableScittCompliance; +/// } +/// +/// +/// +public interface ISupportsScittCompliance +{ + /// + /// Gets or sets a value indicating whether SCITT-compliant CWT claims (issuer and subject) are + /// automatically added to signatures produced by this provider. + /// + /// + /// Implementations should default this to true when SCITT compliance is the expected + /// behavior for the provider. Setting to false suppresses default-claim emission; + /// user-supplied CWT claims attached via header extenders remain unaffected. + /// + bool EnableScittCompliance { get; set; } +} diff --git a/CoseSign1.Certificates.AzureArtifactSigning.Tests/AasClientOptionsExtensionsTests.cs b/CoseSign1.Certificates.AzureArtifactSigning.Tests/AasClientOptionsExtensionsTests.cs new file mode 100644 index 00000000..0edf33c1 --- /dev/null +++ b/CoseSign1.Certificates.AzureArtifactSigning.Tests/AasClientOptionsExtensionsTests.cs @@ -0,0 +1,86 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace CoseSign1.Certificates.AzureArtifactSigning.Tests; + +using System; +using Azure.CodeSigning; +using Azure.Core; +using Azure.Developer.ArtifactSigning.CryptoProvider.Models; +using NUnit.Framework; + +/// +/// Tests for — the AAS analogue of the MST +/// performance optimisation extensions. These verify that interactive signing latency +/// is bounded by short, fixed-delay retries and that callers can override defaults +/// without losing fluent chaining. +/// +[TestFixture] +public class AasClientOptionsExtensionsTests +{ + [Test] + public void ConfigureAasPerformanceOptimizations_AppliesFastFixedRetries() + { + // Arrange + CertificateProfileClientOptions options = new CertificateProfileClientOptions(); + + // Act + CertificateProfileClientOptions returned = options.ConfigureAasPerformanceOptimizations(); + + // Assert + Assert.That(returned, Is.SameAs(options), "Extension method must return the same instance for fluent chaining."); + Assert.That(options.Retry.Mode, Is.EqualTo(RetryMode.Fixed), "Retry mode must be Fixed for predictable interactive latency."); + Assert.That(options.Retry.Delay, Is.EqualTo(AasClientOptionsExtensions.DefaultRetryDelay)); + Assert.That(options.Retry.MaxRetries, Is.EqualTo(AasClientOptionsExtensions.DefaultMaxRetries)); + } + + [Test] + public void ConfigureAasPerformanceOptimizations_HonoursOverrides() + { + // Arrange + CertificateProfileClientOptions options = new CertificateProfileClientOptions(); + TimeSpan customDelay = TimeSpan.FromMilliseconds(100); + const int customRetries = 16; + + // Act + options.ConfigureAasPerformanceOptimizations(customDelay, customRetries); + + // Assert + Assert.That(options.Retry.Delay, Is.EqualTo(customDelay)); + Assert.That(options.Retry.MaxRetries, Is.EqualTo(customRetries)); + } + + [Test] + public void ConfigureAasPerformanceOptimizations_NullOptions_Throws() + { + // Act / Assert + ArgumentNullException ex = Assert.Throws( + () => AasClientOptionsExtensions.ConfigureAasPerformanceOptimizations(null!)); + Assert.That(ex.ParamName, Is.EqualTo("options")); + } + + [Test] + public void ConfigureAasSigningPerformance_ReturnsDefaultsWhenUnset() + { + // Act + AzSignContextOptions opts = AasClientOptionsExtensions.ConfigureAasSigningPerformance(); + + // Assert + Assert.That(opts, Is.Not.Null); + Assert.That(opts.TaskRetryCount, Is.EqualTo(AasClientOptionsExtensions.DefaultSigningTaskRetryCount)); + Assert.That(opts.TaskTimeOutInSeconds, Is.EqualTo(AasClientOptionsExtensions.DefaultSigningTaskTimeoutSeconds)); + } + + [Test] + public void ConfigureAasSigningPerformance_HonoursOverrides() + { + // Act + AzSignContextOptions opts = AasClientOptionsExtensions.ConfigureAasSigningPerformance( + taskRetryCount: 5, + taskTimeoutSeconds: 30); + + // Assert + Assert.That(opts.TaskRetryCount, Is.EqualTo(5)); + Assert.That(opts.TaskTimeOutInSeconds, Is.EqualTo(30)); + } +} diff --git a/CoseSign1.Certificates.AzureArtifactSigning/Extensions/AasClientOptionsExtensions.cs b/CoseSign1.Certificates.AzureArtifactSigning/Extensions/AasClientOptionsExtensions.cs new file mode 100644 index 00000000..16a64991 --- /dev/null +++ b/CoseSign1.Certificates.AzureArtifactSigning/Extensions/AasClientOptionsExtensions.cs @@ -0,0 +1,132 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Azure.CodeSigning; + +using System; +using Azure.Core; +using Azure.Developer.ArtifactSigning.CryptoProvider.Models; + +/// +/// Extension methods for tuning the Azure Artifact Signing (AAS) client pipeline and signing-context +/// timing knobs for improved interactive signing performance. +/// +/// +/// +/// The Azure SDK's default use exponential back-off starting at 800 ms with +/// 3 retries (≈ 5 s before giving up). For interactive signing scenarios — where a user is waiting at +/// a console — that can stretch a transient blip into a multi-second wait. These extensions apply +/// fixed-delay 250 ms retries with up to 8 attempts so the typical recovery window is short, while +/// leaving the AzSignContext long-running signing operation knobs +/// () adjustable via . +/// +/// +/// Retry-After honoured. Azure.Core's RetryPolicy always honours a server-supplied +/// Retry-After header even when is configured. If AAS asks the +/// client to back off (e.g. Retry-After: 30), that value wins over the configured 250 ms delay, +/// so the practical worst-case latency is bounded by the server's policy, not the SDK ceiling. This is +/// intentional: AAS uses Retry-After correctly and the client should respect it. Unlike the MST +/// equivalent (MstClientOptionsExtensions.ConfigureMstPerformanceOptimizations) this helper does +/// not strip Retry-After headers — that is an MST-specific work-around for the Code +/// Transparency Service's eventual-consistency window where the server returns optimistic 1-second +/// hints that the client can safely beat. +/// +/// +public static class AasClientOptionsExtensions +{ + /// + /// The default interval between fast retry attempts when the server does not supply + /// Retry-After. A server-supplied value will override this. + /// + public static readonly TimeSpan DefaultRetryDelay = TimeSpan.FromMilliseconds(250); + + /// + /// The default maximum number of fast retry attempts. With no Retry-After back-pressure + /// the worst-case ceiling is approximately MaxRetries * DefaultRetryDelay (≈ 2 seconds). + /// + public const int DefaultMaxRetries = 8; + + /// + /// The default applied by + /// . Matches the SDK default; surfaced as a constant + /// so callers can tune relative to a known baseline. + /// + public const int DefaultSigningTaskRetryCount = 3; + + /// + /// The default applied by + /// . Matches the SDK default. + /// + public const int DefaultSigningTaskTimeoutSeconds = 60; + + /// + /// Configures the Azure SDK retry pipeline on a + /// instance to use a short fixed delay between retries so transient certificate-profile lookups + /// do not stretch interactive signing latency. Server-supplied Retry-After values are + /// still honoured and override the configured delay. + /// + /// The to configure. + /// + /// Interval between fast retry attempts. Defaults to (250 ms). + /// + /// + /// Maximum number of fast retry attempts before failing. Defaults to (8). + /// + /// The same instance for fluent chaining. + /// Thrown when is null. + /// + /// + /// CertificateProfileClientOptions options = new CertificateProfileClientOptions(); + /// options.ConfigureAasPerformanceOptimizations(); + /// CertificateProfileClient client = new CertificateProfileClient(credential, endpoint, options); + /// + /// + public static CertificateProfileClientOptions ConfigureAasPerformanceOptimizations( + this CertificateProfileClientOptions options, + TimeSpan? retryDelay = null, + int? maxRetries = null) + { + if (options == null) + { + throw new ArgumentNullException(nameof(options)); + } + + options.Retry.Mode = RetryMode.Fixed; + options.Retry.Delay = retryDelay ?? DefaultRetryDelay; + options.Retry.MaxRetries = maxRetries ?? DefaultMaxRetries; + + return options; + } + + /// + /// Builds an instance with the supplied long-running signing + /// timing knobs, falling back to the SDK defaults when a value is not specified. + /// + /// + /// Maximum number of times the signing task is retried before failing. Defaults to + /// . + /// + /// + /// Per-task timeout in seconds for the signing long-running operation. Defaults to + /// . + /// + /// A populated . + /// + /// + /// AzSignContextOptions opts = AasClientOptionsExtensions.ConfigureAasSigningPerformance( + /// taskRetryCount: 5, + /// taskTimeoutSeconds: 30); + /// AzSignContext signContext = new AzSignContext(account, profile, client, null, opts); + /// + /// + public static AzSignContextOptions ConfigureAasSigningPerformance( + int? taskRetryCount = null, + int? taskTimeoutSeconds = null) + { + return new AzSignContextOptions + { + TaskRetryCount = taskRetryCount ?? DefaultSigningTaskRetryCount, + TaskTimeOutInSeconds = taskTimeoutSeconds ?? DefaultSigningTaskTimeoutSeconds + }; + } +} diff --git a/CoseSign1.Certificates.Tests/CertificateCoseSigningKeyProviderEnableScittTests.cs b/CoseSign1.Certificates.Tests/CertificateCoseSigningKeyProviderEnableScittTests.cs index 08e6792b..8f9092d7 100644 --- a/CoseSign1.Certificates.Tests/CertificateCoseSigningKeyProviderEnableScittTests.cs +++ b/CoseSign1.Certificates.Tests/CertificateCoseSigningKeyProviderEnableScittTests.cs @@ -116,4 +116,62 @@ public void TestGetProtectedHeaders_WithScittEnabled_IncludesDefaultCWTClaims() claims!.Issuer.Should().NotBeNull(); claims.Subject.Should().Be(CwtClaims.DefaultSubject); } + + /// + /// Verifies that implements the shared + /// capability interface from + /// CoseSign1.Abstractions. This is the cross-AssemblyLoadContext contract that lets + /// the host toggle SCITT compliance on plugin-returned providers without referencing the + /// concrete provider type — keeping CoseSign1.Certificates plugin-local. + /// + [Test] + public void TestImplementsISupportsScittCompliance_ContractDefinedInAbstractions() + { + // Arrange + X509Certificate2 cert = TestCertificateUtils.CreateCertificate(); + ICertificateChainBuilder chainBuilder = new TestChainBuilder(); + ICoseSigningKeyProvider provider = new X509Certificate2CoseSigningKeyProvider(chainBuilder, cert); + + // Act — host pattern: probe via shared capability interface, never via concrete type. + bool implementsScittCapability = provider is ISupportsScittCompliance; + + // Assert + implementsScittCapability.Should().BeTrue( + "CertificateCoseSigningKeyProvider must expose the shared ISupportsScittCompliance capability so the host can toggle SCITT compliance without depending on the concrete type"); + + // The capability interface MUST live in CoseSign1.Abstractions so it can be host-shared + // across plugin AssemblyLoadContext boundaries. Verifying this stops accidental moves + // into a plugin-local assembly. + typeof(ISupportsScittCompliance).Assembly.GetName().Name.Should().Be( + "CoseSign1.Abstractions", + "ISupportsScittCompliance must remain in the host-shared CoseSign1.Abstractions assembly to preserve cross-ALC type identity"); + } + + /// + /// Verifies the round-trip: setting EnableScittCompliance through the + /// interface flows through to the underlying provider's + /// behavior (matching the SignCommand downcast pattern). + /// + [Test] + public void TestISupportsScittCompliance_RoundTripsThroughInterface() + { + // Arrange + X509Certificate2 cert = TestCertificateUtils.CreateCertificate(); + ICertificateChainBuilder chainBuilder = new TestChainBuilder(); + ICoseSigningKeyProvider provider = new X509Certificate2CoseSigningKeyProvider(chainBuilder, cert); + + // Act — exact mirror of SignCommand's host-side pattern after the Phase 3 refactor. + if (provider is ISupportsScittCompliance scittProvider) + { + scittProvider.EnableScittCompliance = false; + } + + // Assert — concrete property reflects the interface-driven change. + ((CertificateCoseSigningKeyProvider)provider).EnableScittCompliance.Should().BeFalse( + "Setting EnableScittCompliance via the shared interface must affect the underlying provider behavior"); + + CoseHeaderMap headers = provider.GetProtectedHeaders(); + headers.ContainsKey(CWTClaimsHeaderLabels.CWTClaims).Should().BeFalse( + "Disabling SCITT compliance through the shared interface must suppress default CWT claim emission"); + } } diff --git a/CoseSign1.Certificates/CertificateCoseSigningKeyProvider.cs b/CoseSign1.Certificates/CertificateCoseSigningKeyProvider.cs index 3afb389c..0948c263 100644 --- a/CoseSign1.Certificates/CertificateCoseSigningKeyProvider.cs +++ b/CoseSign1.Certificates/CertificateCoseSigningKeyProvider.cs @@ -8,7 +8,7 @@ namespace CoseSign1.Certificates; /// /// Abstract class which contains common logic needed for all certificate based implementations. /// -public abstract class CertificateCoseSigningKeyProvider : ICoseSigningKeyProvider +public abstract class CertificateCoseSigningKeyProvider : ICoseSigningKeyProvider, ISupportsScittCompliance { private static readonly DidX509Generator DefaultDidGenerator = new(); diff --git a/CoseSignTool.Abstractions.Tests/PluginLoadContextAdvancedTests.cs b/CoseSignTool.Abstractions.Tests/PluginLoadContextAdvancedTests.cs index aaced6fd..985a1c9b 100644 --- a/CoseSignTool.Abstractions.Tests/PluginLoadContextAdvancedTests.cs +++ b/CoseSignTool.Abstractions.Tests/PluginLoadContextAdvancedTests.cs @@ -47,37 +47,23 @@ public void TestCleanup() } /// - /// Tests IsSharedFrameworkAssembly with System assemblies. + /// Tests IsHostShared with System assemblies. Note: BCL System.* is no longer + /// pre-emptively shared — the runtime's default-context fallback handles them via TPA. + /// Only the bare System name and the dotted-prefix-less framework families remain + /// in the explicit prefix list. This test is retained as a documented contract. /// [TestMethod] - public void IsSharedFrameworkAssembly_WithSystemAssemblies_ShouldReturnTrue() + public void IsHostShared_WithBareSystemName_ShouldReturnTrue() { - // Arrange - string[] systemAssemblies = { - "System", - "System.Collections", - "System.Collections.Concurrent", - "System.IO", - "System.Runtime", - "System.Text.Json", - "System.Threading.Tasks", - "System.Reflection.Emit" - }; - - // Act & Assert - foreach (string assemblyName in systemAssemblies) - { - AssemblyName name = new AssemblyName(assemblyName); - bool result = InvokeIsSharedFrameworkAssembly(name); - Assert.IsTrue(result, $"'{assemblyName}' should be recognized as a shared framework assembly"); - } + bool result = PluginLoadContext.IsHostShared("System"); + Assert.IsTrue(result, "Bare 'System' assembly must be host-shared (BCL)."); } /// - /// Tests IsSharedFrameworkAssembly with Microsoft.Extensions assemblies. + /// Tests IsHostShared with Microsoft.Extensions assemblies. /// [TestMethod] - public void IsSharedFrameworkAssembly_WithMicrosoftExtensionsAssemblies_ShouldReturnTrue() + public void IsHostShared_WithMicrosoftExtensionsAssemblies_ShouldReturnTrue() { // Arrange string[] extensionsAssemblies = { @@ -92,62 +78,175 @@ public void IsSharedFrameworkAssembly_WithMicrosoftExtensionsAssemblies_ShouldRe // Act & Assert foreach (string assemblyName in extensionsAssemblies) { - AssemblyName name = new AssemblyName(assemblyName); - bool result = InvokeIsSharedFrameworkAssembly(name); + bool result = PluginLoadContext.IsHostShared(assemblyName); Assert.IsTrue(result, $"'{assemblyName}' should be recognized as a shared framework assembly"); } } /// - /// Tests IsSharedFrameworkAssembly with project-specific shared assemblies. + /// Tests IsHostShared with the curated cross-boundary contract assemblies. + /// These are exact-name matches; the prefix-collision smell is gone. /// [TestMethod] - public void IsSharedFrameworkAssembly_WithProjectSharedAssemblies_ShouldReturnTrue() + public void IsHostShared_WithSharedContractAssemblies_ShouldReturnTrue() { - // Arrange - string[] projectSharedAssemblies = { + string[] sharedContracts = { "CoseSignTool.Abstractions", - "CoseHandler", - "CoseSign1", - "CoseIndirectSignature" + "CoseSign1.Abstractions", + "CoseSign1.Headers", }; - // Act & Assert - foreach (string assemblyName in projectSharedAssemblies) + foreach (string assemblyName in sharedContracts) { - AssemblyName name = new AssemblyName(assemblyName); - bool result = InvokeIsSharedFrameworkAssembly(name); - Assert.IsTrue(result, $"'{assemblyName}' should be recognized as a project shared assembly"); + bool result = PluginLoadContext.IsHostShared(assemblyName); + Assert.IsTrue(result, $"'{assemblyName}' is a curated cross-boundary contract and must be host-shared"); } } /// - /// Tests IsSharedFrameworkAssembly with third-party shared assemblies. + /// Tests IsHostShared with bare framework assembly names that need exact matching + /// (no trailing dot) to avoid collisions like "netstandardX" or "mscorlibX". Includes + /// bare "System" because the unprefixed System assembly is BCL-only. /// [TestMethod] - public void IsSharedFrameworkAssembly_WithThirdPartySharedAssemblies_ShouldReturnTrue() + public void IsHostShared_WithBareFrameworkNames_ShouldReturnTrue() { - // Arrange - string[] thirdPartySharedAssemblies = { - "Newtonsoft.Json", + string[] frameworkNames = { + "System", "netstandard", - "mscorlib" + "mscorlib", + "WindowsBase", + "Microsoft.CSharp", + "Microsoft.VisualBasic", }; - // Act & Assert - foreach (string assemblyName in thirdPartySharedAssemblies) + foreach (string assemblyName in frameworkNames) { - AssemblyName name = new AssemblyName(assemblyName); - bool result = InvokeIsSharedFrameworkAssembly(name); - Assert.IsTrue(result, $"'{assemblyName}' should be recognized as a shared framework assembly"); + bool result = PluginLoadContext.IsHostShared(assemblyName); + Assert.IsTrue(result, $"'{assemblyName}' is a bare framework name and must be host-shared"); + } + } + + /// + /// Tests that BCL System.* assemblies do NOT match a shared prefix anymore. They + /// resolve via the runtime's default-context fallback (TPA) when PluginLoadContext.Load + /// returns null, so functional behavior is preserved. The deliberate non-listing here lets + /// out-of-band System.* NuGet packages (like System.ClientModel) load + /// plugin-locally when shipped alongside a plugin. + /// + [TestMethod] + public void IsHostShared_WithSystemDotPrefix_ShouldReturnFalse() + { + string[] systemDotNames = { + "System.Collections", + "System.Collections.Concurrent", + "System.IO", + "System.Runtime", + "System.Text.Json", + "System.Threading.Tasks", + "System.Reflection.Emit", + // NuGet packages with System.* names that legitimately ship as plugin-private deps: + "System.ClientModel", + "System.Memory.Data", + "System.IO.Pipelines", + "System.Diagnostics.DiagnosticSource", + "System.Security.Cryptography.ProtectedData", + }; + + foreach (string assemblyName in systemDotNames) + { + bool result = PluginLoadContext.IsHostShared(assemblyName); + Assert.IsFalse(result, $"'{assemblyName}' must NOT match a shared prefix — BCL System.* resolves via default-context fallback, NuGet System.* may be plugin-local"); + } + } + + /// + /// Tests IsHostShared with assembly names that previously matched the loose "CoseSign1" / + /// "CoseHandler" / "CoseIndirectSignature" prefix and would have been silently absorbed into + /// the host context. After the prefix removal these are plugin-local. + /// + [TestMethod] + public void IsHostShared_WithFormerlySharedPrefixes_ShouldReturnFalse() + { + string[] noLongerShared = { + "CoseHandler", // host-only static usage now + "CoseIndirectSignature", // plugin-local + "CoseSign1", // plugin-local concrete impl + "CoseSign1.Certificates", // plugin-local; Phase 3 ISupportsScittCompliance handles cross-boundary + "CoseSign1.Transparent.MST", // plugin-local; co-located with Azure SDK in plugin ALC + "CoseSign1.Certificates.AzureArtifactSigning", // plugin-local; co-located with Azure SDK in plugin ALC + }; + + foreach (string assemblyName in noLongerShared) + { + bool result = PluginLoadContext.IsHostShared(assemblyName); + Assert.IsFalse(result, $"'{assemblyName}' must NOT be host-shared after the prefix-list cleanup — it is plugin-local"); } } /// - /// Tests IsSharedFrameworkAssembly with plugin-specific assemblies. + /// Tests that a hypothetical 3rd-party plugin assembly whose name happens to start with + /// "CoseSign1" is NOT silently absorbed into the host context. This is the prefix-collision + /// regression test that motivated the move to exact-name matching. /// [TestMethod] - public void IsSharedFrameworkAssembly_WithPluginSpecificAssemblies_ShouldReturnFalse() + public void IsHostShared_WithThirdPartyPrefixCollisions_ShouldReturnFalse() + { + string[] thirdPartyImpostors = { + "CoseSign1Plus", + "CoseSign1Extra.Plugin", + "CoseHandlerExtensions", + "CoseIndirectSignatureV2", + "CoseSignTool.Abstractions.Extras", // looks like an extension but is a different assembly + }; + + foreach (string assemblyName in thirdPartyImpostors) + { + bool result = PluginLoadContext.IsHostShared(assemblyName); + Assert.IsFalse(result, $"'{assemblyName}' must NOT match the shared list — exact-name only"); + } + } + + /// + /// Tests that Azure SDK assemblies are NOT host-shared. Before Phase 2, these were forced into + /// the host context as a workaround for cross-ALC type identity bugs in shared CoseSign1.* libs. + /// After Phase 2, those libs are plugin-local, so the SDK types co-locate with their callers + /// and never need to cross any boundary. + /// + [TestMethod] + public void IsHostShared_WithAzureSdkAssemblies_ShouldReturnFalse() + { + string[] azureSdkAssemblies = { + "Azure.Core", + "Azure.Identity", + "Azure.Security.CodeTransparency", + "Azure.CodeSigning", + "Azure.Developer.ArtifactSigning", + "Azure.Developer.ArtifactSigning.CryptoProvider", + }; + + foreach (string assemblyName in azureSdkAssemblies) + { + bool result = PluginLoadContext.IsHostShared(assemblyName); + Assert.IsFalse(result, $"'{assemblyName}' must NOT be host-shared — Azure SDK packages are plugin-local"); + } + } + + /// + /// Tests that Newtonsoft.Json is NOT host-shared. Plugins ship their own copy if they use it. + /// + [TestMethod] + public void IsHostShared_WithNewtonsoftJson_ShouldReturnFalse() + { + bool result = PluginLoadContext.IsHostShared("Newtonsoft.Json"); + Assert.IsFalse(result, "Newtonsoft.Json must NOT be host-shared — it was a leftover prefix that introduced unwanted host coupling"); + } + + /// + /// Tests IsHostShared with plugin-specific assemblies. + /// + [TestMethod] + public void IsHostShared_WithPluginSpecificAssemblies_ShouldReturnFalse() { // Arrange string[] pluginSpecificAssemblies = { @@ -161,50 +260,43 @@ public void IsSharedFrameworkAssembly_WithPluginSpecificAssemblies_ShouldReturnF // Act & Assert foreach (string assemblyName in pluginSpecificAssemblies) { - AssemblyName name = new AssemblyName(assemblyName); - bool result = InvokeIsSharedFrameworkAssembly(name); + bool result = PluginLoadContext.IsHostShared(assemblyName); Assert.IsFalse(result, $"'{assemblyName}' should NOT be recognized as a shared framework assembly"); } } /// - /// Tests IsSharedFrameworkAssembly with null assembly name. + /// Tests IsHostShared with null and empty assembly name. /// [TestMethod] - public void IsSharedFrameworkAssembly_WithNullAssemblyName_ShouldReturnFalse() + public void IsHostShared_WithNullOrEmptyName_ShouldReturnFalse() { - // Arrange - AssemblyName assemblyName = new AssemblyName(); - // assemblyName.Name will be null - - // Act - bool result = InvokeIsSharedFrameworkAssembly(assemblyName); - - // Assert - Assert.IsFalse(result, "Assembly with null name should return false"); + Assert.IsFalse(PluginLoadContext.IsHostShared(null), "Null name should return false"); + Assert.IsFalse(PluginLoadContext.IsHostShared(string.Empty), "Empty name should return false"); } /// - /// Tests IsSharedFrameworkAssembly with case sensitivity. + /// Tests IsHostShared with case sensitivity. /// [TestMethod] - public void IsSharedFrameworkAssembly_WithDifferentCasing_ShouldReturnTrue() + public void IsHostShared_WithDifferentCasing_ShouldReturnTrue() { - // Arrange + // Arrange — only assemblies that ARE host-shared after the cleanup string[] differentCasingAssemblies = { - "system.collections", - "SYSTEM.IO", - "Microsoft.extensions.logging", + "system", // bare framework name (exact match, case-insensitive) + "Microsoft.extensions.logging", // Microsoft.Extensions.* prefix "COSESIGNTOOL.ABSTRACTIONS", - "cosehandler" + "cosesign1.abstractions", + "cosesign1.HEADERS", + "MSCORLIB", + "NETSTANDARD" }; // Act & Assert foreach (string assemblyName in differentCasingAssemblies) { - AssemblyName name = new AssemblyName(assemblyName); - bool result = InvokeIsSharedFrameworkAssembly(name); - Assert.IsTrue(result, $"'{assemblyName}' should be recognized as shared framework assembly regardless of casing"); + bool result = PluginLoadContext.IsHostShared(assemblyName); + Assert.IsTrue(result, $"'{assemblyName}' should be recognized as shared regardless of casing"); } } @@ -431,28 +523,5 @@ private static IntPtr InvokeLoadUnmanagedDll(PluginLoadContext context, string u } } - /// - /// Uses reflection to invoke the private IsSharedFrameworkAssembly method. - /// - private static bool InvokeIsSharedFrameworkAssembly(AssemblyName assemblyName) - { - MethodInfo? method = typeof(PluginLoadContext).GetMethod("IsSharedFrameworkAssembly", - BindingFlags.NonPublic | BindingFlags.Static); - - Assert.IsNotNull(method, "IsSharedFrameworkAssembly method should exist"); - - try - { - object? result = method.Invoke(null, new object[] { assemblyName }); - Assert.IsInstanceOfType(result, typeof(bool), "IsSharedFrameworkAssembly should return bool"); - return (bool)result; - } - catch (TargetInvocationException ex) when (ex.InnerException != null) - { - // Re-throw the inner exception to preserve original exception type - throw ex.InnerException; - } - } - #endregion } diff --git a/CoseSignTool.Abstractions.Tests/PluginLoadContextBasicTests.cs b/CoseSignTool.Abstractions.Tests/PluginLoadContextBasicTests.cs index e0408f6d..48c14a9f 100644 --- a/CoseSignTool.Abstractions.Tests/PluginLoadContextBasicTests.cs +++ b/CoseSignTool.Abstractions.Tests/PluginLoadContextBasicTests.cs @@ -95,7 +95,7 @@ public void Load_WithNonExistentAssembly_ShouldReturnNull() } /// - /// Tests that shared framework assemblies return null to allow default loading. + /// Tests that shared framework / curated contract assemblies return null to allow default loading. /// [TestMethod] public void Load_WithSharedFrameworkAssembly_ShouldReturnNull() @@ -108,13 +108,20 @@ public void Load_WithSharedFrameworkAssembly_ShouldReturnNull() try { - // Test various shared framework assemblies + // Test framework assemblies and the curated cross-boundary contract assemblies. + // Names like "CoseHandler", "CoseSign1.Certificates", and Azure SDK assemblies are + // intentionally NOT host-shared after the prefix-list cleanup — see + // PluginLoadContextAdvancedTests for the negative cases. string[] sharedAssemblies = { "System.Collections", "Microsoft.Extensions.Logging", + "System", + "mscorlib", + "netstandard", + // Curated cross-boundary contracts: "CoseSignTool.Abstractions", - "CoseHandler", - "System" + "CoseSign1.Abstractions", + "CoseSign1.Headers", }; foreach (string assemblyName in sharedAssemblies) diff --git a/CoseSignTool.Abstractions.Tests/PluginLoadContextIntegrationTests.cs b/CoseSignTool.Abstractions.Tests/PluginLoadContextIntegrationTests.cs index a3f23df7..f4505709 100644 --- a/CoseSignTool.Abstractions.Tests/PluginLoadContextIntegrationTests.cs +++ b/CoseSignTool.Abstractions.Tests/PluginLoadContextIntegrationTests.cs @@ -125,7 +125,8 @@ public void Integration_MultipleContexts_ShouldWorkIndependently() } /// - /// Tests IsSharedFrameworkAssembly logic with comprehensive framework assembly names. + /// Tests that the curated host-shared list (framework + cross-boundary contracts) returns + /// null from so the default ALC handles them. /// [TestMethod] public void Integration_SharedFrameworkAssemblies_ShouldReturnNullForAllFrameworkTypes() @@ -138,7 +139,10 @@ public void Integration_SharedFrameworkAssemblies_ShouldReturnNullForAllFramewor try { - // Test comprehensive list of framework assemblies + // Test the curated host-shared set: framework families + repo-owned cross-boundary + // contracts. Names like "CoseHandler", "CoseSign1", "CoseIndirectSignature", and + // "Newtonsoft.Json" are intentionally excluded from sharing now — they were prefix + // matches that introduced unwanted host coupling and prefix-collision risk. string[] frameworkAssemblies = { // System assemblies "System", @@ -149,26 +153,22 @@ public void Integration_SharedFrameworkAssemblies_ShouldReturnNullForAllFramewor "System.Text.Json", "System.Threading", "System.Reflection", - + // Microsoft Extensions "Microsoft.Extensions.Logging", "Microsoft.Extensions.DependencyInjection", "Microsoft.Extensions.Configuration", "Microsoft.Extensions.Hosting", - + // Core framework "Microsoft.NETCore.App", "netstandard", "mscorlib", - - // Third party shared - "Newtonsoft.Json", - - // Project specific shared + + // Curated cross-boundary contracts (exact-name match, no prefix collision risk) "CoseSignTool.Abstractions", - "CoseHandler", - "CoseSign1", - "CoseIndirectSignature" + "CoseSign1.Abstractions", + "CoseSign1.Headers", }; foreach (string assemblyName in frameworkAssemblies) @@ -187,6 +187,110 @@ public void Integration_SharedFrameworkAssemblies_ShouldReturnNullForAllFramewor } } + /// + /// Regression test for the prefix-collision smell. A plugin-supplied assembly whose name + /// starts with a formerly-shared prefix (CoseSign1, CoseHandler, CoseIndirectSignature) MUST + /// be loaded plugin-locally — not silently absorbed into the host context. + /// + [TestMethod] + public void Integration_PrefixCollisionImpostor_LoadsPluginLocallyNotHostShared() + { + // Arrange + string pluginPath = Path.Join(_pluginDirectory, "Main.dll"); + File.WriteAllText(pluginPath, "main plugin content"); + + // Place a stub "CoseSign1Plus.dll" in the plugin directory. With the OLD prefix matching, + // the loader would have considered this name shared and skipped the plugin-local probe; + // with the NEW exact-name list the plugin-local probe IS attempted (and fails for a stub + // file, returning null after a logged warning). + string impostorPath = Path.Join(_pluginDirectory, "CoseSign1Plus.dll"); + File.WriteAllBytes(impostorPath, Encoding.UTF8.GetBytes("MZ")); + + PluginLoadContext context = new PluginLoadContext(pluginPath, _pluginDirectory); + + try + { + // Act — capture stderr to confirm the plugin-local probe was attempted (proof that + // the shared-list short-circuit did NOT consume this name). + using StringWriter consoleOutput = new StringWriter(); + TextWriter originalError = Console.Error; + Console.SetError(consoleOutput); + + try + { + AssemblyName impostor = new AssemblyName("CoseSign1Plus"); + Assembly? result = InvokeLoad(context, impostor); + + // Assert + Assert.IsNull(result, "Stub file should fail to load and return null"); + + string stderr = consoleOutput.ToString(); + Assert.IsTrue( + stderr.Contains("CoseSign1Plus", StringComparison.OrdinalIgnoreCase), + $"Plugin-local load attempt for 'CoseSign1Plus' should have produced a warning. Captured stderr: '{stderr}'"); + } + finally + { + Console.SetError(originalError); + } + } + finally + { + context.Unload(); + } + } + + /// + /// Regression test for the resolver-bypass guarantee. When a name is host-shared, the loader + /// MUST NOT probe the plugin directory or the AssemblyDependencyResolver — even if a same-named + /// DLL is present in the plugin directory. Otherwise a malformed plugin-local copy could + /// surface a confusing error or, worse, split type identity from the host. + /// + [TestMethod] + public void Integration_SharedNameWithDuplicateInPluginDir_ShortCircuitsResolver() + { + // Arrange + string pluginPath = Path.Join(_pluginDirectory, "Main.dll"); + File.WriteAllText(pluginPath, "main plugin content"); + + // Place a stub "CoseSignTool.Abstractions.dll" in the plugin directory. If the loader + // probed plugin-local first, it would attempt to load this stub and write a warning. We + // assert the warning is NOT written, proving the short-circuit happens before any probe. + string stubSharedPath = Path.Join(_pluginDirectory, "CoseSignTool.Abstractions.dll"); + File.WriteAllBytes(stubSharedPath, Encoding.UTF8.GetBytes("MZ")); + + PluginLoadContext context = new PluginLoadContext(pluginPath, _pluginDirectory); + + try + { + using StringWriter consoleOutput = new StringWriter(); + TextWriter originalError = Console.Error; + Console.SetError(consoleOutput); + + try + { + AssemblyName sharedName = new AssemblyName("CoseSignTool.Abstractions"); + Assembly? result = InvokeLoad(context, sharedName); + + // Assert + Assert.IsNull(result, "Shared name should return null so the default ALC handles it"); + + string stderr = consoleOutput.ToString(); + Assert.IsFalse( + stderr.Contains("CoseSignTool.Abstractions", StringComparison.OrdinalIgnoreCase), + $"Shared name must short-circuit BEFORE plugin-local probe — no warning expected. Captured stderr: '{stderr}'"); + } + finally + { + Console.SetError(originalError); + } + } + finally + { + context.Unload(); + } + } + /// /// Tests that plugin-specific assemblies are loaded from plugin directory when available. /// diff --git a/CoseSignTool.Abstractions/CertificateProviderPluginManager.cs b/CoseSignTool.Abstractions/CertificateProviderPluginManager.cs index ee9ccc86..2ce4e7a3 100644 --- a/CoseSignTool.Abstractions/CertificateProviderPluginManager.cs +++ b/CoseSignTool.Abstractions/CertificateProviderPluginManager.cs @@ -81,6 +81,13 @@ public void DiscoverAndLoadPlugins(string pluginsDirectory) /// Loads certificate provider plugins from the specified assembly file. /// /// Path to the assembly file to load plugins from. + /// + /// Each plugin assembly is loaded into its own isolated so + /// that the plugin's private dependencies (e.g. Azure SDK transitives) cannot bleed into the + /// host or sibling plugins. Only assemblies in the curated host-shared list (defined on + /// ) are sourced from the host context — everything else is + /// resolved plugin-locally from the directory containing the plugin assembly. + /// public void LoadPluginFromAssembly(string assemblyPath) { if (!File.Exists(assemblyPath)) @@ -90,12 +97,14 @@ public void LoadPluginFromAssembly(string assemblyPath) _logger?.LogVerbose($"Loading plugins from: {Path.GetFileName(assemblyPath)}"); - Assembly assembly = Assembly.LoadFrom(assemblyPath); - + string pluginDirectory = Path.GetDirectoryName(assemblyPath) ?? AppContext.BaseDirectory; + PluginLoadContext loadContext = new PluginLoadContext(assemblyPath, pluginDirectory); + Assembly assembly = loadContext.LoadFromAssemblyPath(assemblyPath); + // Find all types implementing ICertificateProviderPlugin Type[] types = assembly.GetTypes() - .Where(t => typeof(ICertificateProviderPlugin).IsAssignableFrom(t) - && !t.IsInterface + .Where(t => typeof(ICertificateProviderPlugin).IsAssignableFrom(t) + && !t.IsInterface && !t.IsAbstract) .ToArray(); diff --git a/CoseSignTool.Abstractions/PluginLoadContext.cs b/CoseSignTool.Abstractions/PluginLoadContext.cs index bf5fe3bb..e9f1932a 100644 --- a/CoseSignTool.Abstractions/PluginLoadContext.cs +++ b/CoseSignTool.Abstractions/PluginLoadContext.cs @@ -7,114 +7,238 @@ namespace CoseSignTool.Abstractions; /// -/// Custom AssemblyLoadContext for loading plugins with isolated dependencies. -/// Each plugin gets its own context to avoid dependency conflicts. +/// Custom that isolates a plugin's dependencies in its own +/// collectible context while sharing a small, deterministic, audited set of cross-boundary +/// contract assemblies with the host. /// +/// +/// +/// Sharing is decided by two mechanisms: +/// +/// +/// +/// Framework prefix match — a tiny prefix list capturing the .NET / BCL families +/// (Microsoft.Extensions.*, Microsoft.NETCore.*) plus exact framework names +/// (bare System, mscorlib, netstandard). True BCL System.* +/// assemblies (System.Collections, System.Runtime, etc.) are not listed +/// explicitly; they live in TPA and are resolved by the runtime's default-context fallback +/// when our returns null. This intentionally allows out-of-band NuGet +/// packages with System.* names (System.ClientModel, System.Memory.Data, +/// System.IO.Pipelines, etc.) to be loaded plugin-locally. +/// +/// +/// Exact-name allow-list — a curated of repo-owned assemblies +/// whose types cross the host/plugin boundary as method parameters or return values. +/// Exact-name only: prefix matching here would risk false positives (e.g. a 3rd-party +/// CoseSign1Plus.dll being silently absorbed into the host context). +/// +/// +/// +/// The shared decision short-circuits the per-plugin probing logic: if a name is +/// host-shared, returns null immediately without touching the plugin +/// directory or the . This prevents a duplicate type +/// identity from appearing if the plugin happens to ship a copy of the same DLL. +/// +/// +/// What is intentionally NOT shared (so plugins remain isolated): +/// +/// CoseHandler, CoseIndirectSignature — used by host and plugins via static +/// method calls only; instances do not cross the boundary. +/// CoseSign1.Certificates — host downcasts use +/// instead of the +/// concrete CertificateCoseSigningKeyProvider type. +/// CoseSign1.Transparent.MST, CoseSign1.Certificates.AzureArtifactSigning — +/// fully plugin-local. The shared library and the SDK types it extends now co-locate in +/// the plugin's ALC, removing the type-identity bug class entirely. +/// Azure.* SDK packages and out-of-band System.* NuGet packages — only used +/// inside individual plugins. +/// Newtonsoft.Json — plugins ship their own copy if they use it. +/// +/// +/// public class PluginLoadContext : AssemblyLoadContext { - private readonly AssemblyDependencyResolver _resolver; - private readonly string _pluginDirectory; + /// + /// Framework / BCL assembly-name prefixes always sourced from the host. Microsoft.Extensions.* + /// is treated as a shared family because + /// (and helpers around it) cross the host/plugin boundary, and the .NET ecosystem co-versions + /// these packages carefully. + /// + /// + /// System.* is intentionally NOT here — that namespace is mixed: BCL assemblies + /// (System.Collections, System.Runtime) live in TPA and resolve naturally via the + /// default context fallback when our returns null, while + /// out-of-band NuGet packages with System.* names (System.ClientModel, + /// System.Memory.Data, System.IO.Pipelines, etc.) are legitimate plugin-private + /// dependencies that must be loadable from the plugin directory. + /// + private static readonly string[] FrameworkPrefixes = + { + "Microsoft.Extensions.", + "Microsoft.NETCore.", + }; + + /// + /// Bare framework assembly names (no trailing dot) that must be sourced from the host. + /// Exact-name match — bare "System" would otherwise collide with anything starting + /// with the letters S-y-s-t-e-m. + /// + private static readonly IReadOnlySet FrameworkExactNames = + new HashSet(StringComparer.OrdinalIgnoreCase) + { + "System", + "mscorlib", + "netstandard", + "WindowsBase", + "Microsoft.CSharp", + "Microsoft.VisualBasic", + }; + + /// + /// Repo-curated cross-boundary contract assemblies. Exact-name match only. + /// + /// + /// Adding to this list expands the host/plugin shared-type surface — review carefully. Each + /// entry must justify itself with a concrete type that crosses the host/plugin boundary as a + /// method parameter, return value, generic argument, or interface implementation. + /// + private static readonly IReadOnlySet SharedAssemblies = + new HashSet(StringComparer.OrdinalIgnoreCase) + { + // Plugin contracts: ICoseSignToolPlugin, IPluginCommand, ICertificateProviderPlugin, + // IPluginLogger, PluginExitCode, CoseHeaderHelper, CoseHeaderDto. + "CoseSignTool.Abstractions", + + // Cose contracts: ICoseSigningKeyProvider (returned by ICertificateProviderPlugin), + // ISupportsScittCompliance (host capability check on plugin-returned providers). + "CoseSign1.Abstractions", + + // ICoseHeaderExtender flows out of CoseHeaderHelper.CreateHeaderExtender (which lives + // in CoseSignTool.Abstractions) and is consumed by plugin signing commands. Both sides + // must agree on the interface's type identity. + "CoseSign1.Headers", + }; + + private readonly AssemblyDependencyResolver dependencyResolver; + private readonly string pluginDirectory; /// - /// Initializes a new instance of the PluginLoadContext class. + /// Initializes a new instance of the class. /// /// The path to the main plugin assembly. /// The directory containing the plugin and its dependencies. public PluginLoadContext(string pluginPath, string pluginDirectory) : base(isCollectible: true) { - _resolver = new AssemblyDependencyResolver(pluginPath); - _pluginDirectory = pluginDirectory; + this.dependencyResolver = new AssemblyDependencyResolver(pluginPath); + this.pluginDirectory = pluginDirectory; + } + + /// + /// Determines whether the assembly with the specified simple name should be sourced from the + /// host rather than loaded plugin-locally. + /// + /// The simple assembly name (e.g. "CoseSign1.Abstractions"). + /// true when the host owns this assembly; false otherwise. + /// + /// Public so test code can verify the contract without reflection. The decision is two-tier: + /// (1) framework prefix or exact framework name, (2) repo-curated exact-name allow-list. + /// Prefix matching is intentionally restricted to .NET-owned namespaces. + /// + public static bool IsHostShared(string? assemblyName) + { + if (string.IsNullOrEmpty(assemblyName)) + { + return false; + } + + if (FrameworkExactNames.Contains(assemblyName) || SharedAssemblies.Contains(assemblyName)) + { + return true; + } + + for (int i = 0; i < FrameworkPrefixes.Length; i++) + { + if (assemblyName.StartsWith(FrameworkPrefixes[i], StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + + return false; } /// /// Loads an assembly with the specified name. - /// First tries to resolve from the plugin directory, then falls back to default context. /// /// The name of the assembly to load. - /// The loaded assembly, or null if not found. + /// The loaded assembly, or null to delegate to the default context. + /// + /// Order of operations is critical: + /// + /// If the name is host-shared, return null immediately. Do not probe the + /// plugin directory or the resolver — doing so risks loading a duplicate copy that + /// would split type identity from the host. + /// Otherwise, look in the plugin directory directly. + /// If not found there, ask the . + /// If still not found, return null and let the runtime fall back to the + /// default context (which will likely fail — by design — for plugin-private deps). + /// + /// protected override Assembly? Load(AssemblyName assemblyName) { - // For shared dependencies (like Microsoft.Extensions.*, System.*, etc.), - // let the default context handle them to avoid duplicating framework assemblies - if (IsSharedFrameworkAssembly(assemblyName)) + if (assemblyName.Name == null) { - return null; // This will fall back to the default context + return null; } - // Look for the assembly directly in the plugin directory first - if (assemblyName.Name != null) + if (IsHostShared(assemblyName.Name)) { - string expectedPath = Path.Join(_pluginDirectory, $"{assemblyName.Name}.dll"); - if (File.Exists(expectedPath)) + return null; + } + + string expectedPath = Path.Join(this.pluginDirectory, $"{assemblyName.Name}.dll"); + if (File.Exists(expectedPath)) + { + try { - try - { - return LoadFromAssemblyPath(expectedPath); - } - catch (Exception ex) - { - Console.Error.WriteLine($"Warning: Failed to load assembly '{assemblyName.Name}' from '{expectedPath}': {ex.Message}"); - } + return LoadFromAssemblyPath(expectedPath); + } + catch (Exception ex) when (ex is not OutOfMemoryException + and not StackOverflowException + and not ThreadAbortException + and not AccessViolationException) + { + Console.Error.WriteLine($"Warning: Failed to load assembly '{assemblyName.Name}' from '{expectedPath}': {ex.Message}"); } } - // Try to resolve the assembly from the plugin directory using the dependency resolver - string? assemblyPath = _resolver.ResolveAssemblyToPath(assemblyName); + string? assemblyPath = this.dependencyResolver.ResolveAssemblyToPath(assemblyName); if (assemblyPath != null && File.Exists(assemblyPath)) { try { return LoadFromAssemblyPath(assemblyPath); } - catch (Exception ex) + catch (Exception ex) when (ex is not OutOfMemoryException + and not StackOverflowException + and not ThreadAbortException + and not AccessViolationException) { Console.Error.WriteLine($"Warning: Failed to load assembly '{assemblyName.Name}' from resolver path '{assemblyPath}': {ex.Message}"); } } - // If we can't find the assembly, return null to allow default loading return null; } - /// - /// Determines if an assembly should be loaded from the shared framework rather than plugin-specific. - /// - /// The assembly name to check. - /// True if this is a shared framework assembly. - private static bool IsSharedFrameworkAssembly(AssemblyName assemblyName) - { - if (assemblyName.Name == null) - { - return false; - } - - // These should come from the main application context to avoid conflicts - string[] sharedPrefixes = - [ - "System.", - "Microsoft.Extensions.", - "Microsoft.NETCore.", - "netstandard", - "mscorlib", - "System", - "Newtonsoft.Json", - "CoseSignTool.Abstractions", // Always use the main version - "CoseHandler", // Shared components - "CoseSign1", - "CoseIndirectSignature" - ]; - - return sharedPrefixes.Any(prefix => assemblyName.Name.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)); - } - /// /// Loads an unmanaged library with the specified name. /// /// The name of the unmanaged library. - /// A handle to the loaded library, or IntPtr.Zero if not found. + /// A handle to the loaded library, or if not found. protected override IntPtr LoadUnmanagedDll(string unmanagedDllName) { - string? libraryPath = _resolver.ResolveUnmanagedDllToPath(unmanagedDllName); + string? libraryPath = this.dependencyResolver.ResolveUnmanagedDllToPath(unmanagedDllName); if (libraryPath != null && File.Exists(libraryPath)) { return LoadUnmanagedDllFromPath(libraryPath); @@ -123,3 +247,4 @@ protected override IntPtr LoadUnmanagedDll(string unmanagedDllName) return IntPtr.Zero; } } + diff --git a/CoseSignTool.AzureArtifactSigning.Plugin/AzureArtifactSigningCertificateProviderPlugin.cs b/CoseSignTool.AzureArtifactSigning.Plugin/AzureArtifactSigningCertificateProviderPlugin.cs index 6020fe2f..aad26c73 100644 --- a/CoseSignTool.AzureArtifactSigning.Plugin/AzureArtifactSigningCertificateProviderPlugin.cs +++ b/CoseSignTool.AzureArtifactSigning.Plugin/AzureArtifactSigningCertificateProviderPlugin.cs @@ -3,8 +3,10 @@ namespace CoseSignTool.AzureArtifactSigning.Plugin; +using Azure.CodeSigning; using Azure.Core; using Azure.Developer.ArtifactSigning.CryptoProvider; +using Azure.Developer.ArtifactSigning.CryptoProvider.Models; using Azure.Identity; using CoseSign1.Certificates.AzureArtifactSigning; using System; @@ -103,20 +105,31 @@ public ICoseSigningKeyProvider CreateProvider(IConfiguration configuration, IPlu }); logger?.LogVerbose("Creating CertificateProfileClient..."); - // Create the Certificate Profile Client + // Create the Certificate Profile Client with fast-retry pipeline tuning so transient + // Azure SDK retries do not balloon interactive signing latency. The default 800 ms + // exponential back-off (3 retries, ~5 s ceiling) is replaced with 250 ms fixed retries + // (8 retries, ~2 s ceiling). Callers can override via ConfigureAasPerformanceOptimizations. + CertificateProfileClientOptions clientOptions = new(); + clientOptions.ConfigureAasPerformanceOptimizations(); + // Constructor: CertificateProfileClient(TokenCredential credential, Uri endpoint, options) Uri endpointUri = new Uri(endpoint); - var certificateProfileClient = new Azure.CodeSigning.CertificateProfileClient( + Azure.CodeSigning.CertificateProfileClient certificateProfileClient = new Azure.CodeSigning.CertificateProfileClient( credential, - endpointUri); + endpointUri, + clientOptions); logger?.LogVerbose("Creating AzSignContext..."); - // Create AzSignContext using the certificate profile client - // Constructor: AzSignContext(accountName, certProfile, cpClient, ...) + // Create AzSignContext with explicit performance options. Defaults match the SDK + // baseline (3 task retries, 60 s task timeout) — surfaced here so future tuning + // touches a single point. + AzSignContextOptions signContextOptions = AasClientOptionsExtensions.ConfigureAasSigningPerformance(); AzSignContext signContext = new AzSignContext( accountName, certProfileName, - certificateProfileClient); + certificateProfileClient, + null, + signContextOptions); logger?.LogVerbose("Creating AzureArtifactSigningCoseSigningKeyProvider..."); // Create and return the key provider diff --git a/CoseSignTool.MST.Plugin.Tests/MstPluginTests.cs b/CoseSignTool.MST.Plugin.Tests/MstPluginTests.cs index 0c6b95e5..6fe5d3a4 100644 --- a/CoseSignTool.MST.Plugin.Tests/MstPluginTests.cs +++ b/CoseSignTool.MST.Plugin.Tests/MstPluginTests.cs @@ -415,31 +415,55 @@ public async Task VerifyCommand_ExecuteAsync_WithCancellation_ReturnsInvalidArgu /// /// Tests for the CodeTransparencyClientHelper class. /// +/// +/// Marked because every test mutates the process-wide +/// environment variables MST_TOKEN and TEST_CTS_TOKEN. Even with per-test +/// try/finally blocks, concurrent execution within the class would race on the env-var slot. +/// / snapshot and restore the values +/// so a test that throws en route still leaves the process clean for downstream classes. +/// [TestClass] +[DoNotParallelize] public class CodeTransparencyClientHelperTests { private const string TestEndpoint = "https://test.confidential-ledger.azure.com"; private const string TestToken = "test-token-12345"; private const string TestEnvVarName = "TEST_CTS_TOKEN"; + private string? OriginalMstToken; + private string? OriginalTestEnvVar; + + [TestInitialize] + public void Setup() + { + OriginalMstToken = Environment.GetEnvironmentVariable("MST_TOKEN"); + OriginalTestEnvVar = Environment.GetEnvironmentVariable(TestEnvVarName); + + // Start every test from a known-clean env-var state. + Environment.SetEnvironmentVariable("MST_TOKEN", null); + Environment.SetEnvironmentVariable(TestEnvVarName, null); + } + + [TestCleanup] + public void Cleanup() + { + // Restore whatever the host shell had set so other test classes (or interactive + // re-runs) are unaffected by our mutations, even if a test threw mid-flight. + Environment.SetEnvironmentVariable("MST_TOKEN", OriginalMstToken); + Environment.SetEnvironmentVariable(TestEnvVarName, OriginalTestEnvVar); + } + [TestMethod] public async Task CreateClientAsync_WithTokenFromDefaultEnvironmentVariable_CreatesClient() { // Arrange Environment.SetEnvironmentVariable("MST_TOKEN", TestToken); - try - { - // Act - Azure.Security.CodeTransparency.CodeTransparencyClient client = await CodeTransparencyClientHelper.CreateClientAsync(TestEndpoint, null); - // Assert - Assert.IsNotNull(client); - } - finally - { - // Cleanup - Environment.SetEnvironmentVariable("MST_TOKEN", null); - } + // Act + Azure.Security.CodeTransparency.CodeTransparencyClient client = await CodeTransparencyClientHelper.CreateClientAsync(TestEndpoint, null, useAzureAuth: false); + + // Assert + Assert.IsNotNull(client); } [TestMethod] @@ -447,75 +471,157 @@ public async Task CreateClientAsync_WithTokenFromCustomEnvironmentVariable_Creat { // Arrange Environment.SetEnvironmentVariable(TestEnvVarName, TestToken); - try - { - // Act - Azure.Security.CodeTransparency.CodeTransparencyClient client = await CodeTransparencyClientHelper.CreateClientAsync(TestEndpoint, TestEnvVarName); - // Assert - Assert.IsNotNull(client); - } - finally - { - // Cleanup - Environment.SetEnvironmentVariable(TestEnvVarName, null); - } + // Act + Azure.Security.CodeTransparency.CodeTransparencyClient client = await CodeTransparencyClientHelper.CreateClientAsync(TestEndpoint, TestEnvVarName, useAzureAuth: false); + + // Assert + Assert.IsNotNull(client); } [TestMethod] - public async Task CreateClientAsync_WithoutTokenEnvironmentVariable_UsesDefaultCredential() + public async Task CreateClientAsync_WithoutTokenAndDefaults_CreatesAnonymousClient() { - // Arrange - Environment.SetEnvironmentVariable("MST_TOKEN", null); - Environment.SetEnvironmentVariable(TestEnvVarName, null); + // Arrange — env vars are clean per [TestInitialize]. + + // Act - default behaviour (useAzureAuth=false) yields an anonymous client; no + // network or credential acquisition occurs. + Azure.Security.CodeTransparency.CodeTransparencyClient client = await CodeTransparencyClientHelper.CreateClientAsync(TestEndpoint, null, useAzureAuth: false); + // Assert + Assert.IsNotNull(client); + } + + [TestMethod] + public async Task CreateClientAsync_WithoutTokenAndAzureAuth_UsesDefaultCredential() + { + // Arrange — env vars are clean per [TestInitialize]. try { // Act - Azure.Security.CodeTransparency.CodeTransparencyClient client = await CodeTransparencyClientHelper.CreateClientAsync(TestEndpoint, TestEnvVarName); + Azure.Security.CodeTransparency.CodeTransparencyClient client = await CodeTransparencyClientHelper.CreateClientAsync(TestEndpoint, null, useAzureAuth: true); // Assert - // If DefaultAzureCredential succeeds (e.g., Azure CLI is logged in), the client should be created + // If DefaultAzureCredential succeeds (e.g., Azure CLI is logged in), the client should be created. Assert.IsNotNull(client); } catch (Azure.Identity.CredentialUnavailableException) { - // Assert - // In a test environment without Azure credentials, DefaultAzureCredential will throw CredentialUnavailableException - // This is also valid behavior - the method attempted to use DefaultAzureCredential as expected + // In a test environment without Azure credentials, DefaultAzureCredential will throw + // CredentialUnavailableException. That is also valid behaviour for this opt-in path. Assert.IsTrue(true, "DefaultAzureCredential correctly threw CredentialUnavailableException when no credentials are available"); } } [TestMethod] - public async Task CreateClientAsync_WithEmptyTokenEnvironmentVariable_UsesDefaultCredential() + public async Task CreateClientAsync_WithExplicitTokenEnvButMissingValue_ThrowsInvalidOperation() + { + // Arrange — explicit env var name supplied but the env var is unset (per [TestInitialize]). + + // Act / Assert + InvalidOperationException ex = await Assert.ThrowsExceptionAsync( + () => CodeTransparencyClientHelper.CreateClientAsync(TestEndpoint, TestEnvVarName, useAzureAuth: false)); + StringAssert.Contains(ex.Message, TestEnvVarName); + } + + [TestMethod] + public async Task CreateClientAsync_WithExplicitTokenEnvButWhitespaceValue_ThrowsInvalidOperation() + { + // Arrange — explicit env var name supplied but the env var is whitespace. + Environment.SetEnvironmentVariable(TestEnvVarName, " "); + + // Act / Assert + InvalidOperationException ex = await Assert.ThrowsExceptionAsync( + () => CodeTransparencyClientHelper.CreateClientAsync(TestEndpoint, TestEnvVarName, useAzureAuth: false)); + StringAssert.Contains(ex.Message, TestEnvVarName); + } + + [TestMethod] + public async Task CreateClientAsync_WithBothTokenAndAzureAuth_TokenWins() { // Arrange - Environment.SetEnvironmentVariable(TestEnvVarName, ""); - try - { - try - { - // Act - Azure.Security.CodeTransparency.CodeTransparencyClient client = await CodeTransparencyClientHelper.CreateClientAsync(TestEndpoint, TestEnvVarName); - - // Assert - // If DefaultAzureCredential succeeds (e.g., Azure CLI is logged in), the client should be created - Assert.IsNotNull(client); - } - catch (Azure.Identity.CredentialUnavailableException) - { - // Assert - // In a test environment without Azure credentials, DefaultAzureCredential will throw CredentialUnavailableException - // This is also valid behavior - the method attempted to use DefaultAzureCredential as expected - Assert.IsTrue(true, "DefaultAzureCredential correctly threw CredentialUnavailableException when no credentials are available"); - } - } - finally - { - // Cleanup - Environment.SetEnvironmentVariable(TestEnvVarName, null); - } + Environment.SetEnvironmentVariable("MST_TOKEN", TestToken); + + // Act — even with --azure-auth set, an explicit MST_TOKEN should be used + // (no DefaultAzureCredential acquisition takes place). + Azure.Security.CodeTransparency.CodeTransparencyClient client = await CodeTransparencyClientHelper.CreateClientAsync(TestEndpoint, null, useAzureAuth: true); + + // Assert + Assert.IsNotNull(client); + } +} + +/// +/// Tests that verify the --azure-auth CLI flag is wired correctly through the option +/// metadata on both MST commands. These exercise the boolean-flag plumbing — a missing +/// BooleanOptions entry would cause the host CLI parser to reject a bare +/// --azure-auth, which would silently re-introduce the bug we just fixed. +/// +[TestClass] +public class MstAzureAuthFlagWiringTests +{ + [TestMethod] + public void RegisterCommand_Options_ContainsAzureAuth() + { + // Arrange + RegisterCommand command = new RegisterCommand(); + + // Assert + Assert.IsTrue(command.Options.ContainsKey("azure-auth"), + "RegisterCommand.Options must include azure-auth so the CLI parser recognises --azure-auth."); + } + + [TestMethod] + public void RegisterCommand_BooleanOptions_ContainsAzureAuth() + { + // Arrange + RegisterCommand command = new RegisterCommand(); + + // Assert + Assert.IsTrue(command.BooleanOptions.Contains("azure-auth"), + "RegisterCommand.BooleanOptions must include azure-auth so a bare --azure-auth is accepted without a value."); + } + + [TestMethod] + public void VerifyCommand_Options_ContainsAzureAuth() + { + // Arrange + VerifyCommand command = new VerifyCommand(); + + // Assert + Assert.IsTrue(command.Options.ContainsKey("azure-auth"), + "VerifyCommand.Options must include azure-auth so the CLI parser recognises --azure-auth."); + } + + [TestMethod] + public void VerifyCommand_BooleanOptions_ContainsAzureAuth() + { + // Arrange + VerifyCommand command = new VerifyCommand(); + + // Assert + Assert.IsTrue(command.BooleanOptions.Contains("azure-auth"), + "VerifyCommand.BooleanOptions must include azure-auth so a bare --azure-auth is accepted without a value."); + } + + [TestMethod] + public void BooleanOptions_IsSharedAcrossInstances() + { + // Arrange + RegisterCommand register1 = new RegisterCommand(); + RegisterCommand register2 = new RegisterCommand(); + VerifyCommand verify = new VerifyCommand(); + + // Assert + // Locks in the perf optimisation: BooleanOptions is backed by a single static array, + // not allocated per instance. ReferenceEquals on the underlying collection guarantees this. + Assert.AreSame(register1.BooleanOptions, register2.BooleanOptions, + "RegisterCommand.BooleanOptions must be the same reference across instances (backed by static readonly)."); + Assert.AreSame(register1.BooleanOptions, verify.BooleanOptions, + "BooleanOptions must be the same reference across all MST commands (shared static readonly)."); } } + + + diff --git a/CoseSignTool.MST.Plugin/CodeTransparencyClientHelper.cs b/CoseSignTool.MST.Plugin/CodeTransparencyClientHelper.cs index b32ef4dd..55a62769 100644 --- a/CoseSignTool.MST.Plugin/CodeTransparencyClientHelper.cs +++ b/CoseSignTool.MST.Plugin/CodeTransparencyClientHelper.cs @@ -13,41 +13,94 @@ namespace CoseSignTool.MST.Plugin; internal static class CodeTransparencyClientHelper { /// - /// Creates a CodeTransparencyClient with the specified endpoint and optional token environment variable. + /// Default scope used when acquiring an access token via DefaultAzureCredential. /// - /// The Azure Code Transparency Service endpoint URL. - /// Optional name of the environment variable containing the access token. - /// If not specified, defaults to "MST_TOKEN". If the environment variable is not set, - /// uses DefaultAzureCredential. + private const string DefaultAzureCredentialScope = "https://confidential-ledger.azure.com/.default"; + + /// + /// Default name of the environment variable to read for an access token when no --token-env + /// override is supplied. + /// + private const string DefaultTokenEnvVarName = "MST_TOKEN"; + + /// + /// Pre-allocated single-element scope array for token requests. + /// Hoisted to static readonly so the array is not re-allocated on every invocation. + /// + private static readonly string[] DefaultAzureCredentialScopes = new[] { DefaultAzureCredentialScope }; + + /// + /// Creates a with the specified endpoint and authentication mode. + /// + /// The Microsoft's Signing Transparency (MST) service endpoint URL. + /// + /// Optional name of the environment variable containing an access token. When the caller supplies + /// this value explicitly (non-whitespace), the variable MUST contain a non-whitespace value or the + /// call fails fast with . When null or whitespace, + /// the helper consults MST_TOKEN as a non-fatal default. + /// + /// + /// When true, fall back to to acquire a token if no + /// access token is found via the environment variable. When false (default), the client is + /// constructed without credentials so calls reach the endpoint anonymously — appropriate for + /// unauthenticated MST instances such as test ledgers. + /// + /// + /// Optional logger. When supplied, the helper emits a verbose-level message indicating which + /// authentication path was selected (token / azure-auth / anonymous). The token contents are + /// never logged. + /// /// Cancellation token for async operations. - /// A configured CodeTransparencyClient instance. - /// Thrown when token environment variable is empty or invalid. - public static async Task CreateClientAsync(string endpoint, string? tokenEnvVarName, CancellationToken cancellationToken = default) + /// A configured instance. + /// + /// Thrown when is supplied explicitly but the named environment + /// variable is missing, empty, or whitespace. + /// + public static async Task CreateClientAsync( + string endpoint, + string? tokenEnvVarName, + bool useAzureAuth, + IPluginLogger? logger = null, + CancellationToken cancellationToken = default) { - Uri uri = new Uri(endpoint); - CodeTransparencyClientOptions clientOptions = new CodeTransparencyClientOptions(); + Uri uri = new(endpoint); + CodeTransparencyClientOptions clientOptions = new(); clientOptions.ConfigureMstPerformanceOptimizations(); - // Use the specified environment variable name or default to MST_TOKEN - string envVarName = tokenEnvVarName ?? "MST_TOKEN"; + bool tokenEnvVarExplicitlyRequested = !string.IsNullOrWhiteSpace(tokenEnvVarName); + string envVarName = tokenEnvVarExplicitlyRequested ? tokenEnvVarName! : DefaultTokenEnvVarName; string? token = Environment.GetEnvironmentVariable(envVarName); - if (!string.IsNullOrEmpty(token)) + if (!string.IsNullOrWhiteSpace(token)) { - // Use the access token from the environment variable - // Use AzureKeyCredential for access tokens as documented in: + // Use the access token from the environment variable. + // AzureKeyCredential is the documented pattern for static tokens, see: // https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/confidentialledger/Azure.Security.CodeTransparency/samples/Sample3_UseYourCredentials.md - AzureKeyCredential credential = new AzureKeyCredential(token); + logger?.LogVerbose($"MST auth: using access token from environment variable '{envVarName}'"); + AzureKeyCredential credential = new(token); return new CodeTransparencyClient(uri, credential, clientOptions); } - // Use default Azure credential (managed identity, Azure CLI, etc.) when no token is provided - // Note: CodeTransparencyClient constructor only accepts TokenCredential if using DefaultAzureCredential - // directly, but the pattern from Azure docs uses AzureKeyCredential with retrieved tokens - DefaultAzureCredential defaultCred = new DefaultAzureCredential(); // CodeQL [SM05137] This is non-production testing code which is not deployed. - string[] defaultScopes = new[] { "https://confidential-ledger.azure.com/.default" }; - AccessToken defaultToken = await defaultCred.GetTokenAsync(new TokenRequestContext(defaultScopes), cancellationToken); - return new CodeTransparencyClient(uri, new AzureKeyCredential(defaultToken.Token), clientOptions); + if (tokenEnvVarExplicitlyRequested) + { + throw new InvalidOperationException( + $"--token-env was set to '{envVarName}' but the environment variable is missing, empty, or whitespace."); + } + + if (useAzureAuth) + { + // Acquire a token via the Azure default credential chain (CLI, MSI, VS, etc.). + logger?.LogVerbose("MST auth: using Azure DefaultAzureCredential (--azure-auth)"); + DefaultAzureCredential defaultCred = new(); // CodeQL [SM05137] This is non-production testing code which is not deployed. + AccessToken defaultToken = await defaultCred.GetTokenAsync(new TokenRequestContext(DefaultAzureCredentialScopes), cancellationToken).ConfigureAwait(false); + return new CodeTransparencyClient(uri, new AzureKeyCredential(defaultToken.Token), clientOptions); + } + + // No credential supplied — construct an anonymous client. Appropriate for unauthenticated + // MST instances (e.g., test ledgers). The SDK omits the bearer auth policy when no + // credential is provided, so no Authorization header is sent. + logger?.LogVerbose("MST auth: anonymous (no credentials sent). Pass --token-env or --azure-auth if the endpoint requires auth."); + return new CodeTransparencyClient(uri, clientOptions); } } diff --git a/CoseSignTool.MST.Plugin/CoseSignTool.MST.Plugin.csproj b/CoseSignTool.MST.Plugin/CoseSignTool.MST.Plugin.csproj index 97f14fdd..edcba0bf 100644 --- a/CoseSignTool.MST.Plugin/CoseSignTool.MST.Plugin.csproj +++ b/CoseSignTool.MST.Plugin/CoseSignTool.MST.Plugin.csproj @@ -55,7 +55,6 @@ - diff --git a/CoseSignTool.MST.Plugin/MstCommandBase.cs b/CoseSignTool.MST.Plugin/MstCommandBase.cs index 2d2cb052..12c1e02b 100644 --- a/CoseSignTool.MST.Plugin/MstCommandBase.cs +++ b/CoseSignTool.MST.Plugin/MstCommandBase.cs @@ -24,12 +24,26 @@ public abstract class MstCommandBase : PluginCommandBase { { "endpoint", "The Microsoft's Signing Transparency (MST) service endpoint URL" }, { "token-env", "The name of the environment variable containing the access token (default: MST_TOKEN)" }, + { "azure-auth", "When set, fall back to Azure DefaultAzureCredential (CLI/MSI/etc.) if no access token is provided. Default behaviour is anonymous (suitable for unauthenticated MST instances)." }, { "payload", "The file path to the payload file" }, { "signature", "The file path to the COSE Sign1 signature file" }, { "output", "The file path where the result will be written (optional)" }, { "timeout", "Timeout in seconds for the operation (default: 30)" } }; + /// + /// Pre-allocated single-element collection of boolean option names exposed by all MST commands. + /// Hoisted to static readonly so each command instance does not allocate its own array. + /// + private static readonly string[] BooleanOptionsArray = new[] { "azure-auth" }; + + /// + /// + /// azure-auth is a boolean flag that opts into Azure DefaultAzureCredential when no + /// explicit access token is supplied. It can be passed without a value (e.g., --azure-auth). + /// + public override IReadOnlyCollection BooleanOptions => BooleanOptionsArray; + /// /// Validates common parameters and returns parsed timeout value. /// @@ -82,7 +96,7 @@ protected static PluginExitCode ValidateFilePaths(Dictionary fil { try { - byte[] signatureBytes = await File.ReadAllBytesAsync(signaturePath, cancellationToken); + byte[] signatureBytes = await File.ReadAllBytesAsync(signaturePath, cancellationToken).ConfigureAwait(false); CoseSign1Message message = CoseMessage.DecodeSign1(signatureBytes); return (message, signatureBytes, PluginExitCode.Success); } @@ -121,7 +135,7 @@ protected static CancellationTokenSource CreateTimeoutCancellationToken(int time protected static async Task WriteJsonResult(string outputPath, object result, CancellationToken cancellationToken, IPluginLogger? logger = null) { string jsonOutput = JsonSerializer.Serialize(result, new JsonSerializerOptions { WriteIndented = true }); - await File.WriteAllTextAsync(outputPath, jsonOutput, cancellationToken); + await File.WriteAllTextAsync(outputPath, jsonOutput, cancellationToken).ConfigureAwait(false); logger?.LogInformation($"Result written to: {outputPath}"); } @@ -166,6 +180,12 @@ protected static PluginExitCode HandleCommonException(Exception ex, IConfigurati FileNotFoundException fileEx => HandleError($"File not found - {fileEx.Message}", PluginExitCode.UserSpecifiedFileNotFound, logger), + // Auth-configuration errors (e.g., explicit --token-env name with missing/whitespace value) + // surface as InvalidOperationException from CodeTransparencyClientHelper. Map them to + // InvalidArgumentValue so the operator gets a semantic exit code rather than UnknownError. + InvalidOperationException invOpEx => + HandleError(invOpEx.Message, PluginExitCode.InvalidArgumentValue, logger), + OperationCanceledException when cancellationToken.IsCancellationRequested => HandleError("Operation was cancelled.", PluginExitCode.UnknownError, logger), @@ -188,11 +208,13 @@ static PluginExitCode HandleError(string message, PluginExitCode code, IPluginLo /// /// The CTS endpoint URL. /// Name of the environment variable containing the access token. + /// When true, fall back to Azure DefaultAzureCredential if no token is set. + /// Optional logger used to record which auth path was selected. /// Cancellation token. /// Configured CodeTransparencyClient. - protected static Task CreateCtsClient(string endpoint, string? tokenEnvVarName, CancellationToken cancellationToken) + protected static Task CreateCtsClient(string endpoint, string? tokenEnvVarName, bool useAzureAuth, IPluginLogger? logger, CancellationToken cancellationToken) { - return CodeTransparencyClientHelper.CreateClientAsync(endpoint, tokenEnvVarName, cancellationToken); + return CodeTransparencyClientHelper.CreateClientAsync(endpoint, tokenEnvVarName, useAzureAuth, logger, cancellationToken); } /// @@ -217,6 +239,7 @@ public override async Task ExecuteAsync(IConfiguration configura // Get optional parameters string? tokenEnvVarName = GetOptionalValue(configuration, "token-env"); string? outputPath = GetOptionalValue(configuration, "output"); + bool useAzureAuth = GetBooleanFlag(configuration, "azure-auth"); // Validate common parameters PluginExitCode validationResult = ValidateCommonParameters(configuration, out int timeoutSeconds, Logger); @@ -242,25 +265,28 @@ public override async Task ExecuteAsync(IConfiguration configura } // Read and decode COSE message - (CoseSign1Message message, byte[] signatureBytes, PluginExitCode readResult) = await ReadAndDecodeCoseMessage(signaturePath, cancellationToken, Logger); + (CoseSign1Message message, byte[] signatureBytes, PluginExitCode readResult) = await ReadAndDecodeCoseMessage(signaturePath, cancellationToken, Logger).ConfigureAwait(false); if (readResult != PluginExitCode.Success || message == null) { return readResult; } + // Honour --timeout for the entire operation including auth acquisition (DefaultAzureCredential + // can hang on developer credentials, so the client construction must respect the timeout too). + using CancellationTokenSource combinedCts = CreateTimeoutCancellationToken(timeoutSeconds, cancellationToken); + // Create CTS client - CodeTransparencyClient client = await CreateCtsClient(endpoint, tokenEnvVarName, cancellationToken); + CodeTransparencyClient client = await CreateCtsClient(endpoint, tokenEnvVarName, useAzureAuth, Logger, combinedCts.Token).ConfigureAwait(false); // Execute the specific operation - using CancellationTokenSource combinedCts = CreateTimeoutCancellationToken(timeoutSeconds, cancellationToken); (PluginExitCode exitCode, object? result) operationResult = await ExecuteSpecificOperation( client, message, signatureBytes, endpoint, payloadPath, signaturePath, - configuration, combinedCts.Token); + configuration, combinedCts.Token).ConfigureAwait(false); // Write output if requested if (!string.IsNullOrEmpty(outputPath) && operationResult.result != null) { - await WriteJsonResult(outputPath, operationResult.result, cancellationToken, Logger); + await WriteJsonResult(outputPath, operationResult.result, cancellationToken, Logger).ConfigureAwait(false); } return operationResult.exitCode; @@ -316,8 +342,10 @@ protected virtual string GetBaseUsage(string commandName, string verb) $" --signature The file path to the COSE Sign1 signature file{Environment.NewLine}" + $"{Environment.NewLine}" + $"Optional arguments:{Environment.NewLine}" + - $" --token-env Name of environment variable containing access token{Environment.NewLine}" + - $" (default: MST_TOKEN, uses default Azure credential if not specified){Environment.NewLine}" + + $" --token-env Name of environment variable containing access token (default: MST_TOKEN){Environment.NewLine}" + + $" --azure-auth Opt into Azure DefaultAzureCredential (CLI / managed identity / etc.){Environment.NewLine}" + + $" when no access token is supplied. Default behaviour is anonymous,{Environment.NewLine}" + + $" which is appropriate for unauthenticated MST instances (test ledgers).{Environment.NewLine}" + $" --output File path where {verb} result will be written{Environment.NewLine}"; } diff --git a/CoseSignTool.MST.Plugin/RegisterCommand.cs b/CoseSignTool.MST.Plugin/RegisterCommand.cs index 7e2a7005..3e5ca1f1 100644 --- a/CoseSignTool.MST.Plugin/RegisterCommand.cs +++ b/CoseSignTool.MST.Plugin/RegisterCommand.cs @@ -61,7 +61,7 @@ protected override string GetExamples() Logger.LogVerbose("Calling MakeTransparentAsync..."); // Register with the transparency service - CoseSign1Message result = await transparencyService.MakeTransparentAsync(message, cancellationToken); + CoseSign1Message result = await transparencyService.MakeTransparentAsync(message, cancellationToken).ConfigureAwait(false); Logger.LogInformation("Registration completed successfully."); diff --git a/CoseSignTool.MST.Plugin/VerifyCommand.cs b/CoseSignTool.MST.Plugin/VerifyCommand.cs index 6a12d824..1606c75c 100644 --- a/CoseSignTool.MST.Plugin/VerifyCommand.cs +++ b/CoseSignTool.MST.Plugin/VerifyCommand.cs @@ -109,15 +109,15 @@ protected override void AddAdditionalFileValidation(Dictionary r { Logger.LogVerbose($"Verifying with specific receipt from: {receiptPath}"); // Verify with a specific receipt - byte[] receipt = await File.ReadAllBytesAsync(receiptPath, cancellationToken); + byte[] receipt = await File.ReadAllBytesAsync(receiptPath, cancellationToken).ConfigureAwait(false); Logger.LogVerbose($"Receipt size: {receipt.Length} bytes"); - isValid = await transparencyService.VerifyTransparencyAsync(message, receipt, cancellationToken); + isValid = await transparencyService.VerifyTransparencyAsync(message, receipt, cancellationToken).ConfigureAwait(false); } else { Logger.LogVerbose("Verifying using embedded transparency information"); // Verify using embedded transparency information - isValid = await message.VerifyTransparencyAsync(transparencyService, cancellationToken); + isValid = await message.VerifyTransparencyAsync(transparencyService, cancellationToken).ConfigureAwait(false); } if (isValid) diff --git a/CoseSignTool/CoseSignTool.csproj b/CoseSignTool/CoseSignTool.csproj index 009c6857..65d806e8 100644 --- a/CoseSignTool/CoseSignTool.csproj +++ b/CoseSignTool/CoseSignTool.csproj @@ -78,7 +78,6 @@ - @@ -109,11 +108,13 @@ - + `--azure-auth` (DefaultAzureCredential) > anonymous. -Set an access token in an environment variable: +### 1. Anonymous (default) + +When no access token is supplied and `--azure-auth` is not passed, the plugin constructs the client without credentials. No `Authorization` header is sent. + +```bash +# Anonymous registration against an unauthenticated test ledger +CoseSignTool mst_register \ + --endpoint https://your-mst-test-instance.confidential-ledger.azure.com \ + --payload myfile.txt \ + --signature myfile.txt.cose +``` + +### 2. Environment Variable Authentication + +Set an access token in an environment variable. When `--token-env ` is supplied explicitly, the named variable **must** contain a non-whitespace value or the command fails fast. ```bash -# Using the default environment variable +# Using the default environment variable (MST_TOKEN) export MST_TOKEN="your-access-token" # Using a custom environment variable export MY_MST_TOKEN="your-access-token" +CoseSignTool mst_register ... --token-env MY_MST_TOKEN ``` -### 2. Azure DefaultCredential (Fallback) +### 3. Azure DefaultCredential (`--azure-auth`) -When no token is provided, the plugin automatically falls back to Azure DefaultCredential, which supports: +Pass the `--azure-auth` flag to opt into [`DefaultAzureCredential`](https://docs.microsoft.com/en-us/dotnet/api/azure.identity.defaultazurecredential), which supports: - Azure CLI credentials (`az login`) - Managed Identity (in Azure environments) - Azure PowerShell credentials - Environment variables (`AZURE_CLIENT_ID`, `AZURE_CLIENT_SECRET`, `AZURE_TENANT_ID`) -- Visual Studio credentials -- VS Code credentials +- Visual Studio / VS Code credentials + +```bash +# Requires az login or other Azure authentication +CoseSignTool mst_register \ + --endpoint https://your-mst-instance.azure.com \ + --payload myfile.txt \ + --signature myfile.txt.cose \ + --azure-auth +``` -> **⚠️ Production Security Note**: When deploying to production environments, create an environment variable named `AZURE_TOKEN_CREDENTIALS` and set its value to `"prod"`. This excludes developer tool credentials from the credential chain, ensuring only production-appropriate credentials are used. This is required when using Azure.Identity version 1.14.0 or later. For more information, see the [DefaultAzureCredential overview](https://docs.microsoft.com/en-us/dotnet/api/azure.identity.defaultazurecredential). +> **⚠️ Production Security Note**: When using `--azure-auth` in production, create an environment variable named `AZURE_TOKEN_CREDENTIALS` and set its value to `"prod"`. This excludes developer tool credentials from the credential chain, ensuring only production-appropriate credentials are used. This is required when using Azure.Identity version 1.14.0 or later. See the [DefaultAzureCredential overview](https://docs.microsoft.com/en-us/dotnet/api/azure.identity.defaultazurecredential). ```bash # In production environments, set this environment variable: @@ -64,15 +87,24 @@ CoseSignTool mst_register [OPTIONS] #### Required Options - `--endpoint` - Azure CTS service endpoint URL - `--payload` - Path to the payload file -- `--signature` - Path to the COSE Sign1 signature file +- `--signature` - Path to the COSE Sign1 signature file (must be **embedded** — MST rejects detached payloads) #### Optional Options -- `--token-env-var` - Environment variable name containing the access token (default: `MST_TOKEN`) +- `--token-env` - Environment variable name containing the access token (default: `MST_TOKEN`). When passed explicitly, the named variable **must** be non-empty. +- `--azure-auth` - Opt into Azure `DefaultAzureCredential` when no token is supplied. Default is anonymous. - `--output` - Output file path for the registration result -- `--timeout` - Operation timeout in seconds (default: 30) +- `--timeout` - Operation timeout in seconds (default: 30). The timeout covers credential acquisition as well as the registration call. #### Examples +**Anonymous (test ledger):** +```bash +CoseSignTool mst_register \ + --endpoint https://aasmsttest.confidential-ledger.azure.com \ + --payload myfile.txt \ + --signature myfile.txt.cose +``` + **Using default environment variable:** ```bash export MST_TOKEN="your-access-token" @@ -89,7 +121,7 @@ CoseSignTool mst_register \ --endpoint https://your-mst-instance.azure.com \ --payload myfile.txt \ --signature myfile.txt.cose \ - --token-env-var MY_MST_TOKEN + --token-env MY_MST_TOKEN ``` **Using Azure DefaultCredential:** @@ -126,10 +158,11 @@ CoseSignTool mst_verify [OPTIONS] - `--signature` - Path to the COSE Sign1 signature file #### Optional Options -- `--token-env-var` - Environment variable name containing the access token (default: `MST_TOKEN`) +- `--token-env` - Environment variable name containing the access token (default: `MST_TOKEN`). When passed explicitly, the named variable **must** be non-empty. +- `--azure-auth` - Opt into Azure `DefaultAzureCredential` when no token is supplied. Default is anonymous. - `--output` - Output file path for the verification result - `--receipt` - Path to a specific receipt file to use for verification -- `--timeout` - Operation timeout in seconds (default: 30) +- `--timeout` - Operation timeout in seconds (default: 30). The timeout covers credential acquisition as well as the verification call. - `--authorized-domains` - Comma-separated list of authorized issuer domains for receipt verification - `--authorized-receipt-behavior` - Behavior for receipts from authorized domains: - `VerifyAnyMatching` - At least one receipt from any authorized domain must be valid diff --git a/docs/Plugins.md b/docs/Plugins.md index 9e0dbae1..516a45fb 100644 --- a/docs/Plugins.md +++ b/docs/Plugins.md @@ -1231,7 +1231,7 @@ CoseSignTool mst_register \ --endpoint https://your-cts-instance.azure.com \ --payload myfile.txt \ --signature myfile.txt.cose \ - --token-env-var MY_MST_TOKEN + --token-env MY_MST_TOKEN # Verify a signature with Microsoft's Signing Transparency (MST) export MST_TOKEN="your-access-token" @@ -1241,26 +1241,37 @@ CoseSignTool mst_verify \ --signature myfile.txt.cose \ --receipt receipt.json -# Using Azure DefaultCredential when no token is provided +# Anonymous registration against an unauthenticated MST instance (default) CoseSignTool mst_register \ - --endpoint https://your-cts-instance.azure.com \ + --endpoint https://your-mst-test-instance.confidential-ledger.azure.com \ --payload myfile.txt \ --signature myfile.txt.cose + +# Using Azure DefaultCredential (opt-in via --azure-auth) +CoseSignTool mst_register \ + --endpoint https://your-cts-instance.azure.com \ + --payload myfile.txt \ + --signature myfile.txt.cose \ + --azure-auth ``` ### Authentication -The MST plugin supports multiple authentication methods with the following priority: +The MST plugin supports three authentication modes with the following precedence (highest first): 1. **Environment Variable Token**: Uses an access token from an environment variable - - `--token-env-var` specifies the environment variable name - - If not specified, defaults to `MST_TOKEN` + - `--token-env` specifies the environment variable name + - If `--token-env` is not specified, defaults to `MST_TOKEN` + - When `--token-env` is supplied explicitly, the named variable **must** contain a non-whitespace value or the command fails fast - This is the recommended approach for CI/CD environments -2. **Azure DefaultCredential**: Falls back to Azure DefaultCredential when no token is found +2. **Azure DefaultCredential** (opt-in via `--azure-auth`): Falls back to Azure DefaultCredential when no token is provided - Automatically uses available Azure credentials (managed identity, Azure CLI, etc.) - Ideal for local development and Azure-hosted environments +3. **Anonymous** (default): The client is constructed without credentials; no `Authorization` header is sent + - Appropriate for unauthenticated MST instances such as test ledgers + #### Authentication Examples ```bash @@ -1270,11 +1281,14 @@ CoseSignTool mst_register --endpoint https://your-cts-instance.azure.com --paylo # Using custom environment variable export MY_CUSTOM_TOKEN="your-access-token" -CoseSignTool mst_register --endpoint https://your-mst-instance.azure.com --payload file.txt --signature file.cose --token-env-var MY_CUSTOM_TOKEN +CoseSignTool mst_register --endpoint https://your-mst-instance.azure.com --payload file.txt --signature file.cose --token-env MY_CUSTOM_TOKEN -# Using Azure DefaultCredential (no token environment variable set) +# Anonymous (unauthenticated test ledger; default behavior) +CoseSignTool mst_register --endpoint https://aasmsttest.confidential-ledger.azure.com --payload file.txt --signature file.cose + +# Using Azure DefaultCredential (opt-in) # Requires Azure CLI login, managed identity, or other Azure credential -CoseSignTool mst_register --endpoint https://your-cts-instance.azure.com --payload file.txt --signature file.cose +CoseSignTool mst_register --endpoint https://your-cts-instance.azure.com --payload file.txt --signature file.cose --azure-auth ``` ### Certificate Provider Plugin: Azure Artifact Signing