Skip to content
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

Sudo fixes broke my userdata script and sudoers file #22

Open
mchristen opened this issue Jul 28, 2017 · 4 comments
Open

Sudo fixes broke my userdata script and sudoers file #22

mchristen opened this issue Jul 28, 2017 · 4 comments

Comments

@mchristen
Copy link

When adding to the sudoers file a newline is not added at the end of the file and if you are using amazon userdata scripts to make additional changes to the sudoers file it will break unless you know to prepend the additional changes w/ a newline.

I basically ended up with a sudoers file that ended w/

%foxpass-sudo ALL=(ALL:ALL) NOPASSWD:ALL%Ops ALL = NOPASSWD: ALL

Luckily I caught this on a newly provisioned server before it started to spread to the rest of the infrastructure.

A few takeaways from this:

  1. I'm not sure if I like the idea of Foxpass deciding what groups are going to be given sudo rights automatically, perhaps it should configurable in the web interface if anything
  2. I should have pinned the version of the setup script in my userdata script instead of using https://raw.githubusercontent.com/foxpass/foxpass-setup/master/linux/ubuntu/16.04/foxpass_setup.py which has the possibility of changing between server provisioning
@aren
Copy link
Contributor

aren commented Jul 28, 2017

Sorry that we broke this for you. We have a PR out to fix it ( #21 ) but it hasn't been merged yet pending some more testing. (Does the new branch work for you?)

I appreciate the rest of the feedback. The first suggestion is especially interesting -- we could make the script dynamic and query Foxpass for your sudo group name.

@mchristen
Copy link
Author

Everything appears to work as before when using #21.

I guess my biggest fear is that there is a large amount of trust in Foxpass in always doing the right thing(not adding backdoors to our servers, etc.). Part of maintaining a secure environment is knowing who/what has access to it and it was definitely disconcerting seeing a foxpass-sudo group added to the sudoers file w/o any sort of knowledge of the change(perhaps I missed an announcement e-mail or something though). Especially knowing that foxpass-sudo was not a group that we had ever created.

It's finally pushed us to start considering pre-baking our AMIs(not a trivial amount of work for our setup) and manually configuring Foxpass instead of relying on the provided scripts.

Thankfully this appeared to only affect our Ubuntu based servers and not ones based off of Amazon Linux, which is what the majority of our production workload runs off of.

@aren
Copy link
Contributor

aren commented Jul 28, 2017

Great, thanks for verifying. You're right that it's the best idea to pin the version of the script that you've tested and reviewed. But still, we try hard to maintain backwards compatibility and not break our users.

The history of the 'foxpass-sudo' group is that previously, we'd suggest making an LDAP group with the same name as the blessed sudo group on the box ('wheel' for RH-based, 'sudo' for debian-based). This causes problems because when an LDAP group and a local group with the same name is used, the two aren't merged. (There is a fix in glibc to merge them, but it's not going to hit the distros for a while).

So we came up with 'foxpass-sudo' as the new suggested group name, and updated our scripts and docs to match.

@joshbranham
Copy link

@mchristen why not just pass the --sudoers-group arg with a group you create? We are provisioning FoxPass via our AMI builds with Packer for now and defining the sudoers as part of the foxpass_setup.py script to avoid anything changing in production

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants