Merge pull request #509 from docker/reject-short-keys

Add check for RSA key len before adding delegation

cmd/notary/delegations_test.go

@@ -68,6 +68,27 @@ func TestAddInvalidDelegationCert(t *testing.T) {
 	assert.Error(t, err)
 }
 
+func TestAddInvalidShortPubkeyCert(t *testing.T) {
+	// Cleanup after test
+	defer os.RemoveAll(testTrustDir)
+
+	// Setup certificate
+	tempFile, err := ioutil.TempFile("/tmp", "pemfile")
+	assert.NoError(t, err)
+	cert, _, err := generateShortRSAKeyTestCert()
+	_, err = tempFile.Write(trustmanager.CertToPEM(cert))
+	assert.NoError(t, err)
+	tempFile.Close()
+	defer os.Remove(tempFile.Name())
+
+	// Setup commander
+	commander := setup()
+
+	// Should error due to short RSA key
+	err = commander.delegationAdd(commander.GetCommand(), []string{"gun", "targets/delegation", tempFile.Name(), "--paths", "path"})
+	assert.Error(t, err)
+}
+
 func TestRemoveInvalidDelegationName(t *testing.T) {
 	// Cleanup after test
 	defer os.RemoveAll(testTrustDir)
@@ -149,3 +170,19 @@ func generateExpiredTestCert() (*x509.Certificate, string, error) {
 	}
 	return cert, keyID, nil
 }
+
+func generateShortRSAKeyTestCert() (*x509.Certificate, string, error) {
+	// 1024 bits is too short
+	privKey, err := trustmanager.GenerateRSAKey(rand.Reader, 1024)
+	if err != nil {
+		return nil, "", err
+	}
+	keyID := privKey.ID()
+	startTime := time.Now()
+	endTime := startTime.AddDate(10, 0, 0)
+	cert, err := cryptoservice.GenerateCertificate(privKey, "gun", startTime, endTime)
+	if err != nil {
+		return nil, "", err
+	}
+	return cert, keyID, nil
+}

const.go

@@ -2,6 +2,8 @@ package notary
 
 // application wide constants
 const (
+	// MinRSABitSize is the minimum bit size for RSA keys allowed in notary
+	MinRSABitSize = 2048
 	// MinThreshold requires a minimum of one threshold for roles; currently we do not support a higher threshold
 	MinThreshold = 1
 	// PrivKeyPerms are the file permissions to use when writing private keys to disk

trustmanager/x509utils.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/Sirupsen/logrus"
 	"github.com/agl/ed25519"
+	"github.com/docker/notary"
 	"github.com/docker/notary/tuf/data"
 )
 
@@ -324,7 +325,7 @@ func ParsePEMPublicKey(pubKeyBytes []byte) (data.PublicKey, error) {
 }
 
 // ValidateCertificate returns an error if the certificate is not valid for notary
-// Currently, this is only a time expiry check
+// Currently this is only a time expiry check, and ensuring the public key has a large enough modulus if RSA
 func ValidateCertificate(c *x509.Certificate) error {
 	if (c.NotBefore).After(c.NotAfter) {
 		return fmt.Errorf("certificate validity window is invalid")
@@ -335,6 +336,16 @@ func ValidateCertificate(c *x509.Certificate) error {
 	if (tomorrow).Before(c.NotBefore) || now.After(c.NotAfter) {
 		return fmt.Errorf("certificate is expired")
 	}
+	// If we have an RSA key, make sure it's long enough
+	if c.PublicKeyAlgorithm == x509.RSA {
+		rsaKey, ok := c.PublicKey.(*rsa.PublicKey)
+		if !ok {
+			return fmt.Errorf("unable to parse RSA public key")
+		}
+		if rsaKey.N.BitLen() < notary.MinRSABitSize {
+			return fmt.Errorf("RSA bit length is too short")
+		}
+	}
 	return nil
 }