Define signature aggregation API and implement it using BLS #42

matejpavlovic · 2024-01-18T00:36:33Z

This is a proposition of an API for the aggregated signatures. In a nutshell, each participant would have an instance of a signature scheme, initialized with its own private key and the power table.

In addition to simple signing and verifying other participant's signatures, the scheme can produce and verify aggregated signatures. Each aggregated signature carries information about the identities of the participants whose signatures it aggregates.

WDYT about removing the Signer interface from the host and using a separate AggSignatureScheme that can be passed to the participant the same way as the VRFer is?

matejpavlovic · 2024-01-18T09:51:13Z

At this stage, the legacy BLS implementation vulnerable to rouge key attack is used. The plan is to fix that before merging. There is a TODO in the code here:
https://github.com/matejpavlovic/go-f3/blob/6026a991a1433d934ff78759f625381223cb8b69/blssignatures/sigscheme.go#L10-L15

Also, a relevant Slack thread: https://filecoinproject.slack.com/archives/C0556MSR945/p1705514311818349

Kubuxu · 2024-01-18T13:02:50Z

Suggestion for my side: separate the signing backend and the pub key resolution.
My initial plan was to use Lotus's APIs for signing. This is the easiest way to access the worker key. It also already supports key delegation and hardware key custody. This means that we are limited to the scheme used by Filecoin today.

If that proves not enough performance-wise, we can implement an alternative scheme, but it will be an increase in scope as it will require "extracting" the key and key format out of lotus.

matejpavlovic · 2024-01-18T17:21:20Z

Ok that makes sense. In that case let's only keep the creation and verification of the aggregate signatures in f3. It would be useful to know exactly what kind of signatures are coming out of the Lotus API. I'll adapt the code in this PR accordingly. But I'd still find it more intuitive if the Signer API didn't require the signer's identity for signing. This is not required under normal circumstances, since the signer already having (directly or indirectly) access to the private key, is aware of the signer's identity anyway.

ranchalp · 2024-01-18T17:25:23Z

IMO this should be 2 PRs, one for the refactoring of the interfaces and one for the BLS implementation. If the one for the implementation bases on the refactoring, then I think both should be merged after #37 is merged. Otherwise BLS can be merged first and the refactoring come after that and #37 's merge.

matejpavlovic · 2024-01-18T17:43:12Z

That's fine with me. Le's get #35 and #37 merged soon then.

Kubuxu · 2024-01-18T18:37:42Z

The code used by lotus: https://github.com/filecoin-project/bls-signatures/blob/master/src/signature.rs

Signed-off-by: Matej Pavlovic <[email protected]>

matejpavlovic · 2024-01-23T16:37:38Z

Ok, here is a new implementation of BLS signature aggregation (the rouge-key-attack-resistant version), including some unit tests. I separated the signer and the aggregator, so the signer can be easily externalized.

