Re: "Fix" of sudo with DSA-946-1
Martin Schulze wrote:
> Proposed updates for woody and sarge are here:
> http://klecker.debian.org/~joey/security/sudo/
> I'd be glad if you could test them.r
That's awesome. Thanks! Here, have some karma :-)
I just installed your version on sarge using:
- Remove my (custom) "Defaults" line in my /etc/sudoers file
- sudo dpkg -i sudo_1.6.8p7-1.4_i386.deb
Most environment variables seem there as I would expect, and those I
don't expect are indeed removed. The only issue I still have is that the
manual page should still be updated. I noticed some changes in
sudoers.pod (in sudo_1.6.8p7-1.4.diff.gz), but somehow that did not pass
to the sudoers.5.gz man page in the sudo_1.6.8p7-1.4_i386.deb.
In addition, I argue that while the current change is technically sound,
it will still break some things (e.g. the EDITOR and LC_* variables),
which is acceptable, but warrants even more attention in both the DSA
note and a seperate README.Debian file. Below are details.
I like to emphasize that any complaint I had earlier was not as much
technical in nature, but mostly about communicating important changes
like this.
I just read through all bugreports, and carefully tried to reproduce
each one to see if all is well now.
Most importantly, the variables that are kept are indeed now the same as
I would get when I specify "Defaults env_reset". Specifically:
DISPLAY variable kept (closes #349085)
XAUTHORITY variable kept (closes #349549)
HOME variable kept (closes #349587)
SHELL variable kept (closes #350776)
Two variables are not kept:
EDITOR variable kept (bug #349196).
LC_ALL variable kept in earlier releases, including sudo_1.6.8p7-1.3
(the previous security fix).
While this probably will break some things, and annoy people, I think it
is acceptable, since these variables are less likely to be used in
automated scripts. I think it is important to communicate this though,
both in (1) the DSA, (2) the changelog and (3) in the man page. It
currently is not mentioned anywhere.
Update manual pages (#349129):
NOT FIXED.
Given that some things still need manual tweaking (e.g. EDITOR or LC_*
variables), it is good to update the page. I noticed that one of the
file you created, sudo_1.6.8p7-1.4.diff.gz, has some manual changes to
sudoers.pod, but these changes are not reflected in the sudoers.5.gz man
page in the sudo_1.6.8p7-1.4_i386.deb. Additionally, if you have not
done so, here is also a patch for the man pages:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=349196;msg=34
sudo -V output is misleading: it gives a very incomplete list of
env vars that are removed. (also #349129)
NOT FIXED.
To be honest, I'm not sure if this should be fixed now. On one hand it
would be good, but I fear that it may introduce too much new code (which
seem a bad thing for a security patch). I would leave it open for etch,
but not fix it in woody or sarge, but the security team can decide best.
Complaint about 'sudo vi <anyfile>':
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=349196;msg=15
Status: I can't reproduce it (or it is simply fixed now)
Complaint about "sudo joe filename":
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=349196;msg=10
Status: I can't reproduce it (or it is simply fixed now)
Complaint that "Defaults env_reset, env_keep=*, always_set_home" gives
two PATH variables instead of one (#354431).
NOT FIXED.
This is indeed an important bug, but I think it is not directly related
to the security bug, and should thus just be fixed in etch.
Last, I checked the behaviour of sudo, with these "Default" lines in
/etc/sudoers:
* No "defaults" line:
Behaves as described (same as "Defaults env_reset")
* Defaults env_reset, env_keep+="EDITOR"
Behaves as expected (preserves limited list + EDITOR)
* Defaults !env_reset
Behaves as expected (preserves all)
* Defaults env_keep+="EDITOR"
Behaves as expected (preserves limited list + EDITOR)
* Defaults env_check=EDITOR
Does only preserve limited list, not EDITOR variable. This is expected
(if I correctly understand how env_check works)
Finally, I suggest to add a /usr/share/doc/sudo/READM.Debian file with
this contents:
- ----
The version of sudo that ships with Debian by default resets the
environment, as described by the "env_reset" flag in the sudoers file.
This implies that all environment variables are removed, except for
HOME, LOGNAME, PATH, SHELL, TERM, DISPLAY, XAUTHORITY, XAUTHORIZATION,
and USER.
In case you want sudo to preserve more environment variables, you must
specify the env_keep variable in the sudoers file. You should edit the
sudoers file using the visudo tool.
Examples:
Preserve the default variables plus the EDITOR variable:
Defaults env_keep+="EDITOR"
Preserve the default variables plus all variables starting with LC_:
Defaults env_keep+="LC_*"
- ----
With kind regards,
Freek Dijkstra
Reply to: