fix: role last-used tracking, and record S3 requests in last-used metadata

Role last-used tracking was missing entirely - `GetRole` returned a `RoleLastUsed` element that nothing ever wrote, rendering the zero time instead of the empty element AWS returns for an unused role - and access key last-used only ever saw the `IAM`/`STS` control plane, so a credential used exclusively against the S3 gateway reported as never used. Roles now record a use whenever a request authenticates with one of their session credentials, through a new `Storer.RecordRoleUsage` mirroring `RecordAccessKeyUsage`, gated on the session's role still being the one it was minted against so a session outliving its role can't attribute its use to a same-named replacement. `LastUsedDate` became a `*time.Time` so an unused role renders as an empty element. Both records now cover the S3 data plane as well: the gateway sends its configured region and `s3` on evaluate-policy and the IAM service records the caller there, so `GetAccessKeyLastUsed's` `ServiceName` is now iam, sts or s3. That call was chosen over derive-signing-key, which runs before signature verification and takes its region and service from the caller's own `Authorization` header - recording there would let anyone who knows an access key id refresh and poison another identity's audit record. Requests denied by a bucket policy or made against a public bucket are not recorded, since neither reaches identity-policy evaluation. To keep per-request recording affordable, an update is skipped while the stored record has the same service and region and is under a minute old; a change of either is written through immediately. Assuming a role is not a use, a request denied by an identity policy is, and both successful and denied S3 requests update the record. Also moves the `OIDC-dependent` tests into the `s3-iam-session` group so runoidctests.sh runs a single group.
2026-09-24 00:44:23 +00:00 · 2026-09-01 19:55:27 +04:00
parent 7a1a3e4775
commit afbee5be01
23 changed files with 1235 additions and 311 deletions
@@ -17,8 +17,10 @@ import (
 	"encoding/json"
 	"maps"
 	"strings"
+	"time"

 	"github.com/gofiber/fiber/v3"
+	"github.com/versity/versitygw/debuglogger"
 	"github.com/versity/versitygw/iamapi/internal/iammiddleware"
 	"github.com/versity/versitygw/iamapi/policy"
 	"github.com/versity/versitygw/iamapi/types"
@@ -55,6 +57,42 @@ func (p *PrivateAPI) handleDeriveSigningKey(ctx fiber.Ctx) error {
 	return ctx.JSON(DeriveSigningKeyResponse{DerivedKey: derivedKey})
 }

+// recordDataPlaneUsage records this S3 request as a use of the credential
+// that made it — an access key's GetAccessKeyLastUsed metadata, a session's
+// role's RoleLastUsed, or both for a session (its role is what AWS reports,
+// and a session has no long-term key of its own). It is the data-plane
+// counterpart of what iammiddleware.VerifyIAMAuth records for the IAM/STS
+// control plane, and is deliberately here rather than on derive-signing-key:
+// this endpoint is only reached once the gateway has verified the request's
+// signature, so an unauthenticated caller who merely knows an access key id
+// cannot refresh — or, since it would supply the credential scope, poison —
+// another identity's last-used record.
+//
+// Everything about it is best-effort: failures are logged and dropped, and a
+// gateway too old to send Region/Service records nothing at all rather than
+// storing a blank service or region.
+func (p *PrivateAPI) recordDataPlaneUsage(ctx fiber.Ctx, identity types.Identity, req EvaluatePolicyRequest) {
+	if req.Region == "" || req.Service == "" {
+		return
+	}
+
+	now := time.Now().UTC()
+	if identity.User != nil {
+		if err := p.store.RecordAccessKeyUsage(ctx.Context(), req.AccessKeyID, req.Service, req.Region, now); err != nil {
+			debuglogger.Logf("failed to record access key last-used metadata for %q: %v", req.AccessKeyID, err)
+		}
+	}
+	// identity.Role is set only when the session's role still exists and is
+	// still the one the session was minted against, so a session outliving
+	// its role records nothing rather than attributing its use to a
+	// same-named replacement — same rule as the control plane.
+	if identity.Role != nil {
+		if err := p.store.RecordRoleUsage(ctx.Context(), identity.Role.RoleName, req.Region, now); err != nil {
+			debuglogger.Logf("failed to record role last-used metadata for %q: %v", identity.Role.RoleName, err)
+		}
+	}
+}
+
 // handleResolveIdentity answers "does this access key exist, and what
 // principal is it" for a batch of access key ids, returning no credential
 // material at all — see ResolveIdentityResponse for why that is what makes
@@ -101,6 +139,8 @@ func (p *PrivateAPI) handleEvaluatePolicy(ctx fiber.Ctx) error {
 		return mapResolveError(err)
 	}

+	p.recordDataPlaneUsage(ctx, *identity, req)
+
 	condition := conditionContextFor(*identity, req.Condition)

 	decisions := make([][]string, len(req.Resources))
@@ -812,6 +812,158 @@ func TestParseProtocolVersion(t *testing.T) {
 	}
 }

