From d193b46449a64a554c3b54dd637a371769bfe195 Mon Sep 17 00:00:00 2001 From: Jimmy Zelinskie Date: Thu, 6 Sep 2018 19:15:06 -0400 Subject: [PATCH] pkg/pagination: init This change refactors a lot of the code dealing with pagination so that fernet implementation details do not leak. - Deletes database/pgsql/token - Introduces a pagination package - Renames idPageNumber to Page and add a constructor and method. --- cmd/clair/config.go | 13 ++-- database/pgsql/notification.go | 12 ++-- database/pgsql/notification_test.go | 24 ++++---- database/pgsql/page.go | 45 ++++++++++++++ database/pgsql/pgsql.go | 36 +++-------- database/pgsql/pgsql_test.go | 9 +-- database/pgsql/token/token.go | 49 --------------- pkg/pagination/pagination.go | 94 +++++++++++++++++++++++++++++ 8 files changed, 169 insertions(+), 113 deletions(-) create mode 100644 database/pgsql/page.go delete mode 100644 database/pgsql/token/token.go create mode 100644 pkg/pagination/pagination.go diff --git a/cmd/clair/config.go b/cmd/clair/config.go index 08f2606691..09a013641f 100644 --- a/cmd/clair/config.go +++ b/cmd/clair/config.go @@ -1,4 +1,4 @@ -// Copyright 2017 clair authors +// Copyright 2018 clair authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -20,7 +20,6 @@ import ( "os" "time" - "github.com/fernet/fernet-go" log "github.com/sirupsen/logrus" "gopkg.in/yaml.v2" @@ -31,6 +30,7 @@ import ( "github.com/coreos/clair/ext/featurens" "github.com/coreos/clair/ext/notification" "github.com/coreos/clair/ext/vulnsrc" + "github.com/coreos/clair/pkg/pagination" ) // ErrDatasourceNotLoaded is returned when the datasource variable in the @@ -108,15 +108,10 @@ func LoadConfig(path string) (config *Config, err error) { // Generate a pagination key if none is provided. if v, ok := config.Database.Options["paginationkey"]; !ok || v == nil || v.(string) == "" { log.Warn("pagination key is empty, generating...") - var key fernet.Key - if err = key.Generate(); err != nil { - return - } - config.Database.Options["paginationkey"] = key.Encode() + config.Database.Options["paginationkey"] = pagination.Must(pagination.NewKey()).String() } else { - _, err = fernet.DecodeKey(config.Database.Options["paginationkey"].(string)) + _, err = pagination.KeyFromString(config.Database.Options["paginationkey"].(string)) if err != nil { - err = errors.New("Invalid Pagination key; must be 32-bit URL-safe base64") return } } diff --git a/database/pgsql/notification.go b/database/pgsql/notification.go index ebc346d3e7..5c9af26241 100644 --- a/database/pgsql/notification.go +++ b/database/pgsql/notification.go @@ -165,10 +165,10 @@ func (tx *pgSession) FindNewNotification(notifiedBefore time.Time) (database.Not func (tx *pgSession) findPagedVulnerableAncestries(vulnID int64, limit int, currentPage database.PageNumber) (database.PagedVulnerableAncestries, error) { vulnPage := database.PagedVulnerableAncestries{Limit: limit} - current := idPageNumber{0} + current := Page{0} if currentPage != "" { var err error - current, err = decryptPage(currentPage, tx.paginationKey) + current, err = PageFromPageNumber(tx.key, currentPage) if err != nil { return vulnPage, err } @@ -211,11 +211,7 @@ func (tx *pgSession) findPagedVulnerableAncestries(vulnID int64, limit int, curr } else { // Use the last ancestry's ID as the next PageNumber. lastIndex = len(ancestries) - 1 - vulnPage.Next, err = encryptPage( - idPageNumber{ - ancestries[len(ancestries)-1].id, - }, tx.paginationKey) - + vulnPage.Next, err = Page{ancestries[len(ancestries)-1].id}.PageNumber(tx.key) if err != nil { return vulnPage, err } @@ -226,7 +222,7 @@ func (tx *pgSession) findPagedVulnerableAncestries(vulnID int64, limit int, curr vulnPage.Affected[int(ancestry.id)] = ancestry.name } - vulnPage.Current, err = encryptPage(current, tx.paginationKey) + vulnPage.Current, err = current.PageNumber(tx.key) if err != nil { return vulnPage, err } diff --git a/database/pgsql/notification_test.go b/database/pgsql/notification_test.go index 0d930d084d..18e27a5c5f 100644 --- a/database/pgsql/notification_test.go +++ b/database/pgsql/notification_test.go @@ -1,4 +1,4 @@ -// Copyright 2017 clair authors +// Copyright 2018 clair authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -73,22 +73,22 @@ func TestPagination(t *testing.T) { if assert.Nil(t, err) && assert.True(t, ok) { assert.Equal(t, "test", noti.Name) if assert.NotNil(t, noti.Old) && assert.NotNil(t, noti.New) { - oldPageNum, err := decryptPage(noti.Old.Current, tx.paginationKey) + oldPage, err := PageFromPageNumber(tx.key, noti.Old.Current) if !assert.Nil(t, err) { assert.FailNow(t, "") } - assert.Equal(t, int64(0), oldPageNum.StartID) - newPageNum, err := decryptPage(noti.New.Current, tx.paginationKey) + assert.Equal(t, int64(0), oldPage.StartID) + newPage, err := PageFromPageNumber(tx.key, noti.New.Current) if !assert.Nil(t, err) { assert.FailNow(t, "") } - newPageNextNum, err := decryptPage(noti.New.Next, tx.paginationKey) + newPageNext, err := PageFromPageNumber(tx.key, noti.New.Next) if !assert.Nil(t, err) { assert.FailNow(t, "") } - assert.Equal(t, int64(0), newPageNum.StartID) - assert.Equal(t, int64(4), newPageNextNum.StartID) + assert.Equal(t, int64(0), newPage.StartID) + assert.Equal(t, int64(4), newPageNext.StartID) noti.Old.Current = "" noti.New.Current = "" @@ -98,26 +98,26 @@ func TestPagination(t *testing.T) { } } - page1, err := encryptPage(idPageNumber{0}, tx.paginationKey) + pageNum1, err := Page{0}.PageNumber(tx.key) if !assert.Nil(t, err) { assert.FailNow(t, "") } - page2, err := encryptPage(idPageNumber{4}, tx.paginationKey) + pageNum2, err := Page{4}.PageNumber(tx.key) if !assert.Nil(t, err) { assert.FailNow(t, "") } - noti, ok, err = tx.FindVulnerabilityNotification("test", 1, page1, page2) + noti, ok, err = tx.FindVulnerabilityNotification("test", 1, pageNum1, pageNum2) if assert.Nil(t, err) && assert.True(t, ok) { assert.Equal(t, "test", noti.Name) if assert.NotNil(t, noti.Old) && assert.NotNil(t, noti.New) { - oldCurrentPage, err := decryptPage(noti.Old.Current, tx.paginationKey) + oldCurrentPage, err := PageFromPageNumber(tx.key, noti.Old.Current) if !assert.Nil(t, err) { assert.FailNow(t, "") } - newCurrentPage, err := decryptPage(noti.New.Current, tx.paginationKey) + newCurrentPage, err := PageFromPageNumber(tx.key, noti.New.Current) if !assert.Nil(t, err) { assert.FailNow(t, "") } diff --git a/database/pgsql/page.go b/database/pgsql/page.go new file mode 100644 index 0000000000..101d1dff32 --- /dev/null +++ b/database/pgsql/page.go @@ -0,0 +1,45 @@ +// Copyright 2018 clair authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pgsql + +import ( + "github.com/coreos/clair/database" + "github.com/coreos/clair/pkg/pagination" +) + +// Page is the representation of a page for the Postgres schema. +type Page struct { + // StartID is the ID being used as the basis for pagination across database + // results. It is used to search for an ancestry with ID >= StartID. + // + // StartID is required to be unique to every ancestry and always increasing. + StartID int64 +} + +// PageNumber converts a Page to a database.PageNumber. +func (p Page) PageNumber(key pagination.Key) (pn database.PageNumber, err error) { + token, err := key.MarshalToken(p) + if err != nil { + return pn, err + } + pn = database.PageNumber(token) + return pn, nil +} + +// PageFromPageNumber converts a database.PageNumber into a Page. +func PageFromPageNumber(key pagination.Key, pn database.PageNumber) (p Page, err error) { + err = key.UnmarshalToken(string(pn), &p) + return +} diff --git a/database/pgsql/pgsql.go b/database/pgsql/pgsql.go index 335b77c7b6..66e8930ee4 100644 --- a/database/pgsql/pgsql.go +++ b/database/pgsql/pgsql.go @@ -33,8 +33,8 @@ import ( "github.com/coreos/clair/database" "github.com/coreos/clair/database/pgsql/migrations" - "github.com/coreos/clair/database/pgsql/token" "github.com/coreos/clair/pkg/commonerr" + "github.com/coreos/clair/pkg/pagination" ) var ( @@ -91,41 +91,21 @@ type pgSQL struct { type pgSession struct { *sql.Tx - paginationKey string + key pagination.Key } -type idPageNumber struct { - // StartID is an implementation detail for paginating by an ID required to - // be unique to every ancestry and always increasing. - // - // StartID is used to search for ancestry with ID >= StartID - StartID int64 -} - -func encryptPage(page idPageNumber, paginationKey string) (result database.PageNumber, err error) { - resultBytes, err := token.Marshal(page, paginationKey) - if err != nil { - return result, err - } - result = database.PageNumber(resultBytes) - return result, nil -} - -func decryptPage(page database.PageNumber, paginationKey string) (result idPageNumber, err error) { - err = token.Unmarshal(string(page), paginationKey, &result) - return -} - -// Begin initiates a transaction to database. The expected transaction isolation -// level in this implementation is "Read Committed". +// Begin initiates a transaction to database. +// +// The expected transaction isolation level in this implementation is "Read +// Committed". func (pgSQL *pgSQL) Begin() (database.Session, error) { tx, err := pgSQL.DB.Begin() if err != nil { return nil, err } return &pgSession{ - Tx: tx, - paginationKey: pgSQL.config.PaginationKey, + Tx: tx, + key: pagination.Must(pagination.KeyFromString(pgSQL.config.PaginationKey)), }, nil } diff --git a/database/pgsql/pgsql_test.go b/database/pgsql/pgsql_test.go index 9624166650..0e5b723464 100644 --- a/database/pgsql/pgsql_test.go +++ b/database/pgsql/pgsql_test.go @@ -24,13 +24,13 @@ import ( "strings" "testing" - fernet "github.com/fernet/fernet-go" "github.com/pborman/uuid" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" yaml "gopkg.in/yaml.v2" "github.com/coreos/clair/database" + "github.com/coreos/clair/pkg/pagination" ) var ( @@ -215,18 +215,13 @@ func generateTestConfig(testName string, loadFixture bool, manageLife bool) data source = fmt.Sprintf(sourceEnv, dbName) } - var key fernet.Key - if err := key.Generate(); err != nil { - panic("failed to generate pagination key" + err.Error()) - } - return database.RegistrableComponentConfig{ Options: map[string]interface{}{ "source": source, "cachesize": 0, "managedatabaselifecycle": manageLife, "fixturepath": fixturePath, - "paginationkey": key.Encode(), + "paginationkey": pagination.MustGenerateNewKey().String(), }, } } diff --git a/database/pgsql/token/token.go b/database/pgsql/token/token.go deleted file mode 100644 index 3492e712a7..0000000000 --- a/database/pgsql/token/token.go +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2017 clair authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// Package token implements encryption/decryption for json encoded interfaces -package token - -import ( - "bytes" - "encoding/json" - "errors" - "time" - - "github.com/fernet/fernet-go" -) - -// Unmarshal decrypts a token using provided key -// and decode the result into interface. -func Unmarshal(token string, key string, v interface{}) error { - k, _ := fernet.DecodeKey(key) - msg := fernet.VerifyAndDecrypt([]byte(token), time.Hour, []*fernet.Key{k}) - if msg == nil { - return errors.New("invalid or expired pagination token") - } - - return json.NewDecoder(bytes.NewBuffer(msg)).Decode(&v) -} - -// Marshal encodes an interface into json bytes and encrypts it. -func Marshal(v interface{}, key string) ([]byte, error) { - var buf bytes.Buffer - err := json.NewEncoder(&buf).Encode(v) - if err != nil { - return nil, err - } - - k, _ := fernet.DecodeKey(key) - return fernet.EncryptAndSign(buf.Bytes(), k) -} diff --git a/pkg/pagination/pagination.go b/pkg/pagination/pagination.go new file mode 100644 index 0000000000..5f4acb667e --- /dev/null +++ b/pkg/pagination/pagination.go @@ -0,0 +1,94 @@ +// Copyright 2018 clair authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package pagination implements a series of utilities for dealing with +// paginating lists of objects for an API. +package pagination + +import ( + "bytes" + "encoding/json" + "errors" + "time" + + "github.com/fernet/fernet-go" +) + +// ErrInvalidToken is returned when a token fails to Unmarshal because it was +// invalid or expired. +var ErrInvalidToken = errors.New("invalid or expired pagination token") + +// ErrInvalidKeyString is returned when the string representing a key is malformed. +var ErrInvalidKeyString = errors.New("invalid pagination key string: must be 32-byte URL-safe base64") + +// Key represents the key used to cryptographically secure the token +// being used to keep track of pages. +type Key struct { + fkey *fernet.Key +} + +// NewKey generates a new random pagination key. +func NewKey() (k Key, err error) { + k.fkey = new(fernet.Key) + err = k.fkey.Generate() + return k, err +} + +// KeyFromString creates the key for a given string. +// +// Strings must be 32-byte URL-safe base64 representations of the key bytes. +func KeyFromString(keyString string) (k Key, err error) { + var fkey *fernet.Key + fkey, err = fernet.DecodeKey(keyString) + if err != nil { + return Key{}, ErrInvalidKeyString + } + return Key{fkey}, err +} + +// Must is a helper that wraps calls returning a Key and and error and panics +// if the error is non-nil. +func Must(k Key, err error) Key { + if err != nil { + panic(err) + } + return k +} + +// String implements the fmt.Stringer interface for Key. +func (k Key) String() string { + return k.fkey.Encode() +} + +// MarshalToken encodes an interface into JSON bytes and encrypts it. +func (k Key) MarshalToken(v interface{}) ([]byte, error) { + var buf bytes.Buffer + err := json.NewEncoder(&buf).Encode(v) + if err != nil { + return nil, err + } + + return fernet.EncryptAndSign(buf.Bytes(), k.fkey) +} + +// UnmarshalToken decrypts a token using provided key and decodes the result +// into the provided interface. +func (k Key) UnmarshalToken(token string, v interface{}) error { + msg := fernet.VerifyAndDecrypt([]byte(token), time.Hour, []*fernet.Key{k.fkey}) + if msg == nil { + return ErrInvalidToken + } + + return json.NewDecoder(bytes.NewBuffer(msg)).Decode(&v) +}