From 6bdc90019cb0c5b10bd04c1a69f27e93b30bf053 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Fri, 8 Jul 2016 13:30:18 -0700 Subject: [PATCH 1/3] Only use delegation passphrase env var for non-base roles Signed-off-by: Riyaz Faizullabhoy --- cmd/notary/main.go | 3 ++- cmd/notary/main_test.go | 28 +++++++++++++++++++++++----- tuf/data/roles.go | 10 ++++++++++ tuf/data/roles_test.go | 11 +++++++++++ 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/cmd/notary/main.go b/cmd/notary/main.go index 796a564f6..252dd7bd1 100644 --- a/cmd/notary/main.go +++ b/cmd/notary/main.go @@ -10,6 +10,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/notary" "github.com/docker/notary/passphrase" + "github.com/docker/notary/tuf/data" "github.com/docker/notary/version" homedir "github.com/mitchellh/go-homedir" "github.com/spf13/cobra" @@ -234,7 +235,7 @@ func getPassphraseRetriever() notary.PassRetriever { // For delegation roles, we can also try the "delegation" alias if it is specified // Note that we don't check if the role name is for a delegation to allow for names like "user" // since delegation keys can be shared across repositories - if v := env["delegation"]; v != "" { + if v := env["delegation"]; !data.IsBaseRole(alias) && v != "" { return v, numAttempts > 1, nil } return baseRetriever(keyName, alias, createNew, numAttempts) diff --git a/cmd/notary/main_test.go b/cmd/notary/main_test.go index 82683554e..ddf743e64 100644 --- a/cmd/notary/main_test.go +++ b/cmd/notary/main_test.go @@ -523,16 +523,34 @@ func TestConfigFileTrustPinning(t *testing.T) { } func TestPassphraseRetrieverCaching(t *testing.T) { - // Set up passphrase environment vars + // Only set up one passphrase environment var first for root require.NoError(t, os.Setenv("NOTARY_ROOT_PASSPHRASE", "root_passphrase")) + defer os.Clearenv() + + // Check that root is cached + retriever := getPassphraseRetriever() + passphrase, giveup, err := retriever("key", data.CanonicalRootRole, false, 0) + require.NoError(t, err) + require.False(t, giveup) + require.Equal(t, passphrase, "root_passphrase") + + passphrase, giveup, err = retriever("key", "user", false, 0) + require.Error(t, err) + passphrase, giveup, err = retriever("key", data.CanonicalTargetsRole, false, 0) + require.Error(t, err) + passphrase, giveup, err = retriever("key", data.CanonicalSnapshotRole, false, 0) + require.Error(t, err) + passphrase, giveup, err = retriever("key", "targets/delegation", false, 0) + require.Error(t, err) + + // Set up the rest of them require.NoError(t, os.Setenv("NOTARY_TARGETS_PASSPHRASE", "targets_passphrase")) require.NoError(t, os.Setenv("NOTARY_SNAPSHOT_PASSPHRASE", "snapshot_passphrase")) require.NoError(t, os.Setenv("NOTARY_DELEGATION_PASSPHRASE", "delegation_passphrase")) - defer os.Clearenv() - // Check the caching - retriever := getPassphraseRetriever() - passphrase, giveup, err := retriever("key", data.CanonicalRootRole, false, 0) + // Get a new retriever and check the caching + retriever = getPassphraseRetriever() + passphrase, giveup, err = retriever("key", data.CanonicalRootRole, false, 0) require.NoError(t, err) require.False(t, giveup) require.Equal(t, passphrase, "root_passphrase") diff --git a/tuf/data/roles.go b/tuf/data/roles.go index caa2e91c1..742242efb 100644 --- a/tuf/data/roles.go +++ b/tuf/data/roles.go @@ -86,6 +86,16 @@ func IsDelegation(role string) bool { isClean } +// IsBaseRole checks if the role is a base role +func IsBaseRole(role string) bool { + for _, baseRole := range BaseRoles { + if role == baseRole { + return true + } + } + return false +} + // BaseRole is an internal representation of a root/targets/snapshot/timestamp role, with its public keys included type BaseRole struct { Keys map[string]PublicKey diff --git a/tuf/data/roles_test.go b/tuf/data/roles_test.go index 7c622febb..594060c4e 100644 --- a/tuf/data/roles_test.go +++ b/tuf/data/roles_test.go @@ -165,6 +165,17 @@ func TestValidRoleFunction(t *testing.T) { require.False(t, ValidRole(path.Join("role"))) } +func TestIsBaseRole(t *testing.T) { + for _, role := range BaseRoles { + require.True(t, IsBaseRole(role)) + } + require.False(t, IsBaseRole("user")) + require.False(t, IsBaseRole( + path.Join(CanonicalTargetsRole, "level1", "level2", "level3"))) + require.False(t, IsBaseRole(path.Join(CanonicalTargetsRole, "level1"))) + require.False(t, IsBaseRole("")) +} + func TestBaseRoleEquals(t *testing.T) { fakeKeyHello := NewRSAPublicKey([]byte("hello")) fakeKeyThere := NewRSAPublicKey([]byte("there")) From c6b5876cc965410d5fbaf9badbb0281d8599e3c3 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Fri, 8 Jul 2016 14:35:28 -0700 Subject: [PATCH 2/3] Also do not include imported roles Signed-off-by: Riyaz Faizullabhoy --- cmd/notary/main.go | 3 ++- cmd/notary/main_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/cmd/notary/main.go b/cmd/notary/main.go index 252dd7bd1..85eeb781c 100644 --- a/cmd/notary/main.go +++ b/cmd/notary/main.go @@ -235,7 +235,8 @@ func getPassphraseRetriever() notary.PassRetriever { // For delegation roles, we can also try the "delegation" alias if it is specified // Note that we don't check if the role name is for a delegation to allow for names like "user" // since delegation keys can be shared across repositories - if v := env["delegation"]; !data.IsBaseRole(alias) && v != "" { + // This cannot be a base role or imported key, though. + if v := env["delegation"]; !data.IsBaseRole(alias) && !strings.Contains(alias, "imported ") && v != "" { return v, numAttempts > 1, nil } return baseRetriever(keyName, alias, createNew, numAttempts) diff --git a/cmd/notary/main_test.go b/cmd/notary/main_test.go index ddf743e64..436d68657 100644 --- a/cmd/notary/main_test.go +++ b/cmd/notary/main_test.go @@ -576,3 +576,43 @@ func TestPassphraseRetrieverCaching(t *testing.T) { require.False(t, giveup) require.Equal(t, passphrase, "delegation_passphrase") } + +func TestPassphraseRetrieverDelegationRoleCaching(t *testing.T) { + // Only set up one passphrase environment var first for delegations + require.NoError(t, os.Setenv("NOTARY_DELEGATION_PASSPHRASE", "delegation_passphrase")) + defer os.Clearenv() + + // Check that any delegation role is cached + retriever := getPassphraseRetriever() + + passphrase, giveup, err := retriever("key", "targets/releases", false, 0) + require.NoError(t, err) + require.False(t, giveup) + require.Equal(t, passphrase, "delegation_passphrase") + passphrase, giveup, err = retriever("key", "targets/delegation", false, 0) + require.NoError(t, err) + require.False(t, giveup) + require.Equal(t, passphrase, "delegation_passphrase") + passphrase, giveup, err = retriever("key", "targets/a/b/c/d", false, 0) + require.NoError(t, err) + require.False(t, giveup) + require.Equal(t, passphrase, "delegation_passphrase") + + // Also check arbitrary usernames that are non-BaseRoles or imported so that this can be shared across keys + passphrase, giveup, err = retriever("key", "user", false, 0) + require.NoError(t, err) + require.False(t, giveup) + require.Equal(t, passphrase, "delegation_passphrase") + + // Make sure base roles fail + passphrase, giveup, err = retriever("key", data.CanonicalRootRole, false, 0) + require.Error(t, err) + passphrase, giveup, err = retriever("key", data.CanonicalTargetsRole, false, 0) + require.Error(t, err) + passphrase, giveup, err = retriever("key", data.CanonicalSnapshotRole, false, 0) + require.Error(t, err) + + // make sure "imported" role fails + passphrase, giveup, err = retriever("key", "imported "+data.CanonicalRootRole, false, 0) + require.Error(t, err) +} From 7a98a074d07d0e1c1a1d5983753a7f6d26e4ecbc Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Tue, 19 Jul 2016 14:54:07 -0700 Subject: [PATCH 3/3] Remove imported role constant with new keystore Signed-off-by: Riyaz Faizullabhoy --- cmd/notary/main.go | 2 +- cmd/notary/main_test.go | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/cmd/notary/main.go b/cmd/notary/main.go index 85eeb781c..bdd7ebc6e 100644 --- a/cmd/notary/main.go +++ b/cmd/notary/main.go @@ -236,7 +236,7 @@ func getPassphraseRetriever() notary.PassRetriever { // Note that we don't check if the role name is for a delegation to allow for names like "user" // since delegation keys can be shared across repositories // This cannot be a base role or imported key, though. - if v := env["delegation"]; !data.IsBaseRole(alias) && !strings.Contains(alias, "imported ") && v != "" { + if v := env["delegation"]; !data.IsBaseRole(alias) && v != "" { return v, numAttempts > 1, nil } return baseRetriever(keyName, alias, createNew, numAttempts) diff --git a/cmd/notary/main_test.go b/cmd/notary/main_test.go index 436d68657..c07ca3680 100644 --- a/cmd/notary/main_test.go +++ b/cmd/notary/main_test.go @@ -605,14 +605,10 @@ func TestPassphraseRetrieverDelegationRoleCaching(t *testing.T) { require.Equal(t, passphrase, "delegation_passphrase") // Make sure base roles fail - passphrase, giveup, err = retriever("key", data.CanonicalRootRole, false, 0) + _, _, err = retriever("key", data.CanonicalRootRole, false, 0) require.Error(t, err) - passphrase, giveup, err = retriever("key", data.CanonicalTargetsRole, false, 0) + _, _, err = retriever("key", data.CanonicalTargetsRole, false, 0) require.Error(t, err) - passphrase, giveup, err = retriever("key", data.CanonicalSnapshotRole, false, 0) - require.Error(t, err) - - // make sure "imported" role fails - passphrase, giveup, err = retriever("key", "imported "+data.CanonicalRootRole, false, 0) + _, _, err = retriever("key", data.CanonicalSnapshotRole, false, 0) require.Error(t, err) }