Use MinDistinctOperators instead of MaxSubmissionsPerOperator to reduce confusion

Author: Freddy Zhang

ctpolicy/chromepolicy.go

@@ -22,7 +22,8 @@ import (
 )
 
 const (
-	dayDuration = 86400 * time.Second // time.Duration of one day
+	dayDuration          = 86400 * time.Second // time.Duration of one day
+	minDistinctOperators = 2                   // Number of distinct CT log operators that submit an SCT
 )
 
 // ChromeCTPolicy implements logic for complying with Chrome's CT log policy
@@ -44,20 +45,18 @@ func (chromeP ChromeCTPolicy) LogsByGroup(cert *x509.Certificate, approved *logl
 		}
 		groups[info.Name] = info
 	}
-	var incCount, maxSubmissionsPerOperator int
+	var incCount int
 	switch t := certLifetime(cert); {
 	case t <= 180*dayDuration:
 		incCount = 2
-		maxSubmissionsPerOperator = 1
 	default:
 		incCount = 3
-		maxSubmissionsPerOperator = 2
 	}
 	baseGroup, err := BaseGroupFor(approved, incCount)
 	if err != nil {
 		return nil, err
 	}
-	baseGroup.MaxSubmissionsPerOperator = maxSubmissionsPerOperator
+	baseGroup.MinDistinctOperators = minDistinctOperators
 	groups[baseGroup.Name] = baseGroup
 	return groups, nil
 }

ctpolicy/chromepolicy_test.go

@@ -61,8 +61,9 @@ func wantedGroups(base int, minusBob bool) LogPolicyData {
 				"https://ct.googleapis.com/racketeer/":      true,
 				"https://log.bob.io":                        true,
 			},
-			MinInclusions: base,
-			IsBase:        true,
+			MinInclusions:        base,
+			MinDistinctOperators: minDistinctOperators,
+			IsBase:               true,
 			LogWeights: map[string]float32{
 				"https://ct.googleapis.com/logs/argon2020/": 1.0,
 				"https://ct.googleapis.com/aviator/":        1.0,
@@ -73,12 +74,6 @@ func wantedGroups(base int, minusBob bool) LogPolicyData {
 			},
 		},
 	}
