Skip to content

Commit 4118662

Browse files
rudy-regazzoni-sonarsourcerudy-regazzoni-sonarsource
authored
SONARPY-1146 Rule S6329: Allowing public network access to cloud resources is security-sensitive - Implement for Instances related resources (#1246)
* SONARPY-1146 Rule S6329: Allowing public network access to cloud resources is security-sensitive - Implement for Instances related resources
1 parent f747164 commit 4118662

File tree

7 files changed

+302
-4
lines changed

7 files changed

+302
-4
lines changed

‎python-checks/src/main/java/org/sonar/python/checks/cdk/CdkPredicate.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,10 @@ public static Predicate<Expression> isListLiteral() {
122122
return expression -> expression.is(Tree.Kind.LIST_LITERAL);
123123
}
124124

125+
public static Predicate<Expression> isSubscriptionExpression() {
126+
return expression -> expression.is(Tree.Kind.SUBSCRIPTION);
127+
}
128+
125129
/**
126130
* @return Predicate which tests if expression is a string and starts with the expected value
127131
*/

‎python-checks/src/main/java/org/sonar/python/checks/cdk/PublicNetworkAccessToCloudResourcesCheck.java‎

Lines changed: 170 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,25 +19,43 @@
1919
*/
2020
package org.sonar.python.checks.cdk;
2121

22+
import java.util.Collections;
2223
import java.util.Optional;
2324
import java.util.Set;
2425
import java.util.function.Predicate;
2526
import org.sonar.check.Rule;
2627
import org.sonar.plugins.python.api.SubscriptionContext;
2728
import org.sonar.plugins.python.api.tree.CallExpression;
29+
import org.sonar.plugins.python.api.tree.Expression;
30+
import org.sonar.plugins.python.api.tree.ListLiteral;
31+
import org.sonar.plugins.python.api.tree.QualifiedExpression;
32+
import org.sonar.plugins.python.api.tree.SubscriptionExpression;
33+
import org.sonar.plugins.python.api.tree.Tree;
34+
import org.sonar.python.tree.TreeUtils;
2835

2936
import static org.sonar.python.checks.cdk.CdkPredicate.isCallExpression;
3037
import static org.sonar.python.checks.cdk.CdkPredicate.isFqn;
38+
import static org.sonar.python.checks.cdk.CdkPredicate.isFqnOf;
39+
import static org.sonar.python.checks.cdk.CdkPredicate.isListLiteral;
40+
import static org.sonar.python.checks.cdk.CdkPredicate.isSubscriptionExpression;
3141
import static org.sonar.python.checks.cdk.CdkPredicate.isTrue;
3242
import static org.sonar.python.checks.cdk.CdkUtils.ExpressionFlow;
3343
import static org.sonar.python.checks.cdk.CdkUtils.getArgument;
44+
import static org.sonar.python.checks.cdk.CdkUtils.getDictionary;
45+
import static org.sonar.python.checks.cdk.CdkUtils.getDictionaryPair;
3446

3547
@Rule(key = "S6329")
3648
public class PublicNetworkAccessToCloudResourcesCheck extends AbstractCdkResourceCheck {
49+
public static final String PUBLICLY_ACCESSIBLE_ARG_NAME = "publicly_accessible";
50+
private static final String SUBNET_TYPE = "subnet_type";
51+
private static final String ASSOCIATE_PUBLIC_IP_ADDRESS = "associate_public_ip_address";
3752

3853
private static final String MESSAGE = "Make sure allowing public network access is safe here.";
54+
55+
private static final String SENSITIVE_SUBNET = "aws_cdk.aws_ec2.SubnetType.PUBLIC";
3956
private static final Set<String> SAFE_SUBNET_TYPES = Set.of("ISOLATED", "PRIVATE_ISOLATED", "PRIVATE", "PRIVATE_WITH_NAT");
40-
public static final String PUBLICLY_ACCESSIBLE_ARG_NAME = "publicly_accessible";
57+
private static final Set<String> COMPLIANT_SUBNETS =
58+
Set.of("aws_cdk.aws_ec2.SubnetType.PRIVATE_ISOLATED", "aws_cdk.aws_ec2.SubnetType.PRIVATE_WITH_EGRESS", "aws_cdk.aws_ec2.SubnetType.PRIVATE_WITH_NAT");
4159

4260
@Override
4361
protected void registerFqnConsumer() {
@@ -55,6 +73,152 @@ protected void registerFqnConsumer() {
5573
argument -> argument.addIssueIf(isTrue(), MESSAGE)
5674
)
5775
);
76+
77+
checkFqn("aws_cdk.aws_ec2.Instance", PublicNetworkAccessToCloudResourcesCheck::checkInstance);
78+
checkFqn("aws_cdk.aws_ec2.CfnInstance", PublicNetworkAccessToCloudResourcesCheck::checkCfnInstance);
79+
}
80+
81+
/**
82+
* Check that a CallExpression (supposedly an aws_cdk.aws_ec2.Instance() call) has a sensitive 'vpc_subnets' argument
83+
* <pre>aws_cdk.aws_ec2.Instance(vpc_subnets=ec2.SubnetSelection(subnet_type=ec2.SubnetType.PUBLIC))</pre>
84+
*/
85+
private static void checkInstance(SubscriptionContext ctx, CallExpression callExpression) {
86+
getArgument(ctx, callExpression, "vpc_subnets")
87+
.ifPresent(flow -> {
88+
checkVpcSubnetAsSensitiveSubnetSelectionCall(ctx, flow);
89+
checkVpcSubnetAsSensitiveDictionary(ctx, flow);
90+
});
91+
}
92+
93+
/**
94+
* Check that the provided ExpressionFlow lead to a sensitive CallExpression of aws_cdk.aws_ec2.SubnetSelection
95+
* <pre>aws_cdk.aws_ec2.SubnetSelection(subnet_type=aws_cdk.aws_ec2.SubnetType.PUBLIC)</pre>
96+
*/
97+
private static void checkVpcSubnetAsSensitiveSubnetSelectionCall(SubscriptionContext ctx, CdkUtils.ExpressionFlow flow) {
98+
flow.getExpression(isFqn("aws_cdk.aws_ec2.SubnetSelection"))
99+
.filter(expression -> expression.is(Tree.Kind.CALL_EXPR)).map(CallExpression.class::cast)
100+
.ifPresent(callExpression ->
101+
getArgument(ctx, callExpression, SUBNET_TYPE)
102+
.flatMap(flowArg -> flowArg.getExpression(isFqn(SENSITIVE_SUBNET)))
103+
.ifPresent(expr -> ctx.addIssue(callExpression.parent(), MESSAGE)));
104+
}
105+
106+
/**
107+
* Check that the provided ExpressionFlow lead to a DictionaryLiteral with a sensitive 'subnet_type' attribute
108+
* <pre>{"subnet_type" : aws_cdk.aws_ec2.SubnetType.PUBLIC}}</pre>
109+
*/
110+
private static void checkVpcSubnetAsSensitiveDictionary(SubscriptionContext ctx, CdkUtils.ExpressionFlow flow) {
111+
getDictionary(flow)
112+
.flatMap(dictionary -> getDictionaryPair(ctx, dictionary, SUBNET_TYPE))
113+
.flatMap(element -> element.value.getExpression(isFqn(SENSITIVE_SUBNET)))
114+
.ifPresent(expression -> raiseIssueOnParent(ctx, expression, Tree.Kind.REGULAR_ARGUMENT));
115+
}
116+
117+
/**
118+
* Check that a CallExpression (supposedly an aws_cdk.aws_ec2.checkCfnInstance() call) has a sensitive 'network_interfaces' argument.
119+
* Also check if a valid and compliant subnet_id is provided, in which case this CallExpression is not considered as sensitive.
120+
* <pre>
121+
* aws_cdk.aws_ec2.CfnInstance(network_interfaces=[aws_cdk.aws_ec2.CfnInstance.NetworkInterfaceProperty(associate_public_ip_address=True)]) # Sensitive
122+
* aws_cdk.aws_ec2.CfnInstance(network_interfaces=[{"associate_public_ip_address" : True}]) # Sensitive
123+
* aws_cdk.aws_ec2.CfnInstance(network_interfaces=[aws_cdk.aws_ec2.CfnInstance.NetworkInterfaceProperty(associate_public_ip_address=True,
124+
* subnet_id=aws_cdk.aws_ec2.Vpc.select_subnets(subnet_type=aws_cdk.aws_ec2.SubnetType.PRIVATE_ISOLATED).subnet_ids[0])]) # Compliant
125+
* aws_cdk.aws_ec2.CfnInstance(network_interfaces=[{"associate_public_ip_address" : True,
126+
* "subnet_id" : aws_cdk.aws_ec2.Vpc.select_subnets(subnet_type=aws_cdk.aws_ec2.SubnetType.PRIVATE_ISOLATED).subnet_ids[0]}]) # Compliant
127+
* </pre>
128+
*/
129+
private static void checkCfnInstance(SubscriptionContext ctx, CallExpression callExpression) {
130+
getArgument(ctx, callExpression, "network_interfaces")
131+
.flatMap(flow -> flow.getExpression(isListLiteral())).map(ListLiteral.class::cast)
132+
.map(listLiteral -> listLiteral.elements().expressions())
133+
.orElse(Collections.emptyList())
134+
.forEach(expression -> {
135+
checkNetworkInterfacesCallExpression(ctx, expression);
136+
checkNetworkInterfacesDictionary(ctx, expression);
137+
});
138+
}
139+
140+
/**
141+
* Check that the provided ExpressionFlow lead to a sensitive CallExpression of aws_cdk.aws_ec2.CfnInstance.NetworkInterfaceProperty
142+
* <pre>
143+
* aws_cdk.aws_ec2.CfnInstance.NetworkInterfaceProperty(associate_public_ip_address=True) # Sensitive
144+
* aws_cdk.aws_ec2.CfnInstance.NetworkInterfaceProperty(associate_public_ip_address=True,
145+
* subnet_id=aws_cdk.aws_ec2.Vpc.select_subnets(subnet_type=aws_cdk.aws_ec2.SubnetType.PRIVATE_ISOLATED).subnet_ids[0])] # Compliant
146+
* </pre>
147+
*/
148+
private static void checkNetworkInterfacesCallExpression(SubscriptionContext ctx, Expression expression) {
149+
Optional.of(expression)
150+
.filter(isCallExpression()).map(CallExpression.class::cast)
151+
.filter(isFqn("aws_cdk.aws_ec2.CfnInstance.NetworkInterfaceProperty"))
152+
.ifPresent(call -> checkSensitiveOptionWithoutCompliantSubnetDefined(ctx, call));
153+
}
154+
155+
private static void checkSensitiveOptionWithoutCompliantSubnetDefined(SubscriptionContext ctx, CallExpression callExpression) {
156+
Optional<CdkUtils.ExpressionFlow> associatedPublicIpAddress = getArgument(ctx, callExpression, ASSOCIATE_PUBLIC_IP_ADDRESS);
157+
Optional<CdkUtils.ExpressionFlow> subnetId = getArgument(ctx, callExpression, "subnet_id");
158+
159+
if (associatedPublicIpAddress.filter(flow -> flow.hasExpression(isTrue())).isPresent()
160+
&& subnetId.filter(PublicNetworkAccessToCloudResourcesCheck::hasPrivateSubnetDefined).isEmpty()) {
161+
associatedPublicIpAddress.get().addIssue(MESSAGE);
162+
}
163+
}
164+
165+
/**
166+
* Check that the provided ExpressionFlow lead to a DictionaryLiteral with a sensitive associate_public_ip_address/subnet_id
167+
* <pre>
168+
* {"associate_public_ip_address" : True} # Sensitive
169+
* {"associate_public_ip_address" : True, "subnet_id" : ec2.Vpc.select_subnets(subnet_type=ec2.SubnetType.PRIVATE_ISOLATED).subnet_ids[0]} #Compliant
170+
* </pre>
171+
*/
172+
private static void checkNetworkInterfacesDictionary(SubscriptionContext ctx, Expression expression) {
173+
getDictionary(expression)
174+
.map(dictionaryLiteral -> UnrestrictedAdministrationCheckPartCfnSecurity.DictionaryAsMap.build(ctx, dictionaryLiteral))
175+
.ifPresent(dictionaryAsMap -> {
176+
if (dictionaryAsMap.hasKeyValuePair(ASSOCIATE_PUBLIC_IP_ADDRESS, isTrue())
177+
&& dictionaryAsMap.getValue("subnet_id").filter(PublicNetworkAccessToCloudResourcesCheck::hasPrivateSubnetDefined).isEmpty()) {
178+
dictionaryAsMap.getKeyString(ASSOCIATE_PUBLIC_IP_ADDRESS).ifPresent(expr -> raiseIssueOnParent(ctx, expr, Tree.Kind.KEY_VALUE_PAIR));
179+
}
180+
});
181+
}
182+
183+
/**
184+
* Check that the provided ExpressionFlow has a compliant (private subnet_type) CallExpression over aws_cdk.aws_ec2.Vpc.select_subnets() method.
185+
* <pre>aws_cdk.aws_ec2.Vpc.select_subnets(subnet_type=ec2.SubnetType.PRIVATE_ISOLATED).subnet_ids[0]</pre>
186+
*/
187+
private static boolean hasPrivateSubnetDefined(CdkUtils.ExpressionFlow subnetId) {
188+
return subnetId.getExpression(isSubscriptionExpression())
189+
.map(SubscriptionExpression.class::cast).map(SubscriptionExpression::object)
190+
.filter(PublicNetworkAccessToCloudResourcesCheck::isCompliantSubnet)
191+
.isPresent();
192+
}
193+
194+
private static boolean isCompliantSubnet(Expression expression) {
195+
Optional<CallExpression> callExpression = getCallSelectSubnets(expression);
196+
197+
return callExpression
198+
.flatMap(call -> getArgument(null, call, SUBNET_TYPE))
199+
.filter(flow -> flow.hasExpression(isFqnOf(COMPLIANT_SUBNETS)))
200+
.isPresent();
201+
}
202+
203+
private static Optional<CallExpression> getCallSelectSubnets(Expression expression) {
204+
if (expression.is(Tree.Kind.QUALIFIED_EXPR)) {
205+
Expression qualifier = ((QualifiedExpression) expression).qualifier();
206+
CdkUtils.ExpressionFlow flow = CdkUtils.ExpressionFlow.build(null, qualifier);
207+
return flow.getExpression(isCallExpression().and(isFqn("aws_cdk.aws_ec2.Vpc.select_subnets")))
208+
.map(CallExpression.class::cast);
209+
}
210+
if (expression.is(Tree.Kind.NAME)) {
211+
CdkUtils.ExpressionFlow flow = CdkUtils.ExpressionFlow.build(null, expression);
212+
return flow.getExpression(isQualifiedExpression()).map(QualifiedExpression.class::cast)
213+
.map(qualifiedExpression -> CdkUtils.ExpressionFlow.build(null, qualifiedExpression.qualifier()))
214+
.flatMap(flow2 -> flow2.getExpression(isCallExpression().and(isFqn("aws_cdk.aws_ec2.Vpc.select_subnets"))))
215+
.map(CallExpression.class::cast);
216+
}
217+
return Optional.empty();
218+
}
219+
220+
private static void raiseIssueOnParent(SubscriptionContext ctx, Expression expression, Tree.Kind kind) {
221+
ctx.addIssue(Optional.ofNullable(TreeUtils.firstAncestorOfKind(expression, kind)).orElse(expression), MESSAGE);
58222
}
59223

60224
private static void checkDatabaseInstance(SubscriptionContext ctx, CallExpression call) {
@@ -63,7 +227,7 @@ private static void checkDatabaseInstance(SubscriptionContext ctx, CallExpressio
63227
Optional<ExpressionFlow> subnetType = vpcSubnets
64228
.flatMap(flow -> flow.getExpression(isCallExpression().and(isFqn("aws_cdk.aws_ec2.SubnetSelection"))))
65229
.map(CallExpression.class::cast)
66-
.flatMap(subnetSelection -> getArgument(ctx, subnetSelection, "subnet_type"));
230+
.flatMap(subnetSelection -> getArgument(ctx, subnetSelection, SUBNET_TYPE));
67231

68232
if (subnetType.filter(isSafeSubnetSelection()).isPresent()) {
69233
return;
@@ -86,7 +250,10 @@ private static Predicate<ExpressionFlow> isSafeSubnetSelection() {
86250
}
87251

88252
private static Predicate<ExpressionFlow> isPublicSubnetSelection() {
89-
return subnetType -> subnetType.hasExpression(isFqn("aws_cdk.aws_ec2.SubnetType.PUBLIC"));
253+
return subnetType -> subnetType.hasExpression(isFqn(SENSITIVE_SUBNET));
90254
}
91255

256+
private static Predicate<Expression> isQualifiedExpression() {
257+
return expression -> expression.is(Tree.Kind.QUALIFIED_EXPR);
258+
}
92259
}

‎python-checks/src/main/java/org/sonar/python/checks/cdk/UnrestrictedAdministrationCheckPartCfnSecurity.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,15 @@ public Optional<Expression> get(String key, Predicate<Expression> valuePredicate
158158
return map.get(key).value.getExpression(valuePredicate);
159159
}
160160

161+
public Optional<Expression> getKeyString(String key) {
162+
return Optional.ofNullable(map.get(key))
163+
.flatMap(keyValuePair -> keyValuePair.key.getExpression(isStringLiteral()));
164+
}
165+
166+
public Optional<CdkUtils.ExpressionFlow> getValue(String key) {
167+
return Optional.ofNullable(map.get(key)).map(keyValuePair -> keyValuePair.value);
168+
}
169+
161170
public void addIssue(String key, String message) {
162171
if (map.containsKey(key)) {
163172
map.get(key).value.addIssue(message);

‎python-checks/src/test/java/org/sonar/python/checks/cdk/PublicNetworkAccessToCloudResourcesCheckTest.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,9 @@ public void testCfnMethod() {
3636
public void testDatabaseInstance() {
3737
PythonCheckVerifier.verify("src/test/resources/checks/cdk/publicNetworkAccessDatabaseInstance.py", check);
3838
}
39+
40+
@Test
41+
public void testInstance() {
42+
PythonCheckVerifier.verify("src/test/resources/checks/cdk/publicNetworkAccessDatabaseMigration_Instance.py", check);
43+
}
3944
}

0 commit comments

Comments
 (0)