Thank you for the feedback. On Friday 26 October 2018 05:41 AM, Daniel Kahn Gillmor wrote: [...] >> # requote arguments using bash builtin feature (see "help >> printf"): >> - su "$MONKEYSPHERE_USER" -s "$(which bash)" -c "$(printf "%q " "$@")" >> + runuser -u "$MONKEYSPHERE_USER" -- "$@" >> ;; > > This makes several different changes besides just moving to runuser. in > particular, it drops the -s "$(which bash)" (and therefore i think > relies on the monkeysphere user's chosen shell in /etc/passwd -- > hopefully that's correct, but it might have changed somehow). Would you > be ok with reinstating the -s "$(which bash)" arguments?
-s "$(which bash)" was necessary to fix #827660 because su, which uses user's shell, was used and monkeysphere user had a shell other than bash. runuser does not seem to depend on the user's shell. It simply runs a given process under that user's id (with a different PAM configuration). This is better at least for the purposes we are using it. Whenever evaluating bash expressions as a user, we are already doing 'su_monkeysphere_user bash -c "<EXPR>"'. # usermod -s /usr/sbin/nologin monkeysphere # su monkeysphere ls This account is currently not available. # runuser --user monkeysphere ls <output> I am additionally proposing another patch which will remove the shell for the monkeysphere user in Debian packaging. This closes #901489. We don't need the monkeysphere user's shell. > > > And, it changes how the invocation is passed through -- rather than > using -c (which can pass shell-specific data) it passes the arguments > directly to runuser. This is fine for most cases, but it might fail if > invoked as something like: > > su_monkeysphere_user echo test '>' /tmp/foo > > this could do two different things: > > a) echo "test", the literal ">" symbol, and the string "/tmp/foo" > b) echo "test" into the file "/tmp/foo" as the monkeysphere user. > > The current code seems to assume that the you might want to pass shell > metacharacters through to be executed that way, and that (b) should be > the correct result. I think it's a bad idea to do that, though, and i > prefer your simpler formulation. > > I'm concerned about the complex constructions (sigh, shell is such a > mess) passed to su_monkeysphere_user in update_users and add_certifier > (for monkeysphere-authentication), and add_revoker (for > monkeysphere-host), though. Have you tested those subcommands on a live > system as the superuser? > > If we can go with your proposed simpler argument passing, we definitely > should! > I didn't test all the situations before, but in this round I did. I ran all commands with bash -x and ensured that every invocation of the method in all branches was indeed triggered. I explicitly changed the name of the method to 'run_as_monkeysphere_user' and changed all invocations to make them reflect the changed semantics and also to see the full impact. All the invocations fall into the these categories: 1) Simple invocation of gpg/openpgp2ssh command without passing environment variables and shell expressions. There is no issues here. 2) Execution of bash snippets. All of these executions where already wrapped with bash -c "<EXPR>". So they all work as is with runuser. 3) Execution of commands with environment variable passing. Since runuser preserves environment from the parent process, we can now say `KEY=VALUE runuser -u monkeysphere gpg` instead of the shell expression `su monkeysphere KEY=VALUE gpg`. I have updated the patch to handle these situations properly. # TESTVAR1='Hello!' runuser -u monkeysphere -- bash -c 'set' |grep TESTVAR1 TESTVAR1='Hello!' 4) Redirection expressions like what you have described above currently do not exist. I added documentation that bash expressions should not be run using the method. It looks like we can keep the simplification. Thanks, -- Sunil PS: I ran into issues with TMPDIR as described in #656750 and I am currently working around them.
From cfc416ac9a99843573c245033f293549ad3520b9 Mon Sep 17 00:00:00 2001
From: Sunil Mohan Adapa <su...@medhas.org>
Date: Thu, 25 Oct 2018 14:43:57 -0700
Subject: [PATCH] Use runuser instead of su
On systems with restricted PAM security, it may not possible to use su.
---
src/monkeysphere-authentication | 6 +++---
src/monkeysphere-host | 2 +-
src/share/common | 20 +++++++-------------
src/share/ma/add_certifier | 2 +-
src/share/ma/update_users | 4 ++--
src/share/mh/add_revoker | 10 +++++-----
src/share/mh/publish_key | 4 ++--
7 files changed, 21 insertions(+), 27 deletions(-)
diff --git a/src/monkeysphere-authentication b/src/monkeysphere-authentication
index b3eb1e6..8d6bee0 100755
--- a/src/monkeysphere-authentication
+++ b/src/monkeysphere-authentication
@@ -81,11 +81,11 @@ gpg_sphere() {
GNUPGHOME="$GNUPGHOME_SPHERE"
export GNUPGHOME
- su_monkeysphere_user gpg --fixed-list-mode --no-greeting --quiet --no-tty "$@"
+ run_as_monkeysphere_user gpg --fixed-list-mode --no-greeting --quiet --no-tty "$@"
}
check_openpgp2ssh_sanity() {
- if [[ `su_monkeysphere_user openpgp2ssh ABC &>/dev/null || echo $?` != "255" ]]; then
+ if [[ `run_as_monkeysphere_user openpgp2ssh ABC &>/dev/null || echo $?` != "255" ]]; then
echo "openpgp2ssh command gives unexpected return code. This can lead to a scenario where no authorized keys are populated, even though they are otherwise valid. Aborting!"
exit 1
fi;
@@ -137,7 +137,7 @@ GNUPGHOME_SPHERE=${MONKEYSPHERE_GNUPGHOME_SPHERE:="${MADATADIR}/sphere"}
CORE_KEYLENGTH=${MONKEYSPHERE_CORE_KEYLENGTH:="2048"}
LOG_PREFIX=${MONKEYSPHERE_LOG_PREFIX:='ms: '}
-# export variables needed in su invocation
+# export variables needed for invoking command under monkeysphere user
export DATE
export LOG_LEVEL
export KEYSERVER
diff --git a/src/monkeysphere-host b/src/monkeysphere-host
index 75895e9..089c2b6 100755
--- a/src/monkeysphere-host
+++ b/src/monkeysphere-host
@@ -360,7 +360,7 @@ PROMPT=${MONKEYSPHERE_PROMPT:=$PROMPT}
GNUPGHOME_HOST=${MONKEYSPHERE_GNUPGHOME_HOST:="${MHDATADIR}"}
LOG_PREFIX=${MONKEYSPHERE_LOG_PREFIX:='ms: '}
-# export variables needed in su invocation
+# export variables needed for invoking command under monkeysphere user
export DATE
export LOG_LEVEL
export KEYSERVER
diff --git a/src/share/common b/src/share/common
index 80ae88a..cccdaaa 100644
--- a/src/share/common
+++ b/src/share/common
@@ -93,20 +93,14 @@ log() {
}
# run command as monkeysphere user
-su_monkeysphere_user() {
+run_as_monkeysphere_user() {
# our main goal here is to run the given command as the the
# monkeysphere user, but without prompting for any sort of
# authentication. If this is not possible, we should just fail.
-
- # FIXME: our current implementation is overly restrictive, because
- # there may be some su PAM configurations that would allow su
- # "$MONKEYSPHERE_USER" -c "$@" to Just Work without prompting,
- # allowing specific users to invoke commands which make use of
- # this user.
-
- # chpst (from runit) would be nice to use, but we don't want to
- # introduce an extra dependency just for this. This may be a
- # candidate for re-factoring if we switch implementation languages.
+ #
+ # A simple command and its arguments are expected. Shell
+ # expressions are not supported. If they are required, they may
+ # be executed with 'bash -c "<EXPR>"'.
case $(id -un) in
# if monkeysphere user, run the command as a subshell
@@ -114,10 +108,10 @@ su_monkeysphere_user() {
( "$@" )
;;
- # if root, su command as monkeysphere user
+ # if root, run command as monkeysphere user
'root')
# requote arguments using bash builtin feature (see "help printf"):
- su "$MONKEYSPHERE_USER" -s "$(which bash)" -c "$(printf "%q " "$@")"
+ runuser -u "$MONKEYSPHERE_USER" -- "$@"
;;
# otherwise, fail
diff --git a/src/share/ma/add_certifier b/src/share/ma/add_certifier
index 05f4b44..bd9885c 100644
--- a/src/share/ma/add_certifier
+++ b/src/share/ma/add_certifier
@@ -100,7 +100,7 @@ if [ -f "$keyID" -o "$keyID" = '-' ] ; then
# check the key is ok as monkeysphere user before loading
log debug "checking keys in file..."
- fingerprint=$(su_monkeysphere_user \
+ fingerprint=$(run_as_monkeysphere_user \
bash -c "$(printf ". %q && list_primary_fingerprints" "${SYSSHAREDIR}/common")" < "$keyID")
if [ $(printf "%s" "$fingerprint" | egrep -c '^[A-F0-9]{40}$') -ne 1 ] ; then
diff --git a/src/share/ma/update_users b/src/share/ma/update_users
index a0ec21b..c15f782 100644
--- a/src/share/ma/update_users
+++ b/src/share/ma/update_users
@@ -78,8 +78,8 @@ for uname in $unames ; do
log verbose "processing authorized_user_ids..."
# process authorized_user_ids file, as monkeysphere user
- su_monkeysphere_user \
- /usr/bin/env "STRICT_MODES=$STRICT_MODES" bash -c "$(printf ". %q && process_authorized_user_ids -" "${SYSSHAREDIR}/common")"\
+ STRICT_MODES="$STRICT_MODES" run_as_monkeysphere_user \
+ bash -c "$(printf ". %q && process_authorized_user_ids -" "${SYSSHAREDIR}/common")"\
< "$authorizedUserIDs" \
> "$tmpAuthorizedKeys"
diff --git a/src/share/mh/add_revoker b/src/share/mh/add_revoker
index 7fafabd..9e9a83b 100644
--- a/src/share/mh/add_revoker
+++ b/src/share/mh/add_revoker
@@ -51,7 +51,7 @@ if [ -f "$revokerKeyID" -o "$revokerKeyID" = '-' ] ; then
# check the key is ok as monkeysphere user before loading
log debug "checking keys in file..."
- fingerprint=$(su_monkeysphere_user \
+ fingerprint=$(run_as_monkeysphere_user \
bash -c "$(printf ". %q && list_primary_fingerprints" "${SYSSHAREDIR}/common")" < "$revokerKeyID")
if [ $(printf "%s" "$fingerprint" | egrep -c '^[A-F0-9]{40}$') -ne 1 ] ; then
@@ -71,12 +71,12 @@ else
# download the key from the keyserver as the monkeysphere user
log verbose "searching keyserver $KEYSERVER for revoker keyID $revokerKeyID..."
- su_monkeysphere_user "GNUPGHOME=$tmpDir" gpg --quiet --keyserver "$KEYSERVER" --recv-key "0x${revokerKeyID}!" \
+ GNUPGHOME="$tmpDir" run_as_monkeysphere_user gpg --quiet --keyserver "$KEYSERVER" --recv-key "0x${revokerKeyID}!" \
|| failure "Could not receive a key with this ID from keyserver '$KEYSERVER'."
# get the full fingerprint of new revoker key
log debug "getting fingerprint of revoker key..."
- fingerprint=$(su_monkeysphere_user "GNUPGHOME=$tmpDir" gpg --list-key --with-colons --with-fingerprint "${revokerKeyID}" \
+ fingerprint=$(GNUPGHOME="$tmpDir" run_as_monkeysphere_user gpg --list-key --with-colons --with-fingerprint "${revokerKeyID}" \
| awk -F: '/^fpr:/{ if (ok) { print $10 }; ok=0 } /^pub:/{ ok=1 }')
# test that there is only a single fingerprint
@@ -90,7 +90,7 @@ EOF
fi
log info "revoker key found:"
- su_monkeysphere_user "GNUPGHOME=$tmpDir" gpg --fingerprint "0x${fingerprint}!"
+ GNUPGHOME="$tmpDir" run_as_monkeysphere_user gpg --fingerprint "0x${fingerprint}!"
if [ "$PROMPT" = "true" ] ; then
printf "Are you sure you want to add the above key as a revoker\nof the key '$keyID'? (Y/n) " >&2
@@ -104,7 +104,7 @@ EOF
# export the new key to the host keyring
log debug "loading revoker key into host keyring..."
- su_monkeysphere_user "GNUPGHOME=$tmpDir" gpg --quiet --export "0x${fingerprint}!" \
+ GNUPGHOME="$tmpDir" run_as_monkeysphere_user gpg --quiet --export "0x${fingerprint}!" \
| gpg_host --import
fi
diff --git a/src/share/mh/publish_key b/src/share/mh/publish_key
index 6987a0a..affb5fb 100644
--- a/src/share/mh/publish_key
+++ b/src/share/mh/publish_key
@@ -45,7 +45,7 @@ cleanup() {
trap cleanup EXIT
# import the key into the tmp dir
-su_monkeysphere_user \
+run_as_monkeysphere_user \
gpg --quiet --import <"$HOST_KEY_FILE"
ANCHORFILE=""
@@ -58,7 +58,7 @@ done
# publish key
log debug "publishing key with the following gpg command line and options:"
-su_monkeysphere_user \
+run_as_monkeysphere_user \
gpg --keyserver "$KEYSERVER" ${ANCHORFILE:+--keyserver-options "ca-cert-file=$ANCHORFILE"} --send-keys "0x${keyID}!"
# remove the tmp file
--
2.19.1
From 7bf2883aef723dce2a6179899ad01f9ed7b0e237 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa <su...@medhas.org> Date: Sat, 27 Oct 2018 11:00:04 -0700 Subject: [PATCH] debian: Remove shell for monkeysphere user Monkeysphere now uses 'runuser' instead of 'su' to perform operation using the monkeysphere user. 'runuser' works when there is no shell for the user. When freshly installing, create a monkeysphere user without a shell. If the monkeysphere user is already present on the system the shell for that user must be removed. Closes #901489. --- debian/monkeysphere.postinst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/debian/monkeysphere.postinst b/debian/monkeysphere.postinst index 8c6a555..a140ab2 100755 --- a/debian/monkeysphere.postinst +++ b/debian/monkeysphere.postinst @@ -16,9 +16,10 @@ case $1 in echo "adding monkeysphere user..." adduser --quiet --system --no-create-home --group \ --home "$VARLIB" \ - --shell '/bin/bash' \ --gecos 'monkeysphere authentication user,,,' \ monkeysphere + else + usermod --shell /usr/sbin/nologin monkeysphere fi # try all available transitions: @@ -29,7 +30,6 @@ case $1 in exit $RET } done - # setup monkeysphere authentication monkeysphere-authentication setup -- 2.19.1
signature.asc
Description: OpenPGP digital signature
