feat: NZIM-v36-1022-20 NagPack #1068

mrpackethead · 2022-11-01T08:25:58Z

dontirun

Did an initial review. Thanks for putting this together @mrpackethead! We'll want to wait for the conformance pack to actually get released before we release this so that we can confirm that we aren't missing anything. That being said, doesn't hurt to get the review started early!

Some additional comments for things that are currently missing for release

The included and excluded rules for the pack listed in the [RULES(https://github.com/cdklabs/cdk-nag/blob/main/RULES.md) file
A Pack Test. This is important so that we don't inadvertently 'lose' rules in the case of internal refactoring.
If we need to write any new rules (which we may), the rules will also need their own unit tests as well

Any new rules that we write

src/packs/nzism-v36-1022-20.ts

mrpackethead · 2022-11-01T23:57:23Z

On the change to the Rules.md file, i can do some of the work now, and then come back and add the Links once the docs are published.

mrpackethead · 2022-11-02T03:47:39Z

@dontirun , I've added a couple of sample lines to the the RULES.md, Before i do all of them, can we check that the sample lines are in the right format?

nzismrule.md

.projenrc.js

mrpackethead · 2022-11-03T22:28:55Z

It looks like there are seven new rules to create. Some of these will be easy, others less so.

A temporary file ( nzismRulestoCreate )

cloudfront-default-root-object-configured
~~ebs-snapshot-public-restorable-check~~ ( syou can't create a snapshot in CloudFormation)
~~ec2-imdsv2-check~~ now included.
~~ec2-instance-managed-by-systems-manager~~ (that's something that would have to be checked in Systems Manager after the instance is launched)
~~elb-custom-security-policy-ssl-check~~ Already done via ELBTlsHttpsListenersOnly.ts)
~~lambda-function-public-access-prohibited~~ @dontirun working on this, via #1072
~~vpc-sg-open-only-to-authorized-ports~~ now done

mrpackethead · 2022-11-04T05:33:44Z

EC2SecurityGroupOnlyTcp443 checks security groups to see if they only have tcp 443 open

dontirun · 2022-11-04T13:39:52Z

EC2SecurityGroupOnlyTcp443 checks security groups to see if they only have tcp 443 open

Is this an implementation of
vpc-sg-open-only-to-authorized-ports?

I don't think we should be implementing this rule since it's highly dependent on business logic.

The config rule takes in an input list because it is supposed to be tuned to business logic

mrpackethead · 2022-11-04T18:38:34Z

EC2SecurityGroupOnlyTcp443 checks security groups to see if they only have tcp 443 open

Is this an implementation of vpc-sg-open-only-to-authorized-ports?

I don't think we should be implementing this rule since it's highly dependent on business logic.

The config rule takes in an input list because it is supposed to be tuned to business logic

The nzism specifically calls out that only tcp/443 shoudl be open.

    Controls: ['3205']
    Type: AWS::Config::ConfigRule
    Properties:
      ConfigRuleName: vpc-sg-open-only-to-authorized-ports
      InputParameters:
        authorizedTcpPorts: '443'
      Scope:
        ComplianceResourceTypes:
        - AWS::EC2::SecurityGroup
      Source:
        Owner: AWS
        SourceIdentifier: VPC_SG_OPEN_ONLY_TO_AUTHORIZED_PORTS
      Description: "SHOULD 18.1.13.C.02[CID:3205]| Network security/Network Management/Limiting network access"```

mrpackethead · 2022-11-06T09:03:08Z

Have added rules for

cloudfront-default-root-object-configured
ec2-imdsv2-check

and added them to the nzism pak.

included lambda-function-public-access-prohibited from the rule that was recently added.

Excluded the other rules, as they are not resolvable at synth time.

To do, Hopefully will get an answer about the tcp443 inbound security group rule, and then will be able to decide what we will do.

dontirun

I'll do a view passes for reviews, but here are some initial comments

README.md

RULES.md

src/rules/ec2/EC2IMDSv2.ts

test/Packs.test.ts

test/rules/EC2.test.ts

mrpackethead · 2022-11-07T19:21:45Z

Tests for EC2IMDv2 are not yet completed.

mrpackethead · 2022-11-13T04:48:05Z

@dontirun, I think this is ready to go now.

dontirun

Focused primarily on the EC2IMDSv2 rule. Will review other rules later

src/rules/ec2/EC2IMDSv2.ts

dontirun · 2022-11-14T21:12:56Z

src/rules/ec2/EC2IMDSv2.ts

+function hasHttpTokens(node: CfnLaunchTemplate): boolean {
+  const launchTemplateData: CfnLaunchTemplate.LaunchTemplateDataProperty =
+    Stack.of(node).resolve(node.launchTemplateData);
+  const meta = Stack.of(node).resolve(
+    launchTemplateData.metadataOptions
+  ) as CfnLaunchTemplate.MetadataOptionsProperty;
+
+  if (meta == undefined) {
+    return false;
+  }
+
+  if (meta.httpTokens === 'required') {
+    return true;
+  }
+  return false;
+}
+
+function launchConfigurationhasTokens(node: CfnLaunchConfiguration): boolean {
+  if (node.metadataOptions != undefined) {
+    const meta: CfnLaunchTemplate.MetadataOptionsProperty = Stack.of(
+      node
+    ).resolve(
+      node.metadataOptions
+    ) as CfnLaunchTemplate.MetadataOptionsProperty;
+
+    if (meta.httpTokens === 'required') {
+      return true;
+    }
+  }
+  return false;
+}


This can be simplified

Suggested change

function hasHttpTokens(node: CfnLaunchTemplate): boolean {

const launchTemplateData: CfnLaunchTemplate.LaunchTemplateDataProperty =

Stack.of(node).resolve(node.launchTemplateData);

const meta = Stack.of(node).resolve(

launchTemplateData.metadataOptions

) as CfnLaunchTemplate.MetadataOptionsProperty;

if (meta == undefined) {

return false;

}

if (meta.httpTokens === 'required') {

return true;

}

return false;

}

function launchConfigurationhasTokens(node: CfnLaunchConfiguration): boolean {

if (node.metadataOptions != undefined) {

const meta: CfnLaunchTemplate.MetadataOptionsProperty = Stack.of(

node

).resolve(

node.metadataOptions

) as CfnLaunchTemplate.MetadataOptionsProperty;

if (meta.httpTokens === 'required') {

return true;

}

}

return false;

}

function hasHttpTokens(node: CfnLaunchTemplate | CfnLaunchConfiguration): boolean {

const metaLocation = node instanceof CfnLaunchTemplate ? Stack.of(node).resolve(node.launchTemplateData) : node;

const meta = Stack.of(node).resolve(metaLocation.metadataOptions)

return Stack.of(node).resolve(meta?.httpTokens) === 'required'

}

dontirun · 2022-11-14T21:13:21Z

src/rules/ec2/EC2IMDSv2.ts

+        for (const child of Stack.of(node).node.findAll()) {
+          if (child instanceof CfnLaunchConfiguration) {
+            if (
+              child.launchConfigurationName === nodeLaunchConfigurationName &&


This isn't accounting for the case where the user provides a Ref for the the Launch Configuration Name, the name is also not a required field on a launch configuration.

I propose the following

function isMatchingLaunchConfiguration( node: CfnLaunchConfiguration, launchConfigurationName: string, ): boolean { const resolvedTemplateName = NagRules.resolveResourceFromInstrinsic(node, launchConfigurationName) return resolvedTemplateName === node.launchConfigurationName || resolvedTemplateName === resolvedTemplateName }

src/rules/ec2/EC2IMDSv2.ts

README.md

src/rules/ec2/SampleEC2IMDSv2.txt

dontirun · 2022-11-14T21:18:10Z

.github/workflows/build.yml

@@ -161,34 +161,6 @@ jobs:
        run: cd .repo && npx projen package:python


Run npx projen locally (don't need to run the entire build process) and commit the change so that these build files return to normal

src/rules/ec2/EC2IMDSv2.ts

chindaws · 2023-05-21T21:26:07Z

Andrew / Arun - Is there an ETA on when this PR may be finalised? I have a couple of organizations interested in using this feature.

dontirun · 2023-05-22T11:54:38Z

@chindaws There is no ETA. This is a community led contribution by @mrpackethead.

I'll give @mrpackethead a chance to respond, but if this is something you would like and @mrpackethead is no longer working on this then I would say that this PR is available for someone else to pick up and continue working on

mrpackethead · 2023-07-13T09:16:01Z

Andrew / Arun - Is there an ETA on when this PR may be finalised? I have a couple of organizations interested in using this feature.

@chindaws , get in touch with me if theres a reason to progress this. I could be convinced to do it if there was a need.

dontirun changed the title ~~added nzism-v36-1022-20 nag pak~~ feat: NZIM-v36-1022-20 NagPack Nov 1, 2022

dontirun reviewed Nov 1, 2022

View reviewed changes

dontirun reviewed Nov 2, 2022

View reviewed changes

nzismrule.md Outdated Show resolved Hide resolved

nzismrule.md Outdated Show resolved Hide resolved

nzismrule.md Outdated Show resolved Hide resolved

.projenrc.js Outdated Show resolved Hide resolved

dontirun requested changes Nov 6, 2022

View reviewed changes

dontirun requested changes Nov 14, 2022

View reviewed changes

mrpackethead closed this Sep 3, 2023

mrpackethead force-pushed the main branch from 09e7ace to 51a9e53 Compare September 3, 2023 22:16

dontirun mentioned this pull request Sep 4, 2023

feat: Add NZISM3.6 #1431

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: NZIM-v36-1022-20 NagPack #1068

feat: NZIM-v36-1022-20 NagPack #1068

mrpackethead commented Nov 1, 2022

dontirun left a comment

mrpackethead commented Nov 1, 2022

mrpackethead commented Nov 2, 2022

mrpackethead commented Nov 3, 2022 •

edited

Loading

mrpackethead commented Nov 4, 2022

dontirun commented Nov 4, 2022

mrpackethead commented Nov 4, 2022

mrpackethead commented Nov 6, 2022

dontirun left a comment

mrpackethead commented Nov 7, 2022

mrpackethead commented Nov 13, 2022

dontirun left a comment

dontirun Nov 14, 2022 •

edited

Loading

dontirun Nov 14, 2022

dontirun Nov 14, 2022

chindaws commented May 21, 2023

dontirun commented May 22, 2023

mrpackethead commented Jul 13, 2023

		@@ -161,34 +161,6 @@ jobs:
		run: cd .repo && npx projen package:python

feat: NZIM-v36-1022-20 NagPack #1068

feat: NZIM-v36-1022-20 NagPack #1068

Conversation

mrpackethead commented Nov 1, 2022

dontirun left a comment

Choose a reason for hiding this comment

mrpackethead commented Nov 1, 2022

mrpackethead commented Nov 2, 2022

mrpackethead commented Nov 3, 2022 • edited Loading

mrpackethead commented Nov 4, 2022

dontirun commented Nov 4, 2022

mrpackethead commented Nov 4, 2022

mrpackethead commented Nov 6, 2022

dontirun left a comment

Choose a reason for hiding this comment

mrpackethead commented Nov 7, 2022

mrpackethead commented Nov 13, 2022

dontirun left a comment

Choose a reason for hiding this comment

dontirun Nov 14, 2022 • edited Loading

Choose a reason for hiding this comment

dontirun Nov 14, 2022

Choose a reason for hiding this comment

dontirun Nov 14, 2022

Choose a reason for hiding this comment

chindaws commented May 21, 2023

dontirun commented May 22, 2023

mrpackethead commented Jul 13, 2023

mrpackethead commented Nov 3, 2022 •

edited

Loading

dontirun Nov 14, 2022 •

edited

Loading