[VAULT-31754] Check removed status in sys/unseal and error out if the node has been removed from the cluster (#28909)

This commit is contained in:
Kuba Wieczorek
2024-11-14 13:23:30 +00:00
committed by GitHub
parent 4536681edf
commit d211f47d9e
5 changed files with 155 additions and 0 deletions

View File

@@ -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 {

View File

@@ -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

View File

@@ -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
}

View File

@@ -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)
}
}

View File

@@ -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)
}