The first two steps for the key generation protocol #17

pdyraga · 2024-10-11T11:25:34Z

~~Depends on #16~~

The first two steps generate a symmetric key between every two group members for further communication over the broadcast channel.

generateEphemeralKeyPair takes the group member list and generates an ephemeral ECDH key pair for every other group member. Generated public ephemeral keys are broadcasted within the group.

generateSymmetricKeys attempts to generate symmetric keys for all remote group members via ECDH. It generates this symmetric key for each remote group member by doing an ECDH between the ephemeral private key generated for a remote group member, and the public key for this member, generated and broadcasted by the remote group member.

The code was ported from the keep-network/keep-core random beacon with some changes.

I am intentionally making no functions/types as exported - leaving it for later once it is decided how to use this package.

Group structure represents the current state of information about the GJKR key generation group. Each GJKR protocol participant should have the same group state at the end of each protocol step. This code has been ported from keep-network/keep-core random beacon's GJKR with small tweaks.

GJKR protocol allows the key generation to continue even if some misbehavior inside of the group is detected. We do not want to stop protocols returning an error yet we want to log all the misbehavior. To not complicate the API of GJKR, a Logger interface implementation will have to be provided when instantiating GJKR protocol members. The current implementation is aligned with the Cosmos SDK logger interface but the client code for the library can implements their own adapters.

GJKR protocol requires group members to have access to messages exchanged between the accuser and the accused party for the stake of complaint resolution. This is what the evidence log provides. For now, only functions for the ephemeral public key message are included. The code has been ported from keep-network/keep-core GJKR implementation for the random beacon with small changes.

The first two steps generate a symmetric key between every two group members for the further communication over the broadcast channel. generateEphemeralKeyPair takes the group member list and generates an ephemeral ECDH keypair for every other group member. Generated public ephemeral keys are broadcasted within the group. generateSymmetricKeys attempts to generate symmetric keys for all remote group members via ECDH. It generates this symmetric key for each remote group member by doing an ECDH between the ephemeral private key generated for a remote group member, and the public key for this member, generated and broadcasted by the remote group member. The code was ported from keep-network/keep-core random beacon with small changes. There are two important parts missing that will be addressed in separate commits: - No member inactivity marking, - Unit tests for the two steps.

lukasz-zimnoch · 2024-10-16T14:48:04Z

gjkr/evidence_log.go