There is one potential caveat though: The underlying drand/kyber implementation of the rouge-key-attack-resistant version of the aggregation only supports keys on the G2 curve. If F3 was in control of key generation, this would not be an issue, it would just use a setup with keys on G2. If I remember correctly, however, F3 is not in control of the BLS scheme parametrization. I skimmed over the code @Kubuxu pointed to, but it was not obvious to me, which curve it uses for signatures (I don't speak Rust yet). Thus, we're now in one of 2 possible situations:

The keys and signatures happen to be on the right curve - we're lucky and can use the code as-is.
The the keys and signatures are on the opposite curves. In this case, we can add support for those in the drand/kyber library. I asked Yolan from Drand and he said that it should be trivial (it's just that nobody bothered implementing it yet).

Kubuxu · 2024-01-23T17:00:34Z

In filecoin the signature is over G2

ranchalp · 2024-01-25T19:31:18Z

@matejpavlovic they are merged.

ranchalp · 2024-01-18T13:29:51Z

f3/api.go

+	// Sign signs a message and returns the signature.
+	Sign(msg []byte) ([]byte, error)
+
+	// Verify verifies a signature sig of message msg produced by participant with ActorID.


Unlike VerifyAggregate, Verify here assumes knowledge of the scheme of the mapping from ActorID to public key. Change it so that it takes the public key as a parameter, as mentioned by @anorth here

ranchalp · 2024-01-18T13:35:04Z

blssignatures/aggregatesig.go

+type AggSignature struct {
+	blsScheme     *BLSScheme
+	Sig           []byte
+	SignerIndexes []int


we have been using bitfield.Bitfield here (at least when serializing) to benefit from run-length encoding.

ranchalp · 2024-01-26T19:04:13Z

blssignatures/aggregator.go

+	blsScheme  *BLSAggScheme
+	sigs       []indexedSig
+	signerMask *sign.Mask


Unclear here what sigs and signerMask really are, comment.

Does sign.Mask use run-length encoding or how is the list of signers stored/marshaled?

ranchalp · 2024-01-26T19:07:14Z

blssignatures/helpertypes.go

+type indexedSig struct {
+	index int
+	sig   []byte
+}


Comment usage here or in SigAggregator at least.

ranchalp · 2024-01-26T19:08:43Z

blssignatures/helpertypes.go

+type indexedSig struct {
+	index int
+	sig   []byte
+}
+
+type aggSigData struct {
+	Sig  []byte
+	Mask []byte
+}


I think aggSigData can just be removed and only the Sig []byte be used. granite.go already keeps track of the signatures being aggregated in the signers bitfield.Bitfield of QuorumSignature. Both are redundant, and I do not see a strong reason to pair together in the serialization of the signature the list of signers. if we need a sign.Mask to make calls to the bdn package, we can just locally create it. Specially if sign.Mask does not serialize as efficiently as bitfield.Bitfield does with RLE.

Also, keep in mind that the list of signers should be easily accessible from Granite's quorumState to check for the existence of a strong quorum in the aggregated signature, and this need not be necessarily performed before the deserialization/verification of the aggregated signature.

So I would rather a verification interface that explicitly takes the list of signers than one that keeps them serialized within the aggregated signature. The bitfield of signers comes sorted by voting power anyways so it should be ok for verification/aggregation AFAIK.

ranchalp · 2024-01-26T19:15:12Z

blssignatures/aggscheme.go

+		return nil
+	}
+
+	aggPubKey, err := bdn.AggregatePublicKeys(s.suite, mask)


This uses G2? Do we use G2 for public keys?

bdn right now supports only G2 pubkeys, we will have to write support for G1 in kyber

ranchalp · 2024-01-26T19:21:14Z

f3/api.go

+	// as this AggSignatureScheme, otherwise the behavior of VerifyAggSig is undefined.
+	// VerifyAggSig returns nil if sig is a valid aggregate signature of msg
+	// (with respect to the associated power table) and a non-nil error otherwise.
+	VerifyAggSig(msg []byte, sig []byte) error


As mentioned, I think there would be benefits from not embedding together list of signers and aggregated signature into the sig []byte.

That would be very surprising! Surely a verification API must take the public keys as parameter.

anorth · 2024-01-29T22:11:33Z

f3/api.go

 	// Verifies a signature for the given sender ID.
-	Verify(sender ActorID, msg, sig []byte) bool
+	Verify(sender ActorID, msg, sig []byte) error


Upstream changes have separating signing (which has implicit knowledge of key) from verification (pure stateless function which takes key as parameter). Please follow that pattern.

anorth · 2024-01-29T22:12:12Z

f3/api.go

+}
+
+// AggSignatureScheme encapsulates signature aggregation functionality.
+// The implementation is expected to be aware of a mapping between ActorIDs and public keys.


As above, I don't think we want this implicit awareness. The API should be purely in terms of keys.

anorth · 2024-01-29T22:13:01Z

f3/api.go

+	// as this AggSignatureScheme, otherwise the behavior of VerifyAggSig is undefined.
+	// VerifyAggSig returns nil if sig is a valid aggregate signature of msg
+	// (with respect to the associated power table) and a non-nil error otherwise.
+	VerifyAggSig(msg []byte, sig []byte) error


That would be very surprising! Surely a verification API must take the public keys as parameter.

anorth · 2024-01-29T22:14:27Z

f3/api.go

+
+// SigAggregator holds the intermediate state of signature aggregation.
+// Use one aggregator per aggregated signature.
+type SigAggregator interface {


As far as I can tell at the moment, we have no need for incremental accumulation of signers or intermediate state. Just make a simple pure function that takes a list of signatures and keys as parameters. We should spec the simplest API here that does what f3 needs, not what the underlying BLS library could potentially do.

anorth · 2024-01-29T22:16:06Z

f3/granite.go

@@ -325,11 +327,14 @@ func (i *instance) isJustified(msg *GMessage) bool {
 }

 // Sends this node's QUALITY message and begins the QUALITY phase.
-func (i *instance) beginQuality() {
+func (i *instance) beginQuality() error {


Discussion in #11 will have some bearing here. I am not keen on propagating error returns here for internal invariant problems. Maybe this will go away once the signing API is purely in terms of keys though?

I think it came up for me with broadcast failing, which is not invariant problem.

matejpavlovic · 2024-02-06T11:17:52Z

Looks like this PR needs to be rebased on top of upstream changes. The new upstream signing API is still different, but it does make sense to me. It should not be hard to modify the kyber-based implementation to use this API.
@Kubuxu are you currently on this, or should I rebase (and adapt the implementation to the new API)?

The kyber library, however, only supports signatures on G1. I created a PR to add support for G2, but it needs to be reviewed (Yolan said he'd have a look after his vacation).

Regarding the signatures on G2, I guess we can do one of

Go forward with this implementation (using sigs on G1) and switch to G2 once kyber supports G2.
Put this PR on hold until G2 support in kyber
Drop the current implementation and look for a alternative library that supports signatures on G2.

What do you think?

Kubuxu · 2024-02-06T12:11:04Z

I have started a new branch based on your code. My plan is to begin with signatures on G1 and make it easy to switch.

matejpavlovic · 2024-02-06T12:13:44Z

Ok, let me know if I can be of any help there.

Kubuxu · 2024-02-13T21:32:27Z

Suberceeded by #70

matejpavlovic force-pushed the sig-scheme branch from 34c157c to aab4125 Compare January 18, 2024 00:39

matejpavlovic marked this pull request as draft January 18, 2024 09:54

matejpavlovic force-pushed the sig-scheme branch from 6026a99 to 68eceaa Compare January 18, 2024 14:12

matejpavlovic force-pushed the sig-scheme branch from 68eceaa to dfebe88 Compare January 23, 2024 13:59

matejpavlovic added 3 commits January 23, 2024 15:08

Define signature aggregation API

67b2808

Signed-off-by: Matej Pavlovic <[email protected]>

Add error handling to Signer API

95e35a5

Signed-off-by: Matej Pavlovic <[email protected]>

Implement BLS signature aggregation

c68fd6d

Signed-off-by: Matej Pavlovic <[email protected]>

matejpavlovic force-pushed the sig-scheme branch 2 times, most recently from 0015943 to 3a52ca1 Compare January 23, 2024 15:44

matejpavlovic marked this pull request as ready for review January 23, 2024 16:18

Separate BLS signing/verifying and aggregation

f51390e

Signed-off-by: Matej Pavlovic <[email protected]>

matejpavlovic force-pushed the sig-scheme branch from 3a52ca1 to f51390e Compare January 23, 2024 16:25

ranchalp mentioned this pull request Jan 23, 2024

Add Aggregator interface and implement fake aggregator #35

Merged

ranchalp reviewed Jan 26, 2024

View reviewed changes

anorth reviewed Jan 29, 2024

View reviewed changes

Kubuxu closed this Feb 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Define signature aggregation API and implement it using BLS #42

Define signature aggregation API and implement it using BLS #42

matejpavlovic commented Jan 18, 2024 •

edited

Loading

matejpavlovic commented Jan 18, 2024

Kubuxu commented Jan 18, 2024

matejpavlovic commented Jan 18, 2024

ranchalp commented Jan 18, 2024

matejpavlovic commented Jan 18, 2024

Kubuxu commented Jan 18, 2024

matejpavlovic commented Jan 23, 2024

Kubuxu commented Jan 23, 2024

ranchalp commented Jan 25, 2024

ranchalp Jan 18, 2024

ranchalp Jan 18, 2024

ranchalp Jan 26, 2024

ranchalp Jan 26, 2024

ranchalp Jan 26, 2024

ranchalp Jan 26, 2024 •

edited

Loading

ranchalp Jan 26, 2024

ranchalp Jan 26, 2024

Kubuxu Feb 5, 2024

ranchalp Jan 26, 2024

anorth Jan 29, 2024

anorth Jan 29, 2024

anorth Jan 29, 2024

anorth Jan 29, 2024

anorth Jan 29, 2024

anorth Jan 29, 2024

Kubuxu Feb 5, 2024

matejpavlovic commented Feb 6, 2024

Kubuxu commented Feb 6, 2024

matejpavlovic commented Feb 6, 2024

Kubuxu commented Feb 13, 2024

Define signature aggregation API and implement it using BLS #42

Define signature aggregation API and implement it using BLS #42

Conversation

matejpavlovic commented Jan 18, 2024 • edited Loading

matejpavlovic commented Jan 18, 2024

Kubuxu commented Jan 18, 2024

matejpavlovic commented Jan 18, 2024

ranchalp commented Jan 18, 2024

matejpavlovic commented Jan 18, 2024

Kubuxu commented Jan 18, 2024

matejpavlovic commented Jan 23, 2024

Kubuxu commented Jan 23, 2024

ranchalp commented Jan 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ranchalp Jan 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matejpavlovic commented Feb 6, 2024

Kubuxu commented Feb 6, 2024

matejpavlovic commented Feb 6, 2024

Kubuxu commented Feb 13, 2024

matejpavlovic commented Jan 18, 2024 •

edited

Loading

ranchalp Jan 26, 2024 •

edited

Loading