Merge pull request #802 from docker/cert-warnings-int-expiry

Output warnings for cert within 6 months expiry, check intermediate cert expiry

Author: endophage

trustpinning/ca.crt

@@ -0,0 +1,37 @@
+-----BEGIN CERTIFICATE-----
+MIIGMzCCBBugAwIBAgIBATANBgkqhkiG9w0BAQsFADBfMQswCQYDVQQGEwJVUzEL
+MAkGA1UECAwCQ0ExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDzANBgNVBAoMBkRv
+Y2tlcjEaMBgGA1UEAwwRTm90YXJ5IFRlc3RpbmcgQ0EwHhcNMTUwNzE2MDQyNTAz
+WhcNMjUwNzEzMDQyNTAzWjBfMRowGAYDVQQDDBFOb3RhcnkgVGVzdGluZyBDQTEL
+MAkGA1UEBhMCVVMxFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDzANBgNVBAoMBkRv
+Y2tlcjELMAkGA1UECAwCQ0EwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoIC
+AQCwVVD4pK7z7pXPpJbaZ1Hg5eRXIcaYtbFPCnN0iqy9HsVEGnEn5BPNSEsuP+m0
+5N0qVV7DGb1SjiloLXD1qDDvhXWk+giS9ppqPHPLVPB4bvzsqwDYrtpbqkYvO0YK
+0SL3kxPXUFdlkFfgu0xjlczm2PhWG3Jd8aAtspL/L+VfPA13JUaWxSLpui1In8rh
+gAyQTK6Q4Of6GbJYTnAHb59UoLXSzB5AfqiUq6L7nEYYKoPflPbRAIWL/UBm0c+H
+ocms706PYpmPS2RQv3iOGmnn9hEVp3P6jq7WAevbA4aYGx5EsbVtYABqJBbFWAuw
+wTGRYmzn0Mj0eTMge9ztYB2/2sxdTe6uhmFgpUXngDqJI5O9N3zPfvlEImCky3HM
+jJoL7g5smqX9o1P+ESLh0VZzhh7IDPzQTXpcPIS/6z0l22QGkK/1N1PaADaUHdLL
+vSav3y2BaEmPvf2fkZj8yP5eYgi7Cw5ONhHLDYHFcl9Zm/ywmdxHJETz9nfgXnsW
+HNxDqrkCVO46r/u6rSrUt6hr3oddJG8s8Jo06earw6XU3MzM+3giwkK0SSM3uRPq
+4AscR1Tv+E31AuOAmjqYQoT29bMIxoSzeljj/YnedwjW45pWyc3JoHaibDwvW9Uo
+GSZBVy4hrM/Fa7XCWv1WfHNW1gDwaLYwDnl5jFmRBvcfuQIDAQABo4H5MIH2MIGR
+BgNVHSMEgYkwgYaAFHUM1U3E4WyL1nvFd+dPY8f4O2hZoWOkYTBfMQswCQYDVQQG
+EwJVUzELMAkGA1UECAwCQ0ExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDzANBgNV
+BAoMBkRvY2tlcjEaMBgGA1UEAwwRTm90YXJ5IFRlc3RpbmcgQ0GCCQDCeDLbemIT
+SzASBgNVHRMBAf8ECDAGAQH/AgEAMB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEF
+BQcDATAOBgNVHQ8BAf8EBAMCAUYwHQYDVR0OBBYEFHe48hcBcAp0bUVlTxXeRA4o
+E16pMA0GCSqGSIb3DQEBCwUAA4ICAQAWUtAPdUFpwRq+N1SzGUejSikeMGyPZscZ
+JBUCmhZoFufgXGbLO5OpcRLaV3Xda0t/5PtdGMSEzczeoZHWknDtw+79OBittPPj
+Sh1oFDuPo35R7eP624lUCch/InZCphTaLx9oDLGcaK3ailQ9wjBdKdlBl8KNKIZp
+a13aP5rnSm2Jva+tXy/yi3BSds3dGD8ITKZyI/6AFHxGvObrDIBpo4FF/zcWXVDj
+paOmxplRtM4Hitm+sXGvfqJe4x5DuOXOnPrT3dHvRT6vSZUoKobxMqmRTOcrOIPa
+EeMpOobshORuRntMDYvvgO3D6p6iciDW2Vp9N6rdMdfOWEQN8JVWvB7IxRHk9qKJ
+vYOWVbczAt0qpMvXF3PXLjZbUM0knOdUKIEbqP4YUbgdzx6RtgiiY930Aj6tAtce
+0fpgNlvjMRpSBuWTlAfNNjG/YhndMz9uI68TMfFpR3PcgVIv30krw/9VzoLi2Dpe
+ow6DrGO6oi+DhN78P4jY/O9UczZK2roZL1Oi5P0RIxf23UZC7x1DlcN3nBr4sYSv
+rBx4cFTMNpwU+nzsIi4djcFDKmJdEOyjMnkP2v0Lwe7yvK08pZdEu+0zbrq17kue
+XpXLc7K68QB15yxzGylU5rRwzmC/YsAVyE4eoGu8PxWxrERvHby4B8YP0vAfOraL
+lKmXlK4dTg==
+-----END CERTIFICATE-----
+

