Skip to content

Commit

Permalink
feat(terraform): improve CKV_AWS_358 OIDC claims analysis for GitHub
Browse files Browse the repository at this point in the history
  • Loading branch information
@aviadhahami
aviadhahami committed Jan 19, 2025
1 parent 5e366b0 commit 287f382
Show file tree
Hide file tree
Showing 3 changed files with 165 additions and 22 deletions.
73 changes: 53 additions & 20 deletions checkov/terraform/checks/data/aws/GithubActionsOIDCTrustPolicy.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,54 +4,87 @@
from checkov.common.util.type_forcers import force_list
from checkov.terraform.checks.data.base_check import BaseDataCheck

gh_repo_regex = re.compile(r'repo:[^/]+/[^/]+')
gh_repo_regex = re.compile(r"[\w]+/.+")
gh_abusable_claims = ["workflow", "environment", "ref", "context", "head_ref", "base_ref"]


class GithubActionsOIDCTrustPolicy(BaseDataCheck):
def __init__(self):
name = 'Ensure GitHub Actions OIDC trust policies only allows actions from a specific known organization'
name = "Ensure GitHub Actions OIDC authorization policies only allows safe claims and claim order"
id = "CKV_AWS_358"
supported_data = ("aws_iam_policy_document",)
categories = [CheckCategories.IAM]
super().__init__(name=name, id=id, categories=categories, supported_data=supported_data)

def scan_data_conf(self, conf: Dict[str, List[Any]]) -> CheckResult:
statements = force_list(conf.get('statement'))
statements = force_list(conf.get("statement"))
for statement in statements:
found_federated_gh_oidc = False
if isinstance(statement, dict):
if statement.get('principals'):
principals = statement['principals']
if statement.get("principals"):
principals = statement["principals"]
for principal in force_list(principals):
if 'type' not in principal and 'identifiers' not in principal:
if "type" not in principal and "identifiers" not in principal:
continue
principal_type = principal['type']
principal_identifiers = principal['identifiers']
if isinstance(principal_type, list) and len(
principal_type) and 'Federated' in principal_type and isinstance(principal_identifiers,
list):
principal_type = principal["type"]
principal_identifiers = principal["identifiers"]
if (
isinstance(principal_type, list)
and len(principal_type)
and "Federated" in principal_type
and isinstance(principal_identifiers, list)
):
for identifier in principal_identifiers:
if isinstance(identifier,
list) and identifier[0] is not None and \
'oidc-provider/token.actions.githubusercontent.com' in identifier[0]:
if (
isinstance(identifier, list)
and identifier[0] is not None
and "oidc-provider/token.actions.githubusercontent.com" in identifier[0]
):
found_federated_gh_oidc = True
break
if not found_federated_gh_oidc:
return CheckResult.PASSED
if found_federated_gh_oidc and not statement.get('condition'):
if found_federated_gh_oidc and not statement.get("condition"):
return CheckResult.FAILED
found_sub_condition_variable = False
found_sub_condition_value = False
for condition in statement.get('condition'):
condition_variables = condition.get('variable')
condition_values = condition.get('values')
for condition in statement.get("condition"):
condition_variables = condition.get("variable")
condition_values = condition.get("values")
if isinstance(condition_variables, list):
for condition_variable in condition_variables:
if condition_variable == 'token.actions.githubusercontent.com:sub':
if condition_variable == "token.actions.githubusercontent.com:sub":
found_sub_condition_variable = True
break
for condition_value in condition_values:
if isinstance(condition_value, list) and gh_repo_regex.search(condition_value[0]):
if isinstance(condition_value, list):
# First -> check if the value is a mere wildcard. If so, it's a fail
# This covers the case where the condition is ['sub':'*']
if len(condition_value) == 1 and condition_value[0] == "*":
print(f"I Failed! --> {condition_value}")
return CheckResult.FAILED
# Split the claims by ':' for deeper inspection
split_claims = condition_value[0].split(":")
# The assertion MUST be of the form ['{claim_name_1}:{claim_value_1}:{claim_name_2}:{claim_value_2}...']
# If the length of the split claims is 1, it means that the assertion is ['sub':'{claim_name}'] - this is a fail
if len(split_claims) == 1:
print(f"I Failed! --> {condition_value}")
return CheckResult.FAILED
# Second -> Check if the value is a wildcard assertion
# This covers the case where the condition is ['sub':'{claim_name}:*']
if split_claims[1] == "*":
print(f"I Failed! --> {condition_value}")
return CheckResult.FAILED
# Third -> Check if the value is an abusable claim
# This covers the case where the condition is ['sub':'{abusable_claim}:{any_value}']
for abusable_claim in gh_abusable_claims:
if split_claims[0].startswith(abusable_claim):
print(f"I Failed! --> {condition_value}")
return CheckResult.FAILED
# # Fourth -> Check if the value is a repo:org/* -> this is a pass with a warning
if split_claims[0] == "repo" and not gh_repo_regex.match(split_claims[1]):
print(f"I Failed! --> {condition_value}")
return CheckResult.FAILED
found_sub_condition_value = True
break
if found_sub_condition_value and found_sub_condition_variable:
Expand Down
106 changes: 106 additions & 0 deletions tests/terraform/checks/data/aws/example_GithubActionsOIDCTrustPolicy/main.tf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,109 @@ data "aws_iam_policy_document" "fail2" {
}
}
}

