Repository: google/certificate-transparency-go
Pull request #649 (2020-02-07 11:30:34)

Allow Hammer to perform more consistency checks
This PR adds a flag (`--strict_sth_consistency_sizes`) which when set to true enforces the current behaviour of only request consistency proofs between tree sizes for which the hammer has seen valid STHs.

When setting this flag to false, if no two usable STHs are available the hammer will attempt to request a consistency proof between the latest STH it's seen and a random smaller (but > 0) tree size.

### Checklist

- [X] I have updated the [CHANGELOG](CHANGELOG.md).
  - Adjust the draft version number according to [semantic versioning](https://semver.org/) rules.
- [ ] I have updated [documentation](docs/) accordingly.


codecov[bot]: # [Codecov](https://codecov.io/gh/google/certificate-transparency-go/pull/649?src=pr&el=h1) Report
> Merging [#649](https://codecov.io/gh/google/certificate-transparency-go/pull/649?src=pr&el=desc) into [master](https://codecov.io/gh/google/certificate-transparency-go/commit/1be4951724ce373947e314735521982b73e26d72?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/google/certificate-transparency-go/pull/649/graphs/tree.svg?width=650&token=kYPnjCLppA&height=150&src=pr)](https://codecov.io/gh/google/certificate-transparency-go/pull/649?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #649   +/-   ##
=======================================
  Coverage   71.78%   71.78%           
=======================================
  Files          90       90           
  Lines        9862     9862           
=======================================
  Hits         7079     7079           
  Misses       2286     2286           
  Partials      497      497
```


| [Impacted Files](https://codecov.io/gh/google/certificate-transparency-go/pull/649?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [fixchain/logger.go](https://codecov.io/gh/google/certificate-transparency-go/pull/649/diff?src=pr&el=tree#diff-Zml4Y2hhaW4vbG9nZ2VyLmdv) | `80.68% <0%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/google/certificate-transparency-go/pull/649?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/google/certificate-transparency-go/pull/649?src=pr&el=footer). Last update [1be4951...35dc55c](https://codecov.io/gh/google/certificate-transparency-go/pull/649?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8f44a26f..ecb80325 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,14 @@
 
 Not yet released; provisionally v1.2.0 (may change).
 
+### Tools
+
+#### CT Hammer
+
+Added a flag (--strict_sth_consistency_sizes) which when set to true enforces the current behaviour of only request consistency proofs between tree sizes for which the hammer has seen valid STHs.
+When setting this flag to false, if no two usable STHs are available the hammer will attempt to request a consistency proof between the latest STH it's seen and a random smaller (but > 0) tree size.
+
+
 ### CTFE
 
 #### Caching
diff --git a/trillian/integration/ct_hammer/main.go b/trillian/integration/ct_hammer/main.go
index 2b4d4733..b2c6f1a1 100644
--- a/trillian/integration/ct_hammer/main.go
+++ b/trillian/integration/ct_hammer/main.go
@@ -79,16 +79,17 @@ var (
 	reqDeadline         = flag.Duration("req_deadline", 10*time.Second, "Deadline to set on individual requests")
 )
 var (
-	addChainBias          = flag.Int("add_chain", 20, "Bias for add-chain operations")
-	addPreChainBias       = flag.Int("add_pre_chain", 20, "Bias for add-pre-chain operations")
-	getSTHBias            = flag.Int("get_sth", 2, "Bias for get-sth operations")
-	getSTHConsistencyBias = flag.Int("get_sth_consistency", 2, "Bias for get-sth-consistency operations")
-	getProofByHashBias    = flag.Int("get_proof_by_hash", 2, "Bias for get-proof-by-hash operations")
-	getEntriesBias        = flag.Int("get_entries", 2, "Bias for get-entries operations")
-	getRootsBias          = flag.Int("get_roots", 1, "Bias for get-roots operations")
-	getEntryAndProofBias  = flag.Int("get_entry_and_proof", 0, "Bias for get-entry-and-proof operations")
-	invalidChance         = flag.Int("invalid_chance", 10, "Chance of generating an invalid operation, as the N in 1-in-N (0 for never)")
-	dupeChance            = flag.Int("duplicate_chance", 10, "Chance of generating a duplicate submission, as the N in 1-in-N (0 for never)")
+	addChainBias             = flag.Int("add_chain", 20, "Bias for add-chain operations")
+	addPreChainBias          = flag.Int("add_pre_chain", 20, "Bias for add-pre-chain operations")
+	getSTHBias               = flag.Int("get_sth", 2, "Bias for get-sth operations")
+	getSTHConsistencyBias    = flag.Int("get_sth_consistency", 2, "Bias for get-sth-consistency operations")
+	getProofByHashBias       = flag.Int("get_proof_by_hash", 2, "Bias for get-proof-by-hash operations")
+	getEntriesBias           = flag.Int("get_entries", 2, "Bias for get-entries operations")
+	getRootsBias             = flag.Int("get_roots", 1, "Bias for get-roots operations")
+	getEntryAndProofBias     = flag.Int("get_entry_and_proof", 0, "Bias for get-entry-and-proof operations")
+	invalidChance            = flag.Int("invalid_chance", 10, "Chance of generating an invalid operation, as the N in 1-in-N (0 for never)")
+	dupeChance               = flag.Int("duplicate_chance", 10, "Chance of generating a duplicate submission, as the N in 1-in-N (0 for never)")
+	strictSTHConsistencySize = flag.Bool("strict_sth_consistency_size", true, "If set to true, hammer will use only tree sizes from STHs it's seen for consistency proofs, otherwise it'll choose a random size for the smaller tree")
 )
 
 func newLimiter(rate int) integration.Limiter {
@@ -290,22 +291,23 @@ func main() {
 		}
 
 		cfg := integration.HammerConfig{
-			LogCfg:              c,
-			MetricFactory:       mf,
-			MMD:                 mmd,
-			ChainGenerator:      generator,
-			ClientPool:          pool,
-			EPBias:              bias,
-			MinGetEntries:       *minGetEntries,
-			MaxGetEntries:       *maxGetEntries,
-			OversizedGetEntries: *oversizedGetEntries,
-			Operations:          *operations,
-			Limiter:             newLimiter(*limit),
-			MaxParallelChains:   *maxParallelChains,
-			IgnoreErrors:        *ignoreErrors,
-			MaxRetryDuration:    *maxRetry,
-			RequestDeadline:     *reqDeadline,
-			DuplicateChance:     *dupeChance,
+			LogCfg:                   c,
+			MetricFactory:            mf,
+			MMD:                      mmd,
+			ChainGenerator:           generator,
+			ClientPool:               pool,
+			EPBias:                   bias,
+			MinGetEntries:            *minGetEntries,
+			MaxGetEntries:            *maxGetEntries,
+			OversizedGetEntries:      *oversizedGetEntries,
+			Operations:               *operations,
+			Limiter:                  newLimiter(*limit),
+			MaxParallelChains:        *maxParallelChains,
+			IgnoreErrors:             *ignoreErrors,
+			MaxRetryDuration:         *maxRetry,
+			RequestDeadline:          *reqDeadline,
+			DuplicateChance:          *dupeChance,
+			StrictSTHConsistencySize: *strictSTHConsistencySize,
 		}
 		go func(cfg integration.HammerConfig) {
 			defer wg.Done()
diff --git a/trillian/integration/hammer.go b/trillian/integration/hammer.go
index 3cc592c9..ac327f28 100644
--- a/trillian/integration/hammer.go
+++ b/trillian/integration/hammer.go
@@ -145,6 +145,11 @@ type HammerConfig struct {
 	// calling add[-pre]-chain (as the N in 1-in-N). Set to 0 to disable sending
 	// duplicates.
 	DuplicateChance int
+	// StrictSTHConsistencySize if set to true will cause Hammer to only request
+	// STH consistency proofs between tree sizes for which it's seen valid STHs.
+	// If set to false, Hammer will request a consistency proof between the
+	// current tree size, and a random smaller size greater than zero.
+	StrictSTHConsistencySize bool
 }
 
 // HammerBias indicates the bias for selecting different log operations.
@@ -647,17 +652,17 @@ func (s *hammerState) chooseSTHs(ctx context.Context) (*ct.SignedTreeHead, *ct.S
 	if s.sth[which] == nil {
 		glog.V(3).Infof("%s: skipping get-sth-consistency as no earlier STH", s.cfg.LogCfg.Prefix)
 		s.needOps(ctfe.GetSTHName)
-		return nil, nil, errSkip{}
+		return nil, sthNow, errSkip{}
 	}
 	if s.sth[which].TreeSize == 0 {
 		glog.V(3).Infof("%s: skipping get-sth-consistency as no earlier STH", s.cfg.LogCfg.Prefix)
 		s.needOps(ctfe.AddChainName, ctfe.GetSTHName)
-		return nil, nil, errSkip{}
+		return nil, sthNow, errSkip{}
 	}
 	if s.sth[which].TreeSize == sthNow.TreeSize {
 		glog.V(3).Infof("%s: skipping get-sth-consistency as same size (%d)", s.cfg.LogCfg.Prefix, sthNow.TreeSize)
 		s.needOps(ctfe.AddChainName, ctfe.GetSTHName)
-		return nil, nil, errSkip{}
+		return nil, sthNow, errSkip{}
 	}
 	return s.sth[which], sthNow, nil
 }
@@ -665,13 +670,35 @@ func (s *hammerState) chooseSTHs(ctx context.Context) (*ct.SignedTreeHead, *ct.S
 func (s *hammerState) getSTHConsistency(ctx context.Context) error {
 	sthOld, sthNow, err := s.chooseSTHs(ctx)
 	if err != nil {
-		return err
+		// bail on actual errors
+		if _, ok := err.(errSkip); !ok {
+			return err
+		}
+		// If we're being asked to skip, it's because we don't have an earlier STH,
+		// if the config says we must only use "known" STHs then we'll have to wait
+		// until we get a larger STH.
+		if s.cfg.StrictSTHConsistencySize {
+			return err
+		}
+
+		// Otherwise, let's use our imagination and make one up, if possible...
+		if sthNow.TreeSize < 2 {
+			glog.V(3).Infof("%s: current STH size too small to invent a smaller STH for consistency proof (%d)", s.cfg.LogCfg.Prefix, sthNow.TreeSize)
+			return errSkip{}
+		}
+		sthOld = &ct.SignedTreeHead{TreeSize: uint64(1 + rand.Int63n(int64(sthNow.TreeSize)))}
+		glog.V(3).Infof("%s: Inventing a smaller STH size for consistency proof (%d)", s.cfg.LogCfg.Prefix, sthOld.TreeSize)
 	}
 
 	proof, err := s.client().GetSTHConsistency(ctx, sthOld.TreeSize, sthNow.TreeSize)
 	if err != nil {
 		return fmt.Errorf("failed to get-sth-consistency(%d, %d): %v", sthOld.TreeSize, sthNow.TreeSize, err)
 	}
+	if sthOld.Timestamp == 0 {
+		glog.V(3).Infof("%s: Skipping consistency proof verification for invented STH", s.cfg.LogCfg.Prefix)
+		return nil
+	}
+
 	if err := s.checkCTConsistencyProof(sthOld, sthNow, proof); err != nil {
 		return fmt.Errorf("get-sth-consistency(%d, %d) proof check failed: %v", sthOld.TreeSize, sthNow.TreeSize, err)
 	}


Test patch:
diff --git a/trillian/integration/hammer_test.go b/trillian/integration/hammer_test.go
index 2bd1d53f..f2fdac18 100644
--- a/trillian/integration/hammer_test.go
+++ b/trillian/integration/hammer_test.go
@@ -174,6 +174,9 @@ type fakeCTServer struct {
 	server *http.Server
 
 	addedCerts []*x509.Certificate
+	sthNow     ct.SignedTreeHead
+
+	getConsistencyCalled bool
 }
 
 func (s *fakeCTServer) addChain(w http.ResponseWriter, req *http.Request) {
@@ -232,6 +235,45 @@ func (s *fakeCTServer) serve() {
 	s.server.Serve(s.lis)
 }
 
+func (s *fakeCTServer) getSTH(w http.ResponseWriter, req *http.Request) {
+	resp := &ct.GetSTHResponse{
+		TreeSize:       s.sthNow.TreeSize,
+		Timestamp:      s.sthNow.Timestamp,
+		SHA256RootHash: []byte(s.sthNow.SHA256RootHash[:]),
+	}
+	var err error
+	resp.TreeHeadSignature, err = tls.Marshal(s.sthNow.TreeHeadSignature)
+	if err != nil {
+		writeErr(w, http.StatusInternalServerError, err)
+		return
+	}
+
+	respBytes, err := json.Marshal(resp)
+	if err != nil {
+		writeErr(w, http.StatusInternalServerError, err)
+		return
+	}
+
+	w.WriteHeader(http.StatusOK)
+	w.Write(respBytes)
+}
+
+func (s *fakeCTServer) getConsistency(w http.ResponseWriter, req *http.Request) {
+	cp := &ct.GetSTHConsistencyResponse{
+		Consistency: [][]byte{[]byte("bogus")},
+	}
+	respBytes, err := json.Marshal(cp)
+	if err != nil {
+		writeErr(w, http.StatusInternalServerError, err)
+		return
+	}
+
+	w.WriteHeader(http.StatusOK)
+	w.Write(respBytes)
+
+	s.getConsistencyCalled = true
+}
+
 func writeErr(w http.ResponseWriter, status int, err error) {
 	w.WriteHeader(status)
 	io.WriteString(w, err.Error())
@@ -252,6 +294,8 @@ func newFakeCTServer(t *testing.T) (*fakeCTServer, *client.LogClient) {
 	mux := http.NewServeMux()
 	mux.HandleFunc("/ct/v1/add-chain", s.addChain)
 	mux.HandleFunc("/ct/v1/add-pre-chain", s.addChain)
+	mux.HandleFunc("/ct/v1/get-sth", s.getSTH)
+	mux.HandleFunc("/ct/v1/get-sth-consistency", s.getConsistency)
 
 	s.server = &http.Server{Handler: mux}
 	go s.serve()
@@ -356,5 +400,51 @@ func TestChooseCertToAdd(t *testing.T) {
 			}
 		})
 	}
+}
+
+func TestStrictSTHConsistencySize(t *testing.T) {
+	ctx := context.Background()
+
+	for _, test := range []struct {
+		name       string
+		strict     bool
+		sthNowSize uint64
+		wantSkip   bool
+	}{
+		{name: "strict", strict: true, wantSkip: true},
+		{name: "relaxed_too_small", sthNowSize: 1, wantSkip: true},
+		{name: "relaxed_invent_size", sthNowSize: 10, wantSkip: false},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			s, lc := newFakeCTServer(t)
+			defer s.close()
+
+			s.sthNow.TreeSize = test.sthNowSize
 
+			hs, err := newHammerState(&HammerConfig{
+				StrictSTHConsistencySize: test.strict,
+				ClientPool:               RandomPool{lc},
+				LogCfg:                   &configpb.LogConfig{},
+			})
+			if err != nil {
+				t.Fatalf("Failed to create HammerState: %v", err)
+			}
+
+			err = hs.getSTHConsistency(ctx)
+			_, gotSkip := err.(errSkip)
+			if gotSkip != test.wantSkip {
+				t.Fatalf("got err %v, wanted Skip=%v", err, test.wantSkip)
+			}
+			if err != nil && !gotSkip {
+				t.Fatalf("got unexpected err %v", err)
+			}
+			if test.wantSkip {
+				return
+			}
+
+			if !s.getConsistencyCalled {
+				t.Fatal("hammer failed to request a consistency proof for invented tree size")
+			}
+		})
+	}
 }