[PATCH] Read /etc/environment first to allow changing environment from user profile

[PATCH] Read /etc/environment first to allow changing environment from user profile

[Carlos Sánchez de La Lama]

[-- Attachment #1: Type: text/plain, Size: 815 bytes --]

Hi all,

as reported in bug #22175, lshd does not honor /etc/environment. This
was fixed here:

http://git.savannah.gnu.org/cgit/guix.git/commit/gnu/system.scm?id=2a5f0db4c45679cac6a747a48993fe73982cadca

However, the order in /etc/profile is problematic: some variables are
set up by "$HOME/.guix-profile/etc/profile", but then they get (wrongly)
overriden by the values in /etc/environment. In my case, this happens
with SSL_CERT_DIR, which has the value

/home/csanchez/.guix-profile/etc/ssl/certs:/etc/ssl/certs

then logging in locally, but only

/etc/ssl/certs

when logging in from lshd.

I attach the proposed patch (just a change of order in /etc/profile). As
'cat' and 'cut' ar most surely available at system-level, it should not
be dangerous to use them before setting up the user profile.

BR

Carlos


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Read-etc-environment-first-to-allow-changing-environ.patch --]
[-- Type: text/x-patch, Size: 1688 bytes --]

From 474e8980ee933e6694cc55ca61607adae86dacf1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20S=C3=A1nchez=20de=20La=20Lama?=
 <csanchezdll@gmail.com>
Date: Wed, 27 Jul 2016 14:27:00 +0200
Subject: [PATCH] Read /etc/environment first to allow changing environment
 from user profile.

---
 gnu/system.scm | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 476720b..3ae4ae7 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -408,6 +408,16 @@ GUIX_PROFILE=/run/current-system/profile \\
 # Prepend setuid programs.
 export PATH=/run/setuid-programs:$PATH
 
