Allow >90-day cert in development. (#316)

When `-development` or `-invalidcert` is passed, a SXG cert with lifetime >90d
will only cause a warning.

Also, add a warning message for missing CanSignHttpExchanges extension in
development.

Author: twifkak

cmd/amppkg/main.go

@@ -90,8 +90,12 @@ func main() {
 	if certs == nil || len(certs) == 0 {
 		die(fmt.Sprintf("no cert found in %s", config.CertFile))
 	}
-	if !(*flagDevelopment || *flagInvalidCert || util.CanSignHttpExchanges(certs[0])) {
-		die("cert is missing CanSignHttpExchanges extension")
+	if err := util.CanSignHttpExchanges(certs[0], time.Now()); err != nil {
+		if *flagDevelopment || *flagInvalidCert {
+			log.Println("WARNING:", err)
+		} else {
+			die(err)
+		}
 	}
 
 	key, err := util.ParsePrivateKey(keyPem)
@@ -101,7 +105,7 @@ func main() {
 
 	for _, urlSet := range config.URLSet {
 		domain := urlSet.Sign.Domain
-		if err := util.CheckCertificate(certs[0], key, domain, time.Now()); err != nil {
+		if err := util.CertificateMatches(certs[0], key, domain); err != nil {
 			die(errors.Wrapf(err, "checking %s", config.CertFile))
 		}
 	}

packager/util/util.go

@@ -71,10 +71,7 @@ func ParsePrivateKey(keyPem []byte) (crypto.PrivateKey, error) {
 	}
 }
 
-// CanSignHttpExchanges returns true if the given certificate has the
-// CanSignHttpExchanges extension. This is not the only requirement for SXGs;
-// it also needs to use the right public key type, which is not checked here.
-func CanSignHttpExchanges(cert *x509.Certificate) bool {
+func hasCanSignHttpExchangesExtension(cert *x509.Certificate) bool {
 	// https://wicg.github.io/webpackage/draft-yasskin-httpbis-origin-signed-exchanges-impl.html#cross-origin-cert-req
 	for _, ext := range cert.Extensions {
 		// 0x05, 0x00 is the DER encoding of NULL.
@@ -85,7 +82,26 @@ func CanSignHttpExchanges(cert *x509.Certificate) bool {
 	return false
 }
 
-func CheckCertificate(cert *x509.Certificate, priv crypto.PrivateKey, domain string, now time.Time) error {
+// CanSignHttpExchanges returns nil if the given certificate has the
+// CanSignHttpExchanges extension, and a valid lifetime per the SXG spec;
+// otherwise it returns an error. These are not the only requirements for SXGs;
+// it also needs to use the right public key type, which is not checked here.
+func CanSignHttpExchanges(cert *x509.Certificate, now time.Time) error {
+	if !hasCanSignHttpExchangesExtension(cert) {
+		return errors.New("Certificate is missing CanSignHttpExchanges extension")
+	}
+
+	// TODO: remove issue date and current time check after 2019-08-01
+	if cert.NotBefore.After(start90DayGracePeriod) || now.After(end90DayGracePeriod) {
+		if cert.NotBefore.AddDate(0,0,90).Before(cert.NotAfter) {
+			return errors.New("Certificate MUST have a Validity Period no greater than 90 days")
+		}
+	}
+	return nil
+}
+
+// Returns nil if the certificate matches the private key and domain, else the appropriate error.
+func CertificateMatches(cert *x509.Certificate, priv crypto.PrivateKey, domain string) error {
 	certPubKey := cert.PublicKey.(*ecdsa.PublicKey)
 	pubKey := priv.(*ecdsa.PrivateKey).PublicKey
 	if certPubKey.Curve != pubKey.Curve {
@@ -100,11 +116,5 @@ func CheckCertificate(cert *x509.Certificate, priv crypto.PrivateKey, domain str
 	if err := cert.VerifyHostname(domain); err != nil {
 		return err
 	}
-	// TODO: remove issue date and current time check after 2019-08-01
-	if cert.NotBefore.After(start90DayGracePeriod) || now.After(end90DayGracePeriod) {
-		if cert.NotBefore.AddDate(0,0,90).Before(cert.NotAfter) {
-			return errors.New("Certificate MUST have a Validity Period no greater than 90 days")
-		}
-	}
 	return nil
 }

packager/util/util_test.go

@@ -29,48 +29,56 @@ func TestParsePrivateKey(t *testing.T) {
 	assert.Equal(t, elliptic.P256(), pkgt.Key.(*ecdsa.PrivateKey).PublicKey.Curve)
 }
 
-func TestCanSignHttpExchanges(t *testing.T) {
+func TestCanSignHttpExchangesExtension(t *testing.T) {
+	// Before grace period, to allow the >90-day lifetime.
+	now := time.Date(2019, time.July, 31, 0, 0, 0, 0, time.UTC)
+
 	// Leaf node has the extension.
-	assert.True(t, util.CanSignHttpExchanges(pkgt.Certs[0]))
+	assert.Nil(t, util.CanSignHttpExchanges(pkgt.Certs[0], now))
 	// CA node does not.
-	assert.False(t, util.CanSignHttpExchanges(pkgt.Certs[1]))
+	assert.EqualError(t, util.CanSignHttpExchanges(pkgt.Certs[1], now), "Certificate is missing CanSignHttpExchanges extension")
 }
 
 func TestParseCertificate(t *testing.T) {
-	assert.Nil(t, util.CheckCertificate(pkgt.B3Certs[0], pkgt.B3Key, "amppackageexample.com", time.Now()))
+	assert.Nil(t, util.CertificateMatches(pkgt.B3Certs[0], pkgt.B3Key, "amppackageexample.com"))
 }
 
 func TestParseCertificateSubjectAltName(t *testing.T) {
-	assert.Nil(t, util.CheckCertificate(pkgt.B3Certs[0], pkgt.B3Key, "www.amppackageexample.com", time.Now()))
+	assert.Nil(t, util.CertificateMatches(pkgt.B3Certs[0], pkgt.B3Key, "www.amppackageexample.com"))
 }
 
 func TestParseCertificateNotMatchX(t *testing.T) {
-	assert.Contains(t, errorFrom(util.CheckCertificate(pkgt.B3Certs[0],
-		pkgt.B3Key2, "amppackageexample.com", time.Now())), "PublicKey.X not match")
+	assert.Contains(t, errorFrom(util.CertificateMatches(pkgt.B3Certs[0],
+		pkgt.B3Key2, "amppackageexample.com")), "PublicKey.X not match")
 }
 
 func TestParseCertificateNotMatchCurve(t *testing.T) {
-	assert.Contains(t, errorFrom(util.CheckCertificate(pkgt.B3Certs[0],
-		pkgt.B3KeyP521, "amppackageexample.com", time.Now())), "PublicKey.Curve not match")
+	assert.Contains(t, errorFrom(util.CertificateMatches(pkgt.B3Certs[0],
+		pkgt.B3KeyP521, "amppackageexample.com")), "PublicKey.Curve not match")
 }
 
 func TestParseCertificateNotMatchDomain(t *testing.T) {
-	assert.Contains(t, errorFrom(util.CheckCertificate(pkgt.B3Certs2[0],
-		pkgt.B3Key2, "amppackageexample.com", time.Now())), "x509: certificate is valid for amppackageexample2.com, www.amppackageexample2.com, not amppackageexample.com")
+	assert.Contains(t, errorFrom(util.CertificateMatches(pkgt.B3Certs2[0],
+		pkgt.B3Key2, "amppackageexample.com")), "x509: certificate is valid for amppackageexample2.com, www.amppackageexample2.com, not amppackageexample.com")
+}
+
+func TestParse90DaysCertificateAfterGracePeriod(t *testing.T) {
+	now := time.Date(2019, time.August, 1, 0, 0, 0, 1, time.UTC)
+	assert.Nil(t, util.CanSignHttpExchanges(pkgt.B3Certs[0], now))
 }
 
 func TestParse91DaysCertificate(t *testing.T) {
-	assert.Contains(t, errorFrom(util.CheckCertificate(pkgt.B3Certs91Days[0],
-		pkgt.B3Key, "amppackageexample.com", time.Now())), "Certificate MUST have a Validity Period no greater than 90 days")
+	assert.Contains(t, errorFrom(util.CanSignHttpExchanges(pkgt.B3Certs91Days[0],
+		time.Now())), "Certificate MUST have a Validity Period no greater than 90 days")
 }
 
 func TestParseCertificateIssuedBeforeMay1InGarcePeriod(t *testing.T) {
 	now := time.Date(2019, time.July, 31, 0, 0, 0, 0, time.UTC)
-	assert.Nil(t, util.CheckCertificate(pkgt.Certs[0], pkgt.Key, "amppackageexample.com", now))
+	assert.Nil(t, util.CanSignHttpExchanges(pkgt.Certs[0], now))
 }
 
 func TestParseCertificateIssuedBeforeMay1AfterGracePeriod(t *testing.T) {
 	now := time.Date(2019, time.August, 1, 0, 0, 0, 1, time.UTC)
-	assert.Contains(t, errorFrom(util.CheckCertificate(pkgt.Certs[0],
-		pkgt.Key, "amppackageexample.com", now)), "Certificate MUST have a Validity Period no greater than 90 days")
+	assert.Contains(t, errorFrom(util.CanSignHttpExchanges(pkgt.Certs[0],
+		now)), "Certificate MUST have a Validity Period no greater than 90 days")
 }