Enhance worker connection security (#1641)
This stores worker connection nonces within the database to ensure uniqueness of connections. Prior the connection required any attacker to be able to decrypt the incoming encrypted authentication message in order to replay (and if the attacker has access to the KMS you have bigger problems), but with this change even if an attacker does have access to the KMS key, they cannot reuse the same message. (If they're able to encrypt against the KMS key, they can still act as a worker with a new message, so this only really helps if they only have decryption capabilities.)
This commit is contained in:
1 parent
a679300b50
commit
1782f6a4e4
23 files changed
+426
-63
No files matched your search
@@ -9,6 +9,13 @@ Canonical reference for changes, improvements, and bugfixes for Boundary.
|
||||
* tls: Boundary's support for TLS 1.0/1.1 on the API listener was broken. Rather
|
||||
than fix this, we are simply not supporting TLS 1.0/1.1 as they are insecure.
|
||||
|
||||
### New and Improved
|
||||
|
||||
* workers: The existing worker connection replay prevention logic has been
|
||||
enhanced to be more robust against attackers that have decryption access to
|
||||
the shared `worker-auth` KMS key
|
||||
([PR](https://github.com/hashicorp/boundary/pull/1641))
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
* tls: Support TLS 1.2 for more clients. This was broken for some clients due to
|
||||
|
||||
@@ -30,4 +30,8 @@ var (
|
||||
// RecoveryTokenValidityPeriod is exported so we can modify it in tests if
|
||||
// we want
|
||||
RecoveryTokenValidityPeriod = 5 * time.Minute
|
||||
|
||||
// WorkerAuthNonceValidityPeriod is exported so we can modify it in tests if
|
||||
// we want
|
||||
WorkerAuthNonceValidityPeriod = 2 * time.Minute
|
||||
)
|
||||
@@ -64,7 +64,6 @@ require (
|
||||
github.com/mr-tron/base58 v1.2.0
|
||||
github.com/oligot/go-mod-upgrade v0.6.1
|
||||
github.com/ory/dockertest/v3 v3.7.0
|
||||
github.com/patrickmn/go-cache v2.1.0+incompatible
|
||||
github.com/pires/go-proxyproto v0.6.1
|
||||
github.com/pkg/errors v0.9.1
|
||||
github.com/posener/complete v1.2.3
|
||||
|
||||
@@ -847,8 +847,6 @@ github.com/pact-foundation/pact-go v1.0.4/go.mod h1:uExwJY4kCzNPcHRj+hCR/HBbOOIw
|
||||
github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
|
||||
github.com/pascaldekloe/goe v0.1.0 h1:cBOtyMzM9HTpWjXfbbunk26uA6nG3a8n06Wieeh0MwY=
|
||||
github.com/pascaldekloe/goe v0.1.0/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
|
||||
github.com/patrickmn/go-cache v2.1.0+incompatible h1:HRMgzkcYKYpi3C8ajMPV8OFXaaRUnok+kx1WdO15EQc=
|
||||
github.com/patrickmn/go-cache v2.1.0+incompatible/go.mod h1:3Qf8kWWT7OJRJbdiICTKqZju1ZixQ/KpMGzzAfe6+WQ=
|
||||
github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k=
|
||||
github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
|
||||
github.com/performancecopilot/speed v3.0.0+incompatible/go.mod h1:/CLtqpZ5gBg1M9iaPbIdPPGyKcA8hKdoy6hAWba7Yac=
|
||||
|
||||
@@ -29,6 +29,7 @@ before
|
||||
insert on server
|
||||
for each row execute procedure default_create_time();
|
||||
|
||||
-- UPDATED in 18/01_nonce
|
||||
create table recovery_nonces (
|
||||
nonce text
|
||||
primary key,
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
begin;
|
||||
|
||||
-- This migration exists to generalize the nonce behavior since we need it for
|
||||
-- more than recovery nonces.
|
||||
|
||||
create table nonce_type_enm (
|
||||
name text primary key
|
||||
constraint only_predefined_nonce_types_allowed
|
||||
check (
|
||||
name in (
|
||||
'recovery',
|
||||
'worker-auth'
|
||||
)
|
||||
)
|
||||
);
|
||||
comment on table nonce_type_enm is
|
||||
'nonce_type_enm is an enumeration table for nonce types. '
|
||||
'It contains rows for representing nonces as either for recovery or worker authentication purposes.';
|
||||
|
||||
insert into nonce_type_enm (name) values
|
||||
('recovery'),
|
||||
('worker-auth');
|
||||
|
||||
-- Drop existing triggers
|
||||
drop trigger default_create_time_column on recovery_nonces;
|
||||
drop trigger immutable_columns on recovery_nonces;
|
||||
|
||||
-- Update table name
|
||||
alter table recovery_nonces rename to nonce;
|
||||
|
||||
-- Add purpose field
|
||||
alter table nonce
|
||||
add column purpose text not null
|
||||
default 'recovery'
|
||||
constraint nonce_type_enm_fkey
|
||||
references nonce_type_enm(name)
|
||||
on update cascade
|
||||
on delete restrict;
|
||||
|
||||
alter table nonce
|
||||
alter column purpose drop default;
|
||||
|
||||
|
||||
-- Recreate triggers
|
||||
create trigger
|
||||
default_create_time_column
|
||||
before
|
||||
insert on nonce
|
||||
for each row execute procedure default_create_time();
|
||||
|
||||
create trigger
|
||||
immutable_columns
|
||||
before
|
||||
update on nonce
|
||||
for each row execute procedure immutable_columns('nonce', 'create_time', 'purpose');
|
||||
|
||||
commit;
|
||||
@@ -47,7 +47,7 @@ func TestMigrations_CredentialDimension(t *testing.T) {
|
||||
))
|
||||
require.NoError(err)
|
||||
|
||||
assert.NoError(m.ApplyMigrations(ctx))
|
||||
require.NoError(m.ApplyMigrations(ctx))
|
||||
state, err := m.CurrentState(ctx)
|
||||
require.NoError(err)
|
||||
want := &schema.State{
|
||||
@@ -127,7 +127,7 @@ func TestMigrations_CredentialDimension(t *testing.T) {
|
||||
require.NoError(err)
|
||||
|
||||
count, err := sessionRepo.TerminateCompletedSessions(ctx)
|
||||
assert.NoError(err)
|
||||
require.NoError(err)
|
||||
assert.Zero(count)
|
||||
|
||||
for _, sess := range sessions {
|
||||
@@ -142,8 +142,9 @@ func TestMigrations_CredentialDimension(t *testing.T) {
|
||||
))
|
||||
require.NoError(err)
|
||||
|
||||
assert.NoError(m.ApplyMigrations(ctx))
|
||||
require.NoError(m.ApplyMigrations(ctx))
|
||||
state, err = m.CurrentState(ctx)
|
||||
require.NoError(err)
|
||||
want = &schema.State{
|
||||
Initialized: true,
|
||||
Editions: []schema.EditionState{
|
||||
|
||||
@@ -0,0 +1,162 @@
|
||||
package oss_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/hashicorp/boundary/internal/db"
|
||||
"github.com/hashicorp/boundary/internal/db/common"
|
||||
"github.com/hashicorp/boundary/internal/db/schema"
|
||||
"github.com/hashicorp/boundary/internal/kms"
|
||||
"github.com/hashicorp/boundary/internal/servers"
|
||||
"github.com/hashicorp/boundary/testing/dbtest"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestMigrations_Nonce(t *testing.T) {
|
||||
const (
|
||||
priorMigration = 16005
|
||||
currentMigration = 18001
|
||||
)
|
||||
|
||||
t.Parallel()
|
||||
assert, require := assert.New(t), require.New(t)
|
||||
ctx := context.Background()
|
||||
dialect := dbtest.Postgres
|
||||
|
||||
c, u, _, err := dbtest.StartUsingTemplate(dialect, dbtest.WithTemplate(dbtest.Template1))
|
||||
require.NoError(err)
|
||||
t.Cleanup(func() {
|
||||
require.NoError(c())
|
||||
})
|
||||
d, err := common.SqlOpen(dialect, u)
|
||||
require.NoError(err)
|
||||
|
||||
// migration to the prior migration (before the one we want to test)
|
||||
m, err := schema.NewManager(ctx, schema.Dialect(dialect), d, schema.WithEditions(
|
||||
schema.TestCreatePartialEditions(schema.Dialect(dialect), schema.PartialEditions{"oss": priorMigration}),
|
||||
))
|
||||
require.NoError(err)
|
||||
|
||||
require.NoError(m.ApplyMigrations(ctx))
|
||||
state, err := m.CurrentState(ctx)
|
||||
require.NoError(err)
|
||||
want := &schema.State{
|
||||
Initialized: true,
|
||||
Editions: []schema.EditionState{
|
||||
{
|
||||
Name: "oss",
|
||||
BinarySchemaVersion: priorMigration,
|
||||
DatabaseSchemaVersion: priorMigration,
|
||||
DatabaseSchemaState: schema.Equal,
|
||||
},
|
||||
},
|
||||
}
|
||||
require.Equal(want, state)
|
||||
|
||||
// get a connection
|
||||
dbType, err := db.StringToDbType(dialect)
|
||||
require.NoError(err)
|
||||
conn, err := db.Open(dbType, u)
|
||||
require.NoError(err)
|
||||
rw := db.New(conn)
|
||||
|
||||
// okay, now we can seed the database with test data and validate existing
|
||||
// functionality. At the end of this we will have two values in the table
|
||||
// with no purpose.
|
||||
{
|
||||
// Create an old version of the struct
|
||||
type RecoveryNonce struct {
|
||||
Nonce string
|
||||
}
|
||||
|
||||
require.NoError(rw.CreateItems(ctx, []interface{}{
|
||||
&RecoveryNonce{Nonce: "abcd"},
|
||||
&RecoveryNonce{Nonce: "dcba"},
|
||||
}))
|
||||
|
||||
var results []*RecoveryNonce
|
||||
require.NoError(rw.SearchWhere(ctx, &results, "", nil))
|
||||
require.Len(results, 2)
|
||||
}
|
||||
|
||||
// now we're ready for the migration we want to test.
|
||||
m, err = schema.NewManager(ctx, schema.Dialect(dialect), d, schema.WithEditions(
|
||||
schema.TestCreatePartialEditions(schema.Dialect(dialect), schema.PartialEditions{"oss": currentMigration}),
|
||||
))
|
||||
require.NoError(err)
|
||||
|
||||
require.NoError(m.ApplyMigrations(ctx))
|
||||
state, err = m.CurrentState(ctx)
|
||||
require.NoError(err)
|
||||
want = &schema.State{
|
||||
Initialized: true,
|
||||
Editions: []schema.EditionState{
|
||||
{
|
||||
Name: "oss",
|
||||
BinarySchemaVersion: currentMigration,
|
||||
DatabaseSchemaVersion: currentMigration,
|
||||
DatabaseSchemaState: schema.Equal,
|
||||
},
|
||||
},
|
||||
}
|
||||
require.Equal(want, state)
|
||||
|
||||
// now we have migrated and we will validate functionality; we'll do this
|
||||
// via the repo
|
||||
{
|
||||
wrapper := db.TestWrapper(t)
|
||||
kmsCache := kms.TestKms(t, conn, wrapper)
|
||||
rw := db.New(conn)
|
||||
repo, err := servers.NewRepository(rw, rw, kmsCache)
|
||||
require.NoError(err)
|
||||
require.NotNil(repo)
|
||||
|
||||
// First list and validate that the upgrade adjusted purpose correctly
|
||||
results, err := repo.ListNonces(ctx, servers.NoncePurposeRecovery)
|
||||
require.NoError(err)
|
||||
assert.Len(results, 2)
|
||||
for _, n := range results {
|
||||
assert.Equal(servers.NoncePurposeRecovery, n.Purpose)
|
||||
}
|
||||
|
||||
// Test the constraint, first via the repo, then directly
|
||||
const badPurpose = "foobar"
|
||||
err = repo.AddNonce(ctx, "xyz", badPurpose)
|
||||
require.Error(err)
|
||||
assert.Contains(err.Error(), "unknown nonce purpose")
|
||||
|
||||
// Check that null errors
|
||||
require.Error(rw.CreateItems(ctx, []interface{}{
|
||||
&servers.Nonce{Nonce: "dcba"},
|
||||
}))
|
||||
// Check that bad purpose errors
|
||||
require.Error(rw.CreateItems(ctx, []interface{}{
|
||||
&servers.Nonce{Nonce: "dcba", Purpose: badPurpose},
|
||||
}))
|
||||
|
||||
// Add some valid values, ensure that we can then list for both purposes
|
||||
// and get different values
|
||||
require.NoError(repo.AddNonce(ctx, "wxyz", servers.NoncePurposeWorkerAuth))
|
||||
require.NoError(repo.AddNonce(ctx, "zyxw", servers.NoncePurposeWorkerAuth))
|
||||
results, err = repo.ListNonces(ctx, servers.NoncePurposeRecovery)
|
||||
require.NoError(err)
|
||||
assert.Len(results, 2)
|
||||
for _, n := range results {
|
||||
assert.Equal(servers.NoncePurposeRecovery, n.Purpose)
|
||||
}
|
||||
results, err = repo.ListNonces(ctx, servers.NoncePurposeWorkerAuth)
|
||||
require.NoError(err)
|
||||
assert.Len(results, 2)
|
||||
for _, n := range results {
|
||||
assert.Equal(servers.NoncePurposeWorkerAuth, n.Purpose)
|
||||
}
|
||||
|
||||
// Last: add the same nonce, ensure we error
|
||||
require.NoError(repo.AddNonce(ctx, "double", servers.NoncePurposeWorkerAuth))
|
||||
err = repo.AddNonce(ctx, "double", servers.NoncePurposeWorkerAuth)
|
||||
require.Error(err)
|
||||
require.Contains(err.Error(), "duplicate key value")
|
||||
}
|
||||
}
|
||||
@@ -353,7 +353,7 @@ func Test_NewEventer(t *testing.T) {
|
||||
t.Parallel()
|
||||
testSetup := TestEventerConfig(t, "Test_NewEventer")
|
||||
|
||||
testSetupWithOpts := TestEventerConfig(t, "Test_NewEventer", TestWithAuditSink(t), TestWithObservationSink(t), testWithSysSink(t))
|
||||
testSetupWithOpts := TestEventerConfig(t, "Test_NewEventer", TestWithAuditSink(t), TestWithObservationSink(t), TestWithSysSink(t))
|
||||
|
||||
testHclogSetup := TestEventerConfig(t, "Test_NewEventer", testWithSinkFormat(t, TextHclogSinkFormat))
|
||||
|
||||
|
||||
@@ -222,7 +222,7 @@ func TestWithBroker(t *testing.T, b broker) Option {
|
||||
}
|
||||
}
|
||||
|
||||
// TestWithObservationSink is an unexported and a test option
|
||||
// TestWithObservationSink is a test option
|
||||
func TestWithObservationSink(t *testing.T) Option {
|
||||
t.Helper()
|
||||
return func(o *options) {
|
||||
@@ -230,7 +230,7 @@ func TestWithObservationSink(t *testing.T) Option {
|
||||
}
|
||||
}
|
||||
|
||||
// TestWithAuditSink is an unexported and a test option
|
||||
// TestWithAuditSink is a test option
|
||||
func TestWithAuditSink(t *testing.T) Option {
|
||||
t.Helper()
|
||||
return func(o *options) {
|
||||
@@ -238,8 +238,8 @@ func TestWithAuditSink(t *testing.T) Option {
|
||||
}
|
||||
}
|
||||
|
||||
// testWithSysSink is an unexported and a test option
|
||||
func testWithSysSink(t *testing.T) Option {
|
||||
// TestWithSysSink is a test option
|
||||
func TestWithSysSink(t *testing.T) Option {
|
||||
t.Helper()
|
||||
return func(o *options) {
|
||||
o.withSysSink = true
|
||||
|
||||
@@ -18,6 +18,7 @@ import (
|
||||
"github.com/hashicorp/boundary/internal/observability/event"
|
||||
"github.com/hashicorp/boundary/internal/perms"
|
||||
"github.com/hashicorp/boundary/internal/requests"
|
||||
"github.com/hashicorp/boundary/internal/servers"
|
||||
"github.com/hashicorp/boundary/internal/servers/controller/common"
|
||||
"github.com/hashicorp/boundary/internal/servers/controller/handlers"
|
||||
"github.com/hashicorp/boundary/internal/types/action"
|
||||
@@ -391,7 +392,7 @@ func (v *verifier) decryptToken(ctx context.Context) {
|
||||
v.requestInfo.TokenFormat = uint32(AuthTokenTypeUnknown)
|
||||
return
|
||||
}
|
||||
if err := repo.AddRecoveryNonce(v.ctx, info.Nonce); err != nil {
|
||||
if err := repo.AddNonce(v.ctx, info.Nonce, servers.NoncePurposeRecovery); err != nil {
|
||||
event.WriteError(ctx, op, err, event.WithInfoMsg("decrypt recovery token: error adding nonce to database (possible replay attack)"))
|
||||
v.requestInfo.TokenFormat = uint32(AuthTokenTypeUnknown)
|
||||
return
|
||||
|
||||
@@ -26,7 +26,6 @@ import (
|
||||
"github.com/hashicorp/boundary/internal/target"
|
||||
"github.com/hashicorp/go-hclog"
|
||||
"github.com/hashicorp/go-secure-stdlib/mlock"
|
||||
"github.com/patrickmn/go-cache"
|
||||
ua "go.uber.org/atomic"
|
||||
"google.golang.org/grpc"
|
||||
)
|
||||
@@ -39,10 +38,10 @@ type Controller struct {
|
||||
baseCancel context.CancelFunc
|
||||
started *ua.Bool
|
||||
|
||||
tickerWg sync.WaitGroup
|
||||
schedulerWg sync.WaitGroup
|
||||
tickerWg *sync.WaitGroup
|
||||
schedulerWg *sync.WaitGroup
|
||||
|
||||
workerAuthCache *cache.Cache
|
||||
workerAuthCache *sync.Map
|
||||
|
||||
// Used for testing and tracking worker health
|
||||
workerStatusUpdateTimes *sync.Map
|
||||
@@ -74,6 +73,9 @@ func New(ctx context.Context, conf *Config) (*Controller, error) {
|
||||
conf: conf,
|
||||
logger: conf.Logger.Named("controller"),
|
||||
started: ua.NewBool(false),
|
||||
tickerWg: new(sync.WaitGroup),
|
||||
schedulerWg: new(sync.WaitGroup),
|
||||
workerAuthCache: new(sync.Map),
|
||||
workerStatusUpdateTimes: new(sync.Map),
|
||||
}
|
||||
|
||||
@@ -163,7 +165,6 @@ func New(ctx context.Context, conf *Config) (*Controller, error) {
|
||||
c.SessionRepoFn = func() (*session.Repository, error) {
|
||||
return session.NewRepository(dbase, dbase, c.kms)
|
||||
}
|
||||
c.workerAuthCache = cache.New(0, 0)
|
||||
|
||||
return c, nil
|
||||
}
|
||||
@@ -178,7 +179,7 @@ func (c *Controller) Start() error {
|
||||
if err := c.registerJobs(); err != nil {
|
||||
return fmt.Errorf("error registering jobs: %w", err)
|
||||
}
|
||||
if err := c.scheduler.Start(c.baseContext, &c.schedulerWg); err != nil {
|
||||
if err := c.scheduler.Start(c.baseContext, c.schedulerWg); err != nil {
|
||||
return fmt.Errorf("error starting scheduler: %w", err)
|
||||
}
|
||||
if err := c.startListeners(c.baseContext); err != nil {
|
||||
@@ -192,7 +193,7 @@ func (c *Controller) Start() error {
|
||||
}()
|
||||
go func() {
|
||||
defer c.tickerWg.Done()
|
||||
c.startRecoveryNonceCleanupTicking(c.baseContext)
|
||||
c.startNonceCleanupTicking(c.baseContext)
|
||||
}()
|
||||
go func() {
|
||||
defer c.tickerWg.Done()
|
||||
|
||||
@@ -195,9 +195,5 @@ func ValidNameDescription(in string) bool {
|
||||
return !unicode.IsPrint(c)
|
||||
})
|
||||
|
||||
if idx != -1 {
|
||||
return false
|
||||
}
|
||||
|
||||
return true
|
||||
return idx == -1
|
||||
}
|
||||
@@ -70,7 +70,7 @@ func (m *interceptingListener) Accept() (net.Conn, error) {
|
||||
}
|
||||
return nil, fmt.Errorf("error reading nonce from worker, expected %d bytes, got %d", 20, read)
|
||||
}
|
||||
workerInfoRaw, found := m.c.workerAuthCache.Get(string(nonce))
|
||||
workerInfoRaw, found := m.c.workerAuthCache.Load(string(nonce))
|
||||
if !found {
|
||||
if err := conn.Close(); err != nil {
|
||||
event.WriteError(ctx, op, err, event.WithInfoMsg("error closing worker connection"))
|
||||
|
||||
@@ -17,7 +17,7 @@ const (
|
||||
)
|
||||
|
||||
// This is exported so it can be tweaked in tests
|
||||
var RecoveryNonceCleanupInterval = 2 * time.Minute
|
||||
var NonceCleanupInterval = 2 * time.Minute
|
||||
|
||||
func (c *Controller) startStatusTicking(cancelCtx context.Context) {
|
||||
const op = "controller.(Controller).startStatusTicking"
|
||||
@@ -51,8 +51,8 @@ func (c *Controller) startStatusTicking(cancelCtx context.Context) {
|
||||
}
|
||||
}
|
||||
|
||||
func (c *Controller) startRecoveryNonceCleanupTicking(cancelCtx context.Context) {
|
||||
const op = "controller.(Controller).startRecoveryNonceCleanupTicking"
|
||||
func (c *Controller) startNonceCleanupTicking(cancelCtx context.Context) {
|
||||
const op = "controller.(Controller).startNonceCleanupTicking"
|
||||
timer := time.NewTimer(0)
|
||||
for {
|
||||
select {
|
||||
@@ -72,7 +72,7 @@ func (c *Controller) startRecoveryNonceCleanupTicking(cancelCtx context.Context)
|
||||
event.WriteSysEvent(cancelCtx, op, "recovery nonce cleanup successful", "nonces_cleaned", nonceCount)
|
||||
}
|
||||
}
|
||||
timer.Reset(RecoveryNonceCleanupInterval)
|
||||
timer.Reset(NonceCleanupInterval)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,10 +7,12 @@ import (
|
||||
"encoding/base64"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net"
|
||||
"strings"
|
||||
|
||||
"github.com/hashicorp/boundary/internal/cmd/base"
|
||||
"github.com/hashicorp/boundary/internal/servers"
|
||||
wrapping "github.com/hashicorp/go-kms-wrapping"
|
||||
"google.golang.org/protobuf/proto"
|
||||
)
|
||||
@@ -20,16 +22,30 @@ type workerAuthEntry struct {
|
||||
conn net.Conn
|
||||
}
|
||||
|
||||
// validateWorkerTls is called by the Go TLS stack with client info. It calls
|
||||
// v1WorkerAuthConfig to validate the encryption and ensure the nonce has not
|
||||
// been replayed, then stores connection information into the auth cache, which
|
||||
// is used by the intercepting listener to log information.
|
||||
func (c Controller) validateWorkerTls(hello *tls.ClientHelloInfo) (*tls.Config, error) {
|
||||
for _, p := range hello.SupportedProtos {
|
||||
switch {
|
||||
case strings.HasPrefix(p, "v1workerauth-"):
|
||||
tlsConf, workerInfo, err := c.v1WorkerAuthConfig(hello.SupportedProtos)
|
||||
if err == nil {
|
||||
// Set the info we need to prevent replays
|
||||
c.workerAuthCache.Set(workerInfo.ConnectionNonce, &workerAuthEntry{
|
||||
// Store info that can be retrieved in the intercepting
|
||||
// listener.
|
||||
//
|
||||
// NOTE: This is not a load-or-store because we need to ensure
|
||||
// replays can't happen across controllers, hence storing in the
|
||||
// database in v1WorkerAuthConfig. Although this could serve as
|
||||
// a secondary check, a load-or-store here keeps the test from
|
||||
// ensuring that it is rejected at the database level via the
|
||||
// nonce unique constraint, which is by far the more important
|
||||
// thing to validate (although there are DB-level tests for that
|
||||
// too).
|
||||
c.workerAuthCache.Store(workerInfo.ConnectionNonce, &workerAuthEntry{
|
||||
WorkerAuthInfo: workerInfo,
|
||||
}, 0)
|
||||
})
|
||||
}
|
||||
return tlsConf, err
|
||||
}
|
||||
@@ -37,6 +53,13 @@ func (c Controller) validateWorkerTls(hello *tls.ClientHelloInfo) (*tls.Config,
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// v1WorkerAuthConfig:
|
||||
//
|
||||
// * Reads the information from ALPN, combining across protos
|
||||
// * Combines them to form the encrypted auth information
|
||||
// * Validates it by decrypting against the configured KMS
|
||||
// * Ensures that the nonce is unique to prevent replay attacks
|
||||
// * Returns the shared TLS configuration that is used to establish the connection
|
||||
func (c Controller) v1WorkerAuthConfig(protos []string) (*tls.Config, *base.WorkerAuthInfo, error) {
|
||||
var firstMatchProto string
|
||||
var encString string
|
||||
@@ -69,6 +92,15 @@ func (c Controller) v1WorkerAuthConfig(protos []string) (*tls.Config, *base.Work
|
||||
return nil, nil, err
|
||||
}
|
||||
|
||||
// Check for replays
|
||||
serversRepo, err := c.ServersRepoFn()
|
||||
if err != nil {
|
||||
return nil, nil, fmt.Errorf("unable to fetch servers repo: %w", err)
|
||||
}
|
||||
if err := serversRepo.AddNonce(c.baseContext, info.ConnectionNonce, servers.NoncePurposeWorkerAuth); err != nil {
|
||||
return nil, nil, fmt.Errorf("unable to add connection nonce to database: %w", err)
|
||||
}
|
||||
|
||||
rootCAs := x509.NewCertPool()
|
||||
if ok := rootCAs.AppendCertsFromPEM(info.CertPEM); !ok {
|
||||
return nil, info, errors.New("unable to add ca cert to cert pool")
|
||||
|
||||
@@ -207,18 +207,38 @@ func (r *Repository) UpsertServer(ctx context.Context, server *Server, opt ...Op
|
||||
return controllers, int(rowsUpdated), nil
|
||||
}
|
||||
|
||||
type RecoveryNonce struct {
|
||||
Nonce string
|
||||
type Nonce struct {
|
||||
Nonce string
|
||||
Purpose string
|
||||
}
|
||||
|
||||
// AddRecoveryNonce adds a nonce
|
||||
func (r *Repository) AddRecoveryNonce(ctx context.Context, nonce string, opt ...Option) error {
|
||||
const op = "servers.AddRecoveryNonce"
|
||||
// TableName returns the table name.
|
||||
func (n *Nonce) TableName() string {
|
||||
return "nonce"
|
||||
}
|
||||
|
||||
const (
|
||||
NoncePurposeRecovery = "recovery"
|
||||
NoncePurposeWorkerAuth = "worker-auth"
|
||||
)
|
||||
|
||||
// AddNonce adds a nonce
|
||||
func (r *Repository) AddNonce(ctx context.Context, nonce, purpose string, opt ...Option) error {
|
||||
const op = "servers.AddNonce"
|
||||
if nonce == "" {
|
||||
return errors.New(ctx, errors.InvalidParameter, op, "empty nonce")
|
||||
}
|
||||
rn := &RecoveryNonce{Nonce: nonce}
|
||||
if err := r.writer.Create(ctx, rn); err != nil {
|
||||
switch purpose {
|
||||
case NoncePurposeRecovery, NoncePurposeWorkerAuth:
|
||||
case "":
|
||||
return errors.New(ctx, errors.InvalidParameter, op, "empty nonce purpose")
|
||||
default:
|
||||
return errors.New(ctx, errors.InvalidParameter, op, fmt.Sprintf("unknown nonce purpose %q", purpose))
|
||||
}
|
||||
if err := r.writer.Create(ctx, &Nonce{
|
||||
Nonce: nonce,
|
||||
Purpose: purpose,
|
||||
}); err != nil {
|
||||
return errors.Wrap(ctx, err, op+":Insertion")
|
||||
}
|
||||
return nil
|
||||
@@ -226,10 +246,15 @@ func (r *Repository) AddRecoveryNonce(ctx context.Context, nonce string, opt ...
|
||||
|
||||
// CleanupNonces removes nonces that no longer need to be stored
|
||||
func (r *Repository) CleanupNonces(ctx context.Context, opt ...Option) (int, error) {
|
||||
// Use the largest validity period out of the various nonces we're looking at
|
||||
maxDuration := globals.RecoveryTokenValidityPeriod
|
||||
if globals.WorkerAuthNonceValidityPeriod > maxDuration {
|
||||
maxDuration = globals.WorkerAuthNonceValidityPeriod
|
||||
}
|
||||
// If something was inserted before 3x the actual validity period, clean it out
|
||||
endTime := time.Now().Add(-3 * globals.RecoveryTokenValidityPeriod)
|
||||
endTime := time.Now().Add(-3 * maxDuration)
|
||||
|
||||
rows, err := r.writer.Delete(ctx, &RecoveryNonce{}, db.WithWhere(deleteWhereCreateTimeSql, endTime))
|
||||
rows, err := r.writer.Delete(ctx, &Nonce{}, db.WithWhere(deleteWhereCreateTimeSql, endTime))
|
||||
if err != nil {
|
||||
return db.NoRowsAffected, errors.Wrap(ctx, err, "servers.CleanupNonces")
|
||||
}
|
||||
@@ -237,9 +262,9 @@ func (r *Repository) CleanupNonces(ctx context.Context, opt ...Option) (int, err
|
||||
}
|
||||
|
||||
// ListNonces lists nonces. Used only for tests at the moment.
|
||||
func (r *Repository) ListNonces(ctx context.Context, opt ...Option) ([]*RecoveryNonce, error) {
|
||||
var nonces []*RecoveryNonce
|
||||
if err := r.reader.SearchWhere(ctx, &nonces, "", nil, db.WithLimit(-1)); err != nil {
|
||||
func (r *Repository) ListNonces(ctx context.Context, purpose string, opt ...Option) ([]*Nonce, error) {
|
||||
var nonces []*Nonce
|
||||
if err := r.reader.SearchWhere(ctx, &nonces, "purpose = ?", []interface{}{purpose}, db.WithLimit(-1)); err != nil {
|
||||
return nil, errors.Wrap(ctx, err, "servers.ListNonces")
|
||||
}
|
||||
return nonces, nil
|
||||
|
||||
@@ -22,7 +22,8 @@ func TestRecoveryNonces(t *testing.T) {
|
||||
|
||||
// Set these low so that we can not have the test run forever
|
||||
globals.RecoveryTokenValidityPeriod = 10 * time.Second
|
||||
controller.RecoveryNonceCleanupInterval = 20 * time.Second
|
||||
globals.WorkerAuthNonceValidityPeriod = 10 * time.Second
|
||||
controller.NonceCleanupInterval = 20 * time.Second
|
||||
|
||||
wrapper := db.TestWrapper(t)
|
||||
tc := controller.NewTestController(t, &controller.TestControllerOpts{
|
||||
@@ -46,14 +47,14 @@ func TestRecoveryNonces(t *testing.T) {
|
||||
roleClient := roles.NewClient(client)
|
||||
_, err = roleClient.Create(tc.Context(), scope.Global.String())
|
||||
require.NoError(err)
|
||||
nonces, err := repo.ListNonces(tc.Context())
|
||||
nonces, err := repo.ListNonces(tc.Context(), servers.NoncePurposeRecovery)
|
||||
require.NoError(err)
|
||||
assert.Len(nonces, 1)
|
||||
|
||||
// Token 1, try 2
|
||||
_, err = roleClient.Create(tc.Context(), scope.Global.String())
|
||||
require.Error(err)
|
||||
nonces, err = repo.ListNonces(tc.Context())
|
||||
nonces, err = repo.ListNonces(tc.Context(), servers.NoncePurposeRecovery)
|
||||
require.NoError(err)
|
||||
assert.Len(nonces, 1)
|
||||
|
||||
@@ -61,13 +62,13 @@ func TestRecoveryNonces(t *testing.T) {
|
||||
roleClient.ApiClient().SetToken(token2)
|
||||
_, err = roleClient.Create(tc.Context(), scope.Global.String())
|
||||
require.NoError(err)
|
||||
nonces, err = repo.ListNonces(tc.Context())
|
||||
nonces, err = repo.ListNonces(tc.Context(), servers.NoncePurposeRecovery)
|
||||
require.NoError(err)
|
||||
assert.Len(nonces, 2)
|
||||
|
||||
// Make sure they get cleaned up
|
||||
time.Sleep(2 * controller.RecoveryNonceCleanupInterval)
|
||||
nonces, err = repo.ListNonces(tc.Context())
|
||||
time.Sleep(2 * controller.NonceCleanupInterval)
|
||||
nonces, err = repo.ListNonces(tc.Context(), servers.NoncePurposeRecovery)
|
||||
require.NoError(err)
|
||||
assert.Len(nonces, 0)
|
||||
|
||||
@@ -76,7 +77,7 @@ func TestRecoveryNonces(t *testing.T) {
|
||||
roleClient.ApiClient().SetToken(token)
|
||||
_, err = roleClient.Create(tc.Context(), scope.Global.String())
|
||||
require.Error(err)
|
||||
nonces, err = repo.ListNonces(tc.Context())
|
||||
nonces, err = repo.ListNonces(tc.Context(), servers.NoncePurposeRecovery)
|
||||
require.NoError(err)
|
||||
assert.Len(nonces, 0)
|
||||
}
|
||||
|
||||
@@ -17,6 +17,7 @@ import (
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/hashicorp/boundary/globals"
|
||||
"github.com/hashicorp/boundary/internal/cmd/base"
|
||||
pbs "github.com/hashicorp/boundary/internal/gen/controller/servers/services"
|
||||
"github.com/hashicorp/boundary/internal/observability/event"
|
||||
@@ -137,11 +138,17 @@ func (w *Worker) createClientConn(addr string) error {
|
||||
func (w *Worker) workerAuthTLSConfig() (*tls.Config, *base.WorkerAuthInfo, error) {
|
||||
var err error
|
||||
info := &base.WorkerAuthInfo{
|
||||
Name: w.conf.RawConfig.Worker.Name,
|
||||
Description: w.conf.RawConfig.Worker.Description,
|
||||
Name: w.conf.RawConfig.Worker.Name,
|
||||
Description: w.conf.RawConfig.Worker.Description,
|
||||
ConnectionNonce: w.testReusedAuthNonce,
|
||||
}
|
||||
if info.ConnectionNonce, err = base62.Random(20); err != nil {
|
||||
return nil, nil, err
|
||||
if info.ConnectionNonce == "" {
|
||||
if info.ConnectionNonce, err = base62.Random(20); err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
if w.testReuseAuthNonces {
|
||||
w.testReusedAuthNonce = info.ConnectionNonce
|
||||
}
|
||||
}
|
||||
|
||||
pubKey, privKey, err := ed25519.GenerateKey(w.conf.SecureRandomReader)
|
||||
@@ -162,7 +169,7 @@ func (w *Worker) workerAuthTLSConfig() (*tls.Config, *base.WorkerAuthInfo, error
|
||||
KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment | x509.KeyUsageKeyAgreement | x509.KeyUsageCertSign,
|
||||
SerialNumber: big.NewInt(mathrand.Int63()),
|
||||
NotBefore: time.Now().Add(-30 * time.Second),
|
||||
NotAfter: time.Now().Add(2 * time.Minute),
|
||||
NotAfter: time.Now().Add(globals.WorkerAuthNonceValidityPeriod),
|
||||
BasicConstraintsValid: true,
|
||||
IsCA: true,
|
||||
}
|
||||
|
||||
@@ -182,6 +182,9 @@ type TestWorkerOpts struct {
|
||||
// The amount of time to wait before marking connections as closed when a
|
||||
// connection cannot be made back to the controller
|
||||
StatusGracePeriodDuration time.Duration
|
||||
|
||||
// Whether to set the replay value
|
||||
EnableAuthReplay bool
|
||||
}
|
||||
|
||||
func NewTestWorker(t *testing.T, opts *TestWorkerOpts) *TestWorker {
|
||||
@@ -277,6 +280,10 @@ func NewTestWorker(t *testing.T, opts *TestWorkerOpts) *TestWorker {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if opts.EnableAuthReplay {
|
||||
tw.w.testReuseAuthNonces = true
|
||||
}
|
||||
|
||||
if !opts.DisableAutoStart {
|
||||
if err := tw.w.Start(); err != nil {
|
||||
tw.Shutdown()
|
||||
|
||||
@@ -52,6 +52,10 @@ type Worker struct {
|
||||
// request. It can be set via startup in New below, or (eventually) via
|
||||
// SIGHUP.
|
||||
updateTags ua.Bool
|
||||
|
||||
// Test-related values
|
||||
testReuseAuthNonces bool
|
||||
testReusedAuthNonce string
|
||||
}
|
||||
|
||||
func New(conf *Config) (*Worker, error) {
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
package cluster
|
||||
|
||||
import (
|
||||
"os"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/hashicorp/boundary/internal/cmd/config"
|
||||
"github.com/hashicorp/boundary/internal/observability/event"
|
||||
"github.com/hashicorp/boundary/internal/servers"
|
||||
"github.com/hashicorp/boundary/internal/servers/controller"
|
||||
"github.com/hashicorp/boundary/internal/servers/worker"
|
||||
"github.com/hashicorp/go-hclog"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestWorkerReplay(t *testing.T) {
|
||||
ec := event.TestEventerConfig(t, "TestWorkerReplay", event.TestWithObservationSink(t), event.TestWithSysSink(t))
|
||||
testLock := &sync.Mutex{}
|
||||
testLogger := hclog.New(&hclog.LoggerOptions{
|
||||
Mutex: testLock,
|
||||
Name: "test",
|
||||
})
|
||||
require.NoError(t, event.InitSysEventer(testLogger, testLock, "use-TestWorkerReplay", event.WithEventerConfig(&ec.EventerConfig)))
|
||||
|
||||
conf, err := config.DevController()
|
||||
conf.Eventing = &ec.EventerConfig
|
||||
require.NoError(t, err)
|
||||
c1 := controller.NewTestController(t, &controller.TestControllerOpts{
|
||||
Config: conf,
|
||||
})
|
||||
defer c1.Shutdown()
|
||||
|
||||
conf, err = config.DevWorker()
|
||||
conf.Eventing = &ec.EventerConfig
|
||||
require.NoError(t, err)
|
||||
w1 := worker.NewTestWorker(t, &worker.TestWorkerOpts{
|
||||
Config: conf,
|
||||
WorkerAuthKms: c1.Config().WorkerAuthKms,
|
||||
InitialControllers: c1.ClusterAddrs(),
|
||||
EnableAuthReplay: true,
|
||||
})
|
||||
|
||||
// Give time for it to connect
|
||||
time.Sleep(10 * time.Second)
|
||||
require.NoError(t, w1.Worker().Shutdown(true))
|
||||
|
||||
// Now, start up again
|
||||
require.NoError(t, w1.Worker().Start())
|
||||
time.Sleep(10 * time.Second)
|
||||
|
||||
// We should find only one nonce, and one successful worker authentication,
|
||||
// both in the output and in the database
|
||||
ec.AllEvents.Close()
|
||||
logBuf, err := os.ReadFile(ec.AllEvents.Name())
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 1, strings.Count(string(logBuf), "worker successfully authed"))
|
||||
nonces, err := c1.ServersRepo().ListNonces(c1.Context(), servers.NoncePurposeWorkerAuth)
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, nonces, 1)
|
||||
}
|
||||
@@ -57,8 +57,9 @@ TLS establishment is performed as follows:
|
||||
encrypted value.
|
||||
|
||||
5. The Controller decrypts this value via the shared KMS. If successful, the
|
||||
Controller validates that the nonce is not known, to ensure that this is not a
|
||||
replay.
|
||||
Controller validates that the nonce is not known to ensure that this is not a
|
||||
replay. The nonce value is checked against and stored into the database,
|
||||
ensuring that nonce replay cannot occur across controllers.
|
||||
|
||||
6. The Controller uses the decrypted parameters to configures its TLS stack with
|
||||
the same certificate and key.
|
||||
@@ -67,11 +68,6 @@ TLS establishment is performed as follows:
|
||||
self-signed CA certificate that was securely transmitted is configured as a
|
||||
valid root CA for validation checking.
|
||||
|
||||
8. If successful, the nonce is stored in the database along with an expiration
|
||||
time set several minutes past the actual expiration time of the certificate
|
||||
itself. This ensures that any replay attempt that occurs is detected and
|
||||
rejected until after the certificate is otherwise invalid.
|
||||
|
||||
## Client-to-Worker TLS
|
||||
|
||||
Workers do not require any configuration for their client-facing listeners to
|
||||
|
||||
Reference in new issue
Block a user