Deduplicate SCT timestamps (#57)

* deduplicate timestamps

* Edit dedup terraform

* address comments

* fix bug

* delete comma

* address comments

Author: phbnf

deployment/modules/gcp/storage/main.tf

@@ -60,6 +60,6 @@ resource "google_spanner_database" "dedup_db" {
   instance = google_spanner_instance.log_spanner.name
   name     = "${var.base_name}-dedup-db"
   ddl = [
-    "CREATE TABLE IDSeq (id INT64 NOT NULL, h BYTES(MAX) NOT NULL, idx INT64 NOT NULL,) PRIMARY KEY (id, h)",
+    "CREATE TABLE IDSeq (id INT64 NOT NULL, h BYTES(MAX) NOT NULL, idx INT64 NOT NULL, timestamp INT64 NOT NULL,) PRIMARY KEY (id, h)",
   ]
 }

handlers.go

@@ -34,6 +34,7 @@ import (
 	"github.com/google/certificate-transparency-go/x509"
 	"github.com/google/certificate-transparency-go/x509util"
 	"github.com/google/trillian/monitoring"
+	"github.com/transparency-dev/static-ct/modules/dedup"
 	tessera "github.com/transparency-dev/trillian-tessera"
 	"github.com/transparency-dev/trillian-tessera/ctonly"
 	"k8s.io/klog/v2"
@@ -319,13 +320,15 @@ func addChainInternal(ctx context.Context, li *logInfo, w http.ResponseWriter, r
 	}
 
 	klog.V(2).Infof("%s: %s => storage.GetCertIndex", li.LogOrigin, method)
-	idx, isDup, err := li.storage.GetCertIndex(ctx, chain[0])
+	sctDedupInfo, isDup, err := li.storage.GetCertDedupInfo(ctx, chain[0])
+	idx := sctDedupInfo.Idx
 	if err != nil {
 		return http.StatusInternalServerError, fmt.Errorf("couldn't deduplicate the request: %s", err)
 	}
 
 	if isDup {
 		klog.V(3).Infof("%s: %s - found duplicate entry at index %d", li.LogOrigin, method, idx)
+		entry.Timestamp = sctDedupInfo.Timestamp
 	} else {
 		if err := li.storage.AddIssuerChain(ctx, chain[1:]); err != nil {
 			return http.StatusInternalServerError, fmt.Errorf("failed to store issuer chain: %s", err)
@@ -344,7 +347,7 @@ func addChainInternal(ctx context.Context, li *logInfo, w http.ResponseWriter, r
 		// It might be stored again later, if a local deduplication storage is synced, potentially
 		// with a smaller value.
 		klog.V(2).Infof("%s: %s => storage.AddCertIndex", li.LogOrigin, method)
-		err := li.storage.AddCertIndex(ctx, chain[0], idx)
+		err = li.storage.AddCertDedupInfo(ctx, chain[0], dedup.SCTDedupInfo{Idx: idx, Timestamp: entry.Timestamp})
 		// TODO: block log writes if deduplication breaks
 		if err != nil {
 			klog.Warningf("AddCertIndex(): failed to store certificate index: %v", err)

handlers_test.go

@@ -36,6 +36,7 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/google/trillian/monitoring"
 	"github.com/transparency-dev/static-ct/mockstorage"
+	"github.com/transparency-dev/static-ct/modules/dedup"
 	"github.com/transparency-dev/static-ct/testdata"
 	"github.com/transparency-dev/trillian-tessera/ctonly"
 	"google.golang.org/grpc/codes"
@@ -291,10 +292,10 @@ func TestAddChainWhitespace(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.descr, func(t *testing.T) {
 			if test.want == http.StatusOK {
-				info.storage.EXPECT().GetCertIndex(deadlineMatcher(), cmpMatcher{leafChain[0]}).Return(uint64(0), false, nil)
+				info.storage.EXPECT().GetCertDedupInfo(deadlineMatcher(), cmpMatcher{leafChain[0]}).Return(dedup.SCTDedupInfo{Idx: uint64(0), Timestamp: fakeTimeMillis}, false, nil)
 				info.storage.EXPECT().AddIssuerChain(deadlineMatcher(), cmpMatcher{leafChain[1:]}).Return(nil)
 				info.storage.EXPECT().Add(deadlineMatcher(), cmpMatcher{req}).Return(func() (uint64, error) { return rsp, nil })
-				info.storage.EXPECT().AddCertIndex(deadlineMatcher(), cmpMatcher{leafChain[0]}, uint64(0)).Return(nil)
+				info.storage.EXPECT().AddCertDedupInfo(deadlineMatcher(), cmpMatcher{leafChain[0]}, dedup.SCTDedupInfo{Idx: uint64(0), Timestamp: fakeTimeMillis}).Return(nil)
 			}
 
 			recorder := httptest.NewRecorder()
@@ -367,11 +368,11 @@ func TestAddChain(t *testing.T) {
 			if len(test.toSign) > 0 {
 				req, leafChain := parseChain(t, false, test.chain, info.roots.RawCertificates()[0])
 				rsp := uint64(0)
-				info.storage.EXPECT().GetCertIndex(deadlineMatcher(), cmpMatcher{leafChain[0]}).Return(uint64(0), false, nil)
+				info.storage.EXPECT().GetCertDedupInfo(deadlineMatcher(), cmpMatcher{leafChain[0]}).Return(dedup.SCTDedupInfo{Idx: uint64(0), Timestamp: fakeTimeMillis}, false, nil)
 				info.storage.EXPECT().AddIssuerChain(deadlineMatcher(), cmpMatcher{leafChain[1:]}).Return(nil)
 				info.storage.EXPECT().Add(deadlineMatcher(), cmpMatcher{req}).Return(func() (uint64, error) { return rsp, test.err })
 				if test.want == http.StatusOK {
-					info.storage.EXPECT().AddCertIndex(deadlineMatcher(), cmpMatcher{leafChain[0]}, uint64(0)).Return(nil)
+					info.storage.EXPECT().AddCertDedupInfo(deadlineMatcher(), cmpMatcher{leafChain[0]}, dedup.SCTDedupInfo{Idx: uint64(0), Timestamp: fakeTimeMillis}).Return(nil)
 				}
 			}
 
@@ -460,11 +461,11 @@ func TestAddPrechain(t *testing.T) {
 			if len(test.toSign) > 0 {
 				req, leafChain := parseChain(t, true, test.chain, info.roots.RawCertificates()[0])
 				rsp := uint64(0)
-				info.storage.EXPECT().GetCertIndex(deadlineMatcher(), cmpMatcher{leafChain[0]}).Return(uint64(0), false, nil)
+				info.storage.EXPECT().GetCertDedupInfo(deadlineMatcher(), cmpMatcher{leafChain[0]}).Return(dedup.SCTDedupInfo{Idx: uint64(0), Timestamp: fakeTimeMillis}, false, nil)
 				info.storage.EXPECT().AddIssuerChain(deadlineMatcher(), cmpMatcher{leafChain[1:]}).Return(nil)
 				info.storage.EXPECT().Add(deadlineMatcher(), cmpMatcher{req}).Return(func() (uint64, error) { return rsp, test.err })
 				if test.want == http.StatusOK {
-					info.storage.EXPECT().AddCertIndex(deadlineMatcher(), cmpMatcher{leafChain[0]}, uint64(0)).Return(nil)
+					info.storage.EXPECT().AddCertDedupInfo(deadlineMatcher(), cmpMatcher{leafChain[0]}, dedup.SCTDedupInfo{Idx: uint64(0), Timestamp: fakeTimeMillis}).Return(nil)
 				}
 			}
 
@@ -484,7 +485,7 @@ func TestAddPrechain(t *testing.T) {
 				if got, want := resp.ID, demoLogID[:]; !bytes.Equal(got, want) {
 					t.Errorf("resp.ID=%x; want %x", got, want)
 				}
-				if got, want := resp.Timestamp, uint64(1469185273000); got != want {
+				if got, want := resp.Timestamp, fakeTimeMillis; got != want {
 					t.Errorf("resp.Timestamp=%d; want %d", got, want)
 				}
 				if got, want := hex.EncodeToString(resp.Signature), "040300067369676e6564"; got != want {

mockstorage/mock_ct_storage.go

@@ -28,25 +28,31 @@ import (
 	"k8s.io/klog/v2"
 )
 
-// LeafIdx holds a LeafID and an Idx for deduplication
-type LeafIdx struct {
+// LeafDedupInfo enables building deduplicated add-pre-chain/add-chain responses.
+type LeafDedupInfo struct {
 	LeafID []byte
-	Idx    uint64
+	SCTDedupInfo
+}
+
+// SCTDedupInfo contains data to build idempotent SCTs.
+type SCTDedupInfo struct {
+	Idx       uint64
+	Timestamp uint64
 }
 
 type BEDedupStorage interface {
-	Add(ctx context.Context, lidxs []LeafIdx) error
-	Get(ctx context.Context, leafID []byte) (uint64, bool, error)
+	Add(ctx context.Context, lidxs []LeafDedupInfo) error
+	Get(ctx context.Context, leafID []byte) (SCTDedupInfo, bool, error)
 }
 
 // TODO: re-architecture to prevent creating a LocaLBEDedupStorage without calling UpdateFromLog
 type LocalBEDedupStorage interface {
-	Add(ctx context.Context, lidxs []LeafIdx) error
-	Get(ctx context.Context, leafID []byte) (uint64, bool, error)
+	Add(ctx context.Context, lidxs []LeafDedupInfo) error
+	Get(ctx context.Context, leafID []byte) (SCTDedupInfo, bool, error)
 	LogSize() (uint64, error)
 }
 
-type ParseBundleFunc func([]byte, uint64) ([]LeafIdx, error)
+type ParseBundleFunc func([]byte, uint64) ([]LeafDedupInfo, error)
 
 // UpdateFromLog synchronises a local best effort deduplication storage with a log.
 func UpdateFromLog(ctx context.Context, lds LocalBEDedupStorage, t time.Duration, fcp client.CheckpointFetcherFunc, fb client.EntryBundleFetcherFunc, pb ParseBundleFunc) {
@@ -97,12 +103,12 @@ func sync(ctx context.Context, lds LocalBEDedupStorage, pb ParseBundleFunc, fcp
 				}
 				return fmt.Errorf("failed to fetch leaf bundle at index %d: %v", i, err)
 			}
-			lidxs, err := pb(eRaw, i)
+			ldis, err := pb(eRaw, i)
 			if err != nil {
 				return fmt.Errorf("parseBundle(): %v", err)
 			}
 
-			if err := lds.Add(ctx, lidxs); err != nil {
+			if err := lds.Add(ctx, ldis); err != nil {
 				return fmt.Errorf("error storing deduplication data for tile %d: %v", i, err)
 			}
 			klog.V(3).Infof("LocalBEDEdup.sync(): stored dedup data for entry bundle %d, %d more bundles to go", i, ckptSize/256-i)

modules/dedup/dedup.go

@@ -176,15 +176,15 @@ func NewCpSigner(signer crypto.Signer, origin string, logID [32]byte, timeSource
 	return ctSigner
 }
 
-// DedupFromBundle converts a bundle into an array of {leafID, idx}.
+// DedupFromBundle converts a bundle into an array of dedup.LeafDedupInfo.
 //
 // The index of a leaf is computed from its position in the log, instead of parsing SCTs.
 // Greatly inspired by https://github.com/FiloSottile/sunlight/blob/main/tile.go
-func DedupFromBundle(bundle []byte, bundleIdx uint64) ([]dedup.LeafIdx, error) {
-	kvs := []dedup.LeafIdx{}
+func DedupFromBundle(bundle []byte, bundleIdx uint64) ([]dedup.LeafDedupInfo, error) {
+	kvs := []dedup.LeafDedupInfo{}
 	s := cryptobyte.String(bundle)
 
-	for len(s) > 0 {
+	for i := bundleIdx * 256; len(s) > 0; i++ {
 		var timestamp uint64
 		var entryType uint16
 		var extensions, fingerprints cryptobyte.String
@@ -213,7 +213,8 @@ func DedupFromBundle(bundle []byte, bundleIdx uint64) ([]dedup.LeafIdx, error) {
 			return nil, fmt.Errorf("invalid data tile: unknown type %d", entryType)
 		}
 		k := sha256.Sum256(crt)
-		kvs = append(kvs, dedup.LeafIdx{LeafID: k[:], Idx: bundleIdx*256 + uint64(len(kvs))})
+		sctDedupInfo := dedup.SCTDedupInfo{Idx: uint64(i), Timestamp: timestamp}
+		kvs = append(kvs, dedup.LeafDedupInfo{LeafID: k[:], SCTDedupInfo: sctDedupInfo})
 	}
 	return kvs, nil
 }

serialize.go

@@ -22,9 +22,9 @@ import (
 	"sync"
 
 	"github.com/google/certificate-transparency-go/x509"
+	"github.com/transparency-dev/static-ct/modules/dedup"
 	tessera "github.com/transparency-dev/trillian-tessera"
 	"github.com/transparency-dev/trillian-tessera/ctonly"
-	"github.com/transparency-dev/static-ct/modules/dedup"
 	"k8s.io/klog/v2"
 )
 
@@ -41,10 +41,10 @@ type Storage interface {
 	Add(context.Context, *ctonly.Entry) tessera.IndexFuture
 	// AddIssuerChain stores every the chain certificate in a content-addressable store under their sha256 hash.
 	AddIssuerChain(context.Context, []*x509.Certificate) error
-	// AddCertIndex stores the index of certificate in a log under its hash.
-	AddCertIndex(context.Context, *x509.Certificate, uint64) error
-	// GetCertIndex gets the index of certificate in a log from its hash.
-	GetCertIndex(context.Context, *x509.Certificate) (uint64, bool, error)
+	// AddCertDedupInfo stores the SCTDedupInfo of certificate in a log under its hash.
+	AddCertDedupInfo(context.Context, *x509.Certificate, dedup.SCTDedupInfo) error
+	// GetCertDedupInfo gets the SCTDedupInfo of certificate in a log from its hash.
+	GetCertDedupInfo(context.Context, *x509.Certificate) (dedup.SCTDedupInfo, bool, error)
 }
 
 type KV struct {
@@ -131,21 +131,21 @@ func cachedStoreIssuers(s IssuerStorage) func(context.Context, []KV) error {
 	}
 }
 
-// AddCertIndex stores <cert_hash, index> in the deduplication storage.
-func (cts CTStorage) AddCertIndex(ctx context.Context, c *x509.Certificate, idx uint64) error {
+// AddCertDedupInfo stores <cert_hash, SCTDedupInfo> in the deduplication storage.
+func (cts CTStorage) AddCertDedupInfo(ctx context.Context, c *x509.Certificate, sctDedupInfo dedup.SCTDedupInfo) error {
 	key := sha256.Sum256(c.Raw)
-	if err := cts.dedupStorage.Add(ctx, []dedup.LeafIdx{{LeafID: key[:], Idx: idx}}); err != nil {
-		return fmt.Errorf("error storing index %d of %q: %v", idx, hex.EncodeToString(key[:]), err)
+	if err := cts.dedupStorage.Add(ctx, []dedup.LeafDedupInfo{{LeafID: key[:], SCTDedupInfo: sctDedupInfo}}); err != nil {
+		return fmt.Errorf("error storing SCTDedupInfo %+v of \"%x\": %v", sctDedupInfo, key, err)
 	}
 	return nil
 }
 
-// GetCertIndex fetches the index of a given certificate from the deduplication storage.
-func (cts CTStorage) GetCertIndex(ctx context.Context, c *x509.Certificate) (uint64, bool, error) {
+// GetCertDedupInfo fetches the SCTDedupInfo of a given certificate from the deduplication storage.
+func (cts CTStorage) GetCertDedupInfo(ctx context.Context, c *x509.Certificate) (dedup.SCTDedupInfo, bool, error) {
 	key := sha256.Sum256(c.Raw)
-	idx, ok, err := cts.dedupStorage.Get(ctx, key[:])
+	sctC, ok, err := cts.dedupStorage.Get(ctx, key[:])
 	if err != nil {
-		return 0, false, fmt.Errorf("error fetching index of %q: %v", hex.EncodeToString(key[:]), err)
+		return dedup.SCTDedupInfo{}, false, fmt.Errorf("error fetching index of \"%x\": %v", key, err)
 	}
-	return idx, ok, nil
+	return sctC, ok, nil
 }