[API Proposal]: Add APIs for COSE signing #69896

Jozkee · 2022-05-27T14:38:34Z

Background and motivation

In order to comply with the executive order on supply chain security, that includes inventory management (SCIM) and bill of materials (SBOM), .NET needs to implement APIs for signing with COSE (CBOR Object Signing and Encryption).
This proposal address above requirement by adding a new OOB package compatible with netstandard2.0 that contains APIs to work with COSE_Sign1 and COSE_Sign formats.

API Proposal

[assembly: System.Runtime.Versioning.UnsupportedOSPlatform("browser")]

namespace System.Security.Cryptography.Cose
{
    public abstract class CoseMessage
    {
        internal CoseMessage() { }
        public ReadOnlyMemory<byte>? Content { get { throw null; } } // "payload" for COSE_Sign* and COSE_Mac*, or "ciphertext" for COSE_Encrypt*
        public CoseHeaderMap ProtectedHeaders { get { throw null; } }
        public CoseHeaderMap UnprotectedHeaders { get { throw null; } }
        public static CoseSign1Message DecodeSign1(byte[] cborPayload) { throw null; }
        public static CoseSign1Message DecodeSign1(ReadOnlySpan<byte> cborPayload) { throw null; }
        public static CoseMultiSignMessage DecodeMultiSign(byte[] cborPayload) { throw null; }
        public static CoseMultiSignMessage DecodeMultiSign(ReadOnlySpan<byte> cborPayload) { throw null; }
        public byte[] Encode() { throw null; }
        public int Encode(Span<byte> destination) { throw null; }
        public abstract bool TryEncode(Span<byte> destination, out int bytesWritten);
        public abstract int GetEncodedLength();
    }

    public sealed class CoseSign1Message : CoseMessage
    {
        internal CoseSign1Message() { }
        public static byte[] SignDetached(byte[] detachedContent, CoseSigner signer, byte[]? associatedData = null) { throw null; }
        public static byte[] SignDetached(ReadOnlySpan<byte> detachedContent, CoseSigner signer, ReadOnlySpan<byte> associatedData = default) { throw null; }
        public static byte[] SignDetached(Stream detachedContent, CoseSigner signer, ReadOnlySpan<byte> associatedData = default) { throw null; }
        public static Task<byte[]> SignDetachedAsync(Stream detachedContent, CoseSigner signer, ReadOnlyMemory<byte> associatedData = default, CancellationToken cancellationToken = default(CancellationToken)) { throw null; }
        
        public static byte[] SignEmbedded(byte[] embeddedContent, CoseSigner signer, byte[]? associatedData = null) { throw null; }
        public static byte[] SignEmbedded(ReadOnlySpan<byte> embeddedContent, CoseSigner signer, ReadOnlySpan<byte> associatedData = default) { throw null; }
        
        public static bool TrySignEmbedded(ReadOnlySpan<byte> embeddedContent, Span<byte> destination, CoseSigner signer, out int bytesWritten, ReadOnlySpan<byte> associatedData = default) { throw null; }
        public static bool TrySignDetached(ReadOnlySpan<byte> detachedContent, Span<byte> destination, CoseSigner signer, out int bytesWritten, ReadOnlySpan<byte> associatedData = default) { throw null; }
        
        public bool VerifyDetached(AsymmetricAlgorithm key, byte[] detachedContent, byte[]? associatedData = null) { throw null; }
        public bool VerifyDetached(AsymmetricAlgorithm key, ReadOnlySpan<byte> detachedContent, ReadOnlySpan<byte> associatedData = default) { throw null; }
        public bool VerifyDetached(AsymmetricAlgorithm key, Stream detachedContent, ReadOnlySpan<byte> associatedData = default) { throw null; }
        public Task<bool> VerifyDetachedAsync(AsymmetricAlgorithm key, Stream detachedContent, ReadOnlyMemory<byte> associatedData = default, CancellationToken cancellationToken = default(CancellationToken)) { throw null; }
        public bool VerifyEmbedded(AsymmetricAlgorithm key, byte[]? associatedData = null) { throw null; }
        public bool VerifyEmbedded(AsymmetricAlgorithm key, ReadOnlySpan<byte> associatedData = default) { throw null; }
        
        public override bool TryEncode(Span<byte> destination, out int bytesWritten) { throw null; }
        public override int GetEncodedLength() { throw null; }
    }

    public sealed class CoseMultiSignMessage : CoseMessage
    {
        internal CoseMultiSignMessage() { }
        public static byte[] SignDetached(byte[] detachedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, byte[]? associatedData = null) { throw null; }
        public static byte[] SignDetached(ReadOnlySpan<byte> detachedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlySpan<byte> associatedData = default) { throw null; }
        public static byte[] SignDetached(Stream detachedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlySpan<byte> associatedData = default) { throw null; }
        public static Task<byte[]> SignDetachedAsync(Stream detachedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlyMemory<byte> associatedData = default, CancellationToken cancellationToken = default(CancellationToken)) { throw null; }
        public static byte[] SignEmbedded(byte[] embeddedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, byte[]? associatedData = null) { throw null; }
        public static byte[] SignEmbedded(ReadOnlySpan<byte> embeddedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlySpan<byte> associatedData = default) { throw null; }
        
        public static bool TrySignEmbedded(ReadOnlySpan<byte> embeddedContent, CoseSigner signer, out int bytesWritten, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlySpan<byte> associatedData = default) { throw null; }
        public static bool TrySignDetached(ReadOnlySpan<byte> detachedContent, CoseSigner signer, out int bytesWritten, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlySpan<byte> associatedData = default) { throw null; }

        public ReadOnlyCollection<CoseSignature> Signatures { get; }
        public void AddSignature(CoseSigner signer, byte[]? associatedData = null) { throw null; }
        public void AddSignature(CoseSigner signer, ReadOnlySpan<byte> associatedData = default) { throw null; }
        public void RemoveSignature(CoseSignature signature) { throw null; }
        public void RemoveSignature(int index) { throw null; }

        public override bool TryEncode(Span<byte> destination, out int bytesWritten) { throw null; }
        public override int GetEncodedLength() { throw null; }
    }

    public sealed class CoseSignature
    {
        internal CoseSignature() { }
        public CoseHeaderMap ProtectedHeaders { get; }
        public CoseHeaderMap UnprotectedHeaders { get; }
        public bool VerifyEmbedded(AsymmetricAlgorithm key, byte[]? associatedData = null) { throw null; }
        public bool VerifyEmbedded(AsymmetricAlgorithm key, ReadOnlySpan<byte> associatedData = default) { throw null; }
        public bool VerifyDetached(AsymmetricAlgorithm key, byte[] detachedContent, byte[]? associatedData = null) { throw null; }
        public bool VerifyDetached(AsymmetricAlgorithm key, ReadOnlySpan<byte> detachedContent, ReadOnlySpan<byte> associatedData = default) { throw null; }
        public bool VerifyDetached(AsymmetricAlgorithm key, Stream detachedContent, ReadOnlySpan<byte> associatedData = default) { throw null; }
        public Task<bool> VerifyDetachedAsync(AsymmetricAlgorithm key, Stream detachedContent, ReadOnlyMemory<byte> associatedData = default) { throw null; }
    }
    
    public sealed class CoseSigner
    {
        // Throw for RSA keys as the user needs to be aware of the RSASignaturePadding.
        public CoseSigner(AsymmetricAlgorithm key, HashAlgorithmName hashAlgorithm, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null) { throw null; }
        public CoseSigner(RSA key, RSASignaturePadding signaturePadding, HashAlgorithmName hashAlgorithm, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null) { throw null; }
        public AsymmetricAlgorithm Key { get; }
        public HashAlgorithmName HashAlgorithm { get; }
        public CoseHeaderMap ProtectedHeaders { get; }
        public CoseHeaderMap UnprotectedHeaders { get; }
        public RSASignaturePadding? RSASignaturePadding{ get; }
    }
    
    public sealed partial class CoseHeaderMap : IDictionary<CoseHeaderLabel, CoseHeaderValue>
    {
        public CoseHeaderValue this[CoseHeaderLabel key] { get { throw null; } set { throw null; } }
        public ICollection<CoseHeaderLabel> Keys { get { throw null; } }
        public ICollection<CoseHeaderValue> Values { get { throw null; } }
        public int Count { get { throw null; } }
        public bool IsReadOnly { get { throw null; } }

        public void Add(CoseHeaderLabel key, CoseHeaderValue value) { throw null; }
        public void Add(KeyValuePair<CoseHeaderLabel, CoseHeaderValue> item) { throw null; }
        public void Clear() { throw null; }
        public bool Contains(KeyValuePair<CoseHeaderLabel, CoseHeaderValue> item) { throw null; }
        public bool ContainsKey(CoseHeaderLabel key) { throw null; }
        public void CopyTo(KeyValuePair<CoseHeaderLabel, CoseHeaderValue>[] array, int arrayIndex) { throw null; }
        public IEnumerator<KeyValuePair<CoseHeaderLabel, CoseHeaderValue>> GetEnumerator() { throw null; }
        public bool Remove(CoseHeaderLabel key) { throw null; }
        public bool Remove(KeyValuePair<CoseHeaderLabel, CoseHeaderValue> item) { throw null; }
        public bool TryGetValue(CoseHeaderLabel key, [MaybeNullWhen(false)] out CoseHeaderValue value) { throw null; }
        IEnumerator IEnumerable.GetEnumerator() { throw null; }

        // Accelerators
        public void Add(CoseHeaderLabel label, int value) { throw null; }
        public void Add(CoseHeaderLabel label, string value) { throw null; }
        public void Add(CoseHeaderLabel label, byte[] value) { throw null; }
        public void Add(CoseHeaderLabel label, ReadOnlySpan<byte> value) { throw null; }
        public int GetValueAsInt32(CoseHeaderLabel label) { throw null; }
        public string GetValueAsString(CoseHeaderLabel label) { throw null; }
        public byte[] GetValueAsBytes(CoseHeaderLabel label) { throw null; }
        public int GetValueAsBytes(CoseHeaderLabel label, Span<byte> destination) { throw null; }
    }

    public readonly struct CoseHeaderLabel : IEquatable<CoseHeaderLabel>
    {
        public CoseHeaderLabel(int label) { throw null; }
        public CoseHeaderLabel(string label) { throw null; }
        // Known headers from https://datatracker.ietf.org/doc/html/rfc8152#page-14 Table 2: Common Header Parameters, excluding IV and PartialIV used only for COSE_Encrypt.
        public static CoseHeaderLabel Algorithm { get { throw null; } }
        public static CoseHeaderLabel ContentType { get { throw null; } }
        public static CoseHeaderLabel CounterSignature { get { throw null; } }
        public static CoseHeaderLabel CriticalHeaders { get { throw null; } }
        public static CoseHeaderLabel KeyIdentifier { get { throw null; } }

        public override bool Equals([NotNullWhenAttribute(true)] object? obj) { throw null; }
        public bool Equals(CoseHeaderLabel other) { throw null; }
        public override int GetHashCode() { throw null; }
        public static bool operator ==(CoseHeaderLabel left, CoseHeaderLabel right) { throw null; }
        public static bool operator !=(CoseHeaderLabel left, CoseHeaderLabel right) { throw null; }
    }

