ansible-community / molecule-plugins

Collection on molecule plugins
MIT License
112 stars 74 forks source link

Template should not be creating a security group #50

Open lod opened 4 years ago

lod commented 4 years ago

Issues

I feel that the templated behaviour of creating a security group is wrong. It involves making security assumptions which I do not feel are suitable for a test program to make.

It is too loose: Allowing public ssh into instances which often have default passwords set is problematic.

It is too restrictive: Windows instances won't work due to the port not being accessible, ansible-community/molecule-plugins#47 is an example of the issues which arise from this. It also means special handling is required to test roles which expose other ports, such as a web server.

It trips up auditing: Security group changes are commonly monitored to detect AWS compromises or misconfigurations. Frequently creating and destroying security groups is undesirable behaviour.

Solutions

At a minimum I feel we should support passing a security group in via the platform configuration. The default security group should only be created if this parameter is not set.

WinRM port 5986 should be opened by the default security group. No other program seems to use the port, it will not meaningfully weaken the security of the Linux instances and will allow contact to Windows instances.

My preference would also be for a security group not to be created if not set. Instead it should fall back to the default security group for that VPC. I believe this was the prior behaviour, however my recollection and the git history don't align.

mksha commented 4 years ago

Would like to add a note here, Usually, if we are concerned about security then first of all we will not have default VPC, that why we are creating a new security group.

On specifying an existing group, it may or may not be valuable because if we have existing SG then why you would modify it for testing purposes if not something part of test infra. If its part of test infra then we are leaving the manual task to do out there.

luupux commented 3 years ago

@ssbarnea , Hello, I did some work to manage the security group in the platform so in which way I can contribute to the project by sharing what I did ?

tbugfinder commented 3 years ago

Hi @luupux , would it make sense to create a PR based on your solution and further discuss within the PR?

ssbarnea commented 3 years ago

@luupux I did send you an invitation a long time ago but you did not accepted it, so you have no extra permissions to the repo. Check https://github.com/ansible-community/molecule-ec2/invitations -- it may work without email, I hope.

luupux commented 3 years ago

Hi @ssbarnea I missed the invitation sorry

jgoldschrafe commented 3 years ago

The bulk of this should also be addressed by ansible-community/molecule-ec2#42—security groups can be added with the security_groups option on a platform, which is just an absolute delight if you have engineers trying to iterate on Ansible code in a regulated environment where they're not allowed to create security groups on the fly.

Great suggestion on WinRM support. There's a further small gotcha around the wait for SSH that should also be adjusted for hosts that use WinRM instead of SSH as their transport.

beargiles commented 2 years ago

FWIW you can lock down a few things with a dedicated IAM role and VPC subnet but I agree that this still leaves a lot open. We can hide some things in a launch template but I suspect we'll end up with a lambda function that handles everything create and destroy do and then replace this plugin with a delegated one that just calls the lambda function. The lambda function can maintain state (e.g., ephemeral keypairs) in dynamo db.

The lambda function will still need permission to modify security groups but the security guys may be fine with it since they can ensure it colors within the lines. The apps that call the lambda function won't need any permissions beyond the ability to call the lambda function.

beargiles commented 2 years ago

BTW the ansible module is https://docs.ansible.com/ansible/latest/collections/community/aws/execute_lambda_module.html so the input would probably be instance properies (or launch template) and the output would be the ip address and ephemeral keypair. I don't think you need more than that.

One really cool thing about this approach is that it could also add an input rule that restricts access to your IP address. (Plus anything granted in your other security groups.) I haven't been happy that the default is to allow world access to the SSH port but it's difficult for the local playbook to determine the correct IP address since the host running the playbooks is probably in a private IP address range. I can think of several solutions, none pretty.