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

"solo machine" client definition #309

Closed
warner opened this issue Nov 14, 2019 · 7 comments
Closed

"solo machine" client definition #309

warner opened this issue Nov 14, 2019 · 7 comments
Assignees
Milestone

Comments

@warner
Copy link

warner commented Nov 14, 2019

@mossid has been helping us (Agoric) with the definition of a "solo machine" client type (related to #160). This would be for a standalone process that wants to speak IBC to a chain. Solo machines don't have provable public state like blockchains do, but they are capable of keeping secrets, like private signing keys. So the "client state" that a receiving chain is tracking is really just a public verifying key and a sequence number. And the "proof" that any given message was emitted by a chain is really a public-key signature.

The IBC specification allows for various "client types", so we're trying to define what the "solo machine client type" should contain. Here's what we have so far.

Suppose that Alice is a solo machine that wants to send messages to chains and/or other solo machines. Alice might have a single private signing key, or she might use separate keys for each correspondent. She maintains a separate "proof sequence number" for each of her correspondents, along with a unique static identifier (a chain-id). Her IBC subsystem will produce a variety of handshake messages, followed by a sequence of packets. We want the receiving chain to correctly accept the packets that Alice creates, in the right order, and reject any false messages injected by attackers.

Each packet will have a "packet sequence number". This grows by one for each packet she emits to a given recipient (they all share the same numberspace, so packet sequence number 4 on the Alice-Bob connection is unreltaed to 4 on the Alice-Carol connection). This packet sequence number does not include handshake messages. In contrast, the "proof sequence number" includes both packet messages and handshake messages.

When Alice creates a new packet, she builds a structure like this:

  • key: ports/$PORTID/channels/$CHANNELID/packets/$PacketSequenceNumber
  • value: packet data
  • proof: empty

and she computes MessageHash as a hash of the safe concatenation of key and value. She then produces a receivePacket message with key, value, proof (which is empty), and the current proof sequence number (as blockHeight).

Before sending this, she constructs a safe concatenation of her "chain-id" for the receipient, the proof sequence number, and the MessageHash. She applies her signing key to this body to produce the MessageSignature.

She then constructs a Header:

  • height: proof sequence number
  • hash: the MessageHash value
  • sig: the MessageSignature

She builds an updateClient message around this Header (where hash is treated like the AppHash in a Tendermint block header, and sig is like the set of validator signatures).

Both the updateClient message and the receivePacket message are put into a Tendermint transaction and signed as usual. The key used to sign these messages is irrelevant (you'd want to deduct gas fees from the matching account, to prevent spam, but the transaction signing key is not meaningful to the IBC client verifier logic).

On the chain, the updateClient message is processed first. The "solo client type" has a specific ConsensusState data structure that remembers:

  • the client-id (aka chain-id) which this sender uses when talking to us
  • the next expected proof sequence number
  • the public verifying key to be applied to the next message

Each client type has a table mapping (what it thinks of as) "block height" to (what it thinks of as) "app hash". For the solo client type, this maps proof sequence number to MessageHash.

When the updateClient message arrives (with its Header), the updateClient() function does:

  • use the claimed clientID to look up the next expected proof sequence number and verifying key
  • assert that the Header's height matches the next expected proof sequence number, else reject
  • build a body by concatenating chain-id, proof sequence number, and the Header's MessageHash
  • verify sig against this body and the stored "next expected verifying key", else reject
  • add the mapping from height to MessageHash to the table
  • increment the next expected proof sequence number
  • (TODO) allow for key rotation by updating the next expected verifying key

Later, when the receivePacket message arrives, the IBC code will extract key, value, proof, and proofHeight from the message. It will then call the client-type-specific verifier function with these four values. Our solo-client verifier function will:

  • construct MessageHash by hashing a safe concatenation of key (which includes the packet sequence number) and value (which has the actual packet data)
  • (TODO: somehow figure out which client this is for??)
  • use proofHeight to look up the expected MessageHash
  • compare MessageHash for equality, else reject

If this works correctly, an attacker should not be able to spoof incoming messages, nor cause incoming messages to be rearranged. However, I'm concerned about the handshake messages, because e.g. all the *Connection messages use the same key string, and the message type (TryConnection as opposed to AckConnection) is not included in the hash (because the client-type verification function is not told which message contained the key/value pair being verified). So an attacker could take a message produced for one purpose and submit for a different purpose. As best we can tell, the two places that might be vulnerable to this are the four *Connection messages (init, try, ack, confirm), and the four *Channel messages (again named init, try, ack, and confirm). There are probably other defenses in place, but this is where my security-reviewer mindset told me to pay attention.

@cwgoes
Copy link
Contributor

cwgoes commented Nov 14, 2019

Part of the idea behind the store layout abstraction was to avoid the requirement of fitting solo machines with single signing keys into the UpdateClient / VerifyProof flow. I realise it's possible to finagle that - as you demonstrate - but do you think it is necessary? I was just thinking that the solo machine client would define a Proof type which is actually a digital signature and that verifyPacketCommitment / verifyPacketAcknowledgement / etc. would simply check that signature - no client updates necessary. That store abstraction logic also applies to verifyConnectionState, which should assuage the handshake concern.

Did you consider that structure and reject it? If so, why? Although I admire your adaptation of the UpdateClient paradigm to solo machines, I would prefer that we find a more ergonomic solution if possible (and one in which it is easier to reason about performance - it should just reduce to signature verifications - and easier to reason about replay prevention).

If you think the spec requires further updates to store abstraction in order to make that (simpler) flow possible, I am more than happy to make them.

@warner
Copy link
Author

warner commented Nov 15, 2019

I'll defer to Joon for the specifics, but I think our stumbling block was the API between receivePacket (which is the same for all client types) and the client-type-specific verification function. From our discussions yesterday, I got the impression that this API had a fixed signature that took a keyPath, value, and proof. We needed somewhere to tell the client-specific verifier the details of which packet is being verified (the signature must cover the port/channel/etc IDs and sequence number, else an attacker could rearrange the messages, and thus the verifier must be given enough information to build the body whose signature is to be checked). And it didn't seem like there was another place to pass that information.

However, we had that discussion without looking very carefully at the code, so I imagine I'm wrong. When Joon comes by today, we'll walk through the spec and code and see if there's a simpler way.

@cwgoes
Copy link
Contributor

cwgoes commented Nov 15, 2019

The client-specific verification function does include the port ID, channel ID, and sequence - check out verifyPacketCommitment under Required functions (in ICS 2).

I wonder if you might have been looking at an older version of the spec - or, if you were looking at the SDK-side code, do note that the SDK is currently implementing 1.0.0-rc4 of the spec, so it does not include store abstraction (which was post-rc4). Hopefully it will soon but I believe that the priority was to polish the current version a bit first.

The delta between the spec & implementation is being tracked here and the particular issue tracking store layout abstraction implementation is here.

(cc @fedekunze)

@cwgoes
Copy link
Contributor

cwgoes commented Dec 17, 2019

See ICS 6 for the basic structure of how I think a solo machine client can work with the new store layout abstraction. Does that make sense?

@cwgoes cwgoes self-assigned this Jan 4, 2020
@cwgoes cwgoes added this to the IBC 1.0.0-rc6 milestone Jan 4, 2020
@cwgoes
Copy link
Contributor

cwgoes commented Jan 4, 2020

Bump - any thoughts from Agoric's side @warner?

Would an implementation to play with help? I think @fedekunze will have it up pretty soon 🎉 .

@fedekunze
Copy link

Yeah, I think we can have an SDK implementation by the end of next week

@cwgoes
Copy link
Contributor

cwgoes commented Jan 4, 2020

Closing since #331 has been merged. Please re-open if #331 does not satisfy your requirements.

@cwgoes cwgoes closed this as completed Jan 4, 2020
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

3 participants