From a6546d092307f6e0d396068066033b434203824d Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 12 Sep 2017 16:27:41 +0100 Subject: [PATCH] Allow log server to return consistency proofs where first=second (#819) * Allow log server to return proofs where first=second This is for additional compatiblity with the older C++ log server code. * lint fix * Fix typo * Fix inconsistent error message. --- server/log_rpc_server_test.go | 86 +++++++++++++++++++++++++++++------ server/validate.go | 4 +- 2 files changed, 73 insertions(+), 17 deletions(-) diff --git a/server/log_rpc_server_test.go b/server/log_rpc_server_test.go index c429495a12..f93552783d 100644 --- a/server/log_rpc_server_test.go +++ b/server/log_rpc_server_test.go @@ -67,6 +67,9 @@ var ( getConsistencyProofRequest25 = trillian.GetConsistencyProofRequest{LogId: logID1, FirstTreeSize: 10, SecondTreeSize: 25} getConsistencyProofRequest7 = trillian.GetConsistencyProofRequest{LogId: logID1, FirstTreeSize: 4, SecondTreeSize: 7} + getConsistencyProofRequest44 = trillian.GetConsistencyProofRequest{LogId: logID1, FirstTreeSize: 4, SecondTreeSize: 4} + getConsistencyProofRequest48 = trillian.GetConsistencyProofRequest{LogId: logID1, FirstTreeSize: 4, SecondTreeSize: 8} + getConsistencyProofRequest54 = trillian.GetConsistencyProofRequest{LogId: logID1, FirstTreeSize: 5, SecondTreeSize: 4} nodeIdsInclusionSize7Index2 = []storage.NodeID{ stestonly.MustCreateNodeIDForTreeCoords(0, 3, 64), @@ -1231,38 +1234,91 @@ func TestGetConsistencyProofCommitFails(t *testing.T) { test.executeCommitFailsTest(t, getConsistencyProofRequest7.LogId) } -func TestGetConsistencyProof(t *testing.T) { +// Ask for a proof from size 4 to 8 but the tree is only size 7. This should fail. +func TestGetConsistencyProofOutsideTree(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() mockStorage := storage.NewMockLogStorage(ctrl) mockTx := storage.NewMockLogTreeTX(ctrl) - mockStorage.EXPECT().SnapshotForTree(gomock.Any(), getConsistencyProofRequest7.LogId).Return(mockTx, nil) + mockStorage.EXPECT().SnapshotForTree(gomock.Any(), getConsistencyProofRequest48.LogId).Return(mockTx, nil) mockTx.EXPECT().LatestSignedLogRoot(gomock.Any()).Return(signedRoot1, nil) - mockTx.EXPECT().ReadRevision().Return(signedRoot1.TreeRevision) - mockTx.EXPECT().GetMerkleNodes(gomock.Any(), revision1, nodeIdsConsistencySize4ToSize7).Return([]storage.Node{{NodeID: stestonly.MustCreateNodeIDForTreeCoords(2, 1, 64), NodeRevision: 3, Hash: []byte("nodehash")}}, nil) - mockTx.EXPECT().Commit().Return(nil) mockTx.EXPECT().Close().Return(nil) registry := extension.Registry{ - AdminStorage: mockAdminStorage(ctrl, getConsistencyProofRequest7.LogId), + AdminStorage: mockAdminStorage(ctrl, getConsistencyProofRequest48.LogId), LogStorage: mockStorage, } server := NewTrillianLogRPCServer(registry, fakeTimeSource) - response, err := server.GetConsistencyProof(context.Background(), &getConsistencyProofRequest7) - if err != nil { - t.Fatalf("failed to get consistency proof: %v", err) + _, err := server.GetConsistencyProof(context.Background(), &getConsistencyProofRequest48) + if err == nil { + t.Fatal("incorrectly returned a proof for out of range tree size") } +} - // Ensure we got the expected proof - expectedProof := trillian.Proof{ - LeafIndex: 0, - Hashes: [][]byte{[]byte("nodehash")}, +// Ask for a proof from size 5 to 4, testing the boundary condition. This request should fail +// before making any storage requests. +func TestGetConsistencyProofRangeError(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockStorage := storage.NewMockLogStorage(ctrl) + registry := extension.Registry{ + AdminStorage: mockAdminStorage(ctrl, getConsistencyProofRequest54.LogId), + LogStorage: mockStorage, } - if !proto.Equal(response.Proof, &expectedProof) { - t.Fatalf("expected proof: %v but got: %v", expectedProof, response.Proof) + server := NewTrillianLogRPCServer(registry, fakeTimeSource) + + _, err := server.GetConsistencyProof(context.Background(), &getConsistencyProofRequest54) + if err == nil { + t.Fatal("incorrectly returned a proof for out of range tree size") + } +} + +func TestGetConsistencyProof(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + // getConsistencyProofRequest7 - tests a normal request which should succeed + // getConsistencyProofRequest44 - tests an edge condition we used to reject but now support + // for compatibility with the older C++ log servers. + testCases := []trillian.GetConsistencyProofRequest{getConsistencyProofRequest7, getConsistencyProofRequest44} + nodeIDs := [][]storage.NodeID{nodeIdsConsistencySize4ToSize7, {}} + hashes := [][][]byte{{[]byte("nodehash")}, {}} + nodes := [][]storage.Node{{{NodeID: stestonly.MustCreateNodeIDForTreeCoords(2, 1, 64), NodeRevision: 3, Hash: []byte("nodehash")}}, {}} + + for i := range testCases { + mockStorage := storage.NewMockLogStorage(ctrl) + mockTx := storage.NewMockLogTreeTX(ctrl) + mockStorage.EXPECT().SnapshotForTree(gomock.Any(), testCases[i].LogId).Return(mockTx, nil) + + mockTx.EXPECT().LatestSignedLogRoot(gomock.Any()).Return(signedRoot1, nil) + mockTx.EXPECT().ReadRevision().Return(signedRoot1.TreeRevision) + mockTx.EXPECT().GetMerkleNodes(gomock.Any(), revision1, nodeIDs[i]).Return(nodes[i], nil) + mockTx.EXPECT().Commit().Return(nil) + mockTx.EXPECT().Close().Return(nil) + + registry := extension.Registry{ + AdminStorage: mockAdminStorage(ctrl, testCases[i].LogId), + LogStorage: mockStorage, + } + server := NewTrillianLogRPCServer(registry, fakeTimeSource) + + response, err := server.GetConsistencyProof(context.Background(), &testCases[i]) + if err != nil { + t.Fatalf("failed to get consistency proof: %v", err) + } + + // Ensure we got the expected proof + expectedProof := trillian.Proof{ + LeafIndex: 0, + Hashes: hashes[i], + } + if !proto.Equal(response.Proof, &expectedProof) { + t.Fatalf("expected proof: %v but got: %v", expectedProof, response.Proof) + } } } diff --git a/server/validate.go b/server/validate.go index 9633079a39..3822b7275e 100644 --- a/server/validate.go +++ b/server/validate.go @@ -51,8 +51,8 @@ func validateGetConsistencyProofRequest(req *trillian.GetConsistencyProofRequest if req.SecondTreeSize <= 0 { return status.Errorf(codes.InvalidArgument, "GetConsistencyProofRequest.SecondTreeSize: %v, want > 0", req.SecondTreeSize) } - if req.SecondTreeSize <= req.FirstTreeSize { - return status.Errorf(codes.InvalidArgument, "GetConsistencyProofRequest.FirstTreeSize: %v < GetConsistencyProofRequest.SecondTreeSize: %v, want > ", req.FirstTreeSize, req.SecondTreeSize) + if req.SecondTreeSize < req.FirstTreeSize { + return status.Errorf(codes.InvalidArgument, "GetConsistencyProofRequest.FirstTreeSize: %v < GetConsistencyProofRequest.SecondTreeSize: %v, want >= ", req.FirstTreeSize, req.SecondTreeSize) } return nil }