Skip to content

Commit 7bf8c12

Browse files
authored
Fix certcache to allow self-signed certs. (#360)
If the cert doesn't have an AIA extension with an OCSP responder URL, then the cache will just include a fake (invalid) OCSP response in its cert-chain+cbor. This is sufficient for displaying the SXG in Chrome with --ignore-certificate-errors-spki-list.
1 parent f5c5c98 commit 7bf8c12

File tree

5 files changed

+83
-11
lines changed

5 files changed

+83
-11
lines changed

cmd/amppkg/main.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package main
1919

2020
import (
21+
"crypto/ecdsa"
2122
"flag"
2223
"fmt"
2324
"io/ioutil"
@@ -87,7 +88,12 @@ func main() {
8788
die(errors.Wrap(err, "loading key file"))
8889
}
8990

90-
certCache, err := certcache.PopulateCertCache(config, key, *flagDevelopment || *flagInvalidCert, *flagAutoRenewCert)
91+
var responder certcache.OCSPResponder = nil
92+
if *flagDevelopment {
93+
// Key is guaranteed to be ECDSA by signedexchange.ParsePrivateKey. This may change in future versions of SXG.
94+
responder = fakeOCSPResponder{key: key.(*ecdsa.PrivateKey)}.Respond
95+
}
96+
certCache, err := certcache.PopulateCertCache(config, key, responder, *flagDevelopment || *flagInvalidCert, *flagAutoRenewCert)
9197
if err != nil {
9298
die(errors.Wrap(err, "building cert cache"))
9399
}

cmd/amppkg/ocsp.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package main
2+
3+
import (
4+
"crypto"
5+
"crypto/x509"
6+
"crypto/x509/pkix"
7+
"encoding/asn1"
8+
"time"
9+
10+
"golang.org/x/crypto/ocsp"
11+
)
12+
13+
// A fake OCSP responder for the SXG certificate. Abstracted from certcache so as not to give it direct access to the private key.
14+
// Assumes that the given cert is self-signed, and therefore that the
15+
// configured KeyFile also corresponds to the cert's issuer.
16+
type fakeOCSPResponder struct {
17+
key crypto.Signer
18+
}
19+
20+
// Generates a current OCSP response for the given certificate.
21+
func (this fakeOCSPResponder) Respond(cert *x509.Certificate) ([]byte, error) {
22+
thisUpdate := time.Now()
23+
24+
// Construct args to ocsp.CreateResponse.
25+
template := ocsp.Response{
26+
SerialNumber: cert.SerialNumber,
27+
Status: ocsp.Good,
28+
ThisUpdate: thisUpdate,
29+
NextUpdate: thisUpdate.Add(time.Hour * 24 * 7),
30+
IssuerHash: crypto.SHA256,
31+
}
32+
subjectDER, err := asn1.Marshal(pkix.Name{CommonName: "fake-responder.example"}.ToRDNSequence())
33+
if err != nil {
34+
return nil, err
35+
}
36+
responderCert := x509.Certificate{
37+
// This is the only field that ocsp.CreateResponse reads.
38+
RawSubject: subjectDER,
39+
}
40+
resp, err := ocsp.CreateResponse(cert, &responderCert, template, this.key)
41+
if err != nil {
42+
return nil, err
43+
}
44+
_, err = ocsp.ParseResponseForCert(resp, cert, cert)
45+
if err != nil {
46+
return nil, err
47+
}
48+
return resp, nil
49+
}

cmd/gateway_server/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func (s *gatewayServer) GenerateSXG(ctx context.Context, request *pb.SXGRequest)
7474
}
7575

7676
// Note: do not initialize certCache, we just want it to hold the certs for now.
77-
certCache := certcache.New(certs, nil, []string{""}, "", "", "");
77+
certCache := certcache.New(certs, nil, []string{""}, "", "", "", nil);
7878

7979
privateKey, err := util.ParsePrivateKey(request.PrivateKey)
8080
if err != nil {

packager/certcache/certcache.go

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ const maxOCSPTries = 10
6969
// TODO(banaag): make 2 days renewal grace period configurable in toml.
7070
const certRenewalInterval = 8 * 24 * time.Hour
7171

72+
type OCSPResponder func(*x509.Certificate) ([]byte, error)
73+
7274
type CertHandler interface {
7375
GetLatestCert() *x509.Certificate
7476
IsHealthy() error
@@ -90,6 +92,10 @@ type CertCache struct {
9092
ocspFile Updateable
9193
ocspFilePath string
9294
client http.Client
95+
// Given a certificate, returns a current OCSP response for the cert;
96+
// this is a fallback, called when in development mode and there is no
97+
// OCSP URL.
98+
generateOCSPResponse OCSPResponder
9399
// Domains to validate
94100
Domains []string
95101
CertFile string
@@ -114,7 +120,7 @@ type CertCache struct {
114120
// An alternative pattern would be to create an IsInitialized() bool or similarly named function that verifies all of the required fields have
115121
// been set. Then callers can just set fields in the struct by name and assert IsInitialized before doing anything with it.
116122
func New(certs []*x509.Certificate, certFetcher *certfetcher.CertFetcher, domains []string,
117-
certFile string, newCertFile string, ocspCache string) *CertCache {
123+
certFile string, newCertFile string, ocspCache string, generateOCSPResponse OCSPResponder) *CertCache {
118124
certName := ""
119125
if len(certs) > 0 && certs[0] != nil {
120126
certName = util.CertName(certs[0])
@@ -135,6 +141,7 @@ func New(certs []*x509.Certificate, certFetcher *certfetcher.CertFetcher, domain
135141
// you'd have one request, in the backend, and updating them all.
136142
ocspFile: &Chained{first: &InMemory{}, second: &LocalFile{path: ocspCache}},
137143
ocspFilePath: ocspCache,
144+
generateOCSPResponse: generateOCSPResponse,
138145
client: http.Client{Timeout: 60 * time.Second},
139146
extractOCSPServer: func(cert *x509.Certificate) (string, error) {
140147
if cert == nil || len(cert.OCSPServer) < 1 {
@@ -441,8 +448,8 @@ func (this *CertCache) maintainOCSP(stop chan struct{}) {
441448
}
442449

443450
// Returns true if OCSP is expired (or near enough).
444-
func (this *CertCache) shouldUpdateOCSP(bytes []byte) bool {
445-
if len(bytes) == 0 {
451+
func (this *CertCache) shouldUpdateOCSP(ocsp []byte) bool {
452+
if len(ocsp) == 0 {
446453
// TODO(twifkak): Use a logging framework with support for debug-only statements.
447454
log.Println("Updating OCSP; none cached yet.")
448455
return true
@@ -454,7 +461,7 @@ func (this *CertCache) shouldUpdateOCSP(bytes []byte) bool {
454461
return false
455462
}
456463
// Compute the midpoint per sleevi #3 (see above).
457-
midpoint, err := this.ocspMidpoint(bytes, issuer)
464+
midpoint, err := this.ocspMidpoint(ocsp, issuer)
458465
if err != nil {
459466
log.Println("Error computing OCSP midpoint:", err)
460467
return true
@@ -537,8 +544,17 @@ func (this *CertCache) fetchOCSP(orig []byte, certs []*x509.Certificate, ocspUpd
537544

538545
ocspServer, err := this.extractOCSPServer(certs[0])
539546
if err != nil {
540-
log.Println("Error extracting OCSP server:", err)
541-
return orig
547+
if this.generateOCSPResponse == nil {
548+
log.Println("Error extracting OCSP server:", err)
549+
return orig
550+
}
551+
log.Println("Cert lacks OCSP URL; using fake OCSP in development mode.")
552+
resp, err := this.generateOCSPResponse(certs[0])
553+
if err != nil {
554+
log.Println("error generating fake OCSP response:", err)
555+
return orig
556+
}
557+
return resp
542558
}
543559

544560
// Conform to the Lightweight OCSP Profile, by preferring GET over POST
@@ -821,7 +837,7 @@ func (this *CertCache) reloadCertIfExpired() {
821837
// Creates cert cache by loading certs and keys from disk, doing validation
822838
// and populating the cert cache with current set of certificate related information.
823839
// If development mode is true, prints a warning for certs that can't sign HTTP exchanges.
824-
func PopulateCertCache(config *util.Config, key crypto.PrivateKey,
840+
func PopulateCertCache(config *util.Config, key crypto.PrivateKey, generateOCSPResponse OCSPResponder,
825841
developmentMode bool, autoRenewCert bool) (*CertCache, error) {
826842

827843
if config.CertFile == "" {
@@ -851,7 +867,7 @@ func PopulateCertCache(config *util.Config, key crypto.PrivateKey,
851867
if err != nil {
852868
return nil, errors.Wrap(err, "creating cert fetcher from config")
853869
}
854-
certCache := New(certs, certFetcher, []string{domain}, config.CertFile, config.NewCertFile, config.OCSPCache)
870+
certCache := New(certs, certFetcher, []string{domain}, config.CertFile, config.NewCertFile, config.OCSPCache, generateOCSPResponse)
855871

856872
return certCache, nil
857873
}

packager/certcache/certcache_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func (this *CertCacheSuite) New() (*CertCache, error) {
8181
// TODO(banaag): Consider adding a test with certfetcher set.
8282
// For now, this tests certcache without worrying about certfetcher.
8383
certCache := New(pkgt.B3Certs, nil, []string{"example.com"}, "cert.crt", "newcert.crt",
84-
filepath.Join(this.tempDir, "ocsp"))
84+
filepath.Join(this.tempDir, "ocsp"), nil)
8585
certCache.extractOCSPServer = func(*x509.Certificate) (string, error) {
8686
return this.ocspServer.URL, nil
8787
}
@@ -357,6 +357,7 @@ func (this *CertCacheSuite) TestPopulateCertCache() {
357357
}},
358358
},
359359
pkgt.B3Key,
360+
nil,
360361
true,
361362
false)
362363
this.Require().NoError(err)

0 commit comments

Comments
 (0)