-	switch base {
-	case 2:
-		gi[BaseName].MaxSubmissionsPerOperator = 1
-	case 3:
-		gi[BaseName].MaxSubmissionsPerOperator = 2
-	}
 	if minusBob {
 		delete(gi[BaseName].LogURLs, "https://log.bob.io")
 		delete(gi[BaseName].LogWeights, "https://log.bob.io")

ctpolicy/ctpolicy.go

@@ -31,13 +31,13 @@ const (
 
 // LogGroupInfo holds information on a single group of logs specified by Policy.
 type LogGroupInfo struct {
-	Name                      string
-	LogURLs                   map[string]bool    // set of members
-	MinInclusions             int                // Required number of submissions.
-	MaxSubmissionsPerOperator int                // Maximum number of submissions from a CT log operator.
-	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
+	Name                 string
+	LogURLs              map[string]bool    // set of members
+	MinInclusions        int                // Required number of submissions.
+	MinDistinctOperators int                // Required number of distinct CT log operators that submit an SCT.
+	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
 }
 
 func (group *LogGroupInfo) setMinInclusions(i int) error {

submission/distributor_test.go

@@ -242,7 +242,6 @@ func buildStubCTPolicy(n int) stubCTPolicy {
 
 func (stubP stubCTPolicy) LogsByGroup(cert *x509.Certificate, approved *loglist3.LogList) (ctpolicy.LogPolicyData, error) {
 	baseGroup, err := ctpolicy.BaseGroupFor(approved, stubP.baseNum)
-	baseGroup.MaxSubmissionsPerOperator = 1
 	groups := ctpolicy.LogPolicyData{baseGroup.Name: baseGroup}
 	return groups, err
 }

submission/races.go

@@ -52,12 +52,12 @@ type groupState struct {
 // When some group is complete cancels all requests that are not needed by any
 // group.
 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.
-	maxSubmissionsPerGroup int            // maximum number of logs that can be submitted to a group.
+	mu                sync.Mutex
+	logToGroups       map[string]ctpolicy.GroupSet
+	groupNeeds        map[string]int // number of logs that need to be submitted for each group.
+	minDistinctGroups int            // number of groups that need a submission
 
-	groups  map[string]int // number of logs submitted to each group..
+	groups  map[string]bool // groups that have a stored result
 	results map[string]*submissionResult
 	cancels map[string]context.CancelFunc
 }
@@ -70,9 +70,9 @@ func newSafeSubmissionState(groups ctpolicy.LogPolicyData) *safeSubmissionState
 		s.groupNeeds[g.Name] = g.MinInclusions
 	}
 	if baseGroup, ok := groups[ctpolicy.BaseName]; ok {
-		s.maxSubmissionsPerGroup = baseGroup.MaxSubmissionsPerOperator
+		s.minDistinctGroups = baseGroup.MinDistinctOperators
 	}
-	s.groups = make(map[string]int)
+	s.groups = make(map[string]bool)
 	s.results = make(map[string]*submissionResult)
 	s.cancels = make(map[string]context.CancelFunc)
 	return &s
@@ -90,10 +90,6 @@ func (sub *safeSubmissionState) request(logURL string, cancel context.CancelFunc
 	sub.results[logURL] = &submissionResult{}
 	isAwaited := false
 	for g := range sub.logToGroups[logURL] {
-		if g != ctpolicy.BaseName && sub.groups[g] < sub.maxSubmissionsPerGroup {
-			isAwaited = true
-			break
-		}
 		if sub.groupNeeds[g] > 0 {
 			isAwaited = true
 			break
@@ -119,21 +115,21 @@ func (sub *safeSubmissionState) setResult(logURL string, sct *ct.SignedCertifica
 	}
 	// group name associated with logURL outside of BaseName.
 	// (this assumes the logURL is associated with only one group ignoring BaseName)
-	var nonBaseGroupName string
 	// If at least one group needs that SCT, result is set. Otherwise dumped.
 	for groupName := range sub.logToGroups[logURL] {
 		// Ignore the base group (All-logs) here to check separately.
 		if groupName == ctpolicy.BaseName {
 			continue
 		}
-		nonBaseGroupName = groupName
-		if sub.groups[groupName] < sub.maxSubmissionsPerGroup {
+		// Set the result if the group does not have a submission.
+		if !sub.groups[groupName] {
 			sub.results[logURL] = &submissionResult{sct: sct, err: err}
+			sub.groups[groupName] = true
 		}
 		if sub.groupNeeds[groupName] > 0 {
 			sub.results[logURL] = &submissionResult{sct: sct, err: err}
+			sub.groups[groupName] = true
 		}
-		sub.groups[groupName]++
 		sub.groupNeeds[groupName]--
 	}
 
@@ -143,19 +139,12 @@ func (sub *safeSubmissionState) setResult(logURL string, sct *ct.SignedCertifica
 			// It is already processed in a non-base group, so we can reduce the groupNeeds for the base group as well.
 			sub.groupNeeds[ctpolicy.BaseName]--
 		} else if sub.groupNeeds[ctpolicy.BaseName] > 0 {
-			minInclusionsForOtherGroup := 0
-			for g, cnt := range sub.groupNeeds {
-				if g != ctpolicy.BaseName && cnt > 0 {
-					minInclusionsForOtherGroup += cnt
-				}
-			}
+			extraSubmissions := sub.minDistinctGroups - len(sub.groups)
 			// Set the result only if the base group still needs SCTs more than total counts
 			// of minimum inclusions for other groups.
-			if sub.groupNeeds[ctpolicy.BaseName] > minInclusionsForOtherGroup {
-				if sub.groups[nonBaseGroupName] < sub.maxSubmissionsPerGroup {
-					sub.results[logURL] = &submissionResult{sct: sct, err: err}
-					sub.groupNeeds[ctpolicy.BaseName]--
-				}
+			if sub.groupNeeds[ctpolicy.BaseName] > extraSubmissions {
+				sub.results[logURL] = &submissionResult{sct: sct, err: err}
+				sub.groupNeeds[ctpolicy.BaseName]--
 			}
 		}
 	}
@@ -165,10 +154,6 @@ func (sub *safeSubmissionState) setResult(logURL string, sct *ct.SignedCertifica
 	for logURL, groupSet := range sub.logToGroups {
 		isAwaited := false
 		for g := range groupSet {
-			if g != ctpolicy.BaseName && sub.groups[g] < sub.maxSubmissionsPerGroup {
-				isAwaited = true
-				break
-			}
 			if sub.groupNeeds[g] > 0 {
 				isAwaited = true
 				break
@@ -189,10 +174,8 @@ func (sub *safeSubmissionState) groupComplete(groupName string) bool {
 	if !ok {
 		return true
 	}
-	for _, submission := range sub.groups {
-		if submission < sub.maxSubmissionsPerGroup {
-			return false
-		}
+	if len(sub.groups) < sub.minDistinctGroups {
+		return false
 	}
 	return needs <= 0
 }

submission/races_test.go

@@ -136,12 +136,12 @@ func TestGetSCTs(t *testing.T) {
 					LogWeights:    map[string]float32{"b1.com": 1.0, "b2.com": 1.0, "b3.com": 1.0, "b4.com": 1.0},
 				},
 				ctpolicy.BaseName: {
-					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:             2,
-					MaxSubmissionsPerOperator: 1,
-					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},
+					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:        2,
+					MinDistinctOperators: 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: 2},
@@ -158,11 +158,12 @@ func TestGetSCTs(t *testing.T) {
 					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,
-					IsBase:        true,
-					LogWeights:    map[string]float32{"a1.com": 1.0, "a2.com": 1.0, "a3.com": 1.0, "a4.com": 1.0},
+					Name:                 ctpolicy.BaseName,
+					LogURLs:              map[string]bool{"a1.com": true, "a2.com": true, "a3.com": true, "a4.com": true},
+					MinInclusions:        2,
+					MinDistinctOperators: 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"),