Security: Removing step 0 'git pull' #33
Conversation
|
Hello @jdsumsion thanks for your concern, you do raise some valid points, but do please be aware that this would not affect "any DataStax AMI in the world," but instead only newly launched images, if the worst case scenario happened. We have considered that in the past, but have avoided that for a few reasons:
Of course, the above reasons don't outweigh your security concerns. That's why we added the following logic in ds0.py: https://github.com/riptano/ComboAMI/blob/2.4/ds0_updater.py#L8 The idea for that was to guarantee that once the image has launched, you now own the image entirely and that it wouldn't change from what you expected via any sort of update. We did make this a manually editable parameter since there have been times that ds1.py needed to be upgraded and upgrading the cluster manually would have been a slight headache from an operational perspective. If, however, you wanted to roll your own AMI without these features, I've provided detailed instructions on how to fork this project and bake the images yourself: https://github.com/riptano/ComboAMI/blob/2.4/presetup/setup.md I was thinking what may have been an alternative solution would be to modify ds4_motd.py to display the date and hash of the last commit... but if someone owned the account that could easily be manipulated within ds4.py and just provide a false sense of security. However, if you were to check the last commit manually on your end, I'm hoping this will be a good workaround. Also, you should be able to easily script this within your launch scripts since we typically push very few fixes since the AMI has been decently stable for the past couple of months. I hope the above information helps and sheds light into our current decision. However, after discussing this with another colleague, we figured signed repos or signed debian packages might make for a good alternative. I've set up some time with our packaging guru and we'll see what we can get out the door. Fwiw, signed repos/packages may have to wait until the next AMI release/bake, which should be coming soon to support #25 . Do you have any timeline in mind for your request? Is this blocking anything now that it's under discussion and planned for a release in the near future? Thanks again for your concern and for creating this issue! |
|
I see, I didn't realize that the AMI was updated that infrequently. In that case, my suggested fix would make things VERY inconvenient. You are right that the "only pull on first boot" reduces the possibility of attack quite a bit. I am glad that you documented how to make images on our own, we will likely do this in preparation for production setup. Maybe we could do something on first boot that would validate the current commit hash against an argument that could be passed into the instance user data. If it didn't match, then things would fail fast, and if it did match, then there are no worries. People who cared would look up the current commit hash and put the validation parameter in, and people who didn't care would just get the current behavior. I looked at
But I didn't necessarily want to duplicate those into Or maybe your idea of pulling these scripts down as a signed deb pkg is better all around. |
|
One of the reasons I'm concerned about this is that the popularity of cassandra, and especially the datastax AMI, makes instances launched this way a more vulnerable & potentially valuable target to attack. After launching a cluster the other day, I saw an ESTABLISHED tcp connection within the first hour to opscenter (port 8888) on one of my instances, from an unknown and unauthorized Russian ip address. I've since locked the security group down, but I realized this is the wild open internet, and anything we can do to reduce the central attack vectors is a good idea. This was likely just a random port scan since 8888 is a popular java app server port, but still it was a wake up call. |
If anyone contrives to gain access to the riptano/ComboAMI repository, they could push a commit that for any subsequent datastax-ami-launched instance, if that instance could talk back to github to do the pull, arbitrary code could be run. They would be able to do this without gaining access to the datastax AWS account. They would then have access inside the AWS account that cassandra is being launched in. Other options for better security: a) fork ComboAMI, apply this patch, build your own AMI based on your fork, or b) apply a firewall to avoid talking back to github (VPC or iptables), or c) build your cassandra clusters without the ComboAMI automation (roll your own) All of those other options sound pretty inconvenient -- it would be nice to have a more secure default launch. If this commit is accepted, then it would be harder to run arbitrary code. It would be necessary to both 1) own riptano's computer (or whoever build the AMI), and 2) insert logic into the AMI at build time. Or it would be necessary to gain access to the datastax AWS account and insert the logic into the AMI post-build. Both of which would seem to me to be less likely than gaining access to a single git repo. The obvious downside is that it's harder to ship ComboAMI fixes/tweaks because it involves building a whole new AMI for any slight change. I think that the pain of having to update the AMI more frequently pays down a significant central security risk, and it's worth the inconvenience.
This reverts commit 117f961. This is impractical because building/replacing AMIs for all the regions is so inconvenient for both the datastax developers, and for consumers who rely on the image id being stable. Perhaps a better idea would be to pass a commit SHA1 in as an optional parameter and just validate it after the first 'git pull', for those who care.
|
Oops, didn't realize that "Close" was going to close the pull request. Duh. |
|
I'm glad you found the bake-your-own-AMI documentation useful. All that I'd ask is if you ever spot a bug or an optimization, do please let us know so that the entire community benefits. :) I really liked your commit hash idea as well, but you're right since ds0.py would have to update the AMI to include that logic and the place to apply the checks would ideally be in ds2.py which could already be tainted. I'm currently working on the new 2.5-dev branch. I'll add that hash functionality there (even if we have to have duplicate functions) within ds0.py so that we can bake just that functionality. I might even add a force_hash option, in addition to a check_hash option. Would the force_hash option be something that you'd be okay with and perhaps keep you using the community AMI? The way I figure it is that if we can cut down your complexity, we'll probably cut down complexity for others as well, especially since this is a valid concern. The added benefit is that your upgrade path's complexity would also be cut down as well. For sure and thank you for these concerns. Crazy story about the port scan. If you want the full list of ports that you'll want open and locked down, do make sure to check this out, if you already haven't: https://github.com/riptano/ComboAMI#ports-needed. |
|
If you did a I'll close this pull request now because there's nothing of substance left, except for this thread of brainstorming. Thank you for your very helpful collaboration on this issue, now and as you implement! |
|
Okay that sounds great. I'll link you to what gets committed (hopefully by the end of today). Sure, never a problem! Glad to have helped! |
|
Hello @jdsumsion , could you verify that this works for you? https://github.com/riptano/ComboAMI/blob/dev-2.5/ds0_updater.py I'll be baking this in a few. Let me know if this works with what you had in mind. Thanks! |
|
Yes, I'll check it out the next cluster we build. Thank you for your prompt response! |
|
Just to clarify, this is not yet included on the 2.4 image. I'll have to bake the 2.5 image, which I'm doing right now, for this version of the file to be safely included. Feel free to email me your AWS account ID and I'll get you beta access as soon as it's baked, if not, it'll only be postponed by a public release in a few hours/days. Cheers. |
|
Just wanted to send a quick update and mention that the new AMIs have now been released. http://www.datastax.com/dev/blog/introducing-the-datastax-auto-clustering-ami-2-5 Thanks again for your suggestion! |
|
Hey, just another heads up: I've patch an issue with the previous --forcecommit code when sending additional user-data. The new 2.5.1 AMIs will now work as intended. |
|
Thank you, I have not built a cluster for a while now, but I'll look at this when I build another one. |
If anyone contrives to gain access to the riptano/ComboAMI repository, they
could push a commit that would immediately start running arbitrary code on
any datastax AMI launch in the world. They would be able to do this without
gaining access to the datastax AWS account. They would then have access
inside the AWS account that cassandra is being launched in, as long as the
launched instance can talk back to github to auto-pull the exploit.
Other options for better security:
a) fork ComboAMI, apply this patch, build your own AMI based on your fork, or
b) apply a firewall to avoid talking back to github (VPC or iptables), or
c) build your cassandra clusters without the ComboAMI automation (roll your own)
All of those other options sound pretty inconvenient -- it would be nice to
have a more secure default launch.
If this commit is accepted, then it would be harder to run arbitrary code.
It would be necessary to both 1) own riptano's computer, and 2) insert logic
into the AMI at build time. Or it would be necessary to gain access to the
datastax AWS account and insert the logic into the AMI post-build. Both of
which would seem to me to be less likely than gaining access to a single git
repo.
The obvious downside is that it's harder to ship ComboAMI fixes/tweaks
because it involves building a whole new AMI for any slight change.
I think that the pain of having to update the AMI more frequently pays down
a significant central security risk, and it's worth the inconvenience.