Skip to content
New issue

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

#

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

Already on GitHub? # to your account

#806 Part 2: Cleanup KeyStore Implementations #808

Closed
wants to merge 2 commits into from
Closed

#806 Part 2: Cleanup KeyStore Implementations #808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08be749
tear out import/export
endophage Jun 28, 2016
1e67617
unify KeyMemoryStore and KeyFileStore in trustmanager
Jun 29, 2016
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
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/.vscode
/cmd/notary-server/notary-server
/cmd/notary-server/local.config.json
/cmd/notary-signer/local.config.json
Expand All @@ -8,4 +9,5 @@ cross
*.swp
.idea
*.iml
*.test
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm good with ignoring this file extension, but out of curiosity what is it used for? VSCode metadata?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, for this workspace I think

On Friday, July 8, 2016, Riyaz Faizullabhoy notifications@github.com
wrote:

In .gitignore
#808 (comment):

@@ -8,4 +9,5 @@ cross
.swp
.idea
*.iml
+.test

I'm good with ignoring this file extension, but out of curiosity what is
it used for? VSCode metadata?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/docker/notary/pull/808/files/1ff0f8c416206fbd7f9bf80bda0c3a7c516a6fa1#r70102210,
or mute the thread
https://github.com/notifications/unsubscribe/AANn4PQdmoXN7_9X4sCTr9RvCb889fxFks5qTnsCgaJpZM4JAytD
.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It appears to be per package rather than for the workspace as a whole. I need to find if there's some VSCode config option that lets me write them all to some other location (like the vim option that let's you put all the ~ files in a specific place) because the files are really annoying.

