Skip to content

Commit

Permalink
Merge pull request #981 from cyli/read-flattened-keystore
Browse files Browse the repository at this point in the history 
Be able to read keys even if they are not in the root/non-root subdirs
  • Loading branch information
@cyli
cyli authored Sep 29, 2016
2 parents f8c0b01 + 128bf0c commit 8f0965c
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 99 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
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
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
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

0 comments on commit 8f0965c

Please sign in to comment.