sudo, pkexec don't apply global environment settings from /etc/environment

Bug #982684 reported by Steve Langasek
108
This bug affects 22 people
Affects Status Importance Assigned to Milestone
PolicyKit
Fix Released
Medium
policykit-1 (Ubuntu)
Fix Released
Medium
Steve Langasek
Precise
Fix Released
Medium
Steve Langasek
Quantal
Fix Released
Medium
Steve Langasek
sudo (Ubuntu)
Fix Released
Medium
Tyler Hicks
Precise
Fix Released
Medium
Steve Langasek
Quantal
Fix Released
Medium
Tyler Hicks

Bug Description

[Impact]
In connection with the recent update-notifier changes (https://wiki.ubuntu.com/Specs/UpdateNotifierPackageDataDownloader), some users who were previously able to download the flashplugin via the apt proxy settings are now unable to download it when running, e.g., 'sudo apt-get install'.

The reason for this is that, even though a global proxy may be configured in /etc/environment, sudo does not allow $http_proxy to be inherited by default and does not reapply the environment from /etc/environment (and from /etc/default/locale) via pam_env.

The first part is reasonable, but I question the second part. Since these are global config files, I believe it's safe for sudo to apply the environment settings by default just as 'su' does; and the settings are intended to apply globally, which would include to sudo sessions.

This would make update-notifier work more reliably for users with proxies, and would probably help with a variety of other cases where global variables are currently not being set as expected for sudo.

[Test Case]
1. Set 'http_proxy=invalid' in /etc/environment
2. Run 'sudo wget http://www.ubuntu.com/', 'pkexec wget http://www.ubuntu.com/'
3. Verify that the commands return successfully
4. Install updated sudo, policykit-1 packages from precise-proposed
5. Repeat the commands from step 2
6. Verify that the commands now fail with an error about invalid proxies
7. Remove the http_proxy line from /etc/environment

[Regression potential]
Some users may consider it a feature that global settings from /etc/environment are not applied to sudo and/or pkexec. However, this is not by design; the cost of not being able to correctly support proxies for users is greater than the cost of changing this behavior in an SRU and breaking expectations of users regarding undocumented behavior.

ProblemType: Bug
DistroRelease: Ubuntu 12.04
Package: sudo 1.8.3p1-1ubuntu3
ProcVersionSignature: Ubuntu 3.2.0-22.35-generic 3.2.14
Uname: Linux 3.2.0-22-generic x86_64
ApportVersion: 2.0.1-0ubuntu3
Architecture: amd64
CheckboxSubmission: 017452a27eca3c8b498abbfa5ef91db9
CheckboxSystem: ecaaad6fa1e0799a0aa1126bf620f39e
Date: Sun Apr 15 16:41:17 2012
InstallationMedia: Ubuntu 10.04.1 LTS "Lucid Lynx" - Release amd64 (20100816.1)
ProcEnviron:
 TERM=xterm
 PATH=(custom, user)
 LANG=en_US.UTF-8
 SHELL=/bin/bash
SourcePackage: sudo
UpgradeStatus: Upgraded to precise on 2011-11-08 (159 days ago)

Revision history for this message
Steve Langasek (vorlon) wrote :
Changed in sudo (Ubuntu):
status: New → Triaged
importance: Undecided → Medium
Steve Langasek (vorlon)
Changed in sudo (Ubuntu Precise):
importance: Undecided → Medium
status: New → Triaged
assignee: nobody → Steve Langasek (vorlon)
Changed in sudo (Ubuntu Quantal):
assignee: nobody → Steve Langasek (vorlon)
status: Triaged → In Progress
Revision history for this message
zub (zub-linux) wrote :

I just found another usecase where fixing this issue helps:

sudo apt-add-repository FOO

does not work when http_proxy is required. (As http_proxy from env. is not set and apt's proxy is not used.)

Revision history for this message
zub (zub-linux) wrote :

Not to mention that when user does not set proxy for apt (in /etc/apt/apt.conf.d), then neither "sudo apt-get ..." nor "sudo aptitude ... " nor update-manager at large work. And my guess is that whatever is the GUI tool provided in Ubuntu to set proxy, it does not configure apt proxy.
One has to resort to sudo -i followed by the command. Then proxy is set.
Solving this ticket should also solve these issues.

Revision history for this message
Daniel Steglich (steglich) wrote :

same problem occures during Flash Plugin installation/update by using /usr/lib/update-notifier/package-data-downloader

Steve Langasek (vorlon)
Changed in sudo (Ubuntu Quantal):
assignee: Steve Langasek (vorlon) → Tyler Hicks (tyhicks)
Steve Langasek (vorlon)
summary: - sudo doesn't apply global environment settings from /etc/environment
+ sudo, pkexec don't apply global environment settings from
+ /etc/environment
Changed in policykit-1 (Ubuntu Precise):
status: New → Triaged
Changed in policykit-1 (Ubuntu Quantal):
status: New → In Progress
Changed in policykit-1 (Ubuntu Precise):
importance: Undecided → Medium
Changed in policykit-1 (Ubuntu Quantal):
importance: Undecided → Medium
Changed in policykit-1 (Ubuntu Precise):
assignee: nobody → Steve Langasek (vorlon)
Changed in policykit-1 (Ubuntu Quantal):
assignee: nobody → Steve Langasek (vorlon)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package policykit-1 - 0.104-2ubuntu1

---------------
policykit-1 (0.104-2ubuntu1) quantal; urgency=low

  * debian/patches/07_pam_environment.patch: set process environment
    from pam_getenvlist(). Closes LP: #982684.
  * debian/patches/01_pam_polkit.patch: adjust patch to invoke pam_env, so
    our global settings from /etc/environment are applied correctly.
 -- Steve Langasek <email address hidden> Tue, 15 May 2012 15:15:52 -0700

Changed in policykit-1 (Ubuntu Quantal):
status: In Progress → Fix Released
Steve Langasek (vorlon)
description: updated
Changed in policykit-1 (Ubuntu Precise):
status: Triaged → In Progress
Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

Sudo already parses /etc/environment when used with the -i option. If our goal is to have it always parse /etc/environment, how about simply doing the following?

Index: sudo-1.8.3p1/plugins/sudoers/sudoers.c
===================================================================
--- sudo-1.8.3p1.orig/plugins/sudoers/sudoers.c 2011-10-25 10:08:12.000000000 -0400
+++ sudo-1.8.3p1/plugins/sudoers/sudoers.c 2012-05-15 20:40:38.997780972 -0400
@@ -581,12 +581,12 @@
      NewArgv[0] = NewArgv[1];
      NewArgv[1] = "--login";
  }
+ }

 #if defined(__linux__) || defined(_AIX)
  /* Insert system-wide environment variables. */
  read_env_file(_PATH_ENVIRONMENT, TRUE);
 #endif
- }

     /* Insert system-wide environment variables. */
     if (def_env_file)

I still need to think about the security ramifications in doing so though.

Revision history for this message
Steve Langasek (vorlon) wrote : Re: [Bug 982684] Re: sudo, pkexec don't apply global environment settings from /etc/environment

On Wed, May 16, 2012 at 12:59:35AM -0000, Marc Deslauriers wrote:
> Sudo already parses /etc/environment when used with the -i option. If
> our goal is to have it always parse /etc/environment, how about simply
> doing the following?

Because there may be other pam modules that also need to set environment
variables for proper operation. pam_krb5 or pam_afs, perhaps?

> I still need to think about the security ramifications in doing so though.

Sure, understood.

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
<email address hidden> <email address hidden>

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

On Wed, 2012-05-16 at 02:18 +0000, Steve Langasek wrote:
> On Wed, May 16, 2012 at 12:59:35AM -0000, Marc Deslauriers wrote:
> > Sudo already parses /etc/environment when used with the -i option. If
> > our goal is to have it always parse /etc/environment, how about simply
> > doing the following?
>
> Because there may be other pam modules that also need to set environment
> variables for proper operation. pam_krb5 or pam_afs, perhaps?

Ah, yes...so we should probably replace sudo's /etc/environment handling
then. Makes sense, thanks.

