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

Check block integrity when necessary #1764

Merged
merged 8 commits into from
Mar 21, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 92 additions & 8 deletions byzcoin/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,26 @@ package byzcoin
import (
"bytes"
"errors"
"fmt"
"math"
"time"

"go.dedis.ch/kyber/v3"
"go.dedis.ch/kyber/v3/sign/schnorr"

"go.dedis.ch/cothority/v3"
"go.dedis.ch/cothority/v3/darc"
"go.dedis.ch/cothority/v3/darc/expression"
"go.dedis.ch/cothority/v3/skipchain"
"go.dedis.ch/kyber/v3"
"go.dedis.ch/kyber/v3/sign/schnorr"
"go.dedis.ch/onet/v3"
"go.dedis.ch/onet/v3/log"
"go.dedis.ch/onet/v3/network"
"go.dedis.ch/protobuf"
)

// for testing purpose until correct interfaces
var testCorruptNewLedgerResponse *CreateGenesisBlockResponse
var testCorruptProofResponse *GetProofResponse

// ServiceName is used for registration on the onet.
const ServiceName = "ByzCoin"

Expand Down Expand Up @@ -60,7 +64,18 @@ func NewLedger(msg *CreateGenesisBlock, keep bool) (*Client, *CreateGenesisBlock
if err := c.SendProtobuf(msg.Roster.List[0], msg, reply); err != nil {
return nil, nil, err
}
c.ID = reply.Skipblock.CalculateHash()

// for testing purpose
if testCorruptNewLedgerResponse != nil {
reply = testCorruptNewLedgerResponse
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's easier to just test verifyGenesisBlock by itself. then you don't need to insert test code into production code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another possibility: can we register a corrupted byzcoin service that always just returns a wrong genesis block (e.g., embed byzcoin.Service inside byzcoin.BadService and overwrite the create genesis block function), it doesn't even have to do consensus or anything like that, just returns the bad block? just a few ideas to think about

Copy link
Contributor Author

@Gilthoniel Gilthoniel Mar 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes for the verifyGenesisBlock but I really want to make sure the request is checking this, and that no one removed this check accidentally.

another posibility...
I tried that first, but the service name is a constant (which is fine) so you can't change which service is going to be used, or am I missing something ? I would love to do it that way..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I found a way to go around without too many complex mocking


// checks if the returned genesis block has the same parameters
if err := verifyGenesisBlock(reply.Skipblock, msg); err != nil {
return nil, nil, err
}

c.ID = reply.Skipblock.Hash
return c, reply, nil
}

Expand Down Expand Up @@ -91,8 +106,8 @@ func (c *Client) AddTransactionAndWait(tx ClientTransaction, wait int) (*AddTxRe
}

// GetProof returns a proof for the key stored in the skipchain by sending a
// message to the node on index 0 of the roster. The proof can be verified with
// the genesis skipblock and can prove the existence or the absence of the key.
// message to the node on index 0 of the roster. The proof can prove the existence
// or the absence of the key. Note that the integrity of the proof is verified.
// The Client's Roster and ID should be initialized before calling this method
// (see NewClientFromConfig).
func (c *Client) GetProof(key []byte) (*GetProofResponse, error) {
Expand All @@ -105,6 +120,17 @@ func (c *Client) GetProof(key []byte) (*GetProofResponse, error) {
if err != nil {
return nil, err
}

if testCorruptProofResponse != nil {
reply = testCorruptProofResponse
}

// verify the integrity of the proof only
err = reply.Proof.Verify(c.ID)
if err != nil {
return nil, err
}

return reply, nil
}

Expand Down Expand Up @@ -251,11 +277,12 @@ func (c *Client) WaitProof(id InstanceID, interval time.Duration, value []byte)
// StreamTransactions sends a streaming request to the service. If successful,
// the handler will be called whenever a new response (a new block) is
// available. This function blocks, the streaming stops if the client or the
// service stops.
// service stops. Only the integrity of the new block is verified.
func (c *Client) StreamTransactions(handler func(StreamingResponse, error)) error {
req := StreamingRequest{
ID: c.ID,
}
// TODO: what if the leader is down ?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the error handler handler(StreamingResponse{}, err) should be called and the function returns

conn, err := c.Stream(c.Roster.List[0], &req)
if err != nil {
return err
Expand All @@ -266,7 +293,13 @@ func (c *Client) StreamTransactions(handler func(StreamingResponse, error)) erro
handler(StreamingResponse{}, err)
return nil
}
handler(resp, nil)

if resp.Block.CalculateHash().Equal(resp.Block.Hash) {
// send the block only if the integrity is correct
handler(resp, nil)
} else {
log.Warnf("got a corrupted block from %v", c.Roster.List[0])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably you should call the handler here too, to say there is an error, like handler(StreamingResponse{}, err)

}
}
}

Expand Down Expand Up @@ -417,3 +450,54 @@ func DefaultGenesisMsg(v Version, r *onet.Roster, rules []string, ids ...darc.Id
}
return &m, nil
}

func verifyGenesisBlock(sb *skipchain.SkipBlock, req *CreateGenesisBlock) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I would name those parameters like: actual *skipchain.SkipBlock, expected *CreateGenesisBlock, so it's obvious which input is trusted.

if !sb.CalculateHash().Equal(sb.Hash) {
return errors.New("got a corrupted block")
}

// check the block is like the proposal
ok, err := sb.Roster.Equal(&req.Roster)
if err != nil {
return err
}
if !ok {
return errors.New("wrong roster in genesis block")
}

darcID, err := extractDarcID(sb)
if err != nil {
return err
}

if !darcID.Equal(req.GenesisDarc.GetID()) {
return errors.New("wrong darc spawned")
}

return nil
}

func extractDarcID(sb *skipchain.SkipBlock) (darc.ID, error) {
var data DataBody
err := protobuf.Decode(sb.Payload, &data)
if err != nil {
return nil, fmt.Errorf("fail to decode data: %v", err)
}

if len(data.TxResults) != 1 || len(data.TxResults[0].ClientTransaction.Instructions) != 1 {
return nil, errors.New("didn't get only one instruction")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return nil, errors.New("didn't get only one instruction")
return nil, errors.New("genesis darc tx should only have one instruction")

}

instr := data.TxResults[0].ClientTransaction.Instructions[0]
if instr.Spawn == nil {
return nil, errors.New("didn't get a spawn instruction")
}

var darc darc.Darc
err = protobuf.Decode(instr.Spawn.Args.Search("darc"), &darc)
if err != nil {
return nil, fmt.Errorf("fail to decode the darc: %v", err)
}

return darc.GetID(), nil
}
112 changes: 112 additions & 0 deletions byzcoin/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,93 @@ import (
"github.com/stretchr/testify/require"
"go.dedis.ch/cothority/v3"
"go.dedis.ch/cothority/v3/darc"
"go.dedis.ch/cothority/v3/skipchain"
"go.dedis.ch/onet/v3"
"go.dedis.ch/onet/v3/log"
"go.dedis.ch/onet/v3/network"
"go.dedis.ch/protobuf"
)

func TestClient_NewLedgerCorrupted(t *testing.T) {
l := onet.NewTCPTest(cothority.Suite)
servers, roster, _ := l.GenTree(3, true)
registerDummy(servers)
defer l.CloseAll()

signer := darc.NewSignerEd25519(nil, nil)
msg, err := DefaultGenesisMsg(CurrentVersion, roster, []string{"spawn:dummy"}, signer.Identity())
msg.BlockInterval = 100 * time.Millisecond
require.Nil(t, err)

sb := skipchain.NewSkipBlock()
testCorruptNewLedgerResponse = &CreateGenesisBlockResponse{Skipblock: sb}
defer func() {
testCorruptNewLedgerResponse = nil
}()

sb.Roster = &onet.Roster{ID: onet.RosterID{}}
sb.Hash = sb.CalculateHash()
_, _, err = NewLedger(msg, false)
require.Error(t, err)
require.Equal(t, "wrong roster in genesis block", err.Error())

sb.Roster = roster
sb.Payload = []byte{1, 2, 3}
sb.Hash = testCorruptNewLedgerResponse.Skipblock.CalculateHash()
_, _, err = NewLedger(msg, false)
require.Error(t, err)
require.Contains(t, err.Error(), "fail to decode data:")

sb.Payload = []byte{}
sb.Hash = sb.CalculateHash()
_, _, err = NewLedger(msg, false)
require.Error(t, err)
require.Equal(t, "didn't get only one instruction", err.Error())

data := &DataBody{
TxResults: []TxResult{
TxResult{ClientTransaction: ClientTransaction{Instructions: []Instruction{Instruction{}}}},
},
}
sb.Payload, err = protobuf.Encode(data)
sb.Hash = sb.CalculateHash()
require.NoError(t, err)
_, _, err = NewLedger(msg, false)
require.Error(t, err)
require.Equal(t, "didn't get a spawn instruction", err.Error())

data.TxResults[0].ClientTransaction.Instructions[0].Spawn = &Spawn{
Args: []Argument{
Argument{
Name: "darc",
Value: []byte{1, 2, 3},
},
},
}
sb.Payload, err = protobuf.Encode(data)
sb.Hash = sb.CalculateHash()
require.NoError(t, err)
_, _, err = NewLedger(msg, false)
require.Error(t, err)
require.Contains(t, err.Error(), "fail to decode the darc:")

darcBytes, _ := protobuf.Encode(&darc.Darc{})
data.TxResults[0].ClientTransaction.Instructions[0].Spawn = &Spawn{
Args: []Argument{
Argument{
Name: "darc",
Value: darcBytes,
},
},
}
sb.Payload, err = protobuf.Encode(data)
sb.Hash = sb.CalculateHash()
require.NoError(t, err)
_, _, err = NewLedger(msg, false)
require.Error(t, err)
require.Equal(t, "wrong darc spawned", err.Error())
}

