Skip to content

Commit

Permalink
Drop support for RSA key generation
Browse files Browse the repository at this point in the history
Signed-off-by: Umayr Shahid <umayr.shahid@gmail.com>
  • Loading branch information
umayr committed Jul 13, 2017
1 parent 87d6d6c commit 0501284
Show file tree
Hide file tree
Showing 13 changed files with 414 additions and 53 deletions.
15 changes: 9 additions & 6 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/docker/notary/trustpinning"
"github.com/docker/notary/tuf/data"
"github.com/docker/notary/tuf/signed"
testutils "github.com/docker/notary/tuf/testutils/keys"
"github.com/docker/notary/tuf/utils"
"github.com/docker/notary/tuf/validation"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -194,7 +195,7 @@ func createRepoAndKey(t *testing.T, rootType, tempBaseDir, gun, url string) (*No
tempBaseDir, data.GUN(gun), url, http.DefaultTransport, rec.retriever, trustpinning.TrustPinConfig{})
require.NoError(t, err, "error creating repo: %s", err)

rootPubKey, err := repo.CryptoService.Create(data.CanonicalRootRole, repo.gun, rootType)
rootPubKey, err := testutils.CreateOrAddKey(repo.CryptoService, data.CanonicalRootRole, repo.gun, rootType)
require.NoError(t, err, "error generating root key: %s", err)

