From 88eb750a798d431b66637751cfb3e3a2a198677b Mon Sep 17 00:00:00 2001 From: Nick Cabatoff Date: Thu, 25 Nov 2021 14:56:50 -0500 Subject: [PATCH] Backport 1.7: Return an error when trying to store a too-large key with Raft (#13282) (#13285) --- changelog/13282.txt | 3 ++ physical/raft/raft.go | 41 +++++++++++-------- physical/raft/raft_test.go | 39 +++++++++++++++--- sdk/physical/physical.go | 1 + .../hashicorp/vault/sdk/physical/physical.go | 1 + 5 files changed, 62 insertions(+), 23 deletions(-) create mode 100644 changelog/13282.txt diff --git a/changelog/13282.txt b/changelog/13282.txt new file mode 100644 index 000000000000..2c32209c0878 --- /dev/null +++ b/changelog/13282.txt @@ -0,0 +1,3 @@ +```release-note:bug +storage/raft: Fix a panic when trying to write a key > 32KB +``` diff --git a/physical/raft/raft.go b/physical/raft/raft.go index 5d207ad0709b..e790a825d072 100644 --- a/physical/raft/raft.go +++ b/physical/raft/raft.go @@ -5,11 +5,6 @@ import ( "crypto/tls" "errors" "fmt" - "github.com/hashicorp/vault/sdk/helper/consts" - "github.com/hashicorp/vault/sdk/helper/jsonutil" - "github.com/hashicorp/vault/sdk/helper/tlsutil" - "github.com/hashicorp/vault/sdk/logical" - "github.com/hashicorp/vault/sdk/physical" "io" "io/ioutil" "os" @@ -18,6 +13,12 @@ import ( "sync" "time" + "github.com/hashicorp/vault/sdk/helper/consts" + "github.com/hashicorp/vault/sdk/helper/jsonutil" + "github.com/hashicorp/vault/sdk/helper/tlsutil" + "github.com/hashicorp/vault/sdk/logical" + "github.com/hashicorp/vault/sdk/physical" + "github.com/armon/go-metrics" "github.com/golang/protobuf/proto" "github.com/hashicorp/errwrap" @@ -31,6 +32,7 @@ import ( raftboltdb "github.com/hashicorp/vault/physical/raft/logstore" "github.com/hashicorp/vault/vault/cluster" "github.com/hashicorp/vault/vault/seal" + "go.etcd.io/bbolt" ) // EnvVaultRaftNodeID is used to fetch the Raft node ID from the environment. @@ -40,10 +42,12 @@ const EnvVaultRaftNodeID = "VAULT_RAFT_NODE_ID" const EnvVaultRaftPath = "VAULT_RAFT_PATH" // Verify RaftBackend satisfies the correct interfaces -var _ physical.Backend = (*RaftBackend)(nil) -var _ physical.Transactional = (*RaftBackend)(nil) -var _ physical.HABackend = (*RaftBackend)(nil) -var _ physical.Lock = (*RaftLock)(nil) +var ( + _ physical.Backend = (*RaftBackend)(nil) + _ physical.Transactional = (*RaftBackend)(nil) + _ physical.HABackend = (*RaftBackend)(nil) + _ physical.Lock = (*RaftLock)(nil) +) var ( // raftLogCacheSize is the maximum number of logs to cache in-memory. @@ -269,7 +273,7 @@ func EnsurePath(path string, dir bool) error { if !dir { path = filepath.Dir(path) } - return os.MkdirAll(path, 0755) + return os.MkdirAll(path, 0o755) } // NewRaftBackend constructs a RaftBackend using the given directory @@ -317,7 +321,7 @@ func NewRaftBackend(conf map[string]string, logger log.Logger) (physical.Backend return nil, err } - if err := ioutil.WriteFile(filepath.Join(path, "node-id"), []byte(id), 0600); err != nil { + if err := ioutil.WriteFile(filepath.Join(path, "node-id"), []byte(id), 0o600); err != nil { return nil, err } @@ -1111,7 +1115,7 @@ func (b *RaftBackend) RestoreSnapshot(ctx context.Context, metadata raft.Snapsho // snapshot applied to a quorum of nodes. command := &LogData{ Operations: []*LogOperation{ - &LogOperation{ + { OpType: restoreCallbackOp, }, }, @@ -1130,7 +1134,7 @@ func (b *RaftBackend) Delete(ctx context.Context, path string) error { defer metrics.MeasureSince([]string{"raft-storage", "delete"}, time.Now()) command := &LogData{ Operations: []*LogOperation{ - &LogOperation{ + { OpType: deleteOp, Key: path, }, @@ -1174,9 +1178,13 @@ func (b *RaftBackend) Get(ctx context.Context, path string) (*physical.Entry, er // or if the call to applyLog fails. func (b *RaftBackend) Put(ctx context.Context, entry *physical.Entry) error { defer metrics.MeasureSince([]string{"raft-storage", "put"}, time.Now()) + if len(entry.Key) > bbolt.MaxKeySize { + return fmt.Errorf("%s, max key size for integrated storage is %d", physical.ErrKeyTooLarge, bbolt.MaxKeySize) + } + command := &LogData{ Operations: []*LogOperation{ - &LogOperation{ + { OpType: putOp, Key: entry.Key, Value: entry.Value, @@ -1387,7 +1395,6 @@ func (l *RaftLock) Lock(stopCh <-chan struct{}) (<-chan struct{}, error) { case <-stopCh: return nil, nil } - } l.b.l.RLock() @@ -1405,7 +1412,7 @@ func (l *RaftLock) Lock(stopCh <-chan struct{}) (<-chan struct{}, error) { if l.b.raft.State() == raft.Leader { err := l.b.applyLog(context.Background(), &LogData{ Operations: []*LogOperation{ - &LogOperation{ + { OpType: putOp, Key: l.key, Value: l.value, @@ -1429,7 +1436,7 @@ func (l *RaftLock) Lock(stopCh <-chan struct{}) (<-chan struct{}, error) { l.b.l.RLock() err := l.b.applyLog(context.Background(), &LogData{ Operations: []*LogOperation{ - &LogOperation{ + { OpType: putOp, Key: l.key, Value: l.value, diff --git a/physical/raft/raft_test.go b/physical/raft/raft_test.go index 8f5af54d5466..cf6952a11589 100644 --- a/physical/raft/raft_test.go +++ b/physical/raft/raft_test.go @@ -20,6 +20,7 @@ import ( hclog "github.com/hashicorp/go-hclog" uuid "github.com/hashicorp/go-uuid" "github.com/hashicorp/raft" + "github.com/hashicorp/vault/sdk/helper/base62" "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/physical" bolt "go.etcd.io/bbolt" @@ -184,7 +185,6 @@ func compareDBs(t *testing.T, boltDB1, boltDB2 *bolt.DB, dataOnly bool) error { return nil }) - if err != nil { t.Fatal(err) } @@ -225,6 +225,34 @@ func TestRaft_Backend(t *testing.T) { physical.ExerciseBackend(t, b) } +func TestRaft_Backend_LargeKey(t *testing.T) { + b, dir := getRaft(t, true, true) + defer os.RemoveAll(dir) + + key, err := base62.Random(bolt.MaxKeySize + 1) + if err != nil { + t.Fatal(err) + } + entry := &physical.Entry{Key: key, Value: []byte(key)} + + err = b.Put(context.Background(), entry) + if err == nil { + t.Fatal("expected error for put entry") + } + + if !strings.Contains(err.Error(), physical.ErrKeyTooLarge) { + t.Fatalf("expected %q, got %v", physical.ErrKeyTooLarge, err) + } + + out, err := b.Get(context.Background(), entry.Key) + if err != nil { + t.Fatalf("unexpected error after failed put: %v", err) + } + if out != nil { + t.Fatal("expected response entry to be nil after a failed put") + } +} + func TestRaft_Backend_LargeValue(t *testing.T) { b, dir := getRaft(t, true, true) defer os.RemoveAll(dir) @@ -259,7 +287,7 @@ func TestRaft_TransactionalBackend_LargeValue(t *testing.T) { rand.Read(value) txns := []*physical.TxnEntry{ - &physical.TxnEntry{ + { Operation: physical.PutOperation, Entry: &physical.Entry{ Key: "foo", @@ -391,15 +419,15 @@ func TestRaft_Recovery(t *testing.T) { if err != nil { t.Fatal(err) } - err = ioutil.WriteFile(filepath.Join(filepath.Join(dir1, raftState), "peers.json"), peersJSONBytes, 0644) + err = ioutil.WriteFile(filepath.Join(filepath.Join(dir1, raftState), "peers.json"), peersJSONBytes, 0o644) if err != nil { t.Fatal(err) } - err = ioutil.WriteFile(filepath.Join(filepath.Join(dir2, raftState), "peers.json"), peersJSONBytes, 0644) + err = ioutil.WriteFile(filepath.Join(filepath.Join(dir2, raftState), "peers.json"), peersJSONBytes, 0o644) if err != nil { t.Fatal(err) } - err = ioutil.WriteFile(filepath.Join(filepath.Join(dir4, raftState), "peers.json"), peersJSONBytes, 0644) + err = ioutil.WriteFile(filepath.Join(filepath.Join(dir4, raftState), "peers.json"), peersJSONBytes, 0o644) if err != nil { t.Fatal(err) } @@ -499,7 +527,6 @@ func TestRaft_Backend_Performance(t *testing.T) { if localConfig.LeaderLeaseTimeout != defaultConfig.LeaderLeaseTimeout { t.Fatalf("bad config: %v", localConfig) } - } func BenchmarkDB_Puts(b *testing.B) { diff --git a/sdk/physical/physical.go b/sdk/physical/physical.go index 8cc4e9ab17f1..808abd50fcd8 100644 --- a/sdk/physical/physical.go +++ b/sdk/physical/physical.go @@ -21,6 +21,7 @@ const ( const ( ErrValueTooLarge = "put failed due to value being too large" + ErrKeyTooLarge = "put failed due to key being too large" ) // Backend is the interface required for a physical diff --git a/vendor/github.com/hashicorp/vault/sdk/physical/physical.go b/vendor/github.com/hashicorp/vault/sdk/physical/physical.go index 8cc4e9ab17f1..808abd50fcd8 100644 --- a/vendor/github.com/hashicorp/vault/sdk/physical/physical.go +++ b/vendor/github.com/hashicorp/vault/sdk/physical/physical.go @@ -21,6 +21,7 @@ const ( const ( ErrValueTooLarge = "put failed due to value being too large" + ErrKeyTooLarge = "put failed due to key being too large" ) // Backend is the interface required for a physical