Skip to content

Commit

Permalink
Lock when generating encryption key (#107)
Browse files Browse the repository at this point in the history
  • Loading branch information
dpanayotov authored Aug 16, 2018
1 parent 99c6dad commit f586159
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 12 deletions.
12 changes: 9 additions & 3 deletions pkg/sm/sm.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"net/http"
"os"
"os/signal"
"time"

"github.com/Peripli/service-manager/api"
"github.com/Peripli/service-manager/config"
Expand Down Expand Up @@ -85,7 +86,7 @@ func New(ctx context.Context, cancel context.CancelFunc, env env.Environment) *S
}

securityStorage := smStorage.Security()
if err := initializeSecureStorage(securityStorage); err != nil {
if err := initializeSecureStorage(ctx, securityStorage); err != nil {
panic(fmt.Sprintf("error initialzing secure storage: %v", err))
}

Expand Down Expand Up @@ -154,7 +155,12 @@ func (smb *ServiceManagerBuilder) Build() *ServiceManager {
}
}

func initializeSecureStorage(secureStorage storage.Security) error {
func initializeSecureStorage(ctx context.Context, secureStorage storage.Security) error {
ctx, cancelFunc := context.WithTimeout(ctx, 2*time.Second)
defer cancelFunc()
if err := secureStorage.Lock(ctx); err != nil {
return err
}
keyFetcher := secureStorage.Fetcher()
encryptionKey, err := keyFetcher.GetEncryptionKey()
if err != nil {
Expand All @@ -172,7 +178,7 @@ func initializeSecureStorage(secureStorage storage.Security) error {
}
logrus.Debug("Successfully generated new encryption key")
}
return nil
return secureStorage.Unlock()
}

func handleInterrupts(ctx context.Context, cancelFunc context.CancelFunc) {
Expand Down
10 changes: 8 additions & 2 deletions storage/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package storage

import (
"context"
"fmt"

"github.com/Peripli/service-manager/pkg/types"
Expand Down Expand Up @@ -107,9 +108,14 @@ type Credentials interface {
}

// Security interface for encryption key operations
type Security interface{
type Security interface {
// Lock locks the storage so that only one process can manipulate the encryption key.
// Returns an error if the process has already acquired the lock
Lock(ctx context.Context) error
// Unlock releases the acquired lock.
Unlock() error
// Fetcher provides means to obtain the encryption key
Fetcher() security.KeyFetcher
// Setter provides means to change the encryption key
Setter() security.KeySetter
}
}
36 changes: 36 additions & 0 deletions storage/postgres/security.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,53 @@
package postgres

import (
"context"
"fmt"
"sync"
"time"

"github.com/Peripli/service-manager/security"
"github.com/jmoiron/sqlx"
"github.com/sirupsen/logrus"
)

const securityLockIndex = 111

type securityStorage struct {
db *sqlx.DB
encryptionKey []byte
isLocked bool
mutex *sync.Mutex
}

// Lock acquires a database lock so that only one process can manipulate the encryption key.
// Returns an error if the process has already acquired the lock
func (s *securityStorage) Lock(ctx context.Context) error {
s.mutex.Lock()
defer s.mutex.Unlock()
if s.isLocked {
return fmt.Errorf("Lock is already acquired")
}
if _, err := s.db.ExecContext(ctx, "SELECT pg_advisory_lock($1)", securityLockIndex); err != nil {
return err
}
s.isLocked = true
return nil
}

// Unlock releases the database lock.
func (s *securityStorage) Unlock() error {
s.mutex.Lock()
defer s.mutex.Unlock()
if !s.isLocked {
return nil
}

if _, err := s.db.Exec("SELECT pg_advisory_unlock($1)", securityLockIndex); err != nil {
return err
}
s.isLocked = false
return nil
}

// Fetcher returns a KeyFetcher configured to fetch a key from the database
Expand Down
73 changes: 72 additions & 1 deletion storage/postgres/security_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@
package postgres

import (
"context"
"crypto/rand"
"database/sql"
"fmt"
"time"
"sync"
"time"

"github.com/DATA-DOG/go-sqlmock"
"github.com/Peripli/service-manager/security"
Expand Down Expand Up @@ -159,4 +161,73 @@ var _ = Describe("Security", func() {
})
})
})

Describe("Locking", func() {
var mockdb *sql.DB
var mock sqlmock.Sqlmock
var storage *securityStorage
envEncryptionKey := make([]byte, 32)

JustBeforeEach(func() {
storage = &securityStorage{
db: sqlx.NewDb(mockdb, "sqlmock"),
encryptionKey: envEncryptionKey,
isLocked: false,
mutex: &sync.Mutex{},
}
})
BeforeEach(func() {
mockdb, mock, _ = sqlmock.New()
rand.Read(envEncryptionKey)
})
AfterEach(func() {
mockdb.Close()
})

Describe("Lock", func() {

Context("When lock is already acquired", func() {
It("Should return an error", func() {
storage.isLocked = true
err := storage.Lock(context.TODO())
Expect(err).ToNot(BeNil())
})
})

Context("When lock is not yet acquired", func() {
AfterEach(func() {
storage.Unlock()
})
BeforeEach(func() {
mock.ExpectExec("SELECT").WillReturnResult(sqlmock.NewResult(int64(1), int64(1)))
})
It("Should acquire lock", func() {
err := storage.Lock(context.TODO())
Expect(err).To(BeNil())
Expect(storage.isLocked).To(Equal(true))
})
})
})

Describe("Unlock", func() {
Context("When lock is not acquired", func() {
It("Should return nil", func() {
storage.isLocked = false
err := storage.Unlock()
Expect(err).To(BeNil())
})
})
Context("When lock is acquired", func() {
BeforeEach(func() {
mock.ExpectExec("SELECT").WillReturnResult(sqlmock.NewResult(int64(1), int64(1)))
})
It("Should release lock", func() {
storage.isLocked = true
err := storage.Unlock()
Expect(err).To(BeNil())
Expect(storage.isLocked).To(Equal(false))
})
})
})
})
})
13 changes: 7 additions & 6 deletions storage/postgres/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@
package postgres

import (
"fmt"
"sync"
"time"

"fmt"

"github.com/Peripli/service-manager/storage"
"github.com/golang-migrate/migrate"
migratepg "github.com/golang-migrate/migrate/database/postgres"
Expand All @@ -39,9 +38,10 @@ func init() {
}

type postgresStorage struct {
db *sqlx.DB
state *storageState
db *sqlx.DB
state *storageState
encryptionKey []byte
mutex *sync.Mutex
}

func (storage *postgresStorage) checkOpen() {
Expand Down Expand Up @@ -72,9 +72,9 @@ func (storage *postgresStorage) Credentials() storage.Credentials {
return &credentialStorage{storage.db}
}

func (storage *postgresStorage) Security() storage.Security{
func (storage *postgresStorage) Security() storage.Security {
storage.checkOpen()
return &securityStorage{storage.db, storage.encryptionKey}
return &securityStorage{storage.db, storage.encryptionKey, false, storage.mutex}
}

func (storage *postgresStorage) Open(uri string, encryptionKey []byte) error {
Expand All @@ -94,6 +94,7 @@ func (storage *postgresStorage) Open(uri string, encryptionKey []byte) error {
db: storage.db,
storageCheckInterval: time.Second * 5,
}
storage.mutex = &sync.Mutex{}
storage.encryptionKey = encryptionKey
logrus.Debug("Updating database schema")
if err := updateSchema(storage.db); err != nil {
Expand Down

0 comments on commit f586159

Please sign in to comment.