Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions CoseIndirectSignature.Tests/IndirectSignatureFactoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -440,4 +440,66 @@ public void TestCreateIndirectSignatureAlreadyProvided()
IndirectSignature.TryGetPayloadHashAlgorithm(out CoseHashAlgorithm? algo).Should().BeTrue();
algo!.Should().Be(CoseHashAlgorithm.SHA256);
}

[Test]
public async Task TestCreateIndirectSignatureAsyncWithPayloadLocation()
{
ICoseSigningKeyProvider coseSigningKeyProvider = TestUtils.SetupMockSigningKeyProvider();
using IndirectSignatureFactory factory = new();
byte[] randomBytes = new byte[50];
new Random().NextBytes(randomBytes);
string testPayloadLocation = "https://example.com/payload/test.bin";

using MemoryStream memStream = new(randomBytes);
CoseSign1Message indirectSignature = await factory.CreateIndirectSignatureAsync(
memStream,
coseSigningKeyProvider,
"application/test.payload",
IndirectSignatureFactory.IndirectSignatureVersion.CoseHashEnvelope,
payloadLocation: testPayloadLocation);

// Verify it's an indirect signature
indirectSignature.IsIndirectSignature().Should().BeTrue();
indirectSignature.SignatureMatches(randomBytes).Should().BeTrue();

// Verify PayloadLocation header is set correctly
indirectSignature.TryGetPayloadLocation(out string? actualLocation).Should().BeTrue();
actualLocation.Should().Be(testPayloadLocation);

// Verify other headers are also correct
indirectSignature.TryGetPreImageContentType(out string? payloadType).Should().Be(true);
payloadType!.Should().Be("application/test.payload");
indirectSignature.TryGetPayloadHashAlgorithm(out CoseHashAlgorithm? algo).Should().BeTrue();
algo!.Should().Be(CoseHashAlgorithm.SHA256);
}

[Test]
public async Task TestCreateIndirectSignatureAsyncWithoutPayloadLocation()
{
ICoseSigningKeyProvider coseSigningKeyProvider = TestUtils.SetupMockSigningKeyProvider();
using IndirectSignatureFactory factory = new();
byte[] randomBytes = new byte[50];
new Random().NextBytes(randomBytes);

using MemoryStream memStream = new(randomBytes);
CoseSign1Message indirectSignature = await factory.CreateIndirectSignatureAsync(
memStream,
coseSigningKeyProvider,
"application/test.payload",
IndirectSignatureFactory.IndirectSignatureVersion.CoseHashEnvelope);

// Verify it's an indirect signature
indirectSignature.IsIndirectSignature().Should().BeTrue();
indirectSignature.SignatureMatches(randomBytes).Should().BeTrue();

// Verify PayloadLocation header is NOT set
indirectSignature.TryGetPayloadLocation(out string? actualLocation).Should().BeFalse();
actualLocation.Should().BeNull();

// Verify other headers are correct
indirectSignature.TryGetPreImageContentType(out string? payloadType).Should().Be(true);
payloadType!.Should().Be("application/test.payload");
indirectSignature.TryGetPayloadHashAlgorithm(out CoseHashAlgorithm? algo).Should().BeTrue();
algo!.Should().Be(CoseHashAlgorithm.SHA256);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public sealed partial class IndirectSignatureFactory
/// <param name="bytePayload">If streamPayload is null then this must be specified and must not be null and will use the Byte API's on the CoseSign1MesssageFactory</param>
/// <param name="payloadHashed">True if the payload represents the raw hash</param>
/// <param name="headerExtender">Optional header extender to add custom headers to the COSE message.</param>
/// <param name="payloadLocation">Optional URI indicating where the payload can be retrieved from.</param>
/// <returns>Either a CoseSign1Message or a ReadOnlyMemory{byte} representing the CoseSign1Message object.</returns>
/// <exception cref="ArgumentNullException">The contentType parameter was empty or null</exception>
/// <exception cref="ArgumentNullException">Either streamPayload or bytePayload must be specified, but not both at the same time, or both cannot be null</exception>
Expand All @@ -30,7 +31,8 @@ private object CreateIndirectSignatureWithChecksInternalCoseHashEnvelopeFormat(
Stream? streamPayload = null,
ReadOnlyMemory<byte>? bytePayload = null,
bool payloadHashed = false,
ICoseHeaderExtender? headerExtender = null)
ICoseHeaderExtender? headerExtender = null,
string? payloadLocation = null)
{
ReadOnlyMemory<byte> hash;
HashAlgorithmName algoName = InternalHashAlgorithmName;
Expand All @@ -57,8 +59,8 @@ private object CreateIndirectSignatureWithChecksInternalCoseHashEnvelopeFormat(
}

ICoseHeaderExtender effectiveHeaderExtender = headerExtender == null ?
new CoseHashEnvelopeHeaderExtender(algoName, contentType, null) :
new CoseSign1.Headers.ChainedCoseHeaderExtender(new[] { headerExtender, new CoseHashEnvelopeHeaderExtender(algoName, contentType, null) });
new CoseHashEnvelopeHeaderExtender(algoName, contentType, payloadLocation) :
new CoseSign1.Headers.ChainedCoseHeaderExtender(new[] { headerExtender, new CoseHashEnvelopeHeaderExtender(algoName, contentType, payloadLocation) });

return returnBytes
? InternalMessageFactory.CreateCoseSign1MessageBytes(
Expand All @@ -84,7 +86,8 @@ private async Task<object> CreateIndirectSignatureWithChecksInternalCoseHashEnve
ReadOnlyMemory<byte>? bytePayload = null,
bool payloadHashed = false,
ICoseHeaderExtender? headerExtender = null,
CancellationToken cancellationToken = default)
CancellationToken cancellationToken = default,
string? payloadLocation = null)
{
// Check for cancellation before starting expensive hash computation
cancellationToken.ThrowIfCancellationRequested();
Expand Down Expand Up @@ -117,8 +120,8 @@ private async Task<object> CreateIndirectSignatureWithChecksInternalCoseHashEnve
}