Revision history for this message
Chris Halse Rogers (raof) wrote : Please test proposed package

Hello Steve, or anyone else affected,

Accepted policykit-1 into precise-proposed. The package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Changed in policykit-1 (Ubuntu Precise):
status: In Progress → Fix Committed
tags: added: verification-needed
Revision history for this message
Tyler Hicks (tyhicks) wrote :

I noticed that upstream sudo (version 1.8.5 and 1.8.4p5) just announced support for merging the PAM environment into the user environment:

http://www.sudo.ws/sudo/stable.html#1.8.5

I've backported the relevant patches to Quantal and Precise. I've verified that the test case in the bug description passes, I've created a test case to add to the test-sudo.py script in the QA Regression Testing (QRT) branch, and I've verified that all existing QRT sudo tests still pass.

Revision history for this message
Tyler Hicks (tyhicks) wrote :

Here's the debdiff for sudo in Quantal. It passes the http_proxy test case in the bug description, as well as test-sudo.py in QRT.

I will need a sponsor for this upload.

Revision history for this message
Tyler Hicks (tyhicks) wrote :

Here's the debdiff for sudo in Precise. It passes the http_proxy test case in the bug description, as well as test-sudo.py in QRT.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

In the mean time (sorry), 1.8.3p2-1ubuntu1 got sponsored into quantal.
Can you please refresh the patch for quantal?

Revision history for this message
Steve Langasek (vorlon) wrote :

No need; the patch applies cleanly and I'm testing it here now.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package sudo - 1.8.3p2-1ubuntu2

