feat: add unit test for online tlog verification

[vishal-chdhry]

Summary

Closes: #53
Adds test for online verification of artifact transparency log using mock rekor

Release Note

Documentation

[vishal-chdhry]

@cmurphy @haydentherapper I tried to add mock test by creating mock Rekor Entry client and adding inclusion proof to GenerateTlogEntry but I could not figure out how to:

1. generate the correct inclusion proof checkpoint and,
2. generate hashes required to compute the inclusion proof

It kept on failing while verifying rekorLogEntry. I don't know enough about rekor internals to create them properly. Meanwhile I have created a test that verifies one of the examples using sigstore public rekor.

[vishal-chdhry]

The test that uses public rekor is passing

I have also added the unfinished test to the PR, What I don't understand is

1. What will be the root hash
2. How to calculate the "hashes"
3. Right way to calculate checkpoints

[cmurphy]

What will be the root hash

The CT RFC has an explainer on merkle tree hash calculations: https://www.rfc-editor.org/rfc/rfc6962.html#section-2.1

The simplest merkle tree will contain only a single leaf, and the root hash will be the SHA-256 hash of the leaf prepended by a null character. The inclusion proof won't need to include additional hashes because there is only one hash needed to verify the leaf's inclusion in the log: https://gist.github.com/cmurphy/1b1203f4fec5b2363c30901d4684c2c4#file-treesize1-go

I think simplifying GenerateTlogEntry to make the tree only have one node is probably fine. The unit test doesn't need to validate the transparency-dev merkle library, just the sigstore-go usage of it.

If you did need to generate a more complex tree, the RFC explains how the root hash is calculated by concatenating the root hashes of its subtrees, and it would look something like this: https://gist.github.com/cmurphy/1b1203f4fec5b2363c30901d4684c2c4#file-treesize2-go

The test that uses public rekor is passing

It would be better if the unit tests did not make real network calls to the public infrastructure. Unit tests should be self-contained and never rely on external services, because:

1. If the external service is down or lagging, the CI running the unit tests will also slow down
2. Unit tests are run often and will significantly add to the network traffic hitting the public infrastructure, which could artificially degrade the service or increase infrastructure costs
3. Developers may want to run unit tests on a machine not connected to the internet

[vishal-chdhry]

Thank you so much for the amazing explanation @cmurphy, I learned a lot about how tlogs actually work.

I fixed inclusion and checkpoint but I still had this problem where SET verification were failing. I noticed that while verifying SETs in offline mode we are converting the log IDs to hex:

sigstore-go/pkg/tlog/entry.go

Lines 267 to 272 in 5845298

	rekorPayload := RekorPayload{
	Body: entry.logEntryAnon.Body,
	IntegratedTime: *entry.logEntryAnon.IntegratedTime,
	LogIndex: *entry.logEntryAnon.LogIndex,
	LogID: hex.EncodeToString([]byte(*entry.logEntryAnon.LogID)),
	}

But the rekor library does not do that https://github.com/sigstore/rekor/blob/8ca864f6b2bcece1b0b9b004938375bbe8fff03c/pkg/verify/verify.go#L190-L195

When I convert logID to hex in my mock log entry anon, SET verification using rekor library starts to pass, see: 50f5a75 (#296)

Is there a reason why we do that? I don't understand why I should convert it to hex in my mock to make it work

[cmurphy]

Is there a reason why we do that? I don't understand why I should convert it to hex in my mock to make it work

It looks like rekor stores the LogID of the log entry as hex encoded:

https://github.com/sigstore/rekor/blob/1e6728717d221a87792a8fc0c4cf588cc31dceb0/pkg/api/entries.go#L92
https://github.com/sigstore/rekor/blob/1e6728717d221a87792a8fc0c4cf588cc31dceb0/pkg/api/api.go#L177

It doesn't need to re-encoded it during verification. But the mock should ensure the log entry is encoded the way the verifier expects it to be.

[vishal-chdhry]

okay, makes sense now.

I have hex encoded the log entry and the PR is ready for review

[cmurphy]

Could you update the PR description to reflect the current purpose of this PR and add a reference to the issue it's addressing?

[cmurphy]

One last thing - this change adds one test, TestOnlineVerification, but it's possible to have inclusion proofs without verifying them online and vice versa. Could there be an additional test that tests online verification without the inclusion proof, and maybe one that tests with the inclusion proof with offline verification?

[vishal-chdhry]

an additional test that tests online verification without the inclusion proof

In online verification, we call rekorVerify.VerifyLogEntry for all valid tlog entires which requires you to have inclusion proof see rekorVerify.VerifyInclusion

So I am assuming this is a failure test case right?

[cmurphy]

In online verification, we call rekorVerify.VerifyLogEntry for all valid tlog entires which requires you to have inclusion proof see rekorVerify.VerifyInclusion

So I am assuming this is a failure test case right?

You could test for that failure case too, but no that's not what I meant. If you call Attest and get a signed entity that doesn't have an inclusion proof, then making an online call to rekor should return the inclusion proof and the online verification should succeed.

[cmurphy]

lgtm 👍

@haydentherapper @steiza

[haydentherapper]

Fantastic work, this looks great!

[vishal-chdhry]

Thank you @cmurphy for all the help!