Skip to content

Commit 0140a44

Browse files
asraaasraa
authored
api: fix inclusion proof verification flake (sigstore#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>
1 parent 77bff12 commit 0140a44

File tree

2 files changed

+116
-18
lines changed

2 files changed

+116
-18
lines changed

‎pkg/api/trillian_client.go‎

Lines changed: 52 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,14 @@ type Response struct {
6969
getConsistencyProofResult *trillian.GetConsistencyProofResponse
7070
}
7171

72+
func unmarshalLogRoot(logRoot []byte) (types.LogRootV1, error) {
73+
var root types.LogRootV1
74+
if err := root.UnmarshalBinary(logRoot); err != nil {
75+
return types.LogRootV1{}, err
76+
}
77+
return root, nil
78+
}
79+
7280
func (t *TrillianClient) root() (types.LogRootV1, error) {
7381
rqst := &trillian.GetLatestSignedLogRootRequest{
7482
LogId: t.logID,
@@ -77,11 +85,7 @@ func (t *TrillianClient) root() (types.LogRootV1, error) {
7785
if err != nil {
7886
return types.LogRootV1{}, err
7987
}
80-
var root types.LogRootV1
81-
if err := root.UnmarshalBinary(resp.SignedLogRoot.LogRoot); err != nil {
82-
return types.LogRootV1{}, err
83-
}
84-
return root, nil
88+
return unmarshalLogRoot(resp.SignedLogRoot.LogRoot)
8589
}
8690

8791
func (t *TrillianClient) addLeaf(byteValue []byte) *Response {
@@ -210,11 +214,19 @@ func (t *TrillianClient) getLeafAndProofByIndex(index int64) *Response {
210214
ctx, cancel := context.WithTimeout(t.context, 20*time.Second)
211215
defer cancel()
212216

213-
root, err := t.root()
217+
rootResp := t.getLatest(0)
218+
if rootResp.err != nil {
219+
return &Response{
220+
status: status.Code(rootResp.err),
221+
err: rootResp.err,
222+
}
223+
}
224+
225+
root, err := unmarshalLogRoot(rootResp.getLatestResult.SignedLogRoot.LogRoot)
214226
if err != nil {
215227
return &Response{
216-
status: status.Code(err),
217-
err: err,
228+
status: status.Code(rootResp.err),
229+
err: rootResp.err,
218230
}
219231
}
220232

@@ -232,24 +244,39 @@ func (t *TrillianClient) getLeafAndProofByIndex(index int64) *Response {
232244
err: err,
233245
}
234246
}
247+
return &Response{
248+
status: status.Code(err),
249+
err: err,
250+
getLeafAndProofResult: &trillian.GetEntryAndProofResponse{
251+
Proof: resp.Proof,
252+
Leaf: resp.Leaf,
253+
SignedLogRoot: rootResp.getLatestResult.SignedLogRoot,
254+
},
255+
}
235256
}
236257

237258
return &Response{
238-
status: status.Code(err),
239-
err: err,
240-
getLeafAndProofResult: resp,
259+
status: status.Code(err),
260+
err: err,
241261
}
242262
}
243263

244264
func (t *TrillianClient) getProofByHash(hashValue []byte) *Response {
245265
ctx, cancel := context.WithTimeout(t.context, 20*time.Second)
246266
defer cancel()
247267

248-
root, err := t.root()
268+
rootResp := t.getLatest(0)
269+
if rootResp.err != nil {
270+
return &Response{
271+
status: status.Code(rootResp.err),
272+
err: rootResp.err,
273+
}
274+
}
275+
root, err := unmarshalLogRoot(rootResp.getLatestResult.SignedLogRoot.LogRoot)
249276
if err != nil {
250277
return &Response{
251-
status: status.Code(err),
252-
err: err,
278+
status: status.Code(rootResp.err),
279+
err: rootResp.err,
253280
}
254281
}
255282

@@ -263,20 +290,27 @@ func (t *TrillianClient) getProofByHash(hashValue []byte) *Response {
263290
if resp != nil {
264291
v := client.NewLogVerifier(rfc6962.DefaultHasher)
265292
for _, proof := range resp.Proof {
266-
267293
if err := v.VerifyInclusionByHash(&root, hashValue, proof); err != nil {
268294
return &Response{
269295
status: status.Code(err),
270296
err: err,
271297
}
272298
}
273299
}
300+
// Return an inclusion proof response with the requested
301+
return &Response{
302+
status: status.Code(err),
303+
err: err,
304+
getProofResult: &trillian.GetInclusionProofByHashResponse{
305+
Proof: resp.Proof,
306+
SignedLogRoot: rootResp.getLatestResult.SignedLogRoot,
307+
},
308+
}
274309
}
275310

276311
return &Response{
277-
status: status.Code(err),
278-
err: err,
279-
getProofResult: resp,
312+
status: status.Code(err),
313+
err: err,
280314
}
281315
}
282316

‎tests/e2e_test.go‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"encoding/json"
3131
"encoding/pem"
3232
"fmt"
33+
"golang.org/x/sync/errgroup"
3334
"io/ioutil"
3435
"os"
3536
"os/exec"
@@ -771,3 +772,66 @@ func TestTufVerifyUpload(t *testing.T) {
771772
out = runCli(t, "search", "--public-key", rootPath, "--pki-format", "tuf")
772773
outputContains(t, out, uuid)
773774
}
775+
776+
// Regression test for https://github.com/sigstore/rekor/pull/956
777+
// Requesting an inclusion proof concurrently with an entry write triggers
778+
// a race where the inclusion proof returned does not verify because the
779+
// tree head changes.
780+
func TestInclusionProofRace(t *testing.T) {
781+
// Create a random artifact and sign it.
782+
artifactPath := filepath.Join(t.TempDir(), "artifact")
783+
sigPath := filepath.Join(t.TempDir(), "signature.asc")
784+
785+
createdX509SignedArtifact(t, artifactPath, sigPath)
786+
dataBytes, _ := ioutil.ReadFile(artifactPath)
787+
h := sha256.Sum256(dataBytes)
788+
dataSHA := hex.EncodeToString(h[:])
789+
790+
// Write the public key to a file
791+
pubPath := filepath.Join(t.TempDir(), "pubKey.asc")
792+
if err := ioutil.WriteFile(pubPath, []byte(rsaCert), 0644); err != nil {
793+
t.Fatal(err)
794+
}
795+
796+
// Upload an entry
797+
runCli(t, "upload", "--type=hashedrekord", "--pki-format=x509", "--artifact-hash", dataSHA, "--signature", sigPath, "--public-key", pubPath)
798+
799+
// Constantly uploads new signatures on an entry.
800+
var uploadRoutine = func(pubPath string) error {
801+
// Create a random artifact and sign it.
802+
artifactPath := filepath.Join(t.TempDir(), "artifact")
803+
sigPath := filepath.Join(t.TempDir(), "signature.asc")
804+
805+
createdX509SignedArtifact(t, artifactPath, sigPath)
806+
dataBytes, _ := ioutil.ReadFile(artifactPath)
807+
h := sha256.Sum256(dataBytes)
808+
dataSHA := hex.EncodeToString(h[:])
809+
810+
// Upload an entry
811+
out := runCli(t, "upload", "--type=hashedrekord", "--pki-format=x509", "--artifact-hash", dataSHA, "--signature", sigPath, "--public-key", pubPath)
812+
outputContains(t, out, "Created entry at")
813+
814+
return nil
815+
}
816+
817+
// Attempts to verify the original entry.
818+
var verifyRoutine = func(dataSHA, sigPath, pubPath string) error {
819+
out := runCli(t, "verify", "--type=hashedrekord", "--pki-format=x509", "--artifact-hash", dataSHA, "--signature", sigPath, "--public-key", pubPath)
820+
821+
if strings.Contains(out, "calculated root") || strings.Contains(out, "wrong") {
822+
return fmt.Errorf(out)
823+
}
824+
825+
return nil
826+
}
827+
828+
var g errgroup.Group
829+
for i := 0; i < 50; i++ {
830+
g.Go(func() error { return uploadRoutine(pubPath) })
831+
g.Go(func() error { return verifyRoutine(dataSHA, sigPath, pubPath) })
832+
}
833+
834+
if err := g.Wait(); err != nil {
835+
t.Fatal(err)
836+
}
837+
}

0 commit comments

Comments
 (0)