Skip to content

Commit

Permalink
Remove the code for signing log roots if the last one is too old. (#266)
Browse files Browse the repository at this point in the history 
* Remove the code for signing log roots if the last one is too old.

This is leftover from when we thought app layers wouldn't have their own
storage. I don't think it's being used and probably doesn't do what we
need anyway.

* Create a root in sequencing if none already exists.

Could be better to move this to log create API when we write it. Then we
can assume a root exists and raise an error if it doesn't. TODO added.
  • Loading branch information
@Martin2112
Martin2112 authored Dec 16, 2016
1 parent b2ea828 commit 48a95f0
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 77 deletions.
18 changes: 4 additions & 14 deletions log/sequencer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,6 @@ type Sequencer struct {
// the subtrees.
const maxTreeDepth = 64

// CurrentRootExpiredFunc examines a signed log root and decides if it has expired with respect
// to a max age duration and a given time source
// TODO(Martin2112): This is all likely to go away when we switch to application STHs
type CurrentRootExpiredFunc func(trillian.SignedLogRoot) bool

// NewSequencer creates a new Sequencer instance for the specified inputs.
func NewSequencer(hasher merkle.TreeHasher, timeSource util.TimeSource, logStorage storage.LogStorage, km crypto.KeyManager) *Sequencer {
return &Sequencer{hasher: hasher, timeSource: timeSource, logStorage: logStorage, keyManager: km}
Expand Down Expand Up @@ -156,7 +151,7 @@ func (s Sequencer) createRootSignature(ctx context.Context, root trillian.Signed
// TODO(Martin2112): Can possibly improve by deferring a function that attempts to rollback,
// which will fail if the tx was committed. Should only do this if we can hide the details of
// the underlying storage transactions and it doesn't create other problems.
func (s Sequencer) SequenceBatch(ctx context.Context, limit int, expiryFunc CurrentRootExpiredFunc) (int, error) {
func (s Sequencer) SequenceBatch(ctx context.Context, limit int) (int, error) {
tx, err := s.logStorage.Begin()

if err != nil {
Expand Down Expand Up @@ -184,22 +179,17 @@ func (s Sequencer) SequenceBatch(ctx context.Context, limit int, expiryFunc Curr
}

// TODO(al): Have a better detection mechanism for there being no stored root.
// TODO(mhs): Might be better to create empty root in provisioning API when it exists
if currentRoot.RootHash == nil {
glog.Warningf("%s: Fresh log - no previous TreeHeads exist.", util.LogIDPrefix(ctx))
return 0, s.SignRoot(ctx)
}

// There might be no work to be done. But we possibly still need to create an STH if the
// current one is too old. If there's work to be done then we'll be creating a root anyway.
if len(leaves) == 0 {
// We have nothing to integrate into the tree
tx.Commit()
if expiryFunc(currentRoot) {
// Current root is too old, sign one. Will use a new TX, safe as we have no writes
// pending in this one.
glog.V(2).Infof("%s: Current root is too old, create new STH", util.LogIDPrefix(ctx))
return 0, s.SignRoot(ctx)
}
return 0, nil
return 0, tx.Commit()
}

merkleTree, err := s.initMerkleTreeFromStorage(ctx, currentRoot, tx)
Expand Down
30 changes: 12 additions & 18 deletions log/sequencer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,6 @@ import (
"github.com/google/trillian/util"
)

// Func that says root never expires, so we can be sure tests will only sign / sequence when we
// expect them to
func rootNeverExpiresFunc(trillian.SignedLogRoot) bool {
return false
}

var treeHasher = merkle.NewRFC6962TreeHasher(crypto.NewSHA256())

// These can be shared between tests as they're never modified
Expand Down Expand Up @@ -230,7 +224,7 @@ func TestBeginTXFails(t *testing.T) {
params := testParameters{beginFails: true, skipDequeue: true, skipStoreSignedRoot: true}
c, ctx := createTestContext(ctrl, params)

leaves, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
leaves, err := c.sequencer.SequenceBatch(ctx, 1)
if leaves != 0 {
t.Fatalf("Unexpectedly sequenced %d leaves on error", leaves)
}
Expand All @@ -245,7 +239,7 @@ func TestSequenceWithNothingQueued(t *testing.T) {

c, ctx := createTestContext(ctrl, params)

leaves, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
leaves, err := c.sequencer.SequenceBatch(ctx, 1)
if leaves != 0 {
t.Fatalf("Unexpectedly sequenced %d leaves on error", leaves)
}
Expand All @@ -268,7 +262,7 @@ func TestGuardWindowPassthrough(t *testing.T) {
c, ctx := createTestContext(ctrl, params)
c.sequencer.SetGuardWindow(guardInterval)

leaves, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
leaves, err := c.sequencer.SequenceBatch(ctx, 1)
if leaves != 0 {
t.Fatalf("Expected no leaves sequenced when in guard interval but got: %d", leaves)
}
Expand All @@ -285,7 +279,7 @@ func TestDequeueError(t *testing.T) {
params := testParameters{dequeueLimit: 1, shouldRollback: true, dequeuedError: errors.New("dequeue")}
c, ctx := createTestContext(ctrl, params)

leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
testonly.EnsureErrorContains(t, err, "dequeue")
if leafCount != 0 {
t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
Expand All @@ -301,7 +295,7 @@ func TestLatestRootError(t *testing.T) {
latestSignedRoot: &testRoot16, latestSignedRootError: errors.New("root")}
c, ctx := createTestContext(ctrl, params)

leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
if leafCount != 0 {
t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
}
Expand All @@ -319,7 +313,7 @@ func TestUpdateSequencedLeavesError(t *testing.T) {
updatedLeavesError: errors.New("unsequenced")}
c, ctx := createTestContext(ctrl, params)

leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
if leafCount != 0 {
t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
}
Expand All @@ -337,7 +331,7 @@ func TestSetMerkleNodesError(t *testing.T) {
merkleNodesSetError: errors.New("setmerklenodes")}
c, ctx := createTestContext(ctrl, params)

leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
if leafCount != 0 {
t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
}
Expand All @@ -358,7 +352,7 @@ func TestStoreSignedRootError(t *testing.T) {
signingResult: []byte("signed")}
c, ctx := createTestContext(ctrl, params)

leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
if leafCount != 0 {
t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
}
Expand All @@ -378,7 +372,7 @@ func TestStoreSignedRootKeyManagerFails(t *testing.T) {
keyManagerError: errors.New("keymanagerfailed")}
c, ctx := createTestContext(ctrl, params)

leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
if leafCount != 0 {
t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
}
Expand All @@ -399,7 +393,7 @@ func TestStoreSignedRootSignerFails(t *testing.T) {
signingError: errors.New("signerfailed")}
c, ctx := createTestContext(ctrl, params)

leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
if leafCount != 0 {
t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
}
Expand All @@ -421,7 +415,7 @@ func TestCommitFails(t *testing.T) {
signingResult: []byte("signed")}
c, ctx := createTestContext(ctrl, params)

leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
if leafCount != 0 {
t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
}
Expand All @@ -444,7 +438,7 @@ func TestSequenceBatch(t *testing.T) {
signingResult: []byte("signed")}
c, ctx := createTestContext(ctrl, params)

leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
if err != nil {
t.Fatalf("Expected sequencing to succeed, but got err: %v", err)
}
Expand Down
12 changes: 1 addition & 11 deletions server/sequencer_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"time"

"github.com/golang/glog"
"github.com/google/trillian"
"github.com/google/trillian/crypto"
"github.com/google/trillian/log"
"github.com/google/trillian/merkle"
Expand All @@ -18,15 +17,6 @@ type SequencerManager struct {
cachedProvider *cachedLogStorageProvider
}

func isRootTooOld(ts util.TimeSource, maxAge time.Duration) log.CurrentRootExpiredFunc {
return func(root trillian.SignedLogRoot) bool {
rootTime := time.Unix(0, root.TimestampNanos)
rootAge := ts.Now().Sub(rootTime)

return rootAge > maxAge
}
}

// NewSequencerManager creates a new SequencerManager instance based on the provided KeyManager instance
// and guard window.
func NewSequencerManager(km crypto.KeyManager, p LogStorageProviderFunc, gw time.Duration) *SequencerManager {
Expand Down Expand Up @@ -71,7 +61,7 @@ func (s SequencerManager) ExecutePass(logIDs []int64, logctx LogOperationManager
sequencer := log.NewSequencer(merkle.NewRFC6962TreeHasher(crypto.NewSHA256()), logctx.timeSource, storage, s.keyManager)
sequencer.SetGuardWindow(s.guardWindow)

leaves, err := sequencer.SequenceBatch(ctx, logctx.batchSize, isRootTooOld(logctx.timeSource, logctx.signInterval))
leaves, err := sequencer.SequenceBatch(ctx, logctx.batchSize)

if err != nil {
glog.Warningf("%s: Error trying to sequence batch for: %v", util.LogIDPrefix(ctx), err)
Expand Down
34 changes: 0 additions & 34 deletions server/sequencer_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,40 +137,6 @@ func TestSequencerManagerSingleLogOneLeaf(t *testing.T) {
sm.ExecutePass([]int64{logID}, createTestContext(provider))
}

// Tests that a new root is signed if it's due even when there is no work to sequence.
// The various failure cases of SignRoot() are tested in the sequencer tests. This is
// an interaction test.
func TestSignsIfNoWorkAndRootExpired(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()

mockStorage := storage.NewMockLogStorage(mockCtrl)
mockTx := storage.NewMockLogTX(mockCtrl)
mockKeyManager := crypto.NewMockKeyManager(mockCtrl)
mockKeyManager.EXPECT().SignatureAlgorithm().AnyTimes().Return(trillian.SignatureAlgorithm_ECDSA)
logID := int64(1)
hasher := crypto.NewSHA256()

mockStorage.EXPECT().Begin().AnyTimes().Return(mockTx, nil)
mockTx.EXPECT().WriteRevision().AnyTimes().Return(writeRev)
mockTx.EXPECT().Commit().AnyTimes().Return(nil)
mockTx.EXPECT().LatestSignedLogRoot().AnyTimes().Return(testRoot0, nil)
mockTx.EXPECT().DequeueLeaves(50, fakeTime).Return([]trillian.LogLeaf{}, nil)
mockTx.EXPECT().StoreSignedLogRoot(updatedRootSignOnly).AnyTimes().Return(nil)

mockSigner := crypto.NewMockSigner(mockCtrl)
mockSigner.EXPECT().Sign(gomock.Any(), []byte{0xeb, 0x7d, 0xa1, 0x4f, 0x1e, 0x60, 0x91, 0x24, 0xa, 0xf7, 0x1c, 0xcd, 0xdb, 0xd4, 0xca, 0x38, 0x4b, 0x12, 0xe4, 0xa3, 0xcf, 0x80, 0x5, 0x55, 0x17, 0x71, 0x35, 0xaf, 0x80, 0x11, 0xa, 0x87}, hasher).Return([]byte("signed"), nil)
mockKeyManager.EXPECT().Signer().Return(mockSigner, nil)

provider := mockStorageProviderForSequencer(mockStorage)
sm := NewSequencerManager(mockKeyManager, provider, zeroDuration)

tc := createTestContext(provider)
// Lower the expiry so we can trigger a signing for a root older than 5 seconds
tc.signInterval = time.Second * 5
sm.ExecutePass([]int64{logID}, tc)
}

func TestSequencerManagerGuardWindow(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
Expand Down

0 comments on commit 48a95f0

Please sign in to comment.