Skip to content

Commit

Permalink
indirect sigs need payload
Browse files Browse the repository at this point in the history
  • Loading branch information
elantiguamsft committed Feb 14, 2024
1 parent 8d7f05c commit 57fc645
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 8 deletions.
14 changes: 8 additions & 6 deletions CoseHandler/CoseHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -602,13 +602,11 @@ internal static ValidationResult ValidateInternal(
bool hasBytes = !payloadBytes.IsNullOrEmpty();

Check warning

Code scanning / CodeQL

Dereferenced variable may be null Warning

Variable
payloadBytes
may be null at this access because of
this
null argument.
Variable
payloadBytes
may be null at this access because of
this
null argument.
Variable
payloadBytes
may be null at this access because of
this
null argument.
bool hasStream = payloadStream is not null;

// Determine the type of content validation to perform.
// Check for an indirect signature, where the content header contains the hash of the payload, and the algorithm is stored in the message.
// If this is the case and external content is provided, we can validate an external payload hash against the hash stored in the cose message content.
bool indirectSignature = msg.IsDetachedSignature();

// Determine the type of content validation to perform.
ContentValidationType cvt = (indirectSignature && (hasBytes || hasStream)) ? ContentValidationType.Indirect :
hasBytes || hasStream ? ContentValidationType.Detached : ContentValidationType.Embedded;
ContentValidationType cvt = msg.IsDetachedSignature() ? ContentValidationType.Indirect :
(hasBytes || hasStream) ? ContentValidationType.Detached : ContentValidationType.Embedded;

try
{
Expand All @@ -618,7 +616,9 @@ internal static ValidationResult ValidateInternal(
case ContentValidationType.Indirect:
messageVerified = hasBytes ?
(msg.VerifyEmbedded(publicKey) && msg.SignatureMatches(payloadBytes)) :
(msg.VerifyEmbedded(publicKey) && msg.SignatureMatches(payloadStream));
hasStream ?
(msg.VerifyEmbedded(publicKey) && msg.SignatureMatches(payloadStream)) :
throw new InvalidOperationException();
break;

// Detached signature validation. Validate external payload against the signature.
Expand Down Expand Up @@ -650,6 +650,8 @@ internal static ValidationResult ValidateInternal(
errorCodes.Add(
payloadBytes is null && payloadStream is null ? ValidationFailureCode.PayloadMissing :
ValidationFailureCode.RedundantPayload);

messageVerified = false;
}

return new ValidationResult(messageVerified, errorCodes, certValidationResults, chain, cvt);
Expand Down
2 changes: 1 addition & 1 deletion CoseHandler/CoseValidationError.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public CoseValidationError(ValidationFailureCode errorCode)
{ ValidationFailureCode.CertificateChainInvalid, "Certificate chain validation failed." },
{ ValidationFailureCode.TrustValidationFailed, "The signature failed to validate against the trust validator." },
{ ValidationFailureCode.PayloadMismatch, "The supplied or embedded payload does not match the hash of the payload that was signed." },
{ ValidationFailureCode.PayloadMissing, "The detached signature could not be validated because the original payload was not supplied."},
{ ValidationFailureCode.PayloadMissing, "The detached or indirect signature could not be validated because the external payload was not supplied."},
{ ValidationFailureCode.PayloadUnreadable, "The payload content could not be read."},
{ ValidationFailureCode.RedundantPayload, "The embedded signature was not validated because external payload was also specified."},
{ ValidationFailureCode.CoseHeadersInvalid, "The COSE headers in the signature could not be read." },
Expand Down
2 changes: 1 addition & 1 deletion CoseHandler/ValidationFailureCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public enum ValidationFailureCode
PayloadUnreadable,

/// <summary>
/// Required payload was not supplied for detached signature.
/// Required payload was not supplied for a detached or indirect signature.
/// </summary>
PayloadMissing,

Expand Down
33 changes: 33 additions & 0 deletions CoseSignTool.tests/ValidateCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
namespace CoseSignUnitTests;

using System;
using System.Linq;
using CoseDetachedSignature;
using CoseSign1.Certificates.Local;
using CoseX509;
Expand Down Expand Up @@ -187,6 +188,38 @@ public void ValidateIndirectSucceedsWithRootPassedIn()
result.ToString(true).Should().Contain("Indirect");
}

/// <summary>
/// Validates that indirect signature validation faills when no payload is passed in
/// </summary>
[TestMethod]
public void ValidateIndirectFailsWithoutPayloadPassedIn()
{
// sign indirectly
var msgFac = new DetachedSignatureFactory();
byte[] signedBytes = msgFac.CreateDetachedSignatureBytes(
payload: File.ReadAllBytes(PayloadFile),
contentType: "application/spdx+json",
signingKeyProvider: new X509Certificate2CoseSigningKeyProvider(SelfSignedCert)).ToArray();

using FileStream coseFile = new(PayloadFile + ".cose", FileMode.Create);
coseFile.Write(signedBytes);
coseFile.Seek(0, SeekOrigin.Begin);

// setup validator
var validator = new ValidateCommand();
var result = validator.RunCoseHandlerCommand(coseFile,
null,
new System.Collections.Generic.List<X509Certificate2> { SelfSignedCert },
X509RevocationMode.Online,
null,
false);
result.Success.Should().BeFalse();
result.ContentValidationType.Should().Be(ContentValidationType.Indirect);
result.ToString(true).Should().Contain("Indirect");
result.Errors.Should().ContainSingle();
result.Errors.FirstOrDefault().ErrorCode.Should().Be(ValidationFailureCode.PayloadMissing);
}

/// <summary>
/// Validates that modified indirect payloads are rejected
/// </summary>
Expand Down

0 comments on commit 57fc645

Please sign in to comment.