Unexport HashLogRoot. Verify with VerifySignedLogRoot. (#1042)

* SignedLogRoot <-> VerifySignedLogRoot

SignLogRoot should produce a *trillian.SignedLogRoot
VerifyLogRoot should verify a *trillian.SignedLogRoot

This prevents clients from needing to parse the SignedLogRoot object
themslves. It also provides a single API point to maintain.

VerifySignedLogRoot will produce a *trillian.SignedLogRoot.
The idea is the force clients to only use data that has been returned
from a successful verification.

* Unexport HashLogRoot

* reviewer edits

client/log_verifier.go

@@ -77,11 +77,7 @@ func (c *LogVerifier) VerifyRoot(trusted, newRoot *trillian.SignedLogRoot,
 	}
 
 	// Verify SignedLogRoot signature.
-	hash, err := tcrypto.HashLogRoot(*newRoot)
-	if err != nil {
-		return err
-	}
-	if err := tcrypto.Verify(c.pubKey, hash, newRoot.Signature); err != nil {
+	if _, err := tcrypto.VerifySignedLogRoot(c.pubKey, newRoot); err != nil {
 		return err
 	}
 

client/log_verifier_test.go

@@ -36,24 +36,18 @@ func TestVerifyRootErrors(t *testing.T) {
 		t.Fatalf("Failed to load public key, err=%v", err)
 	}
 
-	signedRoot := trillian.SignedLogRoot{}
-	hash, err := tcrypto.HashLogRoot(signedRoot)
-	if err != nil {
-		t.Fatalf("HashLogRoot(): %v", err)
-	}
-	signature, err := signer.Sign(hash)
+	signedRoot, err := signer.SignLogRoot(&trillian.SignedLogRoot{})
 	if err != nil {
 		t.Fatal("Failed to create test signature")
 	}
-	signedRoot.Signature = signature
 
 	// Test execution
 	tests := []struct {
 		desc             string
 		trusted, newRoot *trillian.SignedLogRoot
 	}{
-		{desc: "newRootNil", trusted: &signedRoot, newRoot: nil},
-		{desc: "trustedNil", trusted: nil, newRoot: &signedRoot},
+		{desc: "newRootNil", trusted: signedRoot, newRoot: nil},
+		{desc: "trustedNil", trusted: nil, newRoot: signedRoot},
 	}
 	for _, test := range tests {
 		logVerifier := NewLogVerifier(rfc6962.DefaultHasher, pk)

crypto/data_formats.go

@@ -34,10 +34,10 @@ const (
 	mapKeyTreeSize       string = "TreeSize"
 )
 
-// HashLogRoot hashes SignedLogRoot objects using ObjectHash with
+// hashLogRoot hashes SignedLogRoot objects using ObjectHash with
 // "RootHash", "TimestampNanos", and "TreeSize", used as keys in
 // a map.
-func HashLogRoot(root trillian.SignedLogRoot) ([]byte, error) {
+func hashLogRoot(root trillian.SignedLogRoot) ([]byte, error) {
 	// Pull out the fields we want to hash.
 	// Caution: use string format for int64 values as they can overflow when
 	// JSON encoded otherwise (it uses floats). We want to be sure that people

crypto/data_formats_test.go

@@ -32,7 +32,7 @@ func TestHashLogRootKnownValue(t *testing.T) {
 		RootHash:       []byte("Some bytes that won't change"),
 		TreeSize:       167329345,
 	}
-	hash, err := HashLogRoot(root)
+	hash, err := hashLogRoot(root)
 	if err != nil {
 		t.Fatalf("HashLogRoot(): %v", err)
 	}
@@ -75,7 +75,7 @@ func TestHashLogRoot(t *testing.T) {
 			},
 		},
 	} {
-		hash, err := HashLogRoot(test.root)
+		hash, err := hashLogRoot(test.root)
 		if err != nil {
 			t.Fatalf("HashLogRoot(): %v", err)
 		}

crypto/signer.go

@@ -83,10 +83,9 @@ func (s *Signer) SignObject(obj interface{}) (*sigpb.DigitallySigned, error) {
 	return s.Sign(hash[:])
 }
 
-// SignLogRoot hashes and signs the supplied (to-be) SignedLogRoot and returns a
-// signature.  Hashing is performed by github.com/benlaurie/objecthash.
-func (s *Signer) SignLogRoot(root *trillian.SignedLogRoot) (*sigpb.DigitallySigned, error) {
-	hash, err := HashLogRoot(*root)
+// SignLogRoot returns a complete SignedLogRoot (including signature).
+func (s *Signer) SignLogRoot(root *trillian.SignedLogRoot) (*trillian.SignedLogRoot, error) {
+	hash, err := hashLogRoot(*root)
 	if err != nil {
 		return nil, err
 	}
@@ -96,7 +95,10 @@ func (s *Signer) SignLogRoot(root *trillian.SignedLogRoot) (*sigpb.DigitallySign
 		return nil, err
 	}
 
-	return signature, nil
+	ret := *root // Don't modify the input variable.
+	ret.Signature = signature
+
+	return &ret, nil
 }
 
 // SignMapRoot hashes and signs the supplied (to-be) SignedMapRoot and returns a

crypto/signer_test.go

@@ -90,7 +90,7 @@ func TestSignWithSignedLogRoot_SignerFails(t *testing.T) {
 
 	s := testonly.NewSignerWithErr(key, errors.New("signfail"))
 	root := trillian.SignedLogRoot{TimestampNanos: 2267709, RootHash: []byte("Islington"), TreeSize: 2}
-	hash, err := HashLogRoot(root)
+	hash, err := hashLogRoot(root)
 	if err != nil {
 		t.Fatalf("HashLogRoot(): %v", err)
 	}
@@ -110,28 +110,22 @@ func TestSignLogRoot(t *testing.T) {
 	}{
 		{root: trillian.SignedLogRoot{TimestampNanos: 2267709, RootHash: []byte("Islington"), TreeSize: 2}},
 	} {
-		sig, err := signer.SignLogRoot(&test.root)
+		slr, err := signer.SignLogRoot(&test.root)
 		if err != nil {
 			t.Errorf("Failed to sign log root: %v", err)
 			continue
 		}
-		if got := len(sig.Signature); got == 0 {
+		if got := len(slr.Signature.Signature); got == 0 {
 			t.Errorf("len(sig): %v, want > 0", got)
 		}
-		if got, want := sig.HashAlgorithm, sigpb.DigitallySigned_SHA256; got != want {
+		if got, want := slr.Signature.HashAlgorithm, sigpb.DigitallySigned_SHA256; got != want {
 			t.Errorf("Hash alg incorrect, got %s expected %s", got, want)
 		}
-		if got, want := sig.SignatureAlgorithm, sigpb.DigitallySigned_ECDSA; got != want {
+		if got, want := slr.Signature.SignatureAlgorithm, sigpb.DigitallySigned_ECDSA; got != want {
 			t.Errorf("Sig alg incorrect, got %s expected %s", got, want)
 		}
 		// Check that the signature is correct
-		obj, err := HashLogRoot(test.root)
-		if err != nil {
-			t.Errorf("HashLogRoot err: got %v want nil", err)
-			continue
-		}
-
-		if err := Verify(key.Public(), obj, sig); err != nil {
+		if _, err := VerifySignedLogRoot(key.Public(), slr); err != nil {
 			t.Errorf("Verify(%v) failed: %v", test.root, err)
 		}
 	}

crypto/verifier.go

@@ -25,6 +25,7 @@ import (
 	"math/big"
 
 	"github.com/benlaurie/objecthash/go/objecthash"
+	"github.com/google/trillian"
 	"github.com/google/trillian/crypto/sigpb"
 )
 
@@ -36,6 +37,20 @@ var (
 	}
 )
 
+// VerifySignedLogRoot verifies the SignedLogRoot and returns its contents.
+func VerifySignedLogRoot(pub crypto.PublicKey, r *trillian.SignedLogRoot) (*trillian.SignedLogRoot, error) {
+	// Verify SignedLogRoot signature.
+	hash, err := hashLogRoot(*r)
+	if err != nil {
+		return nil, err
+	}
+	if err := Verify(pub, hash, r.Signature); err != nil {
+		return nil, err
+	}
+	return r, nil
+
+}
+
 // VerifyObject verifies the output of Signer.SignObject.
 func VerifyObject(pub crypto.PublicKey, obj interface{}, sig *sigpb.DigitallySigned) error {
 	j, err := json.Marshal(obj)

log/sequencer.go

@@ -385,19 +385,17 @@ func (s Sequencer) IntegrateBatch(ctx context.Context, tree *trillian.Tree, limi
 
 		// Create the log root ready for signing
 		seqTreeSize.Set(float64(merkleTree.Size()), label)
-		newLogRoot = &trillian.SignedLogRoot{
+		newLogRoot, err := s.signer.SignLogRoot(&trillian.SignedLogRoot{
 			RootHash:       merkleTree.CurrentRoot(),
 			TimestampNanos: s.timeSource.Now().UnixNano(),
 			TreeSize:       merkleTree.Size(),
 			LogId:          currentRoot.LogId,
 			TreeRevision:   newVersion,
-		}
-		sig, err := s.signer.SignLogRoot(newLogRoot)
+		})
 		if err != nil {
 			glog.Warningf("%v: signer failed to sign root: %v", tree.TreeId, err)
 			return err
 		}
-		newLogRoot.Signature = sig
 
 		if err := tx.StoreSignedLogRoot(ctx, *newLogRoot); err != nil {
 			glog.Warningf("%v: failed to write updated tree root: %v", tree.TreeId, err)
@@ -457,19 +455,17 @@ func (s Sequencer) SignRoot(ctx context.Context, tree *trillian.Tree) error {
 		if err != nil {
 			return err
 		}
-		newLogRoot := &trillian.SignedLogRoot{
+		newLogRoot, err := s.signer.SignLogRoot(&trillian.SignedLogRoot{
 			RootHash:       merkleTree.CurrentRoot(),
 			TimestampNanos: s.timeSource.Now().UnixNano(),
 			TreeSize:       merkleTree.Size(),
 			LogId:          currentRoot.LogId,
 			TreeRevision:   currentRoot.TreeRevision + 1,
-		}
-		sig, err := s.signer.SignLogRoot(newLogRoot)
+		})
 		if err != nil {
 			glog.Warningf("%v: signer failed to sign root: %v", tree.TreeId, err)
 			return err
 		}
-		newLogRoot.Signature = sig
 
 		// Store the new root and we're done
 		if err := tx.StoreSignedLogRoot(ctx, *newLogRoot); err != nil {

server/log_rpc_server.go

@@ -599,24 +599,22 @@ func (t *TrillianLogRPCServer) InitLog(ctx context.Context, req *trillian.InitLo
 			return status.Errorf(codes.AlreadyExists, "log is already initialised")
 		}
 
-		newRoot = &trillian.SignedLogRoot{
-			RootHash:       hasher.EmptyRoot(),
-			TimestampNanos: t.timeSource.Now().UnixNano(),
-			TreeSize:       0,
-			LogId:          logID,
-			TreeRevision:   0,
-		}
-
 		signer, err := trees.Signer(ctx, tree)
 		if err != nil {
 			return status.Errorf(codes.FailedPrecondition, "Signer() :%v", err)
 		}
 
-		sig, err := signer.SignLogRoot(newRoot)
+		root, err := signer.SignLogRoot(&trillian.SignedLogRoot{
+			RootHash:       hasher.EmptyRoot(),
+			TimestampNanos: t.timeSource.Now().UnixNano(),
+			TreeSize:       0,
+			LogId:          logID,
+			TreeRevision:   0,
+		})
 		if err != nil {
 			return err
 		}
-		newRoot.Signature = sig
+		newRoot = root
 
 		if err := tx.StoreSignedLogRoot(ctx, *newRoot); err != nil {
 			return status.Errorf(codes.FailedPrecondition, "StoreSignedLogRoot(): %v", err)

storage/tools/dump_tree/dumplib/dumplib.go

@@ -202,17 +202,16 @@ func createTree(as storage.AdminStorage, ls storage.LogStorage) (*trillian.Tree,
 		glog.Fatalf("Creating signer: %v", err)
 	}
 
-	sthZero := trillian.SignedLogRoot{
+	sthZero, err := tSigner.SignLogRoot(&trillian.SignedLogRoot{
 		LogId:    createdTree.TreeId,
 		RootHash: hasher.EmptyRoot(),
-	}
-	sthZero.Signature, err = tSigner.SignLogRoot(&sthZero)
+	})
 	if err != nil {
 		glog.Fatalf("SignLogRoot: %v", err)
 	}
 
 	err = ls.ReadWriteTransaction(ctx, createdTree, func(ctx context.Context, tx storage.LogTreeTX) error {
-		if err := tx.StoreSignedLogRoot(ctx, sthZero); err != nil {
+		if err := tx.StoreSignedLogRoot(ctx, *sthZero); err != nil {
 			glog.Fatalf("StoreSignedLogRoot: %v", err)
 		}
 		return nil