Add support for evidence by lukevalenta · Pull Request #16 · bwesterb/mtc

lukevalenta · 2025-02-24T20:28:30Z

What's currently implemented is similar to Option (2) from davidben/merkle-tree-certs#23, but with shadow_assertions renamed to evidence, and the index updated to include offsets in the evidence file.

Rename QueuedAssertion to AssertionRequest, add Evidence field, and move from 'ca' package to 'mtc' package to be able to use private 'unmarshal' function.
Update /ca/queue to accept an AssertionRequest instead of an Assertion
Update index to include offset in evidence file.
Add 'mtc inspect evidence' option

NOTE: README also need to be updated.

mtc.go

- Rename QueuedAssertion to AssertionRequest, add Evidence field, and move from 'ca' package to 'mtc' package to be able to use private 'unmarshal' function. - Update /ca/queue to accept an AssertionRequest instead of an Assertion - Update index to include offset in evidence file. - Add 'mtc inspect evidence' option

cmd/mtc/main.go

bwesterb · 2025-02-24T21:00:57Z

cmd/mtc/main.go

@@ -376,12 +381,12 @@ func handleCaQueue(cc *cli.Context) error {
 }

 func handleNewAssertion(cc *cli.Context) error {


Maybe we should rename this command and the corresponding inspect command.

I was going back and forth on this since assertion-request is a little verbose, but that probably makes sense to avoid confusion.

Today we use CSR. We can use AR? Or simply mtc new request?

Oops, didn't see this before updating the PR. The current version has assertion-request, but we can tweak later. Some of the other CLI flags are pretty verbose too (abridged-assertions, signed-validity-window), so maybe we can just add short aliases (aa, svw, ar) if that seems ergonomic.

cmd/mtc/main.go

mtc.go

bwesterb · 2025-02-24T21:15:19Z

mtc.go

+			return errors.New("non-empty info for empty evidence")
+		}
+	case X509ChainEvidenceType:
+		chain, err := x509.ParseCertificates([]byte(evidenceInfo))


Given unmarshalling AssertionRequests is on the hot path of issuance, we might want to do lazy parsing here: just store the bytes in the struct and parse on demand. (As we do with the public key in TLSSubject)

Makes sense. I'll address in a followup PR!

bwesterb · 2025-02-24T21:16:53Z

mtc.go

+	X509ChainEvidenceType
+)
+
+type Evidence struct {


If this is only for the shadow certs, then we don't need polymorphism: just have a list of certs or an empty list.

If we want to reuse this for something else, then we want a list of possible evidence, instead of just one piece of evidence.

Good point. I'm leaning towards the latter approach, with a list of possible evidence. I'll address in a followup PR.

bwesterb

This is great. One or two issues, but we can do that in follow up PR.

lukevalenta · 2025-02-25T16:14:03Z

ca/ca.go


 	indices map[uint32]*Index
 	aas     map[uint32]*os.File
+	evs     map[uint32]*os.File


Note: I added this bit to mirror how the aas files are handled, but it doesn't look like these maps are ever populated.

Oh, yeah, that's a bug in aaFileFor which should set an entry.

aaFileFor is currently unused since we just recalculate the abridged assertion key from the passed-in assertion in CertificateFor 😁 . We could do an extra check there and make sure we find the abridged assertion where we expect it to be, and that the bytes are equal.

lukevalenta · 2025-02-25T16:15:11Z

ca/ca.go

+	}
+	err = mtc.UnmarshalEvidenceEntries(evFile, func(_ int, ev2 *mtc.Evidence) error {
+		ev = ev2
+		return ErrShortCircuit


This is kind of a weird way to handle this, but it works for now.

lukevalenta · 2025-02-25T16:16:57Z

ca/ca.go

 	}

-	for batch := batches.End - 1; batch <= batches.End; batch-- {
+	for batch := batches.End - 1; batch >= batches.Begin && batch <= batches.End; batch-- {


bugfix here. Say we have the batches 14-18 on disk. Previously, we'd go the way down to 13 and then throw an error since the files don't exist.

lukevalenta · 2025-02-25T16:18:18Z

cmd/mtc/main.go


-		var a mtc.Assertion
-		err = a.UnmarshalBinary(assertionBuf)
+		var ar mtc.AssertionRequest


bugfix: previously trying to parse an Assertion but should have been an AssertionRequest.

ca/ca.go

- Use 'ar' for AssertionRequests to reduce risk of variable shadowing. - bugfix: fix range when iterating over batches. Previously, there was no lower limit to the loop. And since we're iterating backwards with a uint32, keep the upper limit to avoid integer underflow. - Finish adding index support for evidence, so you can now `mtc ca evidence` will properly check the evidence file for an assertion's evidence. - Include Evidence in the AssertionRequest checksum. - Use 'assertion-request' instead of 'assertion' for cli flags.

lukevalenta requested a review from bwesterb February 24, 2025 20:28

lukevalenta commented Feb 24, 2025

View reviewed changes

mtc.go Outdated Show resolved Hide resolved

lukevalenta force-pushed the lvalenta/evidence branch from c2128b3 to 6055bf0 Compare February 24, 2025 20:36

bwesterb reviewed Feb 24, 2025

View reviewed changes

cmd/mtc/main.go Outdated Show resolved Hide resolved

bwesterb reviewed Feb 24, 2025

View reviewed changes

cmd/mtc/main.go Outdated Show resolved Hide resolved

bwesterb reviewed Feb 24, 2025

View reviewed changes

mtc.go Outdated Show resolved Hide resolved

bwesterb reviewed Feb 24, 2025

View reviewed changes

bwesterb approved these changes Feb 24, 2025

View reviewed changes

lukevalenta force-pushed the lvalenta/evidence branch from 6460c94 to 8cf3cd4 Compare February 25, 2025 16:20

lukevalenta commented Feb 25, 2025

View reviewed changes

bwesterb reviewed Feb 25, 2025

View reviewed changes

ca/ca.go Outdated Show resolved Hide resolved

bwesterb approved these changes Feb 25, 2025

View reviewed changes

lukevalenta force-pushed the lvalenta/evidence branch from 8cf3cd4 to 9267f5f Compare February 25, 2025 16:41

lukevalenta merged commit 6e9e4d1 into main Feb 25, 2025
1 check passed

lukevalenta deleted the lvalenta/evidence branch February 25, 2025 16:42

		@@ -376,12 +381,12 @@ func handleCaQueue(cc *cli.Context) error {
		}

		func handleNewAssertion(cc *cli.Context) error {

Conversation

lukevalenta commented Feb 24, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bwesterb Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bwesterb left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bwesterb Feb 24, 2025 •

edited

Loading

bwesterb left a comment •

edited

Loading