[Merge] lp:~kirkland/pam/update-motd-now into lp:~ubuntu-core-dev/pam/ubuntu
Jamie Strandboge
jamie at ubuntu.com
Fri Jan 24 20:38:33 UTC 2014
Review: Needs Information
Thanks for the patch.
Minor nit: the second 'else' clause in debian/update-motd has the wrong indenting.
Couple of questions:
1. should this be an Ubuntu-only change or should it be uploaded to Debian first?
2. What is the intent of using "/usr/bin/env -i PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" before run-parts? On the surface, it seems like a good idea but it feels wrong to me. In Ubuntu, pam_env.so is called as part of 'session' before pam_motd.so. Looking at /etc/pam.d/login, I see this:
# 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
This seems to suggest that at least /etc/security/pam_env.conf, /etc/environment, and /etc/default/locale are all merged before pam_motd.so is consulted, and therefore things in /etc/update-motd.d may be dependent on these environment variables. Furthermore, on Ubuntu sudo already cleans the environment and I imagine a very high percentage of Ubuntu users of this command would run it under sudo.
I'm not sure what the solution would be for this especially if this is intended for Debian. Perhaps you can comment and Steve can comment further.
--
https://code.launchpad.net/~kirkland/pam/update-motd-now/+merge/202896
Your team Ubuntu Core Development Team is subscribed to branch lp:~ubuntu-core-dev/pam/ubuntu.
More information about the Ubuntu-reviews
mailing list