Repository: google/certificate-transparency-go
Pull request #647 (2020-02-06 00:45:43)

Fix Sharding logic with using notbefore of a certificate.
According to https://github.com/chromium/ct-policy/issues/6, a NotAfter
validity field of a X509 Certificate should be used.

The policy is as follows.
For a certificate to be accepted by the Log that has a time range
specified.
The certificate’s ‘Not After’ validity field value must:
Be at or after ‘Start’ and Be before ‘End’
fixes #647 

### 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!
-->

- [ ] 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/647?src=pr&el=h1) Report
> Merging [#647](https://codecov.io/gh/google/certificate-transparency-go/pull/647?src=pr&el=desc) into [master](https://codecov.io/gh/google/certificate-transparency-go/commit/203e829bc01eeef3d5db049bba2f3e970ea38f16?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master     #647      +/-   ##
==========================================
- Coverage   71.78%   71.76%   -0.03%     
==========================================
  Files          90       90              
  Lines        9862     9862              
==========================================
- Hits         7079     7077       -2     
- Misses       2286     2288       +2     
  Partials      497      497
```


| [Impacted Files](https://codecov.io/gh/google/certificate-transparency-go/pull/647?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [loglist2/logfilter.go](https://codecov.io/gh/google/certificate-transparency-go/pull/647/diff?src=pr&el=tree#diff-bG9nbGlzdDIvbG9nZmlsdGVyLmdv) | `100% <100%> (ø)` | :arrow_up: |
| [scanner/scanner.go](https://codecov.io/gh/google/certificate-transparency-go/pull/647/diff?src=pr&el=tree#diff-c2Nhbm5lci9zY2FubmVyLmdv) | `55.41% <0%> (-1.28%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/google/certificate-transparency-go/pull/647?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/647?src=pr&el=footer). Last update [203e829...fe75b78](https://codecov.io/gh/google/certificate-transparency-go/pull/647?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/loglist2/logfilter.go b/loglist2/logfilter.go
index 5d055375..e7a32595 100644
--- a/loglist2/logfilter.go
+++ b/loglist2/logfilter.go
@@ -97,7 +97,7 @@ func (ll *LogList) RootCompatible(certRoot *x509.Certificate, roots LogRoots) Lo
 
 // TemporallyCompatible creates a new LogList containing only the logs of
 // original LogList that are compatible with the provided cert, according to
-// NotBefore and TemporalInterval matching.
+// NotAfter and TemporalInterval matching.
 // Returns empty LogList if nil-cert is provided.
 func (ll *LogList) TemporallyCompatible(cert *x509.Certificate) LogList {
 	var compatible LogList
@@ -113,7 +113,7 @@ func (ll *LogList) TemporallyCompatible(cert *x509.Certificate) LogList {
 				compatibleOp.Logs = append(compatibleOp.Logs, l)
 				continue
 			}
-			if cert.NotBefore.Before(l.TemporalInterval.EndExclusive) && (cert.NotBefore.After(l.TemporalInterval.StartInclusive) || cert.NotBefore.Equal(l.TemporalInterval.StartInclusive)) {
+			if cert.NotAfter.Before(l.TemporalInterval.EndExclusive) && (cert.NotAfter.After(l.TemporalInterval.StartInclusive) || cert.NotAfter.Equal(l.TemporalInterval.StartInclusive)) {
 				compatibleOp.Logs = append(compatibleOp.Logs, l)
 			}
 		}


Test patch:
diff --git a/loglist2/logfilter_test.go b/loglist2/logfilter_test.go
index 230879d6..6ebad7e3 100644
--- a/loglist2/logfilter_test.go
+++ b/loglist2/logfilter_test.go
@@ -174,53 +174,53 @@ func TestTemporallyCompatible(t *testing.T) {
 	cert, _ := x509util.CertificateFromPEM([]byte(testdata.TestPreCertPEM))
 
 	tests := []struct {
-		name      string
-		in        LogList
-		cert      *x509.Certificate
-		notBefore time.Time
-		want      LogList
+		name     string
+		in       LogList
+		cert     *x509.Certificate
+		notAfter time.Time
+		want     LogList
 	}{
 		{
-			name:      "AllLogsFitTemporally",
-			in:        sampleLogList,
-			cert:      cert,
-			notBefore: stripErr(time.Parse(time.UnixDate, "Sat Nov 8 11:06:00 PST 2014")),
-			want:      subLogList(map[string]bool{"https://ct.googleapis.com/aviator/": true, "https://log.bob.io": true, "https://ct.googleapis.com/icarus/": true, "https://ct.googleapis.com/racketeer/": true, "https://ct.googleapis.com/rocketeer/": true}),
+			name:     "AllLogsFitTemporally",
+			in:       sampleLogList,
+			cert:     cert,
+			notAfter: stripErr(time.Parse(time.UnixDate, "Sat Nov 8 11:06:00 PST 2014")),
+			want:     subLogList(map[string]bool{"https://ct.googleapis.com/aviator/": true, "https://log.bob.io": true, "https://ct.googleapis.com/icarus/": true, "https://ct.googleapis.com/racketeer/": true, "https://ct.googleapis.com/rocketeer/": true}),
 		},
 		{
-			name:      "OperatorExcludedAllItsLogsMismatch",
-			in:        sampleLogList,
-			cert:      cert,
-			notBefore: stripErr(time.Parse(time.UnixDate, "Sat Mar 8 11:06:00 PST 2014")),
-			want:      subLogList(map[string]bool{"https://ct.googleapis.com/aviator/": true, "https://ct.googleapis.com/icarus/": true, "https://ct.googleapis.com/racketeer/": true, "https://ct.googleapis.com/rocketeer/": true}),
+			name:     "OperatorExcludedAllItsLogsMismatch",
+			in:       sampleLogList,
+			cert:     cert,
+			notAfter: stripErr(time.Parse(time.UnixDate, "Sat Mar 8 11:06:00 PST 2014")),
+			want:     subLogList(map[string]bool{"https://ct.googleapis.com/aviator/": true, "https://ct.googleapis.com/icarus/": true, "https://ct.googleapis.com/racketeer/": true, "https://ct.googleapis.com/rocketeer/": true}),
 		},
 		{
-			name:      "TwoLogsAfterCertTimeExcluded",
-			in:        sampleLogList,
-			cert:      cert,
-			notBefore: stripErr(time.Parse(time.UnixDate, "Sat Mar 8 11:06:00 PST 2013")),
-			want:      subLogList(map[string]bool{"https://ct.googleapis.com/icarus/": true, "https://ct.googleapis.com/racketeer/": true, "https://ct.googleapis.com/rocketeer/": true}),
+			name:     "TwoLogsAfterCertTimeExcluded",
+			in:       sampleLogList,
+			cert:     cert,
+			notAfter: stripErr(time.Parse(time.UnixDate, "Sat Mar 8 11:06:00 PST 2013")),
+			want:     subLogList(map[string]bool{"https://ct.googleapis.com/icarus/": true, "https://ct.googleapis.com/racketeer/": true, "https://ct.googleapis.com/rocketeer/": true}),
 		},
 		{
-			name:      "TwoLogsBeforeCertTimeExcluded",
-			in:        sampleLogList,
-			cert:      cert,
-			notBefore: stripErr(time.Parse(time.UnixDate, "Sat Mar 8 11:06:00 PST 2016")),
-			want:      subLogList(map[string]bool{"https://ct.googleapis.com/icarus/": true, "https://ct.googleapis.com/racketeer/": true, "https://ct.googleapis.com/rocketeer/": true}),
+			name:     "TwoLogsBeforeCertTimeExcluded",
+			in:       sampleLogList,
+			cert:     cert,
+			notAfter: stripErr(time.Parse(time.UnixDate, "Sat Mar 8 11:06:00 PST 2016")),
+			want:     subLogList(map[string]bool{"https://ct.googleapis.com/icarus/": true, "https://ct.googleapis.com/racketeer/": true, "https://ct.googleapis.com/rocketeer/": true}),
 		},
 		{
-			name:      "NilCert",
-			in:        sampleLogList,
-			cert:      nil,
-			notBefore: stripErr(time.Parse(time.UnixDate, "Sat Nov 8 11:06:00 PST 2014")),
-			want:      subLogList(map[string]bool{}),
+			name:     "NilCert",
+			in:       sampleLogList,
+			cert:     nil,
+			notAfter: stripErr(time.Parse(time.UnixDate, "Sat Nov 8 11:06:00 PST 2014")),
+			want:     subLogList(map[string]bool{}),
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			if test.cert != nil {
-				test.cert.NotBefore = test.notBefore
+				test.cert.NotAfter = test.notAfter
 			}
 			got := test.in.TemporallyCompatible(test.cert)
 			if diff := pretty.Compare(test.want, got); diff != "" {