terraform-google-modules / terraform-google-vpc-service-controls

Handles opinionated VPC Service Controls and Access Context Manager configuration and deployments
https://registry.terraform.io/modules/terraform-google-modules/vpc-service-controls/google
Apache License 2.0
59 stars 67 forks source link

BigQuery Exfil Demo #24

Closed onetwopunch closed 4 years ago

onetwopunch commented 4 years ago

Demonstrates how VPC service controls can prevent exfil from BigQuery in one project to another project:

Creates

onetwopunch commented 4 years ago

@morgante this is ready for another review when you have a moment

morgante commented 4 years ago

@onetwopunch Can you respond/explain this?

I'm wondering if the two separate configurations are really necessary here? Could we consolidate the projects/ into the main configuration? I'm also not sure if it's really necessary to have the seed project setup + separate Service Account created.

it seems like a lot of unnecessary overhead which is unrelated to VPC service controls.

onetwopunch commented 4 years ago

@morgante that was my original idea, but there were several errors that happened when doing so due to the projects not being available before we try to use them. I can refactor/try again and get back to you on which errors popped up. Maybe I missed something

morgante commented 4 years ago

@onetwopunch Could you try again? I'm happy to help debug. IMO we can probably remove things like the 'seed service account' entirely.

onetwopunch commented 4 years ago

@morgante I tried pulling the projects into the top level directory again and got the same error as last time. It's because you can't use for_each with interpolated values that are determined after apply. This is actually a bug with the bastion module that makes it incompatible with project factory output. Issue here: https://github.com/terraform-google-modules/terraform-google-bastion-host/issues/19

Error: Invalid for_each argument

  on .terraform/modules/bastion/terraform-google-modules-terraform-google-bastion-host-7d2e49c/main.tf line 88, in resource "google_project_iam_member" "bastion_sa_bindings":
  88:   for_each = toset(compact(concat(
  89:     var.service_account_roles,
  90:     var.service_account_roles_supplemental,
  91:     ["projects/${var.project}/roles/${google_project_iam_custom_role.compute_os_login_viewer.role_id}"]
  92:   )))

The "for_each" value depends on resource attributes that cannot be determined
until apply, so Terraform cannot predict how many instances will be created.
To work around this, use the -target argument to first apply only the
resources that the for_each depends on.
onetwopunch commented 4 years ago

@morgante Pulled the projects up into the main directory, and all that's working now with the new version of the bastion. I'm curious though what you mean by it's not necessary to have the seed project. Those instructions are mainly to help folks get up and running with project factory in case they haven't used it before. We have to have two projects for this to work and it seems to make sense to create them with project factory so we can destroy them after we're done right?

morgante commented 4 years ago

I'm curious though what you mean by it's not necessary to have the seed project.

For a demo/example scenario, it should be fine to create projects using user credentials (without creating a service account). Using service account credentials is not required for Project Factory to work.

onetwopunch commented 4 years ago

@morgante comments are addressed. Removed the unnecessary Makefile and migrated to CFT network module.

morgante commented 4 years ago

Thanks, great addition!