+# Since 'lshd' does not use pam_env, /etc/environment must be explicitly
+# loaded when someone logs in via SSH.  See <http://bugs.gnu.org/22175>.
+# We need 'PATH' to be defined here, for 'cat' and 'cut'.
+if [ -f /etc/environment -a -n \"$SSH_CLIENT\" \\
+     -a -z \"$LINUX_MODULE_DIRECTORY\" ]
+then
+  . /etc/environment
+  export `cat /etc/environment | cut -d= -f1`
+fi
+
 if [ -f \"$HOME/.guix-profile/etc/profile\" ]
 then
   # Load the user profile's settings.
@@ -419,16 +429,6 @@ else
   export PATH=\"$HOME/.guix-profile/bin:$PATH\"
 fi
 
-# Since 'lshd' does not use pam_env, /etc/environment must be explicitly
-# loaded when someone logs in via SSH.  See <http://bugs.gnu.org/22175>.
-# We need 'PATH' to be defined here, for 'cat' and 'cut'.
-if [ -f /etc/environment -a -n \"$SSH_CLIENT\" \\
-     -a -z \"$LINUX_MODULE_DIRECTORY\" ]
-then
-  . /etc/environment
-  export `cat /etc/environment | cut -d= -f1`
-fi
-
 # Set the umask, notably for users logging in via 'lsh'.
 # See <http://bugs.gnu.org/22650>.
 umask 022
-- 
2.7.3


[-- Attachment #3: Type: text/plain, Size: 216 bytes --]


-- 
'You did very well, Mr. Netherton,' she said.
'I scarcely did anything.'
'Opportunities to do very badly were manyfold. You avoided them. The major part
in any success.'

William Gibson, "The Peripheral" (2014)

Re: [PATCH] Read /etc/environment first to allow changing environment from user profile

[Tobias Geerinckx-Rice]

Carlos,

On 27/07/2016 14:34, Carlos Sánchez de La Lama wrote:
> I attach the proposed patch (just a change of order in /etc/profile). As
> 'cat' and 'cut' are most surely available at system-level, it should not
> be dangerous to use them before setting up the user profile.

That's a valid point, but this should be doable in pure shell. It might
even be faster (untested), and avoids needless forking.

Most importantly, it's fun.

> . /etc/environment
> export `cat /etc/environment | cut -d= -f1`

‘cat file | ...’ can be replaced with ‘... < file’; ‘cut’ isn't needed
since export accepts ‘VAR[=value]’ arguments[1][2]. You've just imported
‘/etc/environment’ on the previous line, and the values haven't been
modified. Hence, the two lines above can be written as:

  while read line; do export "$line"; done < /etc/environment

I don't know if Guix even cares about non-bash shells, but this should™
work in all POSIX®-compliant ones[4]. It's also more readable.

Thoughts? Gotchas?

Kind regards,

T G-R

[1]: http://pubs.opengroup.org/onlinepubs/009696799/utilities/export.html
[2]: even if it didn't, section 2.6.2 of [3] has got you covered
[3]:
http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html
[4]: which reminds me to finish packaging ash for Guix

Re: [PATCH] Read /etc/environment first to allow changing environment from user profile

[Danny Milosavljevic]

>   while read line; do export "$line"; done < /etc/environment
> 
> I don't know if Guix even cares about non-bash shells, but this should™
> work in all POSIX®-compliant ones[4]. It's also more readable.
> 
> Thoughts? Gotchas?

Spaces in keys and/or values don't work that way. The read itself already has problems with them. You might try setting IFS="
" but not sure whether that's standards-compliant.

Also, does csh actually support "export"? The one I have in Solaris doesn't. You need to do "setenv" there.

Re: [PATCH] Read /etc/environment first to allow changing environment from user profile

[Tobias Geerinckx-Rice]

Hi Danny,

On 27/07/2016 18:31, Danny Milosavljevic wrote:
>> while read line; do export "$line"; done < /etc/environment
>> 
>> I don't know if Guix even cares about non-bash shells, but this 
>> should™ work in all POSIX®-compliant ones[4]. It's also more 
>> readable.
>> 
>> Thoughts? Gotchas?
> 
> Spaces in keys and/or values don't work that way.
> The read itself already has problems with them.

No. Perhaps you're confusing it with ‘read < /etc/environment’.

Also, there are no spaces in keys, and technically, spaces in values
work just fine: it's the quotes that don't get dropped. D'oh! :-P

Yeah, trying to emulate unquoting (and all manner of escaping that it
entails) would be utter madness. Uglier option [2] it is then:

  . /etc/environment
  while read line; export "${line%%=*}"; done < /etc/environment

It's ugly and racy and less straightforward.
But then so was the original, I guess.

> You might try setting IFS="
> " but not sure whether that's standards-compliant.

Setting IFS to a newline doesn't make sense here.

To be honest, I've a personal dislike for IFS, though it is
POSIX-compliant. Saving IFS, setting IFS to ‘=’, running ‘read key
value’, ignoring ‘value’, and restoring IFS would work.

> Also, does csh actually support "export"? The one I have in Solaris 
> doesn't. You need to do "setenv" there.

csh isn't a (POSIX) sh.

Thanks for the bug spotting,

T G-R

[1]: It seems to be standard, but I didn't bother testing.

> On 27/07/2016 18:02, Tobias Geerinckx-Rice wrote:
> [2]: even if it didn't, section 2.6.2 of [3] has got you covered
> [3]:
http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html

Re: [PATCH] Read /etc/environment first to allow changing environment from user profile

[Tobias Geerinckx-Rice]

On 27/07/2016 19:18, Tobias Geerinckx-Rice wrote:
> [1]: It seems to be standard, but I didn't bother testing.

Ignore this forgotten line. I couldn't help myself and did bother.
Unfortunately, it worked.

Re: [PATCH] Read /etc/environment first to allow changing environment from user profile

[Ludovic Courtès]

Hi,

csanchezdll@gmail.com (Carlos Sánchez de La Lama) skribis:

> as reported in bug #22175, lshd does not honor /etc/environment. This
> was fixed here:
>
> http://git.savannah.gnu.org/cgit/guix.git/commit/gnu/system.scm?id=2a5f0db4c45679cac6a747a48993fe73982cadca
>
> However, the order in /etc/profile is problematic: some variables are
> set up by "$HOME/.guix-profile/etc/profile", but then they get (wrongly)
> overriden by the values in /etc/environment. In my case, this happens
> with SSL_CERT_DIR, which has the value
>
> /home/csanchez/.guix-profile/etc/ssl/certs:/etc/ssl/certs
>
> then logging in locally, but only
>
> /etc/ssl/certs
>
> when logging in from lshd.

Indeed, good catch!

> I attach the proposed patch (just a change of order in /etc/profile). As
> 'cat' and 'cut' ar most surely available at system-level, it should not
> be dangerous to use them before setting up the user profile.

Yes.

Tobias: if you want to avoid the ‘cut’ invocation, feel free to send
another patch.  :-)

> From 474e8980ee933e6694cc55ca61607adae86dacf1 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Carlos=20S=C3=A1nchez=20de=20La=20Lama?=
>  <csanchezdll@gmail.com>
> Date: Wed, 27 Jul 2016 14:27:00 +0200
> Subject: [PATCH] Read /etc/environment first to allow changing environment
>  from user profile.
>

I tested in ‘guix system vm’ and it seems to work as expected.  Pushed
as cad7e6abafc14de220265e09a0fc4bbd96664599 with a commit log that
follows our conventions.

Thank you!

Ludo’.