From 722a65dab2d552d152d78197c623665d7b7d0328 Mon Sep 17 00:00:00 2001 From: Avi Vaid Date: Thu, 4 Aug 2016 15:56:46 -0700 Subject: [PATCH] fixups as per Riyaz's review Signed-off-by: Avi Vaid --- fixtures/precedence.example.com.key | 30 ++++++++++++++++++++++ trustmanager/keystore.go | 2 +- utils/keys.go | 9 ++++--- utils/keys_test.go | 40 +++++++++++++++++------------ utils/test.key | 27 ------------------- utils/testprecedence.key | 29 --------------------- 6 files changed, 60 insertions(+), 77 deletions(-) create mode 100644 fixtures/precedence.example.com.key delete mode 100644 utils/test.key delete mode 100644 utils/testprecedence.key diff --git a/fixtures/precedence.example.com.key b/fixtures/precedence.example.com.key new file mode 100644 index 0000000000..eaa512bbfb --- /dev/null +++ b/fixtures/precedence.example.com.key @@ -0,0 +1,30 @@ +-----BEGIN RSA PRIVATE KEY----- +role: root + +MIIEowIBAAKCAQEAmLYiYCTAWJBWAuxZLqVmV4FiUdGgEqoQvCbN73zF/mQfhq0C +ITo6xSxs1QiGDOzUtkpzXzziSj4J5+et4JkFleeEKaMcHadeIsSlHGvVtXDv93oR +3ydmfZO+ULRU8xHloqcLr1KrOP1daLfdMRbactd75UQgvw9XTsdeMVX5AlicSENV +KV+AQXvVpv8PT10MSvlBFam4reXuY/SkeMbIaW5pFu6AQv3Zmftt2ta0CB9kb1mY +d+OKru8Hnnq5aJw6R3GhP0TBd25P1PkiSxM2KGYZZk0W/NZqLK9/LTFKTNCv7VjC +bysVo7HxCY0bQe/bDP82v7SnLtb3aZogfva4HQIDAQABAoIBAQCLPj+X5MrRtkIH +BlTHGJ95mIr6yaYofpMlzEgoX1/1dnvcg/IWNA8UbE6L7Oq17FiEItyR8WTwhyLn +JrO/wCd8qQ40HPrs+wf1sdJPWPATMfhMcizLihSE2mtFETkILcByD9iyszFWlIdQ +jZ4NPaZP4rWgtf8Z1zYnqdf0Kk0T2imFya0qyoRLo40kxeb4p5K53JD7rPLQNyvO +YeFXTuKxBrFEMs6/wFjl+TO4nfHQXQlgQp4MNd9L5fEQBj+TvGVX+zcQEmzxljK8 +zFNXyxvXgjBPD+0V7yRhTYjrUfZJ4RX1yKDpdsva6BXL7t9hNEg/aGnKRDYF3i5q +WQz8csCBAoGBAMfdtAr3RCuCxe0TIVBon5wubau6HLOxorcXSvxO5PO2kzhy3+GY +xcCMJ+Wo0dTFXjQD3oxRKuDrPRK7AX/grYn7qJo6W7SM9xYEq3HspJJFGkcRsvem +MALt8bvG5NkGmLJD+pTOKVaTZRjW3BM6GcMzBgsLynQcLllRtNI8Hcw9AoGBAMOa +CMsWQfoOUjUffrXN0UnXLEPEeazPobnCHVtE244FdX/BFu5WMA7qqaPRyvnfK0Vl +vF5sGNiBCOnq1zjYee6FD2eyAzVmWJXM1DB4Ewp4ZaABS0ZCZgNfyd1badY4IZpw +pjYEQprguw+J8yZItNJRo+WBmnSgZy6o1bpDaflhAoGAYf61GS9VkFPlQbFAg1FY ++NXW1f1Bt2VgV48nKAByx3/8PRAt70ndo+PUaAlXIJDI+I3xHzFo6bDNWBKy0IVT +8TSf3UbB0gvP1k7h1NDnfAQ/txrZeg1Uuwr5nE0Pxc0zLyyffzh6EkXgqsYmT5MM +MKYiz2WvlTCAFTE3jGEHZy0CgYBti/cgxnZs9VhVKC5u47YzBK9lxMPgZOjOgEiw +tP/Bqo0D38BX+y0vLX2UogprpvE1DKVSvHetyZaUa1HeJF8llp/qE2h4n7k9LFoq +SxVe588CrbbawpUfjqYfsvKzZvxq4mw0FG65DuO08C2dY1rh75c7EjrO1obzOtt4 +VgkkAQKBgDnRyLnzlMfvjCyW9+cHbURQNe2iupfnlrXWEntg56USBVrFtfRQxDRp +fBtlq+0BNfDVdoVNasTCBW16UKoRBH1/k5idz5QPEbKY2055sNxHMVg0uzdb4HXr +73uaYzNrT8P7wyHFF3UL5bd0aO5DT1VYvGlHHgOhCyqcM+RBgPBS +-----END RSA PRIVATE KEY----- + diff --git a/trustmanager/keystore.go b/trustmanager/keystore.go index 00177beb99..6e6ef8de4b 100644 --- a/trustmanager/keystore.go +++ b/trustmanager/keystore.go @@ -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) } diff --git a/utils/keys.go b/utils/keys.go index 0aafd339b8..9d973d0d17 100644 --- a/utils/keys.go +++ b/utils/keys.go @@ -5,6 +5,7 @@ import ( "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" @@ -27,7 +28,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 consistent. ListFiles has no order guarantee + sort.Strings(keys) // ensure consistency. ListFiles has no order guarantee for _, k := range keys { dir := filepath.Dir(k) if dir == gun { // must be full GUN match @@ -110,7 +111,7 @@ func ImportKeys(from io.Reader, to []Importer, fallbackRole string, fallbackGun } if rawPath := block.Headers["path"]; rawPath != "" { pathWOFileName := strings.TrimSuffix(rawPath, filepath.Base(rawPath)) - if strings.Contains(pathWOFileName, notary.NonRootKeysSubdir) { + if strings.HasPrefix(pathWOFileName, notary.NonRootKeysSubdir) { gunName := strings.TrimPrefix(pathWOFileName, notary.NonRootKeysSubdir) block.Headers["gun"] = gunName[1:(len(gunName) - 1)] //removes the slashes } @@ -132,12 +133,12 @@ func ImportKeys(from io.Reader, to []Importer, fallbackRole string, fallbackGun 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 not contains path header") + 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() - if block.Headers["role"] == "root" { + if block.Headers["role"] == tufdata.CanonicalRootRole { // does not make sense for root keys to have GUNs, so import it without the GUN even if specified loc = filepath.Join(notary.RootKeysSubdir, keyID) } else { diff --git a/utils/keys_test.go b/utils/keys_test.go index 2e6b72dae0..25db830f6b 100644 --- a/utils/keys_test.go +++ b/utils/keys_test.go @@ -224,7 +224,7 @@ func TestExport2InOneFile(t *testing.T) { func TestImportKeys(t *testing.T) { s := NewTestImportStore() - from, _ := os.OpenFile("test.key", os.O_RDONLY, notary.PrivKeyPerms) + from, _ := os.OpenFile("../fixtures/secure.example.com.key", os.O_RDONLY, notary.PrivKeyPerms) b := &pem.Block{ Headers: make(map[string]string), } @@ -251,13 +251,15 @@ func TestImportKeys(t *testing.T) { bFinal, bRest := pem.Decode(s.data["ankh"]) require.Equal(t, b.Bytes, bFinal.Bytes) - require.Equal(t, "", bFinal.Headers["path"]) // path header is stripped during import + _, ok := bFinal.Headers["path"] + require.False(t, ok, "expected no path header, should have been removed at import") require.Equal(t, notary.DefaultImportRole, bFinal.Headers["role"]) // if no role is specified we assume it is a delegation key require.Len(t, bRest, 0) cFinal, cRest := pem.Decode(s.data["morpork"]) require.Equal(t, c.Bytes, cFinal.Bytes) - require.Equal(t, "", bFinal.Headers["path"]) + _, ok = cFinal.Headers["path"] + require.False(t, ok, "expected no path header, should have been removed at import") require.Len(t, cRest, 0) } @@ -267,7 +269,7 @@ func TestImportNoPath(t *testing.T) { b := &pem.Block{ Headers: make(map[string]string), } - from, _ := os.OpenFile("test.key", os.O_RDONLY, notary.PrivKeyPerms) + from, _ := os.OpenFile("../fixtures/secure.example.com.key", os.O_RDONLY, notary.PrivKeyPerms) defer from.Close() b.Bytes, _ = ioutil.ReadAll(from) @@ -278,7 +280,7 @@ func TestImportNoPath(t *testing.T) { for key := range s.data { // no path but role included should work - require.Equal(t, key, filepath.Join(notary.RootKeysSubdir, "7baafcc9e5100ab062d886f06468f6c76e70b54b90e5d38537dadc6299c976d9")) + require.Equal(t, key, filepath.Join(notary.RootKeysSubdir, "12ba0e0a8e05e177bc2c3489bdb6d28836879469f078e68a4812fc8a2d521497")) } err = ImportKeys(in, []Importer{s}, "", "", passphraseRetriever) @@ -293,7 +295,7 @@ func TestNonRootPathInference(t *testing.T) { b := &pem.Block{ Headers: make(map[string]string), } - from, _ := os.OpenFile("test.key", os.O_RDONLY, notary.PrivKeyPerms) + from, _ := os.OpenFile("../fixtures/secure.example.com.key", os.O_RDONLY, notary.PrivKeyPerms) defer from.Close() b.Bytes, _ = ioutil.ReadAll(from) @@ -304,7 +306,7 @@ func TestNonRootPathInference(t *testing.T) { for key := range s.data { // no path but role included should work - require.Equal(t, key, filepath.Join(notary.NonRootKeysSubdir, "somegun", "7baafcc9e5100ab062d886f06468f6c76e70b54b90e5d38537dadc6299c976d9")) + require.Equal(t, key, filepath.Join(notary.NonRootKeysSubdir, "somegun", "12ba0e0a8e05e177bc2c3489bdb6d28836879469f078e68a4812fc8a2d521497")) } } @@ -314,7 +316,7 @@ func TestBlockHeaderPrecedence(t *testing.T) { b := &pem.Block{ Headers: make(map[string]string), } - from, _ := os.OpenFile("testprecedence.key", os.O_RDONLY, notary.PrivKeyPerms) + from, _ := os.OpenFile("../fixtures/precedence.example.com.key", os.O_RDONLY, notary.PrivKeyPerms) defer from.Close() b.Bytes, _ = ioutil.ReadAll(from) @@ -325,7 +327,7 @@ func TestBlockHeaderPrecedence(t *testing.T) { for key := range s.data { // block header role should take precedence over command line flag - require.Equal(t, key, filepath.Join(notary.RootKeysSubdir, "7baafcc9e5100ab062d886f06468f6c76e70b54b90e5d38537dadc6299c976d9")) + require.Equal(t, key, filepath.Join(notary.RootKeysSubdir, "12ba0e0a8e05e177bc2c3489bdb6d28836879469f078e68a4812fc8a2d521497")) } } @@ -367,16 +369,19 @@ func TestImportKeys2InOneFile(t *testing.T) { bFinal, bRest := pem.Decode(s.data["ankh"]) require.Equal(t, b.Bytes, bFinal.Bytes) - require.Equal(t, "", bFinal.Headers["path"]) // path header is stripped during import + _, ok := bFinal.Headers["path"] + require.False(t, ok, "expected no path header, should have been removed at import") b2Final, b2Rest := pem.Decode(bRest) require.Equal(t, b2.Bytes, b2Final.Bytes) - require.Equal(t, "", b2Final.Headers["path"]) // path header is stripped during import + _, ok = b2Final.Headers["path"] + require.False(t, ok, "expected no path header, should have been removed at import") require.Len(t, b2Rest, 0) cFinal, cRest := pem.Decode(s.data["morpork"]) require.Equal(t, c.Bytes, cFinal.Bytes) - require.Equal(t, "", bFinal.Headers["path"]) + _, ok = cFinal.Headers["path"] + require.False(t, ok, "expected no path header, should have been removed at import") require.Len(t, cRest, 0) } @@ -386,7 +391,7 @@ func TestImportKeys2InOneFileNoPath(t *testing.T) { b := &pem.Block{ Headers: make(map[string]string), } - from, _ := os.OpenFile("test.key", os.O_RDONLY, notary.PrivKeyPerms) + from, _ := os.OpenFile("../fixtures/secure.example.com.key", os.O_RDONLY, notary.PrivKeyPerms) b.Bytes, _ = ioutil.ReadAll(from) rand.Read(b.Bytes) b.Headers["path"] = "ankh" @@ -419,16 +424,19 @@ func TestImportKeys2InOneFileNoPath(t *testing.T) { bFinal, bRest := pem.Decode(s.data["ankh"]) require.Equal(t, b.Bytes, bFinal.Bytes) - require.Equal(t, "", bFinal.Headers["path"]) // path header is stripped during import + _, ok := bFinal.Headers["path"] + require.False(t, ok, "expected no path header, should have been removed at import") b2Final, b2Rest := pem.Decode(bRest) require.Equal(t, b2.Bytes, b2Final.Bytes) - require.Equal(t, "", b2Final.Headers["path"]) // path header is stripped during import + _, ok = b2Final.Headers["path"] + require.False(t, ok, "expected no path header, should have been removed at import") require.Len(t, b2Rest, 0) cFinal, cRest := pem.Decode(s.data["morpork"]) require.Equal(t, c.Bytes, cFinal.Bytes) - require.Equal(t, "", bFinal.Headers["path"]) + _, ok = cFinal.Headers["path"] + require.False(t, ok, "expected no path header, should have been removed at import") require.Len(t, cRest, 0) require.Len(t, s.data, 2) diff --git a/utils/test.key b/utils/test.key deleted file mode 100644 index 84f0b11013..0000000000 --- a/utils/test.key +++ /dev/null @@ -1,27 +0,0 @@ ------BEGIN RSA PRIVATE KEY----- -MIIEpQIBAAKCAQEAs0IcsbHQNSqcjnC9+iX4XWGWWoBDtI6UEVThsUv8DugsqGCO -fRIFkwqDFQnDMvjbDNvM/DeBvaUvqEQJFMsIDYHfsz+SrcG92XufoggpGX16E4x/ -CqUXL4TlROkO9Rer6LTFKN8xo051Kz4qWrlwheblzO+qZg4oYCfAOoPa/fhGsm6I -oTlR2SUl/EmKLjxghlLEEFT1IYAp/fmPgA6xWCzMWS6/JYcH3W8kdiPY0Gfg9Az2 -DEhJ2j3P1ne5JxM1526zl9/f4EKrbE2/SafTuT7CvEG8S9OYOZzAAITBUEGF8PzO -ARKY0jSZTwK3ep6E1UWLzzyRjB21FUBoPraxVQIDAQABAoIBAFTJUnUh248qJn6h -CNqGlyDkBY+s11lRjutbsJyBiikq2+SQNxUlLwnLkRCkW3WCi2lblWs+fdlLhm0a -MQlqttZctYg8643P85IojOrA3V+3H8SbUYOhboQFYcPkUDTRC3i3+AFLAFlzxKu9 -bZ8X0wcinVL/kSNQUvyYEK4cq7ovp6dbRsbQHYRJeChp3IFftIuwmDnSZOa/Iba5 -g/GJmPBaTDPTmZYJNz3VrxuRa2/jV2O27gQ0tIOr7R0GRsRXZve2LsxklDN3HVsZ -2nzUlRpfR1tOPj/xzKDPk4uI4TGyX/v05BsASqABRpj87KB50yDJeId+S7L/Y5HP -dfWDf0kCgYEA4WvL2rRFgqHehB1xGoCpWqPMurlZ+s5e3nagEf1BaHH01SchsmSr -drZLASjyzGygD+YzYSgHIQprlaGkfrDSzNInM6TmQAE5N4FObZyI+PV6hObanWYo -6UBUPhhpz+WRI6uo/T97I7/E6i+1oy64/VkPOOMe2cJuul2/fhzgFbsCgYEAy5M3 -z+JbhKdrZVo6jqusT7z8sltV9w+5HgZn/KKVs1Hqwni6zI3UVqPrwcYPYMgmKzZ2 -i/aEPw6ul+JPbEGTrernN4Q3Rz1W2HkNPxa4/ln0StPraRt6tnvqgVHDIbEWvln3 -d6FdAYxd9MXp+rMkUHpXP5Kefh2ZMEFyWStS3C8CgYEAj0iOniiCGmO7Zay1LMxl -WVjOlE2mlRZCFRO/8U9SGQp+PlKOR69XkbCTglw94JSXNSP0uqfPyD+wXL8PKPxK -MajPBDOnqz1b2h0V9fRO2DhotTtr8Cp0jLa4FXQ9Jclc9Rhy5O5J3lJR3y2OPfg/ -28GVPGqZPH8rFMjsJEREZ2sCgYEAw53ocKNDa9nwsgic4kGGp2yjqcNofKqoKHjh -bXrRvOlHW63lWfAjjUmgnqfyWP6a5sVD7sRoWauDC8HUreLpxKJHoiozcAo6LHNN -zkTIaOkJfOnceTiFl1rFgZSOnA5uG2WaDWOKWpWGJ1ISvutrRsX4lscN84P/ksYg -2hxuV9MCgYEAsLx2YX3nVjtw2vO1hbdOBV1w7JI9nZkGTZ8pn8P07G/t4acNVU5s -NEG8fGYyhHLTrklM+H7GWxVxaIHwTOCnF7cHr2WGi5x/jIx0sxYXR0uZ6dUgAnwv -Tq0C6DNAYD7ZXBeB595sGbdrYA9U5g63noJIU2iSRdgRLWQagljJpxY= ------END RSA PRIVATE KEY----- diff --git a/utils/testprecedence.key b/utils/testprecedence.key deleted file mode 100644 index 11b11b25c2..0000000000 --- a/utils/testprecedence.key +++ /dev/null @@ -1,29 +0,0 @@ ------BEGIN RSA PRIVATE KEY----- -role: root - -MIIEpQIBAAKCAQEAs0IcsbHQNSqcjnC9+iX4XWGWWoBDtI6UEVThsUv8DugsqGCO -fRIFkwqDFQnDMvjbDNvM/DeBvaUvqEQJFMsIDYHfsz+SrcG92XufoggpGX16E4x/ -CqUXL4TlROkO9Rer6LTFKN8xo051Kz4qWrlwheblzO+qZg4oYCfAOoPa/fhGsm6I -oTlR2SUl/EmKLjxghlLEEFT1IYAp/fmPgA6xWCzMWS6/JYcH3W8kdiPY0Gfg9Az2 -DEhJ2j3P1ne5JxM1526zl9/f4EKrbE2/SafTuT7CvEG8S9OYOZzAAITBUEGF8PzO -ARKY0jSZTwK3ep6E1UWLzzyRjB21FUBoPraxVQIDAQABAoIBAFTJUnUh248qJn6h -CNqGlyDkBY+s11lRjutbsJyBiikq2+SQNxUlLwnLkRCkW3WCi2lblWs+fdlLhm0a -MQlqttZctYg8643P85IojOrA3V+3H8SbUYOhboQFYcPkUDTRC3i3+AFLAFlzxKu9 -bZ8X0wcinVL/kSNQUvyYEK4cq7ovp6dbRsbQHYRJeChp3IFftIuwmDnSZOa/Iba5 -g/GJmPBaTDPTmZYJNz3VrxuRa2/jV2O27gQ0tIOr7R0GRsRXZve2LsxklDN3HVsZ -2nzUlRpfR1tOPj/xzKDPk4uI4TGyX/v05BsASqABRpj87KB50yDJeId+S7L/Y5HP -dfWDf0kCgYEA4WvL2rRFgqHehB1xGoCpWqPMurlZ+s5e3nagEf1BaHH01SchsmSr -drZLASjyzGygD+YzYSgHIQprlaGkfrDSzNInM6TmQAE5N4FObZyI+PV6hObanWYo -6UBUPhhpz+WRI6uo/T97I7/E6i+1oy64/VkPOOMe2cJuul2/fhzgFbsCgYEAy5M3 -z+JbhKdrZVo6jqusT7z8sltV9w+5HgZn/KKVs1Hqwni6zI3UVqPrwcYPYMgmKzZ2 -i/aEPw6ul+JPbEGTrernN4Q3Rz1W2HkNPxa4/ln0StPraRt6tnvqgVHDIbEWvln3 -d6FdAYxd9MXp+rMkUHpXP5Kefh2ZMEFyWStS3C8CgYEAj0iOniiCGmO7Zay1LMxl -WVjOlE2mlRZCFRO/8U9SGQp+PlKOR69XkbCTglw94JSXNSP0uqfPyD+wXL8PKPxK -MajPBDOnqz1b2h0V9fRO2DhotTtr8Cp0jLa4FXQ9Jclc9Rhy5O5J3lJR3y2OPfg/ -28GVPGqZPH8rFMjsJEREZ2sCgYEAw53ocKNDa9nwsgic4kGGp2yjqcNofKqoKHjh -bXrRvOlHW63lWfAjjUmgnqfyWP6a5sVD7sRoWauDC8HUreLpxKJHoiozcAo6LHNN -zkTIaOkJfOnceTiFl1rFgZSOnA5uG2WaDWOKWpWGJ1ISvutrRsX4lscN84P/ksYg -2hxuV9MCgYEAsLx2YX3nVjtw2vO1hbdOBV1w7JI9nZkGTZ8pn8P07G/t4acNVU5s -NEG8fGYyhHLTrklM+H7GWxVxaIHwTOCnF7cHr2WGi5x/jIx0sxYXR0uZ6dUgAnwv -Tq0C6DNAYD7ZXBeB595sGbdrYA9U5g63noJIU2iSRdgRLWQagljJpxY= ------END RSA PRIVATE KEY-----