[IAC-1866] CIS AWS v1.4 migration guide #512

marinalimeira · 2021-07-28T13:31:19Z

Preview: https://deploy-preview-512--keen-clarke-470db9.netlify.app/guides/upgrades/how-to-update-to-cis-14/

netlify · 2021-07-28T13:32:28Z

✔️ Deploy Preview for keen-clarke-470db9 ready!

🔨 Explore the source changes: 63bc2f7

🔍 Inspect the deploy log: https://app.netlify.com/sites/keen-clarke-470db9/deploys/6144698f75e8fb00076374f7

😎 Browse the preview: https://deploy-preview-512--keen-clarke-470db9.netlify.app

robmorgan

Adding feedback from the first pass of my PR review.

_posts/2021-07-28-how-to-update-to-cis-14.adoc

Co-authored-by: Rob Morgan <robbym@gmail.com>

_posts/2021-07-28-how-to-update-to-cis-14.adoc

ina-stoyanova

This looks really good so far! I've got pretty much 2 main notes, and everything else is really a NIT & safe to ignore:

I think in 1 place we mention that we've fixed a bug on top of the CIS 1.4 changes. I think this is good info, but probably to me it makes more sense that this lives in the release notes. We could alternatively have a section to mention explicitly what we've done in addition to CIS 1.4 within this migration guide. Thoughts?
We've no way to indicate that the migration guide finsihes after the Macie section. We could add a "summary" section reminding users about the previous versions of this guide (we've already got this, but maybe it could live at the end of the guide); or we could also use just a simple final sentence, e.g.: Woop! You're all done. Thanks for following this guide, please reach out to us if you've got any questions or feedback and we're happy to respond!

These notes are just my personal observations. Let me know what your thoughts are @marinalimeira @infraredgirl @robmorgan

_posts/2021-07-28-how-to-update-to-cis-14.adoc

infraredgirl

Looks great, thanks Marina! I left a couple of suggestions inline.

I'll work on adding the Macie stuff.

_posts/2021-07-28-how-to-update-to-cis-14.adoc

Co-authored-by: Ana Krivokapić <ana@gruntwork.io>

infraredgirl · 2021-09-01T17:29:56Z

A couple of issues I've spotted with the current state of the PR - curious to hear everyone's thoughts:

The "Manual steps" section comes before the "Deployment walkthrough" section, even though the manual steps (at least for Macie) need to be performed after deploying the module. I've called that out explicitly in the text, but I worry it still might be confusing. Should we rearrange these sections? Marina, what's the order of the manual step vs deployment itself for the MFA stuff you wrote?
The previous upgrade guide (1.2 -> 1.3) links heavily to the "How to achieve compliance" guide, especially in the "Deployment walkthrough" sections. However, since we change the compliance guide often, some of the sections that the upgrade guide links to no longer exist. I'm not sure if we're ok with that? Maybe we are, as that upgrade guide is outdated anyway, but it makes it a pretty useless document, so I wonder if we should remove it altogether. Going forward, we should probably put the actual deployment walkthroughs in the upgrade guide, rather than link to the compliance guide. This is because the upgrade guide does not get changed much once it's published, and the compliance guide gets pretty much re-written with every new version of the Benchmark. For this reason I went ahead and put the Macie deployment steps directly in the upgrade guide (this PR).

marinalimeira · 2021-09-08T14:40:33Z

The "Manual steps" section comes before the "Deployment walkthrough" section, even though the manual steps (at least for Macie) need to be performed after deploying the module. I've called that out explicitly in the text, but I worry it still might be confusing. Should we rearrange these sections? Marina, what's the order of the manual step vs deployment itself for the MFA stuff you wrote?

Ah, I missed this message. Totally agreed! I moved the sections earlier.

@infraredgirl I am running the Macie manual steps and it mentions that:

Under "Choose a bucket", select your bucket. This can be either one you already have, or the one that the macie module created. You will use the same bucket for every region.

The default for this variable is false, so I couldn't see a bucket created by macie. This applies for the macie module and the account-baseline-app.