coverage.out
4 changes: 2 additions & 2 deletions client/backwards_compatibility_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import (
"time"

"github.com/docker/notary/passphrase"
store "github.com/docker/notary/storage"
"github.com/docker/notary/trustpinning"
"github.com/docker/notary/tuf/data"
"github.com/docker/notary/tuf/store"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -101,7 +101,7 @@ func Test0Dot1RepoFormat(t *testing.T) {

// delete the timestamp metadata, since the server will ignore the uploaded
// one and try to create a new one from scratch, which will be the wrong version
require.NoError(t, repo.fileStore.RemoveMeta(data.CanonicalTimestampRole))
require.NoError(t, repo.fileStore.Remove(data.CanonicalTimestampRole))

// rotate the timestamp key, since the server doesn't have that one
err = repo.RotateKey(data.CanonicalTimestampRole, true)
Expand Down
20 changes: 10 additions & 10 deletions client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ import (
"github.com/docker/notary"
"github.com/docker/notary/client/changelist"
"github.com/docker/notary/cryptoservice"
store "github.com/docker/notary/storage"
"github.com/docker/notary/trustmanager"
"github.com/docker/notary/trustpinning"
"github.com/docker/notary/tuf"
tufclient "github.com/docker/notary/tuf/client"
"github.com/docker/notary/tuf/data"
"github.com/docker/notary/tuf/signed"
"github.com/docker/notary/tuf/store"
"github.com/docker/notary/tuf/utils"
)

Expand Down Expand Up @@ -159,7 +159,7 @@ func rootCertKey(gun string, privKey data.PrivateKey) (data.PublicKey, error) {
return nil, err
}

x509PublicKey := trustmanager.CertToKey(cert)
x509PublicKey := utils.CertToKey(cert)
if x509PublicKey == nil {
return nil, fmt.Errorf(
"cannot use regenerated certificate: format %s", cert.PublicKeyAlgorithm)
Expand Down Expand Up @@ -676,7 +676,7 @@ func (r *NotaryRepository) publish(cl changelist.Changelist) error {
return err
}

return remote.SetMultiMeta(updatedFiles)
return remote.SetMulti(updatedFiles)
}

// bootstrapRepo loads the repository from the local file system (i.e.
Expand All @@ -690,7 +690,7 @@ func (r *NotaryRepository) bootstrapRepo() error {
logrus.Debugf("Loading trusted collection.")

for _, role := range data.BaseRoles {
jsonBytes, err := r.fileStore.GetMeta(role, store.NoSizeLimit)
jsonBytes, err := r.fileStore.GetSized(role, store.NoSizeLimit)
Copy link
Contributor

Choose a reason for hiding this comment

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

we can do this in a separate PR, but should we phase out store.NoSizeLimit and change these calls to Get() instead of GetSized()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I figured we'd do that in a different PR. There's already so much in here and that's an important change with meaningful TUF security implications if we change it somewhere we shouldn't, so a smaller PR with just that seems more sensible.

if err != nil {
if _, ok := err.(store.ErrMetaNotFound); ok &&
// server snapshots are supported, and server timestamp management
Expand Down Expand Up @@ -722,7 +722,7 @@ func (r *NotaryRepository) saveMetadata(ignoreSnapshot bool) error {
if err != nil {
return err
}
err = r.fileStore.SetMeta(data.CanonicalRootRole, rootJSON)
err = r.fileStore.Set(data.CanonicalRootRole, rootJSON)
if err != nil {
return err
}
Expand All @@ -743,7 +743,7 @@ func (r *NotaryRepository) saveMetadata(ignoreSnapshot bool) error {
for role, blob := range targetsToSave {
parentDir := filepath.Dir(role)
os.MkdirAll(parentDir, 0755)
r.fileStore.SetMeta(role, blob)
r.fileStore.Set(role, blob)
}

if ignoreSnapshot {
Expand All @@ -755,7 +755,7 @@ func (r *NotaryRepository) saveMetadata(ignoreSnapshot bool) error {
return err
}

return r.fileStore.SetMeta(data.CanonicalSnapshotRole, snapshotJSON)
return r.fileStore.Set(data.CanonicalSnapshotRole, snapshotJSON)
}

// returns a properly constructed ErrRepositoryNotExist error based on this
Expand Down Expand Up @@ -825,7 +825,7 @@ func (r *NotaryRepository) bootstrapClient(checkInitialized bool) (*tufclient.Cl
// during update which will cause us to download a new root and perform a rotation.
// If we have an old root, and it's valid, then we overwrite the newBuilder to be one
// preloaded with the old root or one which uses the old root for trust bootstrapping.
if rootJSON, err := r.fileStore.GetMeta(data.CanonicalRootRole, store.NoSizeLimit); err == nil {
if rootJSON, err := r.fileStore.GetSized(data.CanonicalRootRole, store.NoSizeLimit); err == nil {
// if we can't load the cached root, fail hard because that is how we pin trust
if err := oldBuilder.Load(data.CanonicalRootRole, rootJSON, minVersion, true); err != nil {
return nil, err
Expand All @@ -852,7 +852,7 @@ func (r *NotaryRepository) bootstrapClient(checkInitialized bool) (*tufclient.Cl

// if remote store successfully set up, try and get root from remote
// We don't have any local data to determine the size of root, so try the maximum (though it is restricted at 100MB)
tmpJSON, err := remote.GetMeta(data.CanonicalRootRole, store.NoSizeLimit)
tmpJSON, err := remote.GetSized(data.CanonicalRootRole, store.NoSizeLimit)
if err != nil {
// we didn't have a root in cache and were unable to load one from
// the server. Nothing we can do but error.
Expand All @@ -865,7 +865,7 @@ func (r *NotaryRepository) bootstrapClient(checkInitialized bool) (*tufclient.Cl
return nil, err
}

err = r.fileStore.SetMeta(data.CanonicalRootRole, tmpJSON)
err = r.fileStore.Set(data.CanonicalRootRole, tmpJSON)
if err != nil {
// if we can't write cache we should still continue, just log error
logrus.Errorf("could not save root to cache: %s", err.Error())
Expand Down
12 changes: 6 additions & 6 deletions client/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ import (
"github.com/docker/notary/passphrase"
"github.com/docker/notary/server"
"github.com/docker/notary/server/storage"
store "github.com/docker/notary/storage"
"github.com/docker/notary/trustmanager"
"github.com/docker/notary/trustpinning"
"github.com/docker/notary/tuf/data"
"github.com/docker/notary/tuf/signed"
"github.com/docker/notary/tuf/store"
"github.com/docker/notary/tuf/utils"
"github.com/docker/notary/tuf/validation"
)
Expand Down Expand Up @@ -102,7 +102,7 @@ func simpleTestServer(t *testing.T, roles ...string) (
mux := http.NewServeMux()

for _, role := range roles {
key, err := trustmanager.GenerateECDSAKey(rand.Reader)
key, err := utils.GenerateECDSAKey(rand.Reader)
require.NoError(t, err)

keys[role] = key
Expand Down Expand Up @@ -1539,7 +1539,7 @@ func testValidateRootKey(t *testing.T, rootType string) {
for _, keyid := range keyids {
key, ok := decodedRoot.Keys[keyid]
require.True(t, ok, "key id not found in keys")
_, err := trustmanager.LoadCertFromPEM(key.Public())
_, err := utils.LoadCertFromPEM(key.Public())
require.NoError(t, err, "key is not a valid cert")
}
}
Expand Down Expand Up @@ -1957,7 +1957,7 @@ func testPublishBadMetadata(t *testing.T, roleName string, repo *NotaryRepositor
addTarget(t, repo, "v1", "../fixtures/intermediate-ca.crt")

// readable, but corrupt file
repo.fileStore.SetMeta(roleName, []byte("this isn't JSON"))
repo.fileStore.Set(roleName, []byte("this isn't JSON"))
err := repo.Publish()
if succeeds {
require.NoError(t, err)
Expand Down Expand Up @@ -2054,7 +2054,7 @@ func createKey(t *testing.T, repo *NotaryRepository, role string, x509 bool) dat
privKey, role, start, start.AddDate(1, 0, 0),
)
require.NoError(t, err)
return data.NewECDSAx509PublicKey(trustmanager.CertToPEM(cert))
return data.NewECDSAx509PublicKey(utils.CertToPEM(cert))
}
return key
}
Expand Down Expand Up @@ -2170,7 +2170,7 @@ func testPublishTargetsDelegationScopeFailIfNoKeys(t *testing.T, clearCache bool

// generate a key that isn't in the cryptoservice, so we can't sign this
// one
aPrivKey, err := trustmanager.GenerateECDSAKey(rand.Reader)
aPrivKey, err := utils.GenerateECDSAKey(rand.Reader)
require.NoError(t, err, "error generating key that is not in our cryptoservice")
aPubKey := data.PublicKeyFromPrivate(aPrivKey)

Expand Down
32 changes: 16 additions & 16 deletions client/client_update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ import (
"github.com/docker/go/canonical/json"
"github.com/docker/notary"
"github.com/docker/notary/passphrase"
store "github.com/docker/notary/storage"
"github.com/docker/notary/trustpinning"
"github.com/docker/notary/tuf/data"
"github.com/docker/notary/tuf/signed"
"github.com/docker/notary/tuf/store"
"github.com/docker/notary/tuf/testutils"
"github.com/gorilla/mux"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -66,7 +66,7 @@ func readOnlyServer(t *testing.T, cache store.MetadataStore, notFoundStatus int,
m := mux.NewRouter()
handler := func(w http.ResponseWriter, r *http.Request) {
vars := mux.Vars(r)
metaBytes, err := cache.GetMeta(vars["role"], store.NoSizeLimit)
metaBytes, err := cache.GetSized(vars["role"], store.NoSizeLimit)
if _, ok := err.(store.ErrMetaNotFound); ok {
w.WriteHeader(notFoundStatus)
} else {
Expand All @@ -84,11 +84,11 @@ type unwritableStore struct {
roleToNotWrite string
}

func (u *unwritableStore) SetMeta(role string, serverMeta []byte) error {
func (u *unwritableStore) Set(role string, serverMeta []byte) error {
if role == u.roleToNotWrite {
return fmt.Errorf("Non-writable")
}
return u.MetadataStore.SetMeta(role, serverMeta)
return u.MetadataStore.Set(role, serverMeta)
}

// Update can succeed even if we cannot write any metadata to the repo (assuming
Expand All @@ -111,7 +111,7 @@ func TestUpdateSucceedsEvenIfCannotWriteNewRepo(t *testing.T) {
require.NoError(t, err)

for r, expected := range serverMeta {
actual, err := repo.fileStore.GetMeta(r, store.NoSizeLimit)
actual, err := repo.fileStore.GetSized(r, store.NoSizeLimit)
if r == role {
require.Error(t, err)
require.IsType(t, store.ErrMetaNotFound{}, err,
Expand Down Expand Up @@ -158,7 +158,7 @@ func TestUpdateSucceedsEvenIfCannotWriteExistingRepo(t *testing.T) {
require.NoError(t, err)

for r, expected := range serverMeta {
actual, err := repo.fileStore.GetMeta(r, store.NoSizeLimit)
actual, err := repo.fileStore.GetSized(r, store.NoSizeLimit)
require.NoError(t, err, "problem getting repo metadata for %s", r)
if role == r {
require.False(t, bytes.Equal(expected, actual),
Expand Down Expand Up @@ -244,12 +244,12 @@ func TestUpdateReplacesCorruptOrMissingMetadata(t *testing.T) {
require.Error(t, err, "%s for %s: expected to error when bootstrapping root", text, role)
// revert our original metadata
for role := range origMeta {
require.NoError(t, repo.fileStore.SetMeta(role, origMeta[role]))
require.NoError(t, repo.fileStore.Set(role, origMeta[role]))
}
} else {
require.NoError(t, err)
for r, expected := range serverMeta {
actual, err := repo.fileStore.GetMeta(r, store.NoSizeLimit)
actual, err := repo.fileStore.GetSized(r, store.NoSizeLimit)
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)
Expand Down Expand Up @@ -298,7 +298,7 @@ func TestUpdateFailsIfServerRootKeyChangedWithoutMultiSign(t *testing.T) {
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, store.NoSizeLimit)
messedUpMeta, err := repo.fileStore.GetSized(data.CanonicalRootRole, store.NoSizeLimit)

if _, ok := err.(store.ErrMetaNotFound); ok { // one of the ways to mess up is to delete metadata

Expand All @@ -307,7 +307,7 @@ func TestUpdateFailsIfServerRootKeyChangedWithoutMultiSign(t *testing.T) {
require.NoError(t, err)
// revert our original metadata
for role := range origMeta {
require.NoError(t, repo.fileStore.SetMeta(role, origMeta[role]))
require.NoError(t, repo.fileStore.Set(role, origMeta[role]))
}
} else {

Expand All @@ -321,7 +321,7 @@ func TestUpdateFailsIfServerRootKeyChangedWithoutMultiSign(t *testing.T) {
// 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, store.NoSizeLimit)
actual, err := repo.fileStore.GetSized(role, store.NoSizeLimit)
require.NoError(t, err, "problem getting repo metadata for %s", role)

if role == data.CanonicalRootRole {
Expand All @@ -336,7 +336,7 @@ func TestUpdateFailsIfServerRootKeyChangedWithoutMultiSign(t *testing.T) {

// revert our original root metadata
require.NoError(t,
repo.fileStore.SetMeta(data.CanonicalRootRole, origMeta[data.CanonicalRootRole]))
repo.fileStore.Set(data.CanonicalRootRole, origMeta[data.CanonicalRootRole]))
}
}
}
Expand Down Expand Up @@ -967,7 +967,7 @@ func waysToMessUpServerNonRootPerRole(t *testing.T) map[string][]swizzleExpectat
keyIDs = append(keyIDs, k)
}
// add the keys from root too
rootMeta, err := s.MetadataCache.GetMeta(data.CanonicalRootRole, store.NoSizeLimit)
rootMeta, err := s.MetadataCache.GetSized(data.CanonicalRootRole, store.NoSizeLimit)
require.NoError(t, err)

signedRoot := &data.SignedRoot{}
Expand Down Expand Up @@ -1349,7 +1349,7 @@ func signSerializeAndUpdateRoot(t *testing.T, signedRoot data.SignedRoot,
require.NoError(t, signed.Sign(serverSwizzler.CryptoService, signedObj, keys, len(keys), nil))
rootBytes, err := json.Marshal(signedObj)
require.NoError(t, err)
require.NoError(t, serverSwizzler.MetadataCache.SetMeta(data.CanonicalRootRole, rootBytes))
require.NoError(t, serverSwizzler.MetadataCache.Set(data.CanonicalRootRole, rootBytes))

// update the hashes on both snapshot and timestamp
require.NoError(t, serverSwizzler.UpdateSnapshotHashes())
Expand All @@ -1374,7 +1374,7 @@ func TestValidateRootRotationWithOldRole(t *testing.T) {
// --- key is saved, but doesn't matter at all for rotation if we're already on
// --- the root metadata with the 3 keys)

rootBytes, err := serverSwizzler.MetadataCache.GetMeta(data.CanonicalRootRole, store.NoSizeLimit)
rootBytes, err := serverSwizzler.MetadataCache.GetSized(data.CanonicalRootRole, store.NoSizeLimit)
require.NoError(t, err)
signedRoot := data.SignedRoot{}
require.NoError(t, json.Unmarshal(rootBytes, &signedRoot))
Expand Down Expand Up @@ -1626,7 +1626,7 @@ func TestRootOnDiskTrustPinning(t *testing.T) {
defer os.RemoveAll(repo.baseDir)
repo.trustPinning = restrictiveTrustPinning
// put root on disk
require.NoError(t, repo.fileStore.SetMeta(data.CanonicalRootRole, meta[data.CanonicalRootRole]))
require.NoError(t, repo.fileStore.Set(data.CanonicalRootRole, meta[data.CanonicalRootRole]))

require.NoError(t, repo.Update(false))
}
2 changes: 1 addition & 1 deletion client/delegations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"github.com/Sirupsen/logrus"
"github.com/docker/notary"
"github.com/docker/notary/client/changelist"
store "github.com/docker/notary/storage"
"github.com/docker/notary/tuf/data"
"github.com/docker/notary/tuf/store"
"github.com/docker/notary/tuf/utils"
)

Expand Down
4 changes: 2 additions & 2 deletions client/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import (

"github.com/Sirupsen/logrus"
"github.com/docker/notary/client/changelist"
tuf "github.com/docker/notary/tuf"
store "github.com/docker/notary/storage"
"github.com/docker/notary/tuf"
"github.com/docker/notary/tuf/data"
"github.com/docker/notary/tuf/store"
"github.com/docker/notary/tuf/utils"
)

Expand Down
3 changes: 1 addition & 2 deletions cmd/notary/delegations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

"github.com/docker/notary"
notaryclient "github.com/docker/notary/client"
"github.com/docker/notary/trustmanager"
"github.com/docker/notary/tuf/data"
"github.com/docker/notary/tuf/utils"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -238,7 +237,7 @@ func (d *delegationCommander) delegationAdd(cmd *cobra.Command, args []string) e
}

// Parse PEM bytes into type PublicKey
pubKey, err := trustmanager.ParsePEMPublicKey(pubKeyBytes)
pubKey, err := utils.ParsePEMPublicKey(pubKeyBytes)
if err != nil {
return fmt.Errorf("unable to parse valid public key certificate from PEM file %s: %v", pubKeyPath, err)
}
Expand Down
Loading