Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop support for RSA key generation #1191

Merged
merged 1 commit into from
Aug 21, 2017
Merged

Drop support for RSA key generation #1191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions client/client_test.go
View file
Open in desktop
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the rationale for using the testutil instead of the direct cryptoservice call?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah of course – because the cryptoservice cannot create RSA keys? If that is the exact reason, this is perfectly fine then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@riyazdf Exactly.

To add more on this, I tried to add keys to crypto service in case of RSA, while for other sort of keys I created them with crypto service which later adds them internally.

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
View file
Open in desktop
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
View file
Open in desktop
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.ConvertPrivateKeyToPKCS8(privKey, "", "", "")
require.NoError(t, err)
Expand Down
3 changes: 1 addition & 2 deletions cmd/notary/keys.go
View file
Open in desktop
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
View file
Open in desktop
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
4 changes: 4 additions & 0 deletions cryptoservice/crypto_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ func NewCryptoService(keyStores ...trustmanager.KeyStore) *CryptoService {

// Create is used to generate keys for targets, snapshots and timestamps
func (cs *CryptoService) Create(role data.RoleName, gun data.GUN, algorithm string) (data.PublicKey, error) {
if algorithm == data.RSAKey {
return nil, fmt.Errorf("%s keys can only be imported", data.RSAKey)
}

privKey, err := utils.GenerateKey(algorithm)
if err != nil {
return nil, fmt.Errorf("failed to generate %s key: %v", algorithm, err)
Expand Down
29 changes: 23 additions & 6 deletions cryptoservice/crypto_service_test.go
View file
Open in desktop
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 @@ -85,6 +86,16 @@ func (c CryptoServiceTester) TestCreateAndGetWhenMultipleKeystores(t *testing.T)
c.errorMsg("Could not find key ID %s", tufKey.ID()))
}

func (c CryptoServiceTester) TestCreateRSAKey(t *testing.T) {
cryptoService := c.cryptoServiceFactory()
cryptoService.keyStores = append(cryptoService.keyStores,
trustmanager.NewKeyMemoryStore(passphraseRetriever))

// Test Create for RSA algo
_, err := cryptoService.Create(c.role, c.gun, c.keyAlgo)
require.Error(t, err, c.errorMsg("rsa keys can only be imported"))
}

// asserts that getting key fails for a non-existent key
func (c CryptoServiceTester) TestGetNonexistentKey(t *testing.T) {
cryptoService := c.cryptoServiceFactory()
Expand All @@ -104,7 +115,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 +181,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 +205,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 +225,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 +386,15 @@ 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, testing it explicitly
if algo != data.RSAKey {
cst.TestCreateAndGetKey(t)
cst.TestCreateAndGetWhenMultipleKeystores(t)
} else {
cst.TestCreateRSAKey(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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1062,13 +1062,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
View file
Open in desktop
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
View file
Open in desktop
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