Ubuntu

~/.bazaar created owned by root (when run under sudo)

Reported by Morgan Collett on 2009-05-14
26
This bug affects 4 people
Affects Status Importance Assigned to Milestone
Bazaar
Medium
Parth Malwankar
bzr (Ubuntu)
Low
Unassigned
etckeeper (Ubuntu)
Medium
Thierry Carrez

Bug Description

Binary package hint: etckeeper

Installing and using etckeeper on a new jaunty server without a ~/.bazaar causes the user to have their .bazaar owned by root.

Steps to reproduce:
On a new jaunty server with no ~/.bazaar:
$ sudo apt-get install etckeeper
$ sudo etckeeper init
$ sudo etckeeper commit "Initial commit"
$ ls -ld .bazaar
drwxr-xr-x 2 root root 4096 2009-05-13 21:13 .bazaar/

This causes setting bzr whoami after this to fail.

Thierry Carrez (ttx) wrote :

This is caused by bzr not really supporting to be run under sudo.
In config.py:ensure_config_dir_exists it creates $HOME/.bazaar without dropping rights to the $SUDO_USER. Unfortunately the same could be said for all other config-writing functions. I'm not exactly sure where (and how) it would be best to fix that.

Adding a bzr task to this bug to drag bzr developers into the discussion. The fix for this one could also be discussed at the bzr/etckeeper session at UDS (server track).

Changed in etckeeper (Ubuntu):
importance: Undecided → Medium
status: New → Confirmed
summary: - .bazaar created owned by root
+ ~/.bazaar created owned by root (when run under sudo)
Jelmer Vernooij (jelmer) on 2009-07-30
Changed in bzr (Ubuntu):
status: New → Confirmed
Changed in bzr:
status: New → Triaged
importance: Undecided → Low
importance: Low → Medium

On Fri, 2009-05-15 at 09:20 +0000, Thierry Carrez wrote:
> This is caused by bzr not really supporting to be run under sudo.
> In config.py:ensure_config_dir_exists it creates $HOME/.bazaar without dropping rights to the $SUDO_USER. Unfortunately the same could be said for all other config-writing functions. I'm not exactly sure where (and how) it would be best to fix that.
>
> Adding a bzr task to this bug to drag bzr developers into the
> discussion. The fix for this one could also be discussed at the
> bzr/etckeeper session at UDS (server track).

Patches appreciated :) certainly sounds like a valid use case. Its a
shame that we'd need to special case stuff.

-Rob

Martin Pool (mbp) wrote :

I agree there's some kind of bug here, but this behavior is very
common in other programs too, and it does seem tragic they'll all need
to specially handle running under sudo. Maybe sudo -H should be the
default.

--
Martin <http://launchpad.net/~mbp/>

Soren Hansen (soren) wrote :

Would using pwd.getpwuid(os.geteuid()).pw_dir rather than os.environ['HOME'] cause more problems than it solves?

On 17 February 2010 00:48, Soren Hansen <email address hidden> wrote:
> Would using pwd.getpwuid(os.geteuid()).pw_dir rather than
> os.environ['HOME'] cause more problems than it solves?

I think so.

The right place to solve this is imo under sudo, by using the set_home
config option or -H option. This is not particularly specific to bzr.

--
Martin <http://launchpad.net/~mbp/>

Martin Pitt (pitti) wrote :

We won't make -H the default in sudo; it would break other use cases which rely on having the original $HOME, and general backwards compatibliity.

Thierry Carrez (ttx) wrote :

That leaves two options:
1/ special-casing sudo in configuration file management in bzr
2/ workaround in etckeeper (override HOME in bzr calls in etckeeper ? Pre-create $HOME/.bazaar and $HOME/.bzr.log correctly ?)

(2) is certainly simpler, however it will fail to catch the case where the etckeeper user runs "sudo bzr diff" by hand. that said, if soren implements the "etckeeper diff" command, that might be considered a corner case.

Daniel Hahler (blueyed) wrote :

Please note that this is not a problem with Git / when using Git with etckeeper:
Apparently Git does not create the .git dir automatically - which may also be an option for Bazaar.

Apart from that, Git behaves the same:
$ sudo git config --global user.name "Daniel Hahler"
$ ls -l ~/.gitconfig
-rw-r--r-- 1 root root 29 2010-02-17 12:29 /home/daniel/.gitconfig

Should there be a Git task then, too?

Martin Pool (mbp) wrote :

Thierry's comment in #1 is interesting:

>> This is caused by bzr not really supporting to be run under sudo. In config.py:ensure_config_dir_exists it creates $HOME/.bazaar without dropping rights to the $SUDO_USER. Unfortunately the same could be said for all other config-writing functions. I'm not exactly sure where (and how) it would be best to fix that.

This would be possible. However, it also seems likely to break in the case that you do "sudo -H bzr" and actually _want_ bzr to use root's configuration.

It may be that we create ~/.bazaar even when we don't need to, and trimming that would make it similar to #12. However the problem may still occur with ~/.bzr.log.

Another option would be to say we always attempt to chown the .bazaar (and .bzr.log) to be the same as the containing directory. That is perhaps not quite unixy, but it seems correct for the main cases: with HOME=/root it's owned by root, with HOME=/home/mbp by me. It's a bit like a setgid directory.

Parth Malwankar (parthm) wrote :

I plan to look into this issue.

I was thinking of going with the approach Martin suggested:

> Another option would be to say we always attempt to chown
> the .bazaar (and .bzr.log) to be the same as the containing
> directory.

Any other thoughts/suggestions welcome.

Changed in bzr:
assignee: nobody → Parth Malwankar (parthm)
status: Triaged → In Progress
Martin Pool (mbp) wrote :

