fix(daemon): deny cross tenant attach on empty tenant id volumes

This commit is contained in:
Julian Sommer
2026-05-01 11:15:58 +10:00
parent b7dea45ee0
commit ca965e48d6
4 changed files with 86 additions and 2 deletions
+2
View File
@@ -1608,6 +1608,7 @@ func TestAttachVolume_ZoneMismatch(t *testing.T) {
SizeGiB: 10,
State: "available",
AvailabilityZone: "us-west-2a",
TenantID: testAccountID,
},
},
}
@@ -3416,6 +3417,7 @@ func TestAttachVolume_VolumeInUse(t *testing.T) {
VolumeID: volumeID,
SizeGiB: 10,
State: "in-use",
TenantID: testAccountID,
},
},
}
+4 -2
View File
@@ -54,9 +54,11 @@ func (d *Daemon) handleAttachVolume(msg *nats.Msg, command types.EC2InstanceComm
return
}
// Account scoping: verify the caller owns this volume
// Account scoping: verify the caller owns this volume.
// Pre-Phase4 volumes (empty TenantID) are root-only; otherwise the caller
// must match the recorded tenant exactly.
callerAccountID := utils.AccountIDFromMsg(msg)
if callerAccountID != "" && volCfg.VolumeMetadata.TenantID != "" && volCfg.VolumeMetadata.TenantID != callerAccountID {
if !volumeVisibleTo(volCfg.VolumeMetadata.TenantID, callerAccountID) {
slog.Warn("AttachVolume: account does not own volume", "volumeId", volumeID, "callerAccount", callerAccountID, "ownerAccount", volCfg.VolumeMetadata.TenantID)
respondWithError(msg, awserrors.ErrorInvalidVolumeNotFound)
return
+12
View File
@@ -43,3 +43,15 @@ func isInstanceVisible(callerAccountID, ownerAccountID string) bool {
}
return callerAccountID == ownerAccountID
}
// volumeVisibleTo reports whether callerAccountID may operate on a volume with
// the given tenantID. Pre-Phase4 volumes (empty tenantID) are root-only —
// without this, the legacy short-circuit `tenantID != "" && tenantID != caller`
// matched every caller and let any tenant attach a legacy/migration volume by
// ID alone.
func volumeVisibleTo(tenantID, callerAccountID string) bool {
if tenantID == "" {
return callerAccountID == utils.GlobalAccountID
}
return callerAccountID == tenantID
}
+68
View File
@@ -61,3 +61,71 @@ func TestIsInstanceVisible(t *testing.T) {
})
}
}
func TestVolumeVisibleTo(t *testing.T) {
tests := []struct {
name string
tenantID string
callerAccountID string
expected bool
}{
{
name: "same tenant",
tenantID: "123456789012",
callerAccountID: "123456789012",
expected: true,
},
{
name: "different tenant denied",
tenantID: "123456789012",
callerAccountID: "999999999999",
expected: false,
},
{
name: "pre-Phase4 volume visible to root",
tenantID: "",
callerAccountID: utils.GlobalAccountID,
expected: true,
},
{
name: "pre-Phase4 volume hidden from non-root tenant",
tenantID: "",
callerAccountID: "123456789012",
expected: false,
},
{
name: "pre-Phase4 volume denied to empty caller",
tenantID: "",
callerAccountID: "",
expected: false,
},
{
name: "tenant volume denied to empty caller",
tenantID: "123456789012",
callerAccountID: "",
expected: false,
},
{
name: "root cannot impersonate tenant volume",
tenantID: "123456789012",
callerAccountID: utils.GlobalAccountID,
expected: false,
},
{
name: "root accessing root-owned volume",
tenantID: utils.GlobalAccountID,
callerAccountID: utils.GlobalAccountID,
expected: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := volumeVisibleTo(tt.tenantID, tt.callerAccountID)
if got != tt.expected {
t.Errorf("volumeVisibleTo(%q, %q) = %v, want %v",
tt.tenantID, tt.callerAccountID, got, tt.expected)
}
})
}
}