func TestClient_GetProof(t *testing.T) {
l := onet.NewTCPTest(cothority.Suite)
servers, roster, _ := l.GenTree(3, true)
Expand Down Expand Up @@ -64,6 +145,37 @@ func TestClient_GetProof(t *testing.T) {
require.Equal(t, value, v0)
}

func TestClient_GetProofCorrupted(t *testing.T) {
l := onet.NewTCPTest(cothority.Suite)
servers, roster, _ := l.GenTree(3, true)
registerDummy(servers)
defer l.CloseAll()

// Initialise the genesis message and send it to the service.
signer := darc.NewSignerEd25519(nil, nil)
msg, err := DefaultGenesisMsg(CurrentVersion, roster, []string{"spawn:dummy"}, signer.Identity())
msg.BlockInterval = 100 * time.Millisecond
require.Nil(t, err)

// The darc inside it should be valid.
d := msg.GenesisDarc
require.Nil(t, d.Verify(true))

c, _, err := NewLedger(msg, false)
require.Nil(t, err)

sb := skipchain.NewSkipBlock()
sb.Data = []byte{1, 2, 3}
testCorruptProofResponse = &GetProofResponse{
Proof: Proof{Latest: *sb},
}

_, err = c.GetProof([]byte{})
require.Error(t, err)

testCorruptProofResponse = nil
}

// Create a streaming client and add blocks in the background. The client
// should receive valid blocks.
func TestClient_Streaming(t *testing.T) {
Expand Down
20 changes: 7 additions & 13 deletions byzcoin/bcadmin/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/sha256"
"encoding/binary"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"io"
Expand All @@ -15,27 +16,23 @@ import (
"time"

"github.com/BurntSushi/toml"
"go.dedis.ch/cothority/v3/byzcoin/contracts"
"go.dedis.ch/kyber/v3/suites"
"go.dedis.ch/kyber/v3/util/encoding"
"go.dedis.ch/protobuf"

"github.com/qantik/qrgo"
"go.dedis.ch/cothority/v3"
"go.dedis.ch/cothority/v3/byzcoin"
"go.dedis.ch/cothority/v3/byzcoin/bcadmin/lib"
"go.dedis.ch/cothority/v3/byzcoin/contracts"
"go.dedis.ch/cothority/v3/darc"
"go.dedis.ch/cothority/v3/darc/expression"
"go.dedis.ch/cothority/v3/skipchain"
"go.dedis.ch/kyber/v3/suites"
"go.dedis.ch/kyber/v3/util/encoding"
"go.dedis.ch/kyber/v3/util/random"
"go.dedis.ch/onet/v3"
"go.dedis.ch/onet/v3/app"
"go.dedis.ch/onet/v3/cfgpath"
"go.dedis.ch/onet/v3/log"
"go.dedis.ch/onet/v3/network"

"encoding/json"

"github.com/qantik/qrgo"
"go.dedis.ch/protobuf"
"gopkg.in/urfave/cli.v1"
)

Expand Down Expand Up @@ -326,10 +323,7 @@ func create(c *cli.Context) error {
}
req.BlockInterval = interval

cl := onet.NewClient(cothority.Suite, byzcoin.ServiceName)

var resp byzcoin.CreateGenesisBlockResponse
err = cl.SendProtobuf(r.List[0], req, &resp)
_, resp, err := byzcoin.NewLedger(req, false)
if err != nil {
return err
}
Expand Down
14 changes: 12 additions & 2 deletions byzcoin/proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ var ErrorVerifyTrieRoot = errors.New("root of trie is not in skipblock")
// have a proper proof that it comes from the genesis block.
var ErrorVerifySkipchain = errors.New("stored skipblock is not properly evolved from genesis block")

// ErrorVerifyHash is returned if the latest block hash does not match
// the target of the last forward link
var ErrorVerifyHash = errors.New("last forward link does not point to the latest block")

// Verify takes a skipchain id and verifies that the proof is valid for this
// skipchain. It verifies the proof, that the merkle-root is stored in the
// skipblock of the proof and the fact that the skipblock is indeed part of the
Expand All @@ -81,13 +85,13 @@ func (p Proof) Verify(scID skipchain.SkipBlockID) error {
if !bytes.Equal(p.InclusionProof.GetRoot(), header.TrieRoot) {
return ErrorVerifyTrieRoot
}
var sbID skipchain.SkipBlockID

sbID := scID
var publics []kyber.Point
for i, l := range p.Links {
if i == 0 {
// The first forward link is a pointer from []byte{} to the genesis
// block and holds the roster of the genesis block.
sbID = scID
publics = l.NewRoster.ServicePublics(skipchain.ServiceName)
continue
}
Expand All @@ -102,6 +106,12 @@ func (p Proof) Verify(scID skipchain.SkipBlockID) error {
publics = l.NewRoster.ServicePublics(skipchain.ServiceName)
}
}

// Check that the given latest block matches the last forward link target
if !p.Latest.CalculateHash().Equal(sbID) {
return ErrorVerifyHash
}

return nil
}

Expand Down
Loading