ICoseHeaderExtender effectiveHeaderExtender = headerExtender == null ?
new CoseHashEnvelopeHeaderExtender(algoName, contentType, null) :
new CoseSign1.Headers.ChainedCoseHeaderExtender(new[] { headerExtender, new CoseHashEnvelopeHeaderExtender(algoName, contentType, null) });
new CoseHashEnvelopeHeaderExtender(algoName, contentType, payloadLocation) :
new CoseSign1.Headers.ChainedCoseHeaderExtender(new[] { headerExtender, new CoseHashEnvelopeHeaderExtender(algoName, contentType, payloadLocation) });

return returnBytes
? await InternalMessageFactory.CreateCoseSign1MessageBytesAsync(
Expand Down
7 changes: 5 additions & 2 deletions CoseIndirectSignature/IndirectSignatureFactory.Stream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ public CoseSign1Message CreateIndirectSignatureFromHash(
/// <param name="signatureVersion">The <see cref="IndirectSignatureVersion"/> this factory should create.</param>
/// <param name="coseHeaderExtender">Optional header extender to add custom headers to the COSE message.</param>
/// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
/// <param name="payloadLocation">Optional URI indicating where the payload can be retrieved from. Only applicable for CoseHashEnvelope format.</param>
/// <returns>A Task which can be awaited which will return a CoseSign1Message which can be used as a Indirect signature validation of the payload.</returns>
/// <exception cref="ArgumentNullException">The contentType parameter was empty or null</exception>
public async Task<CoseSign1Message> CreateIndirectSignatureAsync(
Expand All @@ -193,15 +194,17 @@ public async Task<CoseSign1Message> CreateIndirectSignatureAsync(
string contentType,
IndirectSignatureVersion signatureVersion,
ICoseHeaderExtender? coseHeaderExtender = null,
CancellationToken cancellationToken = default) =>
CancellationToken cancellationToken = default,
string? payloadLocation = null) =>
(CoseSign1Message)await CreateIndirectSignatureWithChecksInternalAsync(
returnBytes: false,
signingKeyProvider: signingKeyProvider,
contentType: contentType,
streamPayload: payload,
signatureVersion: signatureVersion,
headerExtender: coseHeaderExtender,
cancellationToken: cancellationToken).ConfigureAwait(false);
cancellationToken: cancellationToken,
payloadLocation: payloadLocation).ConfigureAwait(false);

/// <summary>
/// Creates a Indirect signature of the payload given a hash of the payload returned as a <see cref="CoseSign1Message"/> following the rules in this class description.
Expand Down
14 changes: 10 additions & 4 deletions CoseIndirectSignature/IndirectSignatureFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ private CoseHashAlgorithm GetCoseHashAlgorithmFromHashAlgorithm(HashAlgorithm al
/// <param name="payloadHashed">True if the payload represents the raw hash</param>
/// <param name="signatureVersion">The <see cref="IndirectSignatureVersion"/> this factory should create.</param>
/// <param name="headerExtender">An optional <see cref="ICoseHeaderExtender"/> to extend the protected headers of the CoseSign1Message.</param>
/// <param name="payloadLocation">Optional URI indicating where the payload can be retrieved from. Only applicable for CoseHashEnvelope format.</param>
/// <returns>Either a CoseSign1Message or a ReadOnlyMemory{byte} representing the CoseSign1Message object.</returns>
/// <exception cref="ArgumentNullException">The contentType parameter was empty or null</exception>
/// <exception cref="ArgumentNullException">Either streamPayload or bytePayload must be specified, but not both at the same time, or both cannot be null</exception>
Expand All @@ -121,7 +122,8 @@ private object CreateIndirectSignatureWithChecksInternal(
Stream? streamPayload = null,
ReadOnlyMemory<byte>? bytePayload = null,
bool payloadHashed = false,
ICoseHeaderExtender? headerExtender = null)
ICoseHeaderExtender? headerExtender = null,
string? payloadLocation = null)
{
if (string.IsNullOrWhiteSpace(contentType))
{
Expand Down Expand Up @@ -166,7 +168,8 @@ private object CreateIndirectSignatureWithChecksInternal(
streamPayload,
bytePayload,
payloadHashed,
headerExtender);
headerExtender,
payloadLocation);
default:
throw new ArgumentOutOfRangeException(nameof(signatureVersion), "Unknown signature version");
}
Expand All @@ -184,6 +187,7 @@ private object CreateIndirectSignatureWithChecksInternal(
/// <param name="signatureVersion">The <see cref="IndirectSignatureVersion"/> this factory should create.</param>
/// <param name="headerExtender">An optional <see cref="ICoseHeaderExtender"/> to extend the protected headers of the CoseSign1Message.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <param name="payloadLocation">Optional URI indicating where the payload can be retrieved from. Only applicable for CoseHashEnvelope format.</param>
/// <returns>Either a CoseSign1Message or a ReadOnlyMemory{byte} representing the CoseSign1Message object.</returns>
/// <exception cref="ArgumentNullException">The contentType parameter was empty or null</exception>
/// <exception cref="ArgumentNullException">Either streamPayload or bytePayload must be specified, but not both at the same time, or both cannot be null</exception>
Expand All @@ -197,7 +201,8 @@ private async Task<object> CreateIndirectSignatureWithChecksInternalAsync(
ReadOnlyMemory<byte>? bytePayload = null,
bool payloadHashed = false,
ICoseHeaderExtender? headerExtender = null,
CancellationToken cancellationToken = default)
CancellationToken cancellationToken = default,
string? payloadLocation = null)
{
if (string.IsNullOrWhiteSpace(contentType))
{
Expand Down Expand Up @@ -245,7 +250,8 @@ private async Task<object> CreateIndirectSignatureWithChecksInternalAsync(
bytePayload,
payloadHashed,
headerExtender,
cancellationToken).ConfigureAwait(false);
cancellationToken,
payloadLocation).ConfigureAwait(false);
default:
throw new ArgumentOutOfRangeException(nameof(signatureVersion), "Unknown signature version");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1019,5 +1019,99 @@
// Assert
Assert.AreEqual(PluginExitCode.InvalidArgumentValue, result);
}

[TestMethod]
public void IndirectSignCommand_Options_ShouldContainPayloadLocationOption()
{
// Arrange
IndirectSignCommand command = new IndirectSignCommand();

// Act
IDictionary<string, string> options = command.Options;

// Assert
Assert.IsTrue(options.ContainsKey("payload-location"), "Options should contain payload-location");

Check notice

Code scanning / CodeQL

Inefficient use of ContainsKey Note test

Inefficient use of 'ContainsKey' and
indexer
.

Copilot Autofix

AI about 2 months ago

In general, to fix this pattern, replace dictionary.ContainsKey(key) followed by dictionary[key] with a single dictionary.TryGetValue(key, out var value) and use value for subsequent checks. This consolidates two lookups into one and avoids a potential KeyNotFoundException.

In this specific test, we should replace the assertion that checks for the key with a TryGetValue call that also retrieves the description. Then we can assert that the retrieved description contains "URI". Concretely, in IndirectSignCommand_Options_ShouldContainPayloadLocationOption in CoseSignTool.IndirectSignature.Plugin.Tests/IndirectSignCommandTests.cs, replace the two lines:

Assert.IsTrue(options.ContainsKey("payload-location"), "Options should contain payload-location");
Assert.IsTrue(options["payload-location"].Contains("URI"), "payload-location description should mention URI");

with code that first asserts TryGetValue succeeds, captures the option description into a local variable, and then checks that this string contains "URI". No new imports or helper methods are needed; we only use existing IDictionary<string,string> APIs and MSTest assertions.

Suggested changeset 1
CoseSignTool.IndirectSignature.Plugin.Tests/IndirectSignCommandTests.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/CoseSignTool.IndirectSignature.Plugin.Tests/IndirectSignCommandTests.cs b/CoseSignTool.IndirectSignature.Plugin.Tests/IndirectSignCommandTests.cs
--- a/CoseSignTool.IndirectSignature.Plugin.Tests/IndirectSignCommandTests.cs
+++ b/CoseSignTool.IndirectSignature.Plugin.Tests/IndirectSignCommandTests.cs
@@ -1030,8 +1030,8 @@
         IDictionary<string, string> options = command.Options;
 
         // Assert
-        Assert.IsTrue(options.ContainsKey("payload-location"), "Options should contain payload-location");
-        Assert.IsTrue(options["payload-location"].Contains("URI"), "payload-location description should mention URI");
+        Assert.IsTrue(options.TryGetValue("payload-location", out string payloadLocationDescription), "Options should contain payload-location");
+        Assert.IsTrue(payloadLocationDescription.Contains("URI"), "payload-location description should mention URI");
     }
 
     [TestMethod]
EOF
@@ -1030,8 +1030,8 @@
IDictionary<string, string> options = command.Options;

// Assert
Assert.IsTrue(options.ContainsKey("payload-location"), "Options should contain payload-location");
Assert.IsTrue(options["payload-location"].Contains("URI"), "payload-location description should mention URI");
Assert.IsTrue(options.TryGetValue("payload-location", out string payloadLocationDescription), "Options should contain payload-location");
Assert.IsTrue(payloadLocationDescription.Contains("URI"), "payload-location description should mention URI");
}