    public readonly struct CoseHeaderValue : IEquatable<CoseHeaderValue>
    {
        public readonly ReadOnlyMemory<byte> EncodedValue { get; }
        public static CoseHeaderValue FromEncodedValue(byte[] encodedValue) { throw null; }
        public static CoseHeaderValue FromEncodedValue(ReadOnlySpan<byte> encodedValue) { throw null; }
        public static CoseHeaderValue FromInt32(int value) { throw null;  }
        public static CoseHeaderValue FromString(string value) { throw null; }
        public static CoseHeaderValue FromBytes(byte[] value) { throw null; }
        public static CoseHeaderValue FromBytes(ReadOnlySpan<byte> value) { throw null; }
        public int GetValueAsInt32() { throw null; }
        public string GetValueAsString() { throw null; }
        public byte[] GetValueAsBytes() { throw null; }
        public int GetValueAsBytes(Span<byte> destination) { throw null; }

        public override bool Equals([NotNullWhenAttribute(true)] object? obj) { throw null; }
        public bool Equals(CoseHeaderValue other) { throw null; }
        public override int GetHashCode() { throw null; }
        public static bool operator ==(CoseHeaderValue left, CoseHeaderValue right) { throw null; }
        public static bool operator !=(CoseHeaderValue left, CoseHeaderValue right) { throw null; }
    }
}

API Usage

Decoding and verifying messages

bool DecodeAndVerify_Sign1(byte[] encodedMsg, AsymmetricAlgorithm key, byte[]? signedContent)
{
    CoseSign1Message msg = CoseMessage.DecodeSign1(encodedMsg);

    // There are two scenarios for verification, 
    // embedded content (it was carried in the message and can be used for verification)
    // and detached content (it was not part of the message and a CBOR nil terminator was placed instead).
    if (msg.Content != null)
    {
        return msg.VerifyEmbedded(key);
    }
    else
    {
        Debug.Assert(signedContent != null);
        return msg.VerifyDetached(key, signedContent);
    }
}

bool DecodeAndVerify_MultiSign(byte[] encodedMsg, AsymmetricAlgorithm key, byte[]? signedContent)
{
    CoseMultiSignMessage msg = CoseMessage.DecodeMultiSign(encodedMsg);
    ReadOnlyCollection<CoseSignature> signatures = msg.Signatures;

    foreach (CoseSignature s in signatures)
    {
        bool verified = msg.Content != null ? s.VerifyEmbedded(key) : s.VerifyDetached(key, signedContent);
        if (verified)
        {
            // Consider a valid verification if at least one signature verifies.
            return true;
        }
    }
    
    return false;
}

Signing and encoding messages

byte[] Sign_WithSign1(byte[] content, AsymmetricAlgorithm key, HashAlgorithmName hash)
{
    var signer = new CoseSigner(key, hash);
    return CoseSign1Message.SignEmbedded(content, signer);
}

byte[] Sign_WithMultiSign(byte[] content, AsymmetricAlgorithm key, HashAlgorithmName hash)
{
    var signer = new CoseSigner(key, hash);
    return CoseMultiSignMessage.SignEmbedded(content, signer);
}

Append unprotected headers or signatures to already signed messages.

byte[] AddUnprotectedHeader(byte[] encodedMsg)
{
    CoseSign1Message msg = CoseMessage.DecodeSign1(encodedMsg);
    msg.UnprotectedHeaders.Add(CoseHeaderLabel.KeyIdentifier, Encoding.UTF8.GetBytes("11"));
    
    // re-encode
    return msg.Encode();
}

byte[] AddSignature(byte[] encodedMsg, Asymmetricalgorithm key, HashAlgorithmName hash)
{
    CoseMultiSignMessage msg = CoseMessage.DecodeMultiSign(encodedMsg);
    msg.AddSignature(new CoseSigner(key, hash));

    return msg.Encode();
}

Use custom headers

byte[] Sign_WithCustomHeaders(byte[] content, AsymmetricAlgorithm key, HashAlgorithmName hash)
{
    var protectedHeaders = new CoseHeaderMap();
    var unprotectedHeaders = new coseHeaderMap();

    // Key identifier values are hints about which key to use. 
    unprotectedHeaders.Add(CoseHeaderLabel.KeyIdentifier, Encoding.UTF8.GetBytes("11"));

    // This parameter is used to indicate which protected header labels an application that is processing a message is required to understand. 
    var writer = new CborWriter();
    writer.WriteStartArray(definiteLength: 1);
    writer.WriteString("42");
    writer.WriteEndArray();
    
    protectedHeaders.Add(CoseHeaderLabel.CriticalHeaders, CoseHeaderValue.FromEncodedValue(writer.Encode()));
    protectedHeaders.Add(new CoseHeaderLabel("42"), "this is my custom critical header.");

    return CoseSign1Message.SignEmbedded(content, key, hash, protectedHeaders, unprotectedHeaders);
}

Supply content via a stream in order to sign large contents.

Task<byte[]> Sign_WithStreamContent(AsymmetricAlgorithm key, HashAlgorithmName hash, Cancellationtoken ct)
{
    Stream contentStream = File.Open("ubuntu-22.04-desktop-amd64.iso", FileMode.Open);
    return CoseSign1Message.SignDetachedAsync(contentStream, key, hash, ct);
}

Sign and Verify with external_aad (Externally supplied data).

byte[] Sign_WithSign1AndAssociatedData(byte[] content, AsymmetricAlgorithm key, HashAlgorithmName hash, byte[]? associatedData)
{
    var signer = new CoseSigner(key, hash);
    return CoseSign1Message.SignEmbedded(content, signer, associatedData);
}

byte[] DecodeAndVerify_WithSign1AndAssociatedData(byte[] content, AsymmetricAlgorithm key, byte[]? signedContent, byte[]? associatedData)
{
    CoseSign1Message msg = CoseMessage.DecodeSign1(encodedMsg);

    if (msg.Content != null)
    {
        return msg.VerifyEmbedded(key, associatedData);
    }
    else
    {
        Debug.Assert(signedContent != null);
        return msg.VerifyDetached(key, signedContent, associatedData);
    }
}

Alternative Designs

Risks

The COSE specification contains other formats, such as COSE_Encrypt and COSE_Mac, that were considered while writing this proposal but that haven't been fully explored, there is a small chance that we could have confilcting APIs if in the future .NET chooses to support those formats as well.

ghost · 2022-05-27T14:38:39Z

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

In order to comply with the executive order on supply chain security, that includes inventory management (SCIM) and bill of materials (SBOM), .NET needs to implement APIs for signing with COSE (CBOR Object Signing and Encryption).
This proposal address above requirement by adding a new OOB package compatible with netstandard2.0 that contains APIs to work with COSE_Sign1 and COSE_Sign formats.

API Proposal

namespace System.Security.Cryptography.Cose
{
    public abstract partial class CoseMessage
    {
        internal CoseMessage() { }
        public ReadOnlyMemory<byte>? Content { get { throw null; } } // "payload" for COSE_Sign* and COSE_Mac*, or "ciphertext" for COSE_Encrypt*
        public CoseHeaderMap ProtectedHeaders { get { throw null; } }
        public CoseHeaderMap UnprotectedHeaders { get { throw null; } }
        public static CoseSign1Message DecodeSign1(byte[] cborPayload) { throw null; }
        public static CoseSign1Message DecodeSign1(ReadOnlySpan<byte> cborPayload) { throw null; }
        public static CoseMultiSignMessage DecodeMultiSign(byte[] cborPayload) { throw null; }
        public static CoseMultiSignMessage DecodeMultiSign(ReadOnlySpan<byte> cborPayload) { throw null; }
        public abstract byte[] Encode();
    }

    public sealed partial class CoseSign1Message : CoseMessage
    {
        internal CoseSign1Message() { }
        [UnsupportedOSPlatformAttribute("browser")]
        public static byte[] Sign(byte[] content, SAsymmetricAlgorithm key, SHashAlgorithmName hashAlgorithm, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, bool isDetached = false) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public static byte[] Sign(Stream detachedContent, SAsymmetricAlgorithm key, SHashAlgorithmName hashAlgorithm, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public static byte[] Sign(ReadOnlySpan<byte> content, SAsymmetricAlgorithm key, SHashAlgorithmName hashAlgorithm, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, bool isDetached = false) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public static Threading.Tasks.Task<byte[]> SignAsync(Stream detachedContent, SAsymmetricAlgorithm key, SHashAlgorithmName hashAlgorithm, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, Threading.CancellationToken cancellationToken = default(Threading.CancellationToken)) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public static bool TrySign(ReadOnlySpan<byte> content, Span<byte> destination, SAsymmetricAlgorithm key, SHashAlgorithmName hashAlgorithm, out int bytesWritten, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, bool isDetached = false) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public bool Verify(SAsymmetricAlgorithm key) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public bool Verify(SAsymmetricAlgorithm key, byte[] content) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public bool Verify(SAsymmetricAlgorithm key, Stream detachedContent) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public bool Verify(SAsymmetricAlgorithm key, ReadOnlySpan<byte> content) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public Threading.Tasks.Task<bool> VerifyAsync(SAsymmetricAlgorithm key, Stream detachedContent, Threading.CancellationToken cancellationToken = default(Threading.CancellationToken)) { throw null; }
        public override byte[] Encode() { throw null; }
    }
    
    public sealed partial class CoseMultiSignMessage : CoseMessage
    {
        internal CoseMultiSignMessage() { }
        [UnsupportedOSPlatformAttribute("browser")]
        public static byte[] Sign(byte[] content, CoseSignatureBuilder signatureBuilder, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, bool isDetached = false) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public static byte[] Sign(Stream detachedContent, CoseSignatureBuilder signatureBuilder, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public static byte[] Sign(ReadOnlySpan<byte> content, CoseSignatureBuilder signatureBuilder, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, bool isDetached = false) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public static Threading.Tasks.Task<byte[]> SignAsync(Stream detachedContent, CoseSignatureBuilder signatureBuilder, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, Threading.CancellationToken cancellationToken = default(Threading.CancellationToken)) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public static bool TrySign(ReadOnlySpan<byte> content, CoseSignatureBuilder signatureBuilder, SHashAlgorithmName hashAlgorithm, out int bytesWritten, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, bool isDetached = false) { throw null; }
        public ObjectModel.ReadOnlyCollection<CoseSignature> Signatures { get; }
        public void AddSignature(CoseSignatureBuilder signatureBuilder) { throw null; }
        public void RemoveSignature(int index) { throw null; }
        public override byte[] Encode() { throw null;  }
    }

