veraison / go-cose

go library for CBOR Object Signing and Encryption (COSE)
Mozilla Public License 2.0
49 stars 26 forks source link

Detached signature verification fails with "missing payload" error #150

Closed henrylyons closed 5 months ago

henrylyons commented 1 year ago

What is the areas you experience the issue in?

Go-COSE Library

What is not working as expected?

Sign1Message.Verify does not support verification of detached signatures, where the payload is in a different file, and the CoseSign1Message payload field is null. Calling Verify on a message with a null payload returns the following error: "missing payload".

The COSE RFC supports this scenario (https://datatracker.ietf.org/doc/html/rfc8152#page-8): "The content may be detached, but the location is still used. The content is wrapped in a bstr when present and is a nil value when detached."

This issue can be worked around by appending the payload bytes to the Sign1Message before calling Verify, but it would be preferable to have the library support this scenario directly.

What did you expect to happen?

The go-cose library has functionality to verify detached CoseSign1 signatures, where the payload is from an external source (e.g., different file).

How can we reproduce it?

The issue can be repro'd with the payload file (manifest.spdx.json) and COSE signature file (manifest.spdx.cose) in this package: go-cose-repro.zip

Describe your environment

vscode on Windows 11.

What is the version of your Go-COSE Library?

require github.com/veraison/go-cose v1.0.0

qmuntal commented 1 year ago

In the last community meeting we discussed about this issue. The consens is that supporting detached signatures should be handled as a new feature, not as a bug. We might want to provide a dedicated API and also should consider edge cases.

@yogeshbdeshpande and @thomas-fossati will investigate it.

OR13 commented 1 year ago

See https://github.com/search?q=repo%3Apanva%2Fjose%20detached&type=code

SteveLasker commented 1 year ago

Triage: @qmuntal to create a new issue with a proposal.

qmuntal commented 1 year ago

Here is the new API proposed:

// SignDetached is like [Sign1Message.Sign] but using payload instead of m.Payload.
func (m *Sign1Message) SignDetached(rand io.Reader, payload []byte, external []byte, signer Signer) error

// VerifyDetached is like [Sign1Message.Verify] but using payload instead of m.Payload.
func (m *Sign1Message) VerifyDetached(payload []byte, external []byte, verifier Verifier) error

// SignDetached is like [Sign1Message.Sign] but using payload instead of m.Payload.
func (m *UntaggedSign1Message) SignDetached(rand io.Reader, payload []byte, external []byte, signer Signer) error

// VerifyDetached is like [Sign1Message.Verify] but using payload instead of m.Payload.
func (m *UntaggedSign1Message) VerifyDetached(payload []byte, external []byte, verifier Verifier) error

// SignDetached is like [SignMessage.Sign] but using payload instead of m.Payload.
func (m *SignMessage) SignDetached(rand io.Reader, payload []byte, external []byte, signers ...Signer) error

// VerifyDetached is like [SignMessage.Verify] but using payload instead of m.Payload.
func (m *SignMessage) VerifyDetached(payload []byte, external []byte, verifiers ...Verifier) error

The payload parameter can't be nil, else these functions will return cose.ErrMissingPayload.

OR13 commented 1 year ago

The parameter name external could be better.

I'm wondering how to set protected headers, I assume that happens before this is called.

You might also consider exposing some generic transformers for attaching and detaching existing envelope payloads.

SteveLasker commented 1 year ago

@qmuntal, thanks. Looks like we're ready to proceed with an implementation based on the above.

SteveLasker commented 6 months ago

Poking to see if we want to complete this, or close. We'll close by March 8th if not resolved.

SteveLasker commented 5 months ago

Closing, happy to see this re-opened with a PR.