---------------
sudo (1.8.3p2-1ubuntu2) quantal; urgency=low

  * debian/patches/pam_env_merge.patch: Merge the PAM environment into the
    user environment (LP: #982684)
  * debian/sudo.pam: Use pam_env to read /etc/environment and
    /etc/default/locale environment files. Reading ~/.pam_environment is not
    permitted due to security reasons.
 -- Tyler Hicks <email address hidden> Mon, 21 May 2012 00:48:10 -0500

Changed in sudo (Ubuntu Quantal):
status: In Progress → Fix Released
Revision history for this message
Chris Halse Rogers (raof) wrote :

Hello Steve, or anyone else affected,

Accepted sudo into precise-proposed. The package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Changed in sudo (Ubuntu Precise):
status: Triaged → Fix Committed
Revision history for this message
Stéphane Graber (stgraber) wrote :

Confirmed on precise.

tags: added: verification-done
removed: verification-needed
Revision history for this message
Chris Halse Rogers (raof) wrote :

(As per IRC, that verification was for both polkit-1 and sudo)

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package policykit-1 - 0.104-1ubuntu1

---------------
policykit-1 (0.104-1ubuntu1) precise-proposed; urgency=low

  * debian/patches/07_pam_environment.patch: set process environment
    from pam_getenvlist(). Closes LP: #982684.
  * debian/patches/01_pam_polkit.patch: adjust patch to invoke pam_env, so
    our global settings from /etc/environment are applied correctly.
 -- Steve Langasek <email address hidden> Tue, 15 May 2012 15:15:52 -0700

Changed in policykit-1 (Ubuntu Precise):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package sudo - 1.8.3p1-1ubuntu3.3

---------------
sudo (1.8.3p1-1ubuntu3.3) precise-proposed; urgency=low

  * debian/patches/pam_env_merge.patch: Merge the PAM environment into the
    user environment (LP: #982684)
  * debian/sudo.pam: Use pam_env to read /etc/environment and
    /etc/default/locale environment files. Reading ~/.pam_environment is not
    permitted due to security reasons.
 -- Tyler Hicks <email address hidden> Mon, 21 May 2012 00:48:10 -0500

Changed in sudo (Ubuntu Precise):
status: Fix Committed → Fix Released
Revision history for this message
pdecat (pdecat) wrote :
Download full text (4.3 KiB)

This bug was causing the following error with flashplugin-installer:

$ sudo apt-get --reinstall install flashplugin-installer
[sudo] password for pdecat:
Lecture des listes de paquets... Fait
Construction de l'arbre des dépendances
Lecture des informations d'état... Fait
0 mis à jour, 0 nouvellement installés, 1 réinstallés, 0 à enlever et 5 non mis à jour.
Il est nécessaire de prendre 0 o/8 076 o dans les archives.
Après cette opération, 0 o d'espace disque supplémentaires seront utilisés.
Préconfiguration des paquets...
(Lecture de la base de données... 266104 fichiers et répertoires déjà installés.)
Préparation du remplacement de flashplugin-installer 11.2.202.238ubuntu0.12.04.1 (en utilisant .../flashplugin-installer_11.2.202.238ubuntu0.12.04.1_amd64.deb) ...
Dépaquetage de la mise à jour de flashplugin-installer ...
Traitement des actions différées (« triggers ») pour « update-notifier-common »...
flashplugin-installer: downloading http://archive.canonical.com/pool/partner/a/adobe-flashplugin/adobe-flashplugin_11.2.202.238.orig.tar.gz
Traceback (most recent call last):
  File "/usr/lib/update-notifier/package-data-downloader", line 234, in process_download_requests
    dest_file = urllib.urlretrieve(files[i])[0]
  File "/usr/lib/python2.7/urllib.py", line 93, in urlretrieve
    return _urlopener.retrieve(url, filename, reporthook, data)
  File "/usr/lib/python2.7/urllib.py", line 239, in retrieve
    fp = self.open(url, data)
  File "/usr/lib/python2.7/urllib.py", line 207, in open
    return getattr(self, name)(url)
  File "/usr/lib/python2.7/urllib.py", line 344, in open_http
    h.endheaders(data)
  File "/usr/lib/python2.7/httplib.py", line 954, in endheaders
    self._send_output(message_body)
  File "/usr/lib/python2.7/httplib.py", line 814, in _send_output
    self.send(msg)
  File "/usr/lib/python2.7/httplib.py", line 776, in send
    self.connect()
  File "/usr/lib/python2.7/httplib.py", line 757, in connect
    self.timeout, self.source_address)
  File "/usr/lib/python2.7/socket.py", line 571, in create_connection
    raise err
IOError: [Errno socket error] [Errno 110] Connection timed out
Paramétrage de flashplugin-installer (11.2.202.238ubuntu0.12.04.1) ...

Fixed by forcing sudo-1.8.3p1-1ubuntu3.3 installation from precise-updates as precise-security has higher priority:

$ apt-cache policy sudo-1.8.3p1-1ubuntu3.3
sudo:
  Installé : 1.8.3p1-1ubuntu3.2
  Candidat : 1.8.3p1-1ubuntu3.2
 Table de version :
     1.8.3p1-1ubuntu3.3 0
        900 http://archive.ubuntu.com/ubuntu/ precise-updates/main amd64 Packages
 *** 1.8.3p1-1ubuntu3.2 0
        990 http://security.ubuntu.com/ubuntu/ precise-security/main amd64 Packages
        100 /var/lib/dpkg/status
     1.8.3p1-1ubuntu3 0
        500 http://archive.ubuntu.com/ubuntu/ precise/main amd64 Packages

$ sudo aptitude install sudo/precise-updates

$ apt-cache policy sudo
sudo:
  Installé : 1.8.3p1-1ubuntu3.3
  Candidat : 1.8.3p1-1ubuntu3.3
 Table de version :
 *** 1.8.3p1-1ubuntu3.3 0
        900 http://archive.ubuntu.com/ubuntu/ precise-updates/main amd64 Packages
        100 /var/lib/dpkg/status
     1.8.3p1-1ubuntu3.2 0
        990 http://security.ubuntu.com/ubuntu/ precise-se...

Read more...

Revision history for this message
Matthäus Brandl (matthaeus) wrote :

@pdecat:
At least using sudo 1.8.5p2-1ubuntu1 from quantal-security everything works fine.
Did you add the proxy settings to /etc/environment?

Revision history for this message
In , Martin Pitt (pitti) wrote :

Various pam modules provide environment variables that are intended to be set in the environment of the pam session. pkexec needs to process the output of pam_getenvlist() to get these.

This will e. g. apply correct locales in pkexec when they are configured in pam_environment.

Revision history for this message
In , Martin Pitt (pitti) wrote :

Created attachment 76150
pkexec: Set process environment from pam_getenvlist()

Steve Langasek applied this patch a while ago to the Ubuntu packages. I adjusted it for current git master and brought it into git format-patch form.

Revision history for this message
In , Zeuthen (zeuthen) wrote :

I'm not sure that's a good idea ... but I can probably be convinced that it is :-) ... So apart from locales, can you give examples of such PAM modules and the environment variables that are set? Thanks.

