Repository: google/certificate-transparency-go (go)

Issue:
Updating chromepolicy.go
Chrome's CT policy is changing, effective for certificates issued on-or-after April 15: https://groups.google.com/a/chromium.org/g/ct-policy/c/507lPdbbwSk/m/JpxJEtrQAwAJ. Specifically, it removes the "one Google log" requirement in favor of (roughly paraphrasing) "2 logs with distinct operators."

I see https://github.com/google/certificate-transparency-go/blob/master/ctpolicy/chromepolicy.go hasn't been updated in a while. Are there plans to update that, and the corresponding submission logic, to match the new Chrome CT policy?

Thanks!

Pull request description:
Update chromepolicy.go to follow the updated Chrome CT Policy
* Use LogGroupInfo in chromepolicy.go to keep track of the different CT log operators.
* Change calculation of minimum number of SCTs using the lifetime in days.
* Add extra fields in the safeSubmissionState struct to keep track of the distinct CT operators that have returned an SCT.
* Add a check in groupComplete to check for distinct CT log operators before marking as a complete group.

Fixes #906 

### Checklist

<!---
Go over all the following points, and put an `x` in all the boxes that apply.
Feel free to not tick any boxes that don't apply to this PR (e.g. refactoring may not need a CHANGELOG update).
If you're unsure about any of these, don't hesitate to ask. We're here to help!
-->

