From 97356dec5bd5cc98c77dab899ff21b9ad498076d Mon Sep 17 00:00:00 2001 From: Aaron Gibson Date: Sun, 4 Oct 2026 05:56:01 +0000 Subject: [PATCH 1/4] Let ECS execution roles read only their own task's secrets Every API service's execution role, and the reuploader's role, could read any Secrets Manager secret and any SSM parameter in the account (GetSecretValue on *, GetParameter* on arn:aws:ssm:*), although ECS only uses it to inject the task's task_secrets. The reuploader's is also its task role, so its own code had that access too. The policies are now built in HCL instead of templates: the secret statements grant secretsmanager:GetSecretValue and ssm:GetParameters, the two actions ECS uses to inject secrets, on the ARNs in task_secrets only (every task_secrets value in dev and prod is an SSM parameter or Secrets Manager secret ARN). A task without secrets, like the reuploader, gets no secret statement; its code only uses S3. The other statements are unchanged. Planned with fake inputs as prod's ooniauth (SSM and Secrets Manager secrets), ooniprobe, reverseproxy and the reuploader, from main and from this change: the same resources, task definitions unchanged, and the policies differ only in those statements. terraform validate passes for dev and prod. oonith_service and ooni_dataapi have the same policy but are not used by either environment. --- tf/modules/ooniapi_service/main.tf | 43 ++++++++++- .../templates/profile_policy.json | 61 ---------------- tf/modules/scheduled_service/main.tf | 48 ++++++++++++- .../templates/profile_policy.json | 71 ------------------- 4 files changed, 89 insertions(+), 134 deletions(-) delete mode 100644 tf/modules/ooniapi_service/templates/profile_policy.json delete mode 100644 tf/modules/scheduled_service/templates/profile_policy.json diff --git a/tf/modules/ooniapi_service/main.tf b/tf/modules/ooniapi_service/main.tf index 94bebfd8..1ed138a4 100644 --- a/tf/modules/ooniapi_service/main.tf +++ b/tf/modules/ooniapi_service/main.tf @@ -29,11 +29,52 @@ resource "aws_iam_role" "ooniapi_service_task" { EOF } +locals { + # The execution role only needs to read the secrets ECS injects into this + # task (task_secrets): every value is an SSM parameter or a Secrets + # Manager secret ARN. + task_secret_arns_secretsmanager = [for arn in values(var.task_secrets) : arn if startswith(arn, "arn:aws:secretsmanager:")] + task_secret_arns_ssm = [for arn in values(var.task_secrets) : arn if startswith(arn, "arn:aws:ssm:")] + task_secret_statements = concat( + length(local.task_secret_arns_secretsmanager) == 0 ? [] : [{ + Effect = "Allow" + Action = ["secretsmanager:GetSecretValue"] + Resource = local.task_secret_arns_secretsmanager + }], + length(local.task_secret_arns_ssm) == 0 ? [] : [{ + Effect = "Allow" + Action = ["ssm:GetParameters"] + Resource = local.task_secret_arns_ssm + }], + ) +} + resource "aws_iam_role_policy" "ooniapi_service_task" { name = "${local.name}-task-role" role = aws_iam_role.ooniapi_service_task.name - policy = templatefile("${path.module}/templates/profile_policy.json", {}) + policy = jsonencode({ + Version = "2012-10-17" + Statement = concat([ + { + Sid = "ecsInstanceRole" + Effect = "Allow" + Action = ["ecs:DeregisterContainerInstance", "ecs:DiscoverPollEndpoint", "ecs:Poll", "ecs:RegisterContainerInstance", "ecs:Submit*", "ecs:StartTelemetrySession"] + Resource = ["*"] + }, + { + Sid = "CloudWatchLogsFullAccess" + Effect = "Allow" + Action = ["logs:*", "cloudwatch:GenerateQuery"] + Resource = "*" + }, + { + Effect = "Allow" + Action = ["ec2:Describe*", "elasticloadbalancing:DeregisterInstancesFromLoadBalancer", "elasticloadbalancing:DeregisterTargets", "elasticloadbalancing:Describe*", "elasticloadbalancing:RegisterInstancesWithLoadBalancer", "elasticloadbalancing:RegisterTargets"] + Resource = "*" + }, + ], local.task_secret_statements) + }) } resource "aws_cloudwatch_log_group" "ooniapi_service" { diff --git a/tf/modules/ooniapi_service/templates/profile_policy.json b/tf/modules/ooniapi_service/templates/profile_policy.json deleted file mode 100644 index 3a772893..00000000 --- a/tf/modules/ooniapi_service/templates/profile_policy.json +++ /dev/null @@ -1,61 +0,0 @@ -{ - "Version": "2012-10-17", - "Statement": [ - { - "Sid": "ecsInstanceRole", - "Effect": "Allow", - "Action": [ - "ecs:DeregisterContainerInstance", - "ecs:DiscoverPollEndpoint", - "ecs:Poll", - "ecs:RegisterContainerInstance", - "ecs:Submit*", - "ecs:StartTelemetrySession" - ], - "Resource": ["*"] - }, - { - "Sid": "CloudWatchLogsFullAccess", - "Effect": "Allow", - "Action": ["logs:*", "cloudwatch:GenerateQuery"], - "Resource": "*" - }, - { - "Effect": "Allow", - "Action": [ - "secretsmanager:GetResourcePolicy", - "secretsmanager:GetSecretValue", - "secretsmanager:DescribeSecret", - "secretsmanager:ListSecretVersionIds" - ], - "Resource": "*" - }, - { - "Effect": "Allow", - "Action": "secretsmanager:ListSecrets", - "Resource": "*" - }, - { - "Effect": "Allow", - "Action": [ - "ssm:GetParameter", - "ssm:GetParameters", - "ssm:GetParameterHistory", - "ssm:GetParametersByPath" - ], - "Resource": "arn:aws:ssm:*" - }, - { - "Effect": "Allow", - "Action": [ - "ec2:Describe*", - "elasticloadbalancing:DeregisterInstancesFromLoadBalancer", - "elasticloadbalancing:DeregisterTargets", - "elasticloadbalancing:Describe*", - "elasticloadbalancing:RegisterInstancesWithLoadBalancer", - "elasticloadbalancing:RegisterTargets" - ], - "Resource": "*" - } - ] -} diff --git a/tf/modules/scheduled_service/main.tf b/tf/modules/scheduled_service/main.tf index 63f745c1..820c7c40 100644 --- a/tf/modules/scheduled_service/main.tf +++ b/tf/modules/scheduled_service/main.tf @@ -29,11 +29,57 @@ resource "aws_iam_role" "scheduled_service_task" { EOF } +locals { + # The execution role only needs to read the secrets ECS injects into this + # task (task_secrets): every value is an SSM parameter or a Secrets + # Manager secret ARN. + task_secret_arns_secretsmanager = [for arn in values(var.task_secrets) : arn if startswith(arn, "arn:aws:secretsmanager:")] + task_secret_arns_ssm = [for arn in values(var.task_secrets) : arn if startswith(arn, "arn:aws:ssm:")] + task_secret_statements = concat( + length(local.task_secret_arns_secretsmanager) == 0 ? [] : [{ + Effect = "Allow" + Action = ["secretsmanager:GetSecretValue"] + Resource = local.task_secret_arns_secretsmanager + }], + length(local.task_secret_arns_ssm) == 0 ? [] : [{ + Effect = "Allow" + Action = ["ssm:GetParameters"] + Resource = local.task_secret_arns_ssm + }], + ) +} + resource "aws_iam_role_policy" "scheduled_service_task" { name = "${local.name}-task-role" role = aws_iam_role.scheduled_service_task.name - policy = templatefile("${path.module}/templates/profile_policy.json", {}) + policy = jsonencode({ + Version = "2012-10-17" + Statement = concat([ + { + Sid = "ecsInstanceRole" + Effect = "Allow" + Action = ["ecs:DeregisterContainerInstance", "ecs:DiscoverPollEndpoint", "ecs:Poll", "ecs:RegisterContainerInstance", "ecs:Submit*", "ecs:StartTelemetrySession"] + Resource = ["*"] + }, + { + Sid = "CloudWatchLogsFullAccess" + Effect = "Allow" + Action = ["logs:*", "cloudwatch:GenerateQuery"] + Resource = "*" + }, + { + Effect = "Allow" + Action = ["ec2:Describe*", "elasticloadbalancing:DeregisterInstancesFromLoadBalancer", "elasticloadbalancing:DeregisterTargets", "elasticloadbalancing:Describe*", "elasticloadbalancing:RegisterInstancesWithLoadBalancer", "elasticloadbalancing:RegisterTargets"] + Resource = "*" + }, + { + Effect = "Allow" + Action = ["events:TagResource", "events:PutRule", "events:PutTargets"] + Resource = "*" + }, + ], local.task_secret_statements) + }) } resource "aws_iam_role" "events_run_task" { diff --git a/tf/modules/scheduled_service/templates/profile_policy.json b/tf/modules/scheduled_service/templates/profile_policy.json deleted file mode 100644 index b0a059d2..00000000 --- a/tf/modules/scheduled_service/templates/profile_policy.json +++ /dev/null @@ -1,71 +0,0 @@ -{ - "Version": "2012-10-17", - "Statement": [ - { - "Sid": "ecsInstanceRole", - "Effect": "Allow", - "Action": [ - "ecs:DeregisterContainerInstance", - "ecs:DiscoverPollEndpoint", - "ecs:Poll", - "ecs:RegisterContainerInstance", - "ecs:Submit*", - "ecs:StartTelemetrySession" - ], - "Resource": ["*"] - }, - { - "Sid": "CloudWatchLogsFullAccess", - "Effect": "Allow", - "Action": ["logs:*", "cloudwatch:GenerateQuery"], - "Resource": "*" - }, - { - "Effect": "Allow", - "Action": [ - "secretsmanager:GetResourcePolicy", - "secretsmanager:GetSecretValue", - "secretsmanager:DescribeSecret", - "secretsmanager:ListSecretVersionIds" - ], - "Resource": "*" - }, - { - "Effect": "Allow", - "Action": "secretsmanager:ListSecrets", - "Resource": "*" - }, - { - "Effect": "Allow", - "Action": [ - "ssm:GetParameter", - "ssm:GetParameters", - "ssm:GetParameterHistory", - "ssm:GetParametersByPath" - ], - "Resource": "arn:aws:ssm:*" - }, - { - "Effect": "Allow", - "Action": [ - "ec2:Describe*", - "elasticloadbalancing:DeregisterInstancesFromLoadBalancer", - "elasticloadbalancing:DeregisterTargets", - "elasticloadbalancing:Describe*", - "elasticloadbalancing:RegisterInstancesWithLoadBalancer", - "elasticloadbalancing:RegisterTargets" - ], - "Resource": "*" - }, - { - "Effect": "Allow", - "Action": [ - "events:TagResource", - "events:PutRule", - "events:PutTargets" - ], - "Resource": "*" - } - - ] -} From 3624df031ae4796a17132313dd4a3ca2f7c92b74 Mon Sep 17 00:00:00 2001 From: Aaron Gibson Date: Sun, 4 Oct 2026 05:57:22 +0000 Subject: [PATCH 2/4] Give ooniprobe a task role for its S3 access ooniprobe uploads failed reports and reads its private config and the anonymous credentials manifests from S3. Its tasks have no task role, so the AWS SDK falls back to the container host's instance credentials, where those permissions are granted. Every container on the host can get those credentials, and they can't be taken away from the containers while ooniprobe depends on them. ooniapi_service gains an optional task_role_policy: given, the task gets a task role with it (ECS_ENABLE_TASK_IAM_ROLE is already on), whose credentials the SDK prefers over the host's. ooniprobe and ooniprobe_legacy get the S3 policy, now declared once as local.ooniprobe_s3_policy. The instance role keeps the same policy for now, so tasks still running the previous revision keep working during the rollout; the next commit removes it. Planned with fake inputs: only ooniprobe gains a role, its policy and a task_role_arn; the other services are unchanged. The instance role's policy is the same statements as before. terraform validate passes for dev and prod. The other services' code makes no AWS calls of its own, apart from ooniauth's SES calls with its own IAM user's keys and the reuploader, which already has a task role. --- tf/environments/dev/main.tf | 72 ++++++++++++++----------- tf/environments/prod/main.tf | 72 ++++++++++++++----------- tf/modules/ooniapi_service/main.tf | 27 ++++++++++ tf/modules/ooniapi_service/variables.tf | 6 +++ 4 files changed, 115 insertions(+), 62 deletions(-) diff --git a/tf/environments/dev/main.tf b/tf/environments/dev/main.tf index 3cdfaf40..62a311af 100644 --- a/tf/environments/dev/main.tf +++ b/tf/environments/dev/main.tf @@ -523,41 +523,47 @@ resource "aws_security_group_rule" "elasticache_sg_rule" { #### OONI Probe service # For accessing the s3 bucket +# The S3 objects ooniprobe reads and writes: failed reports, its private +# config, and the anonymous credentials manifests +locals { + ooniprobe_s3_policy = jsonencode({ + Version = "2012-10-17" + Statement = [ + { + Sid = "" + Effect = "Allow" + Action = "s3:PutObject" + Resource = "${aws_s3_bucket.ooniprobe_failed_reports.arn}/*" + }, + { + Sid = "" + Effect = "Allow" + Action = "s3:GetObject" + Resource = "${aws_s3_bucket.ooni_private_config_bucket.arn}/*" + }, + { + Sid = "" + Effect = "Allow" + Action = "s3:GetObject" + Resource = "${aws_s3_bucket.anoncred_manifests.arn}/*" + }, + { + Sid = "" + Effect = "Allow" + Action = "s3:ListBucket" + Resource = "${aws_s3_bucket.anoncred_manifests.arn}/*" + }, + ] + }) +} + +# Until the tasks use their task role, they get ooniprobe's S3 access through +# the container host's credentials resource "aws_iam_role_policy" "ooniprobe_role" { name = "${local.name}-task-role" role = module.ooniapi_cluster.container_host_role.name - policy = < Date: Sun, 4 Oct 2026 05:58:38 +0000 Subject: [PATCH 3/4] Keep the ECS hosts' instance credentials away from the containers The tasks run in bridge mode, so any container could fetch the container host's instance credentials from the metadata service. Those could read every secret in Secrets Manager and ooniprobe's S3 objects, whatever the container's own task was given. - The launch template requires IMDSv2 with a hop limit of 1. The ECS agent (host network) and docker's awslogs driver still reach it; the containers, a hop further on the bridge network, can't. Tasks get AWS credentials from their task role instead, through the ECS agent. - The instance role no longer reads secrets: ECS injects each task's secrets with that task's execution role, and nothing on the host (ecs-setup.sh) reads any. - ooniprobe's S3 policy is removed from the instance role; ooniprobe and ooniprobe_legacy have it as their task role since the previous commit. Planned with fake inputs from main and from this branch: the same cluster resources; only the instance role policy (the two secret statements) and the launch template's metadata_options change. terraform validate passes for dev and prod. Rollout, per environment: 1. Apply the previous commit, and let ooniprobe and ooniprobe_legacy deploy the task definition with the task role (check that failed report uploads and the manifest and config reads keep working). 2. Apply this commit. The autoscaling group launches new hosts from the new template ($Latest) but doesn't refresh running ones on a template change; start an instance refresh, or run aws ec2 modify-instance-metadata-options --http-tokens required --http-put-response-hop-limit 1 on each running host. --- tf/environments/dev/main.tf | 9 --------- tf/environments/prod/main.tf | 9 --------- tf/modules/ecs_cluster/main.tf | 10 ++++++++++ .../ecs_cluster/templates/profile_policy.json | 15 --------------- 4 files changed, 10 insertions(+), 33 deletions(-) diff --git a/tf/environments/dev/main.tf b/tf/environments/dev/main.tf index 62a311af..7d4d0efa 100644 --- a/tf/environments/dev/main.tf +++ b/tf/environments/dev/main.tf @@ -557,15 +557,6 @@ locals { }) } -# Until the tasks use their task role, they get ooniprobe's S3 access through -# the container host's credentials -resource "aws_iam_role_policy" "ooniprobe_role" { - name = "${local.name}-task-role" - role = module.ooniapi_cluster.container_host_role.name - - policy = local.ooniprobe_s3_policy -} - module "ooniapi_ooniprobe_deployer" { source = "../../modules/ooniapi_service_deployer" diff --git a/tf/environments/prod/main.tf b/tf/environments/prod/main.tf index 373c509e..a57e8767 100644 --- a/tf/environments/prod/main.tf +++ b/tf/environments/prod/main.tf @@ -870,15 +870,6 @@ locals { }) } -# Until the tasks use their task role, they get ooniprobe's S3 access through -# the container host's credentials -resource "aws_iam_role_policy" "ooniprobe_role" { - name = "${local.name}-task-role" - role = module.ooniapi_cluster.container_host_role.name - - policy = local.ooniprobe_s3_policy -} - module "ooniapi_ooniprobe_deployer" { source = "../../modules/ooniapi_service_deployer" diff --git a/tf/modules/ecs_cluster/main.tf b/tf/modules/ecs_cluster/main.tf index 19502f87..48b6ff02 100644 --- a/tf/modules/ecs_cluster/main.tf +++ b/tf/modules/ecs_cluster/main.tf @@ -169,6 +169,16 @@ resource "aws_launch_template" "container_host" { name = aws_iam_instance_profile.container_host.name } + # The instance credentials are for the host: the ECS agent and docker's + # log driver. Requiring IMDSv2 with a hop limit of 1 keeps them from the + # containers, which are a hop further away on the bridge network; tasks + # get credentials from their task role instead. + metadata_options { + http_endpoint = "enabled" + http_tokens = "required" + http_put_response_hop_limit = 1 + } + network_interfaces { associate_public_ip_address = true delete_on_termination = true diff --git a/tf/modules/ecs_cluster/templates/profile_policy.json b/tf/modules/ecs_cluster/templates/profile_policy.json index da305784..1f6c2eb9 100644 --- a/tf/modules/ecs_cluster/templates/profile_policy.json +++ b/tf/modules/ecs_cluster/templates/profile_policy.json @@ -41,21 +41,6 @@ "Action": ["logs:*", "cloudwatch:GenerateQuery"], "Resource": "*" }, - { - "Effect": "Allow", - "Action": [ - "secretsmanager:GetResourcePolicy", - "secretsmanager:GetSecretValue", - "secretsmanager:DescribeSecret", - "secretsmanager:ListSecretVersionIds" - ], - "Resource": "*" - }, - { - "Effect": "Allow", - "Action": "secretsmanager:ListSecrets", - "Resource": "*" - }, { "Effect": "Allow", "Action": [ From 7afd9da11f6891aefa79576611846df06733b7a2 Mon Sep 17 00:00:00 2001 From: Aaron Gibson Date: Sun, 4 Oct 2026 06:43:50 +0000 Subject: [PATCH 4/4] Limit the CodeBuild roles' Secrets Manager access to the Docker Hub token The build roles of ooniapi_service_deployer, ooni_docker_build and oonith_service_deployer had the AWS managed SecretsManagerReadWrite policy. Every buildspec they run (the API services, fastpath, testlists and the reuploader, on master and on the branches dev builds) reads one secret, oonidevops/dockerhub/access_token, to log in to Docker Hub. The roles now get secretsmanager:GetSecretValue on that secret only (its ARN plus the 6 character suffix Secrets Manager adds) instead of the managed policy. The make db-migration targets that read the Postgres URL run on an operator's machine, not in CodeBuild. Planned ooniapi_service_deployer from main and from this change with fake inputs: the same resources, and the build policy differs by that one statement. terraform validate passes for dev and prod. --- tf/modules/ooni_docker_build/main.tf | 11 +++++++++-- tf/modules/ooniapi_service_deployer/main.tf | 11 +++++++++-- tf/modules/oonith_service_deployer/main.tf | 9 ++++++++- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/tf/modules/ooni_docker_build/main.tf b/tf/modules/ooni_docker_build/main.tf index a3f121d5..e35db7cc 100644 --- a/tf/modules/ooni_docker_build/main.tf +++ b/tf/modules/ooni_docker_build/main.tf @@ -4,7 +4,10 @@ data "aws_caller_identity" "current" {} locals { account_id = data.aws_caller_identity.current.account_id - env_label = var.environment == "prod" ? "latest" : "dev" + # the only secret the builds read (buildspec.yml DOCKERHUB_SECRET_ID), to + # log in to Docker Hub + dockerhub_secret_name = "oonidevops/dockerhub/access_token" + env_label = var.environment == "prod" ? "latest" : "dev" } resource "aws_iam_policy" "codebuild" { @@ -63,6 +66,11 @@ resource "aws_iam_policy" "codebuild" { "arn:aws:codebuild:${var.aws_region}:${local.account_id}:report-group/oonidkr-${var.service_name}-*" ] }, + { + "Effect": "Allow", + "Action": "secretsmanager:GetSecretValue", + "Resource": "arn:aws:secretsmanager:${var.aws_region}:${local.account_id}:secret:${local.dockerhub_secret_name}-??????" + }, { "Effect": "Allow", "Action": "codestar-connections:UseConnection", @@ -93,7 +101,6 @@ POLICY managed_policy_arns = [ aws_iam_policy.codebuild.arn, "arn:aws:iam::aws:policy/AmazonEC2ContainerRegistryFullAccess", - "arn:aws:iam::aws:policy/SecretsManagerReadWrite" ] max_session_duration = "3600" name = "codebuild-oonidkr-${var.service_name}" diff --git a/tf/modules/ooniapi_service_deployer/main.tf b/tf/modules/ooniapi_service_deployer/main.tf index fb32fefd..bf780692 100755 --- a/tf/modules/ooniapi_service_deployer/main.tf +++ b/tf/modules/ooniapi_service_deployer/main.tf @@ -4,7 +4,10 @@ data "aws_caller_identity" "current" {} locals { account_id = data.aws_caller_identity.current.account_id - env_label = var.environment == "prod" ? "latest" : "dev" + # the only secret the builds read (buildspec.yml DOCKERHUB_SECRET_ID), to + # log in to Docker Hub + dockerhub_secret_name = "oonidevops/dockerhub/access_token" + env_label = var.environment == "prod" ? "latest" : "dev" } resource "aws_iam_policy" "codebuild" { @@ -63,6 +66,11 @@ resource "aws_iam_policy" "codebuild" { "arn:aws:codebuild:${var.aws_region}:${local.account_id}:report-group/ooniapi-${var.service_name}-*" ] }, + { + "Effect": "Allow", + "Action": "secretsmanager:GetSecretValue", + "Resource": "arn:aws:secretsmanager:${var.aws_region}:${local.account_id}:secret:${local.dockerhub_secret_name}-??????" + }, { "Effect": "Allow", "Action": "codestar-connections:UseConnection", @@ -93,7 +101,6 @@ POLICY managed_policy_arns = [ aws_iam_policy.codebuild.arn, "arn:aws:iam::aws:policy/AmazonEC2ContainerRegistryFullAccess", - "arn:aws:iam::aws:policy/SecretsManagerReadWrite" ] max_session_duration = "3600" name = "codebuild-ooniapi-${var.service_name}" diff --git a/tf/modules/oonith_service_deployer/main.tf b/tf/modules/oonith_service_deployer/main.tf index 6a4db048..2ba76321 100644 --- a/tf/modules/oonith_service_deployer/main.tf +++ b/tf/modules/oonith_service_deployer/main.tf @@ -4,6 +4,9 @@ data "aws_caller_identity" "current" {} locals { account_id = data.aws_caller_identity.current.account_id + # the only secret the builds read (buildspec.yml DOCKERHUB_SECRET_ID), to + # log in to Docker Hub + dockerhub_secret_name = "oonidevops/dockerhub/access_token" } resource "aws_iam_policy" "codebuild" { @@ -62,6 +65,11 @@ resource "aws_iam_policy" "codebuild" { "arn:aws:codebuild:${var.aws_region}:${local.account_id}:report-group/oonith-${var.service_name}-*" ] }, + { + "Effect": "Allow", + "Action": "secretsmanager:GetSecretValue", + "Resource": "arn:aws:secretsmanager:${var.aws_region}:${local.account_id}:secret:${local.dockerhub_secret_name}-??????" + }, { "Effect": "Allow", "Action": "codestar-connections:UseConnection", @@ -92,7 +100,6 @@ POLICY managed_policy_arns = [ aws_iam_policy.codebuild.arn, "arn:aws:iam::aws:policy/AmazonEC2ContainerRegistryFullAccess", - "arn:aws:iam::aws:policy/SecretsManagerReadWrite" ] max_session_duration = "3600" name = "codebuild-oonith-${var.service_name}"