Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Be able to read keys even if they are not in the root/non-root subdirs #981

Merged
merged 2 commits into from
Sep 29, 2016
Merged

Be able to read keys even if they are not in the root/non-root subdirs #981

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
896fbde
Be able to read keys even if they are not in the root/non-root subdirs
cyli Sep 28, 2016
128bf0c
Update changelog
cyli Sep 28, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
+ Preliminary Windows support for notary client [#970](https://github.com/docker/notary/pull/970)
+ Output message to CLI when repo changes have been successfully published [#974](https://github.com/docker/notary/pull/974)
+ Improved error messages for client authentication errors and for the witness command [#972](https://github.com/docker/notary/pull/972)
+ Support for finding keys that are anywhere in the notary directory's "private" directory, not just under "private/root_keys" or "private/tuf_keys" [#981](https://github.com/docker/notary/pull/981)

## [v0.4.0](https://github.com/docker/notary/releases/tag/v0.4.0) 9/21/2016
+ Server-managed key rotations [#889](https://github.com/docker/notary/pull/889)
Expand Down
53 changes: 16 additions & 37 deletions trustmanager/keystore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,26 +173,12 @@ func (s *GenericKeyStore) AddKey(keyInfo KeyInfo, privKey data.PrivateKey) error
func (s *GenericKeyStore) GetKey(name string) (data.PrivateKey, string, error) {
s.Lock()
defer s.Unlock()
// If this is a bare key ID without the gun, prepend the gun so the filestore lookup succeeds
if keyInfo, ok := s.keyInfoMap[name]; ok {
name = filepath.Join(keyInfo.Gun, name)
}

cachedKeyEntry, ok := s.cachedKeys[name]
if ok {
return cachedKeyEntry.key, cachedKeyEntry.alias, nil
}

keyAlias, legacy, err := getKeyRole(s.store, name)
if err != nil {
return nil, "", err
}

if legacy {
name = name + "_" + keyAlias
}

keyBytes, err := s.store.Get(filepath.Join(getSubdir(keyAlias), name))
keyBytes, _, keyAlias, err := getKey(s.store, name)
if err != nil {
return nil, "", err
}
Expand All @@ -218,25 +204,18 @@ func (s *GenericKeyStore) ListKeys() map[string]KeyInfo {
func (s *GenericKeyStore) RemoveKey(keyID string) error {
s.Lock()
defer s.Unlock()
// If this is a bare key ID without the gun, prepend the gun so the filestore lookup succeeds
if keyInfo, ok := s.keyInfoMap[keyID]; ok {
keyID = filepath.Join(keyInfo.Gun, keyID)
}

role, legacy, err := getKeyRole(s.store, keyID)
if err != nil {
_, filename, _, err := getKey(s.store, keyID)
switch err.(type) {
case ErrKeyNotFound, nil:
break
default:
return err
}

delete(s.cachedKeys, keyID)

name := keyID
if legacy {
name = keyID + "_" + role
}

// being in a subdirectory is for backwards compatibliity
err = s.store.Remove(filepath.Join(getSubdir(role), name))
err = s.store.Remove(filename) // removing a file that doesn't exist doesn't fail
if err != nil {
return err
}
Expand Down Expand Up @@ -276,11 +255,11 @@ func KeyInfoFromPEM(pemBytes []byte, filename string) (string, KeyInfo, error) {
return keyID, KeyInfo{Gun: gun, Role: role}, nil
}

// getKeyRole finds the role for the given keyID. It attempts to look
// both in the newer format PEM headers, and also in the legacy filename
// format. It returns: the role, whether it was found in the legacy format
// (true == legacy), and an error
func getKeyRole(s Storage, keyID string) (string, bool, error) {
// getKey finds the key and role for the given keyID. It attempts to
// look both in the newer format PEM headers, and also in the legacy filename
// format. It returns: the key bytes, the filename it was found under, the role,
// and an error
func getKey(s Storage, keyID string) ([]byte, string, string, error) {
name := strings.TrimSpace(strings.TrimSuffix(filepath.Base(keyID), filepath.Ext(keyID)))

for _, file := range s.ListFiles() {
Expand All @@ -289,21 +268,21 @@ func getKeyRole(s Storage, keyID string) (string, bool, error) {
if strings.HasPrefix(filename, name) {
d, err := s.Get(file)
if err != nil {
return "", false, err
return nil, "", "", err
}
block, _ := pem.Decode(d)
if block != nil {
if role, ok := block.Headers["role"]; ok {
return role, false, nil
return d, file, role, nil
}
}

role := strings.TrimPrefix(filename, name+"_")
return role, true, nil
return d, file, role, nil
}
}

return "", false, ErrKeyNotFound{KeyID: keyID}
return nil, "", "", ErrKeyNotFound{KeyID: keyID}
}

// Assumes 2 subdirectories, 1 containing root keys and 1 containing TUF keys
Expand Down
175 changes: 113 additions & 62 deletions trustmanager/keystore_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package trustmanager

import (
"crypto/rand"
"encoding/pem"
"errors"
"fmt"
"io/ioutil"
"os"
"path"
"path/filepath"
"testing"

Expand Down Expand Up @@ -167,29 +169,39 @@ func TestGet(t *testing.T) {

gun := "docker.io/notary"

// Root role needs to go in the rootKeySubdir to be read.
// All other roles can go in the nonRootKeysSubdir, possibly under a GUN
// Root role currently goes into the rootKeySubdir, and all other roles go
// in the nonRootKeysSubdir, possibly under a GUN.
nonRootKeysSubdirWithGUN := filepath.Clean(filepath.Join(notary.NonRootKeysSubdir, gun))

testGetKeyWithRole(t, "", data.CanonicalRootRole, notary.RootKeysSubdir, true)
testGetKeyWithRole(t, "", data.CanonicalRootRole, filepath.Join(notary.PrivDir, notary.RootKeysSubdir), true)
for _, role := range nonRootRolesToTest {
testGetKeyWithRole(t, "", role, notary.NonRootKeysSubdir, true)
testGetKeyWithRole(t, gun, role, nonRootKeysSubdirWithGUN, true)
testGetKeyWithRole(t, "", role, filepath.Join(notary.PrivDir, notary.NonRootKeysSubdir), true)
testGetKeyWithRole(t, gun, role, filepath.Join(notary.PrivDir, nonRootKeysSubdirWithGUN), true)
}

// Root cannot go in the nonRootKeysSubdir, or it won't be able to be read,
// and vice versa
testGetKeyWithRole(t, "", data.CanonicalRootRole, notary.NonRootKeysSubdir, false)
testGetKeyWithRole(t, gun, data.CanonicalRootRole, nonRootKeysSubdirWithGUN, false)
for _, role := range nonRootRolesToTest {
testGetKeyWithRole(t, "", role, notary.RootKeysSubdir, false)
// However, keys of any role can be read from anywhere in the private dir so long as
// it has the right ID
for _, expectedSubdir := range []string{
notary.PrivDir,
filepath.Join(notary.PrivDir, nonRootKeysSubdirWithGUN),
filepath.Join(notary.PrivDir, notary.RootKeysSubdir),
filepath.Join(notary.PrivDir, notary.RootKeysSubdir, gun),
filepath.Join(notary.PrivDir, notary.NonRootKeysSubdir),
} {
for _, role := range append(nonRootRolesToTest, data.CanonicalRootRole) {
testGetKeyWithRole(t, "", role, expectedSubdir, true)
testGetKeyWithRole(t, gun, role, expectedSubdir, true)
}
}
}

func testGetKeyWithRole(t *testing.T, gun, role, expectedSubdir string, success bool) {
testData := []byte(fmt.Sprintf(`-----BEGIN RSA PRIVATE KEY-----
role: %s
// keys outside of the private dir cannot be read
for _, role := range append(nonRootRolesToTest, data.CanonicalRootRole) {
testGetKeyWithRole(t, "", role, "otherDir", false)
testGetKeyWithRole(t, gun, role, "otherDir", false)
}
}

func writeKeyFile(t *testing.T, perms os.FileMode, filename, roleInPEM string) []byte {
testData := []byte(`-----BEGIN RSA PRIVATE KEY-----
MIIEogIBAAKCAQEAyUIXjsrWRrvPa4Bzp3VJ6uOUGPay2fUpSV8XzNxZxIG/Opdr
+k3EQi1im6WOqF3Y5AS1UjYRxNuRN+cAZeo3uS1pOTuoSupBXuchVw8s4hZJ5vXn
TRmGb+xY7tZ1ZVgPfAZDib9sRSUsL/gC+aSyprAjG/YBdbF06qKbfOfsoCEYW1OQ
Expand All @@ -216,7 +228,24 @@ EkqpAoGAJWe8PC0XK2RE9VkbSPg9Ehr939mOLWiHGYTVWPttUcum/rTKu73/X/mj
WxnPWGtzM1pHWypSokW90SP4/xedMxludvBvmz+CTYkNJcBGCrJumy11qJhii9xp
EMl3eFOJXjIch/wIesRSN+2dGOsl7neercjMh1i9RvpCwHDx/E0=
-----END RSA PRIVATE KEY-----
`, role))
`)

if roleInPEM != "" {
block, _ := pem.Decode(testData)
require.NotNil(t, block)
block.Headers = map[string]string{
"role": roleInPEM,
}
testData = pem.EncodeToMemory(block)
}

os.MkdirAll(filepath.Dir(filename), perms)
err := ioutil.WriteFile(filename, testData, perms)
require.NoError(t, err, "failed to write test file")
return testData
}

func testGetKeyWithRole(t *testing.T, gun, role, expectedSubdir string, success bool) {
testName := "keyID"
testExt := "key"
perms := os.FileMode(0755)
Expand All @@ -229,18 +258,16 @@ EMl3eFOJXjIch/wIesRSN+2dGOsl7neercjMh1i9RvpCwHDx/E0=
defer os.RemoveAll(tempBaseDir)

// Since we're generating this manually we need to add the extension '.'
filePath := filepath.Join(tempBaseDir, notary.PrivDir, expectedSubdir, testName+"."+testExt)
os.MkdirAll(filepath.Dir(filePath), perms)
err = ioutil.WriteFile(filePath, testData, perms)
require.NoError(t, err, "failed to write test file")
filePath := filepath.Join(tempBaseDir, expectedSubdir, testName+"."+testExt)
testData := writeKeyFile(t, perms, filePath, role)

// Create our store
store, err := NewKeyFileStore(tempBaseDir, emptyPassphraseRetriever)
require.NoError(t, err, "failed to create new key filestore")

// Call the GetKey function
if gun != "" {
testName = gun + "/keyID"
testName = path.Join(gun, "keyID")
}
privKey, _, err := store.GetKey(testName)
if success {
Expand All @@ -263,34 +290,6 @@ EMl3eFOJXjIch/wIesRSN+2dGOsl7neercjMh1i9RvpCwHDx/E0=
// TestGetLegacyKey ensures we can still load keys where the role
// is stored as part of the filename (i.e. <hexID>_<role>.key
func TestGetLegacyKey(t *testing.T) {
testData := []byte(`-----BEGIN RSA PRIVATE KEY-----
MIIEogIBAAKCAQEAyUIXjsrWRrvPa4Bzp3VJ6uOUGPay2fUpSV8XzNxZxIG/Opdr
+k3EQi1im6WOqF3Y5AS1UjYRxNuRN+cAZeo3uS1pOTuoSupBXuchVw8s4hZJ5vXn
TRmGb+xY7tZ1ZVgPfAZDib9sRSUsL/gC+aSyprAjG/YBdbF06qKbfOfsoCEYW1OQ
82JqHzQH514RFYPTnEGpvfxWaqmFQLmv0uMxV/cAYvqtrGkXuP0+a8PknlD2obw5
0rHE56Su1c3Q42S7L51K38tpbgWOSRcTfDUWEj5v9wokkNQvyKBwbS996s4EJaZd
7r6M0h1pHnuRxcSaZLYRwgOe1VNGg2VfWzgd5QIDAQABAoIBAF9LGwpygmj1jm3R
YXGd+ITugvYbAW5wRb9G9mb6wspnwNsGTYsz/UR0ZudZyaVw4jx8+jnV/i3e5PC6
QRcAgqf8l4EQ/UuThaZg/AlT1yWp9g4UyxNXja87EpTsGKQGwTYxZRM4/xPyWOzR
mt8Hm8uPROB9aA2JG9npaoQG8KSUj25G2Qot3ukw/IOtqwN/Sx1EqF0EfCH1K4KU
a5TrqlYDFmHbqT1zTRec/BTtVXNsg8xmF94U1HpWf3Lpg0BPYT7JiN2DPoLelRDy
a/A+a3ZMRNISL5wbq/jyALLOOyOkIqa+KEOeW3USuePd6RhDMzMm/0ocp5FCwYfo
k4DDeaECgYEA0eSMD1dPGo+u8UTD8i7ZsZCS5lmXLNuuAg5f5B/FGghD8ymPROIb
dnJL5QSbUpmBsYJ+nnO8RiLrICGBe7BehOitCKi/iiZKJO6edrfNKzhf4XlU0HFl
jAOMa975pHjeCoZ1cXJOEO9oW4SWTCyBDBSqH3/ZMgIOiIEk896lSmkCgYEA9Xf5
Jqv3HtQVvjugV/axAh9aI8LMjlfFr9SK7iXpY53UdcylOSWKrrDok3UnrSEykjm7
UL3eCU5jwtkVnEXesNn6DdYo3r43E6iAiph7IBkB5dh0yv3vhIXPgYqyTnpdz4pg
3yPGBHMPnJUBThg1qM7k6a2BKHWySxEgC1DTMB0CgYAGvdmF0J8Y0k6jLzs/9yNE
4cjmHzCM3016gW2xDRgumt9b2xTf+Ic7SbaIV5qJj6arxe49NqhwdESrFohrKaIP
kM2l/o2QaWRuRT/Pvl2Xqsrhmh0QSOQjGCYVfOb10nAHVIRHLY22W4o1jk+piLBo
a+1+74NRaOGAnu1J6/fRKQKBgAF180+dmlzemjqFlFCxsR/4G8s2r4zxTMXdF+6O
3zKuj8MbsqgCZy7e8qNeARxwpCJmoYy7dITNqJ5SOGSzrb2Trn9ClP+uVhmR2SH6
AlGQlIhPn3JNzI0XVsLIloMNC13ezvDE/7qrDJ677EQQtNEKWiZh1/DrsmHr+irX
EkqpAoGAJWe8PC0XK2RE9VkbSPg9Ehr939mOLWiHGYTVWPttUcum/rTKu73/X/mj
WxnPWGtzM1pHWypSokW90SP4/xedMxludvBvmz+CTYkNJcBGCrJumy11qJhii9xp
EMl3eFOJXjIch/wIesRSN+2dGOsl7neercjMh1i9RvpCwHDx/E0=
-----END RSA PRIVATE KEY-----
`)
testName := "docker.com/notary/root"
testExt := "key"
testAlias := "root"
Expand All @@ -305,10 +304,7 @@ EMl3eFOJXjIch/wIesRSN+2dGOsl7neercjMh1i9RvpCwHDx/E0=

// Since we're generating this manually we need to add the extension '.'
filePath := filepath.Join(tempBaseDir, notary.PrivDir, notary.RootKeysSubdir, testName+"_"+testAlias+"."+testExt)

os.MkdirAll(filepath.Dir(filePath), perms)
err = ioutil.WriteFile(filePath, testData, perms)
require.NoError(t, err, "failed to write test file")
writeKeyFile(t, perms, filePath, "")

// Create our store
store, err := NewKeyFileStore(tempBaseDir, emptyPassphraseRetriever)
Expand Down Expand Up @@ -356,12 +352,6 @@ func TestListKeys(t *testing.T) {
require.Equal(t, role, listedInfo.Role)
}

// Write an invalid filename to the directory
filePath := filepath.Join(tempBaseDir, notary.PrivDir, notary.RootKeysSubdir, "fakekeyname.key")
err = ioutil.WriteFile(filePath, []byte("data"), perms)
require.NoError(t, err, "failed to write test file")

// Check to see if the keystore still lists two keys
keyMap := store.ListKeys()
require.Len(t, keyMap, len(roles))

Expand All @@ -372,6 +362,31 @@ func TestListKeys(t *testing.T) {
_, err = store.GetKeyInfo(keyID)
require.NoError(t, err)
}

require.Len(t, store.ListKeys(), len(roles))

// ListKeys() works even if the keys are in non-standard locations
for i, loc := range []string{
filepath.Join(tempBaseDir, notary.PrivDir, notary.RootKeysSubdir),
filepath.Join(tempBaseDir, notary.PrivDir, notary.NonRootKeysSubdir),
filepath.Join(tempBaseDir, notary.PrivDir, notary.RootKeysSubdir, testName),
filepath.Join(tempBaseDir, notary.PrivDir, testName),
filepath.Join(tempBaseDir, notary.PrivDir),
tempBaseDir, // this key won't be read
} {
fp := filepath.Join(loc, fmt.Sprintf("keyID%d.key", i))
writeKeyFile(t, perms, fp, "")

// Ensure file exists
_, err := ioutil.ReadFile(fp)
require.NoError(t, err, "expected file not found")
}

// update our store so we read from the FS again
store, err = NewKeyFileStore(tempBaseDir, passphraseRetriever)
require.NoError(t, err, "failed to create new key filestore")

require.Len(t, store.ListKeys(), len(roles)+5)
}

func TestAddGetKeyMemStore(t *testing.T) {
Expand Down Expand Up @@ -567,6 +582,42 @@ func TestRemoveKey(t *testing.T) {
testRemoveKeyWithRole(t, data.CanonicalSnapshotRole, filepath.Join(notary.NonRootKeysSubdir, gun))
testRemoveKeyWithRole(t, "targets/a/b/c", notary.NonRootKeysSubdir)
testRemoveKeyWithRole(t, "invalidRole", notary.NonRootKeysSubdir)

// create another store for other testing
tempBaseDir, err := ioutil.TempDir("", "notary-test-")
require.NoError(t, err, "failed to create a temporary directory")
defer os.RemoveAll(tempBaseDir)

store, err := NewKeyFileStore(tempBaseDir, passphraseRetriever)
require.NoError(t, err, "failed to create new key filestore")

// write keys to non-standard locations - since we're generating keys manually
// we need to add the extenxion
perms := os.FileMode(0755)
for i, loc := range []string{
filepath.Join(tempBaseDir, notary.PrivDir, notary.RootKeysSubdir),
filepath.Join(tempBaseDir, notary.PrivDir, notary.NonRootKeysSubdir),
filepath.Join(tempBaseDir, notary.PrivDir, notary.RootKeysSubdir, gun),
filepath.Join(tempBaseDir, notary.PrivDir, gun),
filepath.Join(tempBaseDir, notary.PrivDir),
} {
fp := filepath.Join(loc, fmt.Sprintf("keyID%d.key", i))
writeKeyFile(t, perms, fp, "")

// Ensure file exists
_, err := ioutil.ReadFile(fp)
require.NoError(t, err, "expected file not found")

err = store.RemoveKey(fmt.Sprintf("keyID%d", i))
require.NoError(t, err)

// File should no longer exist
_, err = ioutil.ReadFile(fp)
require.True(t, os.IsNotExist(err), "file should not exist")
}

// removing a non-existent key should not error
require.NoError(t, store.RemoveKey("nope"))
}

func testRemoveKeyWithRole(t *testing.T, role, expectedSubdir string) {
Expand Down Expand Up @@ -599,9 +650,9 @@ func testRemoveKeyWithRole(t *testing.T, role, expectedSubdir string) {
err = store.RemoveKey(privKey.ID())
require.NoError(t, err, "unable to remove key")

// Check to see if file still exists
// File should no longer exist
_, err = ioutil.ReadFile(expectedFilePath)
require.Error(t, err, "file should not exist")
require.True(t, os.IsNotExist(err), "file should not exist")
}

func TestKeysAreCached(t *testing.T) {
Expand Down