diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 200c683fd..25d4174a0 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -114,7 +114,7 @@ Operational commands for inspecting cluster state. These fan out NATS requests t | Command | Implemented Flags | Missing Flags | Status | |---------|-------------------|---------------|--------| -| `run-instances` | `--image-id`, `--instance-type`, `--count`, `--key-name`, `--user-data`, `--subnet-id`, `--security-group-ids`, `--tag-specifications` (instance-scoped), `--block-device-mappings` (DeviceName, VolumeSize, VolumeType, Iops, DeleteOnTermination), `--placement` (GroupName), `--iam-instance-profile` (Name/Arn), `--capacity-reservation-specification` (CapacityReservationTarget.CapacityReservationId, targeted-by-id only) | `--dry-run`, `--client-token`, `--disable-api-termination`, `--ebs-optimized`, `--network-interfaces`, `--private-ip-address`, `--monitoring`, `--credit-specification`, `--cpu-options`, `--metadata-options`, `--launch-template`, `--hibernate-options` | **DONE** | +| `run-instances` | `--image-id`, `--instance-type`, `--count`, `--key-name`, `--user-data`, `--subnet-id`, `--security-group-ids`, `--tag-specifications` (instance-scoped), `--block-device-mappings` (DeviceName, VolumeSize, VolumeType, Iops, DeleteOnTermination), `--placement` (GroupName), `--iam-instance-profile` (Name/Arn), `--capacity-reservation-specification` (CapacityReservationTarget.CapacityReservationId, targeted-by-id only), `--metadata-options` (HttpPutResponseHopLimit; IMDSv2-only enforced — rejects `http-tokens=optional`) | `--dry-run`, `--client-token`, `--disable-api-termination`, `--ebs-optimized`, `--network-interfaces`, `--private-ip-address`, `--monitoring`, `--credit-specification`, `--cpu-options`, `--launch-template`, `--hibernate-options` | **DONE** | | `describe-instances` | `--instance-ids`, `--filters` (instance-state-name, instance-id, instance-type, vpc-id, subnet-id, tag:*, tag-key, tag-value) | `--max-results`, `--next-token`, `--dry-run` | **DONE** | | `start-instances` | `--instance-ids` | `--dry-run`, `--force` | **DONE** | | `stop-instances` | `--instance-ids` | `--force`, `--hibernate`, `--dry-run` | **DONE** | @@ -124,6 +124,7 @@ Operational commands for inspecting cluster state. These fan out NATS requests t | `modify-instance-attribute` | `--instance-id`, `--instance-type`, `--user-data`, `--disable-api-termination` | `--ebs-optimized`, `--source-dest-check`, `--instance-initiated-shutdown-behavior`, `--block-device-mappings`, `--groups`, `--ena-support`, `--sriov-net-support` | **DONE** | | `get-console-output` | `--instance-id` | `--latest`, `--dry-run` | **DONE** | | `describe-instance-attribute` | `--instance-id`, `--attribute` (instanceType, userData, disableApiTermination, instanceInitiatedShutdownBehavior, disableApiStop, ebsOptimized, enaSupport, sourceDestCheck, rootDeviceName, kernel, ramdisk) | `--dry-run` | **DONE** | +| `modify-instance-metadata-options` | `--instance-id`, `--http-put-response-hop-limit` (1–64), `--http-tokens` (`required`), `--http-endpoint` (`enabled`), `--http-protocol-ipv6`/`--instance-metadata-tags` (`disabled`) — secure values are no-ops, downgrades return `UnsupportedOperation` | `--dry-run` | **DONE** | | `describe-instance-credit-specifications` | `--instance-ids` | `--filters`, `--max-results`, `--dry-run` | **DONE** (stub — always returns `standard`) | | `describe-instance-status` | `--instance-ids`, `--include-all-instances`, `--filters` (availability-zone, instance-state-code, instance-state-name, tag:*) | `--max-results`, `--next-token`, `--dry-run`, event/instance-status/system-status filters | **DONE** (static health) | | `monitor-instances` | — | `--instance-ids` | **NOT STARTED** | @@ -138,6 +139,13 @@ the node owning the targeted reservation (targeted-by-id only; `open` auto-matching is not supported). The combination with `--placement` (group) is rejected. `describe-instances` then reports the instance's `CapacityReservationId`. +`modify-instance-metadata-options` is the only metadata-options mutator — AWS has +no `get-instance-metadata-options` API, so the per-instance block is read through +`describe-instances`. Only the hop limit is mutable; `--http-tokens optional` and +the other posture-weakening values are refused with `UnsupportedOperation`, since +the platform enforces IMDSv2 permanently. `run-instances --metadata-options` runs +the same validation at launch. + ### EC2 — IAM Instance Profile Associations Stored as `vm.VM.IamInstanceProfileArn` + `IamInstanceProfileAssociationId` @@ -240,6 +248,11 @@ by downstream services. | `modify-instance-metadata-defaults` | — | `--http-tokens`, `--http-put-response-hop-limit`, `--http-endpoint`, `--instance-metadata-tags` | **NOT STARTED** | | `get-instance-metadata-defaults` | — | `--dry-run` | **NOT STARTED** | +The account-level `modify-/get-instance-metadata-defaults` are blocked on an +`aws-sdk-go` bump: the `InstanceMetadataDefaultsResponse` type is absent from the +pinned v1.44.266 (these APIs landed ~March 2024), and the gateway's generic +handler needs the typed input struct to route them. + ### EC2 — VPC Core | Command | Implemented Flags | Missing Flags | Status | @@ -583,6 +596,8 @@ Available at `169.254.169.254` from inside every running guest VM, matching AWS. **IMDSv2-only.** Every read requires a session token. A tokenless (v1-style) `GET` returns `401 Unauthorized` with an empty body. Obtain a token with a `PUT /latest/api/token` carrying `X-aws-ec2-metadata-token-ttl-seconds` (1–21600), then send it back in `X-aws-ec2-metadata-token` on every read. +The EC2 control plane reports this posture faithfully: `describe-instances` returns `MetadataOptions.HttpTokens=required`, and `run-instances`/`modify-instance-metadata-options` reject `--http-tokens optional` (and `--http-endpoint disabled`) with `UnsupportedOperation` — exactly as AWS does under account-level IMDSv2 enforcement. + ```bash # Inside the guest VM: TOKEN=$(curl -s -X PUT "http://169.254.169.254/latest/api/token" \ diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index f68a4cae0..30fc588b3 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -826,6 +826,7 @@ func (d *Daemon) subscribeAll() error { {"ec2.RevokeSecurityGroupIngress", d.handleEC2RevokeSecurityGroupIngress, "spinifex-workers"}, {"ec2.RevokeSecurityGroupEgress", d.handleEC2RevokeSecurityGroupEgress, "spinifex-workers"}, {"ec2.ModifyInstanceAttribute", d.handleEC2ModifyInstanceAttribute, "spinifex-workers"}, + {"ec2.ModifyInstanceMetadataOptions", d.handleEC2ModifyInstanceMetadataOptions, "spinifex-workers"}, {"ec2.start", d.handleEC2StartStoppedInstance, "spinifex-workers"}, {fmt.Sprintf("ec2.start.%s", d.node), d.handleEC2StartStoppedInstanceDirect, ""}, {"ec2.terminate", d.handleEC2TerminateStoppedInstance, "spinifex-workers"}, diff --git a/spinifex/daemon/daemon_handlers_instance.go b/spinifex/daemon/daemon_handlers_instance.go index fdf7ac44e..925f487fd 100644 --- a/spinifex/daemon/daemon_handlers_instance.go +++ b/spinifex/daemon/daemon_handlers_instance.go @@ -590,6 +590,10 @@ func (d *Daemon) handleEC2ModifyInstanceAttribute(msg *nats.Msg) { handleNATSRequest(msg, d.instanceService.ModifyInstanceAttribute) } +func (d *Daemon) handleEC2ModifyInstanceMetadataOptions(msg *nats.Msg) { + handleNATSRequest(msg, d.instanceService.ModifyInstanceMetadataOptions) +} + // handleEC2DescribeInstanceAttribute returns a single requested attribute for an instance. func (d *Daemon) handleEC2DescribeInstanceAttribute(msg *nats.Msg) { handleNATSRequest(msg, d.instanceService.DescribeInstanceAttribute) diff --git a/spinifex/gateway/ec2.go b/spinifex/gateway/ec2.go index da49c7f2e..da151c5cb 100644 --- a/spinifex/gateway/ec2.go +++ b/spinifex/gateway/ec2.go @@ -174,6 +174,9 @@ var ec2Actions = map[string]EC2Handler{ "DescribeInstanceAttribute": ec2Handler(func(input *ec2.DescribeInstanceAttributeInput, gw *GatewayConfig, accountID string) (any, error) { return gateway_ec2_instance.DescribeInstanceAttribute(input, gw.NATSConn, gw.DiscoverActiveNodes(), accountID) }), + "ModifyInstanceMetadataOptions": ec2Handler(func(input *ec2.ModifyInstanceMetadataOptionsInput, gw *GatewayConfig, accountID string) (any, error) { + return gateway_ec2_instance.ModifyInstanceMetadataOptions(input, gw.NATSConn, accountID) + }), "DescribeInstanceCreditSpecifications": ec2Handler(func(input *ec2.DescribeInstanceCreditSpecificationsInput, gw *GatewayConfig, accountID string) (any, error) { return gateway_ec2_instance.DescribeInstanceCreditSpecifications(input) }), diff --git a/spinifex/gateway/ec2/instance/ModifyInstanceMetadataOptions.go b/spinifex/gateway/ec2/instance/ModifyInstanceMetadataOptions.go new file mode 100644 index 000000000..27fbea9f2 --- /dev/null +++ b/spinifex/gateway/ec2/instance/ModifyInstanceMetadataOptions.go @@ -0,0 +1,26 @@ +package gateway_ec2_instance + +import ( + "errors" + "strings" + "time" + + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" + "github.com/mulgadc/spinifex/spinifex/utils" + "github.com/nats-io/nats.go" +) + +// ModifyInstanceMetadataOptions validates the instance ID and round-trips the +// request to the owning daemon, which enforces the IMDSv2-only posture (only the +// hop limit is mutable) and returns the updated block. +func ModifyInstanceMetadataOptions(input *ec2.ModifyInstanceMetadataOptionsInput, natsConn *nats.Conn, accountID string) (*ec2.ModifyInstanceMetadataOptionsOutput, error) { + if input == nil || input.InstanceId == nil || *input.InstanceId == "" { + return nil, errors.New(awserrors.ErrorInvalidInstanceIDMalformed) + } + if !strings.HasPrefix(*input.InstanceId, "i-") { + return nil, errors.New(awserrors.ErrorInvalidInstanceIDMalformed) + } + return utils.NATSRequest[ec2.ModifyInstanceMetadataOptionsOutput]( + natsConn, "ec2.ModifyInstanceMetadataOptions", input, 30*time.Second, accountID) +} diff --git a/spinifex/gateway/ec2/instance/ModifyInstanceMetadataOptions_test.go b/spinifex/gateway/ec2/instance/ModifyInstanceMetadataOptions_test.go new file mode 100644 index 000000000..36320b946 --- /dev/null +++ b/spinifex/gateway/ec2/instance/ModifyInstanceMetadataOptions_test.go @@ -0,0 +1,29 @@ +package gateway_ec2_instance + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// A nil, empty, or non-"i-" instance ID is rejected at the gateway before any +// NATS round-trip, so a nil connection is never dereferenced. +func TestModifyInstanceMetadataOptions_RejectsMalformedID(t *testing.T) { + cases := map[string]*ec2.ModifyInstanceMetadataOptionsInput{ + "nil input": nil, + "nil id": {}, + "empty id": {InstanceId: aws.String("")}, + "bad prefix": {InstanceId: aws.String("x-12345")}, + } + for name, in := range cases { + t.Run(name, func(t *testing.T) { + _, err := ModifyInstanceMetadataOptions(in, nil, "acct") + require.Error(t, err) + assert.Equal(t, awserrors.ErrorInvalidInstanceIDMalformed, err.Error()) + }) + } +} diff --git a/spinifex/handlers/ec2/instance/metadata_options.go b/spinifex/handlers/ec2/instance/metadata_options.go new file mode 100644 index 000000000..0ea3bea3e --- /dev/null +++ b/spinifex/handlers/ec2/instance/metadata_options.go @@ -0,0 +1,65 @@ +package handlers_ec2_instance + +import ( + "errors" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" +) + +// defaultMetadataHopLimit matches the AWS default applied when none is requested. +const defaultMetadataHopLimit = int64(1) + +// buildMetadataOptions returns the constant IMDSv2-only block stamped at launch. +// Every field but the hop limit is a platform invariant; hopLimit applies only in +// the AWS-valid 1-64 range, otherwise falling back to the default. +func buildMetadataOptions(hopLimit *int64) *ec2.InstanceMetadataOptionsResponse { + limit := defaultMetadataHopLimit + if hopLimit != nil && *hopLimit >= 1 && *hopLimit <= 64 { + limit = *hopLimit + } + return &ec2.InstanceMetadataOptionsResponse{ + State: aws.String(ec2.InstanceMetadataOptionsStateApplied), + HttpTokens: aws.String(ec2.HttpTokensStateRequired), + HttpEndpoint: aws.String(ec2.InstanceMetadataEndpointStateEnabled), + HttpProtocolIpv6: aws.String(ec2.InstanceMetadataProtocolStateDisabled), + InstanceMetadataTags: aws.String(ec2.InstanceMetadataTagsStateDisabled), + HttpPutResponseHopLimit: aws.Int64(limit), + } +} + +// validateMetadataOptions rejects any request that weakens the IMDSv2-only +// posture or enables an unmodelled feature. Empty values are "leave unchanged" +// no-ops; only the hop limit (AWS-valid 1-64) is mutable. Shared by RunInstances +// and ModifyInstanceMetadataOptions. +func validateMetadataOptions(httpTokens, httpEndpoint, ipv6, tags string, hopLimit *int64) error { + if httpTokens != "" && httpTokens != ec2.HttpTokensStateRequired { + return errors.New(awserrors.ErrorUnsupportedOperation) + } + if httpEndpoint != "" && httpEndpoint != ec2.InstanceMetadataEndpointStateEnabled { + return errors.New(awserrors.ErrorUnsupportedOperation) + } + if ipv6 != "" && ipv6 != ec2.InstanceMetadataProtocolStateDisabled { + return errors.New(awserrors.ErrorUnsupportedOperation) + } + if tags != "" && tags != ec2.InstanceMetadataTagsStateDisabled { + return errors.New(awserrors.ErrorUnsupportedOperation) + } + if hopLimit != nil && (*hopLimit < 1 || *hopLimit > 64) { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + return nil +} + +// applyMetadataOptions stamps the constant block onto a legacy nil-block instance +// or moves the mutable hop limit. Callers must guard a nil instance. +func applyMetadataOptions(instance *ec2.Instance, hopLimit *int64) { + if instance.MetadataOptions == nil { + instance.MetadataOptions = buildMetadataOptions(hopLimit) + return + } + if hopLimit != nil { + instance.MetadataOptions.HttpPutResponseHopLimit = hopLimit + } +} diff --git a/spinifex/handlers/ec2/instance/metadata_options_test.go b/spinifex/handlers/ec2/instance/metadata_options_test.go new file mode 100644 index 000000000..7a53583e6 --- /dev/null +++ b/spinifex/handlers/ec2/instance/metadata_options_test.go @@ -0,0 +1,259 @@ +package handlers_ec2_instance + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" + "github.com/mulgadc/spinifex/spinifex/utils" + "github.com/mulgadc/spinifex/spinifex/vm" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The shared validator passes the secure/default values (and AWS "leave +// unchanged" empties) as no-ops, rejects any posture downgrade or unmodelled +// feature with UnsupportedOperation, and bounds the hop limit to 1-64. +func TestValidateMetadataOptions(t *testing.T) { + cases := map[string]struct { + httpTokens, httpEndpoint, ipv6, tags string + hopLimit *int64 + wantCode string + }{ + "all omitted": {}, + "secure values": {ec2.HttpTokensStateRequired, ec2.InstanceMetadataEndpointStateEnabled, ec2.InstanceMetadataProtocolStateDisabled, ec2.InstanceMetadataTagsStateDisabled, aws.Int64(1), ""}, + "hop limit lower bound": {hopLimit: aws.Int64(1)}, + "hop limit upper bound": {hopLimit: aws.Int64(64)}, + "tokens optional": {httpTokens: ec2.HttpTokensStateOptional, wantCode: awserrors.ErrorUnsupportedOperation}, + "endpoint disabled": {httpEndpoint: ec2.InstanceMetadataEndpointStateDisabled, wantCode: awserrors.ErrorUnsupportedOperation}, + "ipv6 enabled": {ipv6: ec2.InstanceMetadataProtocolStateEnabled, wantCode: awserrors.ErrorUnsupportedOperation}, + "tags enabled": {tags: ec2.InstanceMetadataTagsStateEnabled, wantCode: awserrors.ErrorUnsupportedOperation}, + "hop limit zero": {hopLimit: aws.Int64(0), wantCode: awserrors.ErrorInvalidParameterValue}, + "hop limit over maximum": {hopLimit: aws.Int64(65), wantCode: awserrors.ErrorInvalidParameterValue}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + err := validateMetadataOptions(tc.httpTokens, tc.httpEndpoint, tc.ipv6, tc.tags, tc.hopLimit) + if tc.wantCode == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + assert.True(t, awserrors.IsErrorCode(err, tc.wantCode), "want %s, got %v", tc.wantCode, err) + }) + } +} + +// Re-enabling IMDSv1 is refused with UnsupportedOperation and the instance is +// left untouched — no partial application of a rejected request. +func TestModifyInstanceMetadataOptions_RejectV1(t *testing.T) { + owner := utils.GlobalAccountID + id := "i-imdsv1" + v := &vm.VM{ + ID: id, AccountID: owner, Status: vm.StateRunning, + Instance: &ec2.Instance{InstanceId: aws.String(id), MetadataOptions: buildMetadataOptions(nil)}, + } + svc := &InstanceServiceImpl{vmMgr: mgrWith(map[string]*vm.VM{id: v})} + + _, err := svc.ModifyInstanceMetadataOptions(&ec2.ModifyInstanceMetadataOptionsInput{ + InstanceId: aws.String(id), + HttpTokens: aws.String(ec2.HttpTokensStateOptional), + }, owner) + require.Error(t, err) + assert.True(t, awserrors.IsErrorCode(err, awserrors.ErrorUnsupportedOperation), "got %v", err) + assert.Equal(t, ec2.HttpTokensStateRequired, aws.StringValue(v.Instance.MetadataOptions.HttpTokens)) + assert.Equal(t, int64(1), aws.Int64Value(v.Instance.MetadataOptions.HttpPutResponseHopLimit)) +} + +// A hop-limit change on a running instance persists and is echoed back — AWS +// allows this in any state, so it must not return IncorrectInstanceState. The +// no-op http-tokens=required is accepted alongside it. +func TestModifyInstanceMetadataOptions_HopLimitRunning(t *testing.T) { + owner := utils.GlobalAccountID + id := "i-hop-run" + v := &vm.VM{ + ID: id, AccountID: owner, Status: vm.StateRunning, + Instance: &ec2.Instance{InstanceId: aws.String(id), MetadataOptions: buildMetadataOptions(nil)}, + } + svc := &InstanceServiceImpl{vmMgr: mgrWith(map[string]*vm.VM{id: v})} + + out, err := svc.ModifyInstanceMetadataOptions(&ec2.ModifyInstanceMetadataOptionsInput{ + InstanceId: aws.String(id), + HttpTokens: aws.String(ec2.HttpTokensStateRequired), + HttpPutResponseHopLimit: aws.Int64(2), + }, owner) + require.NoError(t, err) + require.NotNil(t, out.InstanceMetadataOptions) + assert.Equal(t, int64(2), aws.Int64Value(out.InstanceMetadataOptions.HttpPutResponseHopLimit)) + assert.Equal(t, ec2.HttpTokensStateRequired, aws.StringValue(out.InstanceMetadataOptions.HttpTokens)) + assert.Equal(t, int64(2), aws.Int64Value(v.Instance.MetadataOptions.HttpPutResponseHopLimit)) +} + +// The stopped-store fallback persists the hop-limit change too. +func TestModifyInstanceMetadataOptions_HopLimitStopped(t *testing.T) { + owner := utils.GlobalAccountID + id := "i-hop-stop" + stored := &vm.VM{ + ID: id, AccountID: owner, Status: vm.StateStopped, + Instance: &ec2.Instance{InstanceId: aws.String(id), MetadataOptions: buildMetadataOptions(nil)}, + } + store := &fakeStoppedStore{loadByID: map[string]*vm.VM{id: stored}} + svc := &InstanceServiceImpl{vmMgr: mgrWith(map[string]*vm.VM{}), stoppedStore: store} + + out, err := svc.ModifyInstanceMetadataOptions(&ec2.ModifyInstanceMetadataOptionsInput{ + InstanceId: aws.String(id), + HttpPutResponseHopLimit: aws.Int64(3), + }, owner) + require.NoError(t, err) + assert.Equal(t, int64(3), aws.Int64Value(out.InstanceMetadataOptions.HttpPutResponseHopLimit)) + require.Contains(t, store.wroteStopped, id) + assert.Equal(t, int64(3), aws.Int64Value(store.wroteStopped[id].Instance.MetadataOptions.HttpPutResponseHopLimit)) +} + +// An unknown instance is reported absent, not a server error. +func TestModifyInstanceMetadataOptions_NotFound(t *testing.T) { + svc := &InstanceServiceImpl{ + vmMgr: mgrWith(map[string]*vm.VM{}), + stoppedStore: &fakeStoppedStore{loadByID: map[string]*vm.VM{}}, + } + _, err := svc.ModifyInstanceMetadataOptions(&ec2.ModifyInstanceMetadataOptionsInput{ + InstanceId: aws.String("i-ghost"), + HttpPutResponseHopLimit: aws.Int64(2), + }, utils.GlobalAccountID) + require.Error(t, err) + assert.True(t, awserrors.IsErrorCode(err, awserrors.ErrorInvalidInstanceIDNotFound), "got %v", err) +} + +// A nil Instance (data-integrity case the sibling ModifyInstanceAttribute also +// guards) returns ServerInternal rather than panicking the daemon — on both the +// running and stopped paths. +func TestModifyInstanceMetadataOptions_NilInstance(t *testing.T) { + owner := utils.GlobalAccountID + + t.Run("running", func(t *testing.T) { + id := "i-nil-run" + v := &vm.VM{ID: id, AccountID: owner, Status: vm.StateRunning, Instance: nil} + svc := &InstanceServiceImpl{vmMgr: mgrWith(map[string]*vm.VM{id: v})} + _, err := svc.ModifyInstanceMetadataOptions(&ec2.ModifyInstanceMetadataOptionsInput{ + InstanceId: aws.String(id), HttpPutResponseHopLimit: aws.Int64(2), + }, owner) + require.Error(t, err) + assert.True(t, awserrors.IsErrorCode(err, awserrors.ErrorServerInternal), "got %v", err) + }) + + t.Run("stopped", func(t *testing.T) { + id := "i-nil-stop" + stored := &vm.VM{ID: id, AccountID: owner, Status: vm.StateStopped, Instance: nil} + store := &fakeStoppedStore{loadByID: map[string]*vm.VM{id: stored}} + svc := &InstanceServiceImpl{vmMgr: mgrWith(map[string]*vm.VM{}), stoppedStore: store} + _, err := svc.ModifyInstanceMetadataOptions(&ec2.ModifyInstanceMetadataOptionsInput{ + InstanceId: aws.String(id), HttpPutResponseHopLimit: aws.Int64(2), + }, owner) + require.Error(t, err) + assert.True(t, awserrors.IsErrorCode(err, awserrors.ErrorServerInternal), "got %v", err) + }) +} + +// A legacy instance launched before the constant block (nil MetadataOptions) is +// stamped with the full IMDSv2-only block on the first modify, not just the hop. +func TestModifyInstanceMetadataOptions_LegacyNilBlockStamped(t *testing.T) { + owner := utils.GlobalAccountID + id := "i-legacy" + v := &vm.VM{ + ID: id, AccountID: owner, Status: vm.StateRunning, + Instance: &ec2.Instance{InstanceId: aws.String(id)}, + } + svc := &InstanceServiceImpl{vmMgr: mgrWith(map[string]*vm.VM{id: v})} + + out, err := svc.ModifyInstanceMetadataOptions(&ec2.ModifyInstanceMetadataOptionsInput{ + InstanceId: aws.String(id), HttpPutResponseHopLimit: aws.Int64(2), + }, owner) + require.NoError(t, err) + require.NotNil(t, out.InstanceMetadataOptions) + assert.Equal(t, ec2.HttpTokensStateRequired, aws.StringValue(out.InstanceMetadataOptions.HttpTokens)) + assert.Equal(t, int64(2), aws.Int64Value(out.InstanceMetadataOptions.HttpPutResponseHopLimit)) + require.NotNil(t, v.Instance.MetadataOptions, "the legacy instance must be stamped in place") +} + +// The constant block reports the IMDSv2-only posture: required tokens, endpoint +// enabled, IPv6/tags metadata disabled, state applied. Only the hop limit moves. +func TestBuildMetadataOptions_ConstantBlock(t *testing.T) { + opts := buildMetadataOptions(nil) + require.NotNil(t, opts) + assert.Equal(t, ec2.InstanceMetadataOptionsStateApplied, aws.StringValue(opts.State)) + assert.Equal(t, ec2.HttpTokensStateRequired, aws.StringValue(opts.HttpTokens)) + assert.Equal(t, ec2.InstanceMetadataEndpointStateEnabled, aws.StringValue(opts.HttpEndpoint)) + assert.Equal(t, ec2.InstanceMetadataProtocolStateDisabled, aws.StringValue(opts.HttpProtocolIpv6)) + assert.Equal(t, ec2.InstanceMetadataTagsStateDisabled, aws.StringValue(opts.InstanceMetadataTags)) + assert.Equal(t, int64(1), aws.Int64Value(opts.HttpPutResponseHopLimit), "nil hop limit defaults to 1") +} + +func TestBuildMetadataOptions_HopLimit(t *testing.T) { + cases := map[string]struct { + in *int64 + want int64 + }{ + "in-range 2": {aws.Int64(2), 2}, + "upper bound": {aws.Int64(64), 64}, + "lower bound": {aws.Int64(1), 1}, + "zero clamps": {aws.Int64(0), 1}, + "over clamps": {aws.Int64(65), 1}, + "nil defaults": {nil, 1}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.want, aws.Int64Value(buildMetadataOptions(tc.in).HttpPutResponseHopLimit)) + }) + } +} + +// Every instance launched after this change carries the constant block so +// DescribeInstances surfaces the IMDSv2-only posture without a projection change. +func TestRunInstance_MetadataOptionsSeeded(t *testing.T) { + svc := &InstanceServiceImpl{ + instanceTypes: map[string]*ec2.InstanceTypeInfo{"t3.micro": {InstanceType: aws.String("t3.micro")}}, + } + + _, ec2Instance, err := svc.RunInstance(&ec2.RunInstancesInput{ + ImageId: aws.String("ami-0abcdef1234567890"), + InstanceType: aws.String("t3.micro"), + }) + require.NoError(t, err) + require.NotNil(t, ec2Instance.MetadataOptions, "launch must stamp the metadata-options block") + assert.Equal(t, ec2.HttpTokensStateRequired, aws.StringValue(ec2Instance.MetadataOptions.HttpTokens)) + assert.Equal(t, int64(1), aws.Int64Value(ec2Instance.MetadataOptions.HttpPutResponseHopLimit)) +} + +// A run-instances launch that tries to re-enable IMDSv1 is rejected before any +// capacity is allocated — the same UnsupportedOperation the modify path returns. +// Validation precedes the instance-type lookup, so a bare service suffices. +func TestPrepareRunInstances_RejectV1MetadataOptions(t *testing.T) { + svc := &InstanceServiceImpl{} + _, _, _, err := svc.PrepareRunInstances(&ec2.RunInstancesInput{ + ImageId: aws.String("ami-0abcdef1234567890"), + InstanceType: aws.String("t3.micro"), + MinCount: aws.Int64(1), + MaxCount: aws.Int64(1), + MetadataOptions: &ec2.InstanceMetadataOptionsRequest{HttpTokens: aws.String(ec2.HttpTokensStateOptional)}, + }, utils.GlobalAccountID, "") + require.Error(t, err) + assert.True(t, awserrors.IsErrorCode(err, awserrors.ErrorUnsupportedOperation), "got %v", err) +} + +// A requested hop limit is reflected on the instance; the rest stay invariant. +func TestRunInstance_MetadataOptionsHopLimitFromRequest(t *testing.T) { + svc := &InstanceServiceImpl{ + instanceTypes: map[string]*ec2.InstanceTypeInfo{"t3.micro": {InstanceType: aws.String("t3.micro")}}, + } + + _, ec2Instance, err := svc.RunInstance(&ec2.RunInstancesInput{ + ImageId: aws.String("ami-0abcdef1234567890"), + InstanceType: aws.String("t3.micro"), + MetadataOptions: &ec2.InstanceMetadataOptionsRequest{HttpPutResponseHopLimit: aws.Int64(2)}, + }) + require.NoError(t, err) + require.NotNil(t, ec2Instance.MetadataOptions) + assert.Equal(t, int64(2), aws.Int64Value(ec2Instance.MetadataOptions.HttpPutResponseHopLimit)) + assert.Equal(t, ec2.HttpTokensStateRequired, aws.StringValue(ec2Instance.MetadataOptions.HttpTokens)) +} diff --git a/spinifex/handlers/ec2/instance/service.go b/spinifex/handlers/ec2/instance/service.go index 35f49fbb7..060f5acd9 100644 --- a/spinifex/handlers/ec2/instance/service.go +++ b/spinifex/handlers/ec2/instance/service.go @@ -17,6 +17,7 @@ type InstanceService interface { DescribeStoppedInstances(input *ec2.DescribeInstancesInput, accountID string) (*ec2.DescribeInstancesOutput, error) DescribeTerminatedInstances(input *ec2.DescribeInstancesInput, accountID string) (*ec2.DescribeInstancesOutput, error) ModifyInstanceAttribute(input *ec2.ModifyInstanceAttributeInput, accountID string) (*ec2.ModifyInstanceAttributeOutput, error) + ModifyInstanceMetadataOptions(input *ec2.ModifyInstanceMetadataOptionsInput, accountID string) (*ec2.ModifyInstanceMetadataOptionsOutput, error) StartStoppedInstance(input *StartStoppedInstanceInput, accountID string) (*StartStoppedInstanceOutput, error) TerminateStoppedInstance(input *TerminateStoppedInstanceInput, accountID string) (*TerminateStoppedInstanceOutput, error) } diff --git a/spinifex/handlers/ec2/instance/service_impl.go b/spinifex/handlers/ec2/instance/service_impl.go index 178c6d0db..46b55f5c7 100644 --- a/spinifex/handlers/ec2/instance/service_impl.go +++ b/spinifex/handlers/ec2/instance/service_impl.go @@ -113,6 +113,8 @@ func parseVolumeParams(input *ec2.RunInstancesInput) volumeParams { return p } +var _ InstanceService = (*InstanceServiceImpl)(nil) + // InstanceServiceImpl handles daemon-side EC2 instance operations type InstanceServiceImpl struct { config *config.Config @@ -218,6 +220,14 @@ func (s *InstanceServiceImpl) RunInstance(input *ec2.RunInstancesInput) (*vm.VM, ec2Instance.SetArchitecture(arch) } + // Stamp the constant IMDSv2-only block so DescribeInstances reports the + // posture; only the hop limit is request-driven. + var hopLimit *int64 + if input.MetadataOptions != nil { + hopLimit = input.MetadataOptions.HttpPutResponseHopLimit + } + ec2Instance.MetadataOptions = buildMetadataOptions(hopLimit) + // IAM instance profile attached at launch: gateway has already resolved // the reference to a canonical ARN and enforced iam:PassRole; here we // just persist it on the VM and generate the association ID. Id is left @@ -282,6 +292,21 @@ func (s *InstanceServiceImpl) PrepareRunInstances(input *ec2.RunInstancesInput, return nil, nil, nil, errors.New(awserrors.ErrorMissingParameter) } + // Reject a metadata-options downgrade before allocating capacity: http-tokens= + // optional and other weakenings return UnsupportedOperation under IMDSv2 + // enforcement. RunInstance seeds the hop limit onto the per-instance block. + if opts := input.MetadataOptions; opts != nil { + if err := validateMetadataOptions( + aws.StringValue(opts.HttpTokens), + aws.StringValue(opts.HttpEndpoint), + aws.StringValue(opts.HttpProtocolIpv6), + aws.StringValue(opts.InstanceMetadataTags), + opts.HttpPutResponseHopLimit, + ); err != nil { + return nil, nil, nil, err + } + } + instanceType, exists := s.instanceTypes[*input.InstanceType] if !exists { slog.Error("PrepareRunInstances: invalid instance type", "InstanceType", *input.InstanceType) @@ -1886,6 +1911,94 @@ func (s *InstanceServiceImpl) ModifyInstanceAttribute(input *ec2.ModifyInstanceA return &ec2.ModifyInstanceAttributeOutput{}, nil } +// ModifyInstanceMetadataOptions applies a metadata-options change; only the hop +// limit is mutable. It mirrors DisableApiTermination's running-first/stopped path +// — no stopped-only guard, since AWS permits this on running instances. +func (s *InstanceServiceImpl) ModifyInstanceMetadataOptions(input *ec2.ModifyInstanceMetadataOptionsInput, accountID string) (*ec2.ModifyInstanceMetadataOptionsOutput, error) { + if input.InstanceId == nil || *input.InstanceId == "" { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + instanceID := *input.InstanceId + + if err := validateMetadataOptions( + aws.StringValue(input.HttpTokens), + aws.StringValue(input.HttpEndpoint), + aws.StringValue(input.HttpProtocolIpv6), + aws.StringValue(input.InstanceMetadataTags), + input.HttpPutResponseHopLimit, + ); err != nil { + return nil, err + } + + // Running-first: mutate the live instance's block, falling through to the + // stopped store only when the instance isn't running on this node. + var notVisible, integrityErr bool + var options *ec2.InstanceMetadataOptionsResponse + updated, persistErr := s.vmMgr.UpdateAndPersist(instanceID, func(v *vm.VM) bool { + if !IsInstanceVisible(accountID, v.AccountID) { + notVisible = true + return false + } + if v.Instance == nil { + integrityErr = true + return false + } + applyMetadataOptions(v.Instance, input.HttpPutResponseHopLimit) + opt := *v.Instance.MetadataOptions + options = &opt + return true + }) + if notVisible { + slog.Warn("ModifyInstanceMetadataOptions: instance not visible", + "instanceId", instanceID, "callerAccount", accountID) + return nil, errors.New(awserrors.ErrorInvalidInstanceIDNotFound) + } + if integrityErr { + slog.Error("ModifyInstanceMetadataOptions: instance.Instance is nil, data integrity issue", "instanceId", instanceID) + return nil, errors.New(awserrors.ErrorServerInternal) + } + if updated { + if persistErr != nil { + slog.Error("ModifyInstanceMetadataOptions: persist failed", "instanceId", instanceID, "err", persistErr) + return nil, errors.New(awserrors.ErrorServerInternal) + } + slog.Info("ModifyInstanceMetadataOptions: updated running instance", "instanceId", instanceID) + return &ec2.ModifyInstanceMetadataOptionsOutput{InstanceId: aws.String(instanceID), InstanceMetadataOptions: options}, nil + } + + if s.stoppedStore == nil { + slog.Error("ModifyInstanceMetadataOptions: stopped store not available") + return nil, errors.New(awserrors.ErrorServerInternal) + } + + instance, err := s.stoppedStore.LoadStoppedInstance(instanceID) + if err != nil { + slog.Error("ModifyInstanceMetadataOptions: failed to load stopped instance", "instanceId", instanceID, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + if instance == nil { + slog.Warn("ModifyInstanceMetadataOptions: instance not found", "instanceId", instanceID) + return nil, errors.New(awserrors.ErrorInvalidInstanceIDNotFound) + } + if !IsInstanceVisible(accountID, instance.AccountID) { + slog.Warn("ModifyInstanceMetadataOptions: instance not visible", + "instanceId", instanceID, "callerAccount", accountID, "ownerAccount", instance.AccountID) + return nil, errors.New(awserrors.ErrorInvalidInstanceIDNotFound) + } + if instance.Instance == nil { + slog.Error("ModifyInstanceMetadataOptions: instance.Instance is nil, data integrity issue", "instanceId", instanceID) + return nil, errors.New(awserrors.ErrorServerInternal) + } + + applyMetadataOptions(instance.Instance, input.HttpPutResponseHopLimit) + if err := s.stoppedStore.WriteStoppedInstance(instanceID, instance); err != nil { + slog.Error("ModifyInstanceMetadataOptions: failed to persist stopped instance", "instanceId", instanceID, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + slog.Info("ModifyInstanceMetadataOptions: updated stopped instance", "instanceId", instanceID) + return &ec2.ModifyInstanceMetadataOptionsOutput{InstanceId: aws.String(instanceID), InstanceMetadataOptions: instance.Instance.MetadataOptions}, nil +} + // StoppedInstanceNode returns the node that last hosted the stopped instance, // used to route start requests back to the original node when possible. func (s *InstanceServiceImpl) StoppedInstanceNode(instanceID string) string { diff --git a/spinifex/handlers/ec2/instance/service_mock.go b/spinifex/handlers/ec2/instance/service_mock.go index 7e002f6a5..134bc9816 100644 --- a/spinifex/handlers/ec2/instance/service_mock.go +++ b/spinifex/handlers/ec2/instance/service_mock.go @@ -64,6 +64,10 @@ func (s *MockInstanceService) ModifyInstanceAttribute(_ *ec2.ModifyInstanceAttri return &ec2.ModifyInstanceAttributeOutput{}, nil } +func (s *MockInstanceService) ModifyInstanceMetadataOptions(input *ec2.ModifyInstanceMetadataOptionsInput, _ string) (*ec2.ModifyInstanceMetadataOptionsOutput, error) { + return &ec2.ModifyInstanceMetadataOptionsOutput{InstanceId: input.InstanceId}, nil +} + func (s *MockInstanceService) StartStoppedInstance(input *StartStoppedInstanceInput, _ string) (*StartStoppedInstanceOutput, error) { return &StartStoppedInstanceOutput{Status: "running", InstanceID: input.InstanceID}, nil } diff --git a/spinifex/handlers/ec2/instance/service_nats.go b/spinifex/handlers/ec2/instance/service_nats.go index 919146f10..e986db7d0 100644 --- a/spinifex/handlers/ec2/instance/service_nats.go +++ b/spinifex/handlers/ec2/instance/service_nats.go @@ -66,6 +66,10 @@ func (s *NATSInstanceService) ModifyInstanceAttribute(input *ec2.ModifyInstanceA return utils.NATSRequest[ec2.ModifyInstanceAttributeOutput](s.natsConn, "ec2.ModifyInstanceAttribute", input, 30*time.Second, accountID) } +func (s *NATSInstanceService) ModifyInstanceMetadataOptions(input *ec2.ModifyInstanceMetadataOptionsInput, accountID string) (*ec2.ModifyInstanceMetadataOptionsOutput, error) { + return utils.NATSRequest[ec2.ModifyInstanceMetadataOptionsOutput](s.natsConn, "ec2.ModifyInstanceMetadataOptions", input, 30*time.Second, accountID) +} + func (s *NATSInstanceService) StartStoppedInstance(input *StartStoppedInstanceInput, accountID string) (*StartStoppedInstanceOutput, error) { return utils.NATSRequest[StartStoppedInstanceOutput](s.natsConn, "ec2.StartStoppedInstance", input, 30*time.Second, accountID) }