I think Parth's patch in https://code.edge.launchpad.net/~parthm/bzr/2.0_376388_dot_bazaar_ownership/+merge/19593 is reasonable and useful for bzr in its own right (review welcome)

but probably also etckeeper should run bzr and other programs with HOME set to ~root to avoid the problem altogether.

Thierry Carrez (ttx) wrote :

How about just adding HOME=/root (or something slightly more dynamic) to our etckeeper.conf ? it's sourced everywhere and should ensure bzr is always called with the right env ?

That doesn't solve the case where people run "sudo bzr diff" or others, but that can probably wait for the fix to land in bzr..

Thierry Carrez (ttx) wrote :

(I meant HOME=~root)

Martin Pool (mbp) wrote :

On 19 February 2010 19:06, Thierry Carrez <email address hidden> wrote:
> How about just adding HOME=/root (or something slightly more dynamic) to
> our etckeeper.conf ? it's sourced everywhere and should ensure bzr is
> always called with the right env ?

That makes sense to me.

--
Martin <http://launchpad.net/~mbp/>

Thierry Carrez (ttx) on 2010-02-23
Changed in etckeeper (Ubuntu):
assignee: nobody → Thierry Carrez (ttx)
status: Confirmed → In Progress
Thierry Carrez (ttx) on 2010-02-23
Changed in bzr (Ubuntu):
importance: Undecided → Low
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package etckeeper - 0.41ubuntu3

---------------
etckeeper (0.41ubuntu3) lucid; urgency=low

  * etckeeper: Set HOME=~root if VCS=bzr, so that bzr doesn't create root-owned
    .bazaar and .bzr.log in home directories when run under sudo (LP: #376388)
 -- Thierry Carrez <email address hidden> Tue, 23 Feb 2010 12:05:55 +0100

Changed in etckeeper (Ubuntu):
status: In Progress → Fix Released
Vincent Ladeuil (vila) wrote :

> Another option would be to say we always attempt to chown the .bazaar (and .bzr.log) to be the same
> as the containing directory.

Until parthm nudged me enough to reveal that chown isn't working for the average user, I had always thought
*any* user can freely use chown as long as he had write access to the containing directory.

I was wrong.

chown(2) unambiguously says:

        Only a privileged process (Linux: one with the CAP_CHOWN capability)
       may change the owner of a file. The owner of a file may change the
       group of the file to any group of which that owner is a member. A
       privileged process (Linux: with CAP_CHOWN) may change the group arbi‐
       trarily.

Martin Pool (mbp) wrote :

On 11 March 2010 00:18, Vincent Ladeuil <email address hidden> wrote:
>> Another option would be to say we always attempt to chown the .bazaar (and .bzr.log) to be the same
>> as the containing directory.
>
> Until parthm nudged me enough to reveal that chown isn't working for the average user, I had always thought
> *any* user can freely use chown as long as he had write access to the containing directory.

Sure, I meant to say "always try to", but we must not be surprised if
we get EPERM.

>
> I was wrong.
>
> chown(2) unambiguously says:
>
>        Only a privileged process (Linux: one with  the  CAP_CHOWN  capability)
>       may  change  the  owner  of a file.  The owner of a file may change the
>       group of the file to any group of which that  owner  is  a  member.   A
>       privileged  process  (Linux: with CAP_CHOWN) may change the group arbi‐
>       trarily.

btw I think this may vary between unixes.

--
Martin <http://launchpad.net/~mbp/>

<snip/>

    > btw I think this may vary between unixes.

I did test with FreeBSD and OSX and got the same results, I
mentioned the explanation for Linux because that was the only one
I cared to find but I've seen some mentions about a sysctl
variable for them that control this precise aspect and the
default value also forbids chown for non-root users.

An alternative solution may be to recognize we are working as
root and just use setuid for the relevant operations or something ?

Parth Malwankar (parthm) on 2010-04-25
Changed in bzr:
status: In Progress → Fix Released
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package bzr - 2.2.0-1

---------------
bzr (2.2.0-1) unstable; urgency=low

  * New upstream release.
   + Adds support for setting timestamps to originating revisions.
     Closes: #473450
   + Removes remaining string exception. Closes: #585193, LP: #586926
   + Add C extension to work around Python issue 1628205. LP: #583941,
     Closes: #577110
   + Avoids showing progress bars when --quiet is used. Closes: #542105,
     LP: #320035
   + No longer creates ~/.bazaar as root when run under sudo. LP: #376388
   + 'bzr commit' now supports -p as alternative for --show-diff. LP: #571467
   + 'bzr add' no longer adds .THIS/.BASE/.THEIRS files unless
     explicitly requested. LP: #322767
   + When parsing patch files, Bazaar now supports diff lines before each
     patch. LP: #502076
   + WorkingTrees now no longer requires using signal.signal, so can
     be used in a threaded environment. LP: #521989
   + An assertion error is no longer triggered when pushing to a pre-1.6
     Bazaar server. LP: #528041
  * Bump standards version to 3.9.1.

bzr (2.2.0~b4-1) experimental; urgency=low

  * New upstream beta.
  * Bump standards version to 3.9.0.
  * Drop build dependency on zlib.

bzr (2.2.0~b2-1) experimental; urgency=low

  * New upstream release.
  * Recommend python-launchpadlib. Closes: #568937

bzr (2.1.2-1.1) unstable; urgency=low

  * Non-maintainer upload.
  * Fix access via http_proxy. Closes: #588430
 -- Jelmer Vernooij <email address hidden> Sat, 07 Aug 2010 00:54:52 +0200

Changed in bzr (Ubuntu):
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers