Comment 2 for bug 920749

Created attachment 1332
client-sent environment overrides PAM-read environment

This bug has been reported and discussed in the Debian BTS, see bugs #313317 and #408029 there.
The environment variables sent by AcceptEnv/SendEnv functionalities
should take precedence over PAM variable settings, especially for
locale and terminal related settings (or commands that are
locale-sensitive or terminal sensitive might give incomprehensible
gibberish as output to the user). TERM is already managed in a special
way, but not LANG or LC_* variables.
Currently, the variables LANG and LC_* are set (in a default debian
installation) by pam (/etc/pamd.d/ssh) which in turn reads
/etc/environment and /etc/default/locale. It happens dans in session.c
(function do_child) the environment of the child process is set as
follows: first, copy the environment set by AcceptEnv/SendEnv, set some
more variables (TERM, TZ, depending on the system), then use pam and
copy the PAM environment inside the child environment, thus clobbering
the useful variables sent through AcceptEnv/SendEnv.

Note that there is no way it could be fixed at the PAM level: PAM
prepares the environment for the child not knowing the sent variables.
It is openssh-server that does the things in the wrong order.

What the patch does: it changes the child_set_env function in
copy_environment to child_set_env_safe (basically the same as
child_set_env but with a twist): any variable which has already been
inserted in the environment is not clobbered by copy_environment.

Since the function copy_environment is the one used to bring the PAM
settings inside the environment, the PAM settings no more clobber the
environment sent by the AcceptEnv/SendEnv mechanism. Which yields
(from a client with LANG unset, and to a server with LANG=fr_FR.UTF-8 in
/etc/default/locale)

$ ssh penpen 'echo $LANG $(locale charmap)'
fr_FR.UTF-8 UTF-8
$ LANG=en_GB.UTF-8 ssh penpen 'echo $LANG $(locale charmap)'
en_GB.UTF-8 UTF-8
$ LANG=fr_FR@euro ssh penpen 'echo $LANG $(locale charmap)'
fr_FR@euro ISO-8859-15
$ LANG=fr_FR ssh penpen 'echo $LANG $(locale charmap)'
fr_FR ISO-8859-1

Since the current behaviour is to enforce the admin-set values, and thus
rendering the AcceptEnv/SendEnv almost useless, since critical variables
set in the environment can be enforced by the administrator by refusing
to accept them (in /etc/ssh/sshd_config) and since the default-accepted
environment variables are only limited to locale-related variables and
a default debian installation does not allow those variables to be used
(the "locales" package always sets LANG in /etc/default/locale), I think
this patch is worth being included in openssh-server. I also think it
free of security holes or memory leaks. I think it is worth being
transmitted upstream. I think some consideration should be given about
whether the "no clobber" behaviour should be the default one
(child_set_env is used several times in session.c and some should
probably consider using child_set_env_safe with the same rationale), but
it is part of a more general reflexion on this and does not interfere in
any way with these two bugs.