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

Verify signatures during Parse #3046

Merged
merged 3 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Verify signatures during Parse
  • Loading branch information
StephenButtolph committed May 22, 2024
commit 546ad695136d8596e4c8a32be089b9e3129c7e5c
3 changes: 2 additions & 1 deletion indexer/examples/p-chain/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"github.com/ava-labs/avalanchego/indexer"
"github.com/ava-labs/avalanchego/utils/constants"
"github.com/ava-labs/avalanchego/wallet/subnet/primary"

platformvmblock "github.com/ava-labs/avalanchego/vms/platformvm/block"
Expand All @@ -33,7 +34,7 @@ func main() {
}

platformvmBlockBytes := container.Bytes
proposerVMBlock, err := proposervmblock.Parse(container.Bytes)
proposerVMBlock, err := proposervmblock.Parse(container.Bytes, constants.PlatformChainID)
if err == nil {
platformvmBlockBytes = proposerVMBlock.Block()
}
Expand Down
4 changes: 3 additions & 1 deletion indexer/examples/x-chain-blocks/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"log"
"time"

"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/indexer"
"github.com/ava-labs/avalanchego/vms/proposervm/block"
"github.com/ava-labs/avalanchego/wallet/chain/x/builder"
Expand All @@ -19,6 +20,7 @@ import (
func main() {
var (
uri = primary.LocalAPIURI + "/ext/index/X/block"
xChainID = ids.FromStringOrPanic("2eNy1mUFdmaxXNj1eQHUe7Np4gju9sJsEtWQ4MX3ToiNKuADed")
client = indexer.NewClient(uri)
ctx = context.Background()
nextIndex uint64
Expand All @@ -31,7 +33,7 @@ func main() {
continue
}

proposerVMBlock, err := block.Parse(container.Bytes)
proposerVMBlock, err := block.Parse(container.Bytes, xChainID)
if err != nil {
log.Fatalf("failed to parse proposervm block: %s\n", err)
}
Expand Down
2 changes: 1 addition & 1 deletion vms/proposervm/batched_vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func (vm *VM) BatchedParseBlock(ctx context.Context, blks [][]byte) ([]snowman.B
)
for ; blocksIndex < len(blks); blocksIndex++ {
blkBytes := blks[blocksIndex]
statelessBlock, err := statelessblock.Parse(blkBytes)
statelessBlock, err := statelessblock.Parse(blkBytes, vm.ctx.ChainID)
if err != nil {
break
}
Expand Down
21 changes: 11 additions & 10 deletions vms/proposervm/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ var (
errTimeTooAdvanced = errors.New("time is too far advanced")
errProposerWindowNotStarted = errors.New("proposer window hasn't started")
errUnexpectedProposer = errors.New("unexpected proposer for current window")
errProposerMismatch = errors.New("proposer mismatch")
errProposersNotActivated = errors.New("proposers haven't been activated yet")
errPChainHeightTooLow = errors.New("block P-chain height is too low")
)
Expand Down Expand Up @@ -152,9 +153,9 @@ func (p *postForkCommonComponents) Verify(
return err
}

// Verify the signature of the node
if err := child.SignedBlock.Verify(shouldHaveProposer, p.vm.ctx.ChainID); err != nil {
return err
_, hasProposer := child.SignedBlock.Proposer()
if shouldHaveProposer != hasProposer {
return fmt.Errorf("%w: shouldHaveProposer (%v) != hasProposer (%v)", errProposerMismatch, shouldHaveProposer, hasProposer)
}

p.vm.ctx.Log.Debug("verified post-fork block",
Expand Down Expand Up @@ -328,9 +329,9 @@ func (p *postForkCommonComponents) verifyPreDurangoBlockDelay(
blk *postForkBlock,
) (bool, error) {
var (
blkTimestamp = blk.Timestamp()
childHeight = blk.Height()
proposerID = blk.Proposer()
blkTimestamp = blk.Timestamp()
childHeight = blk.Height()
proposerID, _ = blk.Proposer()
)
minDelay, err := p.vm.Windower.Delay(
ctx,
Expand Down Expand Up @@ -363,10 +364,10 @@ func (p *postForkCommonComponents) verifyPostDurangoBlockDelay(
blk *postForkBlock,
) (bool, error) {
var (
blkTimestamp = blk.Timestamp()
blkHeight = blk.Height()
currentSlot = proposer.TimeToSlot(parentTimestamp, blkTimestamp)
proposerID = blk.Proposer()
blkTimestamp = blk.Timestamp()
blkHeight = blk.Height()
currentSlot = proposer.TimeToSlot(parentTimestamp, blkTimestamp)
proposerID, _ = blk.Proposer()
)

expectedProposerID, err := p.vm.Windower.ExpectedProposer(
Expand Down
45 changes: 22 additions & 23 deletions vms/proposervm/block/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@ import (
var (
_ SignedBlock = (*statelessBlock)(nil)

errUnexpectedProposer = errors.New("expected no proposer but one was provided")
errMissingProposer = errors.New("expected proposer but none was provided")
errInvalidCertificate = errors.New("invalid certificate")
errUnexpectedSignature = errors.New("signature provided when none was expected")
errInvalidCertificate = errors.New("invalid certificate")
)

type Block interface {
Expand All @@ -29,16 +28,18 @@ type Block interface {
Bytes() []byte

initialize(bytes []byte) error
verify(chainID ids.ID) error
}

type SignedBlock interface {
Block

PChainHeight() uint64
Timestamp() time.Time
Proposer() ids.NodeID

Verify(shouldHaveProposer bool, chainID ids.ID) error
// Proposer returns the ID of the node that proposed this block, or false if
// no node signed this block.
Proposer() (ids.NodeID, bool)
StephenButtolph marked this conversation as resolved.
Show resolved Hide resolved
}

type statelessUnsignedBlock struct {
Expand Down Expand Up @@ -101,26 +102,12 @@ func (b *statelessBlock) initialize(bytes []byte) error {
return nil
}

func (b *statelessBlock) PChainHeight() uint64 {
return b.StatelessBlock.PChainHeight
}

func (b *statelessBlock) Timestamp() time.Time {
return b.timestamp
}

func (b *statelessBlock) Proposer() ids.NodeID {
return b.proposer
}

func (b *statelessBlock) Verify(shouldHaveProposer bool, chainID ids.ID) error {
if !shouldHaveProposer {
if len(b.Signature) > 0 || len(b.StatelessBlock.Certificate) > 0 {
return errUnexpectedProposer
func (b *statelessBlock) verify(chainID ids.ID) error {
if len(b.StatelessBlock.Certificate) == 0 {
if len(b.Signature) > 0 {
return errUnexpectedSignature
}
return nil
} else if b.cert == nil {
return errMissingProposer
}

header, err := BuildHeader(chainID, b.StatelessBlock.ParentID, b.id)
Expand All @@ -135,3 +122,15 @@ func (b *statelessBlock) Verify(shouldHaveProposer bool, chainID ids.ID) error {
b.Signature,
)
}

func (b *statelessBlock) PChainHeight() uint64 {
return b.StatelessBlock.PChainHeight
}

func (b *statelessBlock) Timestamp() time.Time {
return b.timestamp
}

func (b *statelessBlock) Proposer() (ids.NodeID, bool) {
return b.proposer, b.cert != nil
}
40 changes: 14 additions & 26 deletions vms/proposervm/block/block_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,37 +14,25 @@ import (
"github.com/ava-labs/avalanchego/utils/units"
)

func equal(require *require.Assertions, chainID ids.ID, want, have SignedBlock) {
func equal(require *require.Assertions, want, have Block) {
require.Equal(want.ID(), have.ID())
require.Equal(want.ParentID(), have.ParentID())
require.Equal(want.PChainHeight(), have.PChainHeight())
require.Equal(want.Timestamp(), have.Timestamp())
require.Equal(want.Block(), have.Block())
require.Equal(want.Proposer(), have.Proposer())
require.Equal(want.Bytes(), have.Bytes())
require.Equal(want.Verify(false, chainID), have.Verify(false, chainID))
require.Equal(want.Verify(true, chainID), have.Verify(true, chainID))
}

func TestVerifyNoCertWithSignature(t *testing.T) {
parentID := ids.ID{1}
timestamp := time.Unix(123, 0)
pChainHeight := uint64(2)
innerBlockBytes := []byte{3}

require := require.New(t)

builtBlockIntf, err := BuildUnsigned(parentID, timestamp, pChainHeight, innerBlockBytes)
require.NoError(err)

builtBlock := builtBlockIntf.(*statelessBlock)
builtBlock.Signature = []byte{0}

err = builtBlock.Verify(false, ids.Empty)
require.ErrorIs(err, errUnexpectedProposer)

err = builtBlock.Verify(true, ids.Empty)
require.ErrorIs(err, errMissingProposer)
signedWant, wantIsSigned := want.(SignedBlock)
signedHave, haveIsSigned := have.(SignedBlock)
require.Equal(wantIsSigned, haveIsSigned)
if !wantIsSigned {
return
}

require.Equal(signedWant.PChainHeight(), signedHave.PChainHeight())
require.Equal(signedWant.Timestamp(), signedHave.Timestamp())
wantProposer, wantHasProposer := signedWant.Proposer()
haveProposer, haveHasProposer := signedHave.Proposer()
require.Equal(wantHasProposer, haveHasProposer)
require.Equal(wantProposer, haveProposer)
}

func TestBlockSizeLimit(t *testing.T) {
Expand Down
16 changes: 7 additions & 9 deletions vms/proposervm/block/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func TestBuild(t *testing.T) {
cert, err := staking.ParseCertificate(tlsCert.Leaf.Raw)
require.NoError(err)
key := tlsCert.PrivateKey.(crypto.Signer)
nodeID := ids.NodeIDFromCert(cert)

builtBlock, err := Build(
parentID,
Expand All @@ -46,10 +47,9 @@ func TestBuild(t *testing.T) {
require.Equal(timestamp, builtBlock.Timestamp())
require.Equal(innerBlockBytes, builtBlock.Block())

require.NoError(builtBlock.Verify(true, chainID))

err = builtBlock.Verify(false, chainID)
require.ErrorIs(err, errUnexpectedProposer)
proppserID, hasProposer := builtBlock.Proposer()
require.True(hasProposer)
require.Equal(nodeID, proppserID)
}

func TestBuildUnsigned(t *testing.T) {
Expand All @@ -67,12 +67,10 @@ func TestBuildUnsigned(t *testing.T) {
require.Equal(pChainHeight, builtBlock.PChainHeight())
require.Equal(timestamp, builtBlock.Timestamp())
require.Equal(innerBlockBytes, builtBlock.Block())
require.Equal(ids.EmptyNodeID, builtBlock.Proposer())

require.NoError(builtBlock.Verify(false, ids.Empty))

err = builtBlock.Verify(true, ids.Empty)
require.ErrorIs(err, errMissingProposer)
proposer, hasProposer := builtBlock.Proposer()
require.False(hasProposer)
require.Equal(ids.EmptyNodeID, proposer)
}

func TestBuildHeader(t *testing.T) {
Expand Down
4 changes: 4 additions & 0 deletions vms/proposervm/block/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,7 @@ func (b *option) initialize(bytes []byte) error {
b.bytes = bytes
return nil
}

func (*option) verify(ids.ID) error {
return nil
}
13 changes: 0 additions & 13 deletions vms/proposervm/block/option_test.go

This file was deleted.

20 changes: 18 additions & 2 deletions vms/proposervm/block/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,25 @@

package block

import "fmt"
import (
"fmt"

func Parse(bytes []byte) (Block, error) {
"github.com/ava-labs/avalanchego/ids"
)

// Parse a block and verify that the signature attached to the block is valid
// for the certificate provided in the block.
func Parse(bytes []byte, chainID ids.ID) (Block, error) {
block, err := ParseWithoutVerification(bytes)
if err != nil {
return nil, err
}
return block, block.verify(chainID)
}

// ParseWithoutVerification parses a block without verifying that the signature
// on the block is correct.
func ParseWithoutVerification(bytes []byte) (Block, error) {
var block Block
parsedVersion, err := Codec.Unmarshal(bytes, &block)
if err != nil {
Expand Down
Loading
Loading