Skip to content

Commit

Permalink
fix: S3 scan object role assume (#170)
Browse files Browse the repository at this point in the history
1. Update the S3 scan object lambda to properly handle the
assumed role's temporary credentials.

2. Update Scan API lambda IAM policy to allow publishing
to the SNS topic.
  • Loading branch information
patheard authored Jul 7, 2022
1 parent f67a2a2 commit b91eacc
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 11 deletions.
5 changes: 2 additions & 3 deletions api/clamav_scanner/scan.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from .common import AV_SIGNATURE_UNKNOWN
from .common import CLAMAV_LAMBDA_SCAN_TASK_NAME

from boto3wrapper.wrapper import get_session, get_credentials
from boto3wrapper.wrapper import get_session
from clamav_scanner.clamav import determine_verdict, scan_file
from database.db import get_db_session
from logger import log
Expand Down Expand Up @@ -74,8 +74,7 @@ def launch_scan(
if session is None:
session = next(get_db_session())

credentials = get_credentials(aws_account)
sns_client = get_session(credentials).client("sns", endpoint_url=AWS_ENDPOINT_URL)
sns_client = get_session().client("sns", endpoint_url=AWS_ENDPOINT_URL)

scan = session.query(Scan).filter(Scan.id == scan_id).one_or_none()
try:
Expand Down
7 changes: 6 additions & 1 deletion module/s3-scan-object/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,12 @@ const getRoleCredentials = async (stsClient, roleArn) => {
try {
const command = new AssumeRoleCommand({ RoleArn: roleArn, RoleSessionName: "s3-scan-object" });
const response = await stsClient.send(command);
credentials = response.Credentials;
// Submitted, without comment or judgement
credentials = {
accessKeyId: response.Credentials.AccessKeyId,
secretAccessKey: response.Credentials.SecretAccessKey,
sessionToken: response.Credentials.SessionToken,
};
} catch (error) {
console.error(`Failed to assume role ${roleArn}: ${error}`);
}
Expand Down
15 changes: 13 additions & 2 deletions module/s3-scan-object/app.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -310,10 +310,21 @@ describe("getRecordEventSource", () => {

describe("getRoleCredentials", () => {
test("successfully assumes role", async () => {
mockSTSClient.on(AssumeRoleCommand).resolves({ Credentials: { foo: "bar" } });
const response = {
Credentials: {
AccessKeyId: "why",
SecretAccessKey: "aws",
SessionToken: "whyyyyy",
},
};
mockSTSClient.on(AssumeRoleCommand).resolves(response);
const credentials = await getRoleCredentials(mockSTSClient, "foo");

expect(credentials).toEqual({ foo: "bar" });
expect(credentials).toEqual({
accessKeyId: "why",
secretAccessKey: "aws",
sessionToken: "whyyyyy",
});
expect(mockSTSClient).toHaveReceivedNthCommandWith(1, AssumeRoleCommand, {
RoleArn: "foo",
RoleSessionName: "s3-scan-object",
Expand Down
3 changes: 2 additions & 1 deletion terragrunt/aws/api/secrets.tf
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
resource "aws_secretsmanager_secret" "api_auth_token" {
name = "/scan-files/api_auth_token"
name = "/scan-files/api_auth_token"
kms_key_id = "aws/secretsmanager"
tags = {
CostCentre = var.billing_code
Terraform = true
Expand Down
38 changes: 38 additions & 0 deletions terragrunt/aws/s3_scan_object/iam.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
resource "aws_iam_policy" "api_sns_publish" {
name = "${var.product_name}-sns-publish"
path = "/"
policy = data.aws_iam_policy_document.api_sns_publish.json

tags = {
CostCentre = var.billing_code
Terraform = true
}
}

data "aws_iam_policy_document" "api_sns_publish" {
statement {
effect = "Allow"
actions = [
"kms:Decrypt",
"kms:GenerateDataKey*",
]
resources = [
aws_kms_key.sns_lambda.arn
]
}

statement {
effect = "Allow"
actions = [
"sns:Publish",
]
resources = [
aws_sns_topic.scan_complete.arn
]
}
}

resource "aws_iam_role_policy_attachment" "api_sns_publish" {
role = var.scan_files_api_function_role_name
policy_arn = aws_iam_policy.api_sns_publish.arn
}
5 changes: 5 additions & 0 deletions terragrunt/aws/s3_scan_object/inputs.tf
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
variable "scan_files_api_function_role_name" {
description = "Name of the Scan Files API function role"
type = string
}

variable "scan_files_api_function_role_arn" {
description = "ARN of the Scan Files API function role"
type = string
Expand Down
10 changes: 6 additions & 4 deletions terragrunt/env/production/s3_scan_object/terragrunt.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,16 @@ dependency "api" {
mock_outputs_allowed_terraform_commands = ["init", "fmt", "validate", "plan", "show"]
mock_outputs_merge_strategy_with_state = "shallow"
mock_outputs = {
function_role_arn = ""
scan_files_api_key_secret_arn = ""
function_name = ""
function_role_arn = ""
scan_files_api_key_secret_arn = ""
}
}

inputs = {
scan_files_api_function_role_arn = dependency.api.outputs.function_role_arn
scan_files_api_key_secret_arn = dependency.api.outputs.scan_files_api_key_secret_arn
scan_files_api_function_role_arn = dependency.api.outputs.function_role_arn
scan_files_api_function_role_name = dependency.api.outputs.function_name
scan_files_api_key_secret_arn = dependency.api.outputs.scan_files_api_key_secret_arn
}

include {
Expand Down

0 comments on commit b91eacc

Please sign in to comment.