Skip to content

Bootstrap: require user to pass Bastion remote access CIDRs #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 17, 2024

Conversation

tjohnes
Copy link
Collaborator

@tjohnes tjohnes commented Jun 14, 2024

Background:

  • The Bastion module (responsible for bringing up the Bastion instance) exposes a remote_access_cidr variable. This is a list of CIDRs that are allowed SSH access to the Bastion.
  • The default is ["0.0.0.0/0"], which allows access from anywhere.
  • This is re-exposed in the Bootstrap module, with the same default.
  • So the default behavior for users of the Bootstrap configuration is that a Bastion instance is created that has unrestricted SSH access (other than normal RSA auth)

Modify this so that:

  • There is no default value for remote_access_cidr in the Boostrap configuration. This means that users of the Bootstrap configuration must provide a value for this. This is a breaking change.
  • This is considered a balance between:
    • More secure than defaulting to 0.0.0.0/0
    • More usable than defaulting to restricting all access - a lot of the point of the Terraform configurations is that users can see how an XRd-capable EC2 node is setup. If no-one can access the Bastion, then no-one can access the nodes.

If the user provides null explicitly, then access to the Bastion is prevented.

The reviewer should consider whether the above is an appropriate balance of i. defaulting to security; ii. usability, both in terms of accessing the Bastion/nodes, and usability of the Terraform configuration given this is a new required variable.

In the aws-quickstart script:

  • Add a -b|--bastion-remote-access-cidr-blocks required argument.
  • This should be a comma-separated list of CIDRs.
  • If the user provides 'null', prevent access to the Bastion.

The implementation of the aws-quickstart script is slightly messy, and an area of focus for the reviewer.

tjohnes added 5 commits June 13, 2024 15:39
This is because the type is not displayed at the interactive prompt.
Also allow passing null to prevent access.
Bootstrap module accepts null or a list.

Bastion module accepts a list, which is non-nullable.  Boostrap should
convert null -> [].

This is a more "minimal" change in line with what we already do
generally in this case - using empty lists to mean "null", rather than
accepting null.
@tjohnes tjohnes marked this pull request as ready for review June 14, 2024 10:30
@paulo-cisco paulo-cisco self-requested a review June 14, 2024 13:14
@tjohnes tjohnes merged commit bf4dee8 into main Jun 17, 2024
2 checks passed
@tjohnes tjohnes deleted the require-bastion-access-cidr branch June 17, 2024 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants