Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support detached payloads in COSESign and COSESign1 #197

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ vendor/

# Editor files
.vscode/
.idea/
1 change: 1 addition & 0 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ var (
ErrEmptySignature = errors.New("empty signature")
ErrInvalidAlgorithm = errors.New("invalid algorithm")
ErrMissingPayload = errors.New("missing payload")
ErrMultiplePayloads = errors.New("multiple payloads")
ErrNoSignatures = errors.New("no signatures attached")
ErrUnavailableHashFunc = errors.New("hash function is not available")
ErrVerification = errors.New("verification error")
Expand Down
85 changes: 77 additions & 8 deletions sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,12 +398,37 @@ func (m *SignMessage) UnmarshalCBOR(data []byte) error {
// Notice: The COSE Sign API is EXPERIMENTAL and may be changed or removed in a
// later release.
func (m *SignMessage) Sign(rand io.Reader, external []byte, signers ...Signer) error {
return m.sign(rand, nil, external, signers)
}

// SignDetached signs a SignMessage using the provided signers corresponding to the
// signatures.
//
// See `Signature.Sign()` for advanced signing scenarios.
//
// Reference: https://datatracker.ietf.org/doc/html/rfc8152#section-4.4
//
// # Experimental
//
// Notice: The COSE Sign API is EXPERIMENTAL and may be changed or removed in a
// later release.
func (m *SignMessage) SignDetached(rand io.Reader, detached, external []byte, signers ...Signer) error {
if detached == nil {
return ErrMissingPayload
}
return m.sign(rand, detached, external, signers)
}

func (m *SignMessage) sign(rand io.Reader, detached, external []byte, signers []Signer) error {
if m == nil {
return errors.New("signing nil SignMessage")
}
if m.Payload == nil {
return ErrMissingPayload

payload, err := checkPayload(m.Payload, detached)
if err != nil {
return err
}

switch len(m.Signatures) {
case 0:
return ErrNoSignatures
Expand All @@ -415,14 +440,14 @@ func (m *SignMessage) Sign(rand io.Reader, external []byte, signers ...Signer) e

// populate common parameters
var protected cbor.RawMessage
protected, err := m.Headers.MarshalProtected()
protected, err = m.Headers.MarshalProtected()
if err != nil {
return err
}

// sign message accordingly
for i, signature := range m.Signatures {
if err := signature.Sign(rand, signers[i], protected, m.Payload, external); err != nil {
if err := signature.Sign(rand, signers[i], protected, payload, external); err != nil {
return err
}
}
Expand All @@ -443,12 +468,41 @@ func (m *SignMessage) Sign(rand io.Reader, external []byte, signers ...Signer) e
// Notice: The COSE Sign API is EXPERIMENTAL and may be changed or removed in a
// later release.
func (m *SignMessage) Verify(external []byte, verifiers ...Verifier) error {
return m.verify(nil, external, verifiers...)
}

// VerifyDetached verifies the signatures on the SignMessage against the corresponding
// verifier, returning nil on success or a suitable error if verification fails.
//
// Returns ErrMissingPayload if detached is nil, and ErrMultiplePayloads if the SignMessage
// also has an attached payload.
//
// See `Signature.Verify()` for advanced verification scenarios like threshold
// policies.
//
// Reference: https://datatracker.ietf.org/doc/html/rfc8152#section-4.4
//
// # Experimental
//
// Notice: The COSE Sign API is EXPERIMENTAL and may be changed or removed in a
// later release.
SteveLasker marked this conversation as resolved.
Show resolved Hide resolved
func (m *SignMessage) VerifyDetached(detached, external []byte, verifiers ...Verifier) error {
if detached == nil {
return ErrMissingPayload
}
return m.verify(detached, external, verifiers...)
SteveLasker marked this conversation as resolved.
Show resolved Hide resolved
}

func (m *SignMessage) verify(detached, external []byte, verifiers ...Verifier) error {
if m == nil {
return errors.New("verifying nil SignMessage")
}
if m.Payload == nil {
return ErrMissingPayload

payload, err := checkPayload(m.Payload, detached)
if err != nil {
return err
}

switch len(m.Signatures) {
case 0:
return ErrNoSignatures
Expand All @@ -460,16 +514,31 @@ func (m *SignMessage) Verify(external []byte, verifiers ...Verifier) error {

// populate common parameters
var protected cbor.RawMessage
protected, err := m.Headers.MarshalProtected()
protected, err = m.Headers.MarshalProtected()
if err != nil {
return err
}

// verify message accordingly
for i, signature := range m.Signatures {
if err := signature.Verify(verifiers[i], protected, m.Payload, external); err != nil {
if err := signature.Verify(verifiers[i], protected, payload, external); err != nil {
return err
}
}
return nil
}

// checkPayload checks that exactly one of payload and detached is non-nil.
func checkPayload(payload, detached []byte) ([]byte, error) {
if detached != nil {
if payload != nil {
return nil, ErrMultiplePayloads
}
return detached, nil
}

if payload == nil {
return nil, ErrMissingPayload
}
return payload, nil
}
118 changes: 106 additions & 12 deletions sign1.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,26 +87,49 @@ func (m *Sign1Message) UnmarshalCBOR(data []byte) error {
//
// Reference: https://datatracker.ietf.org/doc/html/rfc8152#section-4.4
func (m *Sign1Message) Sign(rand io.Reader, external []byte, signer Signer) error {
return m.sign(rand, nil, external, signer)
}

// SignDetached signs a Sign1Message using the provided Signer.
SteveLasker marked this conversation as resolved.
Show resolved Hide resolved
// The signature is stored in m.Signature.
//
// Note that m.Signature is only valid as long as m.Headers.Protected
// remains unchanged after calling this method.
// It is possible to modify m.Headers.Unprotected after signing,
// i.e., add counter signatures or timestamps.
//
// Reference: https://datatracker.ietf.org/doc/html/rfc8152#section-4.4
func (m *Sign1Message) SignDetached(rand io.Reader, detached, external []byte, signer Signer) error {
if detached == nil {
return ErrMissingPayload
}
return m.sign(rand, detached, external, signer)
}

func (m *Sign1Message) sign(rand io.Reader, detached, external []byte, signer Signer) error {
if m == nil {
return errors.New("signing nil Sign1Message")
}
if m.Payload == nil {
return ErrMissingPayload

payload, err := checkPayload(m.Payload, detached)
if err != nil {
return err
}

if len(m.Signature) > 0 {
return errors.New("Sign1Message signature already has signature bytes")
}

// check algorithm if present.
// `alg` header MUST be present if there is no externally supplied data.
alg := signer.Algorithm()
err := m.Headers.ensureSigningAlgorithm(alg, external)
err = m.Headers.ensureSigningAlgorithm(alg, external)
if err != nil {
return err
}

// sign the message
toBeSigned, err := m.toBeSigned(external)
toBeSigned, err := m.toBeSigned(external, payload)
if err != nil {
return err
}
Expand All @@ -124,26 +147,44 @@ func (m *Sign1Message) Sign(rand io.Reader, external []byte, signer Signer) erro
//
// Reference: https://datatracker.ietf.org/doc/html/rfc8152#section-4.4
func (m *Sign1Message) Verify(external []byte, verifier Verifier) error {
return m.verify(nil, external, verifier)
}

// VerifyDetached verifies the signature on the Sign1Message returning nil on
// success or a suitable error if verification fails.
//
// Reference: https://datatracker.ietf.org/doc/html/rfc8152#section-4.4
func (m *Sign1Message) VerifyDetached(detached, external []byte, verifier Verifier) error {
if detached == nil {
return ErrMissingPayload
}
return m.verify(detached, external, verifier)
}

func (m *Sign1Message) verify(detached, external []byte, verifier Verifier) error {
if m == nil {
return errors.New("verifying nil Sign1Message")
}
if m.Payload == nil {
return ErrMissingPayload

payload, err := checkPayload(m.Payload, detached)
if err != nil {
return err
}

if len(m.Signature) == 0 {
return ErrEmptySignature
}

// check algorithm if present.
// `alg` header MUST present if there is no externally supplied data.
alg := verifier.Algorithm()
err := m.Headers.ensureVerificationAlgorithm(alg, external)
err = m.Headers.ensureVerificationAlgorithm(alg, external)
if err != nil {
return err
}

// verify the message
toBeSigned, err := m.toBeSigned(external)
toBeSigned, err := m.toBeSigned(external, payload)
if err != nil {
return err
}
Expand All @@ -153,7 +194,7 @@ func (m *Sign1Message) Verify(external []byte, verifier Verifier) error {
// toBeSigned constructs Sig_structure, computes and returns ToBeSigned.
//
// Reference: https://datatracker.ietf.org/doc/html/rfc8152#section-4.4
func (m *Sign1Message) toBeSigned(external []byte) ([]byte, error) {
func (m *Sign1Message) toBeSigned(external []byte, payload []byte) ([]byte, error) {
// create a Sig_structure and populate it with the appropriate fields.
//
// Sig_structure = [
Expand All @@ -178,7 +219,7 @@ func (m *Sign1Message) toBeSigned(external []byte) ([]byte, error) {
"Signature1", // context
protected, // body_protected
external, // external_aad
m.Payload, // payload
payload, // payload
}

// create the value ToBeSigned by encoding the Sig_structure to a byte
Expand Down Expand Up @@ -238,7 +279,7 @@ func (m *Sign1Message) doUnmarshal(data []byte) error {
// This method is a wrapper of `Sign1Message.Sign()`.
//
// Reference: https://datatracker.ietf.org/doc/html/rfc8152#section-4.4
func Sign1(rand io.Reader, signer Signer, headers Headers, payload []byte, external []byte) ([]byte, error) {
func Sign1(rand io.Reader, signer Signer, headers Headers, payload, external []byte) ([]byte, error) {
msg := Sign1Message{
Headers: headers,
Payload: payload,
Expand All @@ -250,6 +291,22 @@ func Sign1(rand io.Reader, signer Signer, headers Headers, payload []byte, exter
return msg.MarshalCBOR()
}

// Sign1Detached signs a Sign1Message using the provided Signer.
//
// This method is a wrapper of `Sign1Message.SignDetached()`.
//
// Reference: https://datatracker.ietf.org/doc/html/rfc8152#section-4.4
func Sign1Detached(rand io.Reader, signer Signer, headers Headers, detached, external []byte) ([]byte, error) {
msg := Sign1Message{
Headers: headers,
}
err := msg.SignDetached(rand, detached, external, signer)
if err != nil {
return nil, err
}
return msg.MarshalCBOR()
}
Comment on lines +299 to +308
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need

func (m *SignMessage) SignDetached(rand io.Reader, detached, external []byte, signers ...Signer) error
func (m *Sign1Message) SignDetached(rand io.Reader, detached, external []byte, signer Signer) error

?

Can Sign1Detached be implemented as

Suggested change
func Sign1Detached(rand io.Reader, signer Signer, headers Headers, detached, external []byte) ([]byte, error) {
msg := Sign1Message{
Headers: headers,
}
err := msg.SignDetached(rand, detached, external, signer)
if err != nil {
return nil, err
}
return msg.MarshalCBOR()
}
func Sign1Detached(rand io.Reader, signer Signer, headers Headers, detached, external []byte) ([]byte, error) {
msg := Sign1Message{
Headers: headers,
Payload: detached,
}
err := msg.Sign(rand, external, signer)
if err != nil {
return nil, err
}
msg.Payload = nil
return msg.MarshalCBOR()
}

?

Same question for SignDetached.

Copy link
Contributor Author

@alex-richards alex-richards Aug 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fussy to me, would making detached the base case and calling from Sign1 be ok? Saves unsetting the payload on the detached object.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would making detached the base case and calling from Sign1 be ok?

IMO yes. @shizhMSFT wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shizhMSFT, I believe this is the blocking issue on this PR. Can we consider refactoring the implementation, either in this PR, or a separate PR? Either way, can we come to some conclusion here to move forward?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alex-richards Are you suggesting

func sign1(rand io.Reader, signer Signer, headers Headers, payload, external []byte) (*Sign1Message, error) {
	msg := Sign1Message{
		Headers: headers,
		Payload: payload,
	}
	err := msg.Sign(rand, external, signer)
	if err != nil {
		return nil, err
	}
	return msg, nil
}

func Sign1(rand io.Reader, signer Signer, headers Headers, payload, external []byte) ([]byte, error) {
	msg, err := sign1(rand, signer, headers, payload, external)
	if err != nil {
		return nil, err
	}
	return msg.MarshalCBOR()
}

func Sign1Detached(rand io.Reader, signer Signer, headers Headers, detached, external []byte) ([]byte, error) {
	msg, err := sign1(rand, signer, headers, detached, external)
	if err != nil {
		return nil, err
	}
	msg.Payload = nil
	return msg.MarshalCBOR()
}

That refactoring works for me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alex-richards, @shizhMSFT
Can this comment be resoled?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SteveLasker I think it will be better if we gather more inputs from other maintainers.


type UntaggedSign1Message Sign1Message

// MarshalCBOR encodes UntaggedSign1Message into a COSE_Sign1 object.
Expand Down Expand Up @@ -293,6 +350,19 @@ func (m *UntaggedSign1Message) Sign(rand io.Reader, external []byte, signer Sign
return (*Sign1Message)(m).Sign(rand, external, signer)
}

// SignDetached signs an UntaggedSign1Message using the provided Signer.
// The signature is stored in m.Signature.
//
// Note that m.Signature is only valid as long as m.Headers.Protected
// remains unchanged after calling this method.
// It is possible to modify m.Headers.Unprotected after signing,
// i.e., add counter signatures or timestamps.
//
// Reference: https://datatracker.ietf.org/doc/html/rfc8152#section-4.4
func (m *UntaggedSign1Message) SignDetached(rand io.Reader, detached, external []byte, signer Signer) error {
return (*Sign1Message)(m).SignDetached(rand, detached, external, signer)
}

// Verify verifies the signature on the UntaggedSign1Message returning nil on success or
// a suitable error if verification fails.
//
Expand All @@ -301,12 +371,20 @@ func (m *UntaggedSign1Message) Verify(external []byte, verifier Verifier) error
return (*Sign1Message)(m).Verify(external, verifier)
}

// VerifyDetached verifies the signature on the UntaggedSign1Message returning
// nil on success or a suitable error if verification fails.
//
// Reference: https://datatracker.ietf.org/doc/html/rfc8152#section-4.4
func (m *UntaggedSign1Message) VerifyDetached(detached, external []byte, verifier Verifier) error {
return (*Sign1Message)(m).VerifyDetached(detached, external, verifier)
}

// Sign1Untagged signs an UntaggedSign1Message using the provided Signer.
//
// This method is a wrapper of `UntaggedSign1Message.Sign()`.
//
// Reference: https://datatracker.ietf.org/doc/html/rfc8152#section-4.4
func Sign1Untagged(rand io.Reader, signer Signer, headers Headers, payload []byte, external []byte) ([]byte, error) {
func Sign1Untagged(rand io.Reader, signer Signer, headers Headers, payload, external []byte) ([]byte, error) {
msg := UntaggedSign1Message{
Headers: headers,
Payload: payload,
Expand All @@ -317,3 +395,19 @@ func Sign1Untagged(rand io.Reader, signer Signer, headers Headers, payload []byt
}
return msg.MarshalCBOR()
}

// Sign1UntaggedDetached signs an UntaggedSign1Message using the provided Signer.
//
// This method is a wrapper of `UntaggedSign1Message.SignDetached()`.
//
// Reference: https://datatracker.ietf.org/doc/html/rfc8152#section-4.4
func Sign1UntaggedDetached(rand io.Reader, signer Signer, headers Headers, detached, external []byte) ([]byte, error) {
msg := UntaggedSign1Message{
Headers: headers,
}
err := msg.SignDetached(rand, detached, external, signer)
if err != nil {
return nil, err
}
return msg.MarshalCBOR()
}
2 changes: 1 addition & 1 deletion sign1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ func TestSign1Message_toBeSigned(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := tt.m.toBeSigned(tt.external)
got, err := tt.m.toBeSigned(tt.external, tt.m.Payload)
if (err != nil) != tt.wantErr {
t.Errorf("Sign1Message.toBeSigned() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down
Loading
Loading