Under "KMS key alias" select your KMS key. This can be either one you had already have, or the one that the macie module created. You will use the same key for every region.

The default for this variable is also false. This applies for the macie module and the account-baseline-app.

We can update these instructions by:

Updating the text, adding "you can create these by using the variable create_macie_bucket and create_kms_key."
Removing the text where it mentions that "one that the macie module created."
Update the default for both variables to true.

What do you think?

infraredgirl · 2021-09-08T15:35:34Z

Under "Choose a bucket", select your bucket. This can be either one you already have, or the one that the macie module created. You will use the same bucket for every region.

The default for this variable is false, so I couldn't see a bucket created by macie. This applies for the macie module and the account-baseline-app.

Under "KMS key alias" select your KMS key. This can be either one you had already have, or the one that the macie module created. You will use the same key for every region.

The default for this variable is also false. This applies for the macie module and the account-baseline-app.

We can update these instructions by:

Updating the text, adding "you can create these by using the variable create_macie_bucket and create_kms_key."

Removing the text where it mentions that "one that the macie module created."

Update the default for both variables to true.

What do you think?

Good point! Assuming that the create_* variables should remain defaulted to false (if anyone thinks that's wrong, LMK!), I'll fix the text with the option 1 you proposed.

ina-stoyanova · 2021-09-10T12:58:44Z

Good point! Assuming that the create_* variables should remain defaulted to false (if anyone thinks that's wrong, LMK!), I'll fix the text with the option 1 you proposed.

Oh, that's great! I hadn't checked the latest changes, and was here to update it exactly in the way both of you have done already! Nicee!!

ina-stoyanova · 2021-09-10T13:22:54Z

Here's a couple of thoughts:

I think we should also include the terragrunt configuration for the modules here instead of just terraform, since it's probably how users will have their CIS setup. Our CIS deployment guide (the big overall one) has examples with terragrunt and our code examples in /for-production are also terragrunt.
- For Macie, for example I've had to do the following to my terragrunt.hcl file:

  create_macie_bucket = true
  macie_bucket_name = "macie-bucket-test-ina"
  macie_create_kms_key = true
  macie_kms_key_name = "macie_kms_key_test_ina"
  macie_kms_key_users = ["arn:aws:iam::${local.accounts.security}:root","arn:aws:iam::${local.accounts.security}:user/alice"]
  macie_buckets_to_analyze = {
    "eu-west-1" : ["testee1234"],
  }
  force_destroy_macie_bucket = true

I'm thinking it would be useful to have the steps between v0.22.0 and v0.27.0 documented explicitly. For example, we all had to do the following when migrating the versions:
- update the source to be v0.23.0
- run the migrate_to_0_23.sh script
- update the source to be v0.25.0
  - add the multi-region providers & modules logic
- update the source to be v0.27.0
- follow the manual steps we've got in the guide

v0.22.1: Nothing to do
v0.22.2: Nothing to do
v0.23.0: Already ran the script, so good to go
v0.23.1: Nothing todo
v0.23.2: Nothing todo
v0.23.3: Nothing to do
v0.23.4: Nothing to do
v0.23.5: Nothing to do
v0.23.6: Nothing to do
v0.24.0: This release introduces MFA Delete, this is covered at the manual steps section
v0.24.1: Nothing to do
v0.25.0: Already added the multi region modules
v0.25.1: Introduces Amazon Macie, this is covered at the manual steps section  
v0.26.0: Nothing todo
v0.26.1: Multi region Macie, nothing todo.
v0.26.2: LZ with Macie, nothing to do
v0.26.3: Nothing to do
v0.27.0: Updates on MFA Delete

…io.github.io into cis-14-migration

robmorgan · 2021-09-16T09:04:46Z

Ready for review.

Also, simplify Macie configuration changes.

Ready for review

brikis98

Overall, this LGTM! I left some NITs on things we could make clearer, but I don't think we have to block merging/releasing on those items. We could get the guide out, and then come back after to make most of these improvements.

brikis98 · 2021-09-17T10:42:33Z

_posts/2021-07-28-how-to-update-to-cis-14.adoc

+1. https://github.com/gruntwork-io/terraform-aws-cis-service-catalog/releases/tag/v0.23.0[v0.23.0]: Refactored the
+SecurityHub module to remove a Python script that managed invitations between the AWS accounts. It's necessary to run a
+state migration to manage the invitations with Terraform.
+2. https://github.com/gruntwork-io/terraform-aws-cis-service-catalog/releases/tag/v0.24.0[v0.24.0]: This release introduces MFA Delete. You will need to follow the migration guide to ensure all S3 buckets are properly secured. Note: It is unlikely you will need to perform this step on the AWS root account as they typically don't contain S3 buckets. Please ensure you migrate all other AWS accounts.


NIT: If you use account-baseline-root, you'll likely have the Terraform state bucket in root.

brikis98 · 2021-09-17T10:43:00Z

_posts/2021-07-28-how-to-update-to-cis-14.adoc

+Changes in recommendations (both additions and removals) are listed below. You can think of these as a "diff"
+between versions 1.3.0 and 1.4.0.


NIT: to see the diff between versions 1.2.0 and 1.3.0, check out our guide (link).

brikis98 · 2021-09-17T10:46:28Z

_posts/2021-07-28-how-to-update-to-cis-14.adoc

+The account baseline modules had three breaking changes between versions v0.22.0 and v0.27.0. We must manually run
+these migration steps before updating the module versions.


NIT: this phrasing could be a bit misleading. I think what you want users to do is:

Bump the version numbers everywhere to v0.27.0 or above.

Run the state migrations below.

Run plan.

You should see diffs (...).

If all looks good, run apply.

Is that right? If so, considering clarifying that in the guide, and perhaps even showing a step-by-step example with one module. E.g.,

Here's module foo at version X:

module "foo" { source = "...?ref=X" }

First, we update foo to version Y:

module "foo" { source = "...?ref=Y" }

Now, three of the releases between Y and X required state migrations. You'd run those like this:

terraform state mv <old> <new>

Now run plan. You should see diffs (...).

If all looks good, run apply.

brikis98 · 2021-09-17T10:49:07Z

_posts/2021-07-28-how-to-update-to-cis-14.adoc

+Additionally, earlier versions of the account baseline modules did not set the following variables, so please ensure
+that they exist. Here is https://github.com/gruntwork-io/terraform-aws-cis-service-catalog/blob/v0.27.0/examples/for-production/infrastructure-live/logs/_global/account-baseline/terragrunt.hcl#L281[an example] of what you might set the values to for the prod account.
+
+ - `var.config_central_account_id`
+ - `var.security_hub_associate_to_master_account_id`
+ - `var.config_opt_in_regions`
+ - `var.guardduty_opt_in_regions`
+ - `var.kms_cmk_opt_in_regions`
+ - `var.iam_access_analyzer_opt_in_regions`
+ - `var.ebs_opt_in_regions`
+ - `var.security_hub_opt_in_regions`
+ - `var.macie_opt_in_regions`


NIT: This also has a little room for confusion. By listing var.xxx, it could be misunderstood the user needs to add new variables to their code, whereas what you want is for them to set those params in the modules you're using. Perhaps the clearer way to show this is to put them in a code block that looks like you are setting the vars, along with an explanation of what value is needed?

E.g.,:

module "account_baseline" { source = "..." # Add these new input vars if you don't have them already: # Set this to (explanation) config_central_account_id = "..." # Set this to (explanation) security_hub_associate_to_master_account_id = "..." }

brikis98 · 2021-09-17T10:49:41Z

_posts/2021-07-28-how-to-update-to-cis-14.adoc

+ - `var.security_hub_opt_in_regions`
+ - `var.macie_opt_in_regions`
+
+Once you have completed the above migration steps, it is time to update each baseline module to at least version https://github.com/gruntwork-io/terraform-aws-cis-service-catalog/releases/tag/v0.27.0[v0.27.0] and run Terraform/Terragrunt apply. Typically this is done using the `source` parameter:


As per two comments above, this seems wrong; the state mv won't work properly unless you update to the new version first.

brikis98 · 2021-09-17T10:50:01Z

_posts/2021-07-28-how-to-update-to-cis-14.adoc

+git::git@github.com:gruntwork-io/terraform-aws-cis-service-catalog.git//modules/landingzone/account-baseline-root?ref=v0.27.0
+----
+
+Now execute Terraform/Terragrunt `apply`. It should take approximately ~30 minutes to apply the account baseline


Why? What is it doing during those 30 min?

brikis98 · 2021-09-17T10:50:49Z

_posts/2021-07-28-how-to-update-to-cis-14.adoc

+  macie_kms_key_users      = ["arn:aws:iam::${local.accounts[local.account_name]}:root"]
+  macie_opt_in_regions     = local.opt_in_regions
+
+  # The variable below for Amazon Macie needs to be manually maintained. Please ensure you change the defaults.


NIT: wrap the comment to avoid horizontal scrolling here and below.

brikis98 · 2021-09-17T10:54:06Z

_posts/2021-07-28-how-to-update-to-cis-14.adoc

+
+Enabling MFA Delete in your bucket adds another layer of security by requiring MFA in any request to delete a version or change the versioning state of the bucket.
+
+The attribute `mfa_delete` is only used by Terraform to https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket#mfa_delete[reflect the current state of the bucket]. It is not possible to create a bucket if the `mfa_delete` is `true`, because it needs to be activated https://docs.aws.amazon.com/AmazonS3/latest/userguide/MultiFactorAuthenticationDelete.html[using AWS CLI or the API].


NIT: Add a sentence or two of context here first... Something like:

Unfortunately, the way AWS built the MFA delete feature is currently quite hard to use. Due to AWS API limitations, Terraform can't configure MFA delete on S3 buckets; you must first do it using the aws CLI, and then, after that, set mfa_delete = true in your Terraform code to reflect the update. Moreover, to enable MFA delete, you must authenticate as the root user of the AWS account that owns the bucket, and pass in a different MFA token value for every single bucket where you enable MFA delete. We've tried to make it as easy as we can, but due to how AWS built this feature, it is still quite tedious.

brikis98 · 2021-09-17T10:55:13Z

_posts/2021-07-28-how-to-update-to-cis-14.adoc

+
+1. https://docs.aws.amazon.com/IAM/latest/UserGuide/id_root-user.html#id_root-user_manage_add-key[Create access keys for the root user]
+1. https://docs.aws.amazon.com/IAM/latest/UserGuide/id_root-user.html#id_root-user_manage_mfa[Configure MFA for the root user]
+1. Create a bucket with `mfa_delete=false`.


NITs: We should specify which of our modules creates S3 buckets, and therefore, exposes this param. E.g., "If you're using private-s3-bucket or account-baseline-xxx, these all expose an mfa_delete param that should initially be set to false."

brikis98 · 2021-09-17T10:56:08Z

_posts/2021-07-28-how-to-update-to-cis-14.adoc

+to add support for specifying all buckets in a region.
+
+[[known_issues]]
+==== Known Issues


Thanks for capturing these 👍

infraredgirl · 2021-09-17T11:01:23Z

Thanks for the review! In the interest of getting this out the door, I'll merge this as-is, and open an issue to track the comments left here.

infraredgirl · 2021-09-17T11:15:23Z

Bug to track remaining issues: #644.

Init the migration guide for CIS 14

ce6bd27

marinalimeira requested review from eak12913, josh-padnick, oredavids and tsmit291 as code owners July 28, 2021 13:31

Fix links

26f69a7

robmorgan previously requested changes Aug 31, 2021

View reviewed changes

Marina and others added 4 commits August 31, 2021 11:11

Update _posts/2021-07-28-how-to-update-to-cis-14.adoc

1761920

Co-authored-by: Rob Morgan <robbym@gmail.com>

Update _posts/2021-07-28-how-to-update-to-cis-14.adoc

3578386

Co-authored-by: Rob Morgan <robbym@gmail.com>

Update with @robmorgan feedbacks

1720c03

Add TODO

fc3d769

marinalimeira commented Aug 31, 2021

View reviewed changes

_posts/2021-07-28-how-to-update-to-cis-14.adoc Outdated Show resolved Hide resolved

ina-stoyanova reviewed Aug 31, 2021

View reviewed changes

infraredgirl reviewed Aug 31, 2021

View reviewed changes

_posts/2021-07-28-how-to-update-to-cis-14.adoc Outdated Show resolved Hide resolved

_posts/2021-07-28-how-to-update-to-cis-14.adoc Show resolved Hide resolved

infraredgirl and others added 3 commits August 31, 2021 15:30

Add information about Macie

af985cd

Apply suggestions from code review

ecf8151

Co-authored-by: Ana Krivokapić <ana@gruntwork.io>

Adding background & final section to the guide

f212a02

robmorgan and others added 7 commits September 2, 2021 10:50

small tweaks. attempt to fix code block

7694d67

minor tweaks

ea6e03a

Updates on MFA Delete steps

3f665a2

Change manual steps to be after automated steps

62e30d9

Update compatibility table

f9b8072

Add section on account baseline

9a056d5

Fix formatting

d9f706b

Clarify Macie manual steps

0ad69f6

ina-stoyanova and others added 6 commits September 10, 2021 16:42

Updates to the explanations for Macie

1721d63

Talk about Macie in the context of LZ

35141ee

migration guide improvements

52a63e0

Fix macie var names

4cc285c

minor tweaks and improvements

bf85093

Merge branch 'cis-14-migration' of github.com:gruntwork-io/gruntwork-…

b940904

…io.github.io into cis-14-migration

robmorgan changed the title ~~[WIP] [IAC-1866] CIS AWS v1.4 migration guide~~ [IAC-1866] CIS AWS v1.4 migration guide Sep 16, 2021

Ensure Macie vars match those from the deployment guide.

63bc2f7

Also, simplify Macie configuration changes.

brikis98 approved these changes Sep 17, 2021

View reviewed changes

infraredgirl merged commit bb31343 into master Sep 17, 2021

infraredgirl deleted the cis-14-migration branch September 17, 2021 11:02

brikis98 mentioned this pull request Sep 17, 2021

[IAC-1864] CIS 1.4 deployment guide #639

Merged

6 tasks

infraredgirl mentioned this pull request Sep 17, 2021

CIS 1.4 migration guide: Fix comments from PR #512 #644

Open

		Changes in recommendations (both additions and removals) are listed below. You can think of these as a "diff"
		between versions 1.3.0 and 1.4.0.

		The account baseline modules had three breaking changes between versions v0.22.0 and v0.27.0. We must manually run
		these migration steps before updating the module versions.


		Enabling MFA Delete in your bucket adds another layer of security by requiring MFA in any request to delete a version or change the versioning state of the bucket.

		The attribute `mfa_delete` is only used by Terraform to https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket#mfa_delete[reflect the current state of the bucket]. It is not possible to create a bucket if the `mfa_delete` is `true`, because it needs to be activated https://docs.aws.amazon.com/AmazonS3/latest/userguide/MultiFactorAuthenticationDelete.html[using AWS CLI or the API].

Uh oh!

[IAC-1866] CIS AWS v1.4 migration guide #512

[IAC-1866] CIS AWS v1.4 migration guide #512

Uh oh!

Conversation

marinalimeira commented Jul 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Jul 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robmorgan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ina-stoyanova left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

infraredgirl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

infraredgirl commented Sep 1, 2021

Uh oh!

marinalimeira commented Sep 8, 2021

Uh oh!

infraredgirl commented Sep 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ina-stoyanova commented Sep 10, 2021

Uh oh!

ina-stoyanova commented Sep 10, 2021

Uh oh!

robmorgan commented Sep 16, 2021

Uh oh!

brikis98 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

infraredgirl commented Sep 17, 2021

Uh oh!

infraredgirl commented Sep 17, 2021

Uh oh!

Reviewers

Assignees

Labels

marinalimeira commented Jul 28, 2021 •

edited

Loading

netlify bot commented Jul 28, 2021 •

edited

Loading

infraredgirl commented Sep 8, 2021 •

edited

Loading