Opened 7 years ago

Closed 5 weeks ago

#6367 closed defect (fixed)

make dedicated sudo passwords

Reported by: weasel Owned by: anarcat
Priority: Medium Milestone:
Component: Internal Services/Tor Sysadmin Team Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description


Child Tickets

Change History (14)

comment:1 Changed 5 years ago by nickm

+1 on this. If we aim to get this transition done some time after 1 May, what do you need from sudoers first?

comment:2 Changed 5 years ago by weasel

Primarily we need to let them know to set sudo passwords via the web interface on db.tpo and then hold their hands them during the inevitable mail issues

comment:3 Changed 4 years ago by weasel

Severity: Normal

sudo passwords enabled. for now, the unix password still works.

comment:4 Changed 4 years ago by weasel

Announcement mail: https://lists.torproject.org/pipermail/tor-project/2016-March/000199.html

Plan to disable pam_unix for sudo 2nd week of April.

comment:5 Changed 3 years ago by weasel

Owner: set to tpa
Status: newassigned

comment:6 Changed 3 years ago by weasel

Status: assignednew

comment:7 Changed 9 months ago by ln5

The syadmin team meeting in Brussels ([hhttps://trac.torproject.org/projects/tor/wiki/org/meetings/2019BrusselsAdminTeamMinutes#Dedicatedsudopasswords notes]) decided that we stop accepting LDAP passwords for sudo.

Two action items came out:

  • Configure pam on all but the CRM hosts to only accept the sudo passwords
  • Send email to tor-project@ informing about that change.
Last edited 9 months ago by ln5 (previous) (diff)

comment:8 Changed 9 months ago by ln5

Owner: changed from tpa to weasel
Status: newassigned

comment:9 Changed 6 months ago by anarcat

what does this actually involve, at the technical level? it looks like it's simply a matter of removing this line in /etc/pam.d/sudo:

auth    [success=1 default=ignore]      pam_unix.so nullok_secure try_first_pass

... on all servers but the crm* servers? seems like we could just call a flag day and do it alraedy. i'd be happy to do that if you have your hands full...

comment:11 Changed 7 weeks ago by anarcat

Owner: changed from weasel to anarcat

sent the announcement in https://lists.torproject.org/pipermail/tor-project/2019-September/002509.html

next step is to do the actual change, on oct 14.

comment:12 Changed 5 weeks ago by anarcat

Status: assignedneeds_review

couldn't do this yesterday as i was on vacation, and now it feels a bit late in the day to perform the change - i'd like to have time during the day to help people with problems if they happen.

so i'm going to do this tomorrow morning instead.

i've also notified the GR people specifically to see if this will cause any problems on their side. i've pushed the changes to a sudo-ldap branch on the puppetmaster, which is ready for review, but it's basically this patch set:

From 20850426446dab13c090932d8dbb13ccaeeeb3da Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Antoine=20Beaupr=C3=A9?= <anarcat@debian.org>
Date: Tue, 15 Oct 2019 16:32:41 -0400
Subject: [PATCH 1/2] cleanup sudo's pam config: reuse common-auth

The only difference was `try_first_pass` that is missing from
common-auth, but considering we're going to remove that line anyways,
it's worth keeping that refactoring separate in history.
---
 modules/sudo/files/pam | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/modules/sudo/files/pam b/modules/sudo/files/pam
index 1621e8d3..05642199 100644
--- a/modules/sudo/files/pam
+++ b/modules/sudo/files/pam
@@ -5,9 +5,7 @@
 
 #auth [authinfo_unavail=ignore success=done ignore=ignore default=die] pam_pwdfile.so pwdfile=/var/lib/misc/thishost/sudo-passwd
 auth [authinfo_unavail=ignore success=done ignore=ignore default=ignore] pam_pwdfile.so pwdfile=/var/lib/misc/thishost/sudo-passwd
-auth    [success=1 default=ignore]      pam_unix.so nullok_secure try_first_pass
-auth    requisite                       pam_deny.so
-auth    required                        pam_permit.so
 
+@include common-auth
 @include common-account
 @include common-session-noninteractive
-- 
2.20.1
From b4c21e7e31b89e8b89476f16da8eb6bdfc666123 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Antoine=20Beaupr=C3=A9?= <anarcat@debian.org>
Date: Tue, 15 Oct 2019 16:33:36 -0400
Subject: [PATCH 2/2] disable /etc/password for sudo access (see #6367)

---
 modules/sudo/files/pam | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/modules/sudo/files/pam b/modules/sudo/files/pam
index 05642199..7e1ec366 100644
--- a/modules/sudo/files/pam
+++ b/modules/sudo/files/pam
@@ -3,9 +3,10 @@
 ##
 #%PAM-1.0
 
-#auth [authinfo_unavail=ignore success=done ignore=ignore default=die] pam_pwdfile.so pwdfile=/var/lib/misc/thishost/sudo-passwd
-auth [authinfo_unavail=ignore success=done ignore=ignore default=ignore] pam_pwdfile.so pwdfile=/var/lib/misc/thishost/sudo-passwd
+# use the LDAP-derived password file for sudo access
+auth [authinfo_unavail=ignore success=done ignore=ignore default=die] pam_pwdfile.so pwdfile=/var/lib/misc/thishost/sudo-passwd
 
-@include common-auth
+# disable /etc/password for sudo authentication, see #6367
+#@include common-auth
 @include common-account
 @include common-session-noninteractive
-- 
2.20.1

comment:13 in reply to:  12 Changed 5 weeks ago by weasel

Replying to anarcat:

 
-#auth [authinfo_unavail=ignore success=done ignore=ignore default=die] pam_pwdfile.so pwdfile=/var/lib/misc/thishost/sudo-passwd
-auth [authinfo_unavail=ignore success=done ignore=ignore default=ignore] pam_pwdfile.so pwdfile=/var/lib/misc/thishost/sudo-passwd
+# use the LDAP-derived password file for sudo access
+auth [authinfo_unavail=ignore success=done ignore=ignore default=die] pam_pwdfile.so pwdfile=/var/lib/misc/thishost/sudo-passwd
 
-@include common-auth
+# disable /etc/password for sudo authentication, see #6367
+#@include common-auth
 @include common-account
 @include common-session-noninteractive

I'm not convined. Having authinfo_unavail=ignore and ignore=ignore without an explicit next item on the auth stack seems fishy.

Here's what Debian does, and I think it's sane:

auth [authinfo_unavail=ignore success=done ignore=ignore default=die] pam_pwdfile.so pwdfile=/var/lib/misc/thishost/sudo-passwd
auth required pam_unix.so nullok_secure try_first_pass

This does auth against pam_pwdfile, and only if an entry is not there do we fall back to pam_unix. Either that or a flat out deny seems like a good idea.

comment:14 Changed 5 weeks ago by anarcat

Resolution: fixed
Status: needs_reviewclosed

i reviewed the pam_pwdfile.so source code (specifically pam_pwdfile.c) and I believe the following line will be safe and sufficient:

auth    requisite        pam_pwdfile.so pwdfile=/var/lib/misc/thishost/sudo-passwd

The full rationale is explained in the commit log:

commit 713de23ae1d484d870239b5f30d595cc224d71b2 (origin/sudo-ldap, sudo-ldap)
Author: Antoine Beaupré <anarcat@debian.org>
Date:   Wed Oct 16 11:19:21 2019 -0400

    use a standard keyword instead of closer coupling with pwdfile
    
    The rationale here is the interface with the pam module might change
    without notice. By explicitely coupling the expected return values of
    the module, we might inadvertedly misconfigure things.
    
    For example, the module configuration (authinfo_unavail=ignore,
    specifically) made it "fail open" (ie. return "ignore") if there was a
    configuration error (missing file or filename, locking error) while
    using the standard "requisite" will make it fail close (as default is
    "die").
    
    We use "requisite" instead of "required" because the former will
    immediately return in case of failure, skipping the rest of the stack,
    instead of falling through. We do not skip in case of success, but
    that might allow us to do other password checks later. The default
    will be success anyways so that should be okay.

I have deployed this change with Puppet everywhere and sent an announcement about the deployment on tor-project@:

https://lists.torproject.org/pipermail/tor-project/2019-October/002548.html

this is therefore all done.

Note: See TracTickets for help on using tickets.