# fail for wildcard as condition
data "aws_iam_policy_document" "fail-wildcard" {
version = "2012-10-17"

statement {
effect = "Allow"
action = [
"sts:AssumeRoleWithWebIdentity"
]
principals {
identifiers = ["arn:aws:iam::123456123456:oidc-provider/token.actions.githubusercontent.com"]
type = "Federated"
}

condition {
test = "StringEquals"
values = ["*"]
variable = "token.actions.githubusercontent.com:sub"
}
}
}
# fail for abusable value as condition
data "aws_iam_policy_document" "fail-abusable" {
version = "2012-10-17"

statement {
effect = "Allow"
action = [
"sts:AssumeRoleWithWebIdentity"
]
principals {
identifiers = ["arn:aws:iam::123456123456:oidc-provider/token.actions.githubusercontent.com"]
type = "Federated"
}

condition {
test = "StringEquals"
values = ["workflow:github-actions:repo:myOrg/myRepo:ref:refs/heads/MyBranch"]
variable = "token.actions.githubusercontent.com:sub"
}
}
}
# fail for condition that asserts wildcard
data "aws_iam_policy_document" "fail-wildcard-assertion" {
version = "2012-10-17"

statement {
effect = "Allow"
action = [
"sts:AssumeRoleWithWebIdentity"
]
principals {
identifiers = ["arn:aws:iam::123456123456:oidc-provider/token.actions.githubusercontent.com"]
type = "Federated"
}

condition {
test = "StringEquals"
values = ["repo:*"]
variable = "token.actions.githubusercontent.com:sub"
}
}
}
# fail for misused "repo" condition
data "aws_iam_policy_document" "fail-misused-repo" {
version = "2012-10-17"

statement {
effect = "Allow"
action = [
"sts:AssumeRoleWithWebIdentity"
]
principals {
identifiers = ["arn:aws:iam::123456123456:oidc-provider/token.actions.githubusercontent.com"]
type = "Federated"
}

condition {
test = "StringEquals"
values = ["repo:myOrg*"]
variable = "token.actions.githubusercontent.com:sub"
}
}
}
# pass for org only "repo" condition
data "aws_iam_policy_document" "pass-org-only" {
version = "2012-10-17"

statement {
effect = "Allow"
action = [
"sts:AssumeRoleWithWebIdentity"
]
principals {
identifiers = ["arn:aws:iam::123456123456:oidc-provider/token.actions.githubusercontent.com"]
type = "Federated"
}

condition {
test = "StringEquals"
values = ["repo:myOrg/*"]
variable = "token.actions.githubusercontent.com:sub"
}
}
}
8 changes: 6 additions & 2 deletions tests/terraform/checks/data/aws/test_GithubActionsOIDCTrustPolicy.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,15 @@ def test(self):
'aws_iam_policy_document.pass1',
"aws_iam_policy_document.pass2",
"aws_iam_policy_document.pass3",

"aws_iam_policy_document.pass-org-only"
}
failing_resources = {
"aws_iam_policy_document.fail1",
"aws_iam_policy_document.fail2"
"aws_iam_policy_document.fail2",
"aws_iam_policy_document.fail-wildcard",
"aws_iam_policy_document.fail-abusable",
"aws_iam_policy_document.fail-wildcard-assertion",
"aws_iam_policy_document.fail-misused-repo"
}

passed_check_resources = set([c.resource for c in report.passed_checks])
Expand Down

0 comments on commit 287f382

Please sign in to comment.