    public sealed class CoseSignature
    {
        internal CoseSignature() { }
        public CoseHeaderMap ProtectedHeaders { get; }
        public CoseHeaderMap UnprotectedHeaders { get; }
        [UnsupportedOSPlatform("browser")]
        public bool Verify(AsymmetricAlgorithm key) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public bool Verify(AsymmetricAlgorithm key, byte[] content) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public bool Verify(AsymmetricAlgorithm key, Stream detachedContent) { throw null; }
        [UnsupportedOSPlatformAttribute("browser")]
        public bool Verify(AsymmetricAlgorithm key, ReadOnlySpan<byte> content) { throw null; }
    }

    public sealed class CoseSignatureBuilder
    {
        public CoseSignatureBuilder(AsymmetricAlgorithm key, HashAlgorithmName hashAlgorithm, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null) { throw null; }
        public AsymmetricAlgorithm Key { get; set; }
        public HashAlgorithmName HashAlgorithm { get; set; }
        public CoseHeaderMap? ProtectedHeaders { get; set; }
        public CoseHeaderMap? UnprotectedHeaders { get; set; }
    }

    public sealed partial class CoseHeaderMap : IEnumerable<(CoseHeaderLabel Label, ReadOnlyMemory<byte> EncodedValue)>, IEnumerable
    {
        public CoseHeaderMap() { }
        public bool IsReadOnly { get { throw null; } } // true for decoded protected headers; otherwise, false.
        
        public ReadOnlyMemory<byte> GetEncodedValue(CoseHeaderLabel label) { throw null; }
        public ReadOnlySpan<byte> GetValueAsBytes(CoseHeaderLabel label) { throw null; }
        public int GetValueAsInt32(CoseHeaderLabel label) { throw null; }
        public string GetValueAsString(CoseHeaderLabel label) { throw null; }
        public bool TryGetEncodedValue(CoseHeaderLabel label, out ReadOnlyMemory<byte> encodedValue) { throw null; }
        public void SetEncodedValue(CoseHeaderLabel label, ReadOnlySpan<byte> encodedValue) { }
        public void SetValue(CoseHeaderLabel label, int value) { }
        public void SetValue(CoseHeaderLabel label, ReadOnlySpan<byte> value) { }
        public void SetValue(CoseHeaderLabel label, string value) { }
        public void Remove(CoseHeaderLabel label) { }

        public CoseHeaderMap.Enumerator GetEnumerator() { throw null; }
        IEnumerator IEnumerable.GetEnumerator() { throw null; }
        IEnumerator<(CoseHeaderLabel Label, ReadOnlyMemory<byte> EncodedValue)> IEnumerable<(CoseHeaderLabel Label, ReadOnlyMemory<Byte> EncodedValue)>.GetEnumerator() { throw null; }

        public partial struct Enumerator : IEnumerator<(CoseHeaderLabel Label, ReadOnlyMemory<byte> EncodedValue)>, IEnumerator, IDisposable
        {
            public readonly (CoseHeaderLabel Label, ReadOnlyMemory<byte> EncodedValue) Current { get { throw null; } }
            object IEnumerator.Current { get { throw null; } }
            public void Dispose() { }
            public bool MoveNext() { throw null; }
            public void Reset() { }
        }
    }