[TestMethod]
Copilot is powered by AI and may make mistakes. Always verify output.
Assert.IsTrue(options["payload-location"].Contains("URI"), "payload-location description should mention URI");
}

[TestMethod]
public async Task IndirectSignCommand_Execute_WithPayloadLocation_ShouldIncludePayloadLocationHeader()
{
// Arrange
IndirectSignCommand command = new IndirectSignCommand();
string signaturePath = TestSignaturePath + "_payload_location";
string testPayloadLocation = "https://example.com/payload/test.bin";
IConfiguration configuration = CreateConfiguration(new Dictionary<string, string?>
{
["payload"] = TestPayloadPath,
["signature"] = signaturePath,
["pfx"] = TestCertificatePath,
["payload-location"] = testPayloadLocation
});

try
{
// Act
PluginExitCode result = await command.ExecuteAsync(configuration);

// Assert
Assert.AreEqual(PluginExitCode.Success, result);
Assert.IsTrue(File.Exists(signaturePath));

// Verify the signature contains the PayloadLocation header (label 260 per RFC 9054)
byte[] signatureBytes = File.ReadAllBytes(signaturePath);
CoseSign1Message message = CoseMessage.DecodeSign1(signatureBytes);
Assert.IsNotNull(message);
Assert.IsTrue(message.IsIndirectSignature());

// PayloadLocation is COSE header label 260
bool hasPayloadLocation = message.ProtectedHeaders.TryGetValue(new CoseHeaderLabel(260), out CoseHeaderValue payloadLocationValue);
Assert.IsTrue(hasPayloadLocation, "Signature should contain PayloadLocation header (label 260)");
string? actualLocation = payloadLocationValue.GetValueAsString();
Assert.AreEqual(testPayloadLocation, actualLocation, "PayloadLocation header value should match the specified URI");
}
finally
{
SafeDeleteFile(signaturePath);
}
}

