diff --git a/cmd/notary/delegations_test.go b/cmd/notary/delegations_test.go index 02e7c4607e..89e6ccdc29 100644 --- a/cmd/notary/delegations_test.go +++ b/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 +} diff --git a/const.go b/const.go index 6eb278cf98..a1140c0dc0 100644 --- a/const.go +++ b/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 diff --git a/trustmanager/x509utils.go b/trustmanager/x509utils.go index 64ca1e7226..f39ca8eb22 100644 --- a/trustmanager/x509utils.go +++ b/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 }