Repository: google/certificate-transparency-go
Pull request #641 (2020-01-17 12:13:59)

Add a VerifySCT method taking a SignatureVerifier
Adding for use by tools that have already created a ct.SignatureVerifier for verifying all sorts of signatures, and would prefer to use that rather than have a new one spun up every time they want to verify an SCT.

### 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/641?src=pr&el=h1) Report
> Merging [#641](https://codecov.io/gh/google/certificate-transparency-go/pull/641?src=pr&el=desc) into [master](https://codecov.io/gh/google/certificate-transparency-go/commit/84499acf14993f0a353bab785e923bc45f19410f?src=pr&el=desc) will **increase** coverage by `0.04%`.
> The diff coverage is `75%`.

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

```diff
@@            Coverage Diff             @@
##           master     #641      +/-   ##
==========================================
+ Coverage   71.77%   71.81%   +0.04%     
==========================================
  Files          90       90              
  Lines        9859     9867       +8     
==========================================
+ Hits         7076     7086      +10     
+ Misses       2286     2285       -1     
+ Partials      497      496       -1
```


| [Impacted Files](https://codecov.io/gh/google/certificate-transparency-go/pull/641?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [ctutil/ctutil.go](https://codecov.io/gh/google/certificate-transparency-go/pull/641/diff?src=pr&el=tree#diff-Y3R1dGlsL2N0dXRpbC5nbw==) | `76.66% <75%> (-0.26%)` | :arrow_down: |
| [submission/races.go](https://codecov.io/gh/google/certificate-transparency-go/pull/641/diff?src=pr&el=tree#diff-c3VibWlzc2lvbi9yYWNlcy5nbw==) | `90.83% <0%> (+3.05%)` | :arrow_up: |

------

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

taknira: This is largely a copy-past of the existing VerifySCT() and corresponding tests, but taking a ct.SignatureVerifier instead of a crypto.PublicKey.
taknira: PR description updated.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6fa44259..8f44a26f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -61,6 +61,10 @@ The `ct_server` binary added the following flags:
 
 Merged upstream Go 1.13 changes.
 
+#### ctutil
+
+Added VerifySCTWithVerifier() to verify SCTs using a given ct.SignatureVerifier.
+
 ### Configuration Files
 
 Configuration files that previously had to be text-encoded Protobuf messages can
diff --git a/ctutil/ctutil.go b/ctutil/ctutil.go
index 43aaa427..de07ccb9 100644
--- a/ctutil/ctutil.go
+++ b/ctutil/ctutil.go
@@ -108,18 +108,51 @@ func LeafHash(chain []*x509.Certificate, sct *ct.SignedCertificateTimestamp, emb
 //       certificate must also be provided in chain.  The certificate containing
 //       the embedded SCT should be at chain[0], and its issuer at chain[1].
 func VerifySCT(pubKey crypto.PublicKey, chain []*x509.Certificate, sct *ct.SignedCertificateTimestamp, embedded bool) error {
-	leaf, err := createLeaf(chain, sct, embedded)
+	s, err := ct.NewSignatureVerifier(pubKey)
 	if err != nil {
-		return err
+		return fmt.Errorf("error creating signature verifier: %s", err)
 	}
 
-	s, err := ct.NewSignatureVerifier(pubKey)
+	return VerifySCTWithVerifier(s, chain, sct, embedded)
+}
+
+// VerifySCTWithVerifier takes a ct.SignatureVerifier, a certificate chain, and
+// an SCT and verifies whether the SCT is a valid SCT for the certificate at
+// chain[0], signed by the Log whose public key was used to set up the
+// ct.SignatureVerifier.  If the SCT does not verify, an error will be returned.
+//
+// This function can be used with three different types of leaf certificate:
+//   - X.509 Certificate:
+//       If using this function to verify an SCT for a normal X.509 certificate
+//       then it is enough to just provide the end entity certificate in chain.
+//       This case assumes that the SCT being provided is not embedded within
+//       the leaf certificate provided, i.e. the certificate is what was
+//       submitted to the Certificate Transparency Log in order to obtain the
+//       SCT.  For this case, set embedded to false.
+//   - Precertificate:
+//       If using this function to verify an SCT for a precertificate then the
+//       issuing certificate must also be provided in chain.  The precertificate
+//       should be at chain[0], and its issuer at chain[1].  For this case, set
+//       embedded to false.
+//   - X.509 Certificate containing the SCT embedded within it:
+//       If the SCT you wish to verify is embedded within the certificate you
+//       are providing at chain[0], set embedded to true.  VerifySCT will
+//       verify the provided SCT by building the corresponding precertificate.
+//       VerifySCT will return an error if the provided SCT cannot be found
+//       embedded within chain[0].  As with the precertificate case, the issuing
+//       certificate must also be provided in chain.  The certificate containing
+//       the embedded SCT should be at chain[0], and its issuer at chain[1].
+func VerifySCTWithVerifier(sv *ct.SignatureVerifier, chain []*x509.Certificate, sct *ct.SignedCertificateTimestamp, embedded bool) error {
+	if sv == nil {
+		return errors.New("ct.SignatureVerifier is nil")
+	}
+
+	leaf, err := createLeaf(chain, sct, embedded)
 	if err != nil {
-		return fmt.Errorf("error creating signature verifier: %s", err)
+		return err
 	}
 
-	entry := ct.LogEntry{Leaf: *leaf}
-	return s.VerifySCTSignature(*sct, entry)
+	return sv.VerifySCTSignature(*sct, ct.LogEntry{Leaf: *leaf})
 }
 
 func createLeaf(chain []*x509.Certificate, sct *ct.SignedCertificateTimestamp, embedded bool) (*ct.MerkleTreeLeaf, error) {


Test patch:
diff --git a/ctutil/ctutil_test.go b/ctutil/ctutil_test.go
index 54bad4eb..29db8506 100644
--- a/ctutil/ctutil_test.go
+++ b/ctutil/ctutil_test.go
@@ -219,6 +219,93 @@ func TestVerifySCT(t *testing.T) {
 	}
 }
 
+func TestVerifySCTWithVerifier(t *testing.T) {
+	// Parse public key
+	pk, err := ct.PublicKeyFromB64(testdata.LogPublicKeyB64)
+	if err != nil {
+		t.Errorf("error parsing public key: %s", err)
+	}
+
+	// Create signature verifier
+	sv, err := ct.NewSignatureVerifier(pk)
+	if err != nil {
+		t.Errorf("couldn't create signature verifier: %s", err)
+	}
+
+	tests := []struct {
+		desc     string
+		sv       *ct.SignatureVerifier
+		chainPEM string
+		sct      []byte
+		embedded bool
+		wantErr  bool
+	}{
+		{
+			desc:     "nil signature verifier",
+			sv:       nil,
+			chainPEM: testdata.TestCertPEM + testdata.CACertPEM,
+			sct:      testdata.TestCertProof,
+			wantErr:  true,
+		},
+		{
+			desc:     "cert",
+			sv:       sv,
+			chainPEM: testdata.TestCertPEM + testdata.CACertPEM,
+			sct:      testdata.TestCertProof,
+		},
+		{
+			desc:     "precert",
+			sv:       sv,
+			chainPEM: testdata.TestPreCertPEM + testdata.CACertPEM,
+			sct:      testdata.TestPreCertProof,
+		},
+		{
+			desc:     "invalid SCT",
+			sv:       sv,
+			chainPEM: testdata.TestPreCertPEM + testdata.CACertPEM,
+			sct:      testdata.TestCertProof,
+			wantErr:  true,
+		},
+		{
+			desc:     "cert with embedded SCT",
+			sv:       sv,
+			chainPEM: testdata.TestEmbeddedCertPEM + testdata.CACertPEM,
+			sct:      testdata.TestPreCertProof,
+			embedded: true,
+		},
+		{
+			desc:     "cert with invalid embedded SCT",
+			sv:       sv,
+			chainPEM: testdata.TestInvalidEmbeddedCertPEM + testdata.CACertPEM,
+			sct:      testdata.TestInvalidProof,
+			embedded: true,
+			wantErr:  true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			// Parse chain
+			chain, err := x509util.CertificatesFromPEM([]byte(test.chainPEM))
+			if err != nil {
+				t.Fatalf("error parsing certificate chain: %s", err)
+			}
+
+			// Parse SCT
+			var sct ct.SignedCertificateTimestamp
+			if _, err = tls.Unmarshal(test.sct, &sct); err != nil {
+				t.Fatalf("error tls-unmarshalling sct: %s", err)
+			}
+
+			// Test VerifySCTWithVerifier()
+			err = VerifySCTWithVerifier(test.sv, chain, &sct, test.embedded)
+			if gotErr := err != nil; gotErr != test.wantErr {
+				t.Errorf("VerifySCT(_,_,_, %t) = %v, want error? %t", test.embedded, err, test.wantErr)
+			}
+		})
+	}
+}
+
 func TestContainsSCT(t *testing.T) {
 	tests := []struct {
 		desc    string