From a03a7f623f91e20e1c92f2724be9cab4d50a68bc Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Mon, 18 Jul 2016 13:20:57 -0700 Subject: [PATCH 1/4] Refactor passphrase to ask for snapshot and targets passphrases separately, cache delegation passphrases Signed-off-by: Riyaz Faizullabhoy --- passphrase/passphrase.go | 38 ++++++++++------------------------- passphrase/passphrase_test.go | 37 +++++++++++++++++++++++++--------- 2 files changed, 39 insertions(+), 36 deletions(-) diff --git a/passphrase/passphrase.go b/passphrase/passphrase.go index 432500361d..3bea91236e 100644 --- a/passphrase/passphrase.go +++ b/passphrase/passphrase.go @@ -50,13 +50,10 @@ func PromptRetriever() notary.PassRetriever { } type boundRetriever struct { - in io.Reader - out io.Writer - aliasMap map[string]string - userEnteredTargetsSnapshotPass bool - targetsSnapshotPass string - userEnteredRootPass bool - rootPass string + in io.Reader + out io.Writer + aliasMap map[string]string + passphraseCache map[string]string } func (br *boundRetriever) getPassphrase(keyName, alias string, createNew bool, numAttempts int) (string, bool, error) { @@ -65,11 +62,8 @@ func (br *boundRetriever) getPassphrase(keyName, alias string, createNew bool, n fmt.Fprintln(br.out, tufRootKeyGenerationWarning) } - if br.userEnteredTargetsSnapshotPass && (alias == tufSnapshotAlias || alias == tufTargetsAlias) { - return br.targetsSnapshotPass, false, nil - } - if br.userEnteredRootPass && (alias == "root") { - return br.rootPass, false, nil + if pass, ok := br.passphraseCache[alias]; ok { + return pass, false, nil } } else if !createNew { // per `if`, numAttempts > 0 if we're at this `else` if numAttempts > 3 { @@ -173,14 +167,7 @@ func (br *boundRetriever) verifyAndConfirmPassword(stdin *bufio.Reader, retPass, } func (br *boundRetriever) cachePassword(alias, retPass string) { - if alias == tufSnapshotAlias || alias == tufTargetsAlias { - br.userEnteredTargetsSnapshotPass = true - br.targetsSnapshotPass = retPass - } - if alias == tufRootAlias { - br.userEnteredRootPass = true - br.rootPass = retPass - } + br.passphraseCache[alias] = retPass } // PromptRetrieverWithInOut returns a new Retriever which will provide a @@ -190,13 +177,10 @@ func (br *boundRetriever) cachePassword(alias, retPass string) { // is nil, a sensible default will be used. func PromptRetrieverWithInOut(in io.Reader, out io.Writer, aliasMap map[string]string) notary.PassRetriever { bound := &boundRetriever{ - in: in, - out: out, - aliasMap: aliasMap, - userEnteredTargetsSnapshotPass: false, - targetsSnapshotPass: "", - userEnteredRootPass: false, - rootPass: "", + in: in, + out: out, + aliasMap: aliasMap, + passphraseCache: make(map[string]string), } return bound.getPassphrase diff --git a/passphrase/passphrase_test.go b/passphrase/passphrase_test.go index ebfb3c497b..9aaaf47c06 100644 --- a/passphrase/passphrase_test.go +++ b/passphrase/passphrase_test.go @@ -69,11 +69,9 @@ func TestGetPassphraseForCreatingDelegationKey(t *testing.T) { require.Equal(t, expectedText, lines) } -// PromptRetrieverWithInOut, if asked for root, targets, delegation, and -// snapshot passphrases in that order will only prompt for root, targets, and -// delegation passphrases because it caches the targets password and uses it -// for snapshot. -func TestGetRootTargetsDelegation(t *testing.T) { +// PromptRetrieverWithInOut, if asked for root, targets, snapshot, and delegation +// passphrases in that order will cache each of the keys except for the delegation key +func TestRolePromptingAndCaching(t *testing.T) { var in bytes.Buffer var out bytes.Buffer @@ -81,16 +79,37 @@ func TestGetRootTargetsDelegation(t *testing.T) { assertAskOnceForKey(t, &in, &out, retriever, "rootpassword", data.CanonicalRootRole) assertAskOnceForKey(t, &in, &out, retriever, "targetspassword", data.CanonicalTargetsRole) + assertAskOnceForKey(t, &in, &out, retriever, "snapshotpassword", data.CanonicalSnapshotRole) assertAskOnceForKey(t, &in, &out, retriever, "delegationpass", "targets/delegation") - // now ask for snapshot password, but it should already be cached, it - // won't ask and no input necessary. - pass, giveUp, err := retriever("repo/0123456789abcdef", data.CanonicalSnapshotRole, false, 0) + // ask for root password, but it should already be cached + pass, giveUp, err := retriever("repo/0123456789abcdef", data.CanonicalRootRole, false, 0) + require.NoError(t, err) + require.False(t, giveUp) + require.Equal(t, "rootpassword", pass) + + // ask for targets password, but it should already be cached + pass, giveUp, err = retriever("repo/0123456789abcdef", data.CanonicalTargetsRole, false, 0) require.NoError(t, err) require.False(t, giveUp) require.Equal(t, "targetspassword", pass) + // ask for snapshot password, but it should already be cached + pass, giveUp, err = retriever("repo/0123456789abcdef", data.CanonicalSnapshotRole, false, 0) + require.NoError(t, err) + require.False(t, giveUp) + require.Equal(t, "snapshotpassword", pass) + + // ask for targets/delegation password, but it should already be cached + pass, giveUp, err = retriever("repo/0123456789abcdef", "targets/delegation", false, 0) + require.NoError(t, err) + require.False(t, giveUp) + require.Equal(t, "delegationpass", pass) + + // ask for different delegation password, which should not be cached + _, _, err = retriever("repo/0123456789abcdef", "targets/delegation/new", false, 0) + require.Error(t, err) text, err := ioutil.ReadAll(&out) require.NoError(t, err) - require.Empty(t, text) + require.Contains(t, string(text), "Enter passphrase for targets/delegation/new key with ID 0123456 (repo):") } From b82863152fc7580017bb4bd2935ad30456c84546 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Wed, 20 Jul 2016 23:26:43 -0700 Subject: [PATCH 2/4] Ensure what we do not clobber local key id data for nested delegations on list Signed-off-by: Riyaz Faizullabhoy --- client/delegations.go | 17 ++++++----- cmd/notary/integration_test.go | 52 ++++++++++++++++++++++++++++++++++ cmd/notary/prettyprint.go | 4 +-- cmd/notary/prettyprint_test.go | 4 +-- 4 files changed, 66 insertions(+), 11 deletions(-) diff --git a/client/delegations.go b/client/delegations.go index 5fbee5af2b..2c972a8240 100644 --- a/client/delegations.go +++ b/client/delegations.go @@ -238,7 +238,7 @@ func newDeleteDelegationChange(name string, content []byte) *changelist.TUFChang // GetDelegationRoles returns the keys and roles of the repository's delegations // Also converts key IDs to canonical key IDs to keep consistent with signing prompts -func (r *NotaryRepository) GetDelegationRoles() ([]*data.Role, error) { +func (r *NotaryRepository) GetDelegationRoles() ([]data.Role, error) { // Update state of the repo to latest if err := r.Update(false); err != nil { return nil, err @@ -251,7 +251,7 @@ func (r *NotaryRepository) GetDelegationRoles() ([]*data.Role, error) { } // make a copy for traversing nested delegations - allDelegations := []*data.Role{} + allDelegations := []data.Role{} // Define a visitor function to populate the delegations list and translate their key IDs to canonical IDs delegationCanonicalListVisitor := func(tgt *data.SignedTargets, validRole data.DelegationRole) interface{} { @@ -271,20 +271,23 @@ func (r *NotaryRepository) GetDelegationRoles() ([]*data.Role, error) { return allDelegations, nil } -func translateDelegationsToCanonicalIDs(delegationInfo data.Delegations) ([]*data.Role, error) { - canonicalDelegations := make([]*data.Role, len(delegationInfo.Roles)) - copy(canonicalDelegations, delegationInfo.Roles) +func translateDelegationsToCanonicalIDs(delegationInfo data.Delegations) ([]data.Role, error) { + canonicalDelegations := make([]data.Role, len(delegationInfo.Roles)) + // Do a copy by value to ensure local delegation metadata is untouched + for idx, origRole := range delegationInfo.Roles { + canonicalDelegations[idx] = *origRole + } delegationKeys := delegationInfo.Keys for i, delegation := range canonicalDelegations { canonicalKeyIDs := []string{} for _, keyID := range delegation.KeyIDs { pubKey, ok := delegationKeys[keyID] if !ok { - return nil, fmt.Errorf("Could not translate canonical key IDs for %s", delegation.Name) + return []data.Role{}, fmt.Errorf("Could not translate canonical key IDs for %s", delegation.Name) } canonicalKeyID, err := utils.CanonicalKeyID(pubKey) if err != nil { - return nil, fmt.Errorf("Could not translate canonical key IDs for %s: %v", delegation.Name, err) + return []data.Role{}, fmt.Errorf("Could not translate canonical key IDs for %s: %v", delegation.Name, err) } canonicalKeyIDs = append(canonicalKeyIDs, canonicalKeyID) } diff --git a/cmd/notary/integration_test.go b/cmd/notary/integration_test.go index 3b6761b847..e5931d0433 100644 --- a/cmd/notary/integration_test.go +++ b/cmd/notary/integration_test.go @@ -946,6 +946,58 @@ func TestClientDelegationsPublishing(t *testing.T) { output, err = runCommand(t, tempDir, "-s", server.URL, "list", "gun", "--roles", "targets/releases") require.NoError(t, err) require.Contains(t, output, "targets/releases") + + // Setup another certificate + tempFile2, err := ioutil.TempFile("", "pemfile2") + require.NoError(t, err) + + privKey, err = utils.GenerateECDSAKey(rand.Reader) + startTime = time.Now() + endTime = startTime.AddDate(10, 0, 0) + cert, err = cryptoservice.GenerateCertificate(privKey, "gun", startTime, endTime) + require.NoError(t, err) + + _, err = tempFile2.Write(utils.CertToPEM(cert)) + require.NoError(t, err) + require.NoError(t, err) + tempFile2.Close() + defer os.Remove(tempFile2.Name()) + + rawPubBytes2, _ := ioutil.ReadFile(tempFile2.Name()) + parsedPubKey2, _ := utils.ParsePEMPublicKey(rawPubBytes2) + keyID2, err := utils.CanonicalKeyID(parsedPubKey2) + require.NoError(t, err) + + // add a nested delegation under this releases role + output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/releases/nested", tempFile2.Name(), "--paths", "nested/path") + require.NoError(t, err) + require.Contains(t, output, "Addition of delegation role") + require.Contains(t, output, keyID2) + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + require.NoError(t, err) + + // list delegations - we should see two roles + output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + require.NoError(t, err) + require.Contains(t, output, "targets/releases") + require.Contains(t, output, "targets/releases/nested") + require.Contains(t, output, canonicalKeyID) + require.Contains(t, output, keyID2) + require.Contains(t, output, "nested/path") + require.Contains(t, output, "\"\"") + require.Contains(t, output, "") + + // remove by force to delete the nested delegation entirely + output, err = runCommand(t, tempDir, "delegation", "remove", "gun", "targets/releases/nested", "-y") + require.NoError(t, err) + require.Contains(t, output, "Forced removal (including all keys and paths) of delegation role") + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + require.NoError(t, err) + } // Splits a string into lines, and returns any lines that are not empty ( diff --git a/cmd/notary/prettyprint.go b/cmd/notary/prettyprint.go index 5fcdbf0275..51201870ec 100644 --- a/cmd/notary/prettyprint.go +++ b/cmd/notary/prettyprint.go @@ -140,7 +140,7 @@ func (t targetsSorter) Less(i, j int) bool { // --- pretty printing roles --- -type roleSorter []*data.Role +type roleSorter []data.Role func (r roleSorter) Len() int { return len(r) } func (r roleSorter) Swap(i, j int) { r[i], r[j] = r[j], r[i] } @@ -173,7 +173,7 @@ func prettyPrintTargets(ts []*client.TargetWithRole, writer io.Writer) { } // Pretty-prints the list of provided Roles -func prettyPrintRoles(rs []*data.Role, writer io.Writer, roleType string) { +func prettyPrintRoles(rs []data.Role, writer io.Writer, roleType string) { if len(rs) == 0 { writer.Write([]byte(fmt.Sprintf("\nNo %s present in this repository.\n\n", roleType))) return diff --git a/cmd/notary/prettyprint_test.go b/cmd/notary/prettyprint_test.go index 9eec0c64cf..10ed6407c2 100644 --- a/cmd/notary/prettyprint_test.go +++ b/cmd/notary/prettyprint_test.go @@ -205,7 +205,7 @@ func TestPrettyPrintSortedTargets(t *testing.T) { // are no roles. func TestPrettyPrintZeroRoles(t *testing.T) { var b bytes.Buffer - prettyPrintRoles([]*data.Role{}, &b, "delegations") + prettyPrintRoles([]data.Role{}, &b, "delegations") text, err := ioutil.ReadAll(&b) require.NoError(t, err) @@ -218,7 +218,7 @@ func TestPrettyPrintZeroRoles(t *testing.T) { func TestPrettyPrintSortedRoles(t *testing.T) { var err error - unsorted := []*data.Role{ + unsorted := []data.Role{ {Name: "targets/zebra", Paths: []string{"stripes", "black", "white"}, RootRole: data.RootRole{KeyIDs: []string{"101"}, Threshold: 1}}, {Name: "targets/aardvark/unicorn/pony", Paths: []string{"rainbows"}, RootRole: data.RootRole{KeyIDs: []string{"135"}, Threshold: 1}}, {Name: "targets/bee", Paths: []string{"honey"}, RootRole: data.RootRole{KeyIDs: []string{"246"}, Threshold: 1}}, From a7321282a3e19c8a29a52d9f0db14df30fe7f5b8 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Fri, 22 Jul 2016 11:13:06 -0700 Subject: [PATCH 3/4] Add to unit test to ensure delegation roles are respected and unchanged when listing and converting to canonical ID Signed-off-by: Riyaz Faizullabhoy --- client/client_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/client/client_test.go b/client/client_test.go index f45a910f82..e3727e8a2e 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -2325,6 +2325,25 @@ func TestPublishTargetsDelegationSuccessNeedsToDownloadRoles(t *testing.T) { // delegation parents all get signed ownerRec.requireAsked(t, []string{data.CanonicalTargetsRole, "targets/a"}) + // assert both delegation roles appear to the other repo in a call to GetDelegationRoles + delgRoleList, err := delgRepo.GetDelegationRoles() + require.NoError(t, err) + require.Len(t, delgRoleList, 2) + // The walk is a pre-order so we can enforce ordering. Also check that canonical key IDs are reported from this walk + require.Equal(t, delgRoleList[0].Name, "targets/a") + require.NotContains(t, delgRoleList[0].KeyIDs, ownerRepo.tufRepo.Targets[data.CanonicalTargetsRole].Signed.Delegations.Roles[0].KeyIDs) + canonicalAKeyID, err := utils.CanonicalKeyID(aKey) + require.NoError(t, err) + require.Contains(t, delgRoleList[0].KeyIDs, canonicalAKeyID) + require.Equal(t, delgRoleList[1].Name, "targets/a/b") + require.NotContains(t, delgRoleList[1].KeyIDs, ownerRepo.tufRepo.Targets["targets/a"].Signed.Delegations.Roles[0].KeyIDs) + canonicalBKeyID, err := utils.CanonicalKeyID(bKey) + require.NoError(t, err) + require.Contains(t, delgRoleList[1].KeyIDs, canonicalBKeyID) + // assert that the key ID data didn't somehow change between the two repos, since we translated to canonical key IDs + require.Equal(t, ownerRepo.tufRepo.Targets[data.CanonicalTargetsRole].Signed.Delegations.Roles[0].KeyIDs, delgRepo.tufRepo.Targets[data.CanonicalTargetsRole].Signed.Delegations.Roles[0].KeyIDs) + require.Equal(t, ownerRepo.tufRepo.Targets["targets/a"].Signed.Delegations.Roles[0].KeyIDs, delgRepo.tufRepo.Targets["targets/a"].Signed.Delegations.Roles[0].KeyIDs) + // delegated repo now publishes to delegated roles, but it will need // to download those roles first, since it doesn't know about them requirePublishToRolesSucceeds(t, delgRepo, []string{"targets/a/b"}, From 5d6bd65786a7f437f8f9bcb6eb1e53edaabd4acf Mon Sep 17 00:00:00 2001 From: Ying Li Date: Fri, 22 Jul 2016 15:48:52 -0700 Subject: [PATCH 4/4] Remove the HTTP endpoint for Notary Signer, as it's not used by anything. This removes the maintenance burden of keeping them in sync. Signed-off-by: Ying Li --- cmd/notary-signer/config.go | 27 +-- cmd/notary-signer/main.go | 9 +- cmd/notary-signer/main_test.go | 31 +--- docs/reference/signer-config.md | 22 +-- fixtures/signer-config-local.json | 1 - fixtures/signer-config.json | 1 - fixtures/signer-config.rethink.json | 1 - signer.Dockerfile | 2 - signer/api/api.go | 205 ----------------------- signer/api/api_test.go | 250 ---------------------------- signer/signer.go | 1 - 11 files changed, 12 insertions(+), 538 deletions(-) delete mode 100644 signer/api/api.go delete mode 100644 signer/api/api_test.go diff --git a/cmd/notary-signer/config.go b/cmd/notary-signer/config.go index 6565f93ade..2704b9b868 100644 --- a/cmd/notary-signer/config.go +++ b/cmd/notary-signer/config.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "net" - "net/http" "os" "strings" "time" @@ -62,7 +61,7 @@ func parseSignerConfig(configFilePath string) (signer.Config, error) { utils.SetUpBugsnag(bugsnagConf) // parse server config - httpAddr, grpcAddr, tlsConfig, err := getAddrAndTLSConfig(config) + grpcAddr, tlsConfig, err := getAddrAndTLSConfig(config) if err != nil { return signer.Config{}, err } @@ -74,7 +73,6 @@ func parseSignerConfig(configFilePath string) (signer.Config, error) { } return signer.Config{ - HTTPAddr: httpAddr, GRPCAddr: grpcAddr, TLSConfig: tlsConfig, CryptoServices: cryptoServices, @@ -213,33 +211,18 @@ func setupGRPCServer(grpcAddr string, tlsConfig *tls.Config, return grpcServer, lis, nil } -func setupHTTPServer(httpAddr string, tlsConfig *tls.Config, - cryptoServices signer.CryptoServiceIndex) *http.Server { - - return &http.Server{ - Addr: httpAddr, - Handler: api.Handlers(cryptoServices), - TLSConfig: tlsConfig, - } -} - -func getAddrAndTLSConfig(configuration *viper.Viper) (string, string, *tls.Config, error) { +func getAddrAndTLSConfig(configuration *viper.Viper) (string, *tls.Config, error) { tlsConfig, err := utils.ParseServerTLS(configuration, true) if err != nil { - return "", "", nil, fmt.Errorf("unable to set up TLS: %s", err.Error()) + return "", nil, fmt.Errorf("unable to set up TLS: %s", err.Error()) } grpcAddr := configuration.GetString("server.grpc_addr") if grpcAddr == "" { - return "", "", nil, fmt.Errorf("grpc listen address required for server") - } - - httpAddr := configuration.GetString("server.http_addr") - if httpAddr == "" { - return "", "", nil, fmt.Errorf("http listen address required for server") + return "", nil, fmt.Errorf("grpc listen address required for server") } - return httpAddr, grpcAddr, tlsConfig, nil + return grpcAddr, tlsConfig, nil } func bootstrap(s interface{}) error { diff --git a/cmd/notary-signer/main.go b/cmd/notary-signer/main.go index 864863da79..f6b3721019 100644 --- a/cmd/notary-signer/main.go +++ b/cmd/notary-signer/main.go @@ -58,18 +58,11 @@ func main() { logrus.Fatal(err.Error()) } - httpServer := setupHTTPServer(signerConfig.HTTPAddr, signerConfig.TLSConfig, signerConfig.CryptoServices) - if debug { log.Println("RPC server listening on", signerConfig.GRPCAddr) - log.Println("HTTP server listening on", signerConfig.HTTPAddr) } - go grpcServer.Serve(lis) - err = httpServer.ListenAndServeTLS("", "") - if err != nil { - log.Fatal("HTTPS server failed to start:", err) - } + grpcServer.Serve(lis) } func usage() { diff --git a/cmd/notary-signer/main_test.go b/cmd/notary-signer/main_test.go index d1b3a6e66b..16b0fc20e0 100644 --- a/cmd/notary-signer/main_test.go +++ b/cmd/notary-signer/main_test.go @@ -40,16 +40,15 @@ func configure(jsonConfig string) *viper.Viper { // error is propagated. func TestGetAddrAndTLSConfigInvalidTLS(t *testing.T) { invalids := []string{ - `{"server": {"http_addr": ":1234", "grpc_addr": ":2345"}}`, + `{"server": {"grpc_addr": ":2345"}}`, `{"server": { - "http_addr": ":1234", "grpc_addr": ":2345", "tls_cert_file": "nope", "tls_key_file": "nope" }}`, } for _, configJSON := range invalids { - _, _, _, err := getAddrAndTLSConfig(configure(configJSON)) + _, _, err := getAddrAndTLSConfig(configure(configJSON)) require.Error(t, err) require.Contains(t, err.Error(), "unable to set up TLS") } @@ -57,9 +56,8 @@ func TestGetAddrAndTLSConfigInvalidTLS(t *testing.T) { // If a GRPC address is not provided, an error is returned. func TestGetAddrAndTLSConfigNoGRPCAddr(t *testing.T) { - _, _, _, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{ + _, _, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{ "server": { - "http_addr": ":1234", "tls_cert_file": "%s", "tls_key_file": "%s" } @@ -68,31 +66,16 @@ func TestGetAddrAndTLSConfigNoGRPCAddr(t *testing.T) { require.Contains(t, err.Error(), "grpc listen address required for server") } -// If an HTTP address is not provided, an error is returned. -func TestGetAddrAndTLSConfigNoHTTPAddr(t *testing.T) { - _, _, _, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{ - "server": { - "grpc_addr": ":1234", - "tls_cert_file": "%s", - "tls_key_file": "%s" - } - }`, Cert, Key))) - require.Error(t, err) - require.Contains(t, err.Error(), "http listen address required for server") -} - // Success parsing a valid TLS config, HTTP address, and GRPC address. func TestGetAddrAndTLSConfigSuccess(t *testing.T) { - httpAddr, grpcAddr, tlsConf, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{ + grpcAddr, tlsConf, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{ "server": { - "http_addr": ":2345", "grpc_addr": ":1234", "tls_cert_file": "%s", "tls_key_file": "%s" } }`, Cert, Key))) require.NoError(t, err) - require.Equal(t, ":2345", httpAddr) require.Equal(t, ":1234", grpcAddr) require.NotNil(t, tlsConf) } @@ -241,12 +224,6 @@ func TestSetupCryptoServicesInvalidStore(t *testing.T) { require.Equal(t, err.Error(), fmt.Sprintf("%s is not an allowed backend, must be one of: %s", "invalid_backend", []string{notary.SQLiteBackend, notary.MemoryBackend, notary.RethinkDBBackend})) } -func TestSetupHTTPServer(t *testing.T) { - httpServer := setupHTTPServer(":4443", nil, make(signer.CryptoServiceIndex)) - require.Equal(t, ":4443", httpServer.Addr) - require.Nil(t, httpServer.TLSConfig) -} - func TestSetupGRPCServerInvalidAddress(t *testing.T) { _, _, err := setupGRPCServer("nope", nil, make(signer.CryptoServiceIndex)) require.Error(t, err) diff --git a/docs/reference/signer-config.md b/docs/reference/signer-config.md index e9f1450bbb..7ced28e175 100644 --- a/docs/reference/signer-config.md +++ b/docs/reference/signer-config.md @@ -25,7 +25,6 @@ learn more about the configuration section corresponding to that key:
{
   "server": {
-    "http_addr": ":4444",
     "grpc_addr": ":7899",
     "tls_cert_file": "./fixtures/notary-signer.crt",
     "tls_key_file": "./fixtures/notary-signer.key",
@@ -57,7 +56,6 @@ Example:
 
 ```json
 "server": {
-  "http_addr": ":4444",
   "grpc_addr": ":7899",
   "tls_cert_file": "./fixtures/notary-signer.crt",
   "tls_key_file": "./fixtures/notary-signer.key",
@@ -71,22 +69,6 @@ Example:
 		Required
 		Description
 	
-	
-		http_addr
-		yes
-		The TCP address (IP and port) to listen for HTTP
-			traffic on.  Examples:
-			
    -
  • ":4444" means listen on port 4444 on all IPs (and - hence all interfaces, such as those listed when you run - ifconfig)
  • -
  • "127.0.0.1:4444" means listen on port 4444 on - localhost only. That means that the server will not be - accessible except locally (via SSH tunnel, or just on a local - terminal)
  • -
- - grpc_addr yes @@ -107,14 +89,14 @@ Example: tls_key_file yes The path to the private key to use for - HTTPS. The path is relative to the directory of the + GRPC TLS. The path is relative to the directory of the configuration file. tls_cert_file yes The path to the certificate to use for - HTTPS. The path is relative to the directory of the + GRPC TLS. The path is relative to the directory of the configuration file. diff --git a/fixtures/signer-config-local.json b/fixtures/signer-config-local.json index e5da0101e7..5eb18beca1 100644 --- a/fixtures/signer-config-local.json +++ b/fixtures/signer-config-local.json @@ -1,6 +1,5 @@ { "server": { - "http_addr": ":4444", "grpc_addr": ":7899", "tls_cert_file": "./notary-signer.crt", "tls_key_file": "./notary-signer.key", diff --git a/fixtures/signer-config.json b/fixtures/signer-config.json index 2af789a8dc..a14d7104ed 100644 --- a/fixtures/signer-config.json +++ b/fixtures/signer-config.json @@ -1,6 +1,5 @@ { "server": { - "http_addr": ":4444", "grpc_addr": ":7899", "tls_cert_file": "./notary-signer.crt", "tls_key_file": "./notary-signer.key", diff --git a/fixtures/signer-config.rethink.json b/fixtures/signer-config.rethink.json index fc0bc0d5ae..27a414b5ee 100644 --- a/fixtures/signer-config.rethink.json +++ b/fixtures/signer-config.rethink.json @@ -1,6 +1,5 @@ { "server": { - "http_addr": ":4444", "grpc_addr": ":7899", "tls_cert_file": "./notary-signer.crt", "tls_key_file": "./notary-signer.key", diff --git a/signer.Dockerfile b/signer.Dockerfile index 189cbdbee2..9c63f8ff93 100644 --- a/signer.Dockerfile +++ b/signer.Dockerfile @@ -17,8 +17,6 @@ ENV SERVICE_NAME=notary_signer ENV NOTARY_SIGNER_DEFAULT_ALIAS="timestamp_1" ENV NOTARY_SIGNER_TIMESTAMP_1="testpassword" -EXPOSE 4444 - # Install notary-signer RUN go install \ -tags pkcs11 \ diff --git a/signer/api/api.go b/signer/api/api.go deleted file mode 100644 index e71799fcce..0000000000 --- a/signer/api/api.go +++ /dev/null @@ -1,205 +0,0 @@ -package api - -import ( - "crypto/rand" - "encoding/json" - "fmt" - "net/http" - - "github.com/docker/notary/signer" - "github.com/docker/notary/signer/keys" - "github.com/docker/notary/tuf/signed" - "github.com/gorilla/mux" - - pb "github.com/docker/notary/proto" -) - -// Handlers sets up all the handers for the routes, injecting a specific CryptoService object for them to use -func Handlers(cryptoServices signer.CryptoServiceIndex) *mux.Router { - r := mux.NewRouter() - - r.Methods("GET").Path("/{ID}").Handler(KeyInfo(cryptoServices)) - r.Methods("POST").Path("/new/{Algorithm}").Handler(CreateKey(cryptoServices)) - r.Methods("POST").Path("/delete").Handler(DeleteKey(cryptoServices)) - r.Methods("POST").Path("/sign").Handler(Sign(cryptoServices)) - return r -} - -// getCryptoService handles looking up the correct signing service, given the -// algorithm specified in the HTTP request. If the algorithm isn't specified -// or isn't supported, an error is returned to the client and this function -// returns a nil CryptoService -func getCryptoService(algorithm string, cryptoServices signer.CryptoServiceIndex) (signed.CryptoService, error) { - if algorithm == "" { - return nil, fmt.Errorf("algorithm not specified") - } - - if service, ok := cryptoServices[algorithm]; ok { - return service, nil - } - - return nil, fmt.Errorf("algorithm " + algorithm + " not supported") -} - -// KeyInfo returns a Handler that given a specific Key ID param, returns the public key bits of that key -func KeyInfo(cryptoServices signer.CryptoServiceIndex) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - vars := mux.Vars(r) - - tufKey, _, err := FindKeyByID(cryptoServices, &pb.KeyID{ID: vars["ID"]}) - if err != nil { - switch err { - // If we received an ErrInvalidKeyID, the key doesn't exist, return 404 - case keys.ErrInvalidKeyID: - w.WriteHeader(http.StatusNotFound) - w.Write([]byte(err.Error())) - return - // If we received anything else, it is unexpected, and we return a 500 - default: - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - return - } - } - key := &pb.PublicKey{ - KeyInfo: &pb.KeyInfo{ - KeyID: &pb.KeyID{ID: tufKey.ID()}, - Algorithm: &pb.Algorithm{Algorithm: tufKey.Algorithm()}, - }, - PublicKey: tufKey.Public(), - } - json.NewEncoder(w).Encode(key) - return - }) -} - -// CreateKey returns a handler that generates a new key using the provided -// algorithm. Only the public component of the key is returned. -func CreateKey(cryptoServices signer.CryptoServiceIndex) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - vars := mux.Vars(r) - cryptoService, err := getCryptoService(vars["Algorithm"], cryptoServices) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - - tufKey, err := cryptoService.Create("", "", vars["Algorithm"]) - if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - return - } - key := &pb.PublicKey{ - KeyInfo: &pb.KeyInfo{ - KeyID: &pb.KeyID{ID: tufKey.ID()}, - Algorithm: &pb.Algorithm{Algorithm: tufKey.Algorithm()}, - }, - PublicKey: tufKey.Public(), - } - json.NewEncoder(w).Encode(key) - return - }) -} - -// DeleteKey returns a handler that delete a specific KeyID -func DeleteKey(cryptoServices signer.CryptoServiceIndex) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var keyID *pb.KeyID - err := json.NewDecoder(r.Body).Decode(&keyID) - defer r.Body.Close() - if err != nil || keyID.ID == "" { - w.WriteHeader(http.StatusBadRequest) - jsonErr, _ := json.Marshal("Malformed request") - w.Write([]byte(jsonErr)) - return - } - - _, cryptoService, err := FindKeyByID(cryptoServices, keyID) - - if err != nil { - switch err { - // If we received an ErrInvalidKeyID, the key doesn't exist, return 404 - case keys.ErrInvalidKeyID: - w.WriteHeader(http.StatusNotFound) - w.Write([]byte(err.Error())) - return - // If we received anything else, it is unexpected, and we return a 500 - default: - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - return - } - } - - if err = cryptoService.RemoveKey(keyID.ID); err != nil { - switch err { - // If we received an ErrInvalidKeyID, the key doesn't exist, return 404 - case keys.ErrInvalidKeyID: - w.WriteHeader(http.StatusNotFound) - w.Write([]byte(err.Error())) - return - // If we received anything else, it is unexpected, and we return a 500 - default: - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - return - } - } - // In case we successfully delete this key, return 200 - return - }) -} - -// Sign returns a handler that is able to perform signatures on a given blob -func Sign(cryptoServices signer.CryptoServiceIndex) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var sigRequest *pb.SignatureRequest - err := json.NewDecoder(r.Body).Decode(&sigRequest) - defer r.Body.Close() - if err != nil || sigRequest.Content == nil || - sigRequest.KeyID == nil { - w.WriteHeader(http.StatusBadRequest) - jsonErr, _ := json.Marshal("Malformed request") - w.Write([]byte(jsonErr)) - return - } - - tufKey, cryptoService, err := FindKeyByID(cryptoServices, sigRequest.KeyID) - if err == keys.ErrInvalidKeyID { - w.WriteHeader(http.StatusNotFound) - w.Write([]byte(err.Error())) - return - } else if err != nil { - // We got an unexpected error - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - return - } - - privKey, _, err := cryptoService.GetPrivateKey(tufKey.ID()) - if err != nil { - // We got an unexpected error - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - return - } - sig, err := privKey.Sign(rand.Reader, sigRequest.Content, nil) - if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - return - } - signature := &pb.Signature{ - KeyInfo: &pb.KeyInfo{ - KeyID: &pb.KeyID{ID: tufKey.ID()}, - Algorithm: &pb.Algorithm{Algorithm: tufKey.Algorithm()}, - }, - Algorithm: &pb.Algorithm{Algorithm: privKey.SignatureAlgorithm().String()}, - Content: sig, - } - - json.NewEncoder(w).Encode(signature) - return - }) -} diff --git a/signer/api/api_test.go b/signer/api/api_test.go deleted file mode 100644 index fec2dd63f3..0000000000 --- a/signer/api/api_test.go +++ /dev/null @@ -1,250 +0,0 @@ -package api_test - -import ( - "encoding/json" - "fmt" - "io" - "io/ioutil" - "net/http" - "net/http/httptest" - "strings" - "testing" - - "github.com/docker/notary/cryptoservice" - "github.com/docker/notary/signer" - "github.com/docker/notary/signer/api" - "github.com/docker/notary/trustmanager" - "github.com/docker/notary/tuf/data" - "github.com/stretchr/testify/require" - - pb "github.com/docker/notary/proto" -) - -var ( - server *httptest.Server - reader io.Reader - deleteKeyBaseURL string - createKeyBaseURL string - keyInfoBaseURL string - signBaseURL string - passphraseRetriever = func(string, string, bool, int) (string, bool, error) { return "passphrase", false, nil } -) - -func setup(cryptoServices signer.CryptoServiceIndex) { - server = httptest.NewServer(api.Handlers(cryptoServices)) - deleteKeyBaseURL = fmt.Sprintf("%s/delete", server.URL) - createKeyBaseURL = fmt.Sprintf("%s/new", server.URL) - keyInfoBaseURL = fmt.Sprintf("%s", server.URL) - signBaseURL = fmt.Sprintf("%s/sign", server.URL) -} - -func TestDeleteKeyHandlerReturns404WithNonexistentKey(t *testing.T) { - keyStore := trustmanager.NewKeyMemoryStore(passphraseRetriever) - cryptoService := cryptoservice.NewCryptoService(keyStore) - setup(signer.CryptoServiceIndex{data.ED25519Key: cryptoService, data.RSAKey: cryptoService, data.ECDSAKey: cryptoService}) - - fakeID := "c62e6d68851cef1f7e55a9d56e3b0c05f3359f16838cad43600f0554e7d3b54d" - - keyID := &pb.KeyID{ID: fakeID} - requestJSON, _ := json.Marshal(keyID) - reader = strings.NewReader(string(requestJSON)) - - request, err := http.NewRequest("POST", deleteKeyBaseURL, reader) - require.Nil(t, err) - - res, err := http.DefaultClient.Do(request) - require.Nil(t, err) - - require.Equal(t, 404, res.StatusCode) -} - -func TestDeleteKeyHandler(t *testing.T) { - keyStore := trustmanager.NewKeyMemoryStore(passphraseRetriever) - cryptoService := cryptoservice.NewCryptoService(keyStore) - setup(signer.CryptoServiceIndex{data.ED25519Key: cryptoService, data.RSAKey: cryptoService, data.ECDSAKey: cryptoService}) - - tufKey, _ := cryptoService.Create("", "", data.ED25519Key) - require.NotNil(t, tufKey) - - requestJSON, _ := json.Marshal(&pb.KeyID{ID: tufKey.ID()}) - reader = strings.NewReader(string(requestJSON)) - - request, err := http.NewRequest("POST", deleteKeyBaseURL, reader) - require.Nil(t, err) - - res, err := http.DefaultClient.Do(request) - require.Nil(t, err) - - require.Equal(t, 200, res.StatusCode) -} - -func TestKeyInfoHandler(t *testing.T) { - keyStore := trustmanager.NewKeyMemoryStore(passphraseRetriever) - cryptoService := cryptoservice.NewCryptoService(keyStore) - setup(signer.CryptoServiceIndex{data.ED25519Key: cryptoService, data.RSAKey: cryptoService, data.ECDSAKey: cryptoService}) - - tufKey, _ := cryptoService.Create("", "", data.ED25519Key) - require.NotNil(t, tufKey) - - keyInfoURL := fmt.Sprintf("%s/%s", keyInfoBaseURL, tufKey.ID()) - - request, err := http.NewRequest("GET", keyInfoURL, nil) - require.Nil(t, err) - - res, err := http.DefaultClient.Do(request) - require.Nil(t, err) - - jsonBlob, err := ioutil.ReadAll(res.Body) - require.Nil(t, err) - - var pubKey *pb.PublicKey - err = json.Unmarshal(jsonBlob, &pubKey) - require.Nil(t, err) - - require.Equal(t, tufKey.ID(), pubKey.KeyInfo.KeyID.ID) - require.Equal(t, 200, res.StatusCode) -} - -func TestKeyInfoHandlerReturns404WithNonexistentKey(t *testing.T) { - // We associate both key types with this signing service to bypass the - // ID -> keyType logic in the tests - keyStore := trustmanager.NewKeyMemoryStore(passphraseRetriever) - cryptoService := cryptoservice.NewCryptoService(keyStore) - setup(signer.CryptoServiceIndex{data.ED25519Key: cryptoService, data.RSAKey: cryptoService, data.ECDSAKey: cryptoService}) - - fakeID := "c62e6d68851cef1f7e55a9d56e3b0c05f3359f16838cad43600f0554e7d3b54d" - keyInfoURL := fmt.Sprintf("%s/%s", keyInfoBaseURL, fakeID) - - request, err := http.NewRequest("GET", keyInfoURL, nil) - require.Nil(t, err) - - res, err := http.DefaultClient.Do(request) - require.Nil(t, err) - - require.Equal(t, 404, res.StatusCode) -} - -func TestSoftwareCreateKeyHandler(t *testing.T) { - keyStore := trustmanager.NewKeyMemoryStore(passphraseRetriever) - cryptoService := cryptoservice.NewCryptoService(keyStore) - setup(signer.CryptoServiceIndex{data.ED25519Key: cryptoService, data.RSAKey: cryptoService, data.ECDSAKey: cryptoService}) - - createKeyURL := fmt.Sprintf("%s/%s", createKeyBaseURL, data.ED25519Key) - - request, err := http.NewRequest("POST", createKeyURL, nil) - require.Nil(t, err) - - res, err := http.DefaultClient.Do(request) - require.Nil(t, err) - - require.Equal(t, 200, res.StatusCode) - - jsonBlob, err := ioutil.ReadAll(res.Body) - require.Nil(t, err) - - var keyInfo *pb.PublicKey - err = json.Unmarshal(jsonBlob, &keyInfo) - require.Nil(t, err) -} - -func TestSoftwareSignHandler(t *testing.T) { - keyStore := trustmanager.NewKeyMemoryStore(passphraseRetriever) - cryptoService := cryptoservice.NewCryptoService(keyStore) - setup(signer.CryptoServiceIndex{data.ED25519Key: cryptoService, data.RSAKey: cryptoService, data.ECDSAKey: cryptoService}) - - tufKey, err := cryptoService.Create("", "", data.ED25519Key) - require.Nil(t, err) - - sigRequest := &pb.SignatureRequest{KeyID: &pb.KeyID{ID: tufKey.ID()}, Content: make([]byte, 10)} - requestJSON, _ := json.Marshal(sigRequest) - - reader = strings.NewReader(string(requestJSON)) - - request, err := http.NewRequest("POST", signBaseURL, reader) - - require.Nil(t, err) - - res, err := http.DefaultClient.Do(request) - require.Nil(t, err) - - require.Equal(t, 200, res.StatusCode) - - jsonBlob, err := ioutil.ReadAll(res.Body) - require.Nil(t, err) - - var sig *pb.Signature - err = json.Unmarshal(jsonBlob, &sig) - require.Nil(t, err) - - require.Equal(t, tufKey.ID(), sig.KeyInfo.KeyID.ID) -} - -func TestSoftwareSignWithInvalidRequestHandler(t *testing.T) { - keyStore := trustmanager.NewKeyMemoryStore(passphraseRetriever) - cryptoService := cryptoservice.NewCryptoService(keyStore) - setup(signer.CryptoServiceIndex{data.ED25519Key: cryptoService, data.RSAKey: cryptoService, data.ECDSAKey: cryptoService}) - - requestJSON := "{\"blob\":\"7d16f1d0b95310a7bc557747fc4f20fcd41c1c5095ae42f189df0717e7d7f4a0a2b55debce630f43c4ac099769c612965e3fda3cd4c0078ee6a460f14fa19307\"}" - reader = strings.NewReader(requestJSON) - - request, err := http.NewRequest("POST", signBaseURL, reader) - - require.Nil(t, err) - - res, err := http.DefaultClient.Do(request) - require.Nil(t, err) - - jsonBlob, err := ioutil.ReadAll(res.Body) - require.Nil(t, err) - - var sig *pb.Signature - err = json.Unmarshal(jsonBlob, &sig) - require.Error(t, err) - require.Equal(t, 400, res.StatusCode) -} - -func TestSignHandlerReturns404WithNonexistentKey(t *testing.T) { - keyStore := trustmanager.NewKeyMemoryStore(passphraseRetriever) - cryptoService := cryptoservice.NewCryptoService(keyStore) - setup(signer.CryptoServiceIndex{data.ED25519Key: cryptoService, data.RSAKey: cryptoService, data.ECDSAKey: cryptoService}) - - fakeID := "c62e6d68851cef1f7e55a9d56e3b0c05f3359f16838cad43600f0554e7d3b54d" - - cryptoService.Create("", "", data.ED25519Key) - - sigRequest := &pb.SignatureRequest{KeyID: &pb.KeyID{ID: fakeID}, Content: make([]byte, 10)} - requestJSON, _ := json.Marshal(sigRequest) - - reader = strings.NewReader(string(requestJSON)) - - request, err := http.NewRequest("POST", signBaseURL, reader) - require.Nil(t, err) - - res, err := http.DefaultClient.Do(request) - require.Nil(t, err) - - require.Equal(t, 404, res.StatusCode) -} - -func TestCreateKeyHandlerWithInvalidAlgorithm(t *testing.T) { - keyStore := trustmanager.NewKeyMemoryStore(passphraseRetriever) - cryptoService := cryptoservice.NewCryptoService(keyStore) - setup(signer.CryptoServiceIndex{data.ED25519Key: cryptoService, data.RSAKey: cryptoService, data.ECDSAKey: cryptoService}) - - // The `rbtree-algorithm` is expected as not supported - createKeyURL := fmt.Sprintf("%s/%s", createKeyBaseURL, "rbtree-algorithm") - - request, err := http.NewRequest("POST", createKeyURL, nil) - require.Nil(t, err) - - res, err := http.DefaultClient.Do(request) - require.Nil(t, err) - - require.Equal(t, http.StatusBadRequest, res.StatusCode) - - body, err := ioutil.ReadAll(res.Body) - require.Nil(t, err) - - // The body may contains some `\r\n`, so we use require.Contains not require.Equals - require.Contains(t, string(body), "algorithm rbtree-algorithm not supported") -} diff --git a/signer/signer.go b/signer/signer.go index f7a6c92dc0..074282ac97 100644 --- a/signer/signer.go +++ b/signer/signer.go @@ -38,7 +38,6 @@ type Signer interface { // Config tells how to configure a notary-signer type Config struct { - HTTPAddr string GRPCAddr string TLSConfig *tls.Config CryptoServices CryptoServiceIndex