diff --git a/terraform/environments/aws/ci-cd-permissions/policy_compute.tf b/terraform/environments/aws/ci-cd-permissions/policy_compute.tf index e7d2b793d..55d1554c0 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_compute.tf +++ b/terraform/environments/aws/ci-cd-permissions/policy_compute.tf @@ -152,6 +152,19 @@ resource "aws_iam_policy" "compute" { # Function mutations live in policy_compute_b.tf — the function # resource type does not support aws:ResourceTag per the AWS # Service Authorization Reference, so it needs ARN scoping. + # + # StringEqualsIgnoreCase for the same reason as KMSMutateTaggedOnly + # below (see PR #1496): the distribution's Project tag comes from + # local.common_tags (terraform/environments/aws/main.tf), which sets + # Project = var.project_name, and project_name is the lowercase + # "cudly". That resource-level tag overrides the provider's + # default_tags value of "CUDly" for this distribution, so a + # case-sensitive match against "CUDly" never matched and every action + # in this statement was silently denied. This is the same defect + # #1496 fixed on the KMS statements; it was left unfixed here because + # enable_cdn is false in every tfvars, so nothing exercised it. Note + # it would also have killed the destroy path: deleting a distribution + # requires UpdateDistribution to disable it first. Sid = "CloudFrontMutateTaggedOnly" Effect = "Allow" Action = [ @@ -162,7 +175,7 @@ resource "aws_iam_policy" "compute" { ] Resource = "*" Condition = { - StringEquals = { + StringEqualsIgnoreCase = { "aws:ResourceTag/Project" = "CUDly" } } diff --git a/terraform/environments/aws/ci-cd-permissions/policy_compute_b.tf b/terraform/environments/aws/ci-cd-permissions/policy_compute_b.tf index eb213010a..278533932 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_compute_b.tf +++ b/terraform/environments/aws/ci-cd-permissions/policy_compute_b.tf @@ -97,6 +97,38 @@ resource "aws_iam_policy" "compute_b" { } } }, + { + # ecr:PutImageScanningConfiguration is the writer for + # aws_ecr_repository's image_scanning_configuration block. Its sibling + # ecr:PutImageTagMutability is granted in policy_compute.tf's + # ECRRepositoryScoped but this one was not, so any change to + # scan_on_push (or importing a repository whose setting differs from + # the config) fails the apply. Same repository ARN scope as the + # statement it belongs with; it lives here only because + # policy_compute.tf is at the 6144-char ceiling. + Sid = "ECRImageScanningConfig" + Effect = "Allow" + Action = ["ecr:PutImageScanningConfiguration"] + Resource = "arn:aws:ecr:*:*:repository/cudly-*" + }, + { + # The ELB update path that policy_compute.tf's ELBFargate misses. + # ELBFargate grants Create/Delete plus the Modify* family, but changing + # a load balancer's subnets or security groups (which is what bumping + # az_count does) goes through the Set* family instead, and + # ModifyListenerAttributes is the writer whose reader + # (DescribeListenerAttributes) is already granted. Resource = "*" + # matches ELBFargate, whose ARNs are not name-scopeable. + Sid = "ELBFargateSetAttributes" + Effect = "Allow" + Action = [ + "elasticloadbalancing:ModifyListenerAttributes", + "elasticloadbalancing:SetIpAddressType", + "elasticloadbalancing:SetSecurityGroups", + "elasticloadbalancing:SetSubnets", + ] + Resource = "*" + }, { Sid = "KMSAliasMutate" Effect = "Allow" diff --git a/terraform/environments/aws/ci-cd-permissions/policy_data.tf b/terraform/environments/aws/ci-cd-permissions/policy_data.tf index eca60627e..22996efb3 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_data.tf +++ b/terraform/environments/aws/ci-cd-permissions/policy_data.tf @@ -39,6 +39,17 @@ resource "aws_iam_policy" "data" { "iam:UntagInstanceProfile", "iam:UntagPolicy", "iam:UntagRole", + # iam:UpdateAssumeRolePolicy / UpdateRole / UpdateRoleDescription are + # what the provider calls when a role's assume_role_policy, + # max_session_duration or description drifts (resourceRoleUpdate in + # internal/service/iam/role.go). Without them any edit to a trust + # policy or role description in terraform/modules/** fails the apply + # rather than the plan, which is why this gap stayed invisible. + # IAMDenyModifyDeployRoleAndPolicies below stops these from being + # turned on the deploy role itself. + "iam:UpdateAssumeRolePolicy", + "iam:UpdateRole", + "iam:UpdateRoleDescription", ] Resource = [ "arn:aws:iam::*:role/cudly-*", @@ -65,6 +76,20 @@ resource "aws_iam_policy" "data" { "lambda.amazonaws.com", "ecs-tasks.amazonaws.com", "rds.amazonaws.com", + # ec2: the fck-nat instance profile referenced by the NAT launch + # template and Auto Scaling group + # (terraform/modules/networking/aws, enable_nat_gateway is + # hardcoded true in terraform/environments/aws/networking.tf, so + # this is on the deploy path in every environment). + "ec2.amazonaws.com", + # vpc-flow-logs: aws_flow_log passes its delivery role via + # DeliverLogsPermissionArn (enable_flow_logs is true for staging + # and prod). + "vpc-flow-logs.amazonaws.com", + # events: EventBridge targets that carry a role_arn, used by the + # scheduled ECS tasks on the Fargate deploy path + # (enable_scheduled_tasks is true in every tfvars). + "events.amazonaws.com", ] } } @@ -85,6 +110,54 @@ resource "aws_iam_policy" "data" { Action = ["iam:PassRole"] Resource = ["arn:aws:iam::*:role/cudly-terraform-deploy"] }, + { + # IAMDenyPassDeployRole above closes the pass-the-deploy-role door. + # This closes the other two doors into the same escalation, both of + # which IAMRolesAndPolicies leaves open because cudly-terraform-deploy + # is itself a cudly-* role and cudly-deploy-* are cudly-* policies: + # + # 1. Role side: iam:AttachRolePolicy / iam:PutRolePolicy let the + # deploy role grant itself AdministratorAccess, and + # iam:UpdateAssumeRolePolicy (added to IAMRolesAndPolicies above) + # lets it rewrite its own trust policy so an arbitrary principal + # can assume it. + # 2. Policy side: iam:CreatePolicyVersion on cudly-deploy-data (or + # any sibling) rewrites the deploy role's own permissions in + # place, reaching the same admin without ever touching the role. + # Denying only the role side would look complete and would not be. + # + # Either turns a leaked GitHub Actions token into persistent + # account-wide admin. An explicit Deny always beats the Allow, so this + # closes both loops while leaving the workload roles and policies + # (cudly---*) fully manageable. + # + # This costs the deploy path nothing: cudly-terraform-deploy and the + # four cudly-deploy-* policies are all declared in this bootstrap + # root, which is applied manually by a privileged human and never by a + # deploy workflow. Action and Resource are matched as a cross product, + # so the combinations that do not apply (a role action against a + # policy ARN and vice versa) are simply inert. + Sid = "IAMDenyModifyDeployRoleAndPolicies" + Effect = "Deny" + Action = [ + "iam:AttachRolePolicy", + "iam:CreatePolicyVersion", + "iam:DeletePolicy", + "iam:DeletePolicyVersion", + "iam:DeleteRole", + "iam:DeleteRolePolicy", + "iam:DetachRolePolicy", + "iam:PutRolePolicy", + "iam:SetDefaultPolicyVersion", + "iam:UpdateAssumeRolePolicy", + "iam:UpdateRole", + "iam:UpdateRoleDescription", + ] + Resource = [ + "arn:aws:iam::*:role/cudly-terraform-deploy", + "arn:aws:iam::*:policy/cudly-deploy-*", + ] + }, { Sid = "IAMReadForPassRole" Effect = "Allow" @@ -95,28 +168,61 @@ resource "aws_iam_policy" "data" { Resource = "*" }, { + # Service-linked roles are created lazily by AWS on the FIRST use of a + # service in an account/region, and the caller needs + # iam:CreateServiceLinkedRole for it. That makes these gaps invisible + # in an account that already has the role and a hard failure in a + # fresh region, which is exactly the kind of latent break this audit + # was meant to surface. autoscaling (fck-nat ASG, on every deploy + # path), elasticloadbalancing and ecs (Fargate ALB and cluster) were + # all missing; note that ecs.application-autoscaling is a different + # principal from plain ecs and does not cover it. Sid = "IAMServiceLinkedRole" Effect = "Allow" Action = ["iam:CreateServiceLinkedRole"] Resource = [ "arn:aws:iam::*:role/aws-service-role/rds.amazonaws.com/AWSServiceRoleForRDS", "arn:aws:iam::*:role/aws-service-role/ecs.application-autoscaling.amazonaws.com/AWSServiceRoleForApplicationAutoScaling_ECSService", + "arn:aws:iam::*:role/aws-service-role/autoscaling.amazonaws.com/AWSServiceRoleForAutoScaling", + "arn:aws:iam::*:role/aws-service-role/elasticloadbalancing.amazonaws.com/AWSServiceRoleForElasticLoadBalancing", + "arn:aws:iam::*:role/aws-service-role/ecs.amazonaws.com/AWSServiceRoleForECS", ] Condition = { StringLike = { "iam:AWSServiceName" = [ "rds.amazonaws.com", "ecs.application-autoscaling.amazonaws.com", + "autoscaling.amazonaws.com", + "elasticloadbalancing.amazonaws.com", + "ecs.amazonaws.com", ] } } }, { - # RDS actions that take a specific resource ARN are scoped to - # cudly-* DB instances, subnet groups, and proxies. This prevents - # the deploy SA from deleting or modifying unrelated RDS instances - # in the same account. DescribeDBEngineVersions does not accept a - # resource ARN (account-wide catalogue lookup) and is split below. + # RDS actions whose request carries a name we control are scoped to + # cudly-* DB instances, subnet groups and snapshots. This prevents the + # deploy SA from deleting or modifying unrelated RDS resources in the + # same account. + # + # NO rds:Describe* action lives here; they are all in + # RDSDescribeAccountWide below. See that statement for why. + # + # KNOWN DEAD SCOPES: the proxy actions below (DeleteDBProxy, + # ModifyDBProxy, RegisterDBProxyTargets, DeregisterDBProxyTargets) + # can never be authorized, because their only resource types are + # `proxy` and `target-group`, whose real ARNs are + # arn:aws:rds:::db-proxy:prx- and + # arn:aws:rds:::target-group:prx-tg-: a different ARN + # segment (`db-proxy`, not `proxy`) and a server-assigned opaque id + # that never contains the resource name. `proxy:cudly-*` and + # `target-group:cudly-*` therefore match nothing. Nothing exercises + # this today (modules/database/aws is instantiated with + # enable_rds_proxy = false, terraform/environments/aws/database.tf), + # so it is left as-is rather than widened to db-proxy:* here; fixing + # it needs a scoping mechanism that actually constrains an opaque id + # (tag condition), tracked separately. Do NOT add new proxy actions + # to this statement; they would be dead on arrival. Sid = "RDSResourceScoped" Effect = "Allow" Action = [ @@ -126,16 +232,12 @@ resource "aws_iam_policy" "data" { "rds:CreateDBSubnetGroup", "rds:DeleteDBInstance", "rds:DeleteDBSubnetGroup", - "rds:DescribeDBInstances", - "rds:DescribeDBSubnetGroups", "rds:ListTagsForResource", "rds:DeleteDBProxy", "rds:DeregisterDBProxyTargets", - "rds:DescribeDBProxies", - "rds:DescribeDBProxyTargetGroups", - "rds:DescribeDBProxyTargets", "rds:ModifyDBInstance", "rds:ModifyDBProxy", + "rds:ModifyDBSubnetGroup", "rds:RegisterDBProxyTargets", "rds:RemoveTagsFromResource", ] @@ -148,11 +250,66 @@ resource "aws_iam_policy" "data" { ] }, { - # DescribeDBEngineVersions is a catalogue lookup that doesn't - # accept a resource ARN at the API level. Read-only, no state change. - Sid = "RDSDescribeEngineVersions" - Effect = "Allow" - Action = ["rds:DescribeDBEngineVersions"] + # RDS reads that an ARN-scoped grant can never authorize. + # + # rds:DescribeDBEngineVersions is a catalogue lookup with no resource + # type at all, so it can only be granted on "*". + # + # rds:DescribeDBInstances is here because of the request shape the AWS + # provider uses, not because the action lacks a resource type. The + # Terraform id of aws_db_instance IS the DbiResourceId (`db-`, + # set at internal/service/rds/instance.go in provider v5.100.0), and + # findDBInstanceByID branches on that: when the id looks like a + # DbiResourceId it sends DescribeDBInstances with + # Filters=[dbi-resource-id] and leaves DBInstanceIdentifier nil. RDS + # authorizes an identifier-less DescribeDBInstances against the + # wildcard ARN arn:aws:rds:::db:*, which + # `db:cudly-*` cannot match, so every refresh of the instance 403'd: + # AccessDenied: ... not authorized to perform: rds:DescribeDBInstances + # on resource: arn:aws:rds:us-east-1:...:db:* + # This is unconditional on every read, not a not-found fallback: the + # provider's retry with the plain identifier only fires on NotFound, + # and AccessDenied is not NotFound, so the plan hard-fails. + # + # rds:DescribeDBSubnetGroups is here for the same reason as + # DescribeDBInstances, and it is worth being explicit about why, + # because the tempting simplification is wrong in both directions. + # Neither action lacks a resource type: per AWS's Service + # Authorization Reference both DO support one (`db` and `subgrp` + # respectively), and the real names DO match the patterns + # (cudly---postgres against db:cudly-*, + # cudly---db-subnet against subgrp:cudly-*). So this is not + # "ARN-scoped Describes are always dead" and it is not a name-prefix + # bug; the scoped grants were syntactically valid. What differs is the + # request shape: a resource-scoped grant authorizes only the form that + # targets one named resource, while the enumerate form (no identifier + # in the request) is authorized against the wildcard ARN. The + # production 403 on DescribeDBInstances is the empirical proof that + # the scoped form did not suffice, and DescribeDBSubnetGroups sits in + # exactly the same position: aws_db_subnet_group.main + # (terraform/modules/database/aws/main.tf) is created in every + # environment, so leaving it scoped just primes the next 403. + # + # The DB proxy reads are here for a third reason: their resource ARNs + # use the `db-proxy:`/`target-group:` segments with server-assigned + # opaque ids (see the RDSResourceScoped note above), so no name-based + # ARN scope can ever match them. Those are belt-and-braces rather than + # load-bearing: enable_rds_proxy is a literal `false` at + # terraform/environments/aws/database.tf:28, not a variable, so no + # tfvars can turn the proxy resources on. + # + # All six are read-only and expose only resource metadata; every + # mutating RDS action stays ARN-scoped above. + Sid = "RDSDescribeAccountWide" + Effect = "Allow" + Action = [ + "rds:DescribeDBEngineVersions", + "rds:DescribeDBInstances", + "rds:DescribeDBProxies", + "rds:DescribeDBProxyTargetGroups", + "rds:DescribeDBProxyTargets", + "rds:DescribeDBSubnetGroups", + ] Resource = "*" }, { @@ -171,6 +328,20 @@ resource "aws_iam_policy" "data" { Resource = "*" }, { + # secretsmanager:ListSecrets used to be in this list. It has no + # resource type per the AWS Service Authorization Reference, so + # inside an ARN-scoped statement it could never be satisfied: a + # silently dead grant. Nothing in the provider calls it (secret + # lookups go through DescribeSecret with an explicit id), so it is + # dropped rather than moved to a "*" statement. + # + # ListSecretVersionIds and UpdateSecretVersionStage are what + # aws_secretsmanager_secret_version calls when a version carries a + # stage other than a lone AWSCURRENT, and on the delete path when it + # has to move stages off the version before removing it + # (resourceSecretVersionDelete/Update in + # internal/service/secretsmanager/secret_version.go). Both take the + # secret ARN, so the cudly-* scope below applies. Sid = "SecretsManager" Effect = "Allow" Action = [ @@ -179,13 +350,14 @@ resource "aws_iam_policy" "data" { "secretsmanager:DescribeSecret", "secretsmanager:GetResourcePolicy", "secretsmanager:GetSecretValue", - "secretsmanager:ListSecrets", + "secretsmanager:ListSecretVersionIds", "secretsmanager:PutSecretValue", "secretsmanager:RestoreSecret", "secretsmanager:RotateSecret", "secretsmanager:TagResource", "secretsmanager:UntagResource", "secretsmanager:UpdateSecret", + "secretsmanager:UpdateSecretVersionStage", ] Resource = "arn:aws:secretsmanager:*:*:secret:cudly-*" }, diff --git a/terraform/environments/aws/ci-cd-permissions/policy_networking.tf b/terraform/environments/aws/ci-cd-permissions/policy_networking.tf index 3336d451c..0ff3ec119 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_networking.tf +++ b/terraform/environments/aws/ci-cd-permissions/policy_networking.tf @@ -22,6 +22,17 @@ resource "aws_iam_policy" "networking" { "ec2:CreateEgressOnlyInternetGateway", "ec2:CreateInternetGateway", "ec2:CreateLaunchTemplate", + # The fck-nat launch template sources its AMI from a + # most_recent = true data.aws_ami lookup, so every upstream AMI + # republish takes the launch template's UPDATE path rather than + # create: the provider calls CreateLaunchTemplateVersion for the new + # image_id and ModifyLaunchTemplate to move the default version + # (resourceLaunchTemplateUpdate). Only Create/DeleteLaunchTemplate + # were granted, so the first AMI rotation after the ASG exists would + # have failed the apply. + "ec2:CreateLaunchTemplateVersion", + "ec2:DeleteLaunchTemplateVersions", + "ec2:ModifyLaunchTemplate", "ec2:CreateRoute", "ec2:CreateRouteTable", "ec2:CreateSecurityGroup", @@ -39,6 +50,12 @@ resource "aws_iam_policy" "networking" { "ec2:DeleteRouteTable", "ec2:DeleteSecurityGroup", "ec2:DeleteSubnet", + # ec2:DeleteTags is the other half of ec2:CreateTags: the provider's + # tag update path removes dropped keys with DeleteTags before adding + # new ones with CreateTags, so dropping any key from common_tags / + # default_tags on an existing VPC, subnet, route table, gateway, + # security group or launch template fails the apply without it. + "ec2:DeleteTags", "ec2:DeleteVpc", "ec2:DeleteVpcEndpoints", "ec2:DescribeAccountAttributes", @@ -66,6 +83,11 @@ resource "aws_iam_policy" "networking" { "ec2:ModifyVpcAttribute", "ec2:ModifyVpcEndpoint", "ec2:ReplaceRoute", + # aws_route_table_association's update path calls + # ReplaceRouteTableAssociation rather than + # Disassociate + Associate, so moving a subnet between route tables + # needs it even though both halves of that pair are granted. + "ec2:ReplaceRouteTableAssociation", "ec2:RevokeSecurityGroupEgress", "ec2:RevokeSecurityGroupIngress", ] @@ -73,16 +95,28 @@ resource "aws_iam_policy" "networking" { }, { # ec2:RunInstances is needed for the fck-nat AutoScaling group - # (one t4g.nano per AZ). Scoping to the fck-nat launch template - # and restricting allowed instance types prevents the deploy SA - # from launching arbitrary large instances and attaching CUDly IAM - # roles to exfiltrate credentials or run compute at account cost. + # (one t4g.nano per AZ). Restricting allowed instance types prevents + # the deploy SA from launching arbitrary large instances and attaching + # CUDly IAM roles to exfiltrate credentials or run compute at account + # cost. + # + # The operator MUST be StringEqualsIfExists, not StringEquals. AWS + # authorizes a single RunInstances call against every resource type it + # touches (instance, volume, network-interface, security-group, + # subnet, image, key-pair, launch-template), and ec2:InstanceType is + # only in the request context for the INSTANCE leg. With a plain + # StringEquals the key is absent on every other leg, the condition + # evaluates false there, and the call is denied as a whole even though + # the instance type is correct. StringEqualsIfExists keeps the t4g.nano + # restriction exactly where the key exists and lets the supporting legs + # through, which is the pattern the IAM condition-operators reference + # documents for precisely this call. Sid = "EC2RunInstancesFckNAT" Effect = "Allow" Action = ["ec2:RunInstances"] Resource = "*" Condition = { - StringEquals = { + StringEqualsIfExists = { "ec2:InstanceType" = ["t4g.nano"] } }