Revision history for this message
In , Zeuthen (zeuthen) wrote :

Comment on attachment 76150
pkexec: Set process environment from pam_getenvlist()

Review of attachment 76150:
-----------------------------------------------------------------

Looks good but the the coding style is wrong

 - curly-braces / indentation wrong
 - should use guint instead of int
 - should use 'n' as a counter/iterator, not 'i' (like the rest of the code)

These are style issues but consistency is important.

Revision history for this message
In , Steve Langasek (vorlon) wrote :

On Fri, Mar 08, 2013 at 06:06:28PM +0000, <email address hidden> wrote:
> ... So apart from locales, can you give examples of such PAM modules and
> the environment variables that are set? Thanks.

The pam_env module is a big one, which is used by admins to configure
arbitrary environment settings for all sessions. The specific case that
prompted this had to do with proxy settings configured in the environment.

Other modules that may need to set environment variables include pam_krb5
and pam_afs_session, whose environment settings may be required for proper
filesystem access.

Revision history for this message
In , Martin Pitt (pitti) wrote :

Created attachment 76324
pkexec: Set process environment from pam_getenvlist()

Fixed coding style.

Changed in policykit:
importance: Unknown → Medium
status: Unknown → Incomplete
Revision history for this message
In , Colin Walters (walters) wrote :

Comment on attachment 76324
pkexec: Set process environment from pam_getenvlist()

Review of attachment 76324:
-----------------------------------------------------------------

::: src/programs/pkexec.c
@@ +182,5 @@
> + {
> + guint n;
> + for (n = 0; envlist[n]; n++)
> + putenv (envlist[n]);
> + free (envlist);

From my reading of the manual page (haven't looked at the source), it looks to me like you need to free() the individual elements too. I.e., do what g_strfreev() does.

Right?

(Just looked at the source of Linux-PAM-1.1.6, and that does indeed appear to be the case)

Revision history for this message
In , Steve Langasek (vorlon) wrote :

> From my reading of the manual page (haven't looked at the source),
> it looks to me like you need to free() the individual elements too.

Not according to the manpage for putenv(), which states that the string passed to putenv() becomes part of the environment directly ("The string pointed to by string becomes part of the environment, so altering the string changes the environment").

Revision history for this message
In , Colin Walters (walters) wrote :

(In reply to comment #7)
> > From my reading of the manual page (haven't looked at the source),
> > it looks to me like you need to free() the individual elements too.
>
> Not according to the manpage for putenv(), which states that the string
> passed to putenv() becomes part of the environment directly ("The string
> pointed to by string becomes part of the environment, so altering the string
> changes the environment").

Sorry about the slow response; I subscribed to drm-devel@ recently and that ended up in a total flood of my <email address hidden> mail.

So anyways, yes looks like you're right. The environment APIs are just ugly.

I just looked at the GDM source, and it appears to call the same API, so this makes sense. Pushed to master, thanks!

Changed in policykit:
status: Incomplete → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.