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.

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

adding --role flag to key import #882

Merged
merged 9 commits into from
Aug 10, 2016
514 changes: 506 additions & 8 deletions cmd/notary/integration_test.go

Large diffs are not rendered by default.

18 changes: 12 additions & 6 deletions cmd/notary/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,11 @@ type keyCommander struct {

input io.Reader

exportGUNs []string
exportKeyIDs []string
outFile string
keysImportRole string
keysImportGUN string
exportGUNs []string
exportKeyIDs []string
outFile string
}

func (k *keyCommander) GetCommand() *cobra.Command {
Expand All @@ -97,7 +99,12 @@ func (k *keyCommander) GetCommand() *cobra.Command {
"Required for timestamp role, optional for snapshot role")
cmd.AddCommand(cmdRotateKey)

cmd.AddCommand(cmdKeyImportTemplate.ToCommand(k.importKeys))
cmdKeysImport := cmdKeyImportTemplate.ToCommand(k.importKeys)
cmdKeysImport.Flags().StringVarP(
&k.keysImportRole, "role", "r", "", "Role to import key with, if a role is not already given in a PEM header")
cmdKeysImport.Flags().StringVarP(
Copy link
Contributor

@riyazdf riyazdf Aug 8, 2016

Choose a reason for hiding this comment

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

nit: can we rephrase both of these role and gun flag descriptions to something shorter like:

Role to import key with, if a role is not already given in a PEM header

I think the pathing logic can be explained from informative error messages rather than adding complexity to the CLI flags, since the error should depend on how the user imports the keys (straight from openssl vs. from notary key export)

&k.keysImportGUN, "gun", "g", "", "Gun to import key with, if a gun is not already given in a PEM header")
cmd.AddCommand(cmdKeysImport)
cmdExport := cmdKeyExportTemplate.ToCommand(k.exportKeys)
cmdExport.Flags().StringSliceVar(
&k.exportGUNs,
Expand Down Expand Up @@ -408,8 +415,7 @@ func (k *keyCommander) importKeys(cmd *cobra.Command, args []string) error {
for _, file := range args {
from, err := os.OpenFile(file, os.O_RDONLY, notary.PrivKeyPerms)
defer from.Close()

if err = utils.ImportKeys(from, importers); err != nil {
if err = utils.ImportKeys(from, importers, k.keysImportRole, k.keysImportGUN, k.getRetriever()); err != nil {
return err
}
}
Expand Down
2 changes: 2 additions & 0 deletions const.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ const (
MemoryBackend = "memory"
SQLiteBackend = "sqlite3"
RethinkDBBackend = "rethinkdb"

DefaultImportRole = "delegation"
)

// NotaryDefaultExpiries is the construct used to configure the default expiry times of
Expand Down
2 changes: 1 addition & 1 deletion trustmanager/keystore.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func (s *GenericKeyStore) AddKey(keyInfo KeyInfo, privKey data.PrivateKey) error
}

if chosenPassphrase != "" {
pemPrivKey, err = utils.EncryptPrivateKey(privKey, keyInfo.Role, chosenPassphrase)
pemPrivKey, err = utils.EncryptPrivateKey(privKey, keyInfo.Role, keyInfo.Gun, chosenPassphrase)
} else {
pemPrivKey, err = utils.KeyToPEM(privKey, keyInfo.Role)
}
Expand Down
6 changes: 5 additions & 1 deletion tuf/utils/x509.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ func KeyToPEM(privKey data.PrivateKey, role string) ([]byte, error) {

// EncryptPrivateKey returns an encrypted PEM key given a Privatekey
// and a passphrase
func EncryptPrivateKey(key data.PrivateKey, role, passphrase string) ([]byte, error) {
func EncryptPrivateKey(key data.PrivateKey, role, gun, passphrase string) ([]byte, error) {
bt, err := blockType(key)
if err != nil {
return nil, err
Expand All @@ -455,6 +455,10 @@ func EncryptPrivateKey(key data.PrivateKey, role, passphrase string) ([]byte, er
}
encryptedPEMBlock.Headers["role"] = role

if gun != "" {
encryptedPEMBlock.Headers["gun"] = gun
}

return pem.EncodeToMemory(encryptedPEMBlock), nil
}

Expand Down
28 changes: 22 additions & 6 deletions tuf/utils/x509_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,15 @@ func TestKeyOperations(t *testing.T) {
rsaKey, err := GenerateRSAKey(rand.Reader, 512)

// Encode our ED private key
edPEM, err := KeyToPEM(edKey, "root")
edPEM, err := KeyToPEM(edKey, data.CanonicalRootRole)
require.NoError(t, err)

// Encode our EC private key
ecPEM, err := KeyToPEM(ecKey, "root")
ecPEM, err := KeyToPEM(ecKey, data.CanonicalRootRole)
require.NoError(t, err)

// Encode our RSA private key
rsaPEM, err := KeyToPEM(rsaKey, "root")
rsaPEM, err := KeyToPEM(rsaKey, data.CanonicalRootRole)
require.NoError(t, err)

// Check to see if ED key it is encoded
Expand Down Expand Up @@ -125,31 +125,34 @@ func TestKeyOperations(t *testing.T) {
require.Equal(t, rsaKey.Private(), decodedRSAKey.Private())

// Encrypt our ED Key
encryptedEDKey, err := EncryptPrivateKey(edKey, "root", "ponies")
encryptedEDKey, err := EncryptPrivateKey(edKey, data.CanonicalRootRole, "", "ponies")
require.NoError(t, err)

// Encrypt our EC Key
encryptedECKey, err := EncryptPrivateKey(ecKey, "root", "ponies")
encryptedECKey, err := EncryptPrivateKey(ecKey, data.CanonicalRootRole, "", "ponies")
require.NoError(t, err)

// Encrypt our RSA Key
encryptedRSAKey, err := EncryptPrivateKey(rsaKey, "root", "ponies")
encryptedRSAKey, err := EncryptPrivateKey(rsaKey, data.CanonicalRootRole, "", "ponies")
require.NoError(t, err)

// Check to see if ED key it is encrypted
stringEncryptedEDKey := string(encryptedEDKey)
require.True(t, strings.Contains(stringEncryptedEDKey, "-----BEGIN ED25519 PRIVATE KEY-----"))
require.True(t, strings.Contains(stringEncryptedEDKey, "Proc-Type: 4,ENCRYPTED"))
require.True(t, strings.Contains(stringEncryptedEDKey, "role: root"))

// Check to see if EC key it is encrypted
stringEncryptedECKey := string(encryptedECKey)
require.True(t, strings.Contains(stringEncryptedECKey, "-----BEGIN EC PRIVATE KEY-----"))
require.True(t, strings.Contains(stringEncryptedECKey, "Proc-Type: 4,ENCRYPTED"))
require.True(t, strings.Contains(stringEncryptedECKey, "role: root"))

// Check to see if RSA key it is encrypted
stringEncryptedRSAKey := string(encryptedRSAKey)
require.True(t, strings.Contains(stringEncryptedRSAKey, "-----BEGIN RSA PRIVATE KEY-----"))
require.True(t, strings.Contains(stringEncryptedRSAKey, "Proc-Type: 4,ENCRYPTED"))
require.True(t, strings.Contains(stringEncryptedRSAKey, "role: root"))

// Decrypt our ED Key
decryptedEDKey, err := ParsePEMPrivateKey(encryptedEDKey, "ponies")
Expand All @@ -166,6 +169,19 @@ func TestKeyOperations(t *testing.T) {
require.NoError(t, err)
require.Equal(t, rsaKey.Private(), decryptedRSAKey.Private())

// quick test that gun headers are being added appropriately
// Encrypt our RSA Key, one type of key should be enough since headers are treated the same
testGunKey, err := EncryptPrivateKey(rsaKey, data.CanonicalRootRole, "ilove", "ponies")
require.NoError(t, err)

testNoGunKey, err := EncryptPrivateKey(rsaKey, data.CanonicalRootRole, "", "ponies")
require.NoError(t, err)

stringTestGunKey := string(testGunKey)
require.True(t, strings.Contains(stringTestGunKey, "gun: ilove"))

stringTestNoGunKey := string(testNoGunKey)
require.False(t, strings.Contains(stringTestNoGunKey, "gun:"))
}

// X509PublickeyID returns the public key ID of a RSA X509 key rather than the
Expand Down
95 changes: 89 additions & 6 deletions utils/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ package utils

import (
"encoding/pem"
"errors"
"fmt"
"github.com/Sirupsen/logrus"
"github.com/docker/notary"
tufdata "github.com/docker/notary/tuf/data"
"github.com/docker/notary/tuf/utils"
"io"
"io/ioutil"
"path/filepath"
Expand All @@ -26,7 +29,7 @@ type Importer interface {
// ExportKeysByGUN exports all keys filtered to a GUN
func ExportKeysByGUN(to io.Writer, s Exporter, gun string) error {
keys := s.ListFiles()
sort.Strings(keys) // ensure consistenct. ListFiles has no order guarantee
sort.Strings(keys) // ensure consistency. ListFiles has no order guarantee
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for fixing this typo!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

of course 😄

for _, k := range keys {
dir := filepath.Dir(k)
if dir == gun { // must be full GUN match
Expand Down Expand Up @@ -93,7 +96,7 @@ func ExportKeys(to io.Writer, s Exporter, from string) error {
// Each block is written to the subpath indicated in the "path" PEM
// header. If the file already exists, the file is truncated. Multiple
// adjacent PEMs with the same "path" header are appended together.
func ImportKeys(from io.Reader, to []Importer) error {
func ImportKeys(from io.Reader, to []Importer, fallbackRole string, fallbackGun string, passRet notary.PassRetriever) error {
data, err := ioutil.ReadAll(from)
if err != nil {
return err
Expand All @@ -103,11 +106,91 @@ func ImportKeys(from io.Reader, to []Importer) error {
toWrite []byte
)
for block, rest := pem.Decode(data); block != nil; block, rest = pem.Decode(rest) {
// if there is a path then we set the gun header from this path
if rawPath := block.Headers["path"]; rawPath != "" {
pathWOFileName := strings.TrimSuffix(rawPath, filepath.Base(rawPath))
if strings.HasPrefix(pathWOFileName, notary.NonRootKeysSubdir) {
gunName := strings.TrimPrefix(pathWOFileName, notary.NonRootKeysSubdir)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a test-case that if we initialize a repo, export keys, then import keys to a fresh file-based keystore the gun PEM headers are set?

Also, for this logic - it seems like delegation keys may end up with a gun header with an empty value? Should we add a check for gunName being an empty string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, thanks 😄

gunName = gunName[1:(len(gunName) - 1)] // remove the slashes
if gunName != "" {
block.Headers["gun"] = gunName
}
}
}
if block.Headers["gun"] == "" {
Copy link
Contributor

@cyli cyli Aug 3, 2016

Choose a reason for hiding this comment

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

Non-blocking: this has probably already been discussed and resolved, and I just completely forgot all the cases, but is it ok for the gun and path to mismatch? e.g. gun to be docker.io/hello/world and path to be /tuf_keys/docker.io/nothello/notworld/DEADBEEF0A...key? I know we don't really use the header GUN for anything right now, so it doesn't much matter - just wondering if it's worth trying to grab the GUN from the path first before falling back on no gun?

This wouldn't affect where to store it at all, just a field which is not currently used but may be used in the future.

if fallbackGun != "" {
block.Headers["gun"] = fallbackGun
}
}
if block.Headers["role"] == "" {
if fallbackRole == "" {
block.Headers["role"] = notary.DefaultImportRole
} else {
block.Headers["role"] = fallbackRole
}
}
loc, ok := block.Headers["path"]
// only if the path isn't specified do we get into this parsing path logic
if !ok || loc == "" {
logrus.Info("failed to import key to store: PEM headers did not contain import path")
continue // don't know where to copy this key. Skip it.
// if the path isn't specified, we will try to infer the path rel to trust dir from the role (and then gun)
// parse key for the keyID which we will save it by.
// if the key is encrypted at this point, we will generate an error and continue since we don't know the ID to save it by
decodedKey, err := utils.ParsePEMPrivateKey(pem.EncodeToMemory(block), "")
if err != nil {
logrus.Info("failed to import key to store: Invalid key generated, key may be encrypted and does not contain path header")
continue
}
keyID := decodedKey.ID()
switch block.Headers["role"] {
case tufdata.CanonicalRootRole:
// this is a root key so import it to trustDir/root_keys/
loc = filepath.Join(notary.RootKeysSubdir, keyID)
case tufdata.CanonicalSnapshotRole, tufdata.CanonicalTargetsRole, tufdata.CanonicalTimestampRole:
// this is a canonical key
loc = filepath.Join(notary.NonRootKeysSubdir, block.Headers["gun"], keyID)
default:
//this is a delegation key
loc = filepath.Join(notary.NonRootKeysSubdir, keyID)
}
}

// A root key or a delegations key should not have a gun
// Note that a key that is not any of the canonical roles (except root) is a delegations key and should not have a gun
if block.Headers["role"] != tufdata.CanonicalSnapshotRole && block.Headers["role"] != tufdata.CanonicalTargetsRole && block.Headers["role"] != tufdata.CanonicalTimestampRole {
delete(block.Headers, "gun")
} else {
// check if the key is missing a gun header or has an empty gun and error out since we don't know where to import this key to
if block.Headers["gun"] == "" {
logrus.Info("failed to import key to store: Cannot have canonical role key without a gun, don't know where to import it")
continue
}
}

// the path header is not of any use once we've imported the key so strip it away
delete(block.Headers, "path")

// we are now all set for import but let's first encrypt the key
blockBytes := pem.EncodeToMemory(block)
// check if key is encrypted, note: if it is encrypted at this point, it will have had a path header
if privKey, err := utils.ParsePEMPrivateKey(blockBytes, ""); err == nil {
// Key is not encrypted- ask for a passphrase and encrypt this key
var chosenPassphrase string
for attempts := 0; ; attempts++ {
var giveup bool
chosenPassphrase, giveup, err = passRet(loc, block.Headers["role"], true, attempts)
if err == nil {
break
}
if giveup || attempts > 10 {
return errors.New("maximum number of passphrase attempts exceeded")
}
}
blockBytes, err = utils.EncryptPrivateKey(privKey, block.Headers["role"], block.Headers["gun"], chosenPassphrase)
if err != nil {
return errors.New("failed to encrypt key with given passphrase")
}
}

if loc != writeTo {
// next location is different from previous one. We've finished aggregating
// data for the previous file. If we have data, write the previous file,
Expand All @@ -121,8 +204,8 @@ func ImportKeys(from io.Reader, to []Importer) error {
toWrite = nil
writeTo = loc
}
delete(block.Headers, "path")
toWrite = append(toWrite, pem.EncodeToMemory(block)...)

toWrite = append(toWrite, blockBytes...)
}
if toWrite != nil { // close out final iteration if there's data left
return importToStores(to, writeTo, toWrite)
Expand Down
Loading