Bug#656750: [monkeysphere] Bug#656750: monkeysphere: wrongly preserves TMPDIR across accounts
On Wed 2019-01-16 18:40:06 -0800, Sunil Mohan Adapa wrote: > tags 656750 + patch Thanks for this, Sunil! I'll try to review it soon. Feel free to ping if i haven't moved on it in a few days. --dkg signature.asc Description: PGP signature
Bug#656750: [monkeysphere] Bug#656750: monkeysphere: wrongly preserves TMPDIR across accounts
tags 656750 + patch
thanks
On Mon, 23 Jan 2012 12:55:58 -0500 Daniel Kahn Gillmor
wrote:
> On 01/23/2012 12:19 PM, Jameson Graef Rollins wrote:
> > It occurs to me that we already have/use a tmp directory in the
> > monkeysphere authentication directory
> > (/var/lib/monkeysphere/authentication/tmp). Maybe we should just
> > explicitly set TMPDIR for the monkeysphere user to be that?
>
> Doing this and documenting it clearly seems like a reasonable approach
> to me.
>
> --dkg
>
The attached patch fixes the problem. Patch sets TMPDIR to
/var/lib/monkeysphere/authentication/tmp only when needed, but I have
tested other cases where temporary directory was being created.
* Need Change
- monkeysphere-host: add_revoker: with key from a remote server and with
key file.
- monkeysphere-host: publish_key:
- monkeysphere-authentication: add_certifier: with key from remote
server and with key file.
* No change needed:
- monkeysphere-host: show_key: does not use monkeysphere user.
- monkeysphere-host: revoke_key: does not use monkeysphere user. So any
temporary directory works.
- monkeysphere: import_subkey: does not use monkeysphere user. Not
implemented yet.
- monkeysphere: gen_subkey: does not use monkeysphere user.
- monkeysphere-authentication: update_users: Creates files that are fed
to less privileged process via stdin. Could not test properly.
With this change, I am hoping for a new release of monkeysphere suitable
for FreedomBox in buster.
Thanks,
--
Sunil
From 34b83f6ee26d527d415f033fd57db6bfe81eed90 Mon Sep 17 00:00:00 2001
From: Sunil Mohan Adapa
Date: Wed, 16 Jan 2019 16:15:21 -0800
Subject: [PATCH] Better sharing of temp directory across root and monkeysphere
user
In a couple of cases, monkeysphere commands running as run create a temporary
directory in TMPDIR (provided by environment) and then change the
ownership/permissions on that directory for monkeysphere user to use that
directory.
This works in a normal setup but fails when libpam-tmpdir is installed. This PAM
module causes the tmp directory to be /tmp/user/0/ so that it is harder to for
users to access each other temporary files. This improves security but causes
problem for above situation as the parent directory of the directory to be
shared is not allowed access by other users.
To fix this, explicitly set the TMPDIR to a known location that can be used to
share files across users. /var/lib/monkeysphere/authentication/tmp is a
directory that is already being setup and used for such purposes. Reuse it
instead of created a new one. Apply the fix conservatively only in cases needed.
Closes: #656750.
Signed-off-by: Sunil Mohan Adapa
---
src/monkeysphere-host | 9 +
src/share/ma/add_certifier | 15 ++-
src/share/mh/add_revoker | 13 +++--
src/share/mh/publish_key | 1 +
4 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/src/monkeysphere-host b/src/monkeysphere-host
index 089c2b6..1f47df0 100755
--- a/src/monkeysphere-host
+++ b/src/monkeysphere-host
@@ -30,6 +30,15 @@ MHSHAREDIR="${SYSSHAREDIR}/mh"
# datadir for host functions
MHDATADIR="${SYSDATADIR}/host"
+# temp directory to enable sharing a temporary directory between root and
+# monkeysphere users. This is needed so that on secure environments with
+# libpam-tmpdir, simply changing ownership/permissions on a directory is not
+# enough to share the directory. Parent directories such as /tmp/user/0 will be
+# inaccessible to monkeysphere user.
+#
+# XXX: Reusing monkeysphere-authentication's tmp directory.
+MHTMPDIR="${SYSDATADIR}/authentication/tmp"
+
# host pub key files
HOST_KEY_FILE="${SYSDATADIR}/host_keys.pub.pgp"
diff --git a/src/share/ma/add_certifier b/src/share/ma/add_certifier
index bd9885c..498bb68 100644
--- a/src/share/ma/add_certifier
+++ b/src/share/ma/add_certifier
@@ -98,15 +98,28 @@ if [ -f "$keyID" -o "$keyID" = '-' ] ; then
log verbose "reading key from file '$keyID'..."
fi
+TMPDIR=$MATMPDIR
+tmpDir=$(msmktempdir)
+trap "rm -rf $tmpDir" EXIT
+
+# fix permissions and ownership on temporary directory which will
+# be used by monkeysphere user
+chmod 0700 "$tmpDir"
+chown "$MONKEYSPHERE_USER":"$MONKEYSPHERE_GROUP" "$tmpDir"
+
# check the key is ok as monkeysphere user before loading
log debug "checking keys in file..."
-fingerprint=$(run_as_monkeysphere_user \
+fingerprint=$(run_as_monkeysphere_user /usr/bin/env TMPDIR=$tmpDir \
bash -c "$(printf ". %q && list_primary_fingerprints" "${SYSSHAREDIR}/common")" < "$keyID")
if [ $(printf "%s" "$fingerprint" | egrep -c '^[A-F0-9]{40}$') -ne 1 ] ; then
failure "There was not exactly one gpg key in the file."
fi
+# remove the temporary directory
+trap - EXIT
+rm -rf "$tmpDir"
+
# load the key
gpg_sphere --import <"$keyID" 2>/dev/null \
|| failure "could not read key from '$keyID'"
diff --git a/src/share/mh/add_revoker b/src/share/mh/add_re
Bug#656750: [monkeysphere] Bug#656750: monkeysphere: wrongly preserves TMPDIR across accounts
On 01/23/2012 12:19 PM, Jameson Graef Rollins wrote: > It occurs to me that we already have/use a tmp directory in the > monkeysphere authentication directory > (/var/lib/monkeysphere/authentication/tmp). Maybe we should just > explicitly set TMPDIR for the monkeysphere user to be that? Doing this and documenting it clearly seems like a reasonable approach to me. --dkg signature.asc Description: OpenPGP digital signature
Bug#656750: [monkeysphere] Bug#656750: monkeysphere: wrongly preserves TMPDIR across accounts
On Mon, 23 Jan 2012 09:47:06 -0500, Daniel Kahn Gillmor wrote: > I'm pretty sure i disagree with this; we actually may want to pass > environment variables across the switch-user call, and (for example) the > admin might want to set TMPDIR to instruct monkeysphere-authentication > where to place its tempfiles; if we were to reset (or clear) TMPDIR (or > other variables) across the privilege-drop, those attempts would fail. It occurs to me that we already have/use a tmp directory in the monkeysphere authentication directory (/var/lib/monkeysphere/authentication/tmp). Maybe we should just explicitly set TMPDIR for the monkeysphere user to be that? jamie. pgpKWEAJJsNVV.pgp Description: PGP signature
Bug#656750: [monkeysphere] Bug#656750: monkeysphere: wrongly preserves TMPDIR across accounts
On 12-01-23 at 09:47am, Daniel Kahn Gillmor wrote: > On 01/23/2012 06:54 AM, Jonas Smedegaard wrote: > > Yes, sounds most sensible to me that you _do_ reset all variables > > (i.e. spawn a login-like shell when switching user) and then pass > > explicitly what you want transfered. It is not like you need to > > support executing random user commands, only specific ones in your > > control, right? > > I'm pretty sure i disagree with this; we actually may want to pass > environment variables across the switch-user call, and (for example) > the admin might want to set TMPDIR to instruct > monkeysphere-authentication where to place its tempfiles; if we were > to reset (or clear) TMPDIR (or other variables) across the > privilege-drop, those attempts would fail. How do I then, as a local admin, tell your script - which switches user internally - to use a user-specific TMPDIR? > Basically, it would leave us open to bug reports like "monkeysphere: > does not honor TMPDIR" :P Heh. What would really be the meaning of such bugreport. I meant that PAM-resolved TMPDIR was not honored. If someone was to file such bugreport with the opposite meaning of not _preserving_ TMPDIR from the invoking environment, then I see no way to do so sensibly: That would require TMPDIR to be writable by both users, which is a bogus assumption to make. IMO you can address such bugreport by documenting that the command changes accounts internally and that TMPDIR of the switched-to user therefore need to be setup via PAM, not passed in environment. - Jonas -- * Jonas Smedegaard - idealist & Internet-arkitekt * Tlf.: +45 40843136 Website: http://dr.jones.dk/ [x] quote me freely [ ] ask before reusing [ ] keep private signature.asc Description: Digital signature
Bug#656750: [monkeysphere] Bug#656750: monkeysphere: wrongly preserves TMPDIR across accounts
On 01/23/2012 06:54 AM, Jonas Smedegaard wrote: > Yes, sounds most sensible to me that you _do_ reset all variables (i.e. > spawn a login-like shell when switching user) and then pass explicitly > what you want transfered. It is not like you need to support executing > random user commands, only specific ones in your control, right? I'm pretty sure i disagree with this; we actually may want to pass environment variables across the switch-user call, and (for example) the admin might want to set TMPDIR to instruct monkeysphere-authentication where to place its tempfiles; if we were to reset (or clear) TMPDIR (or other variables) across the privilege-drop, those attempts would fail. Basically, it would leave us open to bug reports like "monkeysphere: does not honor TMPDIR" :P > BTW why do you use "$*" instead of "$@"? The latter, I believe, > preserves the arguments (when quoted like that!) whereas the former does > not. https://labs.riseup.net/code/issues/442 --dkg signature.asc Description: OpenPGP digital signature
Bug#656750: [monkeysphere] Bug#656750: monkeysphere: wrongly preserves TMPDIR across accounts
retitle 656750 monkeysphere: wrongly preserves TMPDIR across accounts
thanks
On 12-01-22 at 11:54pm, Daniel Kahn Gillmor wrote:
> On 01/22/2012 11:33 PM, Jameson Graef Rollins wrote:
> > However, it seems to me that what you've shown is that monkeysphere
> > (gpg really) actually *is* respecting TMPDIR, given that you've set
> > it in the second example and it did change the behavior.
Oh, yeah - silly me :-)
> yep, i think this is the correct analysis. The main way that we
> switch to the non-privileged monkeysphere user is in the
> su_monkeysphere_user() function in /usr/share/monkeysphere/common,
> where it uses su.
>
> su "$MONKEYSPHERE_USER" -c "$*"
>
> i suppose it's possible to argue that we should be supplying the -l
> flag to su here, though i'm not sure what else that would break, since
> it would end up clearing the entire environment for the subprocess
> (which i'm pretty sure we don't want to do).
Yes, sounds most sensible to me that you _do_ reset all variables (i.e.
spawn a login-like shell when switching user) and then pass explicitly
what you want transfered. It is not like you need to support executing
random user commands, only specific ones in your control, right?
BTW why do you use "$*" instead of "$@"? The latter, I believe,
preserves the arguments (when quoted like that!) whereas the former does
not.
> > Jonas, what are you settings in /etc/pam.d/su? Do you see any
> > noticeable differences in regards to libpam-tmpdir between
> > /etc/pam.d/su and, say, /etc/pam.d/{login,sudo,etc}?
I did no customization to any files below /etc/pam.d.
Some of them attached.
Makes good sense to me that pam-tmpdir only kicks in on login-like
actions, not when environment is requested to not change (like su
without -l).
- Jonas
--
* Jonas Smedegaard - idealist & Internet-arkitekt
* Tlf.: +45 40843136 Website: http://dr.jones.dk/
[x] quote me freely [ ] ask before reusing [ ] keep private
#
# The PAM configuration file for the Shadow `su' service
#
# This allows root to su without passwords (normal operation)
auth sufficient pam_rootok.so
# Uncomment this to force users to be a member of group root
# before they can use `su'. You can also add "group=foo"
# to the end of this line if you want to use a group other
# than the default "root" (but this may have side effect of
# denying "root" user, unless she's a member of "foo" or explicitly
# permitted earlier by e.g. "sufficient pam_rootok.so").
# (Replaces the `SU_WHEEL_ONLY' option from login.defs)
# auth required pam_wheel.so
# Uncomment this if you want wheel members to be able to
# su without a password.
# auth sufficient pam_wheel.so trust
# Uncomment this if you want members of a specific group to not
# be allowed to use su at all.
# auth required pam_wheel.so deny group=nosu
# Uncomment and edit /etc/security/time.conf if you need to set
# time restrainst on su usage.
# (Replaces the `PORTTIME_CHECKS_ENAB' option from login.defs
# as well as /etc/porttime)
# accountrequisite pam_time.so
# This module parses environment configuration file(s)
# and also allows you to use an extended config
# file /etc/security/pam_env.conf.
#
# parsing /etc/environment needs "readenv=1"
session required pam_env.so readenv=1
# locale variables are also kept into /etc/default/locale in etch
# reading this file *in addition to /etc/environment* does not hurt
session required pam_env.so readenv=1 envfile=/etc/default/locale
# Defines the MAIL environment variable
# However, userdel also needs MAIL_DIR and MAIL_FILE variables
# in /etc/login.defs to make sure that removing a user
# also removes the user's mail spool file.
# See comments in /etc/login.defs
#
# "nopen" stands to avoid reporting new mail when su'ing to another user
sessionoptional pam_mail.so nopen
# Sets up user limits, please uncomment and read /etc/security/limits.conf
# to enable this functionality.
# (Replaces the use of /etc/limits in old login)
# sessionrequired pam_limits.so
# The standard Unix authentication modules, used with
# NIS (man nsswitch) as well as normal /etc/passwd and
# /etc/shadow entries.
@include common-auth
@include common-account
@include common-session
#%PAM-1.0
@include common-auth
@include common-account
session required pam_permit.so
session required pam_limits.so
#
# /etc/pam.d/common-session - session-related modules common to all services
#
# This file is included from other service-specific PAM config files,
# and should contain a list of modules that define tasks to be performed
# at the start and end of sessions of *any* kind (both interactive and
# non-interactive).
#
# As of pam 1.0.1-6, this file is managed by pam-auth-update by default.
# To take advantage of this, it is recommended that you configure any
# local modules either before or after the default block, and use
# pam-auth-update to manage selection of other modules. See
# pam
