From bf0c6d0844f2ca2b853d9cbdaf1b5d924f31f296 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Fri, 4 Dec 2015 13:47:54 -0800 Subject: [PATCH 1/2] Fix bug with ED25519 cryptoservice's ListKeys Signed-off-by: Ying Li --- tuf/signed/ed25519.go | 6 ++++-- tuf/signed/ed25519_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 tuf/signed/ed25519_test.go diff --git a/tuf/signed/ed25519.go b/tuf/signed/ed25519.go index 6ee5a3c4fb..3f7ad1ed28 100644 --- a/tuf/signed/ed25519.go +++ b/tuf/signed/ed25519.go @@ -46,8 +46,10 @@ func (e *Ed25519) RemoveKey(keyID string) error { // ListKeys returns the list of keys IDs for the role func (e *Ed25519) ListKeys(role string) []string { keyIDs := make([]string, 0, len(e.keys)) - for id := range e.keys { - keyIDs = append(keyIDs, id) + for id, edCryptoKey := range e.keys { + if edCryptoKey.role == role { + keyIDs = append(keyIDs, id) + } } return keyIDs } diff --git a/tuf/signed/ed25519_test.go b/tuf/signed/ed25519_test.go new file mode 100644 index 0000000000..18e0ccff2b --- /dev/null +++ b/tuf/signed/ed25519_test.go @@ -0,0 +1,24 @@ +package signed + +import ( + "testing" + + "github.com/docker/notary/tuf/data" + "github.com/stretchr/testify/assert" +) + +// ListKeys only returns the keys for that role +func TestListKeys(t *testing.T) { + c := NewEd25519() + tskey, err := c.Create(data.CanonicalTimestampRole, data.ED25519Key) + assert.NoError(t, err) + + _, err = c.Create(data.CanonicalRootRole, data.ED25519Key) + assert.NoError(t, err) + + tsKeys := c.ListKeys(data.CanonicalTimestampRole) + assert.Len(t, tsKeys, 1) + assert.Equal(t, tskey.ID(), tsKeys[0]) + + assert.Len(t, c.ListKeys(data.CanonicalTargetsRole), 0) +} From d59ae2d90fb474839a5ee0b164edda31ed238e0e Mon Sep 17 00:00:00 2001 From: Ying Li Date: Fri, 4 Dec 2015 13:56:03 -0800 Subject: [PATCH 2/2] Add the handler for GET-ting a snapshot key. Signed-off-by: Ying Li --- server/handlers/default.go | 44 ++++++++--- server/handlers/default_test.go | 128 ++++++++++++++++++++++++++++++-- server/server.go | 7 +- server/server_test.go | 29 ++++++++ 4 files changed, 189 insertions(+), 19 deletions(-) diff --git a/server/handlers/default.go b/server/handlers/default.go index 7b56bc774a..c15e3bd23b 100644 --- a/server/handlers/default.go +++ b/server/handlers/default.go @@ -208,46 +208,70 @@ func getSnapshot(ctx context.Context, w http.ResponseWriter, logger ctxu.Logger, return nil } -// GetTimestampKeyHandler returns a timestamp public key, creating a new key-pair +// GetKeyHandler returns a public key for the specified role, creating a new key-pair // it if it doesn't yet exist -func GetTimestampKeyHandler(ctx context.Context, w http.ResponseWriter, r *http.Request) error { +func GetKeyHandler(ctx context.Context, w http.ResponseWriter, r *http.Request) error { + defer r.Body.Close() vars := mux.Vars(r) - gun := vars["imageName"] + return getKeyHandler(ctx, w, r, vars) +} + +func getKeyHandler(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { + gun, ok := vars["imageName"] + if !ok || gun == "" { + return errors.ErrUnknown.WithDetail("no gun") + } + role, ok := vars["tufRole"] + if !ok || role == "" { + return errors.ErrUnknown.WithDetail("no role") + } logger := ctxu.GetLoggerWithField(ctx, gun, "gun") s := ctx.Value("metaStore") store, ok := s.(storage.MetaStore) - if !ok { + if !ok || store == nil { logger.Error("500 GET storage not configured") return errors.ErrNoStorage.WithDetail(nil) } c := ctx.Value("cryptoService") crypto, ok := c.(signed.CryptoService) - if !ok { + if !ok || crypto == nil { logger.Error("500 GET crypto service not configured") return errors.ErrNoCryptoService.WithDetail(nil) } algo := ctx.Value("keyAlgorithm") keyAlgo, ok := algo.(string) - if !ok { + if !ok || keyAlgo == "" { logger.Error("500 GET key algorithm not configured") return errors.ErrNoKeyAlgorithm.WithDetail(nil) } keyAlgorithm := keyAlgo - key, err := timestamp.GetOrCreateTimestampKey(gun, store, crypto, keyAlgorithm) + var ( + key data.PublicKey + err error + ) + switch role { + case data.CanonicalTimestampRole: + key, err = timestamp.GetOrCreateTimestampKey(gun, store, crypto, keyAlgorithm) + case data.CanonicalSnapshotRole: + key, err = snapshot.GetOrCreateSnapshotKey(gun, store, crypto, keyAlgorithm) + default: + logger.Errorf("400 GET %s key: %v", role, err) + return errors.ErrInvalidRole.WithDetail(role) + } if err != nil { - logger.Errorf("500 GET timestamp key: %v", err) + logger.Errorf("500 GET %s key: %v", role, err) return errors.ErrUnknown.WithDetail(err) } out, err := json.Marshal(key) if err != nil { - logger.Error("500 GET timestamp key") + logger.Errorf("500 GET %s key", role) return errors.ErrUnknown.WithDetail(err) } - logger.Debug("200 GET timestamp key") + logger.Debugf("200 GET %s key", role) w.Write(out) return nil } diff --git a/server/handlers/default_test.go b/server/handlers/default_test.go index b4d1652cea..3164782957 100644 --- a/server/handlers/default_test.go +++ b/server/handlers/default_test.go @@ -10,6 +10,7 @@ import ( "golang.org/x/net/context" + ctxu "github.com/docker/distribution/context" "github.com/docker/notary/server/storage" "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/signed" @@ -19,6 +20,29 @@ import ( "github.com/stretchr/testify/assert" ) +type handlerState struct { + // interface{} so we can test invalid values + store interface{} + crypto interface{} + keyAlgo interface{} +} + +func defaultState() handlerState { + return handlerState{ + store: storage.NewMemStorage(), + crypto: signed.NewEd25519(), + keyAlgo: data.ED25519Key, + } +} + +func getContext(h handlerState) context.Context { + ctx := context.Background() + ctx = context.WithValue(ctx, "metaStore", h.store) + ctx = context.WithValue(ctx, "keyAlgorithm", h.keyAlgo) + ctx = context.WithValue(ctx, "cryptoService", h.crypto) + return ctxu.WithLogger(ctx, ctxu.GetRequestLogger(ctx)) +} + func TestMainHandlerGet(t *testing.T) { hand := utils.RootHandlerFactory(nil, context.Background(), &signed.Ed25519{}) handler := hand(MainHandler) @@ -46,6 +70,102 @@ func TestMainHandlerNotGet(t *testing.T) { } } +// GetKeyHandler needs to have access to a metadata store and cryptoservice, +// a key algorithm +func TestGetKeyHandlerInvalidConfiguration(t *testing.T) { + noStore := defaultState() + noStore.store = nil + + invalidStore := defaultState() + invalidStore.store = "not a store" + + noCrypto := defaultState() + noCrypto.crypto = nil + + invalidCrypto := defaultState() + invalidCrypto.crypto = "not a cryptoservice" + + noKeyAlgo := defaultState() + noKeyAlgo.keyAlgo = "" + + invalidKeyAlgo := defaultState() + invalidKeyAlgo.keyAlgo = 1 + + invalidStates := map[string][]handlerState{ + "no storage": {noStore, invalidStore}, + "no cryptoservice": {noCrypto, invalidCrypto}, + "no keyalgorithm": {noKeyAlgo, invalidKeyAlgo}, + } + + vars := map[string]string{ + "imageName": "gun", + "tufRole": data.CanonicalTimestampRole, + } + req := &http.Request{Body: ioutil.NopCloser(bytes.NewBuffer(nil))} + for errString, states := range invalidStates { + for _, s := range states { + err := getKeyHandler(getContext(s), httptest.NewRecorder(), req, vars) + assert.Error(t, err) + assert.Contains(t, err.Error(), errString) + } + } +} + +// GetKeyHandler needs to be set up such that an imageName and tufRole are both +// provided and non-empty. +func TestGetKeyHandlerNoRoleOrRepo(t *testing.T) { + state := defaultState() + req := &http.Request{Body: ioutil.NopCloser(bytes.NewBuffer(nil))} + + for _, key := range []string{"imageName", "tufRole"} { + vars := map[string]string{ + "imageName": "gun", + "tufRole": data.CanonicalTimestampRole, + } + + // not provided + delete(vars, key) + err := getKeyHandler(getContext(state), httptest.NewRecorder(), req, vars) + assert.Error(t, err) + assert.Contains(t, err.Error(), "unknown") + + // empty + vars[key] = "" + err = getKeyHandler(getContext(state), httptest.NewRecorder(), req, vars) + assert.Error(t, err) + assert.Contains(t, err.Error(), "unknown") + } +} + +// Getting a key for a non-supported role results in a 400. +func TestGetKeyHandlerInvalidRole(t *testing.T) { + state := defaultState() + vars := map[string]string{ + "imageName": "gun", + "tufRole": data.CanonicalRootRole, + } + req := &http.Request{Body: ioutil.NopCloser(bytes.NewBuffer(nil))} + + err := getKeyHandler(getContext(state), httptest.NewRecorder(), req, vars) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid role") +} + +// Getting the key for a valid role and gun succeeds +func TestGetKeyHandlerCreatesOnce(t *testing.T) { + state := defaultState() + roles := []string{data.CanonicalTimestampRole, data.CanonicalSnapshotRole} + req := &http.Request{Body: ioutil.NopCloser(bytes.NewBuffer(nil))} + + for _, role := range roles { + vars := map[string]string{"imageName": "gun", "tufRole": role} + recorder := httptest.NewRecorder() + err := getKeyHandler(getContext(state), recorder, req, vars) + assert.NoError(t, err) + assert.True(t, len(recorder.Body.String()) > 0) + } +} + func TestGetHandlerRoot(t *testing.T) { store := storage.NewMemStorage() _, repo, _ := testutils.EmptyRepo() @@ -77,9 +197,7 @@ func TestGetHandlerTimestamp(t *testing.T) { store := storage.NewMemStorage() _, repo, crypto := testutils.EmptyRepo() - ctx := context.Background() - ctx = context.WithValue(ctx, "metaStore", store) - ctx = context.WithValue(ctx, "cryptoService", crypto) + ctx := getContext(handlerState{store: store, crypto: crypto}) sn, err := repo.SignSnapshot(data.DefaultExpires("snapshot")) snJSON, err := json.Marshal(sn) @@ -110,9 +228,7 @@ func TestGetHandlerSnapshot(t *testing.T) { store := storage.NewMemStorage() _, repo, crypto := testutils.EmptyRepo() - ctx := context.Background() - ctx = context.WithValue(ctx, "metaStore", store) - ctx = context.WithValue(ctx, "cryptoService", crypto) + ctx := getContext(handlerState{store: store, crypto: crypto}) sn, err := repo.SignSnapshot(data.DefaultExpires("snapshot")) snJSON, err := json.Marshal(sn) diff --git a/server/server.go b/server/server.go index de16c0392c..cb09a48e5b 100644 --- a/server/server.go +++ b/server/server.go @@ -93,10 +93,11 @@ func RootHandler(ac auth.AccessController, ctx context.Context, trust signed.Cry prometheus.InstrumentHandlerWithOpts( prometheusOpts("GetRole"), hand(handlers.GetHandler, "pull"))) - r.Methods("GET").Path("/v2/{imageName:.*}/_trust/tuf/timestamp.key").Handler( + r.Methods("GET").Path( + "/v2/{imageName:.*}/_trust/tuf/{tufRole:(snapshot|timestamp)}.key").Handler( prometheus.InstrumentHandlerWithOpts( - prometheusOpts("GetTimestampKey"), - hand(handlers.GetTimestampKeyHandler, "push", "pull"))) + prometheusOpts("GetKey"), + hand(handlers.GetKeyHandler, "push", "pull"))) r.Methods("DELETE").Path("/v2/{imageName:.*}/_trust/tuf/").Handler( prometheus.InstrumentHandlerWithOpts( prometheusOpts("DeleteTuf"), diff --git a/server/server_test.go b/server/server_test.go index 986eaa1069..978d952b51 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1,6 +1,7 @@ package server import ( + "fmt" "net" "net/http" "net/http/httptest" @@ -8,6 +9,8 @@ import ( "testing" _ "github.com/docker/distribution/registry/auth/silly" + "github.com/docker/notary/server/storage" + "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/signed" "github.com/stretchr/testify/assert" "golang.org/x/net/context" @@ -56,3 +59,29 @@ func TestMetricsEndpoint(t *testing.T) { assert.NoError(t, err) assert.Equal(t, http.StatusOK, res.StatusCode) } + +// GetKeys supports only the timestamp and snapshot key endpoints +func TestGetKeysEndpoint(t *testing.T) { + ctx := context.WithValue( + context.Background(), "metaStore", storage.NewMemStorage()) + ctx = context.WithValue(ctx, "keyAlgorithm", data.ED25519Key) + + handler := RootHandler(nil, ctx, signed.NewEd25519()) + ts := httptest.NewServer(handler) + defer ts.Close() + + rolesToStatus := map[string]int{ + data.CanonicalTimestampRole: http.StatusOK, + data.CanonicalSnapshotRole: http.StatusOK, + data.CanonicalTargetsRole: http.StatusNotFound, + data.CanonicalRootRole: http.StatusNotFound, + "somerandomrole": http.StatusNotFound, + } + + for role, expectedStatus := range rolesToStatus { + res, err := http.Get( + fmt.Sprintf("%s/v2/gun/_trust/tuf/%s.key", ts.URL, role)) + assert.NoError(t, err) + assert.Equal(t, expectedStatus, res.StatusCode) + } +}