+// sent by the accused party. To do this, they read the round 3 message from the
+// log, and decrypt it using the symmetric key used between the accuser and
+// accused party. The key is publicly revealed by the accuser.
+type evidenceLog interface {


What do you think about making evidenceLog a concrete structure and removing dkgEvidenceLog altogether? It does not make much sense to have an interface here. We are using one in-house implementation anyway. I know this was the original design in keep-core but maybe we can take the opportunity and clean it up.

Makes sense. I was blindly assuming we'll use some sort of a mock interface in protocol integration tests but I browsed keep-core and I see it is not the case. Updated in 20254f7.

lukasz-zimnoch · 2024-10-16T14:50:51Z

gjkr/evidence_log.go

+
+type messageStorage struct {
+	cache     map[memberIndex]interface{}
+	cacheLock sync.Mutex


Using sync.RWMutex will be a small improvement here.

lukasz-zimnoch · 2024-10-16T15:08:24Z

gjkr/evidence_log.go

+}
+
+type messageStorage struct {
+	cache     map[memberIndex]interface{}


Loose idea: We can rejuvenate this code and use generics:

type messageStorage[[T interface{}] struct { cache map[memberIndex]T (...) }

This way, we can create typed storages within dkgEvidenceLog:

type dkgEvidenceLog struct { pubKeyMessageLog *messageStorage[*ephemeralPublicKeyMessage] }

and get rid of the type assertion in getEphemeralPublicKeyMessage.

With generics, this code will be more elegant once we add more messages.

Good call. Done in baff3ad.

lukasz-zimnoch · 2024-10-16T15:13:18Z

gjkr/group.go

+// generation group. Each GJKR protocol participant should have the same group
+// state at the end of each protocol step.
+type group struct {
+	dishonestThreshold uint16


Nit: dishonestThreshold caused some confusion in keep-core in the past. What do you think about leaning on threshold and exposing a function dishonestThreshold that will compute it as groupSize - threshold? This approach feels much more intuitive.

I was not sure about it either but wanted to approach it in the next PR when we will be adding the remaining steps of the protocol. I noticed we usually do +1 in keep-core unit tests which is a symptom of the problem you described but I did not find yet in the protocol where the dishonest threshold would be problematic and I assumed we could have some reason for leaving it. What about not worrying about it now but leaving this thread open and revisiting in the next PR?

lukasz-zimnoch · 2024-10-16T15:18:56Z

gjkr/member.go

+
+import "threshold.network/roast/ephemeral"
+
+type memberIndex uint16


Worth dropping a comment that memberIndex starts from 1.

See 78a4eae, please.

lukasz-zimnoch · 2024-10-16T15:20:40Z

gjkr/message.go

+	ephemeralPublicKeys map[memberIndex]*ephemeral.PublicKey // j -> Y_ij
+}
+
+func (m *ephemeralPublicKeyMessage) senderIdx() memberIndex {


I think Idx can be easily confused with Id. What about the full name:

Suggested change

func (m *ephemeralPublicKeyMessage) senderIdx() memberIndex {

func (m *ephemeralPublicKeyMessage) senderIndex() memberIndex {

This is what I started with but the compiler does not like the fact the field name and the function name are the same. I would be happy to remove this function altogether but we need a type constraint for func deduplicateBySender[T interface{ senderIndex() memberIndex }](list []T,) []T. Since senderIdx is used only there, I figured this might be okay.

I refactored it in 02c828c. After adding sessionID, this conflict was not the only one so I ended up with get* functions.

lukasz-zimnoch · 2024-10-16T15:28:09Z

gjkr/group.go

+	for _, disqualifiedMemberIndex := range g.disqualifiedMemberIndexes {
+		if memberIndex == disqualifiedMemberIndex {
+			return true
+		}
+	}
+
+	return false


Same as above:

Suggested change

for _, disqualifiedMemberIndex := range g.disqualifiedMemberIndexes {

if memberIndex == disqualifiedMemberIndex {

return true

}

}

return false

return slices.Contains(g.disqualifiedMemberIndexes, memberIndex)

Totally! Please see 2ea14cd.

lukasz-zimnoch · 2024-10-16T15:35:01Z

gjkr/message.go

+// must know its ephemeral public key prior to exchanging any messages. Hence,
+// this message contains all the generated public keys and it is broadcast
+// within the group.
+type ephemeralPublicKeyMessage struct {


I think we need a sessionID as well. If we don't want to add it in this PR, let's add a TODO at least.

I was thinking about moving session management outside of the main protocol but after reconsidering it, I think we should have it included, as originally. It could happen inactivity or disqualification could lead to slashing (out of the scope of this protocol) and we should not make assumptions about how the messages are signed. Including session ID on the protocol level would eliminate replay attacks at the core. Added a point to the TODO list.

Added session ID filtering in 02c828c.

It does not make sense to separate the interface from the implementation as we keep them in the same package and will use the concrete implementation for the code and tests.

The messageStore structure used internally for the evidenceLog can use generics for a shorter code. We could also use RWMutex to separate read and write locks.

With this version of Go we can use slices.Contains instead of reinventing the wheel.

This will allow us to use slices.Equal (to be used in the next commit). Also, 1.22.2 is the version used by mezo-org/mezod that will probably be the first integration of this library.

The phase 2 of the GJKR protocol generating symmetric keys should find members inactive in phase 1 and mark them accordingly. I introduced this logic in a new file, message_filter.go and moved there the deduplication logic as well. The goal is to have all the message pre-processing for all phases in one place. For phase 2, the pre-processing includes deduplication, inactive member detection, and session ID handling (to be added in the next commits). For further phases, we will also have to filter out members marked as inactive or disqualified in previous phases of the protocol.

Added message pre-processing step ensuring messages with a session ID different from the current one are filtered out. This protects against the replay attacks on the protocol level.

In the previous commit a filtering by session ID was implemented but I missed adding the session ID to the ephemeralPublicKeyMessage generated by the first phase of the protocol. Doing it now.

When we pre-process messages before executing GJKR protocol phase, we now also remove the from-self message from the list.

The test executes phase 1 and phase 2 and ensures the expected number of symmetric keys has been generated.

pdyraga · 2024-10-25T15:13:36Z

This is ready for another chance @lukasz-zimnoch.

pdyraga added 4 commits October 10, 2024 15:29

Base automatically changed from one-curve to main October 15, 2024 15:37

lukasz-zimnoch reviewed Oct 16, 2024

View reviewed changes

pdyraga added 10 commits October 21, 2024 09:15

Removed separate dkgEvidenceLog implementation struct

20254f7

It does not make sense to separate the interface from the implementation as we keep them in the same package and will use the concrete implementation for the code and tests.

RWMutex and generic type used for messageStore

baff3ad

The messageStore structure used internally for the evidenceLog can use generics for a shorter code. We could also use RWMutex to separate read and write locks.

Use slices.Contains for isInactive and isDiqualified

2ea14cd

With this version of Go we can use slices.Contains instead of reinventing the wheel.

Better documentation for the memberIndex type

78a4eae

Bumped up Go version to 1.22.2

f7fe459

This will allow us to use slices.Equal (to be used in the next commit). Also, 1.22.2 is the version used by mezo-org/mezod that will probably be the first integration of this library.

Filter out ephemeral pub key messages by session ID

02c828c

Added message pre-processing step ensuring messages with a session ID different from the current one are filtered out. This protects against the replay attacks on the protocol level.

Include the session ID when constructing ephemeral pubkey message

ec297fd

In the previous commit a filtering by session ID was implemented but I missed adding the session ID to the ephemeralPublicKeyMessage generated by the first phase of the protocol. Doing it now.

Filter out from-self messages in GJKR

8449f5d

When we pre-process messages before executing GJKR protocol phase, we now also remove the from-self message from the list.

Test for the first two phases of GJKR protocol

c8a5ae4

The test executes phase 1 and phase 2 and ensures the expected number of symmetric keys has been generated.

pdyraga marked this pull request as ready for review October 25, 2024 15:13

Merge branch 'main' into first-steps

6c17028

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The first two steps for the key generation protocol #17

The first two steps for the key generation protocol #17

pdyraga commented Oct 11, 2024 •

edited

Loading

lukasz-zimnoch Oct 16, 2024

pdyraga Oct 21, 2024

lukasz-zimnoch Oct 16, 2024

pdyraga Oct 21, 2024

lukasz-zimnoch Oct 16, 2024

pdyraga Oct 21, 2024

lukasz-zimnoch Oct 16, 2024

pdyraga Oct 21, 2024

lukasz-zimnoch Oct 16, 2024

pdyraga Oct 21, 2024

lukasz-zimnoch Oct 16, 2024

pdyraga Oct 21, 2024

pdyraga Oct 23, 2024

lukasz-zimnoch Oct 16, 2024

pdyraga Oct 21, 2024

lukasz-zimnoch Oct 16, 2024

pdyraga Oct 21, 2024 •

edited

Loading

pdyraga Oct 23, 2024

pdyraga commented Oct 25, 2024


		import "threshold.network/roast/ephemeral"

		type memberIndex uint16

	func (m *ephemeralPublicKeyMessage) senderIdx() memberIndex {
	func (m *ephemeralPublicKeyMessage) senderIndex() memberIndex {

The first two steps for the key generation protocol #17

Are you sure you want to change the base?

The first two steps for the key generation protocol #17

Conversation

pdyraga commented Oct 11, 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

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

pdyraga Oct 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdyraga commented Oct 25, 2024

pdyraga commented Oct 11, 2024 •

edited

Loading

pdyraga Oct 21, 2024 •

edited

Loading