Repository: google/certificate-transparency-go
Pull request #1154 (2023-09-20 16:19:09)

Revert "Update chromepolicy.go to follow the updated Chrome CT Policy"
Reverts #1067 

I had to manually revert the changes.

### 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.
- [x] I have updated [documentation](docs/) accordingly.



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 668f3373..7fdf25c1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,9 +8,6 @@
 ### Add support for IBM Z operating system z/OS
 * Add build tags for zos
 
-### Chrome CT Policy Update
- * #906: Update chromepolicy.go to follow the updated Chrome CT policy.
-
 ### LogList
  * Add support for "is_all_logs" field
 
diff --git a/ctpolicy/chromepolicy.go b/ctpolicy/chromepolicy.go
index 910fb7d6..fbfd5fed 100644
--- a/ctpolicy/chromepolicy.go
+++ b/ctpolicy/chromepolicy.go
@@ -15,17 +15,10 @@
 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 {
 }
@@ -34,30 +27,37 @@ 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) {
-	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
+	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
 	}
 	var incCount int
-	switch t := certLifetime(cert); {
-	case t <= 180*dayDuration:
+	switch m := lifetimeInMonths(cert); {
+	case m < 15:
 		incCount = 2
-	default:
+	case m <= 27:
 		incCount = 3
+	case m <= 39:
+		incCount = 4
+	default:
+		incCount = 5
 	}
 	baseGroup, err := BaseGroupFor(approved, incCount)
 	if err != nil {
 		return nil, err
 	}
-	baseGroup.MinOperators = minOperators
-	groups[baseGroup.Name] = baseGroup
+	groups := LogPolicyData{
+		googGroup.Name:    &googGroup,
+		nonGoogGroup.Name: &nonGoogGroup,
+		baseGroup.Name:    baseGroup,
+	}
 	return groups, nil
 }
 
diff --git a/ctpolicy/ctpolicy.go b/ctpolicy/ctpolicy.go
index d0d44ac5..a250eadd 100644
--- a/ctpolicy/ctpolicy.go
+++ b/ctpolicy/ctpolicy.go
@@ -18,7 +18,6 @@ package ctpolicy
 import (
 	"fmt"
 	"sync"
-	"time"
 
 	"github.com/google/certificate-transparency-go/loglist3"
 	"github.com/google/certificate-transparency-go/x509"
@@ -34,7 +33,6 @@ 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
@@ -207,13 +205,6 @@ 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 f9edb1fa..7367dd1d 100644
--- a/submission/races.go
+++ b/submission/races.go
@@ -54,10 +54,8 @@ type groupState struct {
 type safeSubmissionState struct {
 	mu          sync.Mutex
 	logToGroups map[string]ctpolicy.GroupSet
-	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.
+	groupNeeds  map[string]int
 
-	groups  map[string]bool // set of groups that have a log submitted.
 	results map[string]*submissionResult
 	cancels map[string]context.CancelFunc
 }
@@ -69,10 +67,6 @@ 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
@@ -122,7 +116,6 @@ 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]--
 	}
 
@@ -172,9 +165,6 @@ 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 eba41ba7..061558f4 100644
--- a/ctpolicy/chromepolicy_test.go
+++ b/ctpolicy/chromepolicy_test.go
@@ -21,10 +21,10 @@ import (
 	"github.com/kylelemons/godebug/pretty"
 )
 
-func wantedGroups(base int, minusBob bool) LogPolicyData {
+func wantedGroups(goog int, nonGoog int, base int, minusBob bool) LogPolicyData {
 	gi := LogPolicyData{
-		"Google": {
-			Name: "Google",
+		"Google-operated": {
+			Name: "Google-operated",
 			LogURLs: map[string]bool{
 				"https://ct.googleapis.com/logs/argon2020/": true,
 				"https://ct.googleapis.com/aviator/":        true,
@@ -32,7 +32,8 @@ func wantedGroups(base int, minusBob bool) LogPolicyData {
 				"https://ct.googleapis.com/rocketeer/":      true,
 				"https://ct.googleapis.com/racketeer/":      true,
 			},
-			IsBase: false,
+			MinInclusions: goog,
+			IsBase:        false,
 			LogWeights: map[string]float32{
 				"https://ct.googleapis.com/logs/argon2020/": 1.0,
 				"https://ct.googleapis.com/aviator/":        1.0,
@@ -41,12 +42,13 @@ func wantedGroups(base int, minusBob bool) LogPolicyData {
 				"https://ct.googleapis.com/racketeer/":      1.0,
 			},
 		},
-		"Bob's CT Log Shop": {
-			Name: "Bob's CT Log Shop",
+		"Non-Google-operated": {
+			Name: "Non-Google-operated",
 			LogURLs: map[string]bool{
 				"https://log.bob.io": true,
 			},
-			IsBase: false,
+			MinInclusions: nonGoog,
+			IsBase:        false,
 			LogWeights: map[string]float32{
 				"https://log.bob.io": 1.0,
 			},
@@ -62,7 +64,6 @@ func wantedGroups(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,
@@ -77,12 +78,11 @@ func wantedGroups(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["Bob's CT Log Shop"].LogURLs, "https://log.bob.io")
-		delete(gi["Bob's CT Log Shop"].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")
 	}
 	return gi
 }
-
 func TestCheckChromePolicy(t *testing.T) {
 	tests := []struct {
 		name string
@@ -92,28 +92,27 @@ func TestCheckChromePolicy(t *testing.T) {
 		{
 			name: "Short",
 			cert: getTestCertPEMShort(),
-			want: wantedGroups(2, false),
+			want: wantedGroups(1, 1, 2, false),
 		},
 		{
 			name: "2-year",
 			cert: getTestCertPEM2Years(),
-			want: wantedGroups(3, false),
+			want: wantedGroups(1, 1, 3, false),
 		},
 		{
 			name: "3-year",
 			cert: getTestCertPEM3Years(),
-			want: wantedGroups(3, false),
+			want: wantedGroups(1, 1, 4, false),
 		},
 		{
 			name: "Long",
 			cert: getTestCertPEMLongOriginal(),
-			want: wantedGroups(3, false),
+			want: wantedGroups(1, 1, 5, false),
 		},
 	}
 
 	var policy ChromeCTPolicy
 	sampleLogList := sampleLogList(t)
-
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			got, err := policy.LogsByGroup(test.cert, sampleLogList)
@@ -126,3 +125,59 @@ 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 06200485..0d4c1576 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(2012, 6, 1, 0, 0, 0, 0, time.UTC)
+	cert.NotAfter = time.Date(2013, 1, 1, 0, 0, 0, 0, time.UTC)
 	return cert
 }
 
@@ -104,52 +104,6 @@ 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 90b8045b..a90ab674 100644
--- a/submission/races_test.go
+++ b/submission/races_test.go
@@ -139,35 +139,12 @@ 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 {