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

Create user accounts instead of sharing root #253

Closed
wants to merge 2 commits into from

Conversation

@edunham
Copy link
Contributor

edunham commented Mar 15, 2016

SSH best practices is to fully disallow remote root login. To do that, we have to start by getting everyone logging in as themselves.

r? @aneeshusa


This change is Reviewable

@asajeffrey
Copy link
Member

asajeffrey commented Mar 15, 2016

Travis failures... https://travis-ci.org/servo/saltfs/jobs/116193785

https://gist.github.com/edunham/eb8f3bae70131e5d61df (edunham edited this comment, moving large code block into gist, to make thread easier to read)

@aneeshusa
Copy link
Member

aneeshusa commented Mar 15, 2016

I've been waiting for this for so long :)

@@ -45,8 +45,15 @@ host-{{ hostname }}:
{% endfor %}
{% for ssh_user in common.ssh_users %}
{{ ssh_user }}:
user.present:
- home: /home/{{ ssh_user }}

This comment has been minimized.

@aneeshusa

aneeshusa Mar 15, 2016

Member

This won't work on the Macs, I believe we'll need to use /Users instead of /home there. We should do this via a Jinja variable that we pull in from the map.jinja file. I think this is also causing the Travis failure.

user.present:
- home: /home/{{ ssh_user }}
- optional_groups:
- wheel

This comment has been minimized.

@aneeshusa

aneeshusa Mar 15, 2016

Member

Is this sufficient to provide sudo privileges on OS X?

This comment has been minimized.

@edunham

edunham Mar 15, 2016

Author Contributor

AFAICT, OSX's BSD roots show through here and wheel does the usual thing. https://discussions.apple.com/thread/2210858?start=0&tstart=0

- home: /home/{{ ssh_user }}
- optional_groups:
- wheel
- empty_password: True

This comment has been minimized.

@aneeshusa

aneeshusa Mar 15, 2016

Member

I'm ok with this for now (strictly better than status quo), but just wanted to leave a link to https://sourceforge.net/projects/pamsshagentauth/ as something I like to configure for password-less sudo that still has some security. I'm pretty sure there's nothing similar for OS X.

- optional_groups:
- wheel
- empty_password: True
sshkey-{{ ssh_user }}:
ssh_auth.present:

This comment has been minimized.

@aneeshusa

aneeshusa Mar 15, 2016

Member

We can combine this state with the previous one, so that they share the same ID.

This comment has been minimized.

@edunham

edunham Mar 15, 2016

Author Contributor

As in going like this?

{{ssh_user}}
  user.present:                                                                 
    - createhome: True                                                          
    - optional_groups:                                                          
        - wheel                                                                 
    - empty_password: True
  ssh_auth.present:                                                             
    - user: {{ ssh_user }}                                                      
    - source: salt://{{ tpldir }}/ssh/{{ ssh_user }}.pub                

Sorry, still pretty new to Salt.

This comment has been minimized.

@aneeshusa

aneeshusa Mar 15, 2016

Member

Err, don't forget the colon after the {{ ssh_user }}: (in the id on the top line).

@aneeshusa
Copy link
Member

aneeshusa commented Mar 15, 2016

After this lands, we should:

  • update the wiki
  • (probably as part of #254) add a state that removes root's authorized_keys file. It shouldn't matter with `PermitRootLogin no, but it's nice to have written down explicitly.
@aneeshusa
Copy link
Member

aneeshusa commented Mar 15, 2016

Hmm, it seems GitHub has removed the ability to comment on individual commits. FYI: I like to use servo-macpro1 for testing OS X related things, since I can verify settings by hand afterwards.

@aneeshusa
Copy link
Member

aneeshusa commented Mar 15, 2016

It looks like the Travis failures are because of the empty_password, which calls the shadow.del_password Salt function, which wasn't implemented for Mac until January of this year (and won't be available until Salt 2016.3). This module also implements setting passwords, so we can't set a (non-empty) password without it either.

We'll need to backport the module manually for now. Here's the docs if you want to give it a spin: https://docs.saltstack.com/en/2015.5/ref/modules/; let me know if you have questions.

@aneeshusa
Copy link
Member

aneeshusa commented Mar 16, 2016

Another alternative to backporting the mac_shadow module would be enabling passwordless sudo, so that the passwords on user accounts don't matter.

edunham added 2 commits Mar 15, 2016
SSH best practices is to fully disallow remote root login
@bors-servo
Copy link
Contributor

bors-servo commented Jan 17, 2017

The latest upstream changes (presumably #577) made this pull request unmergeable. Please resolve the merge conflicts.

@edunham
Copy link
Contributor Author

edunham commented Mar 27, 2017

Superseded by #628. Feedback from here is now applied there.

@edunham edunham closed this Mar 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.