From 665e28ab2e434f9d30de8b96fd3323c808ec5799 Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Tue, 2 Aug 2022 19:08:20 -0400 Subject: [PATCH] Introduce Domain Separation FIP --- FIPS/fip-0043.md | 128 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 FIPS/fip-0043.md diff --git a/FIPS/fip-0043.md b/FIPS/fip-0043.md new file mode 100644 index 000000000..882e25ea5 --- /dev/null +++ b/FIPS/fip-0043.md @@ -0,0 +1,128 @@ +--- +fip: FIP-0043 +title: Signature Domain Separation +author: Geoff Stuart (@geoff-vball), Aayush Rajesekaran (@arajasek) +discussions-to: https://github.com/filecoin-project/FIPs/discussions/387 +status: Draft +type: Technical +category: Core +created: 2022-08-02 +specs-sections: 4.5.1 Signatures +--- + +## Simple Summary +Add a tag data before signing to indicate what type of data the serialized bytes correspond to. +This prevents data from being de-serialized into the wrong data type. + +## Abstract +All signed data on the Filecoin network is first serialized to a byte-array using `cbor` encoding. +A signed byte-array could potentially be deserialized into a different data type, which could be used maliciously. + +Imagine User **A** signs a payment channel voucher and sends it to a storage provider, +but the malicious storage provider submits the signed data to the Consensus Fault method, +which decodes the data as an invalid block header and slashes User **A**. This FIP plans to prevent +these possible collisions between data types. + +This proposal adds a fixed-length prefix to the data before signing which corresponds to the data type, +allowing us to reject deserializations into the wrong format. + +## Change Motivation +Currently, all signed data types on the Filecoin Network currently have implicit domain separation by coincidence. +The metadata at the start of each `cbor` encoding used on the network currently all differ, making it impossible to +deserialize any of them into the wrong type, which is why this FIP has not yet been implemented. +The motivation for the timing of this FIP is to allow users to safely sign arbitrary data with +the upcoming release of programmable contracts on the FVM. + +## Specification +We can add the tag when signing and verifying signatures, but do not need to transmit the tag, +as we can infer it based on the type of data we're signing or verifying. +Most calls to `walletSign()` include `MsgMeta` from `api_wallet.go` which contains +`MsgType` which is already implemented for `dealproposal`, `block`, and `message`. +We can extend the use of `MsgMeta` to include all other data types, and +use this metadata to know which tag to include when signing. + +We propose using a single tag for all user-generated data that might be used in a smart contract. +This prevents us from having to deal with an entire "user-space" for tags. +Smart contracts will be free to implement their own tagging system below the +top level tag if desired. + +We already have a domain separation tag for `RemoveDataCapProposal` structs. +Below is simplified code of the current implementation: + +### Signing +```go +const SignatureDomainSeparation_RemoveDataCap = "fil_removedatacap:" + +params := verifregtypes.RemoveDataCapProposal{...} + +paramBuf := new(bytes.Buffer) +paramBuf.WriteString(SignatureDomainSeparation_RemoveDataCap) +err = params.MarshalCBOR(paramBuf) + +signature, err := api.WalletSign(ctx, verifier, paramBuf.Bytes()) +``` + +### Verifying +```go +func removeDataCapRequestIsValidOrAbort(...) { + proposal := RemoveDataCapProposal{...} + + buf := bytes.Buffer{} + buf.WriteString(SignatureDomainSeparation_RemoveDataCap) + err := proposal.MarshalCBOR(&buf) + + err := rt.VerifySignature(VerifierSignature, VerifierAddress, buf.Bytes()); err != nil { + rt.Abortf(...) + } +} +``` + +### Data Types +A (hopefully) exhaustive list of the different types of data we sign. + +- Messages +- Block Headers +- Deal Proposals +- Storage Ask +- Payment Channel Vouchers +- VRFs (Already uses int64 as DST) +- Remove Verified Data Cap (Already uses DST) +- BLS Aggregation (Do we run into problems here?) + + +## Design Rationale +This FIP details what we believe to be these simplest and most robust way to prevent +unintentional and malicious collisions between message types. A fixed-length tag was chosen +because variable-length tags may not offer total collision resistance. + +## Backwards Compatibility +We propose doing this upgrade in a 2-step process. In the first step, we will sign all +new messages with the DS Tag, but will verify messages with or without the tag. This +will allow for any messages signed before the upgrade to still be valid after, and will +also give 3rd party wallets a chance to update to use the new tags. + +In a later network upgrade, we will enforce the use of tags in the verification process, +invalidating any messages signed without tags. There is one potential exception: +payment channel vouchers. These are designed to be signed messages that live off chain +and have no expiry. We must be very careful if we want to invalidate any payment channel vouchers. + +## Test Cases +For each data type, we should have test cases for valid data with and without tags. +We should also include test cases where the incorrect tag is supplied for each data type. + +## Security Considerations +This FIP should be preventing a security concern that might be introduced by the FVM. +This FIP should not create any new security concerns. + +## Other Considerations/ Open Questions +This FIP will have to be implemented by 3rd party wallets between steps 1 and 2. +The timing of step 2 can be adjusted if we need more time to strengthen compliance. + +BLS aggregation may cause issues. `VerifyBlsAggregate()` may not being able to accept +both tagged and untagged messages in the same aggregation. + +## Implementation +To be provided before "Final" status. + +## Copyright +Copyright and related rights waived via [CC0](https://creativecommons.org/publicdomain/zero/1.0/).