client library deletion functionality, and integration into remove cert

CLI

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

certs/certs_test.go

@@ -236,7 +236,7 @@ func filestoreWithTwoCerts(t *testing.T, gun, keyAlg string) (
 
 	cryptoService := cryptoservice.NewCryptoService(gun, fileKeyStore)
 
-	// Create a Manager
+	// Create a store
 	trustPath := filepath.Join(tempBaseDir, notary.TrustedCertsDir)
 	certStore, err := trustmanager.NewX509FilteredFileStore(
 		trustPath,

client/client.go

@@ -13,7 +13,6 @@ import (
 	"time"
 
 	"github.com/Sirupsen/logrus"
-
 	"github.com/docker/notary"
 	"github.com/docker/notary/certs"
 	"github.com/docker/notary/client/changelist"
@@ -910,3 +909,27 @@ func (r *NotaryRepository) rootFileKeyChange(role, action string, key data.Publi
 	}
 	return nil
 }
+
+// DeleteTrustData removes the trust data stored for this repo in the TUF cache and certificate store on the client side
+func (r *NotaryRepository) DeleteTrustData() error {
+	// Clear TUF files and cache
+	if err := r.fileStore.RemoveAll(); err != nil {
+		return fmt.Errorf("error clearing TUF repo data: %v", err)
+	}
+	r.tufRepo = tuf.NewRepo(nil, nil)
+	// Clear certificates
+	certificates, err := r.CertStore.GetCertificatesByCN(r.gun)
+	if err != nil {
+		// If there were no certificates to delete, we're done
+		if _, ok := err.(*trustmanager.ErrNoCertificatesFound); ok {
+			return nil
+		}
+		return fmt.Errorf("error retrieving certificates for %s: %v", r.gun, err)
+	}
+	for _, cert := range certificates {
+		if err := r.CertStore.RemoveCert(cert); err != nil {
+			return fmt.Errorf("error removing certificate: %v: %v", cert, err)
+		}
+	}
+	return nil
+}

client/client_test.go

@@ -2836,3 +2836,117 @@ func testPublishTargetsDelgationCanUseUserKeyWithArbitraryRole(t *testing.T, x50
 
 	delgRec.assertAsked(t, []string{"targets/a/b"})
 }
+
+// TestDeleteRepo tests that local repo data, certificate, and keys are deleted from the client library call
+func TestDeleteRepo(t *testing.T) {
+	gun := "docker.com/notary"
+
+	ts, _, _ := simpleTestServer(t)
+	defer ts.Close()
+
+	repo, rootKeyID := initializeRepo(t, data.ECDSAKey, gun, ts.URL, false)
+	defer os.RemoveAll(repo.baseDir)
+
+	// Assert initialization was successful before we delete
+	assertRepoHasExpectedKeys(t, repo, rootKeyID, true)
+	assertRepoHasExpectedCerts(t, repo)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalRootRole, true)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalTargetsRole, true)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, true)
+
+	// Delete all client trust data for repo
+	err := repo.DeleteTrustData()
+	assert.NoError(t, err)
+
+	// Assert no metadata for this repo exists locally
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalRootRole, false)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalTargetsRole, false)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, false)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalTimestampRole, false)
+
+	// Assert no certs for this repo exist locally
+	_, err = repo.CertStore.GetCertificatesByCN(gun)
+	assert.Error(t, err)
+	assert.IsType(t, &trustmanager.ErrNoCertificatesFound{}, err)
+	assert.NotNil(t, err)
+
+	// Assert keys for this repo exist locally
+	assertRepoHasExpectedKeys(t, repo, rootKeyID, true)
+}
+
+type brokenRemoveFilestore struct {
+	store.MetadataStore
+}
+
+func (s *brokenRemoveFilestore) RemoveAll() error {
+	return fmt.Errorf("can't remove from this broken filestore")
+}
+
+// TestDeleteRepoBadFilestore tests that we properly error when trying to remove against a faulty filestore
+func TestDeleteRepoBadFilestore(t *testing.T) {
+	gun := "docker.com/notary"
+
+	ts, _, _ := simpleTestServer(t)
+	defer ts.Close()
+
+	repo, rootKeyID := initializeRepo(t, data.ECDSAKey, gun, ts.URL, false)
+	defer os.RemoveAll(repo.baseDir)
+
+	// Assert initialization was successful before we delete
+	assertRepoHasExpectedKeys(t, repo, rootKeyID, true)
+	assertRepoHasExpectedCerts(t, repo)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalRootRole, true)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalTargetsRole, true)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, true)
+
+	// Make the filestore faulty on remove
+	repo.fileStore = &brokenRemoveFilestore{repo.fileStore}
+
+	// Delete all client trust data for repo, assert an error on the filestore removal
+	err := repo.DeleteTrustData()
+	assert.Error(t, err)
+}
+
+// TestDeleteRepoNoCerts tests that local repo data is deleted successfully without an error even when we do not have certificates
+func TestDeleteRepoNoCerts(t *testing.T) {
+	gun := "docker.com/notary"
+
+	ts, _, _ := simpleTestServer(t)
+	defer ts.Close()
+
+	repo, rootKeyID := initializeRepo(t, data.ECDSAKey, gun, ts.URL, false)
+	defer os.RemoveAll(repo.baseDir)
+
+	// Assert initialization was successful before we delete
+	assertRepoHasExpectedKeys(t, repo, rootKeyID, true)
+	assertRepoHasExpectedCerts(t, repo)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalRootRole, true)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalTargetsRole, true)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, true)
+
+	// Delete the certificate store contents and assert it has been fully deleted
+	repo.CertStore.RemoveAll()
+	_, err := repo.CertStore.GetCertificatesByCN(gun)
+	assert.Error(t, err)
+	assert.IsType(t, &trustmanager.ErrNoCertificatesFound{}, err)
+	assert.NotNil(t, err)
+
+	// Delete all client trust data for repo
+	err = repo.DeleteTrustData()
+	assert.NoError(t, err)
+
+	// Assert no metadata for this repo exists locally
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalRootRole, false)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalTargetsRole, false)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, false)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalTimestampRole, false)
+
+	// Assert no certs for this repo exist locally
+	_, err = repo.CertStore.GetCertificatesByCN(gun)
+	assert.Error(t, err)
+	assert.IsType(t, &trustmanager.ErrNoCertificatesFound{}, err)
+	assert.NotNil(t, err)
+
+	// Assert keys for this repo exist locally
+	assertRepoHasExpectedKeys(t, repo, rootKeyID, true)
+}

