search

hashicorp / terraform-aws-vault

A Terraform Module for how to run Vault on AWS using Terraform and Packer
Apache License 2.0
656 stars 465 forks source link

No apt update in packer build? #235

Closed queglay closed 3 years ago

queglay commented 3 years ago

This doc says that when using packer in production the build file should be copied more or less exactly: https://github.com/hashicorp/terraform-aws-vault/tree/master/examples/vault-consul-ami

But a question I have about security in that document is shouldn't we also run apt update or upgrade as part of a base image?

I usually run updates once and put them in a seperate base image, then builds for everything else run another ami on top of that, is this not recommended for vault server?

brikis98 commented 3 years ago

This doc says that when using packer in production the build file should be copied more or less exactly: https://github.com/hashicorp/terraform-aws-vault/tree/master/examples/vault-consul-ami

Hm, we typically recommend using these templates as a starting point. Do the docs really say to copy "more or less exactly" somewhere?

But a question I have about security in that document is shouldn't we also run apt update or upgrade as part of a base image?

I usually run updates once and put them in a seperate base image, then builds for everything else run another ami on top of that, is this not recommended for vault server?

Yes, you should. FWIW, the install-vault script does that: https://github.com/hashicorp/terraform-aws-vault/blob/master/modules/install-vault/install-vault#L121

queglay commented 3 years ago

Thanks for your reply @brikis98. Yes you can see those words in there: https://github.com/hashicorp/terraform-aws-vault/blob/master/examples/vault-consul-ami/README.md

Thanks for letting me know the install script does run updates. I think running updates in the installer could be a problem if people are trying to build and rebuild immutable images, since it rules out the possibility of using a base ami (for software updates), and then building the secondary AMI in a repeatable fashion (no updates). Updates can be inconsistent fairly often enough (had lots of problems with pip and Ansible in the past), or on the odd occasion contain software vulnerabilities so I've heard. I've also heard some companies check the versions of all the packages for vulnerabilities, so if an installer runs updates again that process would need to occur again. What do you think?

brikis98 commented 3 years ago

Thanks for your reply @brikis98. Yes you can see those words in there: https://github.com/hashicorp/terraform-aws-vault/blob/master/examples/vault-consul-ami/README.md

Ah, thanks, I can see how that is a bit ambiguous. We mean you can copy the Packer template exactly, but not that you shouldn't add anything else to it at all! A PR to improve that wording is very welcome.

Thanks for letting me know the install script does run updates. I think running updates in the installer could be a problem if people are trying to build and rebuild immutable images, since it rules out the possibility of using a base ami (for software updates), and then building the secondary AMI in a repeatable fashion (no updates). Updates can be inconsistent fairly often enough (had lots of problems with pip and Ansible in the past), or on the odd occasion contain software vulnerabilities so I've heard. I've also heard some companies check the versions of all the packages for vulnerabilities, so if an installer runs updates again that process would need to occur again. What do you think?

As always, it's a trade-off. If the script doesn't run update itself, then for users who didn't add that, some of the install steps run by the script will end up with either old (possibly incompatible) versions, or fail entirely. On the other hand, for users in your camp, it causes problems. Perhaps it could be on by default, but we offer a flag to disable it?

queglay commented 3 years ago

A flag could be good. What do you think it should be? Perhaps...

--update-packages false

I'd be happy to submit a PR for that.

brikis98 commented 3 years ago

Maybe --skip-package-update?

A PR would be great. Thank you!

brikis98 commented 3 years ago

Fixed in #235 and released in https://github.com/hashicorp/terraform-aws-vault/releases/tag/v0.15.1.