[TestMethod]
public async Task IndirectSignCommand_Execute_WithoutPayloadLocation_ShouldNotIncludePayloadLocationHeader()
{
// Arrange
IndirectSignCommand command = new IndirectSignCommand();
string signaturePath = TestSignaturePath + "_no_payload_location";
IConfiguration configuration = CreateConfiguration(new Dictionary<string, string?>
{
["payload"] = TestPayloadPath,
["signature"] = signaturePath,
["pfx"] = TestCertificatePath
});

try
{
// Act
PluginExitCode result = await command.ExecuteAsync(configuration);

// Assert
Assert.AreEqual(PluginExitCode.Success, result);
Assert.IsTrue(File.Exists(signaturePath));

// Verify the signature does NOT contain the PayloadLocation header (label 260)
byte[] signatureBytes = File.ReadAllBytes(signaturePath);
CoseSign1Message message = CoseMessage.DecodeSign1(signatureBytes);
Assert.IsNotNull(message);
Assert.IsTrue(message.IsIndirectSignature());

// PayloadLocation is COSE header label 260
bool hasPayloadLocation = message.ProtectedHeaders.TryGetValue(new CoseHeaderLabel(260), out _);
Assert.IsFalse(hasPayloadLocation, "Signature should NOT contain PayloadLocation header when not specified");
}
finally
{
SafeDeleteFile(signaturePath);
}
}
}

16 changes: 15 additions & 1 deletion CoseSignTool.IndirectSignature.Plugin/IndirectSignCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ public override IDictionary<string, string> Options
options["cwt-subject"] = "The CWT subject (sub) claim. Defaults to 'UnknownIntent'";
options["cwt-audience"] = "The CWT audience (aud) claim (optional)";

// Add payload location option for CoseHashEnvelope format
options["payload-location"] = "A URI indicating where the payload can be retrieved from (optional, CoseHashEnvelope format only)";

return options;
}
}
Expand Down Expand Up @@ -150,6 +153,13 @@ public override async Task<PluginExitCode> ExecuteAsync(IConfiguration configura
Logger.LogVerbose($"Custom CWT claims count: {cwtClaims.Count}");
}

// Parse payload location for CoseHashEnvelope format
string? payloadLocation = GetOptionalValue(configuration, "payload-location");
if (!string.IsNullOrEmpty(payloadLocation))
{
Logger.LogVerbose($"Payload location: {payloadLocation}");
}

// Create the indirect signature
using CancellationTokenSource combinedCts = CreateTimeoutCancellationToken(timeoutSeconds, cancellationToken);
Logger.LogVerbose($"Creating indirect signature with hash algorithm: {hashAlgorithm.Name}, version: {signatureVersion}");
Expand All @@ -167,6 +177,7 @@ public override async Task<PluginExitCode> ExecuteAsync(IConfiguration configura
cwtSubject,
cwtAudience,
cwtClaims,
payloadLocation,
Logger,
combinedCts.Token);

Expand Down Expand Up @@ -210,6 +221,7 @@ public override async Task<PluginExitCode> ExecuteAsync(IConfiguration configura
/// <param name="cwtSubject">Optional CWT subject claim.</param>
/// <param name="cwtAudience">Optional CWT audience claim.</param>
/// <param name="cwtClaims">Optional custom CWT claims as label:value pairs.</param>
/// <param name="payloadLocation">Optional URI indicating where the payload can be retrieved from.</param>
/// <param name="logger">Logger for diagnostic output.</param>
/// <param name="cancellationToken">Cancellation token with timeout.</param>
/// <returns>Tuple containing the exit code and optional result object for JSON output.</returns>
Expand All @@ -227,6 +239,7 @@ public override async Task<PluginExitCode> ExecuteAsync(IConfiguration configura
string? cwtSubject,
string? cwtAudience,
List<string>? cwtClaims,
string? payloadLocation,
IPluginLogger logger,
CancellationToken cancellationToken)
{
Expand Down Expand Up @@ -318,7 +331,8 @@ public override async Task<PluginExitCode> ExecuteAsync(IConfiguration configura
contentType: contentType,
signatureVersion: signatureVersion,
coseHeaderExtender: headerExtender,
cancellationToken: cancellationToken);
cancellationToken: cancellationToken,
payloadLocation: payloadLocation);

// Encode and write signature to file
byte[] signatureBytes = indirectSignature.Encode();
Expand Down
Loading
Loading