cmd/notary/cert.go

@@ -6,8 +6,8 @@ import (
 	"path/filepath"
 
 	"github.com/docker/notary"
+	notaryclient "github.com/docker/notary/client"
 	"github.com/docker/notary/trustmanager"
-
 	"github.com/spf13/cobra"
 )
 
@@ -43,6 +43,7 @@ var cmdCertRemove = &cobra.Command{
 }
 
 // certRemove deletes a certificate given a cert ID or a gun
+// If given a gun, certRemove will also remove local TUF data
 func certRemove(cmd *cobra.Command, args []string) {
 	// If the user hasn't provided -g with a gun, or a cert ID, show usage
 	// If the user provided -g and a cert ID, also show usage
@@ -63,31 +64,39 @@ func certRemove(cmd *cobra.Command, args []string) {
 	}
 
 	var certsToRemove []*x509.Certificate
+	var certFoundByID *x509.Certificate
+	var removeTrustData bool
 
 	// If there is no GUN, we expect a cert ID
 	if certRemoveGUN == "" {
 		certID := args[0]
-		// This is an invalid ID
-		if len(certID) != idSize {
-			fatalf("Invalid certificate ID provided: %s", certID)
-		}
-		// Attempt to find this certificates
-		cert, err := certStore.GetCertificateByCertID(certID)
+		// Attempt to find this certificate
+		certFoundByID, err = certStore.GetCertificateByCertID(certID)
 		if err != nil {
+			// This is an invalid ID, the user might have forgotten a character
+			if len(certID) != notary.Sha256HexSize {
+				fatalf("Unable to retrieve certificate with invalid certificate ID provided: %s", certID)
+			}
 			fatalf("Unable to retrieve certificate with cert ID: %s", certID)
 		}
-		certsToRemove = append(certsToRemove, cert)
-	} else {
-		// We got the -g flag, it's a GUN
-		toRemove, err := certStore.GetCertificatesByCN(
-			certRemoveGUN)
-		if err != nil {
-			fatalf("%v", err)
-		}
-		certsToRemove = append(certsToRemove, toRemove...)
+		// the GUN is the CN from the certificate
+		certRemoveGUN = certFoundByID.Subject.CommonName
+		certsToRemove = []*x509.Certificate{certFoundByID}
 	}
 
-	// List all the keys about to be removed
+	toRemove, err := certStore.GetCertificatesByCN(certRemoveGUN)
+	// We could not find any certificates matching the user's query, so propagate the error
+	if err != nil {
+		fatalf("%v", err)
+	}
+
+	// If we specified a GUN or if the ID we specified is the only certificate with its CN, remove all GUN certs and trust data too
+	if certFoundByID == nil || len(toRemove) == 1 {
+		removeTrustData = true
+		certsToRemove = toRemove
+	}
+
+	// List all the certificates about to be removed
 	cmd.Printf("The following certificates will be removed:\n\n")
 	for _, cert := range certsToRemove {
 		// This error can't occur because we're getting certs off of an
@@ -95,6 +104,10 @@ func certRemove(cmd *cobra.Command, args []string) {
 		certID, _ := trustmanager.FingerprintCert(cert)
 		cmd.Printf("%s - %s\n", cert.Subject.CommonName, certID)
 	}
+	// If we were given a GUN or the last ID for a GUN, inform the user that we'll also delete all TUF data
+	if removeTrustData {
+		cmd.Printf("\nAll local trust data will be removed for %s\n", certRemoveGUN)
+	}
 	cmd.Println("\nAre you sure you want to remove these certificates? (yes/no)")
 
 	// Ask for confirmation before removing certificates, unless -y is provided
@@ -105,11 +118,24 @@ func certRemove(cmd *cobra.Command, args []string) {
 		}
 	}
 
-	// Remove all the certs
-	for _, cert := range certsToRemove {
-		err = certStore.RemoveCert(cert)
+	if removeTrustData {
+		// Remove all TUF data, so call RemoveTrustData on a NotaryRepository with the GUN
+		// no online operations are performed so the transport argument is nil
+		nRepo, err := notaryclient.NewNotaryRepository(trustDir, certRemoveGUN, getRemoteTrustServer(mainViper), nil, retriever)
 		if err != nil {
-			fatalf("Failed to remove root certificate for %s", cert.Subject.CommonName)
+			fatalf("Could not establish trust data for GUN %s", certRemoveGUN)
+		}
+		// DeleteTrustData will pick up all of the same certificates by GUN (CN) and remove them
+		err = nRepo.DeleteTrustData()
+		if err != nil {
+			fatalf("Failed to delete trust data for %s", certRemoveGUN)
+		}
+	} else {
+		for _, cert := range certsToRemove {
+			err = certStore.RemoveCert(cert)
+			if err != nil {
+				fatalf("Failed to remove cert %s", cert)
+			}
 		}
 	}
 }

cmd/notary/integration_test.go

@@ -15,6 +15,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/Sirupsen/logrus"
 	ctxu "github.com/docker/distribution/context"
@@ -525,11 +526,17 @@ func TestClientCertInteraction(t *testing.T) {
 	_, err = runCommand(t, tempDir, "-s", server.URL, "init", "gun2")
 	assert.NoError(t, err)
 	certs := assertNumCerts(t, tempDir, 2)
+	assertNumKeys(t, tempDir, 1, 4, !rootOnHardware())
 
 	// remove certs for one gun
 	_, err = runCommand(t, tempDir, "cert", "remove", "-g", "gun1", "-y")
 	assert.NoError(t, err)
 	certs = assertNumCerts(t, tempDir, 1)
+	// assert that when we remove cert by gun, we do not remove repo signing keys
+	assertNumKeys(t, tempDir, 1, 4, !rootOnHardware())
+	// assert that when we remove cert by gun, we also remove TUF metadata
+	_, err = os.Stat(filepath.Join(tempDir, "tuf", "gun1"))
+	assert.Error(t, err)
 
 	// remove a single cert
 	certID := strings.Fields(certs[0])[1]
@@ -539,6 +546,42 @@ func TestClientCertInteraction(t *testing.T) {
 	_, err = runCommand(t, tempDir, "cert", "remove", certID, "-y", "-g", "")
 	assert.NoError(t, err)
 	assertNumCerts(t, tempDir, 0)
+	// assert that when we remove the last cert ID for a gun, we also remove TUF metadata
+	_, err = os.Stat(filepath.Join(tempDir, "tuf", "gun2"))
+	assert.Error(t, err)
+
+	// Setup certificate with nonexistent repo GUN
+	// Check that we can only remove one certificate when specifying one ID
+	startTime := time.Now()
+	privKey, err := trustmanager.GenerateECDSAKey(rand.Reader)
+	assert.NoError(t, err)
+	noGunCert, err := cryptoservice.GenerateCertificate(
+		privKey, "nonexistent", startTime, startTime.AddDate(10, 0, 0))
+	assert.NoError(t, err)
+	certStore, err := trustmanager.NewX509FileStore(filepath.Join(tempDir, "trusted_certificates"))
+	assert.NoError(t, err)
+	err = certStore.AddCert(noGunCert)
+	assert.NoError(t, err)
+
+	certs = assertNumCerts(t, tempDir, 1)
+	certID = strings.Fields(certs[0])[1]
+
+	privKey, err = trustmanager.GenerateECDSAKey(rand.Reader)
+	assert.NoError(t, err)
+	noGunCert2, err := cryptoservice.GenerateCertificate(
+		privKey, "nonexistent", startTime, startTime.AddDate(10, 0, 0))
+	assert.NoError(t, err)
+	err = certStore.AddCert(noGunCert2)
+	assert.NoError(t, err)
+
+	certs = assertNumCerts(t, tempDir, 2)
+
+	// passing an empty gun to overwrite previously stored gun
+	_, err = runCommand(t, tempDir, "cert", "remove", certID, "-y", "-g", "")
+	assert.NoError(t, err)
+
+	// Since another cert with the same GUN exists, we didn't remove everything
+	assertNumCerts(t, tempDir, 1)
 }
 
 // Tests default root key generation

cmd/notary/keys.go

@@ -16,6 +16,7 @@ import (
 	"github.com/docker/notary/passphrase"
 	"github.com/docker/notary/trustmanager"
 
+	"github.com/docker/notary"
 	"github.com/docker/notary/tuf/data"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
@@ -253,7 +254,7 @@ func (k *keyCommander) keysExportRoot(cmd *cobra.Command, args []string) error {
 	keyID := args[0]
 	exportFilename := args[1]
 
-	if len(keyID) != idSize {
+	if len(keyID) != notary.Sha256HexSize {
 		return fmt.Errorf("Please specify a valid root key ID")
 	}
 
@@ -473,7 +474,7 @@ func (k *keyCommander) keyRemove(cmd *cobra.Command, args []string) error {
 	keyID := args[0]
 
 	// This is an invalid ID
-	if len(keyID) != idSize {
+	if len(keyID) != notary.Sha256HexSize {
 		return fmt.Errorf("invalid key ID provided: %s", keyID)
 	}
 	cmd.Println("")

cmd/notary/main.go

@@ -17,7 +17,6 @@ import (
 const (
 	configDir        = ".notary/"
 	defaultServerURL = "https://notary-server:4443"
-	idSize           = 64
 )
 
 var (

const.go

@@ -4,5 +4,6 @@ package notary
 const (
 	PrivKeyPerms    = 0700
 	PubCertPerms    = 0755
+	Sha256HexSize   = 64
 	TrustedCertsDir = "trusted_certificates"
 )

tuf/store/filestore.go

@@ -84,3 +84,8 @@ func (f *FilesystemStore) SetMeta(name string, meta []byte) error {
 	}
 	return nil
 }
+
+// RemoveAll clears the existing filestore by removing its base directory
+func (f *FilesystemStore) RemoveAll() error {
+	return os.RemoveAll(f.baseDir)
+}

tuf/store/filestore_test.go

@@ -109,3 +109,30 @@ func TestGetMetaNoSuchMetadata(t *testing.T) {
 	assert.Error(t, err)
 	assert.IsType(t, ErrMetaNotFound{}, err)
 }
+
+func TestRemoveAll(t *testing.T) {
+	s, err := NewFilesystemStore(testDir, "metadata", "json", "targets")
+	assert.Nil(t, err, "Initializing FilesystemStore returned unexpected error: %v", err)
+	defer os.RemoveAll(testDir)
+
+	testContent := []byte("test data")
+
+	// Write some files in metadata and targets dirs
+	metaPath := path.Join(testDir, "metadata", "testMeta.json")
+	ioutil.WriteFile(metaPath, testContent, 0600)
+	targetsPath := path.Join(testDir, "targets", "testTargets.json")
+	ioutil.WriteFile(targetsPath, testContent, 0600)
+
+	// Remove all
+	err = s.RemoveAll()
+	assert.Nil(t, err, "Removing all from FilesystemStore returned unexpected error: %v", err)
+
+	// Test that files no longer exist
+	_, err = ioutil.ReadFile(metaPath)
+	assert.True(t, os.IsNotExist(err))
+	_, err = ioutil.ReadFile(targetsPath)
+	assert.True(t, os.IsNotExist(err))
+
+	// Removing the empty filestore returns nil
+	assert.Nil(t, s.RemoveAll())
+}

tuf/store/httpstore.go

@@ -227,6 +227,11 @@ func (s HTTPStore) SetMultiMeta(metas map[string][]byte) error {
 	return translateStatusToError(resp, "POST metadata endpoint")
 }
 
+// RemoveAll in the interface is not supported, admins should use the DeleteHandler endpoint directly to delete remote data for a GUN
+func (s HTTPStore) RemoveAll() error {
+	return errors.New("remove all functionality not supported for HTTPStore")
+}
+
 func (s HTTPStore) buildMetaURL(name string) (*url.URL, error) {
 	var filename string
 	if name != "" {

tuf/store/httpstore_test.go

@@ -244,3 +244,19 @@ func TestTranslateErrorsWhenCannotParse400(t *testing.T) {
 		assert.IsType(t, ErrInvalidOperation{}, err)
 	}
 }
+
+func TestHTTPStoreRemoveAll(t *testing.T) {
+	// Set up a simple handler and server for our store
+	handler := func(w http.ResponseWriter, r *http.Request) {
+		w.Write([]byte(testRoot))
+	}
+	server := httptest.NewServer(http.HandlerFunc(handler))
+	defer server.Close()
+	store, err := NewHTTPStore(server.URL, "metadata", "json", "targets", "key", http.DefaultTransport)
+	assert.NoError(t, err)
+
+	// currently unsupported since there is no use case
+	// check for the error
+	err = store.RemoveAll()
+	assert.Error(t, err)
+}

tuf/store/interfaces.go

@@ -14,6 +14,7 @@ type MetadataStore interface {
 	GetMeta(name string, size int64) ([]byte, error)
 	SetMeta(name string, blob []byte) error
 	SetMultiMeta(map[string][]byte) error
+	RemoveAll() error
 }
 
 // PublicKeyStore must be implemented by a key service

tuf/store/memorystore.go

@@ -95,3 +95,11 @@ func (m *memoryStore) Commit(map[string][]byte, bool, map[string]data.Hashes) er
 func (m *memoryStore) GetKey(role string) ([]byte, error) {
 	return nil, fmt.Errorf("GetKey is not implemented for the memoryStore")
 }
+
+// Clear this existing memory store by setting this store as new empty one
+func (m *memoryStore) RemoveAll() error {
+	m.meta = make(map[string][]byte)
+	m.files = make(map[string][]byte)
+	m.keys = make(map[string][]data.PrivateKey)
+	return nil
+}

tuf/store/memorystore_test.go

@@ -0,0 +1,30 @@
+package store
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestMemoryStore(t *testing.T) {
+	s := NewMemoryStore(nil, nil)
+	_, err := s.GetMeta("nonexistent", 0)
+	require.Error(t, err)
+	require.IsType(t, ErrMetaNotFound{}, err)
+
+	metaContent := []byte("content")
+	metaSize := int64(7)
+	err = s.SetMeta("exists", metaContent)
+	require.NoError(t, err)
+
+	meta, err := s.GetMeta("exists", metaSize)
+	require.NoError(t, err)
+	require.Equal(t, metaContent, meta)
+
+	err = s.RemoveAll()
+	require.NoError(t, err)
+
+	_, err = s.GetMeta("exists", 0)
+	require.Error(t, err)
+	require.IsType(t, ErrMetaNotFound{}, err)
+}

tuf/store/offlinestore.go

@@ -41,3 +41,8 @@ func (es OfflineStore) GetKey(role string) ([]byte, error) {
 func (es OfflineStore) GetTarget(path string) (io.ReadCloser, error) {
 	return nil, err
 }
+
+// RemoveAll return ErrOffline
+func (es OfflineStore) RemoveAll() error {
+	return err
+}

tuf/store/offlinestore_test.go

@@ -27,6 +27,10 @@ func TestOfflineStore(t *testing.T) {
 	_, err = s.GetTarget("")
 	require.Error(t, err)
 	require.IsType(t, ErrOffline{}, err)
+
+	err = s.RemoveAll()
+	require.Error(t, err)
+	require.IsType(t, ErrOffline{}, err)
 }
 
 func TestErrOffline(t *testing.T) {