diff --git a/client/backwards_compatibility_test.go b/client/backwards_compatibility_test.go index 62b43b77a3..de10b1286c 100644 --- a/client/backwards_compatibility_test.go +++ b/client/backwards_compatibility_test.go @@ -29,10 +29,6 @@ func requireValidFixture(t *testing.T, notaryRepo *NotaryRepository) { for _, targetObj := range notaryRepo.tufRepo.Targets { require.True(t, targetObj.Signed.Expires.After(tenYearsInFuture)) } - - for _, cert := range notaryRepo.CertStore.GetCertificates() { - require.True(t, cert.NotAfter.After(tenYearsInFuture)) - } } // recursively copies the contents of one directory into another - ignores diff --git a/client/client.go b/client/client.go index 8a7c7230c3..d1586a707d 100644 --- a/client/client.go +++ b/client/client.go @@ -2,7 +2,6 @@ package client import ( "bytes" - "crypto/x509" "encoding/json" "fmt" "io/ioutil" @@ -87,7 +86,6 @@ type NotaryRepository struct { CryptoService signed.CryptoService tufRepo *tuf.Repo roundTrip http.RoundTripper - CertStore trustmanager.X509Store trustPinning trustpinning.TrustPinConfig } @@ -97,15 +95,6 @@ type NotaryRepository struct { func repositoryFromKeystores(baseDir, gun, baseURL string, rt http.RoundTripper, keyStores []trustmanager.KeyStore, trustPin trustpinning.TrustPinConfig) (*NotaryRepository, error) { - certPath := filepath.Join(baseDir, notary.TrustedCertsDir) - certStore, err := trustmanager.NewX509FilteredFileStore( - certPath, - trustmanager.FilterCertsExpiredSha1, - ) - if err != nil { - return nil, err - } - cryptoService := cryptoservice.NewCryptoService(keyStores...) nRepo := &NotaryRepository{ @@ -115,7 +104,6 @@ func repositoryFromKeystores(baseDir, gun, baseURL string, rt http.RoundTripper, tufRepoPath: filepath.Join(baseDir, tufDir, filepath.FromSlash(gun)), CryptoService: cryptoService, roundTrip: rt, - CertStore: certStore, trustPinning: trustPin, } @@ -162,22 +150,22 @@ func NewTarget(targetName string, targetPath string) (*Target, error) { return &Target{Name: targetName, Hashes: meta.Hashes, Length: meta.Length}, nil } -func rootCertKey(gun string, privKey data.PrivateKey) (*x509.Certificate, data.PublicKey, error) { +func rootCertKey(gun string, privKey data.PrivateKey) (data.PublicKey, error) { // Hard-coded policy: the generated certificate expires in 10 years. startTime := time.Now() cert, err := cryptoservice.GenerateCertificate( privKey, gun, startTime, startTime.Add(notary.Year*10)) if err != nil { - return nil, nil, err + return nil, err } x509PublicKey := trustmanager.CertToKey(cert) if x509PublicKey == nil { - return nil, nil, fmt.Errorf( + return nil, fmt.Errorf( "cannot use regenerated certificate: format %s", cert.PublicKeyAlgorithm) } - return cert, x509PublicKey, nil + return x509PublicKey, nil } // Initialize creates a new repository by using rootKey as the root Key for the @@ -218,11 +206,10 @@ func (r *NotaryRepository) Initialize(rootKeyID string, serverManagedRoles ...st } } - rootCert, rootKey, err := rootCertKey(r.gun, privKey) + rootKey, err := rootCertKey(r.gun, privKey) if err != nil { return err } - r.CertStore.AddCert(rootCert) var ( rootRole = data.NewBaseRole( @@ -790,9 +777,6 @@ func (r *NotaryRepository) Update(forWrite bool) error { // Partially populates r.tufRepo with this root metadata (only; use // tufclient.Client.Update to load the rest). // -// As another side effect, r.CertManager's list of trusted certificates -// is updated with data from the loaded root.json. -// // Fails if the remote server is reachable and does not know the repo // (i.e. before the first r.Publish()), in which case the error is // store.ErrMetaNotFound, or if the root metadata (from whichever source is used) @@ -883,19 +867,20 @@ func (r *NotaryRepository) validateRoot(rootJSON []byte, fromRemote bool) (*data var prevRoot *data.SignedRoot if fromRemote { prevRootJSON, err := r.fileStore.GetMeta(data.CanonicalRootRole, -1) + // A previous root exists, so we attempt to use it + // If for some reason we can't extract it (ex: it's corrupted), we should error client-side to be conservative if err == nil { prevSignedRoot := &data.Signed{} err = json.Unmarshal(prevRootJSON, prevSignedRoot) - if err == nil { - prevRoot, err = data.RootFromSigned(prevSignedRoot) + if err != nil { + return nil, &trustpinning.ErrValidationFail{fmt.Sprintf("unable to unmarshal previously trusted root from disk: %v", err)} + } + prevRoot, err = data.RootFromSigned(prevSignedRoot) + if err != nil { + return nil, &trustpinning.ErrValidationFail{fmt.Sprintf("error loading previously trusted root into valid role format: %v", err)} } } } - // If we had any errors while trying to retrieve the previous root, just set it to nil - if err != nil { - prevRoot = nil - } - err = trustpinning.ValidateRoot(prevRoot, root, r.gun, r.trustPinning) if err != nil { return nil, err @@ -947,7 +932,7 @@ func (r *NotaryRepository) RotateKey(role string, serverManagesKey bool) error { if err != nil { return err } - _, pubKey, err = rootCertKey(r.gun, privKey) + pubKey, err = rootCertKey(r.gun, privKey) if err != nil { return err } @@ -982,26 +967,12 @@ func (r *NotaryRepository) rootFileKeyChange(cl changelist.Changelist, role, act return cl.Add(c) } -// DeleteTrustData removes the trust data stored for this repo in the TUF cache and certificate store on the client side +// DeleteTrustData removes the trust data stored for this repo in the TUF cache 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) - // 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 } diff --git a/client/client_root_validation_test.go b/client/client_root_validation_test.go index 92e7e56252..1b58240aa3 100644 --- a/client/client_root_validation_test.go +++ b/client/client_root_validation_test.go @@ -33,11 +33,6 @@ func validateRootSuccessfully(t *testing.T, rootType string) { err := repo.tufRepo.InitTimestamp() require.NoError(t, err, "error creating repository: %s", err) - // Initialize is supposed to have created new certificate for this repository - // Lets check for it and store it for later use - allCerts := repo.CertStore.GetCertificates() - require.Len(t, allCerts, 1) - fakeServerData(t, repo, mux, keys) // diff --git a/client/client_test.go b/client/client_test.go index f902d6650a..40626a8612 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1911,17 +1911,15 @@ func TestPublishRootCorrupt(t *testing.T) { defer os.RemoveAll(repo.baseDir) testPublishBadMetadata(t, data.CanonicalRootRole, repo, false, false) - // publish first - publish should still succeed if root corrupt since the - // remote root is signed with the same key. + // publish first - publish should still fail if the local root is corrupt since + // we can't determine whether remote root is signed with the same key. repo, _ = initializeRepo(t, data.ECDSAKey, "docker.com/notary2", ts.URL, false) defer os.RemoveAll(repo.baseDir) - testPublishBadMetadata(t, data.CanonicalRootRole, repo, true, true) + testPublishBadMetadata(t, data.CanonicalRootRole, repo, true, false) } // When publishing snapshot, root, or target, if the repo hasn't been published -// before, if the metadata is corrupt, it can't be published. If it has been -// published already, then the corrupt metadata can just be re-downloaded, so -// publishing is successful. +// before, if the metadata is corrupt, it can't be published. func testPublishBadMetadata(t *testing.T, roleName string, repo *NotaryRepository, publishFirst, succeeds bool) { @@ -1938,7 +1936,11 @@ func testPublishBadMetadata(t *testing.T, roleName string, repo *NotaryRepositor require.NoError(t, err) } else { require.Error(t, err) - require.IsType(t, ®Json.SyntaxError{}, err) + if roleName == data.CanonicalRootRole && publishFirst { + require.IsType(t, &trustpinning.ErrValidationFail{}, err) + } else { + require.IsType(t, ®Json.SyntaxError{}, err) + } } // make an unreadable file by creating a directory instead of a file @@ -1950,7 +1952,7 @@ func testPublishBadMetadata(t *testing.T, roleName string, repo *NotaryRepositor defer os.RemoveAll(path) err = repo.Publish() - if succeeds { + if succeeds || publishFirst { require.NoError(t, err) } else { require.Error(t, err) diff --git a/client/client_update_test.go b/client/client_update_test.go index bc351d9e52..5051e40dfd 100644 --- a/client/client_update_test.go +++ b/client/client_update_test.go @@ -208,8 +208,9 @@ var waysToMessUpLocalMetadata = []swizzleExpectations{ // actively sabotage and break their own local repo (particularly the root.json) } -// If a repo has corrupt metadata (in that the hash doesn't match the snapshot) or -// missing metadata, an update will replace all of it +// If a repo has missing metadata, an update will replace all of it +// If a repo has corrupt metadata for root, the update will fail +// For other roles, corrupt metadata will be replaced func TestUpdateReplacesCorruptOrMissingMetadata(t *testing.T) { if testing.Short() { t.Skip("skipping test in short mode") @@ -230,104 +231,115 @@ func TestUpdateReplacesCorruptOrMissingMetadata(t *testing.T) { repoSwizzler := testutils.NewMetadataSwizzler("docker.com/notary", serverMeta, cs) repoSwizzler.MetadataCache = repo.fileStore + origMeta := testutils.CopyRepoMetadata(serverMeta) + for _, role := range repoSwizzler.Roles { for _, expt := range waysToMessUpLocalMetadata { text, messItUp := expt.desc, expt.swizzle for _, forWrite := range []bool{true, false} { require.NoError(t, messItUp(repoSwizzler, role), "could not fuzz %s (%s)", role, text) err := repo.Update(forWrite) - require.NoError(t, err) - for r, expected := range serverMeta { - actual, err := repo.fileStore.GetMeta(r, -1) - require.NoError(t, err, "problem getting repo metadata for %s", role) - require.True(t, bytes.Equal(expected, actual), - "%s for %s: expected to recover after update", text, role) + // if this is a root role, we should error if it's corrupted data + if role == data.CanonicalRootRole && expt.desc == "invalid JSON" { + require.Error(t, err) + // revert our original metadata + for role := range origMeta { + require.NoError(t, repo.fileStore.SetMeta(role, origMeta[role])) + } + } else { + require.NoError(t, err) + for r, expected := range serverMeta { + actual, err := repo.fileStore.GetMeta(r, -1) + require.NoError(t, err, "problem getting repo metadata for %s", role) + require.True(t, bytes.Equal(expected, actual), + "%s for %s: expected to recover after update", text, role) + } } } } } } -// TODO(riyazdf): this test goes against the new root pinning because the certstore -// no longer exists, so roots on the local filestore must be correct. -// TODO(riyazdf): break this test up to only test valid rejections, not associated -// with rotations -- such as expired metadata, etc. -//// If a repo has an invalid root (signed by wrong key, expired, invalid version, -//// invalid number of signatures, etc.), the repo will just get the new root from -//// the server, whether or not the update is for writing (forced update), but -//// it will refuse to update if the root key has changed and the new root is -//// not signed by the old and new key -//func TestUpdateFailsIfServerRootKeyChangedWithoutMultiSign(t *testing.T) { -// if testing.Short() { -// t.Skip("skipping test in short mode") -// } -// -// serverMeta, serverSwizzler := newServerSwizzler(t) -// origMeta := testutils.CopyRepoMetadata(serverMeta) -// -// ts := readOnlyServer(t, serverSwizzler.MetadataCache, http.StatusNotFound, "docker.com/notary") -// defer ts.Close() -// -// repo := newBlankRepo(t, ts.URL) -// defer os.RemoveAll(repo.baseDir) -// -// err := repo.Update(false) // ensure we have all metadata to start with -// require.NoError(t, err) -// -// // rotate the server's root.json root key so that they no longer match trust anchors -// require.NoError(t, serverSwizzler.ChangeRootKey()) -// // bump versions, update snapshot and timestamp too so it will not fail on a hash -// bumpVersions(t, serverSwizzler, 1) -// -// // we want to swizzle the local cache, not the server, so create a new one -// repoSwizzler := &testutils.MetadataSwizzler{ -// MetadataCache: repo.fileStore, -// CryptoService: serverSwizzler.CryptoService, -// Roles: serverSwizzler.Roles, -// } -// -// for _, expt := range waysToMessUpLocalMetadata { -// text, messItUp := expt.desc, expt.swizzle -// for _, forWrite := range []bool{true, false} { -// require.NoError(t, messItUp(repoSwizzler, data.CanonicalRootRole), "could not fuzz root (%s)", text) -// messedUpMeta, err := repo.fileStore.GetMeta(data.CanonicalRootRole, -1) -// -// if _, ok := err.(store.ErrMetaNotFound); ok { // one of the ways to mess up is to delete metadata -// -// err = repo.Update(forWrite) -// require.Error(t, err) // the new server has a different root key, update fails -// -// } else { -// -// require.NoError(t, err) -// -// err = repo.Update(forWrite) -// require.Error(t, err) // the new server has a different root, update fails -// -// // we can't test that all the metadata is the same, because we probably would -// // have downloaded a new timestamp and maybe snapshot. But the root should be the -// // same because it has failed to update. -// for role, expected := range origMeta { -// if role != data.CanonicalTimestampRole && role != data.CanonicalSnapshotRole { -// actual, err := repo.fileStore.GetMeta(role, -1) -// require.NoError(t, err, "problem getting repo metadata for %s", role) -// -// if role == data.CanonicalRootRole { -// expected = messedUpMeta -// } -// require.True(t, bytes.Equal(expected, actual), -// "%s for %s: expected to not have updated", text, role) -// } -// } -// -// } -// -// // revert our original root metadata -// require.NoError(t, -// repo.fileStore.SetMeta(data.CanonicalRootRole, origMeta[data.CanonicalRootRole])) -// } -// } -//} +// If a repo has an invalid root (signed by wrong key, expired, invalid version, +// invalid number of signatures, etc.), the repo will just get the new root from +// the server, whether or not the update is for writing (forced update), but +// it will refuse to update if the root key has changed and the new root is +// not signed by the old and new key +func TestUpdateFailsIfServerRootKeyChangedWithoutMultiSign(t *testing.T) { + if testing.Short() { + t.Skip("skipping test in short mode") + } + + serverMeta, serverSwizzler := newServerSwizzler(t) + origMeta := testutils.CopyRepoMetadata(serverMeta) + + ts := readOnlyServer(t, serverSwizzler.MetadataCache, http.StatusNotFound, "docker.com/notary") + defer ts.Close() + + repo := newBlankRepo(t, ts.URL) + defer os.RemoveAll(repo.baseDir) + + err := repo.Update(false) // ensure we have all metadata to start with + require.NoError(t, err) + + // rotate the server's root.json root key so that they no longer match trust anchors + require.NoError(t, serverSwizzler.ChangeRootKey()) + // bump versions, update snapshot and timestamp too so it will not fail on a hash + bumpVersions(t, serverSwizzler, 1) + + // we want to swizzle the local cache, not the server, so create a new one + repoSwizzler := &testutils.MetadataSwizzler{ + MetadataCache: repo.fileStore, + CryptoService: serverSwizzler.CryptoService, + Roles: serverSwizzler.Roles, + } + + for _, expt := range waysToMessUpLocalMetadata { + text, messItUp := expt.desc, expt.swizzle + for _, forWrite := range []bool{true, false} { + require.NoError(t, messItUp(repoSwizzler, data.CanonicalRootRole), "could not fuzz root (%s)", text) + messedUpMeta, err := repo.fileStore.GetMeta(data.CanonicalRootRole, -1) + + if _, ok := err.(store.ErrMetaNotFound); ok { // one of the ways to mess up is to delete metadata + + err = repo.Update(forWrite) + // the new server has a different root key, but we don't have any way of pinning against a previous root + require.NoError(t, err) + // revert our original metadata + for role := range origMeta { + require.NoError(t, repo.fileStore.SetMeta(role, origMeta[role])) + } + } else { + + require.NoError(t, err) + + err = repo.Update(forWrite) + require.Error(t, err) // the new server has a different root, update fails + + // we can't test that all the metadata is the same, because we probably would + // have downloaded a new timestamp and maybe snapshot. But the root should be the + // same because it has failed to update. + for role, expected := range origMeta { + if role != data.CanonicalTimestampRole && role != data.CanonicalSnapshotRole { + actual, err := repo.fileStore.GetMeta(role, -1) + require.NoError(t, err, "problem getting repo metadata for %s", role) + + if role == data.CanonicalRootRole { + expected = messedUpMeta + } + require.True(t, bytes.Equal(expected, actual), + "%s for %s: expected to not have updated -- swizzle method %s", text, role, expt.desc) + } + } + + } + + // revert our original root metadata + require.NoError(t, + repo.fileStore.SetMeta(data.CanonicalRootRole, origMeta[data.CanonicalRootRole])) + } + } +} type updateOpts struct { notFoundCode int // what code to return when the cache doesn't have the metadata @@ -774,15 +786,15 @@ func testUpdateRemoteFileChecksumWrong(t *testing.T, opts updateOpts, errExpecte // this does not include delete, which is tested separately so we can try to get // 404s and 503s var waysToMessUpServer = []swizzleExpectations{ - {desc: "invalid JSON", expectErrs: []interface{}{&json.SyntaxError{}}, + {desc: "invalid JSON", expectErrs: []interface{}{&trustpinning.ErrValidationFail{}, &json.SyntaxError{}}, swizzle: (*testutils.MetadataSwizzler).SetInvalidJSON}, - {desc: "an invalid Signed", expectErrs: []interface{}{&json.UnmarshalTypeError{}}, + {desc: "an invalid Signed", expectErrs: []interface{}{&trustpinning.ErrValidationFail{}, &json.UnmarshalTypeError{}}, swizzle: (*testutils.MetadataSwizzler).SetInvalidSigned}, {desc: "an invalid SignedMeta", // it depends which field gets unmarshalled first - expectErrs: []interface{}{&json.UnmarshalTypeError{}, &time.ParseError{}}, + expectErrs: []interface{}{&trustpinning.ErrValidationFail{}, &json.UnmarshalTypeError{}, &time.ParseError{}}, swizzle: (*testutils.MetadataSwizzler).SetInvalidSignedMeta}, // for the errors below, when we bootstrap root, we get cert.ErrValidationFail failures @@ -1201,12 +1213,6 @@ func TestUpdateLocalAndRemoteRootCorrupt(t *testing.T) { } for _, localExpt := range waysToMessUpLocalMetadata { for _, serverExpt := range waysToMessUpServer { - if localExpt.desc == "expired metadata" && serverExpt.desc == "lower metadata version" { - // TODO: bug right now where if the local metadata is invalid, we just download a - // new version - we verify the signatures and everything, but don't check the version - // against the previous if we can - continue - } testUpdateLocalAndRemoteRootCorrupt(t, true, localExpt, serverExpt) testUpdateLocalAndRemoteRootCorrupt(t, false, localExpt, serverExpt) } diff --git a/cmd/notary/cert.go b/cmd/notary/cert.go deleted file mode 100644 index 738e8dd67e..0000000000 --- a/cmd/notary/cert.go +++ /dev/null @@ -1,194 +0,0 @@ -package main - -import ( - "crypto/x509" - "fmt" - "os" - "path/filepath" - - "github.com/docker/notary" - notaryclient "github.com/docker/notary/client" - "github.com/docker/notary/passphrase" - "github.com/docker/notary/trustmanager" - "github.com/spf13/cobra" - "github.com/spf13/viper" -) - -var cmdCertTemplate = usageTemplate{ - Use: "cert", - Short: "Operates on certificates.", - Long: `Operations on certificates.`, -} - -var cmdCertListTemplate = usageTemplate{ - Use: "list", - Short: "Lists certificates.", - Long: "Lists root certificates known to notary.", -} - -var cmdCertRemoveTemplate = usageTemplate{ - Use: "remove [ certID ]", - Short: "Removes the certificate with the given cert ID.", - Long: "Remove the certificate with the given cert ID from the local host.", -} - -type certCommander struct { - // these need to be set - configGetter func() (*viper.Viper, error) - retriever passphrase.Retriever - - // these are for command line parsing - no need to set - certRemoveGUN string - certRemoveYes bool -} - -func (c *certCommander) GetCommand() *cobra.Command { - cmd := cmdCertTemplate.ToCommand(nil) - cmd.AddCommand(cmdCertListTemplate.ToCommand(c.certList)) - - cmdCertRemove := cmdCertRemoveTemplate.ToCommand(c.certRemove) - cmdCertRemove.Flags().StringVarP( - &c.certRemoveGUN, "gun", "g", "", "Globally unique name to delete certificates for") - cmdCertRemove.Flags().BoolVarP( - &c.certRemoveYes, "yes", "y", false, "Answer yes to the removal question (no confirmation)") - - cmd.AddCommand(cmdCertRemove) - - return cmd -} - -// certRemove deletes a certificate given a cert ID or a gun -// If given a gun, certRemove will also remove local TUF data -func (c *certCommander) certRemove(cmd *cobra.Command, args []string) error { - // 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 - if (len(args) < 1 && c.certRemoveGUN == "") || (len(args) > 0 && c.certRemoveGUN != "") { - cmd.Usage() - return fmt.Errorf("Must specify the cert ID or the GUN of the certificates to remove") - } - config, err := c.configGetter() - if err != nil { - return err - } - - trustDir := config.GetString("trust_dir") - certPath := filepath.Join(trustDir, notary.TrustedCertsDir) - certStore, err := trustmanager.NewX509FilteredFileStore( - certPath, - trustmanager.FilterCertsExpiredSha1, - ) - if err != nil { - return fmt.Errorf("Failed to create a new truststore with directory: %s", trustDir) - } - - var certsToRemove []*x509.Certificate - var certFoundByID *x509.Certificate - var removeTrustData bool - - // If there is no GUN, we expect a cert ID - if c.certRemoveGUN == "" { - certID := args[0] - // 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 { - return fmt.Errorf("Unable to retrieve certificate with invalid certificate ID provided: %s", certID) - } - return fmt.Errorf("Unable to retrieve certificate with cert ID: %s", certID) - } - // the GUN is the CN from the certificate - c.certRemoveGUN = certFoundByID.Subject.CommonName - certsToRemove = []*x509.Certificate{certFoundByID} - } - - toRemove, err := certStore.GetCertificatesByCN(c.certRemoveGUN) - // We could not find any certificates matching the user's query, so propagate the error - if err != nil { - return fmt.Errorf("%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 - // x509 store that indexes by ID. - 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", c.certRemoveGUN) - } - cmd.Println("\nAre you sure you want to remove these certificates? (yes/no)") - - // Ask for confirmation before removing certificates, unless -y is provided - if !c.certRemoveYes { - confirmed := askConfirm(os.Stdin) - if !confirmed { - return fmt.Errorf("Aborting action.") - } - } - - 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 - trustPin, err := getTrustPinning(config) - if err != nil { - return err - } - nRepo, err := notaryclient.NewNotaryRepository( - trustDir, c.certRemoveGUN, getRemoteTrustServer(config), nil, c.retriever, trustPin) - if err != nil { - return fmt.Errorf("Could not establish trust data for GUN %s", c.certRemoveGUN) - } - // DeleteTrustData will pick up all of the same certificates by GUN (CN) and remove them - err = nRepo.DeleteTrustData() - if err != nil { - return fmt.Errorf("Failed to delete trust data for %s", c.certRemoveGUN) - } - } else { - for _, cert := range certsToRemove { - err = certStore.RemoveCert(cert) - if err != nil { - return fmt.Errorf("Failed to remove cert %s", cert) - } - } - } - return nil -} - -func (c *certCommander) certList(cmd *cobra.Command, args []string) error { - if len(args) > 0 { - cmd.Usage() - return fmt.Errorf("") - } - config, err := c.configGetter() - if err != nil { - return err - } - - trustDir := config.GetString("trust_dir") - certPath := filepath.Join(trustDir, notary.TrustedCertsDir) - // Load all individual (non-CA) certificates that aren't expired and don't use SHA1 - certStore, err := trustmanager.NewX509FilteredFileStore( - certPath, - trustmanager.FilterCertsExpiredSha1, - ) - if err != nil { - return fmt.Errorf("Failed to create a new truststore with directory: %s", trustDir) - } - - trustedCerts := certStore.GetCertificates() - - cmd.Println("") - prettyPrintCerts(trustedCerts, cmd.Out()) - cmd.Println("") - return nil -} diff --git a/cmd/notary/integration_test.go b/cmd/notary/integration_test.go index 2c2b84621f..52a2fe8ead 100644 --- a/cmd/notary/integration_test.go +++ b/cmd/notary/integration_test.go @@ -1346,98 +1346,6 @@ func TestClientKeyImportExportAllRoles(t *testing.T) { } } -func assertNumCerts(t *testing.T, tempDir string, expectedNum int) []string { - output, err := runCommand(t, tempDir, "cert", "list") - require.NoError(t, err) - lines := splitLines(strings.TrimSpace(output)) - - if expectedNum == 0 { - require.Len(t, lines, 1) - require.Equal(t, "No trusted root certificates present.", lines[0]) - return []string{} - } - - require.Len(t, lines, expectedNum+2) - return lines[2:] -} - -// TestClientCertInteraction -func TestClientCertInteraction(t *testing.T) { - // -- setup -- - setUp(t) - - tempDir := tempDirWithConfig(t, "{}") - defer os.RemoveAll(tempDir) - - server := setupServer() - defer server.Close() - - // -- tests -- - _, err := runCommand(t, tempDir, "-s", server.URL, "init", "gun1") - require.NoError(t, err) - _, err = runCommand(t, tempDir, "-s", server.URL, "init", "gun2") - require.NoError(t, err) - certs := assertNumCerts(t, tempDir, 2) - // root is always on disk, because even if there's a yubikey a backup is created - assertNumKeys(t, tempDir, 1, 4, true) - - // remove certs for one gun - _, err = runCommand(t, tempDir, "cert", "remove", "-g", "gun1", "-y") - require.NoError(t, err) - certs = assertNumCerts(t, tempDir, 1) - // assert that when we remove cert by gun, we do not remove repo signing keys - // (root is always on disk, because even if there's a yubikey a backup is created) - assertNumKeys(t, tempDir, 1, 4, true) - // assert that when we remove cert by gun, we also remove TUF metadata - _, err = os.Stat(filepath.Join(tempDir, "tuf", "gun1")) - require.Error(t, err) - - // remove a single cert - certID := strings.Fields(certs[0])[1] - // passing an empty gun here because the string for the previous gun has - // has already been stored (a drawback of running these commands without) - // shelling out - _, err = runCommand(t, tempDir, "cert", "remove", certID, "-y", "-g", "") - require.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")) - require.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) - require.NoError(t, err) - noGunCert, err := cryptoservice.GenerateCertificate( - privKey, "nonexistent", startTime, startTime.AddDate(10, 0, 0)) - require.NoError(t, err) - certStore, err := trustmanager.NewX509FileStore(filepath.Join(tempDir, "trusted_certificates")) - require.NoError(t, err) - err = certStore.AddCert(noGunCert) - require.NoError(t, err) - - certs = assertNumCerts(t, tempDir, 1) - certID = strings.Fields(certs[0])[1] - - privKey, err = trustmanager.GenerateECDSAKey(rand.Reader) - require.NoError(t, err) - noGunCert2, err := cryptoservice.GenerateCertificate( - privKey, "nonexistent", startTime, startTime.AddDate(10, 0, 0)) - require.NoError(t, err) - err = certStore.AddCert(noGunCert2) - require.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", "") - require.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 func TestDefaultRootKeyGeneration(t *testing.T) { // -- setup -- diff --git a/cmd/notary/main.go b/cmd/notary/main.go index 749771b3b6..a97fd3a4b5 100644 --- a/cmd/notary/main.go +++ b/cmd/notary/main.go @@ -180,11 +180,6 @@ func (n *notaryCommander) GetCommand() *cobra.Command { retriever: n.getRetriever(), } - cmdCertGenerator := &certCommander{ - configGetter: n.parseConfig, - retriever: n.getRetriever(), - } - cmdTufGenerator := &tufCommander{ configGetter: n.parseConfig, retriever: n.getRetriever(), @@ -192,7 +187,6 @@ func (n *notaryCommander) GetCommand() *cobra.Command { notaryCmd.AddCommand(cmdKeyGenerator.GetCommand()) notaryCmd.AddCommand(cmdDelegationGenerator.GetCommand()) - notaryCmd.AddCommand(cmdCertGenerator.GetCommand()) cmdTufGenerator.AddToCommand(¬aryCmd) diff --git a/cmd/notary/main_test.go b/cmd/notary/main_test.go index e11793414c..8c035f5464 100644 --- a/cmd/notary/main_test.go +++ b/cmd/notary/main_test.go @@ -164,8 +164,6 @@ var exampleValidCommands = []string{ "key import backup.pem", "key remove e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "key passwd e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", - "cert list", - "cert remove e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "delegation list repo", "delegation add repo targets/releases path/to/pem/file.pem", "delegation remove repo targets/releases", @@ -205,8 +203,8 @@ func TestInsufficientArgumentsReturnsErrorAndPrintsUsage(t *testing.T) { cmd.SetOutput(b) arglist := strings.Fields(args) - if args == "key list" || args == "cert list" || args == "key generate rsa" { - // in these case, "key" or "cert" or "key generate" are valid commands, so add an arg to them instead + if args == "key list" || args == "key generate rsa" { + // in these case, "key" or "key generate" are valid commands, so add an arg to them instead arglist = append(arglist, "extraArg") } else { arglist = arglist[:len(arglist)-1] @@ -240,8 +238,8 @@ func TestBareCommandPrintsUsageAndNoError(t *testing.T) { // usage is printed require.Contains(t, b.String(), "Usage:", "expected usage when running `notary`") - // notary key, notary cert, and notary delegation - for _, bareCommand := range []string{"key", "cert", "delegation"} { + // notary key and notary delegation + for _, bareCommand := range []string{"key", "delegation"} { b := new(bytes.Buffer) cmd := NewNotaryCommand() cmd.SetOutput(b) diff --git a/cmd/notary/prettyprint.go b/cmd/notary/prettyprint.go index 4bea6fd7ac..8f43d45825 100644 --- a/cmd/notary/prettyprint.go +++ b/cmd/notary/prettyprint.go @@ -1,14 +1,11 @@ package main import ( - "crypto/x509" "encoding/hex" "fmt" "io" - "math" "sort" "strings" - "time" "github.com/docker/notary/client" "github.com/docker/notary/trustmanager" @@ -204,52 +201,3 @@ func prettyPrintPaths(paths []string) string { } return strings.Join(prettyPaths, "\n") } - -// --- pretty printing certs --- - -// cert by repo name then expiry time. Don't bother sorting by fingerprint. -type certSorter []*x509.Certificate - -func (t certSorter) Len() int { return len(t) } -func (t certSorter) Swap(i, j int) { t[i], t[j] = t[j], t[i] } -func (t certSorter) Less(i, j int) bool { - if t[i].Subject.CommonName < t[j].Subject.CommonName { - return true - } else if t[i].Subject.CommonName > t[j].Subject.CommonName { - return false - } - - return t[i].NotAfter.Before(t[j].NotAfter) -} - -// Given a list of Ceritifcates in order of listing preference, pretty-prints -// the cert common name, fingerprint, and expiry -func prettyPrintCerts(certs []*x509.Certificate, writer io.Writer) { - if len(certs) == 0 { - writer.Write([]byte("\nNo trusted root certificates present.\n\n")) - return - } - - sort.Stable(certSorter(certs)) - - table := getTable([]string{ - "GUN", "Fingerprint of Trusted Root Certificate", "Expires In"}, writer) - - for _, c := range certs { - days := math.Floor(c.NotAfter.Sub(time.Now()).Hours() / 24) - expiryString := "< 1 day" - if days == 1 { - expiryString = "1 day" - } else if days > 1 { - expiryString = fmt.Sprintf("%d days", int(days)) - } - - certID, err := trustmanager.FingerprintCert(c) - if err != nil { - fatalf("Could not fingerprint certificate: %v", err) - } - - table.Append([]string{c.Subject.CommonName, certID, expiryString}) - } - table.Render() -} diff --git a/cmd/notary/prettyprint_test.go b/cmd/notary/prettyprint_test.go index 20ad8999d8..4cde238919 100644 --- a/cmd/notary/prettyprint_test.go +++ b/cmd/notary/prettyprint_test.go @@ -3,7 +3,6 @@ package main import ( "bytes" "crypto/rand" - "crypto/x509" "encoding/hex" "fmt" "io/ioutil" @@ -11,10 +10,8 @@ import ( "sort" "strings" "testing" - "time" "github.com/docker/notary/client" - "github.com/docker/notary/cryptoservice" "github.com/docker/notary/passphrase" "github.com/docker/notary/trustmanager" "github.com/docker/notary/tuf/data" @@ -201,19 +198,6 @@ func TestPrettyPrintSortedTargets(t *testing.T) { } } -// --- tests for pretty printing certs --- - -func generateCertificate(t *testing.T, gun string, expireInHours int64) *x509.Certificate { - ecdsaPrivKey, err := trustmanager.GenerateECDSAKey(rand.Reader) - require.NoError(t, err) - - startTime := time.Now() - endTime := startTime.Add(time.Hour * time.Duration(expireInHours)) - cert, err := cryptoservice.GenerateCertificate(ecdsaPrivKey, gun, startTime, endTime) - require.NoError(t, err) - return cert -} - // --- tests for pretty printing roles --- // If there are no roles, no table is printed, only a line saying that there @@ -268,53 +252,3 @@ func TestPrettyPrintSortedRoles(t *testing.T) { require.Equal(t, expected[i], splitted) } } - -// If there are no certs in the cert store store, a message that there are no -// certs should be displayed. -func TestPrettyPrintZeroCerts(t *testing.T) { - var b bytes.Buffer - prettyPrintCerts([]*x509.Certificate{}, &b) - text, err := ioutil.ReadAll(&b) - require.NoError(t, err) - - lines := strings.Split(strings.TrimSpace(string(text)), "\n") - require.Len(t, lines, 1) - require.Equal(t, "No trusted root certificates present.", lines[0]) -} - -// Certificates are pretty-printed in table form sorted by gun and then expiry -func TestPrettyPrintSortedCerts(t *testing.T) { - unsorted := []*x509.Certificate{ - generateCertificate(t, "xylitol", 77), // 3 days 5 hours - generateCertificate(t, "xylitol", 12), // less than 1 day - generateCertificate(t, "cheesecake", 25), // a little more than 1 day - generateCertificate(t, "baklava", 239), // almost 10 days - } - - var b bytes.Buffer - prettyPrintCerts(unsorted, &b) - text, err := ioutil.ReadAll(&b) - require.NoError(t, err) - - expected := [][]string{ - {"baklava", "9 days"}, - {"cheesecake", "1 day"}, - {"xylitol", "< 1 day"}, - {"xylitol", "3 days"}, - } - - lines := strings.Split(strings.TrimSpace(string(text)), "\n") - require.Len(t, lines, len(expected)+2) - - // starts with headers - require.True(t, reflect.DeepEqual(strings.Fields(lines[0]), strings.Fields( - "GUN FINGERPRINT OF TRUSTED ROOT CERTIFICATE EXPIRES IN"))) - require.Equal(t, "----", lines[1][:4]) - - for i, line := range lines[2:] { - splitted := strings.Fields(line) - require.True(t, len(splitted) >= 3) - require.Equal(t, expected[i][0], splitted[0]) - require.Equal(t, expected[i][1], strings.Join(splitted[2:], " ")) - } -} diff --git a/trustpinning/certs_test.go b/trustpinning/certs_test.go index f1081da66e..8f59de920c 100644 --- a/trustpinning/certs_test.go +++ b/trustpinning/certs_test.go @@ -568,12 +568,41 @@ func testValidateSuccessfulRootRotation(t *testing.T, keyAlg, rootKeyType string origRootCert := certificates[0] replRootCert := certificates[1] - // We need the PEM representation of the replacement key to put it into the TUF data + // Set up the previous root prior to rotating + // We need the PEM representation of the original key to put it into the TUF data origRootPEMCert := trustmanager.CertToPEM(origRootCert) - replRootPEMCert := trustmanager.CertToPEM(replRootCert) // Tuf key with PEM-encoded x509 certificate origRootKey := data.NewPublicKey(rootKeyType, origRootPEMCert) + + origRootRole, err := data.NewRole(data.CanonicalRootRole, 1, []string{origRootKey.ID()}, nil) + require.NoError(t, err) + + origTestRoot, err := data.NewRoot( + map[string]data.PublicKey{origRootKey.ID(): origRootKey}, + map[string]*data.RootRole{ + data.CanonicalRootRole: &origRootRole.RootRole, + data.CanonicalTargetsRole: &origRootRole.RootRole, + data.CanonicalSnapshotRole: &origRootRole.RootRole, + data.CanonicalTimestampRole: &origRootRole.RootRole, + }, + false, + ) + require.NoError(t, err, "Failed to create new root") + + signedOrigTestRoot, err := origTestRoot.ToSigned() + require.NoError(t, err) + + // We only sign with the new key, and not with the original one. + err = signed.Sign(cs, signedOrigTestRoot, []data.PublicKey{origRootKey}, 1, nil) + require.NoError(t, err) + prevRoot, err := data.RootFromSigned(signedOrigTestRoot) + require.NoError(t, err) + + // We need the PEM representation of the replacement key to put it into the TUF data + replRootPEMCert := trustmanager.CertToPEM(replRootCert) + + // Tuf key with PEM-encoded x509 certificate replRootKey := data.NewPublicKey(rootKeyType, replRootPEMCert) rootRole, err := data.NewRole(data.CanonicalRootRole, 1, []string{replRootKey.ID()}, nil) @@ -598,7 +627,7 @@ func testValidateSuccessfulRootRotation(t *testing.T, keyAlg, rootKeyType string // This call to ValidateRoot will succeed since we are using a valid PEM // encoded certificate, and have no other certificates for this CN - err = ValidateRoot(nil, signedTestRoot, gun, TrustPinConfig{}) + err = ValidateRoot(prevRoot, signedTestRoot, gun, TrustPinConfig{}) require.NoError(t, err) }