diff --git a/http/sys_seal.go b/http/sys_seal.go index 4852d57d5e..81af5551db 100644 --- a/http/sys_seal.go +++ b/http/sys_seal.go @@ -85,6 +85,14 @@ func handleSysUnseal(core *vault.Core) http.Handler { return } + // Check if this node was removed from the cluster. If so, respond with an error and return, + // since we don't want a removed node to be able to unseal. + removed, ok := core.IsRemovedFromCluster() + if ok && removed { + respondError(w, http.StatusInternalServerError, errors.New("node was removed from a HA cluster")) + return + } + // Parse the request var req UnsealRequest if _, err := parseJSONRequest(core.PerfStandby(), r, w, &req); err != nil { diff --git a/http/sys_seal_test.go b/http/sys_seal_test.go index 694dc971c7..173d0ab541 100644 --- a/http/sys_seal_test.go +++ b/http/sys_seal_test.go @@ -439,6 +439,35 @@ func TestSysUnseal_Reset(t *testing.T) { } } +// TestSysUnseal_NodeRemovedFromCluster verifies that a call to /sys/unseal fails +// with an appropriate error when the node has been removed from a cluster. +func TestSysUnseal_NodeRemovedFromCluster(t *testing.T) { + core, err := vault.TestCoreWithMockRemovableNodeHABackend(t, true) + if err != nil { + t.Fatalf("err: %s", err) + } + ln, addr := TestServer(t, core) + defer ln.Close() + + // The value of key doesn't matter here, we just need to make a request. + resp := testHttpPut(t, "", addr+"/v1/sys/unseal", map[string]interface{}{ + "key": "foo", + }) + + testResponseStatus(t, resp, 500) + var actual map[string]interface{} + testResponseBody(t, resp, &actual) + + errors, ok := actual["errors"].([]interface{}) + if !ok { + t.Fatalf("no errors in the response, request should be invalid") + } + expectedErrorMsg := "node was removed from a HA cluster" + if !strings.Contains(errors[0].(string), expectedErrorMsg) { + t.Fatalf("error message should contain %q", expectedErrorMsg) + } +} + // Test Seal's permissions logic, which is slightly different than normal code // paths in that it queries the ACL rather than having checkToken do it. This // is because it was abusing RootPaths in logical_system, but that caused some diff --git a/vault/core.go b/vault/core.go index 981d2a2fe7..d635165e3c 100644 --- a/vault/core.go +++ b/vault/core.go @@ -4573,3 +4573,24 @@ func (c *Core) setupAuditedHeadersConfig(ctx context.Context) error { return nil } + +// IsRemovedFromCluster checks whether this node has been removed from the +// cluster. This is only applicable to physical HA backends that satisfy the +// RemovableNodeHABackend interface. The value of the `ok` result will be false +// if the HA and underlyingPhysical backends are nil or do not support this operation. +func (c *Core) IsRemovedFromCluster() (removed, ok bool) { + var haBackend any + if c.ha != nil { + haBackend = c.ha + } else if c.underlyingPhysical != nil { + haBackend = c.underlyingPhysical + } else { + return false, false + } + removableNodeHA, ok := haBackend.(physical.RemovableNodeHABackend) + if !ok { + return false, false + } + + return removableNodeHA.IsRemoved(), true +} diff --git a/vault/core_test.go b/vault/core_test.go index 3506574051..0f2e0913fd 100644 --- a/vault/core_test.go +++ b/vault/core_test.go @@ -3700,3 +3700,59 @@ func TestBarrier_DeadlockDetection(t *testing.T) { t.Fatal("barrierLock doesn't have deadlock detection enabled, it should") } } + +// TestCore_IsRemovedFromCluster exercises all the execution paths in the +// IsRemovedFromCluster convenience method of the Core struct. +func TestCore_IsRemovedFromCluster(t *testing.T) { + core := &Core{} + + // Test case where both HA and underlying physical backends ares nil + removed, ok := core.IsRemovedFromCluster() + if removed || ok { + t.Fatalf("expected removed and ok to be false, got removed: %v, ok: %v", removed, ok) + } + + // Test case where HA backend is nil, but the underlying physical is there and does not support RemovableNodeHABackend + core.underlyingPhysical = &MockHABackend{} + removed, ok = core.IsRemovedFromCluster() + if removed || ok { + t.Fatalf("expected removed and ok to be false, got removed: %v, ok: %v", removed, ok) + } + + // Test case where HA backend is nil, but the underlying physical is there, supports RemovableNodeHABackend, and is not removed + mockHA := &MockRemovableNodeHABackend{} + core.underlyingPhysical = mockHA + removed, ok = core.IsRemovedFromCluster() + if removed || !ok { + t.Fatalf("expected removed and ok to be false, got removed: %v, ok: %v", removed, ok) + } + + // Test case where HA backend is nil, but the underlying physical is there, supports RemovableNodeHABackend, and is removed + mockHA.Removed = true + removed, ok = core.IsRemovedFromCluster() + if !removed || !ok { + t.Fatalf("expected removed to be false and ok to be true, got removed: %v, ok: %v", removed, ok) + } + + // Test case where HA backend does not support RemovableNodeHABackend + core.ha = &MockHABackend{} + removed, ok = core.IsRemovedFromCluster() + if removed || ok { + t.Fatalf("expected removed and ok to be false, got removed: %v, ok: %v", removed, ok) + } + + // Test case where HA backend supports RemovableNodeHABackend and is not removed + mockHA.Removed = false + core.ha = mockHA + removed, ok = core.IsRemovedFromCluster() + if removed || !ok { + t.Fatalf("expected removed and ok to be true, got removed: %v, ok: %v", removed, ok) + } + + // Test case where HA backend supports RemovableNodeHABackend and is removed + mockHA.Removed = true + removed, ok = core.IsRemovedFromCluster() + if !removed || !ok { + t.Fatalf("expected removed to be false and ok to be true, got removed: %v, ok: %v", removed, ok) + } +} diff --git a/vault/testing.go b/vault/testing.go index a3d952a76d..e6f1f26fcf 100644 --- a/vault/testing.go +++ b/vault/testing.go @@ -2315,3 +2315,44 @@ func TestCreateStorageGroup(ctx context.Context, c *Core, entityIDs []string) er } return nil } + +// Mock HABackend is a non-functional HABackend for testing purposes +type MockHABackend struct { + physical.HABackend + physical.Backend +} + +func (m *MockHABackend) HAEnabled() bool { + return true +} + +// MockRemovableNodeHABackend is a barely functional RemovableNodeHABackend for testing purposes. +// It has a functional IsRemoved method and an exported Removed field so that the desired state can be easily set. +type MockRemovableNodeHABackend struct { + physical.RemovableNodeHABackend + physical.Backend + Removed bool +} + +func (m *MockRemovableNodeHABackend) HAEnabled() bool { + return true +} + +func (m *MockRemovableNodeHABackend) IsRemoved() bool { + return m.Removed +} + +func TestCoreWithMockRemovableNodeHABackend(t *testing.T, removed bool) (*Core, error) { + t.Helper() + logger := corehelpers.NewTestLogger(t) + inmha, err := physInmem.NewInmemHA(nil, logger) + if err != nil { + t.Fatal(err) + } + conf := testCoreConfig(t, inmha, logger) + mockHABackend := &MockRemovableNodeHABackend{Removed: removed} + conf.HAPhysical = mockHABackend + conf.RedirectAddr = "http://127.0.0.1:8200" + + return NewCore(conf) +}