From edf0520c9b56869a73f4e1fb17f3b95c039f4b3f Mon Sep 17 00:00:00 2001 From: Ying Li Date: Fri, 13 Nov 2015 02:17:58 -0800 Subject: [PATCH] Remove KeyStoreManager's dependency on a KeyStore. The root generation code is handled by CryptoService now. Signed-off-by: Ying Li --- client/client_test.go | 17 ++++++---- client/client_test_pkcs11.go | 19 +++++++++++ client/repo.go | 4 +-- client/repo_pkcs11.go | 6 ++-- cmd/notary/cert.go | 12 ++----- keystoremanager/keystoremanager.go | 33 +------------------ keystoremanager/keystoremanager_test.go | 43 +++++++++---------------- 7 files changed, 53 insertions(+), 81 deletions(-) create mode 100644 client/client_test_pkcs11.go diff --git a/client/client_test.go b/client/client_test.go index 156d640ccf..c89a5ab1e1 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -69,13 +69,13 @@ func initializeRepo(t *testing.T, rootType, tempBaseDir, gun, url string) (*Nota tempBaseDir, gun, url, http.DefaultTransport, passphraseRetriever) assert.NoError(t, err, "error creating repo: %s", err) - rootKeyID, err := repo.KeyStoreManager.GenRootKey(rootType) + rootPubKey, err := repo.CryptoService.Create("root", rootType) assert.NoError(t, err, "error generating root key: %s", err) - err = repo.Initialize(rootKeyID) + err = repo.Initialize(rootPubKey.ID()) assert.NoError(t, err, "error creating repository: %s", err) - return repo, rootKeyID + return repo, rootPubKey.ID() } // TestInitRepo runs through the process of initializing a repository and makes @@ -119,9 +119,12 @@ func testInitRepo(t *testing.T, rootType string) { // Look for keys in private. The filenames should match the key IDs // in the private key store. - privKeyList := repo.KeyStoreManager.KeyStore.ListFiles() + keyFileStore, err := trustmanager.NewKeyFileStore(tempBaseDir, passphraseRetriever) + assert.NoError(t, err) + + privKeyList := keyFileStore.ListFiles() for _, privKeyName := range privKeyList { - privKeyFileName := filepath.Join(repo.KeyStoreManager.KeyStore.BaseDir(), privKeyName) + privKeyFileName := filepath.Join(keyFileStore.BaseDir(), privKeyName) _, err := os.Stat(privKeyFileName) assert.NoError(t, err, "missing private key: %s", privKeyName) } @@ -315,7 +318,9 @@ func fakeServerData(t *testing.T, repo *NotaryRepository, mux *http.ServeMux) { savedTUFRepo := repo.tufRepo // in case this is overwritten - repo.KeyStoreManager.KeyStore.AddKey( + fileStore, err := trustmanager.NewKeyFileStore(repo.baseDir, passphraseRetriever) + assert.NoError(t, err) + fileStore.AddKey( filepath.Join(filepath.FromSlash(repo.gun), tempKey.ID()), "nonroot", tempKey) diff --git a/client/client_test_pkcs11.go b/client/client_test_pkcs11.go new file mode 100644 index 0000000000..01439c97d0 --- /dev/null +++ b/client/client_test_pkcs11.go @@ -0,0 +1,19 @@ +// +build pkcs11 + +package client + +import "github.com/docker/notary/trustmanager/yubikey" + +// clear out all keys +func init() { + yubikey.SetYubikeyKeyMode(0) + if !yubikey.YubikeyAccessible() { + return + } + store, err := yubikey.NewYubiKeyStore(nil, nil) + if err == nil { + for k := range store.ListKeys() { + store.RemoveKey(k) + } + } +} diff --git a/client/repo.go b/client/repo.go index d289d7ab68..7cc3986348 100644 --- a/client/repo.go +++ b/client/repo.go @@ -24,12 +24,12 @@ func NewNotaryRepository(baseDir, gun, baseURL string, rt http.RoundTripper, return nil, fmt.Errorf("failed to create private key store in directory: %s", baseDir) } - keyStoreManager, err := keystoremanager.NewKeyStoreManager(baseDir, fileKeyStore) + keyStoreManager, err := keystoremanager.NewKeyStoreManager(baseDir) if err != nil { return nil, err } - cryptoService := cryptoservice.NewCryptoService(gun, keyStoreManager.KeyStore) + cryptoService := cryptoservice.NewCryptoService(gun, fileKeyStore) nRepo := &NotaryRepository{ gun: gun, diff --git a/client/repo_pkcs11.go b/client/repo_pkcs11.go index c8a9d27823..e72dd43827 100644 --- a/client/repo_pkcs11.go +++ b/client/repo_pkcs11.go @@ -27,13 +27,13 @@ func NewNotaryRepository(baseDir, gun, baseURL string, rt http.RoundTripper, return nil, fmt.Errorf("failed to create private key store in directory: %s", baseDir) } - keyStoreManager, err := keystoremanager.NewKeyStoreManager(baseDir, fileKeyStore) + keyStoreManager, err := keystoremanager.NewKeyStoreManager(baseDir) yubiKeyStore, _ := yubikey.NewYubiKeyStore(fileKeyStore, retriever) var cryptoService signed.CryptoService if yubiKeyStore == nil { - cryptoService = cryptoservice.NewCryptoService(gun, keyStoreManager.KeyStore) + cryptoService = cryptoservice.NewCryptoService(gun, fileKeyStore) } else { - cryptoService = cryptoservice.NewCryptoService(gun, yubiKeyStore, keyStoreManager.KeyStore) + cryptoService = cryptoservice.NewCryptoService(gun, yubiKeyStore, fileKeyStore) } nRepo := &NotaryRepository{ diff --git a/cmd/notary/cert.go b/cmd/notary/cert.go index a21e6979f3..f729c5bad3 100644 --- a/cmd/notary/cert.go +++ b/cmd/notary/cert.go @@ -54,11 +54,7 @@ func certRemove(cmd *cobra.Command, args []string) { parseConfig() trustDir := mainViper.GetString("trust_dir") - fileKeyStore, err := trustmanager.NewKeyFileStore(trustDir, retriever) - if err != nil { - fatalf("Failed to create private key store in directory: %s", trustDir) - } - keyStoreManager, err := keystoremanager.NewKeyStoreManager(trustDir, fileKeyStore) + keyStoreManager, err := keystoremanager.NewKeyStoreManager(trustDir) if err != nil { fatalf("Failed to create a new truststore manager with directory: %s", trustDir) } @@ -122,11 +118,7 @@ func certList(cmd *cobra.Command, args []string) { parseConfig() trustDir := mainViper.GetString("trust_dir") - fileKeyStore, err := trustmanager.NewKeyFileStore(trustDir, retriever) - if err != nil { - fatalf("Failed to create private key store in directory: %s", trustDir) - } - keyStoreManager, err := keystoremanager.NewKeyStoreManager(trustDir, fileKeyStore) + keyStoreManager, err := keystoremanager.NewKeyStoreManager(trustDir) if err != nil { fatalf("Failed to create a new truststore manager with directory: %s", trustDir) } diff --git a/keystoremanager/keystoremanager.go b/keystoremanager/keystoremanager.go index d22da537e0..ccf5380ea9 100644 --- a/keystoremanager/keystoremanager.go +++ b/keystoremanager/keystoremanager.go @@ -1,12 +1,10 @@ package keystoremanager import ( - "crypto/rand" "crypto/x509" "errors" "fmt" "path/filepath" - "strings" "time" "github.com/Sirupsen/logrus" @@ -18,7 +16,6 @@ import ( // KeyStoreManager is an abstraction around the root and non-root key stores, // and related CA stores type KeyStoreManager struct { - KeyStore *trustmanager.KeyFileStore trustedCAStore trustmanager.X509Store trustedCertificateStore trustmanager.X509Store } @@ -54,7 +51,7 @@ func (err ErrRootRotationFail) Error() string { // NewKeyStoreManager returns an initialized KeyStoreManager, or an error // if it fails to create the KeyFileStores or load certificates -func NewKeyStoreManager(baseDir string, keyStore *trustmanager.KeyFileStore) (*KeyStoreManager, error) { +func NewKeyStoreManager(baseDir string) (*KeyStoreManager, error) { trustPath := filepath.Join(baseDir, trustDir) // Load all CAs that aren't expired and don't use SHA1 @@ -82,7 +79,6 @@ func NewKeyStoreManager(baseDir string, keyStore *trustmanager.KeyFileStore) (*K } return &KeyStoreManager{ - KeyStore: keyStore, trustedCAStore: trustedCAStore, trustedCertificateStore: trustedCertificateStore, }, nil @@ -110,33 +106,6 @@ func (km *KeyStoreManager) AddTrustedCACert(cert *x509.Certificate) { km.trustedCAStore.AddCert(cert) } -// GenRootKey generates a new root key -func (km *KeyStoreManager) GenRootKey(algorithm string) (string, error) { - var err error - var privKey data.PrivateKey - - // We don't want external API callers to rely on internal TUF data types, so - // the API here should continue to receive a string algorithm, and ensure - // that it is downcased - switch strings.ToLower(algorithm) { - case data.RSAKey: - privKey, err = trustmanager.GenerateRSAKey(rand.Reader, rsaRootKeySize) - case data.ECDSAKey: - privKey, err = trustmanager.GenerateECDSAKey(rand.Reader) - default: - return "", fmt.Errorf("only RSA or ECDSA keys are currently supported. Found: %s", algorithm) - - } - if err != nil { - return "", fmt.Errorf("failed to generate private key: %v", err) - } - - // Changing the root - km.KeyStore.AddKey(privKey.ID(), "root", privKey) - - return privKey.ID(), nil -} - /* ValidateRoot receives a new root, validates its correctness and attempts to do root key rotation if needed. diff --git a/keystoremanager/keystoremanager_test.go b/keystoremanager/keystoremanager_test.go index 9304b5971c..b582a5bf2a 100644 --- a/keystoremanager/keystoremanager_test.go +++ b/keystoremanager/keystoremanager_test.go @@ -121,11 +121,8 @@ func TestValidateRoot(t *testing.T) { defer os.RemoveAll(tempBaseDir) assert.NoError(t, err, "failed to create a temporary directory: %s", err) - fileKeyStore, err := trustmanager.NewKeyFileStore(tempBaseDir, passphraseRetriever) - assert.NoError(t, err) - // Create a FileStoreManager - keyStoreManager, err := NewKeyStoreManager(tempBaseDir, fileKeyStore) + keyStoreManager, err := NewKeyStoreManager(tempBaseDir) assert.NoError(t, err) // Execute our template @@ -135,17 +132,13 @@ func TestValidateRoot(t *testing.T) { // Unmarshal our signedroot json.Unmarshal(signedRootBytes.Bytes(), &testSignedRoot) - // // This call to ValidateRoot will succeed since we are using a valid PEM // encoded certificate, and have no other certificates for this CN - // err = keyStoreManager.ValidateRoot(&testSignedRoot, "docker.com/notary") assert.NoError(t, err) - // // This call to ValidateRoot will fail since we are passing in a dnsName that // doesn't match the CN of the certificate. - // err = keyStoreManager.ValidateRoot(&testSignedRoot, "diogomonica.com/notary") if assert.Error(t, err, "An error was expected") { assert.Equal(t, err, &ErrValidationFail{Reason: "unable to retrieve valid leaf certificates"}) @@ -228,23 +221,25 @@ func TestValidateSuccessfulRootRotation(t *testing.T) { // manager and certificates for two keys which have been added to the keystore. // Also returns the temporary directory so it can be cleaned up. func filestoreWithTwoCerts(t *testing.T, gun, keyAlg string) ( - string, *KeyStoreManager, []*x509.Certificate) { + string, *KeyStoreManager, *cryptoservice.CryptoService, []*x509.Certificate) { tempBaseDir, err := ioutil.TempDir("", "notary-test-") assert.NoError(t, err, "failed to create a temporary directory: %s", err) fileKeyStore, err := trustmanager.NewKeyFileStore(tempBaseDir, passphraseRetriever) assert.NoError(t, err) + cryptoService := cryptoservice.NewCryptoService(gun, fileKeyStore) + // Create a FileStoreManager - keyStoreManager, err := NewKeyStoreManager(tempBaseDir, fileKeyStore) + keyStoreManager, err := NewKeyStoreManager(tempBaseDir) assert.NoError(t, err) certs := make([]*x509.Certificate, 2) for i := 0; i < 2; i++ { - keyID, err := keyStoreManager.GenRootKey(keyAlg) + pubKey, err := cryptoService.Create("root", keyAlg) assert.NoError(t, err) - key, _, err := keyStoreManager.KeyStore.GetKey(keyID) + key, _, err := fileKeyStore.GetKey(pubKey.ID()) assert.NoError(t, err) cert, err := cryptoservice.GenerateCertificate(key, gun) @@ -252,14 +247,14 @@ func filestoreWithTwoCerts(t *testing.T, gun, keyAlg string) ( certs[i] = cert } - return tempBaseDir, keyStoreManager, certs + return tempBaseDir, keyStoreManager, cryptoService, certs } func testValidateSuccessfulRootRotation(t *testing.T, keyAlg, rootKeyType string) { // The gun to test gun := "docker.com/notary" - tempBaseDir, keyStoreManager, certs := filestoreWithTwoCerts(t, gun, keyAlg) + tempBaseDir, keyStoreManager, cs, certs := filestoreWithTwoCerts(t, gun, keyAlg) defer os.RemoveAll(tempBaseDir) origRootCert := certs[0] replRootCert := certs[1] @@ -288,8 +283,6 @@ func testValidateSuccessfulRootRotation(t *testing.T, keyAlg, rootKeyType string signedTestRoot, err := testRoot.ToSigned() assert.NoError(t, err) - cs := cryptoservice.NewCryptoService(gun, keyStoreManager.KeyStore) - err = signed.Sign(cs, signedTestRoot, replRootKey) assert.NoError(t, err) @@ -322,7 +315,8 @@ func TestValidateRootRotationMissingOrigSig(t *testing.T) { func testValidateRootRotationMissingOrigSig(t *testing.T, keyAlg, rootKeyType string) { gun := "docker.com/notary" - tempBaseDir, keyStoreManager, certs := filestoreWithTwoCerts(t, gun, keyAlg) + tempBaseDir, keyStoreManager, cryptoService, certs := filestoreWithTwoCerts( + t, gun, keyAlg) defer os.RemoveAll(tempBaseDir) origRootCert := certs[0] replRootCert := certs[1] @@ -350,15 +344,11 @@ func testValidateRootRotationMissingOrigSig(t *testing.T, keyAlg, rootKeyType st assert.NoError(t, err) // We only sign with the new key, and not with the original one. - err = signed.Sign( - cryptoservice.NewCryptoService(gun, keyStoreManager.KeyStore), - signedTestRoot, replRootKey) + err = signed.Sign(cryptoService, signedTestRoot, replRootKey) assert.NoError(t, err) - // // This call to ValidateRoot will succeed since we are using a valid PEM // encoded certificate, and have no other certificates for this CN - // err = keyStoreManager.ValidateRoot(signedTestRoot, gun) assert.Error(t, err, "insuficient signatures on root") @@ -382,7 +372,8 @@ func TestValidateRootRotationMissingNewSig(t *testing.T) { func testValidateRootRotationMissingNewSig(t *testing.T, keyAlg, rootKeyType string) { gun := "docker.com/notary" - tempBaseDir, keyStoreManager, certs := filestoreWithTwoCerts(t, gun, keyAlg) + tempBaseDir, keyStoreManager, cryptoService, certs := filestoreWithTwoCerts( + t, gun, keyAlg) defer os.RemoveAll(tempBaseDir) origRootCert := certs[0] replRootCert := certs[1] @@ -412,15 +403,11 @@ func testValidateRootRotationMissingNewSig(t *testing.T, keyAlg, rootKeyType str assert.NoError(t, err) // We only sign with the old key, and not with the new one - err = signed.Sign( - cryptoservice.NewCryptoService(gun, keyStoreManager.KeyStore), - signedTestRoot, origRootKey) + err = signed.Sign(cryptoService, signedTestRoot, origRootKey) assert.NoError(t, err) - // // This call to ValidateRoot will succeed since we are using a valid PEM // encoded certificate, and have no other certificates for this CN - // err = keyStoreManager.ValidateRoot(signedTestRoot, gun) assert.Error(t, err, "insuficient signatures on root")