fix(secrev): apply IAM cross-review FIXes
GPT-4.1 IAM cross-review 2026-06-18: APPROVE, no BLOCKs. Applied FIXes: - trust policy: add aws:SourceAccount=328440206208 (confused-deputy guard) alongside the existing aws:SourceArn trust-anchor pin - readonly policy: remove ec2:DescribeImages (data minimization — AMIs are not an idle-spend signal) - aws:RequestedRegion NIT: deliberately SKIPPED — ce:* and s3:ListAllMyBuckets are global-endpoint services a blanket region condition could DENY; rationale recorded in aws-posture-readonly-policy.rationale.md - rationale.md + CROSS-REVIEW-PACKET.md: record APPROVE + FIXes + NIT answers (snapshots=account-owned idle signal; s3 list=names-only; no logs:* needed)
This commit is contained in:
parent
77302a1ebb
commit
acdba9a4d4
4 changed files with 65 additions and 12 deletions
|
|
@ -1,10 +1,17 @@
|
|||
# Cross-review packet — R720 aws-posture IAM (step-ca → Roles Anywhere → read-only AWS role)
|
||||
|
||||
> **GPT-4.1 cross-review 2026-06-18: APPROVE, no BLOCKs; FIXes applied**
|
||||
> (`aws:SourceAccount` added to the trust policy; `ec2:DescribeImages` removed from the
|
||||
> permission policy). NIT answers recorded in `aws-posture-readonly-policy.rationale.md`:
|
||||
> snapshots = account-owned idle-spend signal (kept); `s3:ListAllMyBuckets` = names-only
|
||||
> inventory, no object data (kept); no `logs:*` needed; `aws:RequestedRegion` deliberately
|
||||
> SKIPPED (global-endpoint `ce:*`/`s3:ListAllMyBuckets` could be DENYed by a blanket region pin).
|
||||
|
||||
**Audience:** the mandatory GPT-4.1 IAM cross-review + Adam.
|
||||
**Status:** these are AUTHORED FILES, nothing is applied to AWS. aws-posture (the checker that
|
||||
*uses* this role) is **hard-gated behind this review** (design `docs/r720-agent-team-design.md`
|
||||
§7, B3) and is NOT built in this change. Approving this packet unblocks provisioning + building
|
||||
aws-posture; it does not itself change AWS.
|
||||
**Status:** these are AUTHORED FILES, nothing is applied to AWS. The review has now PASSED
|
||||
(APPROVE, no BLOCKs), which unblocks **building** aws-posture (done in this Phase-3 change set,
|
||||
PROVISIONING-GATED — the checker makes no AWS call until step-ca + Roles Anywhere are stood up).
|
||||
Approving this packet unblocks provisioning; it does not itself change AWS.
|
||||
|
||||
**Account:** 328440206208 · **Region:** us-east-1 · **Box:** the always-on R720 secrev VM
|
||||
(single-user, unattended, currently holds a long-lived read-only GitHub PAT).
|
||||
|
|
@ -52,6 +59,7 @@ sts:AssumeRole on role/r720-aws-posture-readonly
|
|||
│ trust policy (aws-posture-trust-policy.json) requires ALL of:
|
||||
│ (1) Principal = rolesanywhere.amazonaws.com (came via Roles Anywhere)
|
||||
│ (2) aws:SourceArn = THIS trust anchor (not some other anchor)
|
||||
│ (2b) aws:SourceAccount = 328440206208 (confused-deputy guard, added in cross-review)
|
||||
│ (3) x509Subject/CN = "r720-aws-posture" AND x509Issuer/CN = the internal CA
|
||||
▼
|
||||
1-hour STS session, permissions = aws-posture-readonly-policy.json (read-only cost + idle inventory)
|
||||
|
|
@ -61,8 +69,8 @@ read-only AWS APIs: ce:Get*, cloudwatch:GetMetric*/DescribeAlarms, ec2/elb/rds:D
|
|||
```
|
||||
|
||||
Three independent conditions must ALL hold to assume the role: via Roles Anywhere, from THIS
|
||||
anchor, presenting a leaf with the pinned subject CN + issuer CN. Any one missing → AssumeRole
|
||||
denied.
|
||||
anchor (in THIS account, via the `aws:SourceAccount` guard added in cross-review), presenting a
|
||||
leaf with the pinned subject CN + issuer CN. Any one missing → AssumeRole denied.
|
||||
|
||||
## Least-privilege rationale (summary; full table in the rationale .md)
|
||||
|
||||
|
|
@ -137,21 +145,34 @@ snapshot-restore away from gone.
|
|||
> gone. Paste the transcript here. Until this is filled in, the rollback is "written, not
|
||||
> exercised" and the phase is NOT accepted (design §7).
|
||||
|
||||
## Specific things for the cross-reviewer to scrutinize
|
||||
## Specific things for the cross-reviewer to scrutinize (with resolutions)
|
||||
|
||||
1. **Trust-policy condition completeness.** Are `aws:PrincipalTag/x509Subject/CN` +
|
||||
`aws:PrincipalTag/x509Issuer/CN` + `ArnEquals aws:SourceArn` (the trust anchor) sufficient
|
||||
to prevent any other cert (or another trust anchor in the account) from assuming the role?
|
||||
Is there a confused-deputy gap I should also pin (e.g. should I add `aws:SourceAccount`)?
|
||||
→ **RESOLVED (FIX applied):** added `aws:SourceAccount = 328440206208` to the StringEquals
|
||||
condition. The trust now pins anchor (SourceArn) **and** account (SourceAccount) plus the
|
||||
cert CN/issuer — closing the confused-deputy gap the reviewer raised.
|
||||
2. **`Resource: "*"` statements.** Confirm each is an API that genuinely has no resource-level
|
||||
support, and that no statement could be tightened with a condition (e.g. `aws:RequestedRegion`
|
||||
= us-east-1) without breaking the checker.
|
||||
→ **RESOLVED (NIT, SKIPPED with rationale):** every `Resource:"*"` statement is an
|
||||
API family without resource-level ARNs (ce, the cloudwatch metric-data calls, ec2/elb/rds
|
||||
`Describe*`). `aws:RequestedRegion` is **NOT** applied — `ce:*` and `s3:ListAllMyBuckets` are
|
||||
global-endpoint services that a blanket region condition could DENY. Full reasoning in
|
||||
`aws-posture-readonly-policy.rationale.md` ("Cross-review NIT answers").
|
||||
3. **Action allow-list.** Anything in here that is NOT needed for idle/anomalous-spend (i.e.
|
||||
over-grant), or any read verb that leaks data we don't want (the intent is pricing + inventory,
|
||||
no object/secret/log data).
|
||||
→ **RESOLVED (FIX applied):** removed `ec2:DescribeImages` (AMIs are not an idle-spend signal).
|
||||
`ec2:DescribeSnapshots` kept (orphan-snapshot waste, account-owned metadata only);
|
||||
`s3:ListAllMyBuckets` kept (bucket *names* only, no `s3:GetObject`); no `logs:*` granted.
|
||||
4. **Session duration vs leaf lifetime.** 1h STS session + ~24h leaf — acceptable blast window?
|
||||
→ Accepted as-is (no change requested).
|
||||
5. **Rollback ordering.** Is disabling the profile/anchor first the correct fastest-cut order,
|
||||
and does step 2/3 leave any orphaned grant?
|
||||
→ Accepted as-is (no change requested).
|
||||
|
||||
## Process note
|
||||
|
||||
|
|
|
|||
|
|
@ -40,7 +40,6 @@
|
|||
"ec2:DescribeAddresses",
|
||||
"ec2:DescribeNatGateways",
|
||||
"ec2:DescribeSnapshots",
|
||||
"ec2:DescribeImages",
|
||||
"ec2:DescribeRegions"
|
||||
],
|
||||
"Resource": "*"
|
||||
|
|
|
|||
|
|
@ -5,8 +5,17 @@ is kept strictly valid (no inline `Comment` keys — IAM rejects those), so all
|
|||
here. This policy is the permission set for the **aws-posture** checker (design D5 / §4):
|
||||
idle / anomalous-spend watch on the Sea Haven AWS account (328440206208, us-east-1).
|
||||
|
||||
**aws-posture itself is NOT built in this change** — it is hard-gated behind the mandatory
|
||||
GPT-4.1 IAM cross-review (design §7, B3). This file + the policy are the review inputs.
|
||||
**Cross-review status (2026-06-18):** GPT-4.1 IAM cross-review returned **APPROVE, no BLOCKs**.
|
||||
FIXes applied to the policy as a result:
|
||||
- **Removed `ec2:DescribeImages`** (data minimization — AMIs are not part of the idle-spend
|
||||
signal; orphan EBS snapshots already cover the storage-waste case via `ec2:DescribeSnapshots`).
|
||||
- The trust policy (`aws-posture-trust-policy.json`) gained **`aws:SourceAccount` =
|
||||
`328440206208`** as an extra confused-deputy guard alongside the existing `aws:SourceArn`
|
||||
trust-anchor pin (see that file).
|
||||
|
||||
**aws-posture itself is built in Phase-3 (this change set) but stays PROVISIONING-GATED** — the
|
||||
checker never calls AWS until step-ca + Roles Anywhere (this packet) are stood up. This file + the
|
||||
policy are the IAM cross-review inputs (design §7, B3).
|
||||
|
||||
## Design principle
|
||||
|
||||
|
|
@ -23,7 +32,7 @@ allow-list** (only the specific read verbs), not by narrowing `Resource`.
|
|||
|---|---|---|
|
||||
| `CostAndUsageReadOnly` | The core idle/anomalous-spend signal (the design flags ≈$330/mo). `GetCostAndUsage`, forecasts, dimensions, and the native CE anomaly detectors. | Cost Explorer is an account-scoped service; its API has no resource-level ARNs, so `Resource:*` is the only valid form. Only `Get*` verbs — no `ce:Update*/Create*/Delete*`, no budget mutation. |
|
||||
| `CloudWatchMetricsReadOnly` | Correlate spend with utilization (an instance billing but at ~0% CPU is idle). `GetMetricData`/`GetMetricStatistics`/`ListMetrics`; `DescribeAlarms*` to see whether an idle resource is already alarmed. | These metric-read APIs do not support resource-level permissions. **No `PutMetricData`, no alarm create/modify/delete.** |
|
||||
| `Ec2DescribeReadOnly` | The classic idle-spend inventory: stopped instances still paying for EBS, unattached volumes, unassociated Elastic IPs, idle NAT gateways, orphan snapshots/AMIs. | `Describe*` is read-only; these list calls don't take resource ARNs. **No `Run*/Start*/Stop*/Terminate*/Modify*/Create*/Delete*`.** |
|
||||
| `Ec2DescribeReadOnly` | The classic idle-spend inventory: stopped instances still paying for EBS, unattached volumes, unassociated Elastic IPs, idle NAT gateways, orphan snapshots. (`ec2:DescribeImages` was **removed** in cross-review — AMIs are not an idle-spend signal aws-posture acts on.) | `Describe*` is read-only; these list calls don't take resource ARNs. **No `Run*/Start*/Stop*/Terminate*/Modify*/Create*/Delete*`.** |
|
||||
| `ElbAndRdsDescribeReadOnly` | Idle load balancers (no healthy targets) and idle/oversized RDS are frequent waste. `Describe*` only. | List APIs without resource-level ARNs. **No `rds:Modify*/Delete*/Reboot*`, no ELB mutation.** |
|
||||
| `LambdaAndStorageInventoryReadOnly` | Inventory functions + buckets to correlate against CloudWatch idle metrics. | **Deliberately excludes `s3:GetObject`** — the role never reads object *data*, only `ListAllMyBuckets` + `GetBucketLocation` (existence/region). **No `lambda:InvokeFunction`, no Lambda mutation.** This is the tightest the inventory can be while still seeing what exists. |
|
||||
|
||||
|
|
@ -38,6 +47,29 @@ allow-list** (only the specific read verbs), not by narrowing `Resource`.
|
|||
A leaked session from this role can **enumerate and price the account, and nothing more** — it
|
||||
cannot read application data, secrets, or change a single resource.
|
||||
|
||||
## Cross-review NIT answers (2026-06-18)
|
||||
|
||||
- **`ec2:DescribeSnapshots` kept (NIT: is it needed?)** — yes. Orphan EBS snapshots are a common
|
||||
idle-spend line item (snapshots of long-deleted volumes keep billing); the checker lists them
|
||||
to flag that waste. It returns only account-owned metadata (we query with `OwnerIds=["self"]`),
|
||||
no snapshot data. `ec2:DescribeImages` (AMIs) was the over-grant and was **removed**.
|
||||
- **`s3:ListAllMyBuckets` kept (NIT: data exposure?)** — it returns only bucket *names* you own,
|
||||
no object data and no bucket contents; `s3:GetBucketLocation` returns only the region. Both are
|
||||
account-owned inventory queries needed to correlate idle buckets/regions against cost. **No
|
||||
`s3:GetObject`** anywhere, so there is no data-plane read path.
|
||||
- **No `logs:*` (NIT: do we need CloudWatch Logs?)** — no. aws-posture reasons over *metrics*
|
||||
(`cloudwatch:GetMetric*`) and the cost/inventory describe calls; it never needs log *events*.
|
||||
Omitting `logs:GetLogEvents`/`logs:FilterLogEvents` keeps the role off the log-exfil path.
|
||||
- **`aws:RequestedRegion` condition (NIT: optional region pin) — SKIPPED, deliberately.** The
|
||||
reviewer flagged this as optional. It is **NOT applied** because Cost Explorer (`ce:*`) and
|
||||
`s3:ListAllMyBuckets` are **global-endpoint services** that resolve to us-east-1 with request
|
||||
contexts where `aws:RequestedRegion` does not reliably equal `us-east-1` — a blanket region
|
||||
condition risks **DENYing the core cost signal**. Scoping it to a separate statement covering
|
||||
only the regional `Describe*` calls (ec2/rds/elb/cloudwatch) would add a fourth+ statement for
|
||||
marginal benefit (the action allow-list already bounds blast radius, and the box only ever runs
|
||||
in us-east-1). Per the task's guidance, we prefer SKIP over a region pin that could break the
|
||||
global-service statements.
|
||||
|
||||
## Comparison to the AWS-managed alternatives
|
||||
|
||||
`ReadOnlyAccess` / `ViewOnlyAccess` are far broader (they include `s3:GetObject`,
|
||||
|
|
|
|||
|
|
@ -15,7 +15,8 @@
|
|||
"Condition": {
|
||||
"StringEquals": {
|
||||
"aws:PrincipalTag/x509Subject/CN": "r720-aws-posture",
|
||||
"aws:PrincipalTag/x509Issuer/CN": "Sea Haven Internal CA - R720 Roles Anywhere"
|
||||
"aws:PrincipalTag/x509Issuer/CN": "Sea Haven Internal CA - R720 Roles Anywhere",
|
||||
"aws:SourceAccount": "328440206208"
|
||||
},
|
||||
"ArnEquals": {
|
||||
"aws:SourceArn": "arn:aws:rolesanywhere:us-east-1:328440206208:trust-anchor/REPLACE_WITH_TRUST_ANCHOR_ID"
|
||||
|
|
|
|||
Reference in a new issue