+// TestEvaluatePolicyRecordsDataPlaneUsage covers the S3 side of last-used
+// tracking: authorizing a data-plane request records it against the
+// credential that made it — a user's access key (with the "s3" service name
+// its control-plane counterpart could never produce) and, for a session, the
+// role it assumed.
+func TestEvaluatePolicyRecordsDataPlaneUsage(t *testing.T) {
+	p, store := newTestServer(t)
+	ctx := context.Background()
+	const allowGet = `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:GetObject","Resource":"*"}]}`
+
+	createTestUser(t, store, "nina", "AKIDNINA", "ninasecret", allowGet)
+	role := createTestRole(t, store, "reader", allowGet)
+	session := createTestSessionForRole(t, store, role, "ASIASESSION1", "sessionsecret", "sessiontoken", "")
+
+	before := time.Now().UTC().Add(-time.Second)
+
+	evaluateAs(t, p, EvaluatePolicyRequest{
+		AccessKeyID: "AKIDNINA",
+		Actions:     []string{"s3:GetObject"},
+		Resources:   []string{"arn:aws:s3:::bucket/key"},
+		Region:      "us-west-2",
+		Service:     "s3",
+	})
+
+	lastUsed, err := store.GetAccessKeyLastUsed(ctx, "AKIDNINA")
+	if err != nil {
+		t.Fatalf("GetAccessKeyLastUsed: %v", err)
+	}
+	if lastUsed.ServiceName != "s3" || lastUsed.Region != "us-west-2" {
+		t.Fatalf("access key last used = %s/%s, want s3/us-west-2", lastUsed.ServiceName, lastUsed.Region)
+	}
+	if lastUsed.LastUsedDate.Before(before) {
+		t.Fatalf("access key LastUsedDate = %v, want at or after %v", lastUsed.LastUsedDate, before)
+	}
+
+	evaluateAs(t, p, EvaluatePolicyRequest{
+		AccessKeyID:  session.AccessKeyId,
+		SessionToken: session.SessionToken,
+		Actions:      []string{"s3:GetObject"},
+		Resources:    []string{"arn:aws:s3:::bucket/key"},
+		Region:       "us-west-2",
+		Service:      "s3",
+	})
+
+	stored, err := store.GetRole(ctx, "reader")
+	if err != nil {
+		t.Fatalf("GetRole: %v", err)
+	}
+	if stored.RoleLastUsed == nil || stored.RoleLastUsed.LastUsedDate == nil {
+		t.Fatalf("RoleLastUsed = %#v, want a recorded date", stored.RoleLastUsed)
+	}
+	if stored.RoleLastUsed.Region != "us-west-2" {
+		t.Fatalf("RoleLastUsed.Region = %q, want us-west-2", stored.RoleLastUsed.Region)
+	}
+	if stored.RoleLastUsed.LastUsedDate.Before(before) {
+		t.Fatalf("RoleLastUsed.LastUsedDate = %v, want at or after %v", stored.RoleLastUsed.LastUsedDate, before)
+	}
+}
+
+// TestEvaluatePolicyWithoutRegionRecordsNothing covers a gateway too old to
+// send Region/Service: the request must still authorize normally, and must
+// not store a blank service or region as if it were real data.
+func TestEvaluatePolicyWithoutRegionRecordsNothing(t *testing.T) {
+	p, store := newTestServer(t)
+	createTestUser(t, store, "nina", "AKIDNINA", "ninasecret",
+		`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:GetObject","Resource":"*"}]}`)
+
+	out := evaluateAs(t, p, EvaluatePolicyRequest{
+		AccessKeyID: "AKIDNINA",
+		Actions:     []string{"s3:GetObject"},
+		Resources:   []string{"arn:aws:s3:::bucket/key"},
+	})
+	if len(out.Decisions) != 1 || out.Decisions[0][0] != DecisionAllow {
+		t.Fatalf("Decisions = %v, want [[allow]] — authorization must not depend on the reporting fields", out.Decisions)
+	}
+
+	lastUsed, err := store.GetAccessKeyLastUsed(context.Background(), "AKIDNINA")
+	if err != nil {
+		t.Fatalf("GetAccessKeyLastUsed: %v", err)
+	}
+	if !lastUsed.LastUsedDate.IsZero() || lastUsed.ServiceName != "" || lastUsed.Region != "" {
+		t.Fatalf("last used = %#v, want nothing recorded", lastUsed)
+	}
+}
+
+// TestEvaluatePolicyCoalescesRepeatedUsage confirms the write-coalescing
+// that makes per-request recording affordable: a second request in the same
+// region and service leaves the stored timestamp alone, while a request from
+// a different region is written through immediately, since that is the part
+// an operator reads to see what a credential is being used for.
+func TestEvaluatePolicyCoalescesRepeatedUsage(t *testing.T) {
+	p, store := newTestServer(t)
+	ctx := context.Background()
+	createTestUser(t, store, "nina", "AKIDNINA", "ninasecret",
+		`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:GetObject","Resource":"*"}]}`)
+
+	req := EvaluatePolicyRequest{
+		AccessKeyID: "AKIDNINA",
+		Actions:     []string{"s3:GetObject"},
+		Resources:   []string{"arn:aws:s3:::bucket/key"},
+		Region:      "us-west-2",
+		Service:     "s3",
+	}
+
+	evaluateAs(t, p, req)
+	first, err := store.GetAccessKeyLastUsed(ctx, "AKIDNINA")
+	if err != nil {
+		t.Fatalf("GetAccessKeyLastUsed: %v", err)
+	}
+
+	evaluateAs(t, p, req)
+	second, err := store.GetAccessKeyLastUsed(ctx, "AKIDNINA")
+	if err != nil {
+		t.Fatalf("GetAccessKeyLastUsed: %v", err)
+	}
+	if !second.LastUsedDate.Equal(first.LastUsedDate) {
+		t.Fatalf("LastUsedDate = %v after a repeat request, want it coalesced to %v", second.LastUsedDate, first.LastUsedDate)
+	}
+
+	req.Region = "eu-west-1"
+	evaluateAs(t, p, req)
+	moved, err := store.GetAccessKeyLastUsed(ctx, "AKIDNINA")
+	if err != nil {
+		t.Fatalf("GetAccessKeyLastUsed: %v", err)
+	}
+	if moved.Region != "eu-west-1" || !moved.LastUsedDate.After(first.LastUsedDate) {
+		t.Fatalf("last used = %s at %v, want eu-west-1 written through after %v", moved.Region, moved.LastUsedDate, first.LastUsedDate)
+	}
+}
+
+// evaluateAs posts one evaluate-policy request as root and returns the
+// decoded response, failing the test on any non-200.
+func evaluateAs(t *testing.T, p *PrivateAPI, req EvaluatePolicyRequest) EvaluatePolicyResponse {
+	t.Helper()
+
+	body, err := json.Marshal(req)
+	if err != nil {
+		t.Fatalf("marshal request: %v", err)
+	}
+	resp := doPrivateRequest(t, p, http.MethodPost, EvaluatePath, testRoot.Access, testRoot.Secret, body)
+	raw := readBody(t, resp)
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("evaluate-policy status = %d, body=%s", resp.StatusCode, raw)
+	}
+
+	var out EvaluatePolicyResponse
+	if err := json.Unmarshal([]byte(raw), &out); err != nil {
+		t.Fatalf("unmarshal %s: %v", raw, err)
+	}
+	return out
+}
+
 // createTestRole creates a role with an optional inline permission policy
 // directly against store, the same way createTestUser bypasses the
 // control-plane API. Arn and RoleID are set explicitly because
@@ -35,13 +35,24 @@ type DeriveSigningKeyResponse struct {
 	DerivedKey []byte `json:"derivedKey"`
 }

-// EvaluatePolicyRequest is the evaluate-policy request body
+// EvaluatePolicyRequest is the evaluate-policy request body.
+//
+// Region and Service describe the data-plane request being authorized, and
+// exist only so the identity's last-used metadata (GetAccessKeyLastUsed,
+// GetRole's RoleLastUsed) can record S3 use. They are recorded rather than
+// evaluated — nothing about the authorization decision depends on them —
+// and they are the gateway's own configured region and a fixed "s3", never
+// anything a client supplied: the credential scope in a request's
+// Authorization header is attacker-controlled until the signature is
+// verified, and this endpoint is the first one reached after that.
 type EvaluatePolicyRequest struct {
 	AccessKeyID  string              `json:"accessKeyId"`
 	SessionToken string              `json:"sessionToken,omitempty"`
 	Actions      []string            `json:"actions"`
 	Resources    []string            `json:"resources"`
 	Condition    map[string][]string `json:"condition,omitempty"`
+	Region       string              `json:"region,omitempty"`
+	Service      string              `json:"service,omitempty"`
 }

 // ResolveIdentityRequest asks whether each access key id exists and what