- [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.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4bff3ed0..87c1050c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 ## HEAD
 
+### Chrome CT Policy Update
+ * #906: Update chromepolicy.go to follow the updated Chrome CT policy.
+ 
 ## v1.1.6
 
 ## Dependency update
diff --git a/ctpolicy/chromepolicy.go b/ctpolicy/chromepolicy.go
index fbfd5fed..910fb7d6 100644
--- a/ctpolicy/chromepolicy.go
+++ b/ctpolicy/chromepolicy.go
@@ -15,10 +15,17 @@
 package ctpolicy
 
 import (
+	"time"
+
 	"github.com/google/certificate-transparency-go/loglist3"
 	"github.com/google/certificate-transparency-go/x509"
 )
 
+const (
+	minOperators = 2                   // minimum number of distinct CT log operators that issue an SCT.
+	dayDuration  = 86400 * time.Second // time.Duration of one day
+)
+
 // ChromeCTPolicy implements logic for complying with Chrome's CT log policy
 type ChromeCTPolicy struct {
 }
@@ -27,37 +34,30 @@ type ChromeCTPolicy struct {
 // https://github.com/chromium/ct-policy/blob/master/ct_policy.md#qualifying-certificate.
 // Returns an error if it's not possible to satisfy the policy with the provided loglist.
 func (chromeP ChromeCTPolicy) LogsByGroup(cert *x509.Certificate, approved *loglist3.LogList) (LogPolicyData, error) {
-	googGroup := LogGroupInfo{Name: "Google-operated", IsBase: false}
-	googGroup.populate(approved, func(op *loglist3.Operator) bool { return op.GoogleOperated() })
-	if err := googGroup.setMinInclusions(1); err != nil {
-		return nil, err
-	}
-
-	nonGoogGroup := LogGroupInfo{Name: "Non-Google-operated", IsBase: false}
-	nonGoogGroup.populate(approved, func(op *loglist3.Operator) bool { return !op.GoogleOperated() })
-	if err := nonGoogGroup.setMinInclusions(1); err != nil {
-		return nil, err
+	groups := LogPolicyData{}
+	for _, op := range approved.Operators {
+		info := &LogGroupInfo{Name: op.Name, IsBase: false}
+		info.LogURLs = make(map[string]bool)
+		info.LogWeights = make(map[string]float32)
+		for _, l := range op.Logs {
+			info.LogURLs[l.URL] = true
+			info.LogWeights[l.URL] = 1.0
+		}
+		groups[info.Name] = info
 	}
 	var incCount int
-	switch m := lifetimeInMonths(cert); {
-	case m < 15:
+	switch t := certLifetime(cert); {
+	case t <= 180*dayDuration:
 		incCount = 2
-	case m <= 27:
-		incCount = 3
-	case m <= 39:
-		incCount = 4
 	default:
-		incCount = 5
+		incCount = 3
 	}
 	baseGroup, err := BaseGroupFor(approved, incCount)
 	if err != nil {
 		return nil, err
 	}
-	groups := LogPolicyData{
-		googGroup.Name:    &googGroup,
-		nonGoogGroup.Name: &nonGoogGroup,
-		baseGroup.Name:    baseGroup,
-	}
+	baseGroup.MinOperators = minOperators
+	groups[baseGroup.Name] = baseGroup
 	return groups, nil
 }
 
diff --git a/ctpolicy/ctpolicy.go b/ctpolicy/ctpolicy.go
index a250eadd..d0d44ac5 100644
--- a/ctpolicy/ctpolicy.go
+++ b/ctpolicy/ctpolicy.go
@@ -18,6 +18,7 @@ package ctpolicy
 import (
 	"fmt"
 	"sync"
+	"time"
 
 	"github.com/google/certificate-transparency-go/loglist3"
 	"github.com/google/certificate-transparency-go/x509"
@@ -33,6 +34,7 @@ type LogGroupInfo struct {
 	Name          string
 	LogURLs       map[string]bool    // set of members
 	MinInclusions int                // Required number of submissions.
+	MinOperators  int                // Required number of distinct CT log operators.
 	IsBase        bool               // True only for Log-group covering all logs.
 	LogWeights    map[string]float32 // weights used for submission, default weight is 1
 	wMu           sync.RWMutex       // guards weights
@@ -205,6 +207,13 @@ func lifetimeInMonths(cert *x509.Certificate) int {
 	return lifetimeInMonths
 }
 
+// certLifetime calculates and returns the lifetime of the given certificate as a time.Duration
+func certLifetime(cert *x509.Certificate) time.Duration {
+	start := cert.NotBefore
+	end := cert.NotAfter
+	return end.Sub(start)
+}
+
 // GroupSet is set of Log-group names.
 type GroupSet map[string]bool
 
diff --git a/submission/races.go b/submission/races.go
index 222c4b10..65eaa001 100644
--- a/submission/races.go
+++ b/submission/races.go
@@ -54,8 +54,10 @@ type groupState struct {
 type safeSubmissionState struct {
 	mu          sync.Mutex
 	logToGroups map[string]ctpolicy.GroupSet
-	groupNeeds  map[string]int
+	groupNeeds  map[string]int // number of logs that need to be submitted for each group.
+	minGroups   int            // minimum number of distinct groups that need a log submitted.
 
+	groups  map[string]bool // set of groups that have a log submitted.
 	results map[string]*submissionResult
 	cancels map[string]context.CancelFunc
 }
@@ -67,6 +69,10 @@ func newSafeSubmissionState(groups ctpolicy.LogPolicyData) *safeSubmissionState
 	for _, g := range groups {
 		s.groupNeeds[g.Name] = g.MinInclusions
 	}
+	if baseGroup, ok := groups[ctpolicy.BaseName]; ok {
+		s.minGroups = baseGroup.MinOperators
+	}
+	s.groups = make(map[string]bool)
 	s.results = make(map[string]*submissionResult)
 	s.cancels = make(map[string]context.CancelFunc)
 	return &s
@@ -116,6 +122,7 @@ func (sub *safeSubmissionState) setResult(logURL string, sct *ct.SignedCertifica
 		if sub.groupNeeds[groupName] > 0 {
 			sub.results[logURL] = &submissionResult{sct: sct, err: err}
 		}
+		sub.groups[groupName] = true
 		sub.groupNeeds[groupName]--
 	}
 
@@ -165,6 +172,9 @@ func (sub *safeSubmissionState) groupComplete(groupName string) bool {
 	if !ok {
 		return true
 	}
+	if len(sub.groups) < sub.minGroups {
+		return false
+	}
 	return needs <= 0
 }
 


Test patch:
diff --git a/ctpolicy/chromepolicy_test.go b/ctpolicy/chromepolicy_test.go
index 8cff6e08..eba41ba7 100644
--- a/ctpolicy/chromepolicy_test.go
+++ b/ctpolicy/chromepolicy_test.go
@@ -21,10 +21,10 @@ import (
 	"github.com/kylelemons/godebug/pretty"
 )
 
-func wantedGroups(goog int, nonGoog int, base int, minusBob bool) LogPolicyData {
+func wantedGroups(base int, minusBob bool) LogPolicyData {
 	gi := LogPolicyData{
-		"Google-operated": {
-			Name: "Google-operated",
+		"Google": {
+			Name: "Google",
 			LogURLs: map[string]bool{
 				"https://ct.googleapis.com/logs/argon2020/": true,
 				"https://ct.googleapis.com/aviator/":        true,
@@ -32,8 +32,7 @@ func wantedGroups(goog int, nonGoog int, base int, minusBob bool) LogPolicyData
 				"https://ct.googleapis.com/rocketeer/":      true,
 				"https://ct.googleapis.com/racketeer/":      true,
 			},
-			MinInclusions: goog,
-			IsBase:        false,
+			IsBase: false,
 			LogWeights: map[string]float32{
 				"https://ct.googleapis.com/logs/argon2020/": 1.0,
 				"https://ct.googleapis.com/aviator/":        1.0,
@@ -42,13 +41,12 @@ func wantedGroups(goog int, nonGoog int, base int, minusBob bool) LogPolicyData
 				"https://ct.googleapis.com/racketeer/":      1.0,
 			},
 		},
-		"Non-Google-operated": {
-			Name: "Non-Google-operated",
+		"Bob's CT Log Shop": {
+			Name: "Bob's CT Log Shop",
 			LogURLs: map[string]bool{
 				"https://log.bob.io": true,
 			},
-			MinInclusions: nonGoog,
-			IsBase:        false,
+			IsBase: false,
 			LogWeights: map[string]float32{
 				"https://log.bob.io": 1.0,
 			},
@@ -64,6 +62,7 @@ func wantedGroups(goog int, nonGoog int, base int, minusBob bool) LogPolicyData
 				"https://log.bob.io":                        true,
 			},
 			MinInclusions: base,
+			MinOperators:  minOperators,
 			IsBase:        true,
 			LogWeights: map[string]float32{
 				"https://ct.googleapis.com/logs/argon2020/": 1.0,
@@ -78,8 +77,8 @@ func wantedGroups(goog int, nonGoog int, base int, minusBob bool) LogPolicyData
 	if minusBob {
 		delete(gi[BaseName].LogURLs, "https://log.bob.io")
 		delete(gi[BaseName].LogWeights, "https://log.bob.io")
-		delete(gi["Non-Google-operated"].LogURLs, "https://log.bob.io")
-		delete(gi["Non-Google-operated"].LogWeights, "https://log.bob.io")
+		delete(gi["Bob's CT Log Shop"].LogURLs, "https://log.bob.io")
+		delete(gi["Bob's CT Log Shop"].LogWeights, "https://log.bob.io")
 	}
 	return gi
 }
@@ -93,22 +92,22 @@ func TestCheckChromePolicy(t *testing.T) {
 		{
 			name: "Short",
 			cert: getTestCertPEMShort(),
-			want: wantedGroups(1, 1, 2, false),
+			want: wantedGroups(2, false),
 		},
 		{
 			name: "2-year",
 			cert: getTestCertPEM2Years(),
-			want: wantedGroups(1, 1, 3, false),
+			want: wantedGroups(3, false),
 		},
 		{
 			name: "3-year",
 			cert: getTestCertPEM3Years(),
-			want: wantedGroups(1, 1, 4, false),
+			want: wantedGroups(3, false),
 		},
 		{
 			name: "Long",
 			cert: getTestCertPEMLongOriginal(),
-			want: wantedGroups(1, 1, 5, false),
+			want: wantedGroups(3, false),
 		},
 	}
 
@@ -127,59 +126,3 @@ func TestCheckChromePolicy(t *testing.T) {
 		})
 	}
 }
-
-func TestCheckChromePolicyWarnings(t *testing.T) {
-	tests := []struct {
-		name    string
-		cert    *x509.Certificate
-		want    LogPolicyData
-		warning string
-	}{
-		{
-			name:    "Short",
-			cert:    getTestCertPEMShort(),
-			want:    LogPolicyData{},
-			warning: "trying to assign 1 minimal inclusion number while only 0 logs are part of group \"Non-Google-operated\"",
-		},
-		{
-			name:    "2-year",
-			cert:    getTestCertPEM2Years(),
-			want:    LogPolicyData{},
-			warning: "trying to assign 1 minimal inclusion number while only 0 logs are part of group \"Non-Google-operated\"",
-		},
-		{
-			name:    "3-year",
-			cert:    getTestCertPEM3Years(),
-			want:    LogPolicyData{},
-			warning: "trying to assign 1 minimal inclusion number while only 0 logs are part of group \"Non-Google-operated\"",
-		},
-		{
-			name:    "Long",
-			cert:    getTestCertPEMLongOriginal(),
-			want:    LogPolicyData{},
-			warning: "trying to assign 1 minimal inclusion number while only 0 logs are part of group \"Non-Google-operated\"",
-		},
-	}
-
-	var policy ChromeCTPolicy
-	sampleLogList := sampleLogList(t)
-	// Removing Bob-log.
-	sampleLogList.Operators = sampleLogList.Operators[:1]
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-
-			got, err := policy.LogsByGroup(test.cert, sampleLogList)
-			if diff := pretty.Compare(test.want, got); diff != "" {
-				t.Errorf("LogsByGroup: (-want +got)\n%s", diff)
-			}
-			if err == nil && len(test.warning) > 0 {
-				t.Errorf("LogsByGroup returned no error when expected")
-			} else if err != nil {
-				if err.Error() != test.warning {
-					t.Errorf("LogsByGroup returned error message %q while expected %q", err.Error(), test.warning)
-				}
-			}
-		})
-	}
-}
diff --git a/ctpolicy/ctpolicy_test.go b/ctpolicy/ctpolicy_test.go
index 0d4c1576..06200485 100644
--- a/ctpolicy/ctpolicy_test.go
+++ b/ctpolicy/ctpolicy_test.go
@@ -27,7 +27,7 @@ import (
 
 func getTestCertPEMShort() *x509.Certificate {
 	cert, _ := x509util.CertificateFromPEM([]byte(testdata.TestCertPEM))
-	cert.NotAfter = time.Date(2013, 1, 1, 0, 0, 0, 0, time.UTC)
+	cert.NotAfter = time.Date(2012, 6, 1, 0, 0, 0, 0, time.UTC)
 	return cert
 }
 
@@ -104,6 +104,52 @@ func TestLifetimeInMonths(t *testing.T) {
 	}
 }
 
+func TestCertLifetime(t *testing.T) {
+	tests := []struct {
+		name      string
+		notBefore time.Time
+		notAfter  time.Time
+		want      time.Duration
+	}{
+		{
+			name:      "ExactDays",
+			notBefore: time.Date(2012, 6, 1, 0, 0, 0, 0, time.UTC),
+			notAfter:  time.Date(2013, 1, 1, 0, 0, 0, 0, time.UTC),
+			want:      18489600 * time.Second,
+		},
+		{
+			name:      "ExactYears",
+			notBefore: time.Date(2012, 6, 1, 0, 0, 0, 0, time.UTC),
+			notAfter:  time.Date(2015, 1, 1, 0, 0, 0, 0, time.UTC),
+			want:      81561600 * time.Second,
+		},
+		{
+			name:      "PartialSingleDay",
+			notBefore: time.Date(2012, 6, 1, 0, 0, 0, 0, time.UTC),
+			notAfter:  time.Date(2012, 6, 1, 15, 0, 0, 0, time.UTC),
+			want:      54000 * time.Second,
+		},
+		{
+			name:      "PartialDays",
+			notBefore: time.Date(2012, 6, 25, 0, 0, 0, 0, time.UTC),
+			notAfter:  time.Date(2012, 6, 30, 12, 0, 0, 0, time.UTC),
+			want:      475200 * time.Second,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			cert := getTestCertPEMLongOriginal()
+			cert.NotBefore = test.notBefore
+			cert.NotAfter = test.notAfter
+			got := certLifetime(cert)
+			if got != test.want {
+				t.Errorf("certLifetime(%v, %v)=%d, want %d", test.notBefore, test.notAfter, got, test.want)
+			}
+		})
+	}
+}
+
 func TestGroupByLogs(t *testing.T) {
 	tests := []struct {
 		name      string
diff --git a/submission/races_test.go b/submission/races_test.go
index a90ab674..90b8045b 100644
--- a/submission/races_test.go
+++ b/submission/races_test.go
@@ -139,12 +139,35 @@ func TestGetSCTs(t *testing.T) {
 					Name:          ctpolicy.BaseName,
 					LogURLs:       map[string]bool{"a1.com": true, "a2.com": true, "a3.com": true, "a4.com": true, "b1.com": true, "b2.com": true, "b3.com": true, "b4.com": true},
 					MinInclusions: 5,
+					MinOperators:  2,
 					IsBase:        true,
 					LogWeights:    map[string]float32{"a1.com": 1.0, "a2.com": 1.0, "a3.com": 1.0, "a4.com": 1.0, "b1.com": 1.0, "b2.com": 1.0, "b3.com": 1.0, "b4.com": 1.0},
 				},
 			},
 			resultTrail: map[string]int{"a": 1, "b": 1, ctpolicy.BaseName: 5},
 		},
+		{
+			name:   "notEnoughDistinctOperators",
+			sbMock: &mockSubmitter{fixedDelay: map[byte]time.Duration{'a': 0, 'b': 2 * time.Second}, firstLetterURLReqNumber: make(map[byte]int)},
+			groups: ctpolicy.LogPolicyData{
+				"a": {
+					Name:          "a",
+					LogURLs:       map[string]bool{"a1.com": true, "a2.com": true, "a3.com": true, "a4.com": true},
+					MinInclusions: 1,
+					IsBase:        false,
+					LogWeights:    map[string]float32{"a1.com": 1.0, "a2.com": 1.0, "a3.com": 1.0, "a4.com": 1.0},
+				},
+				ctpolicy.BaseName: {
+					Name:          ctpolicy.BaseName,
+					LogURLs:       map[string]bool{"a1.com": true, "a2.com": true, "a3.com": true, "a4.com": true},
+					MinInclusions: 2,
+					MinOperators:  2,
+					IsBase:        true,
+					LogWeights:    map[string]float32{"a1.com": 1.0, "a2.com": 1.0, "a3.com": 1.0, "a4.com": 1.0},
+				},
+			},
+			errRegexp: regexp.MustCompile("didn't receive enough SCTs"),
+		},
 	}
 
 	for _, tc := range testCases {