api: fix inclusion proof verification flake (#956)

* wip: demonstrate the problem with inclusion proof verification Signed-off-by: Asra Ali <asraa@google.com> update with fix Signed-off-by: Asra Ali <asraa@google.com> fix with root resp Signed-off-by: Asra Ali <asraa@google.com> fix Signed-off-by: Asra Ali <asraa@google.com> fix Signed-off-by: Asra Ali <asraa@google.com> fix Signed-off-by: Asra Ali <asraa@google.com> update Signed-off-by: Asra Ali <asraa@google.com> * Address comments and add test Signed-off-by: Asra Ali <asraa@google.com>
sigstore · Aug 9, 2022 · 0140a44 · 0140a44
1 parent 77bff12
commit 0140a44
Show file tree

Hide file tree

Showing 2 changed files with 116 additions and 18 deletions.
diff --git a/pkg/api/trillian_client.go b/pkg/api/trillian_client.go
@@ -69,6 +69,14 @@ type Response struct {
 	getConsistencyProofResult *trillian.GetConsistencyProofResponse
 }
 
+func unmarshalLogRoot(logRoot []byte) (types.LogRootV1, error) {
+	var root types.LogRootV1
+	if err := root.UnmarshalBinary(logRoot); err != nil {
+		return types.LogRootV1{}, err
+	}
+	return root, nil
+}
+
 func (t *TrillianClient) root() (types.LogRootV1, error) {
 	rqst := &trillian.GetLatestSignedLogRootRequest{
 		LogId: t.logID,
@@ -77,11 +85,7 @@ func (t *TrillianClient) root() (types.LogRootV1, error) {
 	if err != nil {
 		return types.LogRootV1{}, err
 	}
-	var root types.LogRootV1
-	if err := root.UnmarshalBinary(resp.SignedLogRoot.LogRoot); err != nil {
-		return types.LogRootV1{}, err
-	}
-	return root, nil
+	return unmarshalLogRoot(resp.SignedLogRoot.LogRoot)
 }
 
 func (t *TrillianClient) addLeaf(byteValue []byte) *Response {
@@ -210,11 +214,19 @@ func (t *TrillianClient) getLeafAndProofByIndex(index int64) *Response {
 	ctx, cancel := context.WithTimeout(t.context, 20*time.Second)
 	defer cancel()
 
-	root, err := t.root()
+	rootResp := t.getLatest(0)
+	if rootResp.err != nil {
+		return &Response{
+			status: status.Code(rootResp.err),
+			err:    rootResp.err,
+		}
+	}
+
+	root, err := unmarshalLogRoot(rootResp.getLatestResult.SignedLogRoot.LogRoot)
 	if err != nil {
 		return &Response{
-			status: status.Code(err),
-			err:    err,
+			status: status.Code(rootResp.err),
+			err:    rootResp.err,
 		}
 	}
 
@@ -232,24 +244,39 @@ func (t *TrillianClient) getLeafAndProofByIndex(index int64) *Response {
 				err:    err,
 			}
 		}
+		return &Response{
+			status: status.Code(err),
+			err:    err,
+			getLeafAndProofResult: &trillian.GetEntryAndProofResponse{
+				Proof:         resp.Proof,
+				Leaf:          resp.Leaf,
+				SignedLogRoot: rootResp.getLatestResult.SignedLogRoot,
+			},
+		}
 	}
 
 	return &Response{
-		status:                status.Code(err),
-		err:                   err,
-		getLeafAndProofResult: resp,
+		status: status.Code(err),
+		err:    err,
 	}
 }
 
 func (t *TrillianClient) getProofByHash(hashValue []byte) *Response {
 	ctx, cancel := context.WithTimeout(t.context, 20*time.Second)
 	defer cancel()
 
-	root, err := t.root()
+	rootResp := t.getLatest(0)
+	if rootResp.err != nil {
+		return &Response{
+			status: status.Code(rootResp.err),
+			err:    rootResp.err,
+		}
+	}
+	root, err := unmarshalLogRoot(rootResp.getLatestResult.SignedLogRoot.LogRoot)
 	if err != nil {
 		return &Response{
-			status: status.Code(err),
-			err:    err,
+			status: status.Code(rootResp.err),
+			err:    rootResp.err,
 		}
 	}
 
@@ -263,20 +290,27 @@ func (t *TrillianClient) getProofByHash(hashValue []byte) *Response {
 	if resp != nil {
 		v := client.NewLogVerifier(rfc6962.DefaultHasher)
 		for _, proof := range resp.Proof {
-
 			if err := v.VerifyInclusionByHash(&root, hashValue, proof); err != nil {
 				return &Response{
 					status: status.Code(err),
 					err:    err,
 				}
 			}
 		}
+		// Return an inclusion proof response with the requested
+		return &Response{
+			status: status.Code(err),
+			err:    err,
+			getProofResult: &trillian.GetInclusionProofByHashResponse{
+				Proof:         resp.Proof,
+				SignedLogRoot: rootResp.getLatestResult.SignedLogRoot,
+			},
+		}
 	}
 
 	return &Response{
-		status:         status.Code(err),
-		err:            err,
-		getProofResult: resp,
+		status: status.Code(err),
+		err:    err,
 	}
 }
 

diff --git a/tests/e2e_test.go b/tests/e2e_test.go
@@ -30,6 +30,7 @@ import (
 	"encoding/json"
 	"encoding/pem"
 	"fmt"
+	"golang.org/x/sync/errgroup"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -771,3 +772,66 @@ func TestTufVerifyUpload(t *testing.T) {
 	out = runCli(t, "search", "--public-key", rootPath, "--pki-format", "tuf")
 	outputContains(t, out, uuid)
 }
+
+// Regression test for https://github.com/sigstore/rekor/pull/956
+// Requesting an inclusion proof concurrently with an entry write triggers
+// a race where the inclusion proof returned does not verify because the
+// tree head changes.
+func TestInclusionProofRace(t *testing.T) {
+	// Create a random artifact and sign it.
+	artifactPath := filepath.Join(t.TempDir(), "artifact")
+	sigPath := filepath.Join(t.TempDir(), "signature.asc")
+
+	createdX509SignedArtifact(t, artifactPath, sigPath)
+	dataBytes, _ := ioutil.ReadFile(artifactPath)
+	h := sha256.Sum256(dataBytes)
+	dataSHA := hex.EncodeToString(h[:])
+
+	// Write the public key to a file
+	pubPath := filepath.Join(t.TempDir(), "pubKey.asc")
+	if err := ioutil.WriteFile(pubPath, []byte(rsaCert), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	// Upload an entry
+	runCli(t, "upload", "--type=hashedrekord", "--pki-format=x509", "--artifact-hash", dataSHA, "--signature", sigPath, "--public-key", pubPath)
+
+	// Constantly uploads new signatures on an entry.
+	var uploadRoutine = func(pubPath string) error {
+		// Create a random artifact and sign it.
+		artifactPath := filepath.Join(t.TempDir(), "artifact")
+		sigPath := filepath.Join(t.TempDir(), "signature.asc")
+
+		createdX509SignedArtifact(t, artifactPath, sigPath)
+		dataBytes, _ := ioutil.ReadFile(artifactPath)
+		h := sha256.Sum256(dataBytes)
+		dataSHA := hex.EncodeToString(h[:])
+
+		// Upload an entry
+		out := runCli(t, "upload", "--type=hashedrekord", "--pki-format=x509", "--artifact-hash", dataSHA, "--signature", sigPath, "--public-key", pubPath)
+		outputContains(t, out, "Created entry at")
+
+		return nil
+	}
+
+	// Attempts to verify the original entry.
+	var verifyRoutine = func(dataSHA, sigPath, pubPath string) error {
+		out := runCli(t, "verify", "--type=hashedrekord", "--pki-format=x509", "--artifact-hash", dataSHA, "--signature", sigPath, "--public-key", pubPath)
+
+		if strings.Contains(out, "calculated root") || strings.Contains(out, "wrong") {
+			return fmt.Errorf(out)
+		}
+
+		return nil
+	}
+
+	var g errgroup.Group
+	for i := 0; i < 50; i++ {
+		g.Go(func() error { return uploadRoutine(pubPath) })
+		g.Go(func() error { return verifyRoutine(dataSHA, sigPath, pubPath) })
+	}
+
+	if err := g.Wait(); err != nil {
+		t.Fatal(err)
+	}
+}