    public readonly partial struct CoseHeaderLabel : IEquatable<CoseHeaderLabel>
    {
        public CoseHeaderLabel(int label) { throw null; }
        public CoseHeaderLabel(string label) { throw null; }
        // Known headers from https://datatracker.ietf.org/doc/html/rfc8152#page-14 Table 2: Common Header Parameters
        public static CoseHeaderLabel Algorithm { get { throw null; } }
        public static CoseHeaderLabel ContentType { get { throw null; } }
        public static CoseHeaderLabel CounterSignature { get { throw null; } }
        public static CoseHeaderLabel Critical { get { throw null; } }
        public static CoseHeaderLabel IV { get { throw null; } }
        public static CoseHeaderLabel KeyIdentifier { get { throw null; } }
        public static CoseHeaderLabel PartialIV { get { throw null; } }

        public override bool Equals([NotNullWhenAttribute(true)] object? obj) { throw null; }
        public bool Equals(CoseHeaderLabel other) { throw null; }
        public override int GetHashCode() { throw null; }
        public static bool operator ==(CoseHeaderLabel left, CoseHeaderLabel right) { throw null; }
        public static bool operator !=(CoseHeaderLabel left, CoseHeaderLabel right) { throw null; }
    }
}

API Usage

Decoding and verifying messages

bool DecodeAndVerify_Sign1(byte[] encodedMsg, AsymmetricAlgorithm key, byte[]? signedContent)
{
    CoseSign1Message msg = CoseMessage.DecodeSign1(encodedMsg);

    // There are two scenarios for verification, 
    // embedded content (it was carried in the message and can be used for verification)
    // and detached content (it was not part of the message and a CBOR nil terminator was placed instead).
    if (msg.Content != null)
    {
        return msg.Verify(key);
    }
    else
    {
        Debug.Assert(signedContent != null);
        return msg.Verify(key, signedContent);
    }
}

bool DecodeAndVerify_MultiSign(byte[] encodedMsg, AsymmetricAlgorithm key, byte[]? signedContent)
{
    CoseSign1Message msg = CoseMessage.DecodeMultiSign(encodedMsg);
    ReadOnlyCollection<CoseSignature> signatures = msg.Sginatures;

    foreach (CoseSignature s in signatures)
    {
        bool verified = msg.Content != null ? s.Verify(key) : s.Verify(key, signedContent);
        if (verified)
        {
            // Consider a valid verification if at least one signature verifies.
            return true;
        }
    }
    
    return false;
}

Signing and encoding messages

byte[] Sign_WithSign1(byte[] content, AsymmetricAlgorithm key, HashAlgorithmName hash)
{
    return CoseSign1Message.Sign(content, key, hash);
}

byte[] Sign_WithMultiSign(byte[] content, AsymmetricAlgorithm key, HashAlgorithmName hash)
{
    // COSE_Sign uses COSE_Signature for transporting signature, that's why we use CoseSignatureBuilder, which can also accept headers.
    var signatureBuilder = new CoseSignatureBuilder(key, hash);
    return CoseMultiSignMessage.Sign(content, signatureBuilder);
}

Append unprotected headers or signatures to already signed messages.

byte[] AddUnprotectedHeader(byte[] encodedMsg)
{
    CoseSign1Message msg = CoseMessage.DecoseSign1(encodedMsg);
    msg.UnprotectedHeaders.SetValue(CoseHeaderLabel.ContentType, "application/json");
    
    // re-encode
    msg.Encode();
}

byte[] AddSignature(byte[] encodedMsg, Asymmetricalgorithm key, HashAlgorithmName hash)
{
    CoseMultiSignMessage msg = CoseMessage.DecoseMultiSign(encodedMsg);
    msg.AddSignature(new CoseSignatureBuilder(key, hash));

    return msg.Encode();
}

Use custom headers

byte[] Sign_WithCustomHeaders(byte[] content, AsymmetricAlgorithm key, HashAlgorithmName hash)
{
    var protectedHeaders = new CoseHeaderMap();
    var unprotectedHeaders = new coseHeaderMap();

    // Key identifier values are hints about which key to use. 
    unprotectedHeaders.SetValue(CoseHeaderLabel.KeyIdentifier, Encoding.UTF8.GetBytes("11"));

    // This parameter is used to indicate which protected header labels an application that is processing a message is required to understand. 
    var writer = new CborWriter();
    writer.WriteStartArray(definiteLength: 1);
    writer.WriteString("42");
    writer.WriteEndArray();
    protectedHeaders.SetEncodedValue(CoseHeaderLabel.Critical, writer.Encode());

    protectedHeaders.SetValue(new CoseHeaderLabel("42"), "this is my custom critical header.");

    return CoseSign1Message.Sign(content, key, hash, protectedHeaders, unprotectedHeaders);
}

Supply content via a stream in order to sign large contents.

Task<byte[]> Sign_WithStreamContent(AsymmetricAlgorithm key, HashAlgorithmName hash, Cancellationtoken ct)
{
    Stream contentStream = File.Open("ubuntu-22.04-desktop-amd64.iso", FileMode.Open);
    return CoseSign1Message.SignAsync(contentStream, key, hash, ct);
}

Alternative Designs

No response

Risks

The COSE specification contains other formats, such as COSE_Encrypt and COSE_Mac, that were considered while writing this proposal but that haven't been fully explored, there is a small chance that we could have confilcting APIs in the future if .NET chooses to support those formats as well.

Author:	Jozkee
Assignees:	Jozkee
Labels:	`api-suggestion`, `area-System.Security`, `blocking`
Milestone:	7.0.0

vcsjones · 2022-05-27T16:08:26Z

Some thoughts:

Any place we have Encode, should we also have a bool TryEncode(Span<byte> destination, out int bytesWritten) and/or int Encode(Span<byte> destination)?
Do we need RSASignaturePadding anywhere? IANA has both PSS and PKCS1 registered https://www.iana.org/assignments/cose/cose.xhtml. PKCS1 notably is useful in WebAuthN / CTAP scenarios and is registered by RFC 8812, and RFC 8230 registers PSS.

letmaik · 2022-05-27T16:11:17Z

Some observations:

Using Stream implies detached payloads. Could that be confusing? Should the Stream overload be like all other overloads?
CoseHeaderMap is an IEnumerable. I think it would be useful to have a Contains(label) method. Any reason this can't be an IDictionary?
The example AddUnprotectedHeader should use a different label since putting the content type in the unprotected header could cause security problems. I suggest the key identifier example from Sign_WithCustomHeaders.

Jozkee · 2022-05-27T16:20:02Z

should we also have a bool TryEncode(Span destination, out int bytesWritten) and/or int Encode(Span destination)?

@vcsjones, yes, I will include them in the proposal.

Do we need RSASignaturePadding anywhere?

@bartonjs, thoughts?

Should the Stream overload be like all other overloads?

@letmaik, I thought about it when I was implementing those overloads, it seems that wanting embedded content in Stream scenarios defies the whole purpose of using a Stream because it will load in memory the whole content, which is what motivated the overload to exists, don't you think?

kevinmkane · 2022-05-27T16:28:10Z

What about support for using the _Tagged versions of COSE_Sign1 and COSE_Sign? Either a method to encode the structure including the enclosing tag, and/or a decoding method that determines the type of the message from the tag, if the caller knows to expect one, just to save the caller dealing with the CBOR of the tag directly.
What about content for the external_aad member of the Sig_structure, external data bound to the signature that isn't part of the payload?
Can you explain the thinking behind the CoseSignatureBuilder? In particular, why the Sign APIs for CoseMultiSignMessage take CoseSignatureBuilder but also a separate set of those same signing parameters?
+1 about the Sign APIs being clearer about generating embedded versus detached payloads. I'd advocate for a different method name entirely for generating detached signatures, instead of an optional boolean parameter, to avoid confusion. Verify knows which state it's in, at least, so it can throw if it gets called in the wrong circumstance (without a payload when it's a detached signature, or with a payload when the payload is embedded). In fact I would expect all of those overloads that take content to call that parameter detachedContent and never just content as there would be no need if the payload is embedded.
RemoveSignature in CoseMultiSignMessage takes an index. I'm assuming this is an index into the collection returned by Signatures. Would an overload that takes a CoseSignature also be appropriate?

Nits: The usage of msg.Encode() is inconsistent. Sometimes it's called as a procedure and other times its return value is referenced. I think you want the latter, and certainly that is what I would advocate for, that the encoded version is returned by that method. You've also got CoseSign1Message as the type of msg in DecodeAndVerify_MultiSign where you want CoseMultiSignMessage.

bartonjs · 2022-05-27T17:00:54Z

Do we need RSASignaturePadding anywhere? ... RFC 8812 ...

Somehow RFC 8812 shows up for me as a previously visited page, but the contents were not in my brain. I had internalized that COSE was RSA-PSS only.

That means Sign1 needs either an optional parameter with AsymmetricAlgorithm (and RSA throws if the padding type wasn't specified) or change it to be overloaded by algorithm type and the RSA overload has a mandatory RSASignaturePadding parameter.

CoseSignatureBuilder

Since there's no Build(...) method on this type it isn't a builder. A more appropriate name seems to be CoseSigner. And, apparently, it also needs an RSASignaturePadding.

I'd advocate for a different method name entirely for generating detached signatures, instead of an optional boolean parameter, to avoid confusion.

Yeah, the public members being Sign and SignDetached might give better cluing as to what's going on. I'd go ahead and split Verify to Verify/VerifyDetached at the same time, to just make it clear what the pairings are.

Splitting verify would probably become more relevant with AAD. (Ooh, it doesn't make a distinction between "no AAD" and "empty AAD", so we can use default spans)

public bool Verify(AsymmetricAlgorithm key, ReadOnlySpan<byte> someNameForAad = default);
public bool VerifyDetached(AsymmetricAlgorithm key, ReadOnlySpan<byte> detachedContent, ReadOnlySpan<byte> someNameForAad = default);
public bool VerifyDetached(AsymmetricAlgorithm key, Stream detachedContent, ReadOnlySpan<byte> someNameForAad = default);
public bool VerifyDetachedAsync(AsymmetricAlgorithm key, Stream detachedContent, ReadOnlyMemory<byte> someNameForAad = default, CancellationToken cancellationToken = default);

// plus byte[] overloads

letmaik · 2022-05-27T17:04:05Z

Should the Stream overload be like all other overloads?

@letmaik, I thought about it when I was implementing those overloads, it seems that wanting embedded content in Stream scenarios defies the whole purpose of using a Stream because it will load in memory the whole content, which is what motivated the overload to exists, don't you think?

I don't have experience with Stream, but I think you're saying that code using Stream always assumes potentially large data. Is that really true? Separate SignDetached methods as suggested by others would also avoid confusion. Then it would be ok if Stream only exists under SignDetached.

vcsjones · 2022-05-27T17:06:50Z

or change it to be overloaded by algorithm type and the RSA overload has a mandatory RSASignaturePadding parameter.

That's similar to what we did with CmsSigner, so, I would advocate to follow that pattern.

bartonjs · 2022-05-27T17:07:40Z

What about support for using the _Tagged versions of COSE_Sign1 and COSE_Sign? Either a method to encode the structure including the enclosing tag

This implementation, currently, only produces tagged versions.

a decoding method that determines the type of the message from the tag, if the caller knows to expect one, just to save the caller dealing with the CBOR of the tag directly.

We did talk about that early on. It's probably not terrible, but does come with a bit of an annoyance. It'd be something like public static CoseMessage [CoseMessage::]DecodeAnyTagged(...)... and then you have to as-cast it to Sign1/MultiSign/Mac/Mac0/Encrypt/Encrypt0. "Call and cast" always feels like a pit of failure to me.

kevinmkane · 2022-05-27T17:32:02Z

What about support for using the _Tagged versions of COSE_Sign1 and COSE_Sign? Either a method to encode the structure including the enclosing tag

This implementation, currently, only produces tagged versions.

Is tagged the common usage? It's what we're doing in the application I'm working on, but I don't have visibility into the broader COSE-using ecosystem.

Somehow RFC 8812 shows up for me as a previously visited page, but the contents were not in my brain. I had internalized that COSE was RSA-PSS only.

I think once upon a time the hope was COSE could leave behind the legacy of PKCS#1v1.5 padding but it just has a way of worming its way back in. Indeed, I think COSE hoped to leave RSA behind entirely, but then RFC8230 came along and specified PSS for it.

Yeah, the public members being Sign and SignDetached might give better cluing as to what's going on. I'd go ahead and split Verify to Verify/VerifyDetached at the same time, to just make it clear what the pairings are.

I still advocate for the Verify variants to throw if they're called in the wrong state (providing content when embedded and vice-versa).

We did talk about that early on. It's probably not terrible, but does come with a bit of an annoyance. It'd be something like public static CoseMessage [CoseMessage::]DecodeAnyTagged(...)... and then you have to as-cast it to Sign1/MultiSign/Mac/Mac0/Encrypt/Encrypt0. "Call and cast" always feels like a pit of failure to me.

It's grating but hard to avoid when you're dealing with variant types. It's either that, or reference the superclass and have a bunch of "if (is subtype A) { ... } else if (is subtype B) { ... }" logic. Right now the proposal assumes the caller knows which to expect ahead of time, and if they don't, they'll have to do the peeking themselves.

Jozkee · 2022-06-01T19:58:40Z

@ everyone, please take another look at the API proposal, I've updated it based on the feedback received.

CoseSignatureBuilder

We renamed CoseSignatureBuilder to CoseSigner, as the type is not really building anything, it is signing with the provided inputs.

RSASignaturePadding

We added RSASignaturePadding to CoseSigner and replaced Asymmetricalgorithm, HashAlgorithmName, and CoseHeaderMaps parameters for a Cosesigner signer for CoseSign1Message.Sign* and CoseMiltiSignMessage.Sign*, for Sign1 we will use the headers form the signer as the headers of the document.

Sign APIs being clearer about generating embedded versus detached payloads

We removed the bool isDetached parameter and instead replaced Sign() with SignEmbedded and SignDetached. This naming convention also applies for Verify*.

CoseHeaderMap is an IEnumerable. I think it would be useful to have a Contains(label) method. Any reason this can't be an IDictionary?

We think that CoseHeaderMap doesn't need to be a dictionary, if you need a Contains method, you can use map.TryGetEncodedValue(yourLabel, out _). In the hypotetical case where we turn CoseHeaderMap into a dictionary, it will be awkward to implement certain methods, e.g: Add(), it would need to only accept a CBOR encoded value, having a custom map allow us to expose Get/SetValue(CoseHeaderLabel, int), Get/SetValue(CoseHeaderLabel, string), etc. and Get/SetEncodedValue(CoseHeaderLabel, ReadOnlySpan<byte>), keeping the method name clearer, as how you suggested with SignEmbedded/SignDetached, without tying the type to an Add(CoseHeaderLabel, ReadOnlySpan<byte> encodedValue).

external_aad

Yes, we will support it; all Sign operations will contain a optional param. ReadOnlySpan<byte> associatedData = default or byte[] associatedData = null.

RemoveSignature in CoseMultiSignMessage takes an index. Would an overload that takes a CoseSignature also be appropriate?

Yes, we will support RemoveSignature(CoseSignature signature) as well.

_Tagged versions of COSE_Sign1 and COSE_Sign

We still don't have a case where it makes sense to produce an untagged message. For decoding, we accept both, tagged and untagged variants.

a decoding method that determines the type of the message from the tag, if the caller knows to expect one, just to save the caller dealing with the CBOR of the tag directly.

For untagged messages, you can't really distinguish between messages, because the COSE structures are ambiguous, so it depends on the caller on how they choose to interpret the message.
For tagged messages, we could provide a helper that returns the kind of message you have, but the implementation of such method would be to look at the first byte of the CBOR payload (the tag), which isn't that valuable.
For now, you need to figure out yourself ahead of time which kind of meesage you are trying to decode.

kevinmkane · 2022-06-01T20:43:37Z

I suggest changing associatedData to externalData just because the RFC calls this "externally supplied data". "Associated" isn't a word used with this in the RFC.

The handling of the two header buckets I find confusing because of the following:

COSE_Sign1 has a single set of these buckets. The API here for signing a single-signer message does not take them as parameters directly, but it takes a CoseSigner which has these buckets, and so it would seem the headers in that object end up in the Headers field of the COSE_Sign1 output.

COSE_Sign has multiple sets of these buckets. There's a top-level set of headers for the entire message, and then each individual signature has its own headers. The Sign* methods for CoseMultiSignMessage take a CoseSigner and optional protected and unprotected header buckets. Looking at this API in isolation, I would expect the two top-level arguments would populate the top-level headers for the COSE_Sign message, and then the header buckets inside the CoseSigner would populate the headers for the COSE_Signature added to the signatures array for this signer. But this differs from what appears to be the only interpretation of the signing APIs for CoseSign1Message.

Is my interpretation correct?

vcsjones · 2022-06-01T20:53:43Z

public abstract int Encode(Span<byte> destination);

Does this need to be abstract? Can't it just be a regular method that basically does:

public int Encode(Span<byte> destination) {
    if (!TryEncode(destination, out int written)) {
        throw new ArgumentException(SR.Argument_DestinationTooShort, nameof(destination));
    }

    return written;
}

I think, ideally, the APIs for encode would look like:

public byte[] Encode();
public int Encode(Span<byte> destination);
public bool TryEncode(Span<byte> destination, out int bytesWritten);

public abstract int ComputeEncodedSize();
protected abstract TryEncodeCore(Span<byte> destination, out int bytesWritten);

I don't know how feasible it is to up-front compute the length of a COSE message. But if we had ComputeEncodedSize and TryEncodeCore as the only things that need to be implemented, then the rest can be implemented in terms of TryEncodeCore.

public void RemoveSignature(CoseSignature signature) { throw null; }

Out of curiosity, how does this work? CoseSignature does not implement IEquatable<CoseSignature>. Should it? Or will the comparison remain a private implementation detail, or will it rely entirely on reference equality?

public void SetEncodedValue(CoseHeaderLabel label, ReadOnlySpan<byte> encodedValue)

This does not have a byte[] overload like we typically do. I don't know if it's needed, but thought I would call it out as something that stands out to me. I get that the value will need to be cloned regardless of whether it is a span or an array, but we seem to be going out of out way to have byte[] overloads where possible, except here.

public sealed class CoseSigner {
    public AsymmetricAlgorithm Key { get; set; }
    public HashAlgorithmName HashAlgorithm { get; set; }
    public CoseHeaderMap? ProtectedHeaders { get; set; }
    public CoseHeaderMap? UnprotectedHeaders { get; set; }
    public RSASignaturePadding? RSASignaturePadding{ get; set; }
}

What's the thought behind all of these having a set? Why not just leave them as get only?

Otherwise you end up having to do a lot of validation again in the set, not mixing / matching RSASignaturePadding with ECDsa, etc.

bartonjs · 2022-06-01T21:07:26Z

Otherwise you end up having to do a lot of validation again in the set, not mixing / matching RSASignaturePadding with ECDsa, etc.

For RSASignaturePadding, at least, the settableness came from CmsSigner. In SignedCms we just ignore the property if the key wasn't RSA (99% confident on that statement). And there, we predated PSS, so it defaults to PKCS1 if the padding wasn't specified... whereas here it's required.

The scenario for the set on RSASignaturePadding is... slightly convoluted... but exists.

A netstandard2.0 library uses COSE.
They have opinions about RSA signature padding and always make it be PSS, or PKCS1, whichever.
They accept keys from their callers and just pass them through.
We add support for some new algorithm in the future, but with no netstandard2.0 ref. (EdDSA is mentioned in the spec, so let's go with that one)

If we broke it up like

public partial class CoseSigner
{
    public CoseSigner(RSA key, RSASignaturePadding signaturePadding, ...);
    public CoseSigner(ECDsa key, ...);
    public CoseSigner(EdDSA key, ...);
}

then this library wouldn't be able to pass through an EdDSA key without split compiling. Instead, they can do

CoseSigner signer = new CoseSigner(key, ...)
{
    RSASignaturePadding = RSASignaturePadding.IHaveOpinionsHere,
};

and that works until/unless EdDSA (or whatever) also needs a sidecar parameter.

But, I agree, the rest don't seem to have justification for having setters.

Jozkee · 2022-06-01T21:11:40Z

Is my interpretation correct?

@kevinmkane yes it is correct, we removed CoseHeaderMaps from CoseSign1Message.Sign() in order to avoid duplication with the headers on CoseSigner, it also makes more sense for using CoseSigner in both scenarios (multi and single).

public abstract int Encode(Span destination);

Does this need to be abstract? Can't it just be a regular method that basically does:

@vcsjones yes, [Try]Encode methods can be non-abstract.
ComputeEncodedSize is actually implemented internally:

runtime/src/libraries/System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseSign1Message.cs

Line 99 in 89cee6e

    
           int expectedSize = ComputeEncodedSize(protectedHeaders, unprotectedHeaders, algHeaderValueToSlip, contentLength, isDetached, key.KeySize, keyType);

And TryEncodeCore can be exposed as you suggest, although I don't know about the naming, is it common to expose methods called DoSomethingCore? Could we keep TryEncode as abstract?

CoseSignature does not implement IEquatable. Should it? Or will the comparison remain a private implementation detail, or will it rely entirely on reference equality?

Reference equality should suffice. Users are not able to instantiate the type themselves, so it shouldn't be an issue. I can also see that CmsSigner doesn't implement IEquatable either

bartonjs · 2022-06-01T21:16:29Z

Is my interpretation [of CoseSigner+Headers] correct?

Yeah. Really the question was "how do we fix the RSASignaturePadding problem for Sign1 now that we fixed it for MultiSign?", and that's what I came up with as a solution. Here's the logic. If you don't agree with it after, we'd be happy to entertain suggestions of how to cleanly solve it another way (I can't figure out how to reword that with no sarcasm tone implied, but there's only sincerity).

Look at MultiSign first, since that's basically CMS's SignedData CBOR-ified.
Think "it's weird when there's only one signer, because what's the difference between a document's headers and a signer's headers?"
Create Sign1 in your head by smacking COSE_Sign.signers[0] with a sledgehammer until it merges into the superstructure, and realize "the signer headers and the document headers are the same".
Extract the tuple of (key, optional padding mode, digest algorithm, protected headers, unprotected headers) into CoseSigner.
The API shape we came up with.

The next best alternative was to keep the document headers and either

Merge them with the signer headers (which creates a merge conflict policy problem), or
Throw if the signer headers weren't empty

This slightly wonky approach seemed the least awkward... but feels somewhat intuitive if you design Sign before Sign1. (The problem is we all came to it from the other way around)

vcsjones · 2022-06-01T21:21:18Z

For RSASignaturePadding, at least, the settableness came from CmsSigner.

I... suppose. When we added it to CmsSigner I had assumed it was done "because everything else on the class is settable" and mostly a consistency thing, not "this needs to be settable"

The scenario for the set on RSASignaturePadding is... slightly convoluted... but exists.

I'm not sure I follow this:

If we broke it up like

I think we can leave the constructors as-is. I think what I am suggesting is, if a library author is super opinionated...

using AsymmetricAlgorithm key = GetKey();

CoseSigner signer = key switch {
    RSA rsa => new CoseSigner(rsa, RSASignaturePadding.Pss, ...), // Opinions!
    AsymmetricAlgorithm alg => new CoseSigner(alg, ...), // Pass though
    null => throw new ArgumentNullException(nameof(key)); // Agh!
};

Jozkee · 2022-06-01T21:23:55Z

I suggest changing associatedData to externalData just because the RFC calls this "externally supplied data". "Associated" isn't a word used with this in the RFC.

@kevinmkane the RFC 8152 also calls it external_add, where aad stands for Additional Associated Data. We have implemented AEAD (Authenticated Encryption with Associated Data) before and it has been called associatedData in other .NET APIs (see AesGcm.Decrypt (..., byte[]? associatedData = default), it seem to me that both terms are used interchangeably, hence the proposed name of the C# parameter.

bartonjs · 2022-06-01T21:26:51Z

I think we can leave the constructors as-is. [and then a convincing example]

That's.... fair.

I guess we can get away with none of the properties being settable for now.

bartonjs · 2022-06-01T21:28:12Z

It's helpful, I guess, in that CoseSigner..ctor(Asymmetricalgorithm, ... can just throw for RSA keys, instead of deferring the exception to a call to Sign.

vcsjones · 2022-06-01T21:30:07Z

is it common to expose methods called DoSomethingCore? Could we keep TryEncode as abstract?

In an attempt to please @bartonjs, I will borrow from the Framework Design Guidelines:

“CONSIDER naming protected virtual members that provide extensibility points for nonvirtual members by suffixing the nonvirtual member name with “Core.”

So, hopefully I am interpreting that correctly, but we also did it for the symmetric crypto one-shots.

vcsjones · 2022-06-01T21:31:42Z

To clarify the TryEncode TryEncodeCore:

TryEncodeCore would be protected and do little validation. TryEncode would be public, validate parameters if needed, and then itself defer to TryEncodeCore.

bartonjs · 2022-06-01T21:31:49Z

And TryEncodeCore can be exposed as you suggest, although I don't know about the naming, is it common to expose methods called DoSomethingCore?

For protected members, yes. Framework Design Guidelines, 3rd edition, section 9.9 (Template Method (Pattern)).

As @vcsjones just beat me to quoting 😄.

bartonjs · 2022-06-01T21:38:03Z

In this case, since there are no parameters other than the destination span, I don't know if the template method is useful.

If the non-virtual Try calls ComputeEncodedSize as a precondition, then the Core method would be the int-returning Encode, since it would never have a return false;. And without that check, TryEncode would just call TryEncodeCore with no further information. So it seems like

public byte[] Encode()
{
    byte[] dest = new byte[ComputeEncodedSize()];
    int written = Encode(dest);
    Debug.Assert(written == dest.Length);
    return dest;
}

public int Encode(Span<byte> destination)
{
    if (TryEncode(destination, out int written))
    {
        return written;
    }

    throw new ArgumentException(SR.WhateverSpanTooSmallIs);
}

public abstract bool TryEncode(Span<byte> destination, out int bytesWritten);

does seem to be the full complement.

kevinmkane · 2022-06-01T21:38:50Z

This slightly wonky approach seemed the least awkward... but feels somewhat intuitive if you design Sign before Sign1. (The problem is we all came to it from the other way around)

@bartonjs I can appreciate the logic, and I'm not advocating for a change here. I think this is a problem that can be solved by good API documentation for the multi-sign signing APIs. You could rename the parameters to messageProtectedHeaders and messageUnprotectedHeaders but I don't have a strong preference either way.

I suggest changing associatedData to externalData just because the RFC calls this "externally supplied data". "Associated" isn't a word used with this in the RFC.

The RFC 8152 also calls it external_add, where aad stands for Additional Associated Data. We have implemented AEAD (Authenticated Encryption with Associated Data) before and it has been called associatedData in other .NET APIs (see AesGcm.Decrypt (..., byte[]? associatedData = default), it seem to me that both terms are used interchangeably, hence the proposed name of the C# parameter.

@Jozkee Now that I look closely, RFC 8152 erroneously calls AEAD "Authenticated Encryption with Authenticated Data", and in section 5.3 defines AAD as "Additional Authenticated Data". So this terminology looks like it may have been a mistake. If existing .NET APIs already follow the "associated" naming convention, then I agree it's sensible to stick to that.

bartonjs · 2022-06-01T21:42:47Z

Sadly, "A" means either Associated or Additional depending on which abbreviation you base it on.

https://csrc.nist.gov/glossary/term/aead

Authenticated Encryption with Associated Data

https://csrc.nist.gov/glossary/term/aad

Additional Authentication Data

vcsjones · 2022-06-01T21:48:10Z

I don't know if the template method is useful.

That's a fair point. There is nothing to validate.

protected bool TryEncode(Span<byte> destination, out int bytesWritten);

Assuming then you meant for that one to be public?

bartonjs · 2022-06-01T21:49:24Z

And abstract. Do what I mean, not what I say 😁

GrabYourPitchforks · 2022-06-01T22:18:22Z

If existing .NET APIs already follow the "associated" naming convention, then I agree it's sensible to stick to that.

Yeah, I agree it's ok to stick with existing conventions.

My (weak) preference is that we don't choose the name additionalData. The term "additional" here could imply that the payload is carrying the data in some form, which is not the case. "Associated" doesn't carry this connotation. At least to me. :)

In the end I imagine it's not going to matter all that much. The docs will point to https://cose-wg.github.io/cose-spec/#rfc.section.4.3 or whatever other section is relevant, and the caller will say "oh, I get it!"

Jozkee · 2022-06-01T22:21:49Z

This is what I captured from today's feedback.

Changes to [Try]Encode methods and inclusion of int ComputeEncodedSize().

Replace:

public abstract byte[] Encode();
public abstract int Encode(Span<byte> destination);
public abstract bool TryEncode(Span<byte> destination, out int bytesWritten);

With:

public abstract int ComputeEncodedSize();
public byte[] Encode() { }
public int Encode(Span<byte> destination) { }
public abstract bool TryEncode(Span<byte> destination, out int bytesWritten);

Add public void SetEncodedValue(CoseHeaderLabel label, byte[] encodedValue) for parity with other byte-array overloads.
Remove all setters in CoseSigner properties.

If there's no more feedback, I will label this as api-ready-for-review.

vcsjones · 2022-06-01T22:35:23Z

public abstract int ComputeEncodedSize();

I don't know that that is the right name for a public API. AsnWriter for example calls it GetEncodedLength. I don't have super strong feelings on the name, but would perhaps reconsider my suggestion (which was based entirely off the existing internal method).

Jozkee · 2022-06-01T22:45:15Z

There are not many existing .NET APIs using GetEncodedLength and GetEncodedSize (only one result for each, both in the crypto space). So I guess either one is fine. For ComputeEncodedSize there were no results.

https://www.google.com/search?q=GetEncodedLength+site%3Adocs.microsoft.com&rlz=1C1GCEU_enMX930MX930&sxsrf=ALiCzsbpkQSG0nelBY6gSeGGSZiw1LWu5g%3A1654123159639&ei=l-qXYoXLJtvFqtsPheKQqAw&ved=0ahUKEwjF8uD6qI34AhXbomoFHQUxBMUQ4dUDCA4&uact=5&oq=GetEncodedLength+site%3Adocs.microsoft.com&gs_lcp=Cgdnd3Mtd2l6EAM6BwgjELADECc6BwghEAoQoAE6BAgjECdKBAhBGAFKBAhGGABQuwNYw5YEYNGZBGgBcAB4AIABVogB0wGSAQEzmAEAoAEBoAECyAEBwAEB&sclient=gws-wiz

https://www.google.com/search?q=GetEncodedSize+site%3Adocs.microsoft.com&rlz=1C1GCEU_enMX930MX930&sxsrf=ALiCzsa51fntOxiRrCJm659tCSk-uq3mpQ%3A1654123231888&ei=3-qXYt3YNbLEqtsP2YWWmAg&ved=0ahUKEwidxJqdqY34AhUyomoFHdmCBYMQ4dUDCA4&uact=5&oq=GetEncodedSize+site%3Adocs.microsoft.com&gs_lcp=Cgdnd3Mtd2l6EANKBAhBGAFKBAhGGABQkgNYkgNg0wRoAXAAeACAATuIATuSAQExmAEAoAEBwAEB&sclient=gws-wiz

kevinmkane · 2022-06-02T14:16:30Z

Remove all setters in CoseSigner properties.

In this case, ProtectedHeaders and UnprotectedHeaders shouldn't be nullable. Each CoseHeaderMap should always be there, but may just be empty. Then a caller can always access it to modify the headers after creation but before doing a signing operation. And in that case, a method on the map to clear it would be useful.

Jozkee · 2022-06-02T15:29:22Z

Each CoseHeaderMap should always be there, but may just be empty.

That will create allocations where they can be avoided.
If we keep it as nullable, we can change it in the future to non-nullable, the same can't be said the other way around.
What's wrong with forcing users to call new CoseSigner(key, hash, myProtectedHeaders)?

kevinmkane · 2022-06-02T16:09:48Z

Each CoseHeaderMap should always be there, but may just be empty.

That will create allocations where they can be avoided.

The allocations could be avoided by using a backing field that only gets allocated if a caller calls the property getter when the object hadn't been originally created with the map present.

What's wrong with forcing users to call new CoseSigner(key, hash, myProtectedHeaders)?

Since CoseHeaderMap supports its contents being modified, a caller could reuse a CoseSigner to add to the unprotected headers bucket by calling signer.UnprotectedHeaders.SetValue(...) but only if the CoseSigner had originally been created with a non-null unprotected headers bucket. The comment on the IsReadOnly property implies UnprotectedHeaders will be returned with this property false and so allow this kind of modification (if it exists in the CoseSigner).

As a result, in order to modify the unprotected headers, either the application always has to new up a CoseSigner, or have some logic to see if UnprotectedHeaders is null, and then either calling SetValue on the existing object to avoid an unnecessary allocation, or newing up a new CoseSigner.

It just seems to lead to clunky application code because, even though there's no setter, the property allows its contents to be modified. When there was a setter, if it was null, the caller could have provided one rather than copy the contents into a newly allocated CoseSigner.

If IsReadOnly will always be true for header maps returned by CoseSigner (as opposed to only ProtectedHeaders and ones allocated by callers), then the above pattern becomes impossible. But then we're deciding there is no in-place modification of unprotected headers and always requires allocating a new CoseSigner.

terrajobst · 2022-06-02T19:19:39Z

Video

Consider either marking the entire assembly as [UnsupportedOSPlatform("browser")] or just the leaf nodes (e.g. CoseSigner's constructor).
Looks like the assembly isn't fully null annotated (e.g. the byte[] parameters)
- We should do that, even if it's targeting .NET Standard 2.0.
Consider making CoseHeaderMap a dictionary of CoseHeaderKey/CoseHeaderValue, which makes the operations more uniform and also solves the problem that all operations can work on byte[] not just ReadOnlyMemory<byte>

namespace System.Security.Cryptography.Cose;

public abstract class CoseMessage
{
    public ReadOnlyMemory<byte>? Content { get; }
    public CoseHeaderMap ProtectedHeaders { get; }
    public CoseHeaderMap UnprotectedHeaders { get; }
    public static CoseSign1Message DecodeSign1(byte[] cborPayload);
    public static CoseSign1Message DecodeSign1(ReadOnlySpan<byte> cborPayload);
    public static CoseMultiSignMessage DecodeMultiSign(byte[] cborPayload);
    public static CoseMultiSignMessage DecodeMultiSign(ReadOnlySpan<byte> cborPayload);
    public byte[] Encode();
    public int Encode(Span<byte> destination);
    public abstract bool TryEncode(Span<byte> destination, out int bytesWritten);
    public abstract int GetEncodedLength();
}
public sealed class CoseSign1Message : CoseMessage
{
    [UnsupportedOSPlatform("browser")]
    public static byte[] SignDetached(byte[] detachedContent, CoseSigner signer, byte[]? associatedData = null);
    [UnsupportedOSPlatform("browser")]
    public static byte[] SignDetached(ReadOnlySpan<byte> detachedContent, CoseSigner signer, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public static byte[] SignDetached(Stream detachedContent, CoseSigner signer, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public static Task<byte[]> SignDetachedAsync(Stream detachedContent, CoseSigner signer, ReadOnlyMemory<byte> associatedData = default, CancellationToken cancellationToken = default(CancellationToken));
    [UnsupportedOSPlatform("browser")]
    public static byte[] SignEmbedded(byte[] embeddedContent, CoseSigner signer, byte[]? associatedData = null);
    [UnsupportedOSPlatform("browser")]
    public static byte[] SignEmbedded(ReadOnlySpan<byte> embeddedContent, CoseSigner signer, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public static bool TrySignEmbedded(ReadOnlySpan<byte> embeddedContent, Span<byte> destination, CoseSigner signer, out int bytesWritten, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public static bool TrySignDetached(ReadOnlySpan<byte> detachedContent, Span<byte> destination, CoseSigner signer, out int bytesWritten, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public bool VerifyDetached(AsymmetricAlgorithm key, byte[] detachedContent, byte[]? associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public bool VerifyDetached(AsymmetricAlgorithm key, ReadOnlySpan<byte> detachedContent, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public bool VerifyDetached(AsymmetricAlgorithm key, Stream detachedContent, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public Task<bool> VerifyDetachedAsync(AsymmetricAlgorithm key, Stream detachedContent, ReadOnlyMemory<byte> associatedData, CancellationToken cancellationToken = default(CancellationToken));

    [UnsupportedOSPlatform("browser")]
    public bool VerifyEmbedded(AsymmetricAlgorithm key, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public bool VerifyEmbedded(AsymmetricAlgorithm key, byte[]? associatedData = null);

    public override bool TryEncode(Span<byte> destination, out int bytesWritten);
    public override int GetEncodedLength();
}
public sealed class CoseMultiSignMessage : CoseMessage
{
    [UnsupportedOSPlatform("browser")]
    public static byte[] SignDetached(byte[] detachedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, byte[]? associatedData = null);
    [UnsupportedOSPlatform("browser")]
    public static byte[] SignDetached(ReadOnlySpan<byte> detachedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public static byte[] SignDetached(Stream detachedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public static Task<byte[]> SignDetachedAsync(Stream detachedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlyMemory<byte> associatedData = default, CancellationToken cancellationToken = default(CancellationToken));
    [UnsupportedOSPlatform("browser")]
    public static byte[] SignEmbedded(byte[] embeddedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, byte[]? associatedData = null);
    [UnsupportedOSPlatform("browser")]
    public static byte[] SignEmbedded(ReadOnlySpan<byte> embeddedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public static bool TrySignEmbedded(ReadOnlySpan<byte> embeddedContent, CoseSigner signer, out int bytesWritten, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public static bool TrySignDetached(ReadOnlySpan<byte> detachedContent, CoseSigner signer, out int bytesWritten, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlySpan<byte> associatedData = default);
    public ReadOnlyCollection<CoseSignature> Signatures { get; }
    [UnsupportedOSPlatform("browser")]
    public void AddSignature(CoseSigner signer, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public void AddSignature(CoseSigner signer, byte[]? associatedData = null);
    public void RemoveSignature(CoseSignature signature);
    public void RemoveSignature(int index);
    public override bool TryEncode(Span<byte> destination, out int bytesWritten);
    public override int GetEncodedLength();
}
public sealed class CoseSignature
{
    public CoseHeaderMap ProtectedHeaders { get; }
    public CoseHeaderMap UnprotectedHeaders { get; }
    [UnsupportedOSPlatform("browser")]
    public bool VerifyEmbedded(AsymmetricAlgorithm key, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public bool VerifyDetached(AsymmetricAlgorithm key, byte[] detachedContent, byte[]? associatedData = null);
    [UnsupportedOSPlatform("browser")]
    public bool VerifyDetached(AsymmetricAlgorithm key, ReadOnlySpan<byte> detachedContent, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public bool VerifyDetached(AsymmetricAlgorithm key, Stream detachedContent, ReadOnlySpan<byte> associatedData = default);
    [UnsupportedOSPlatform("browser")]
    public Task<bool> VerifyDetachedAsync(AsymmetricAlgorithm key, Stream detachedContent, ReadOnlyMemory<byte> associatedData = default);
}
public sealed class CoseSigner
{
    public CoseSigner(AsymmetricAlgorithm key, HashAlgorithmName hashAlgorithm, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null);
    public CoseSigner(RSA key, RSASignaturePadding signaturePadding, HashAlgorithmName hashAlgorithm, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null);
    public AsymmetricAlgorithm Key { get; }
    public HashAlgorithmName HashAlgorithm { get; }
    public CoseHeaderMap? ProtectedHeaders { get; }
    public CoseHeaderMap? UnprotectedHeaders { get; }
    public RSASignaturePadding? RSASignaturePadding { get; }
}
public sealed class CoseHeaderMap : IEnumerable<(CoseHeaderLabel Label, ReadOnlyMemory<byte> EncodedValue)>, IEnumerable
{
    public CoseHeaderMap();
    public bool IsReadOnly { get; }
    public ReadOnlyMemory<byte> GetEncodedValue(CoseHeaderLabel label);
    public ReadOnlySpan<byte> GetValueAsBytes(CoseHeaderLabel label);
    public int GetValueAsInt32(CoseHeaderLabel label);
    public string GetValueAsString(CoseHeaderLabel label);
    public bool TryGetEncodedValue(CoseHeaderLabel label, out ReadOnlyMemory<byte> encodedValue);
    public void SetEncodedValue(CoseHeaderLabel label, ReadOnlySpan<byte> encodedValue);
    public void SetEncodedValue(CoseHeaderLabel label, byte[] encodedValue);
    public void SetValue(CoseHeaderLabel label, int value);
    public void SetValue(CoseHeaderLabel label, ReadOnlySpan<byte> value);
    public void SetValue(CoseHeaderLabel label, string value);
    public void Remove(CoseHeaderLabel label);
    public CoseHeaderMap.Enumerator GetEnumerator();
    IEnumerator IEnumerable.GetEnumerator();
    IEnumerator<(CoseHeaderLabel Label, ReadOnlyMemory<byte> EncodedValue)> IEnumerable<(CoseHeaderLabel Label, ReadOnlyMemory<Byte> EncodedValue)>.GetEnumerator();
    public struct Enumerator : IEnumerator<(CoseHeaderLabel Label, ReadOnlyMemory<byte> EncodedValue)>, IEnumerator, IDisposable
    {
        public readonly (CoseHeaderLabel Label, ReadOnlyMemory<byte> EncodedValue) Current { get; }
        object IEnumerator.Current { get; }
        public void Dispose();
        public bool MoveNext();
        public void Reset();
    }
}
public readonly struct CoseHeaderLabel : IEquatable<CoseHeaderLabel>
{
    public CoseHeaderLabel(int label);
    public CoseHeaderLabel(string label);
    public static CoseHeaderLabel Algorithm { get; }
    public static CoseHeaderLabel ContentType { get; }
    public static CoseHeaderLabel CounterSignature { get; }
    public static CoseHeaderLabel Critical { get; }
    public static CoseHeaderLabel IV { get; }
    public static CoseHeaderLabel KeyIdentifier { get; }
    public static CoseHeaderLabel PartialIV { get; }
    public override bool Equals([NotNullWhenAttribute(true)] object? obj);
    public bool Equals(CoseHeaderLabel other);
    public override int GetHashCode();
    public static bool operator ==(CoseHeaderLabel left, CoseHeaderLabel right);
    public static bool operator !=(CoseHeaderLabel left, CoseHeaderLabel right);
}

Jozkee · 2022-06-03T17:41:02Z

The allocations could be avoided by using a backing field that only gets allocated if a caller calls the property getter when the object hadn't been originally created with the map present.

What's wrong with forcing users to call new CoseSigner(key, hash, myProtectedHeaders)?

Since CoseHeaderMap supports its contents being modified, a caller could reuse a CoseSigner to add to the unprotected headers bucket by calling signer.UnprotectedHeaders.SetValue(...) but only if the CoseSigner had originally been created with a non-null unprotected headers bucket. The comment on the IsReadOnly property implies UnprotectedHeaders will be returned with this property false and so allow this kind of modification (if it exists in the CoseSigner).

Allright, that makes sense, we can enable that scenario as you suggest. Just keep in mind that reusing CoseSigner and changing items in the CoseHeaderMaps on async scenarios may incur into race conditions.

kevinmkane · 2022-06-03T20:56:37Z

Allright, that makes sense, we can enable that scenario as you suggest. Just keep in mind that reusing CoseSigner and changing items in the CoseHeaderMaps on async scenarios may incur into race conditions.

I would hope callers of async APIs in general know they shouldn't change an input to an async call while the task is still pending. If you think that might be too error-prone for callers, there's still the option of always making both returned header maps read-only, at the cost of the caller having to make a couple of allocations in order to accomplish the aforementioned scenario. Whatever results in an API that minimizes the chances of callers shooting themselves in the foot!

Jozkee · 2022-06-06T20:04:43Z

Update: addressed feedback from API review.

Added CoseHeaderValue struct.
CoseHeaderMap is now an IDictionary<CoseHeaderLabel, CoseHeaderValue>, see the API usage section to undestand the usability changes this implies.
- The CoseHeaderMap.Enumerator was removed.
Added overloeads for byte[]? associatedData = null where they were missing.
Added missing nullability.
Marked the whole asembly as unsupported in browser.
I attempted to not mark the whole assembly, but since Sign*/Verify* still needs to call RSA/ECDsa.TrySignHash, the unsupported atttibute will still be needed unless we explicitly suppress the warning.

terrajobst · 2022-06-07T19:09:57Z

Video

CoseHeaderMap
- Add IReadOnlyDictionary<CoseHeaderLabel, CoseHeaderValue>

[assembly: System.Runtime.Versioning.UnsupportedOSPlatform("browser")]
namespace System.Security.Cryptography.Cose;

public abstract class CoseMessage
{
    public ReadOnlyMemory<byte>? Content { get; }
    public CoseHeaderMap ProtectedHeaders { get; }
    public CoseHeaderMap UnprotectedHeaders { get; }
    public static CoseSign1Message DecodeSign1(byte[] cborPayload);
    public static CoseSign1Message DecodeSign1(ReadOnlySpan<byte> cborPayload);
    public static CoseMultiSignMessage DecodeMultiSign(byte[] cborPayload);
    public static CoseMultiSignMessage DecodeMultiSign(ReadOnlySpan<byte> cborPayload);
    public byte[] Encode();
    public int Encode(Span<byte> destination);
    public abstract bool TryEncode(Span<byte> destination, out int bytesWritten);
    public abstract int GetEncodedLength();
}

public sealed class CoseSign1Message : CoseMessage
{
    public static byte[] SignDetached(byte[] detachedContent, CoseSigner signer, byte[]? associatedData = null);
    public static byte[] SignDetached(ReadOnlySpan<byte> detachedContent, CoseSigner signer, ReadOnlySpan<byte> associatedData = default);
    public static byte[] SignDetached(Stream detachedContent, CoseSigner signer, ReadOnlySpan<byte> associatedData = default);
    public static Task<byte[]> SignDetachedAsync(Stream detachedContent, CoseSigner signer, ReadOnlyMemory<byte> associatedData = default, CancellationToken cancellationToken = default(CancellationToken));
    
    public static byte[] SignEmbedded(byte[] embeddedContent, CoseSigner signer, byte[]? associatedData = null);
    public static byte[] SignEmbedded(ReadOnlySpan<byte> embeddedContent, CoseSigner signer, ReadOnlySpan<byte> associatedData = default);
    
    public static bool TrySignEmbedded(ReadOnlySpan<byte> embeddedContent, Span<byte> destination, CoseSigner signer, out int bytesWritten, ReadOnlySpan<byte> associatedData = default);
    public static bool TrySignDetached(ReadOnlySpan<byte> detachedContent, Span<byte> destination, CoseSigner signer, out int bytesWritten, ReadOnlySpan<byte> associatedData = default);
    
    public bool VerifyDetached(AsymmetricAlgorithm key, byte[] detachedContent, byte[]? associatedData = null);
    public bool VerifyDetached(AsymmetricAlgorithm key, ReadOnlySpan<byte> detachedContent, ReadOnlySpan<byte> associatedData = default);
    public bool VerifyDetached(AsymmetricAlgorithm key, Stream detachedContent, ReadOnlySpan<byte> associatedData = default);
    public Task<bool> VerifyDetachedAsync(AsymmetricAlgorithm key, Stream detachedContent, ReadOnlyMemory<byte> associatedData = default, CancellationToken cancellationToken = default(CancellationToken));
    public bool VerifyEmbedded(AsymmetricAlgorithm key, byte[]? associatedData = null);
    public bool VerifyEmbedded(AsymmetricAlgorithm key, ReadOnlySpan<byte> associatedData = default);
    
    public override bool TryEncode(Span<byte> destination, out int bytesWritten);
    public override int GetEncodedLength();
}

public sealed class CoseMultiSignMessage : CoseMessage
{
    public static byte[] SignDetached(byte[] detachedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, byte[]? associatedData = null);
    public static byte[] SignDetached(ReadOnlySpan<byte> detachedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlySpan<byte> associatedData = default);
    public static byte[] SignDetached(Stream detachedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlySpan<byte> associatedData = default);
    public static Task<byte[]> SignDetachedAsync(Stream detachedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlyMemory<byte> associatedData = default, CancellationToken cancellationToken = default(CancellationToken));
    public static byte[] SignEmbedded(byte[] embeddedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, byte[]? associatedData = null);
    public static byte[] SignEmbedded(ReadOnlySpan<byte> embeddedContent, CoseSigner signer, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlySpan<byte> associatedData = default);
    
    public static bool TrySignEmbedded(ReadOnlySpan<byte> embeddedContent, CoseSigner signer, out int bytesWritten, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlySpan<byte> associatedData = default);
    public static bool TrySignDetached(ReadOnlySpan<byte> detachedContent, CoseSigner signer, out int bytesWritten, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null, ReadOnlySpan<byte> associatedData = default);

    public ReadOnlyCollection<CoseSignature> Signatures { get; }
    public void AddSignature(CoseSigner signer, byte[]? associatedData = null);
    public void AddSignature(CoseSigner signer, ReadOnlySpan<byte> associatedData = default);
    public void RemoveSignature(CoseSignature signature);
    public void RemoveSignature(int index);

    public override bool TryEncode(Span<byte> destination, out int bytesWritten);
    public override int GetEncodedLength();
}

public sealed class CoseSignature
{
    public CoseHeaderMap ProtectedHeaders { get; }
    public CoseHeaderMap UnprotectedHeaders { get; }
    public bool VerifyEmbedded(AsymmetricAlgorithm key, byte[]? associatedData = null);
    public bool VerifyEmbedded(AsymmetricAlgorithm key, ReadOnlySpan<byte> associatedData = default);
    public bool VerifyDetached(AsymmetricAlgorithm key, byte[] detachedContent, byte[]? associatedData = null);
    public bool VerifyDetached(AsymmetricAlgorithm key, ReadOnlySpan<byte> detachedContent, ReadOnlySpan<byte> associatedData = default);
    public bool VerifyDetached(AsymmetricAlgorithm key, Stream detachedContent, ReadOnlySpan<byte> associatedData = default);
    public Task<bool> VerifyDetachedAsync(AsymmetricAlgorithm key, Stream detachedContent, ReadOnlyMemory<byte> associatedData = default);
}

public sealed class CoseSigner
{
    public CoseSigner(AsymmetricAlgorithm key, HashAlgorithmName hashAlgorithm, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null);
    public CoseSigner(RSA key, RSASignaturePadding signaturePadding, HashAlgorithmName hashAlgorithm, CoseHeaderMap? protectedHeaders = null, CoseHeaderMap? unprotectedHeaders = null);
    public AsymmetricAlgorithm Key { get; }
    public HashAlgorithmName HashAlgorithm { get; }
    public CoseHeaderMap ProtectedHeaders { get; }
    public CoseHeaderMap UnprotectedHeaders { get; }
    public RSASignaturePadding? RSASignaturePadding{ get; }
}

public sealed partial class CoseHeaderMap : IDictionary<CoseHeaderLabel, CoseHeaderValue>
{
    public CoseHeaderValue this[CoseHeaderLabel key] { get; set; }
    public ICollection<CoseHeaderLabel> Keys { get; }
    public ICollection<CoseHeaderValue> Values { get; }
    public int Count { get; }
    public bool IsReadOnly { get; }

    public void Add(CoseHeaderLabel key, CoseHeaderValue value);
    public void Add(KeyValuePair<CoseHeaderLabel, CoseHeaderValue> item);
    public void Clear();
    public bool Contains(KeyValuePair<CoseHeaderLabel, CoseHeaderValue> item);
    public bool ContainsKey(CoseHeaderLabel key);
    public void CopyTo(KeyValuePair<CoseHeaderLabel, CoseHeaderValue>[] array, int arrayIndex);
    public IEnumerator<KeyValuePair<CoseHeaderLabel, CoseHeaderValue>> GetEnumerator();
    public bool Remove(CoseHeaderLabel key);
    public bool Remove(KeyValuePair<CoseHeaderLabel, CoseHeaderValue> item);
    public bool TryGetValue(CoseHeaderLabel key, [MaybeNullWhen(false)] out CoseHeaderValue value);
    IEnumerator IEnumerable.GetEnumerator();

    public void Add(CoseHeaderLabel label, int value);
    public void Add(CoseHeaderLabel label, string value);
    public void Add(CoseHeaderLabel label, byte[] value);
    public void Add(CoseHeaderLabel label, ReadOnlySpan<byte> value);
    public int GetValueAsInt32(CoseHeaderLabel label);
    public string GetValueAsString(CoseHeaderLabel label);
    public byte[] GetValueAsBytes(CoseHeaderLabel label);
    public int GetValueAsBytes(CoseHeaderLabel label, Span<byte> destination);
}

public readonly struct CoseHeaderLabel : IEquatable<CoseHeaderLabel>
{
    public CoseHeaderLabel(int label);
    public CoseHeaderLabel(string label);
    public static CoseHeaderLabel Algorithm { get; }
    public static CoseHeaderLabel ContentType { get; }
    public static CoseHeaderLabel CounterSignature { get; }
    public static CoseHeaderLabel CriticalHeaders { get; }
    public static CoseHeaderLabel KeyIdentifier { get; }

    public override bool Equals([NotNullWhenAttribute(true)] object? obj);
    public bool Equals(CoseHeaderLabel other);
    public override int GetHashCode();
    public static bool operator ==(CoseHeaderLabel left, CoseHeaderLabel right);
    public static bool operator !=(CoseHeaderLabel left, CoseHeaderLabel right);
}

public readonly struct CoseHeaderValue : IEquatable<CoseHeaderValue>
{
    public readonly ReadOnlyMemory<byte> EncodedValue { get; }
    public static CoseHeaderValue FromEncodedValue(byte[] encodedValue);
    public static CoseHeaderValue FromEncodedValue(ReadOnlySpan<byte> encodedValue);
    public static CoseHeaderValue FromInt32(int value);
    public static CoseHeaderValue FromString(string value);
    public static CoseHeaderValue FromBytes(byte[] value);
    public static CoseHeaderValue FromBytes(ReadOnlySpan<byte> value);
    public int GetValueAsInt32();
    public string GetValueAsString();
    public byte[] GetValueAsBytes();
    public int GetValueAsBytes(Span<byte> destination);

    public override bool Equals([NotNullWhenAttribute(true)] object? obj);
    public bool Equals(CoseHeaderValue other);
    public override int GetHashCode();
    public static bool operator ==(CoseHeaderValue left, CoseHeaderValue right);
    public static bool operator !=(CoseHeaderValue left, CoseHeaderValue right);
}

Jozkee · 2022-07-07T19:44:13Z

We also approved the following change via email:

public class CoseMultiSignMessage
{
-    public void AddSignature(CoseSigner signer, byte[]? associatedData = null) { }
-    public void AddSignature(CoseSigner signer, ReadOnlySpan<byte> associatedData) { }
+    public void AddSignatureForEmbedded(CoseSigner signer, byte[]? associatedData = null) { }
+    public void AddSignatureForEmbedded(CoseSigner signer, ReadOnlySpan<byte> associatedData) { }
+    public void AddSignatureForDetached(byte[] detachedContent, CoseSigner signer, byte[]? associatedData = null)
+    public void AddSignatureForDetached(ReadOnlySpan<byte> detachedContent, CoseSigner signer, ReadOnlySpan<byte> associatedData = default)
+    public void AddSignatureForDetached(Stream detachedContent, CoseSigner signer, ReadOnlySpan<byte> associatedData = default)
+    public Task AddSignatureForDetachedAsync(Stream detachedContent, CoseSigner signer, ReadOnlyMemory<byte> associatedData = default, CancellationToken token = default)
}

AddSignature needed to expand to also support detached content scenarios, similar to what we have with the rest of operations in CoseSign1Message and CoseMultiSignMessage types.

Jozkee added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Security blocking Marks issues that we want to fast track in order to unblock other important work labels May 27, 2022

Jozkee added this to the 7.0.0 milestone May 27, 2022

Jozkee self-assigned this May 27, 2022

Jozkee added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jun 1, 2022

terrajobst added api-needs-work API needs work before it is approved, it is NOT ready for implementation and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jun 2, 2022

Jozkee added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-needs-work API needs work before it is approved, it is NOT ready for implementation labels Jun 6, 2022

terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jun 7, 2022

Jozkee mentioned this issue Jun 28, 2022

S.S.C.Cose: Add new MultiSign APIs and address API review feedback on existing ones #71390

Merged

Jozkee closed this as completed in #71390 Jul 7, 2022

bartonjs removed the blocking Marks issues that we want to fast track in order to unblock other important work label Jul 29, 2022

dotnet locked as resolved and limited conversation to collaborators Aug 28, 2022

[API Proposal]: Add APIs for COSE signing #69896

[API Proposal]: Add APIs for COSE signing #69896

Comments

Jozkee commented May 27, 2022 • edited

Background and motivation

API Proposal

API Usage

Decoding and verifying messages

Signing and encoding messages

Append unprotected headers or signatures to already signed messages.

Use custom headers

Supply content via a stream in order to sign large contents.

Sign and Verify with external_aad (Externally supplied data).

Alternative Designs

Risks

ghost commented May 27, 2022

Background and motivation

API Proposal

API Usage

Decoding and verifying messages

Signing and encoding messages

Append unprotected headers or signatures to already signed messages.

Use custom headers

Supply content via a stream in order to sign large contents.

Alternative Designs

Risks

vcsjones commented May 27, 2022 • edited

letmaik commented May 27, 2022 • edited

Jozkee commented May 27, 2022 • edited

kevinmkane commented May 27, 2022

bartonjs commented May 27, 2022

letmaik commented May 27, 2022

vcsjones commented May 27, 2022

bartonjs commented May 27, 2022

kevinmkane commented May 27, 2022

Jozkee commented Jun 1, 2022 • edited

kevinmkane commented Jun 1, 2022

vcsjones commented Jun 1, 2022

bartonjs commented Jun 1, 2022

Jozkee commented Jun 1, 2022 • edited

bartonjs commented Jun 1, 2022

vcsjones commented Jun 1, 2022 • edited

Jozkee commented Jun 1, 2022 • edited

bartonjs commented Jun 1, 2022

bartonjs commented Jun 1, 2022

vcsjones commented Jun 1, 2022

vcsjones commented Jun 1, 2022

bartonjs commented Jun 1, 2022

bartonjs commented Jun 1, 2022 • edited

kevinmkane commented Jun 1, 2022

bartonjs commented Jun 1, 2022

vcsjones commented Jun 1, 2022

bartonjs commented Jun 1, 2022

GrabYourPitchforks commented Jun 1, 2022

Jozkee commented Jun 1, 2022

vcsjones commented Jun 1, 2022 • edited

Jozkee commented Jun 1, 2022 • edited

kevinmkane commented Jun 2, 2022

Jozkee commented Jun 2, 2022

kevinmkane commented Jun 2, 2022

terrajobst commented Jun 2, 2022 • edited by dotnet-api-review bot

Jozkee commented Jun 3, 2022

kevinmkane commented Jun 3, 2022

Jozkee commented Jun 6, 2022

terrajobst commented Jun 7, 2022 • edited by dotnet-api-review bot

Jozkee commented Jul 7, 2022

Jozkee commented May 27, 2022 •

edited

vcsjones commented May 27, 2022 •

edited

letmaik commented May 27, 2022 •

edited

Jozkee commented May 27, 2022 •

edited

Jozkee commented Jun 1, 2022 •

edited

Jozkee commented Jun 1, 2022 •

edited

vcsjones commented Jun 1, 2022 •

edited

Jozkee commented Jun 1, 2022 •

edited

bartonjs commented Jun 1, 2022 •

edited

vcsjones commented Jun 1, 2022 •

edited

Jozkee commented Jun 1, 2022 •

edited

terrajobst commented Jun 2, 2022 •

edited by dotnet-api-review bot

terrajobst commented Jun 7, 2022 •

edited by dotnet-api-review bot