trustpinning/certs.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"strings"
-	"time"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/notary/tuf/data"
@@ -98,6 +97,7 @@ func ValidateRoot(prevRoot *data.SignedRoot, root *data.Signed, gun string, trus
 	// Retrieve all the leaf and intermediate certificates in root for which the CN matches the GUN
 	allLeafCerts, allIntCerts := parseAllCerts(signedRoot)
 	certsFromRoot, err := validRootLeafCerts(allLeafCerts, gun, true)
+	validIntCerts := validRootIntCerts(allIntCerts)
 
 	if err != nil {
 		logrus.Debugf("error retrieving valid leaf certificates for: %s, %v", gun, err)
@@ -140,7 +140,7 @@ func ValidateRoot(prevRoot *data.SignedRoot, root *data.Signed, gun string, trus
 		validPinnedCerts := map[string]*x509.Certificate{}
 		for id, cert := range certsFromRoot {
 			logrus.Debugf("checking trust-pinning for cert: %s", id)
-			if ok := trustPinCheckFunc(cert, allIntCerts[id]); !ok {
+			if ok := trustPinCheckFunc(cert, validIntCerts[id]); !ok {
 				logrus.Debugf("trust-pinning check failed for cert: %s", id)
 				continue
 			}
@@ -156,7 +156,7 @@ func ValidateRoot(prevRoot *data.SignedRoot, root *data.Signed, gun string, trus
 	// Note that certsFromRoot is guaranteed to be unchanged only if we had prior cert data for this GUN or enabled TOFUS
 	// If we attempted to pin a certain certificate or CA, certsFromRoot could have been pruned accordingly
 	err = signed.VerifySignatures(root, data.BaseRole{
-		Keys: utils.CertsToKeys(certsFromRoot, allIntCerts), Threshold: rootRole.Threshold})
+		Keys: utils.CertsToKeys(certsFromRoot, validIntCerts), Threshold: rootRole.Threshold})
 	if err != nil {
 		logrus.Debugf("failed to verify TUF data for: %s, %v", gun, err)
 		return nil, &ErrValidationFail{Reason: "failed to validate integrity of roots"}
@@ -181,17 +181,8 @@ func validRootLeafCerts(allLeafCerts map[string]*x509.Certificate, gun string, c
 			continue
 		}
 		// Make sure the certificate is not expired if checkExpiry is true
-		if checkExpiry && time.Now().After(cert.NotAfter) {
-			logrus.Debugf("error leaf certificate is expired")
-			continue
-		}
-
-		// We don't allow root certificates that use SHA1
-		if cert.SignatureAlgorithm == x509.SHA1WithRSA ||
-			cert.SignatureAlgorithm == x509.DSAWithSHA1 ||
-			cert.SignatureAlgorithm == x509.ECDSAWithSHA1 {
-
-			logrus.Debugf("error certificate uses deprecated hashing algorithm (SHA1)")
+		// and warn if it hasn't expired yet but is within 6 months of expiry
+		if err := utils.ValidateCertificate(cert, checkExpiry); err != nil {
 			continue
 		}
 
@@ -208,6 +199,24 @@ func validRootLeafCerts(allLeafCerts map[string]*x509.Certificate, gun string, c
 	return validLeafCerts, nil
 }
 
+// validRootIntCerts filters the passed in structure of intermediate certificates to only include non-expired, non-sha1 certificates
+// Note that this "validity" alone does not imply any measure of trust.
+func validRootIntCerts(allIntCerts map[string][]*x509.Certificate) map[string][]*x509.Certificate {
+	validIntCerts := make(map[string][]*x509.Certificate)
+
+	// Go through every leaf cert ID, and build its valid intermediate certificate list
+	for leafID, intCertList := range allIntCerts {
+		for _, intCert := range intCertList {
+			if err := utils.ValidateCertificate(intCert, true); err != nil {
+				continue
+			}
+			validIntCerts[leafID] = append(validIntCerts[leafID], intCert)
+		}
+
+	}
+	return validIntCerts
+}
+
 // parseAllCerts returns two maps, one with all of the leafCertificates and one
 // with all the intermediate certificates found in signedRoot
 func parseAllCerts(signedRoot *data.SignedRoot) (map[string]*x509.Certificate, map[string][]*x509.Certificate) {

trustpinning/certs_test.go

@@ -9,15 +9,17 @@ import (
 	"crypto/x509/pkix"
 	"encoding/json"
 	"encoding/pem"
+	"fmt"
 	"io/ioutil"
 	"math/big"
 	"os"
 	"path/filepath"
 	"testing"
 	"text/template"
-
 	"time"
 
+	"github.com/Sirupsen/logrus"
+	"github.com/docker/notary"
 	"github.com/docker/notary/cryptoservice"
 	"github.com/docker/notary/trustmanager"
 	"github.com/docker/notary/trustpinning"
@@ -786,9 +788,9 @@ func testValidateRootRotationMissingNewSig(t *testing.T, keyAlg, rootKeyType str
 	require.Error(t, err, "insuficient signatures on root")
 }
 
-func generateTestingCertificate(rootKey data.PrivateKey, gun string) (*x509.Certificate, error) {
+func generateTestingCertificate(rootKey data.PrivateKey, gun string, timeToExpire time.Duration) (*x509.Certificate, error) {
 	startTime := time.Now()
-	return cryptoservice.GenerateCertificate(rootKey, gun, startTime, startTime.AddDate(10, 0, 0))
+	return cryptoservice.GenerateCertificate(rootKey, gun, startTime, startTime.Add(timeToExpire))
 }
 
 func generateExpiredTestingCertificate(rootKey data.PrivateKey, gun string) (*x509.Certificate, error) {
@@ -804,3 +806,244 @@ func generateRootKeyIDs(r *data.SignedRoot) {
 		}
 	}
 }
+
+func TestCheckingCertExpiry(t *testing.T) {
+	gun := "notary"
+	pass := func(keyName, alias string, createNew bool, attempts int) (passphrase string, giveup bool, err error) {
+		return "password", false, nil
+	}
+	memStore := trustmanager.NewKeyMemoryStore(pass)
+	cs := cryptoservice.NewCryptoService(memStore)
+	testPubKey, err := cs.Create(data.CanonicalRootRole, gun, data.ECDSAKey)
+	require.NoError(t, err)
+	testPrivKey, _, err := memStore.GetKey(testPubKey.ID())
+	require.NoError(t, err)
+
+	almostExpiredCert, err := generateTestingCertificate(testPrivKey, gun, notary.Day*30)
+	require.NoError(t, err)
+	almostExpiredPubKey, err := utils.ParsePEMPublicKey(utils.CertToPEM(almostExpiredCert))
+	require.NoError(t, err)
+
+	// set up a logrus logger to capture warning output
+	origLevel := logrus.GetLevel()
+	logrus.SetLevel(logrus.WarnLevel)
+	defer logrus.SetLevel(origLevel)
+	logBuf := bytes.NewBuffer(nil)
+	logrus.SetOutput(logBuf)
+
+	rootRole, err := data.NewRole(data.CanonicalRootRole, 1, []string{almostExpiredPubKey.ID()}, nil)
+	require.NoError(t, err)
+	testRoot, err := data.NewRoot(
+		map[string]data.PublicKey{almostExpiredPubKey.ID(): almostExpiredPubKey},
+		map[string]*data.RootRole{
+			data.CanonicalRootRole:      &rootRole.RootRole,
+			data.CanonicalTimestampRole: &rootRole.RootRole,
+			data.CanonicalTargetsRole:   &rootRole.RootRole,
+			data.CanonicalSnapshotRole:  &rootRole.RootRole},
+		false,
+	)
+	testRoot.Signed.Version = 1
+	require.NoError(t, err, "Failed to create new root")
+
+	signedTestRoot, err := testRoot.ToSigned()
+	require.NoError(t, err)
+
+	err = signed.Sign(cs, signedTestRoot, []data.PublicKey{almostExpiredPubKey}, 1, nil)
+	require.NoError(t, err)
+
+	// This is a valid root certificate, but check that we get a Warn-level message that the certificate is near expiry
+	_, err = trustpinning.ValidateRoot(nil, signedTestRoot, gun, trustpinning.TrustPinConfig{})
+	require.NoError(t, err)
+	require.Contains(t, logBuf.String(), fmt.Sprintf("certificate with CN %s is near expiry", gun))
+
+	expiredCert, err := generateExpiredTestingCertificate(testPrivKey, gun)
+	require.NoError(t, err)
+	expiredPubKey := utils.CertToKey(expiredCert)
+
+	rootRole, err = data.NewRole(data.CanonicalRootRole, 1, []string{expiredPubKey.ID()}, nil)
+	require.NoError(t, err)
+	testRoot, err = data.NewRoot(
+		map[string]data.PublicKey{expiredPubKey.ID(): expiredPubKey},
+		map[string]*data.RootRole{
+			data.CanonicalRootRole:      &rootRole.RootRole,
+			data.CanonicalTimestampRole: &rootRole.RootRole,
+			data.CanonicalTargetsRole:   &rootRole.RootRole,
+			data.CanonicalSnapshotRole:  &rootRole.RootRole},
+		false,
+	)
+	testRoot.Signed.Version = 1
+	require.NoError(t, err, "Failed to create new root")
+
+	signedTestRoot, err = testRoot.ToSigned()
+	require.NoError(t, err)
+
+	err = signed.Sign(cs, signedTestRoot, []data.PublicKey{expiredPubKey}, 1, nil)
+	require.NoError(t, err)
+
+	// This is an invalid root certificate since it's expired
+	_, err = trustpinning.ValidateRoot(nil, signedTestRoot, gun, trustpinning.TrustPinConfig{})
+	require.Error(t, err)
+}
+
+func TestValidateRootWithExpiredIntermediate(t *testing.T) {
+	now := time.Now()
+	serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128)
+
+	pass := func(keyName, alias string, createNew bool, attempts int) (passphrase string, giveup bool, err error) {
+		return "password", false, nil
+	}
+	memStore := trustmanager.NewKeyMemoryStore(pass)
+	cs := cryptoservice.NewCryptoService(memStore)
+
+	// generate CA cert
+	serialNumber, err := rand.Int(rand.Reader, serialNumberLimit)
+	require.NoError(t, err)
+	caTmpl := x509.Certificate{
+		SerialNumber: serialNumber,
+		Subject: pkix.Name{
+			CommonName: "notary testing CA",
+		},
+		NotBefore:             now.Add(-time.Hour),
+		NotAfter:              now.Add(time.Hour),
+		KeyUsage:              x509.KeyUsageCertSign,
+		BasicConstraintsValid: true,
+		IsCA:       true,
+		MaxPathLen: 3,
+	}
+	caPrivKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err)
+	_, err = x509.CreateCertificate(
+		rand.Reader,
+		&caTmpl,
+		&caTmpl,
+		caPrivKey.Public(),
+		caPrivKey,
+	)
+
+	// generate expired intermediate
+	intTmpl := x509.Certificate{
+		SerialNumber: serialNumber,
+		Subject: pkix.Name{
+			CommonName: "EXPIRED notary testing intermediate",
+		},
+		NotBefore:             now.Add(-2 * notary.Year),
+		NotAfter:              now.Add(-notary.Year),
+		KeyUsage:              x509.KeyUsageCertSign,
+		BasicConstraintsValid: true,
+		IsCA:       true,
+		MaxPathLen: 2,
+	}
+	intPrivKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err)
+	intCert, err := x509.CreateCertificate(
+		rand.Reader,
+		&intTmpl,
+		&caTmpl,
+		intPrivKey.Public(),
+		caPrivKey,
+	)
+	require.NoError(t, err)
+
+	// generate leaf
+	serialNumber, err = rand.Int(rand.Reader, serialNumberLimit)
+	require.NoError(t, err)
+	leafTmpl := x509.Certificate{
+		SerialNumber: serialNumber,
+		Subject: pkix.Name{
+			CommonName: "docker.io/notary/test",
+		},
+		NotBefore: now.Add(-time.Hour),
+		NotAfter:  now.Add(time.Hour),
+
+		KeyUsage:              x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
+		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageCodeSigning},
+		BasicConstraintsValid: true,
+	}
+
+	leafPubKey, err := cs.Create("root", "docker.io/notary/test", data.ECDSAKey)
+	require.NoError(t, err)
+	leafPrivKey, _, err := cs.GetPrivateKey(leafPubKey.ID())
+	require.NoError(t, err)
+	signer := leafPrivKey.CryptoSigner()
+	leafCert, err := x509.CreateCertificate(
+		rand.Reader,
+		&leafTmpl,
+		&intTmpl,
+		signer.Public(),
+		intPrivKey,
+	)
+
+	rootBundleWriter := bytes.NewBuffer(nil)
+	pem.Encode(
+		rootBundleWriter,
+		&pem.Block{
+			Type:  "CERTIFICATE",
+			Bytes: leafCert,
+		},
+	)
+	pem.Encode(
+		rootBundleWriter,
+		&pem.Block{
+			Type:  "CERTIFICATE",
+			Bytes: intCert,
+		},
+	)
+
+	rootBundle := rootBundleWriter.Bytes()
+
+	ecdsax509Key := data.NewECDSAx509PublicKey(rootBundle)
+
+	otherKey, err := cs.Create("targets", "docker.io/notary/test", data.ED25519Key)
+	require.NoError(t, err)
+
+	root := data.SignedRoot{
+		Signatures: make([]data.Signature, 0),
+		Signed: data.Root{
+			SignedCommon: data.SignedCommon{
+				Type:    "Root",
+				Expires: now.Add(time.Hour),
+				Version: 1,
+			},
+			Keys: map[string]data.PublicKey{
+				ecdsax509Key.ID(): ecdsax509Key,
+				otherKey.ID():     otherKey,
+			},
+			Roles: map[string]*data.RootRole{
+				"root": {
+					KeyIDs:    []string{ecdsax509Key.ID()},
+					Threshold: 1,
+				},
+				"targets": {
+					KeyIDs:    []string{otherKey.ID()},
+					Threshold: 1,
+				},
+				"snapshot": {
+					KeyIDs:    []string{otherKey.ID()},
+					Threshold: 1,
+				},
+				"timestamp": {
+					KeyIDs:    []string{otherKey.ID()},
+					Threshold: 1,
+				},
+			},
+		},
+		Dirty: true,
+	}
+
+	signedRoot, err := root.ToSigned()
+	require.NoError(t, err)
+	err = signed.Sign(cs, signedRoot, []data.PublicKey{ecdsax509Key}, 1, nil)
+	require.NoError(t, err)
+
+	tempBaseDir, err := ioutil.TempDir("", "notary-test-")
+	defer os.RemoveAll(tempBaseDir)
+	require.NoError(t, err, "failed to create a temporary directory: %s", err)
+
+	_, err = trustpinning.ValidateRoot(
+		nil,
+		signedRoot,
+		"docker.io/notary/test",
+		trustpinning.TrustPinConfig{},
+	)
+	require.Error(t, err, "failed to invalidate expired intermediate certificate")
+}