Bug#313317: patch to fix 313317 and 408029 (openssh-server: PAM environment and AcceptEnv)
Package: openssh-server
Tags: patch
Followup-For: Bug #313317
Hello,
The rationale for these two merged bugs (313317 and 408029) is the same:
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.
AcceptEnv/SendEnv is a mechanism that allows the environment to be
passed from the local machine (ssh-client) to the remote machine
(ssh-server). The default debian installation sends LANG and all
variables matching LC_* (which is a good idea, and done by other
distributions).
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.
Moreover, I did not raise the importance of this bug, but for the
record, this bug is one that seriously hampers the UTF-8 transition
(since some machines are still in ISO-8859 charsets, people that access
both have to cope manually with guessing what is the best encoding). For
this reason, I know a few computer scientists that decided to stay in
ISO-8859 encoding (since their remote machines do not recognise
automatically the encoding they are in, and are usually set in
ISO-8859).
-- System Information:
Debian Release: 4.0
APT prefers unstable
APT policy: (500, 'unstable'), (1, 'experimental')
Architecture: i386 (i686)
Shell: /bin/sh linked to /bin/bash
Kernel: Linux 2.6.18-1-686
Locale: LANG=fr_FR.UTF-8, LC_CTYPE=fr_FR.UTF-8 (charmap=UTF-8)
Versions of packages openssh-server depends on:
ii adduser 3.102 Add and remove users and groups
ii debconf 1.5.13 Debian configuration management sy
ii dpkg 1.13.25 package maintenance system for Deb
ii libc6 2.3.6.ds1-13 GNU C Library: Shared libraries
ii libcomer 1.39+1.40-WIP-2006.11.14+dfsg-2 common error description library
ii libkrb53 1.4.4-8 MIT Kerberos runtime libraries
ii libpam-m 0.79-4.1 Pluggable Authentication Modules f
ii libpam-r 0.79-4.1 Runtime support for the PAM librar
ii libpam0g 0.79-4.1 Pluggable Authentication Modules l
ii libselin 1.32-3 SELinux shared libraries
ii libssl0. 0.9.8e-4 SSL shared libraries
ii libwrap0 7.6.dbs-13 Wietse Venema's TCP wrappers libra
ii openssh- 1:4.3p2-9.1 Secure shell client, an rlogin/rsh
ii zlib1g 1:1.2.3-13 compression library - runtime
openssh-server recommends no packages.
-- debconf information:
ssh/insecure_rshd:
ssh/insecure_telnetd:
ssh/new_config: true
* ssh/use_old_init_script: true
ssh/disable_cr_auth: false
ssh/encrypted_host_key_but_no_keygen:
diff -ur ./debian/changelog ../openssh-4.3p2.orig/debian/changelog
--- ./debian/changelog 2007-04-08 13:25:45.000000000 +0200
+++ ../openssh-4.3p2.orig/debian/changelog 2007-04-08 13:16:21.000000000 +0200
@@ -1,10 +1,3 @@
-openssh (1:4.3p2-9.1) unstable; urgency=low
-
- * Add patch so that client-sent environment overrides PAM-read environment
- (closes: #313317, #392856, #408029)
-
- -- Jean-Christophe Dubacq <jcdubacq1@free.fr> Sun, 08 Apr 2007 13:21:46 +0200
-
openssh (1:4.3p2-9) unstable; urgency=high
[ Russ Allbery ]
diff -ur ./session.c ../openssh-4.3p2.orig/session.c
--- ./session.c 2007-04-08 13:34:45.000000000 +0200
+++ ../openssh-4.3p2.orig/session.c 2007-04-08 13:16:21.000000000 +0200
@@ -788,11 +788,11 @@
/*
* Sets the value of the given variable in the environment. If the variable
- * already exists, its value is kept/overriden according to clobber.
+ * already exists, its value is overriden.
*/
void
-child_set_env_safe(char ***envp, u_int *envsizep, const char *name,
- const char *value, char clobber)
+child_set_env(char ***envp, u_int *envsizep, const char *name,
+ const char *value)
{
char **env;
u_int envsize;
@@ -818,10 +818,7 @@
for (i = 0; env[i]; i++)
if (strncmp(env[i], name, namelen) == 0 && env[i][namelen] == '=')
break;
- if (env[i] && !clobber) {
- /* No use for this variable: already exists and no clobber */
- return;
- } else if (env[i] && clobber) {
+ if (env[i]) {
/* Reuse the slot. */
xfree(env[i]);
} else {
@@ -844,17 +841,6 @@
}
/*
- * Sets the value of the given variable in the environment. If the variable
- * already exists, its value is overriden.
- */
-void
-child_set_env(char ***envp, u_int *envsizep, const char *name,
- const char *value)
-{
- child_set_env_safe(envp,envsizep,name,value,1);
-}
-
-/*
* Reads environment variables from the given file and adds/overrides them
* into the environment. If the file does not exist, this does nothing.
* Otherwise, it must consist of empty lines, comments (line starts with '#')
@@ -972,7 +958,7 @@
*var_val++ = '\0';
debug3("Copy environment: %s=%s", var_name, var_val);
- child_set_env_safe(env, envsize, var_name, var_val,0);
+ child_set_env(env, envsize, var_name, var_val);
xfree(var_name);
}
Reply to: