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

[Malleability] CollectionGuarantee #6722

Open
Tracked by #6647
UlyanaAndrukhiv opened this issue Nov 14, 2024 · 0 comments
Open
Tracked by #6647

[Malleability] CollectionGuarantee #6722

UlyanaAndrukhiv opened this issue Nov 14, 2024 · 0 comments

Comments

@UlyanaAndrukhiv
Copy link
Contributor

UlyanaAndrukhiv commented Nov 14, 2024

CollectionGuarantee Malleability

// CollectionGuarantee is a signed hash for a collection, which is used
// to announce collections to consensus nodes.
type CollectionGuarantee struct {
CollectionID Identifier // ID of the collection being guaranteed
ReferenceBlockID Identifier // defines expiry of the collection
ChainID ChainID // the chainID of the cluster in order to determine which cluster this guarantee belongs to
SignerIndices []byte // encoded indices of the signers
Signature crypto.Signature // guarantor signatures
}

The current CollectionGuarantee implementation uses the ID() method to return the CollectionID as the unique identifier:

func (cg *CollectionGuarantee) ID() Identifier {
return cg.CollectionID
}

This method assumes that CollectionID is sufficient to uniquely identify a CollectionGuarantee. While this approach is simple, it introduces potential malleability concerns. The identifier computation currently only considers the CollectionID field, leaving out important fields like:

  • ReferenceBlockID,
  • ChainID,
  • SignerIndices,
  • Signature

This omission means that two CollectionGuarantee instances with the same CollectionID but differing values for these fields would produce the same identifier, which is incorrect.

Proposed Solution

  1. Separate mutable and immutable fields:
    Create a CollectionGuaranteeBody struct containing the immutable fields:
  • CollectionID
  • ReferenceBlockID,
  • ChainID.
  1. Implement ID() for CollectionGuaranteeBody:
    Use the MakeID() function to compute the identifier for CollectionGuaranteeBody. All fields will be encoded consistently:
  • Primitive and byte array fields (CollectionID, ReferenceBlockID, ChainID) are encoded directly as raw bytes using RLP (encoding RLP rules).
  1. Update theCollectionGuarantee struct:
    Include the CollectionGuaranteeBody struct within CollectionGuarantee.
type CollectionGuarantee struct {
	Body             CollectionGuaranteeBody
	SignerIndices    []byte           // encoded indices of the signers
	Signature        crypto.Signature // guarantor signatures
}
  1. Update ID() for CollectionGuarantee:
    The ID() method will compute the identifier based on the entire CollectionGuarantee struct using MakeID().
func (cg *CollectionGuarantee) ID() Identifier {
	body := struct {
		BodyID         flow.Identifier
		SignerIndices    []byte          
	        Signature        crypto.Signature
	}{
		BodyID:         cg.Body.ID(),
		SignerIndices:   cg.SignerIndices,
		Signature:     cg.Signature,
	}
	return MakeID(body)
}

By encoding all fields, this approach guarantees that the resulting ID is malleability-resistant. Changes to any field of CollectionGuarantee will produce a different Identifier.

  1. Remove unused function: The unused Checksum() function will be removed.

Potential problems

  • During the signing process of CollectionGuarantee : The Signature field is mutable and should not contribute to the signature. Only the immutable fields (CollectionID, ReferenceBlockID, ChainID) should be signed. The Signature forCollectionGuarantee is not implemented yet, but in case that it will be implemented, CollectionGuarantee struct will require refactoring to separate mutable and immutable fields (this refactoring is already described in 1- 4). Such a change would simplify the process of signing the CollectionGuarantee.

Definition of Done

  1. The CollectionGuaranteeBody has been created and CollectionGuaranteeBody.ID() method has been implemented.
  2. The CollectionGuarantee.ID() method has been updated using MakeID.
  3. The unused Checksum() functions has been removed.
  4. Unit tests have been updated to validate the new behavior, ensuring identifiers change as expected when data is modified.
  5. Documentation and comments have been updated to reflect the changes and clarify the purpose of the ID() method.
@UlyanaAndrukhiv UlyanaAndrukhiv changed the title [Malleability C] CollectionGuarantee [Malleability] CollectionGuarantee Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant