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.

Sign up for GitHub

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

Remove the code for signing log roots if the last one is too old. #266

Merged
merged 2 commits into from
Dec 16, 2016
Merged

Remove the code for signing log roots if the last one is too old. #266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0b66fd9
Remove the code for signing log roots if the last one is too old.
Martin2112 Dec 13, 2016
f1ab9e8
Create a root in sequencing if none already exists.
Martin2112 Dec 13, 2016
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
18 changes: 4 additions & 14 deletions log/sequencer.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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