diff --git a/client/helpers_test.go b/client/helpers_test.go index b54d62c2bd..847bdd4c26 100644 --- a/client/helpers_test.go +++ b/client/helpers_test.go @@ -4,6 +4,7 @@ import ( "bytes" "crypto/sha256" "encoding/json" + "net/http" "testing" "time" @@ -1019,3 +1020,15 @@ func TestAllNotNearExpiry(t *testing.T) { require.NotContains(t, a.String(), "snapshot is nearing expiry, you should re-sign the role metadata", "Snapshot should not show near expiry") require.NotContains(t, a.String(), "timestamp", "there should be no logrus warnings pertaining to timestamp") } + +func TestRotateRemoteKeyOffline(t *testing.T) { + // without a valid roundtripper, rotation should fail since we cannot initialize a HTTPStore + key, err := rotateRemoteKey("invalidURL", "gun", data.CanonicalSnapshotRole, nil) + require.Error(t, err) + require.Nil(t, key) + + // if the underlying remote store is faulty and cannot rotate keys, we should get back the error + key, err = rotateRemoteKey("https://notary-server", "gun", data.CanonicalSnapshotRole, http.DefaultTransport) + require.Error(t, err) + require.Nil(t, key) +} diff --git a/server/handlers/default.go b/server/handlers/default.go index 3664f06b18..3eb04437e9 100644 --- a/server/handlers/default.go +++ b/server/handlers/default.go @@ -184,43 +184,12 @@ func GetKeyHandler(ctx context.Context, w http.ResponseWriter, r *http.Request) } func getKeyHandler(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - gun, ok := vars["imageName"] - logger := ctxu.GetLoggerWithField(ctx, gun, "gun") - if !ok || gun == "" { - logger.Info("400 GET no gun in request") - return errors.ErrUnknown.WithDetail("no gun") - } - - role, ok := vars["tufRole"] - if !ok || role == "" { - logger.Info("400 GET no role in request") - return errors.ErrUnknown.WithDetail("no role") - } - - s := ctx.Value("metaStore") - store, ok := s.(storage.MetaStore) - 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 || 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 || keyAlgo == "" { - logger.Error("500 GET key algorithm not configured") - return errors.ErrNoKeyAlgorithm.WithDetail(nil) + role, gun, keyAlgorithm, store, crypto, err := setupKeyHandler(ctx, w, r, vars, http.MethodGet) + if err != nil { + return err } - keyAlgorithm := keyAlgo - - var ( - key data.PublicKey - err error - ) + var key data.PublicKey + logger := ctxu.GetLoggerWithField(ctx, gun, "gun") switch role { case data.CanonicalTimestampRole: key, err = timestamp.GetOrCreateTimestampKey(gun, store, crypto, keyAlgorithm) @@ -253,43 +222,12 @@ func RotateKeyHandler(ctx context.Context, w http.ResponseWriter, r *http.Reques } func rotateKeyHandler(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - gun, ok := vars["imageName"] - logger := ctxu.GetLoggerWithField(ctx, gun, "gun") - if !ok || gun == "" { - logger.Info("400 POST no gun in request") - return errors.ErrUnknown.WithDetail("no gun") - } - - role, ok := vars["tufRole"] - if !ok || role == "" { - logger.Info("400 POST no role in request") - return errors.ErrUnknown.WithDetail("no role") - } - - s := ctx.Value("metaStore") - store, ok := s.(storage.MetaStore) - if !ok || store == nil { - logger.Error("500 POST storage not configured") - return errors.ErrNoStorage.WithDetail(nil) - } - c := ctx.Value("cryptoService") - crypto, ok := c.(signed.CryptoService) - if !ok || crypto == nil { - logger.Error("500 POST crypto service not configured") - return errors.ErrNoCryptoService.WithDetail(nil) - } - algo := ctx.Value("keyAlgorithm") - keyAlgo, ok := algo.(string) - if !ok || keyAlgo == "" { - logger.Error("500 POST key algorithm not configured") - return errors.ErrNoKeyAlgorithm.WithDetail(nil) + role, gun, keyAlgorithm, store, crypto, err := setupKeyHandler(ctx, w, r, vars, http.MethodPost) + if err != nil { + return err } - keyAlgorithm := keyAlgo - - var ( - key data.PublicKey - err error - ) + var key data.PublicKey + logger := ctxu.GetLoggerWithField(ctx, gun, "gun") switch role { case data.CanonicalTimestampRole: key, err = timestamp.RotateTimestampKey(gun, store, crypto, keyAlgorithm) @@ -314,6 +252,43 @@ func rotateKeyHandler(ctx context.Context, w http.ResponseWriter, r *http.Reques return nil } +// To be called before getKeyHandler or rotateKeyHandler +func setupKeyHandler(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string, actionVerb string) (string, string, string, storage.MetaStore, signed.CryptoService, error) { + gun, ok := vars["imageName"] + logger := ctxu.GetLoggerWithField(ctx, gun, "gun") + if !ok || gun == "" { + logger.Infof("400 %s no gun in request", actionVerb) + return "", "", "", nil, nil, errors.ErrUnknown.WithDetail("no gun") + } + + role, ok := vars["tufRole"] + if !ok || role == "" { + logger.Infof("400 %s no role in request", actionVerb) + return "", "", "", nil, nil, errors.ErrUnknown.WithDetail("no role") + } + + s := ctx.Value("metaStore") + store, ok := s.(storage.MetaStore) + if !ok || store == nil { + logger.Errorf("500 %s storage not configured", actionVerb) + return "", "", "", nil, nil, errors.ErrNoStorage.WithDetail(nil) + } + c := ctx.Value("cryptoService") + crypto, ok := c.(signed.CryptoService) + if !ok || crypto == nil { + logger.Errorf("500 %s crypto service not configured", actionVerb) + return "", "", "", nil, nil, errors.ErrNoCryptoService.WithDetail(nil) + } + algo := ctx.Value("keyAlgorithm") + keyAlgo, ok := algo.(string) + if !ok || keyAlgo == "" { + logger.Errorf("500 %s key algorithm not configured", actionVerb) + return "", "", "", nil, nil, errors.ErrNoKeyAlgorithm.WithDetail(nil) + } + + return role, gun, keyAlgo, store, crypto, nil +} + // NotFoundHandler is used as a generic catch all handler to return the ErrMetadataNotFound // 404 response func NotFoundHandler(ctx context.Context, w http.ResponseWriter, r *http.Request) error { diff --git a/server/handlers/default_test.go b/server/handlers/default_test.go index 31c5f1a5b3..0d5e218103 100644 --- a/server/handlers/default_test.go +++ b/server/handlers/default_test.go @@ -193,8 +193,8 @@ func TestRotateKeyHandlerInvalidRole(t *testing.T) { } } -// Rotating the key fails if we don't pass a valid key algorithm, or if it isn't Ed25519 -func TestRotateKeyHandlerNonED25519(t *testing.T) { +// Rotating the key fails if we don't pass a valid key algorithm +func TestRotateKeyHandlerInvalidKeyAlgo(t *testing.T) { roles := []string{data.CanonicalTimestampRole, data.CanonicalSnapshotRole} req := &http.Request{Body: ioutil.NopCloser(bytes.NewBuffer(nil))} @@ -205,9 +205,6 @@ func TestRotateKeyHandlerNonED25519(t *testing.T) { invalidKeyAlgoState.keyAlgo = "notactuallyakeyalgorithm" err := rotateKeyHandler(getContext(invalidKeyAlgoState), recorder, req, vars) require.Error(t, err) - invalidKeyAlgoState.keyAlgo = data.ECDSAKey - err = rotateKeyHandler(getContext(invalidKeyAlgoState), recorder, req, vars) - require.Error(t, err) } } diff --git a/server/snapshot/snapshot.go b/server/snapshot/snapshot.go index 8c4161b607..23e133cd85 100644 --- a/server/snapshot/snapshot.go +++ b/server/snapshot/snapshot.go @@ -31,7 +31,7 @@ func GetOrCreateSnapshotKey(gun string, store storage.MetaStore, crypto signed.C return key, nil } - // If we have a current root, parse out the public key ID for the snapshot role and get it from the underlying cryptoservice + // If we have a current root, parse out the public key for the snapshot role, and return it repoSignedRoot := new(data.SignedRoot) if err := json.Unmarshal(rootJSON, repoSignedRoot); err != nil { logrus.Errorf("Failed to unmarshal existing root for GUN %s to retrieve snapshot key ID", gun) diff --git a/server/snapshot/snapshot_test.go b/server/snapshot/snapshot_test.go index 71126daa6b..6f2d7bfc31 100644 --- a/server/snapshot/snapshot_test.go +++ b/server/snapshot/snapshot_test.go @@ -4,6 +4,7 @@ import ( "bytes" "crypto/sha256" "encoding/hex" + "fmt" "testing" "time" @@ -51,6 +52,46 @@ func TestGetSnapshotKeyCreate(t *testing.T) { require.NotNil(t, k2, "Key should not be nil") } +type FailingStore struct { + *storage.MemStorage +} + +func (f FailingStore) GetCurrent(role, gun string) (*time.Time, []byte, error) { + return nil, nil, fmt.Errorf("failing store failed") +} + +func TestGetSnapshotKeyCreateWithFailingStore(t *testing.T) { + store := FailingStore{storage.NewMemStorage()} + crypto := signed.NewEd25519() + k, err := GetOrCreateSnapshotKey("gun", store, crypto, data.ED25519Key) + require.Error(t, err, "Expected error") + require.Nil(t, k, "Key should be nil") +} + +type CorruptedStore struct { + *storage.MemStorage +} + +func (c CorruptedStore) GetCurrent(role, gun string) (*time.Time, []byte, error) { + return &time.Time{}, []byte("junk"), nil +} + +func TestGetSnapshotKeyCreateWithCorruptedStore(t *testing.T) { + store := CorruptedStore{storage.NewMemStorage()} + crypto := signed.NewEd25519() + k, err := GetOrCreateSnapshotKey("gun", store, crypto, data.ED25519Key) + require.Error(t, err, "Expected error") + require.Nil(t, k, "Key should be nil") +} + +func TestGetSnapshotKeyCreateWithInvalidAlgo(t *testing.T) { + store := storage.NewMemStorage() + crypto := signed.NewEd25519() + k, err := GetOrCreateSnapshotKey("gun", store, crypto, "notactuallyanalgorithm") + require.Error(t, err, "Expected error") + require.Nil(t, k, "Key should be nil") +} + func TestGetSnapshotKeyExisting(t *testing.T) { repo, crypto, err := testutils.EmptyRepo("gun") diff --git a/server/storage/rethinkdb.go b/server/storage/rethinkdb.go index 83ff1cfb16..5801d80ce3 100644 --- a/server/storage/rethinkdb.go +++ b/server/storage/rethinkdb.go @@ -267,12 +267,10 @@ func (rdb RethinkDB) Bootstrap() error { // CheckHealth checks that all tables and databases exist and are query-able func (rdb RethinkDB) CheckHealth() error { - for _, table := range []string{TUFFilesRethinkTable.Name} { - res, err := gorethink.DB(rdb.dbName).Table(table).Info().Run(rdb.sess) - if err != nil { - return fmt.Errorf("%s is unavailable, or missing one or more tables, or permissions are incorrectly set", rdb.dbName) - } - defer res.Close() + res, err := gorethink.DB(rdb.dbName).Table(TUFFilesRethinkTable.Name).Info().Run(rdb.sess) + if err != nil { + return fmt.Errorf("%s is unavailable, or missing one or more tables, or permissions are incorrectly set", rdb.dbName) } + defer res.Close() return nil } diff --git a/server/storage/sqldb.go b/server/storage/sqldb.go index 8687b78553..c8449c774b 100644 --- a/server/storage/sqldb.go +++ b/server/storage/sqldb.go @@ -159,21 +159,15 @@ func (db *SQLStorage) Delete(gun string) error { return db.Unscoped().Where(&TUFFile{Gun: gun}).Delete(TUFFile{}).Error } -// CheckHealth asserts that both required tables are present +// CheckHealth asserts that the tuf_files table is present func (db *SQLStorage) CheckHealth() error { - interfaces := []interface { - TableName() string - }{&TUFFile{}} - - for _, model := range interfaces { - tableOk := db.HasTable(model) - if db.Error != nil { - return db.Error - } - if !tableOk { - return fmt.Errorf( - "Cannot access table: %s", model.TableName()) - } + tableOk := db.HasTable(&TUFFile{}) + if db.Error != nil { + return db.Error + } + if !tableOk { + return fmt.Errorf( + "Cannot access table: %s", TUFFile{}.TableName()) } return nil } diff --git a/server/storage/sqldb_test.go b/server/storage/sqldb_test.go index 9f24d7c07f..c3ccf843a3 100644 --- a/server/storage/sqldb_test.go +++ b/server/storage/sqldb_test.go @@ -24,11 +24,9 @@ func SetupSQLDB(t *testing.T, dbtype, dburl string) *SQLStorage { // verify that the tables are empty var count int - for _, model := range [1]interface{}{&TUFFile{}} { - query := dbStore.DB.Model(model).Count(&count) - require.NoError(t, query.Error) - require.Equal(t, 0, count) - } + query := dbStore.DB.Model(&TUFFile{}).Count(&count) + require.NoError(t, query.Error) + require.Equal(t, 0, count) return dbStore } diff --git a/server/timestamp/timestamp.go b/server/timestamp/timestamp.go index 3c0e9265cb..caea80fd5b 100644 --- a/server/timestamp/timestamp.go +++ b/server/timestamp/timestamp.go @@ -36,7 +36,7 @@ func GetOrCreateTimestampKey(gun string, store storage.MetaStore, crypto signed. return key, nil } - // If we have a current root, parse out the public key ID for the snapshot role and get it from the underlying cryptoservice + // If we have a current root, parse out the public key for the timestamp role, and return it repoSignedRoot := new(data.SignedRoot) if err := json.Unmarshal(rootJSON, repoSignedRoot); err != nil { logrus.Errorf("Failed to unmarshal existing root for GUN %s to retrieve timestamp key ID", gun) diff --git a/server/timestamp/timestamp_test.go b/server/timestamp/timestamp_test.go index 8f0bde1cc9..61ad949241 100644 --- a/server/timestamp/timestamp_test.go +++ b/server/timestamp/timestamp_test.go @@ -2,16 +2,16 @@ package timestamp import ( "bytes" + "fmt" "testing" "time" "github.com/docker/go/canonical/json" + "github.com/docker/notary/server/storage" "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/signed" "github.com/docker/notary/tuf/testutils" "github.com/stretchr/testify/require" - - "github.com/docker/notary/server/storage" ) func TestTimestampExpired(t *testing.T) { @@ -226,3 +226,43 @@ func TestCreateTimestampNoKeyInCrypto(t *testing.T) { require.Error(t, err) require.IsType(t, signed.ErrInsufficientSignatures{}, err) } + +type FailingStore struct { + *storage.MemStorage +} + +func (f FailingStore) GetCurrent(role, gun string) (*time.Time, []byte, error) { + return nil, nil, fmt.Errorf("failing store failed") +} + +func TestGetTimestampKeyCreateWithFailingStore(t *testing.T) { + store := FailingStore{storage.NewMemStorage()} + crypto := signed.NewEd25519() + k, err := GetOrCreateTimestampKey("gun", store, crypto, data.ED25519Key) + require.Error(t, err, "Expected error") + require.Nil(t, k, "Key should be nil") +} + +type CorruptedStore struct { + *storage.MemStorage +} + +func (c CorruptedStore) GetCurrent(role, gun string) (*time.Time, []byte, error) { + return &time.Time{}, []byte("junk"), nil +} + +func TestGetTimestampKeyCreateWithCorruptedStore(t *testing.T) { + store := CorruptedStore{storage.NewMemStorage()} + crypto := signed.NewEd25519() + k, err := GetOrCreateTimestampKey("gun", store, crypto, data.ED25519Key) + require.Error(t, err, "Expected error") + require.Nil(t, k, "Key should be nil") +} + +func TestGetSnapshotKeyCreateWithInvalidAlgo(t *testing.T) { + store := storage.NewMemStorage() + crypto := signed.NewEd25519() + k, err := GetOrCreateTimestampKey("gun", store, crypto, "notactuallyanalgorithm") + require.Error(t, err, "Expected error") + require.Nil(t, k, "Key should be nil") +} diff --git a/signer/keydbstore/keydbstore_test.go b/signer/keydbstore/keydbstore_test.go index 58d265980a..fc5ca88aa4 100644 --- a/signer/keydbstore/keydbstore_test.go +++ b/signer/keydbstore/keydbstore_test.go @@ -144,6 +144,12 @@ func testMarkKeyActive(t *testing.T, dbStore keyActivator) (data.PrivateKey, dat func testGetPendingKey(t *testing.T, dbStore keyActivator) (data.PrivateKey, data.PrivateKey) { // Create a test key and add it to the db such that it will be pending (never marked active) keyInfo := trustmanager.KeyInfo{Role: data.CanonicalSnapshotRole, Gun: "gun"} + + // There should be no keys to start + retrievedKey, err := dbStore.GetPendingKey(keyInfo) + require.Error(t, err) + require.Nil(t, retrievedKey) + pendingTestKey, err := utils.GenerateECDSAKey(rand.Reader) require.NoError(t, err) requireGetKeyFailure(t, dbStore, pendingTestKey.ID()) @@ -151,7 +157,7 @@ func testGetPendingKey(t *testing.T, dbStore keyActivator) (data.PrivateKey, dat require.NoError(t, err) requireGetKeySuccess(t, dbStore, data.CanonicalSnapshotRole, pendingTestKey) - retrievedKey, err := dbStore.GetPendingKey(keyInfo) + retrievedKey, err = dbStore.GetPendingKey(keyInfo) require.NoError(t, err) require.Equal(t, pendingTestKey.Public(), retrievedKey.Public())