[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