rec.requireCreated(t, []string{data.CanonicalRootRole.String()},
Expand Down Expand Up @@ -699,7 +700,8 @@ func testInitRepoAttemptsExceeded(t *testing.T, rootType string) {
retriever := passphrase.ConstantRetriever("password")
repo, err := NewFileCachedNotaryRepository(tempBaseDir, gun, ts.URL, http.DefaultTransport, retriever, trustpinning.TrustPinConfig{})
require.NoError(t, err, "error creating repo: %s", err)
rootPubKey, err := repo.CryptoService.Create(data.CanonicalRootRole, repo.gun, rootType)

rootPubKey, err := testutils.CreateOrAddKey(repo.CryptoService, data.CanonicalRootRole, repo.gun, rootType)
require.NoError(t, err, "error generating root key: %s", err)

retriever = passphrase.ConstantRetriever("incorrect password")
Expand Down Expand Up @@ -737,7 +739,8 @@ func testInitRepoPasswordInvalid(t *testing.T, rootType string) {
retriever := passphrase.ConstantRetriever("password")
repo, err := NewFileCachedNotaryRepository(tempBaseDir, gun, ts.URL, http.DefaultTransport, retriever, trustpinning.TrustPinConfig{})
require.NoError(t, err, "error creating repo: %s", err)
rootPubKey, err := repo.CryptoService.Create(data.CanonicalRootRole, repo.gun, rootType)

rootPubKey, err := testutils.CreateOrAddKey(repo.CryptoService, data.CanonicalRootRole, repo.gun, rootType)
require.NoError(t, err, "error generating root key: %s", err)

// repo.CryptoService’s FileKeyStore caches the unlocked private key, so to test
Expand Down Expand Up @@ -1387,7 +1390,7 @@ func testListTargetWithDelegates(t *testing.T, rootType string) {
currentTarget := addTarget(t, repo, "current", "../fixtures/intermediate-ca.crt")

// setup delegated targets/level1 role
k, err := repo.CryptoService.Create("targets/level1", repo.gun, rootType)
k, err := testutils.CreateOrAddKey(repo.CryptoService, "targets/level1", repo.gun, rootType)
require.NoError(t, err)
err = repo.tufRepo.UpdateDelegationKeys("targets/level1", []data.PublicKey{k}, []string{}, 1)
require.NoError(t, err)
Expand All @@ -1397,7 +1400,7 @@ func testListTargetWithDelegates(t *testing.T, rootType string) {
otherTarget := addTarget(t, repo, "other", "../fixtures/root-ca.crt", "targets/level1")

// setup delegated targets/level2 role
k, err = repo.CryptoService.Create("targets/level2", repo.gun, rootType)
k, err = testutils.CreateOrAddKey(repo.CryptoService, "targets/level2", repo.gun, rootType)
require.NoError(t, err)
err = repo.tufRepo.UpdateDelegationKeys("targets/level2", []data.PublicKey{k}, []string{}, 1)
require.NoError(t, err)
Expand Down Expand Up @@ -1429,7 +1432,7 @@ func testListTargetWithDelegates(t *testing.T, rootType string) {

// setup delegated targets/level1/level2 role separately, which can only modify paths prefixed with "level2"
// This is done separately due to target shadowing
k, err = repo.CryptoService.Create("targets/level1/level2", repo.gun, rootType)
k, err = testutils.CreateOrAddKey(repo.CryptoService, "targets/level1/level2", repo.gun, rootType)
require.NoError(t, err)
err = repo.tufRepo.UpdateDelegationKeys("targets/level1/level2", []data.PublicKey{k}, []string{}, 1)
require.NoError(t, err)
Expand Down
3 changes: 2 additions & 1 deletion cmd/notary/delegations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"github.com/docker/notary/cryptoservice"
testutils "github.com/docker/notary/tuf/testutils/keys"
"github.com/docker/notary/tuf/utils"
"github.com/spf13/viper"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -207,7 +208,7 @@ func generateExpiredTestCert() (*x509.Certificate, string, error) {

func generateShortRSAKeyTestCert() (*x509.Certificate, string, error) {
// 1024 bits is too short
privKey, err := utils.GenerateRSAKey(rand.Reader, 1024)
privKey, err := testutils.GetRSAKey(1024)
if err != nil {
return nil, "", err
}
Expand Down
7 changes: 4 additions & 3 deletions cmd/notary/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
nstorage "github.com/docker/notary/storage"
"github.com/docker/notary/trustmanager"
"github.com/docker/notary/tuf/data"
testutils "github.com/docker/notary/tuf/testutils/keys"
"github.com/docker/notary/tuf/utils"
"github.com/spf13/cobra"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -2073,7 +2074,7 @@ func generateCertPrivKeyPair(t *testing.T, gun, keyAlgorithm string) (*x509.Cert
case data.ECDSAKey:
privKey, err = utils.GenerateECDSAKey(rand.Reader)
case data.RSAKey:
privKey, err = utils.GenerateRSAKey(rand.Reader, 4096)
privKey, err = testutils.GetRSAKey(4096)
default:
err = fmt.Errorf("invalid key algorithm provided: %s", keyAlgorithm)
}
Expand Down Expand Up @@ -2741,7 +2742,7 @@ func TestAddDelImportKeyPublishFlow(t *testing.T) {
tempFile, err := ioutil.TempFile("", "pemfile")
require.NoError(t, err)

privKey, err := utils.GenerateRSAKey(rand.Reader, 2048)
privKey, err := testutils.GetRSAKey(2048)
require.NoError(t, err)
startTime := time.Now()
endTime := startTime.AddDate(10, 0, 0)
Expand Down Expand Up @@ -3003,7 +3004,7 @@ func TestDelegationKeyImportExport(t *testing.T) {
keyFile, err := ioutil.TempFile("", "pemfile")
require.NoError(t, err)
defer os.Remove(keyFile.Name())
privKey, err := utils.GenerateRSAKey(rand.Reader, 2048)
privKey, err := testutils.GetRSAKey(2048)
require.NoError(t, err)
pemBytes, err := utils.EncryptPrivateKey(privKey, "", "", "")
require.NoError(t, err)
Expand Down
3 changes: 1 addition & 2 deletions cmd/notary/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,10 @@ func (k *keyCommander) keysGenerate(cmd *cobra.Command, args []string) error {

allowedCiphers := map[string]bool{
data.ECDSAKey: true,
data.RSAKey: true,
}

if !allowedCiphers[strings.ToLower(algorithm)] {
return fmt.Errorf("Algorithm not allowed, possible values are: RSA, ECDSA")
return fmt.Errorf("Algorithm not allowed, possible values are: ECDSA")
}

config, err := k.configGetter()
Expand Down
4 changes: 2 additions & 2 deletions cmd/notary/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ var exampleValidCommands = []string{
"verify repo v1",
"key list",
"key rotate repo snapshot",
"key generate rsa",
"key generate ecdsa",
"key remove e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
"key passwd e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
"key import backup.pem",
Expand Down Expand Up @@ -205,7 +205,7 @@ func TestInsufficientArgumentsReturnsErrorAndPrintsUsage(t *testing.T) {
cmd.SetOutput(b)

arglist := strings.Fields(args)
if args == "key list" || args == "key generate rsa" {
if args == "key list" || args == "key generate ecdsa" {
// in these case, "key" or "key generate" are valid commands, so add an arg to them instead
arglist = append(arglist, "extraArg")
} else {
Expand Down
17 changes: 11 additions & 6 deletions cryptoservice/crypto_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/docker/notary/tuf/data"
"github.com/docker/notary/tuf/signed"
"github.com/docker/notary/tuf/testutils/interfaces"
testutils "github.com/docker/notary/tuf/testutils/keys"
"github.com/docker/notary/tuf/utils"
)

Expand Down Expand Up @@ -104,7 +105,7 @@ func (c CryptoServiceTester) TestSignWithKey(t *testing.T) {
cryptoService := c.cryptoServiceFactory()
content := []byte("this is a secret")

tufKey, err := cryptoService.Create(c.role, c.gun, c.keyAlgo)
tufKey, err := testutils.CreateOrAddKey(cryptoService, c.role, c.gun, c.keyAlgo)
require.NoError(t, err, c.errorMsg("error creating key"))

// Test Sign
Expand Down Expand Up @@ -170,7 +171,7 @@ func (c CryptoServiceTester) TestGetPrivateKeyPasswordInvalid(t *testing.T) {
store, err := trustmanager.NewKeyFileStore(tempBaseDir, retriever)
require.NoError(t, err)
cryptoService := NewCryptoService(store)
pubKey, err := cryptoService.Create(c.role, c.gun, c.keyAlgo)
pubKey, err := testutils.CreateOrAddKey(cryptoService, c.role, c.gun, c.keyAlgo)
require.NoError(t, err, "error generating key: %s", err)

// cryptoService's FileKeyStore caches the unlocked private key, so to test
Expand All @@ -194,7 +195,7 @@ func (c CryptoServiceTester) TestGetPrivateKeyAttemptsExceeded(t *testing.T) {
store, err := trustmanager.NewKeyFileStore(tempBaseDir, retriever)
require.NoError(t, err)
cryptoService := NewCryptoService(store)
pubKey, err := cryptoService.Create(c.role, c.gun, c.keyAlgo)
pubKey, err := testutils.CreateOrAddKey(cryptoService, c.role, c.gun, c.keyAlgo)
require.NoError(t, err, "error generating key: %s", err)

// trustmanager.KeyFileStore and trustmanager.KeyMemoryStore both cache the unlocked
Expand All @@ -214,7 +215,7 @@ func (c CryptoServiceTester) TestGetPrivateKeyAttemptsExceeded(t *testing.T) {
func (c CryptoServiceTester) TestRemoveCreatedKey(t *testing.T) {
cryptoService := c.cryptoServiceFactory()

tufKey, err := cryptoService.Create(c.role, c.gun, c.keyAlgo)
tufKey, err := testutils.CreateOrAddKey(cryptoService, c.role, c.gun, c.keyAlgo)
require.NoError(t, err, c.errorMsg("error creating key"))
require.NotNil(t, cryptoService.GetKey(tufKey.ID()))

Expand Down Expand Up @@ -375,9 +376,13 @@ func testCryptoService(t *testing.T, gun data.GUN) {
keyAlgo: algo,
gun: gun,
}
// cryptoservice can't generate RSA keys, so avoiding tests that include
// directly creating keys in case of RSA algorithm
if algo != data.RSAKey {
cst.TestCreateAndGetKey(t)
cst.TestCreateAndGetWhenMultipleKeystores(t)
}
cst.TestAddKey(t)
cst.TestCreateAndGetKey(t)
cst.TestCreateAndGetWhenMultipleKeystores(t)
cst.TestGetNonexistentKey(t)
cst.TestSignWithKey(t)
cst.TestSignNoMatchingKeys(t)
Expand Down
16 changes: 9 additions & 7 deletions trustpinning/certs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1105,13 +1105,15 @@ func TestParsePEMPublicKey(t *testing.T) {
require.NoError(t, err)

// can parse RSA PEM
rsaPubKey, err := cs.Create("root", "docker.io/notary/test2", data.RSAKey)
require.NoError(t, err)
rsaPemBytes := pem.EncodeToMemory(&pem.Block{
Type: "PUBLIC KEY",
Headers: nil,
Bytes: rsaPubKey.Public(),
})
rsaPemBytes := []byte(`-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA7HQxZ0fDsxPTFIABQXNX
i9b25AZWtBoR+k8myrrI0cb08ISoB2NBpYwDbxhxLvjN1OpjFzCOjbmK+sD2zCkt
Rxg1Z9NimY4J/p9uWF2EcRklmCqdHJ2KW7QD3j5uy7e7KsSyLPcsMtIrRYVtk2Z8
oGKEOQUsTudXoH0W9lVtBNgQi0S3FiuesRXKc0jDsZRXxtQUB0MzzRJ8zjgZbuKw
6XBlfidMEo3E10jQk8lrV1iio0xpkYuW+sbfefgNDyGBoSpsSG9Kh0sDHCyRteCm
zKJV1ck/b6x3x7eLNtsAErkJfp6aNKcvGrXMUgB/pZTaC4lpfxKq4s3+zY6sgabr
jwIDAQAB
-----END PUBLIC KEY-----`)
_, err = utils.ParsePEMPublicKey(rsaPemBytes)
require.NoError(t, err)

Expand Down
23 changes: 21 additions & 2 deletions tuf/signed/sign_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package signed
import (
"crypto"
"crypto/rand"
"crypto/x509"
"encoding/pem"
"io"
"testing"
Expand Down Expand Up @@ -208,8 +209,26 @@ func TestSignReturnsNoSigs(t *testing.T) {
}

func TestSignWithX509(t *testing.T) {
// generate a key because we need a cert
privKey, err := utils.GenerateRSAKey(rand.Reader, 1024)
// parse a RSA key because we need a cert
block, _ := pem.Decode([]byte(`-----BEGIN RSA PRIVATE KEY-----
MIICXAIBAAKBgQDJ8BO2/HOHLJgrb3srafbNRUD8r0SGNJFi5h7t4vxZ4F5oBW/4
O2/aZmdToinyuCm0eGguK77HAsTfSHqDUoEfuInNg7pPk4F6xa4feQzEeG6P0YaL
+VbApUdCHLBE0tVZg1SCW97+27wqIM4Cl1Tcsbb+aXfgMaOFGxlyga+a6wIDAQAB
AoGBAKDWLH2kGMfjBtghlLKBVWcs75PSbPuPRvTEYIIMNf3HrKmhGwtVG8ORqF5+
XHbLo7vv4tpTUUHkvLUyXxHVVq1oX+QqiRwTRm+ROF0/T6LlrWvTzvowTKtkRbsm
mqIYEbc+fBZ/7gEeW2ycCfE7HWgxNGvbUsK4LNa1ozJbrVEBAkEA8ML0mXyxq+cX
CwWvdXscN9vopLG/y+LKsvlKckoI/Hc0HjPyraq5Docwl2trZEmkvct1EcN8VvcV
vCtVsrAfwQJBANa4EBPfcIH2NKYHxt9cP00n74dVSHpwJYjBnbec5RCzn5UTbqd2
i62AkQREYhHZAryvBVE81JAFW3nqI9ZTpasCQBqEPlBRTXgzYXRTUfnMb1UvoTXS
Zd9cwRppHmvr/4Ve05yn+AhsjyksdouWxyMqgTxuFhy4vQ8O85Pf6fZeM4ECQCPp
Wv8H4thJplqSeGeJFSlBYaVf1SRtN0ndIBTCj+kwMaOMQXiOsiPNmfN9wG09v2Bx
YVFJ/D8uNjN4vo+tI8sCQFbtF+Qkj4uSFDZGMESF6MOgsGt1R1iCpvpMSr9h9V02
LPXyS3ozB7Deq26pEiCrFtHxw2Pb7RJO6GEqH7Dg4oU=
-----END RSA PRIVATE KEY-----`))
key, err := x509.ParsePKCS1PrivateKey(block.Bytes)
require.NoError(t, err)

privKey, err := utils.RSAToPrivateKey(key)
require.NoError(t, err)

// make a RSA x509 key
Expand Down
5 changes: 3 additions & 2 deletions tuf/testutils/interfaces/cryptoservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/docker/notary/tuf/data"
"github.com/docker/notary/tuf/signed"
testutils "github.com/docker/notary/tuf/testutils/keys"
"github.com/docker/notary/tuf/utils"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -89,7 +90,7 @@ func AddGetKeyCryptoServiceInterfaceBehaviorTests(t *testing.T, cs signed.Crypto
role := data.BaseRoles[i+1]
switch algo {
case data.RSAKey:
addedPrivKey, err = utils.GenerateRSAKey(rand.Reader, 2048)
addedPrivKey, err = testutils.GetRSAKey(2048)
case data.ECDSAKey:
addedPrivKey, err = utils.GenerateECDSAKey(rand.Reader)
case data.ED25519Key:
Expand Down Expand Up @@ -121,7 +122,7 @@ func AddListKeyCryptoServiceInterfaceBehaviorTests(t *testing.T, cs signed.Crypt
role := data.BaseRoles[i+1]
switch algo {
case data.RSAKey:
addedPrivKey, err = utils.GenerateRSAKey(rand.Reader, 2048)
addedPrivKey, err = testutils.GetRSAKey(2048)
case data.ECDSAKey:
addedPrivKey, err = utils.GenerateECDSAKey(rand.Reader)
case data.ED25519Key:
Expand Down
Loading

0 comments on commit 0501284

Please sign in to comment.