ping does not work as a normal user on trusty tarball cloud images.

Bug #1313550 reported by Clint Byrum on 2014-04-28
20
This bug affects 1 person
Affects Status Importance Assigned to Milestone
MAAS
High
Unassigned
curtin
High
Unassigned
curtin (Ubuntu)
High
Unassigned
Trusty
High
Unassigned
iputils (Ubuntu)
High
Unassigned
maas (Ubuntu)
High
Unassigned
Saucy
High
Unassigned
Trusty
High
Unassigned
Vivid
High
Unassigned
Wily
High
Unassigned
tar (Ubuntu)
Medium
Unassigned
Precise
High
Unassigned
Saucy
High
Unassigned
Trusty
High
Unassigned

Bug Description

With trusty, /bin/ping relies on having extended attributes and kernel capabilities to gain the cap_net_raw+p capability. This allows removing the suid bit.

However, the tarball cloud images do not preserve the extended attributes, and thus /bin/ping does not work on a system derived from them.

Summary of problem per package:
 * lxc: ubuntu cloud template needs to extract
 * download template needs to extract with xattr flags
 * server side download creation tools need xattr flags
 * [unconfirmed] tarball caches need creation and extraction with xattr flags
 * tar: need the '--xattr' and '--acl' flags backported
 * maas: uec2roottgz needs to use xattr/acl flags
 * curtin: extraction needs to use xattr/acl flags.
 * cloud-image-build: needs to create -root.tar.gz with xattr/acl flags

Related Bugs:
 * bug 1382632: horizon insecure key file permissions
 * bug 1386237: tar strange behavior with --acl and xattr
 * bug 1313550: ping broken (xattrs lost in tar extraction)
 * bug 1302192: capabilities not preserved on installation

Related branches

Scott Moser (smoser) on 2014-04-28
Changed in maas:
status: New → Confirmed
Changed in iputils (Ubuntu):
status: New → Confirmed
Changed in maas (Ubuntu):
status: New → Confirmed
Changed in maas:
importance: Undecided → High
Changed in iputils (Ubuntu):
importance: Undecided → High
Changed in maas (Ubuntu):
importance: Undecided → High
tags: added: cloud-images cloud-images-build
Scott Moser (smoser) wrote :

OK,
  so i put this as affecting curtin and affecting maas.
  - maas: uec2roottgz (which creates the -root.tar.gz file from an image file) will be affected
  - curtin: extracts the tarball and will need to do so with xattrs in place.

Changed in curtin:
status: New → Confirmed
Changed in curtin (Ubuntu):
status: New → Confirmed
Changed in curtin:
importance: Undecided → High
Changed in curtin (Ubuntu):
importance: Undecided → High
Scott Moser (smoser) wrote :

This would seem straight forward enough (I thought I could just add '--xattrs' to both creation and extraction of tar), but that doesn't seem to look. See the attachment, and its output here, it seems that tar is losing these.

$ sudo /tmp/xattr-save-ping
$ ls -l /bin/ping
-rwxr-xr-x 1 root root 44168 Mar 15 02:24 /bin/ping
$ attr -l /bin/ping
Attribute "capability" has a 20 byte value for /bin/ping
$ tar -C / --xattrs -Scpf - bin/ping | tar -C $tmpd --xattrs -Sxpf -
$ ( cd $tmpd && attr -l bin/ping )
$ ( cd $tmpd && attr -l ping_rsync )
Attribute "capability" has a 20 byte value for ping_rsync

So, above, it seems like bin/ping that was created by tar did not get the attr laid down.

Ryan Beisner (1chb1n) wrote :

FYI may also want to see comment 5 from previous/related bug 1302192; attributes were OK in cloudimage at that point.
https://bugs.launchpad.net/ubuntu/+source/ubiquity/+bug/1302192/comments/5

Scott Moser (smoser) on 2014-04-28
Changed in lxc (Ubuntu):
status: New → Confirmed
importance: Undecided → High
Scott Moser (smoser) wrote :

OK, so with 14.04 level maas, this is fairly trivial.
  'tar --xattrs --xattrs-include=*'

Unfortunately, 12.04 level maas doesn't have that, so we'd have to do some backwards compatibility check/fix/hack if we want to support

the fix in the installer was to do something like:
 getfattr | setfattr

sudo getfattr --absolute-names --recursive -hysical --match=- --dump /
seems to dump attrs, we could store that inside the tar as a file.

Scott Moser (smoser) on 2014-04-28
Changed in tar (Ubuntu):
status: New → Fix Released
importance: Undecided → Medium
Changed in tar (Ubuntu Precise):
assignee: nobody → Serge Hallyn (serge-hallyn)
status: New → Confirmed
importance: Undecided → High
Changed in tar (Ubuntu Trusty):
importance: Undecided → High
status: New → Fix Released
no longer affects: iputils (Ubuntu Precise)
no longer affects: iputils (Ubuntu Saucy)
Scott Moser (smoser) on 2014-04-28
no longer affects: curtin (Ubuntu Precise)
Scott Moser (smoser) on 2014-04-28
no longer affects: maas (Ubuntu Precise)
Changed in tar (Ubuntu Saucy):
status: New → Confirmed
Changed in maas (Ubuntu Trusty):
status: New → Confirmed
Changed in maas (Ubuntu Saucy):
status: New → Confirmed
Changed in lxc (Ubuntu Trusty):
status: New → Confirmed
importance: Undecided → High
Changed in maas (Ubuntu Saucy):
importance: Undecided → High
Changed in maas (Ubuntu Trusty):
importance: Undecided → High
Changed in lxc (Ubuntu Precise):
importance: Undecided → High
status: New → Confirmed
Changed in lxc (Ubuntu Saucy):
importance: Undecided → High
status: New → Confirmed
no longer affects: iputils (Ubuntu Trusty)
Changed in curtin (Ubuntu Saucy):
importance: Undecided → High
status: New → Confirmed
Scott Moser (smoser) on 2014-04-28
Changed in curtin (Ubuntu Trusty):
importance: Undecided → High
status: New → Confirmed
Changed in tar (Ubuntu Saucy):
importance: Undecided → High
Scott Moser (smoser) on 2014-04-28
description: updated
Serge Hallyn (serge-hallyn) wrote :

Unfortunately src/xattrs.c is wholly non-existent in the precise version. So backporting xattr support would include src/xattr.{c,h} as well as inserting the calls to functions defined there throughout the rest of the code. Unfortunately that doesn't seem SRU-able. The safest way forward would be to take 1.27 into precise (which I assume is also not sru-able)

Changed in tar (Ubuntu Precise):
assignee: Serge Hallyn (serge-hallyn) → nobody
Scott Moser (smoser) wrote :

Serge,
  I don't see why new files would make something non-SRU-able.
  I dont' think that is a blocker in and of itself.

 The complexity of the patch and likelyhood of regression is the bigger concern.

 We have a real bug here, and we have 2 ways to fix it (possibly others that i've not thought of):
a.) backport functionality into 'tar' and then make programs able to use that functionality in exactly the same way that they would/do in Utopic.
b.) craft a special tarball that has a '.xattrs_hack file in it, and then SRU patches to programs to say something like "if there is a .xattrs_hack file in a tarball, then apply those attributes with setfattr and then remove .xattrs_hack".

b seems a hack, and probably means adding a dependency on 'attr' to maas and lxc. Admittedly maas only has this problem in saucy and trusty (and saucy EOL shortly). So lxc on precise is the real issue.

In summary, to fix this bug in precise, we can do it fairly cleanly (and I think probably pretty safely) with 'a', or hackily in 'b'.

If I were SRU team, 'a' would look more appealing, or some other option I havent thought of.

Stéphane Graber (stgraber) wrote :

There's a third I mentioned on IRC, just make ping setuid again for 14.04 and only switch to capabilities in 14.10, assuming we don't intend to support 14.10 on 12.04, we'll be good as 14.04 will have a suitable version of tar.

Quoting Scott Moser (<email address hidden>):
> Serge,
> I don't see why new files would make something non-SRU-able.

A new file by itself would be nice as it's self-contained. It's
particularly adding the new calls that would seem protentially
problematic.

Anyway if it seems sane I'll post a debdiff at some point hopefully
tomorrow. But given the importance of tar in general (meaning, if
there were regressions in 1.27 in trusty we would know about them)
I think backporting 1.27 to precise seems more sane than cherrypicking
something very intrusive and hoping we don't get weird side effects
or subtle bugs.

Scott Moser (smoser) wrote :

I am kind of leaning towards stgraber's suggestion of "fix"ing iputils in 14.04 to be setuid.

Scott Moser (smoser) wrote :

trusty tarball daily (20140429) now correctly contains the capability info:
$ wget http://cloud-images.ubuntu.com/trusty/20140429/trusty-server-cloudimg-amd64-root.tar.gz
$ sudo tar --xattrs '--xattrs-include=*' --acls -Szxpf trusty-server-cloudimg-amd64-root.tar.gz bin/ping

$ attr -l bin/ping
Attribute "capability" has a 20 byte value for bin/ping
$ getcap bin/ping
bin/ping = cap_net_raw+p

Scott Moser (smoser) wrote :

one other hting to think about if we're going the --xattrs route anywhere, we likely have to consider the fact that '--xattrs' might *fail* on a filesystem that doesn't support xattrs. I've not looked at how tar handles that. that makes that a bit tricky.

Serge Hallyn (serge-hallyn) wrote :

This debdiff against the precise version of tar implements xattr support.

tags: added: patch
Adam Conrad (adconrad) on 2014-05-07
tags: added: verification-needed
Stéphane Graber (stgraber) wrote :

After some more discussion we decided to simply make it setuid again for now and then spend some time thinking about a proper solution for all this, possibly involving small dpkg/debhelper changes so there's a cleaner nicer way of declaring filesystem capabilities.

Hello Clint, or anyone else affected,

Accepted iputils into trusty-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/iputils/3:20121221-4ubuntu1.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags: added: verification-done
removed: verification-needed

The verification of the Stable Release Update for iputils has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package iputils - 3:20121221-4ubuntu1.1

---------------
iputils (3:20121221-4ubuntu1.1) trusty; urgency=medium

  * Mark ping and ping6 setuid again as there's currently no good ways
    to have capabilities be kept in all our images. (LP: #1313550)
 -- Stephane Graber <email address hidden> Wed, 07 May 2014 14:46:35 -0500

Changed in iputils (Ubuntu):
status: Confirmed → Fix Released
Jason Gerard DeRose (jderose) wrote :

This also affects the `gnome-keyring` package. The System76 imaging system (Tribble) uses a tar-based approach similar to the MAAS fast-path installer, and we've had to add a work-around for /usr/bin/gnome-keyring-daemon on our desktop images:

$ getcap /usr/bin/gnome-keyring-daemon
/usr/bin/gnome-keyring-daemon = cap_ipc_lock+ep

I reckon there are other imaging systems out there likewise affected by this. I strongly feel that the correct fix is to fix tar so its --xattrs option works as expected. But in the meantime, it might be good to switch back to using suid on /usr/bin/gnome-keyring-daemon.

Stéphane Graber (stgraber) wrote :

gnome-keyring-daemon isn't really a problem because all official images shipping it have installer hooks to restore the flag.

Setting the binary setuid would also be completely wrong as we never want this to run as root, we just want it to have extra ipc locking capabilities. My understanding is that those aren't even entirely required and that it has a fallback mode for systems where the capability isn't set.

Jason Gerard DeRose (jderose) wrote :

Stéphane,

Gotcha, thanks for the feedback! So am I correct in thinking that the --xattrs option is currently broken in tar on 14.04? If so, is there any chance this could be fixed in an SRU?

Stéphane Graber (stgraber) wrote :

So yeah, my understanding is that --xattr is broken at the moment, we should fix that as an SRU but we also have the problem that:
 1) It's not set by default in either create or extract mode
 2) Not all tar implementations we use support it
 3) Not all version of gnu-tar we support have it

So as long as we don't get this fixed, we shouldn't rely on xattrs working in tarballs and should investigate alternative methods of shipping and setting those attributes.

Excerpts from Jason Gerard DeRose's message of 2014-05-08 16:45:23 UTC:
> Stéphane,
>
> Gotcha, thanks for the feedback! So am I correct in thinking that the
> --xattrs option is currently broken in tar on 14.04? If so, is there any
> chance this could be fixed in an SRU?
>

No, --xattrs works fine in 14.04. The problem is that it must be used.
Note that you also have to use --xattrs-include=* while creating the
tar to make use of it (which is, IMO, a bug, as if I said --xattrs,
I meant _USE XATTRS_. ;)

Anyway, another problem is that 12.04 tar does not support it, so the
tarball images are not consumable from the previous release, which is a
problem for a shop trying to upgrade. I think that may be a worthy SRU,
as tars with xattrs will start to become more and more commonplace over
the next 3 years that 12.04 is still a supported platform.

Certainly somebody should get 14.04's tar into precise-backports while
this SRU is debated.

Jason Gerard DeRose (jderose) wrote :

Clint,

Ah, thanks for bringing up --xattrs-include=*, I didn't notice this option!

I agree this is really a bug/misfeature in tar... if I use --xattrs both when creating and unpacking a tarball, I expect it to just work.

Scott Moser (smoser) on 2014-10-21
Changed in curtin (Ubuntu Saucy):
status: Confirmed → Won't Fix
description: updated
description: updated
Scott Moser (smoser) on 2014-10-27
description: updated
Scott Moser (smoser) on 2014-10-27
description: updated

Hello Clint, or anyone else affected,

Accepted curtin into trusty-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/curtin/0.1.0~bzr195-0ubuntu1~14.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in curtin (Ubuntu Trusty):
status: Confirmed → Fix Committed
tags: removed: verification-done
tags: added: verification-needed
Rolf Leggewie (r0lf) wrote :

saucy has seen the end of its life and is no longer receiving any updates. Marking the saucy task for this ticket as "Won't Fix".

Changed in lxc (Ubuntu Saucy):
status: Confirmed → Won't Fix
Changed in maas (Ubuntu Saucy):
status: Confirmed → Won't Fix
Changed in tar (Ubuntu Saucy):
status: Confirmed → Won't Fix
Larry Michel (lmic) wrote :
Download full text (6.8 KiB)

This was the test case:

1) Update trusty daily root-tgz image with a copy of dcap and cap properties.
2) Sync image to cache
3) Deploy a node with trusty
4) Access deployed node
5) Ensure that cap properties for the new file are preserved on deployed system.

This test passed.

Here are test details:
======================================================================
1) Update image
root@ubuntumaas:/var/lib/maas/boot-resources/cache/root# ls -l usr/bin/dpkgcap
-rwxr-xr-x 1 root root 261840 Dec 17 09:33 usr/bin/dpkgcap
root@ubuntumaas:/var/lib/maas/boot-resources/cache/root# getcap usr/bin/dpkgcap
usr/bin/dpkgcap = cap_net_raw+p
root@ubuntumaas:/var/lib/maas/boot-resources/cache/root# tar --xattrs '--xattrs-include=*' -czf root.tar.gz *
tar: root.tar.gz: file changed as we read it

2) Sync image
root@ubuntumaas:/var/lib/maas/boot-resources/cache/root# ls -l
total 550812
drwxr-xr-x 2 root root 4096 Dec 17 02:54 bin
drwxr-xr-x 3 root root 4096 Dec 8 14:34 boot
drwxr-xr-x 6 root root 4096 Dec 8 14:34 dev
drwxr-xr-x 96 root root 4096 Dec 17 02:56 etc
drwxr-xr-x 2 root root 4096 Apr 10 2014 home
lrwxrwxrwx 1 root root 33 Dec 8 14:33 initrd.img -> boot/initrd.img-3.13.0-40-generic
drwxr-xr-x 22 root root 4096 Dec 8 14:31 lib
drwxr-xr-x 2 root root 4096 Dec 4 18:40 lib64
drwx------ 2 root root 4096 Dec 4 18:43 lost+found
drwxr-xr-x 2 root root 4096 Dec 4 18:40 media
drwxr-xr-x 2 root root 4096 Apr 10 2014 mnt
drwxr-xr-x 2 root root 4096 Dec 4 18:40 opt
drwxr-xr-x 2 root root 4096 Apr 10 2014 proc
drwx------ 2 root root 4096 Dec 17 02:05 root
-rw-r--r-- 1 root root 563942052 Dec 17 09:40 root.tar.gz
drwxr-xr-x 4 root root 4096 Dec 8 14:33 run
drwxr-xr-x 2 root root 4096 Dec 17 02:54 sbin
drwxr-xr-x 2 root root 4096 Dec 4 18:40 srv
drwxr-xr-x 2 root root 4096 Mar 12 2014 sys
drwxrwxrwt 4 root root 4096 Dec 17 02:55 tmp
drwxr-xr-x 10 root root 4096 Dec 4 18:40 usr
drwxr-xr-x 12 root root 4096 Dec 4 18:43 var
lrwxrwxrwx 1 root root 30 Dec 8 14:33 vmlinuz -> boot/vmlinuz-3.13.0-40-generic
root@ubuntumaas:/var/lib/maas/boot-resources/cache/root# ls -l ../root-tgz-3d15bdc99ae5cfe7e0be2e06e084636dc6fd809ec09ca54732ec83c9224376a2
-rw-r--r-- 1 root root 424884409 Dec 17 03:28 ../root-tgz-3d15bdc99ae5cfe7e0be2e06e084636dc6fd809ec09ca54732ec83c9224376a2
root@ubuntumaas:/var/lib/maas/boot-resources/cache/root# cp root.tar.gz ../root-tgz-3d15bdc99ae5cfe7e0be2e06e084636dc6fd809ec09ca54732ec83c9224376a2
root@ubuntumaas:/var/lib/maas/boot-resources/cache/root# ls -l ../root-tgz-3d15bdc99ae5cfe7e0be2e06e084636dc6fd809ec09ca54732ec83c9224376a2
-rw-r--r-- 1 root root 563942052 Dec 17 09:42 ../root-tgz-3d15bdc99ae5cfe7e0be2e06e084636dc6fd809ec09ca54732ec83c9224376a2
root@ubuntumaas:/var/lib/maas/boot-resources/cache/root# service tgt restart
tgt stop/waiting
tgt start/running, process 16692
root@ubuntumaas:/var/lib/maas/boot-resources/cache/root# cp ../root-tgz-3d15bdc99ae5cfe7e0be2e06e084636dc6fd809ec09ca54732ec83c9224376a2 ../../current/ubuntu/amd64/generic/trusty/daily/root-tgz
root@ubuntumaas:/var/lib/maas/boot-...

Read more...

tags: added: verification-done
removed: verification-needed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package curtin - 0.1.0~bzr195-0ubuntu1~14.04.1

---------------
curtin (0.1.0~bzr195-0ubuntu1~14.04.1) trusty-proposed; urgency=medium

  * New upstream snapshot.
    - hardware enablement: ppc64 support (LP: #1386394)
    - hardware enablement: know kernel mapping for utopic (hwe-u = 3.16)
      (LP: #1386394)
    - feature: support installing disk images including windows. (LP: #1386394)
    - feature: support creating swap file (LP: #1386394)
    - feature: support reporting logs back to MAAS (LP: #1386394)
    - feature: enable logging of installation to /var/log/curtin/install.log
      (LP: #1378910)
    - bug fix: extract tar files with xattr support when available (LP: #1313550)
    - bug fix: fix broken use of os.path.join for curtin hooks (LP: #1328521)
    - bug fix: util.subp to decode command output as utf-8 (LP: #1370249).
    - bug fix: call update-grub to ensure that /boot/grub/grub.cfg is created
      (LP: #1373137)
    - bug fix: do not use '--acl' when extracting tar files (LP: #1382632)
      as it inadvertently writes default directory acls.
    - bug fix: invoke lsblk with '--output' rather than '--out' to avoid
      ambiguity in newer versions of lsblk (LP: #1386275)
    - internal: part2bd helper added in helpers/common
    - internal: helpers: inherit curtin_verbosity (make the helper tools
      verbose if curtin invoked with verbose flags)
 -- Scott Moser <email address hidden> Mon, 27 Oct 2014 20:58:43 -0400

Changed in curtin (Ubuntu Trusty):
status: Fix Committed → Fix Released
Serge Hallyn (serge-hallyn) wrote :

Should this be marked fix-released (or invalid) for lxc?

Changed in maas:
status: Confirmed → Fix Committed
Scott Moser (smoser) on 2015-06-18
Changed in curtin:
status: Confirmed → Fix Committed
Changed in curtin (Ubuntu):
status: Confirmed → Fix Released
no longer affects: curtin (Ubuntu Saucy)
Changed in lxc (Ubuntu Precise):
status: Confirmed → Won't Fix
no longer affects: lxc (Ubuntu Precise)
no longer affects: lxc (Ubuntu Saucy)
Changed in lxc (Ubuntu Trusty):
status: Confirmed → Triaged
Changed in lxc (Ubuntu):
status: Confirmed → Triaged
Changed in lxc (Ubuntu Vivid):
importance: Undecided → High
status: New → Triaged
Changed in lxc (Ubuntu Wily):
importance: Undecided → High
status: New → Triaged
no longer affects: iputils (Ubuntu Vivid)
no longer affects: iputils (Ubuntu Wily)
no longer affects: curtin (Ubuntu Vivid)
no longer affects: curtin (Ubuntu Wily)
no longer affects: tar (Ubuntu Vivid)
no longer affects: tar (Ubuntu Wily)
Changed in maas:
status: Fix Committed → Fix Released
Serge Hallyn (serge-hallyn) wrote :

Marking as invalid for lxc as there is nothing it can do. One day I hope to get file capabilities in user namespaces working, but that will be kernel work.

Changed in lxc (Ubuntu):
status: Triaged → Invalid
Changed in lxc (Ubuntu Trusty):
status: Triaged → Invalid
no longer affects: lxc (Ubuntu Trusty)
no longer affects: lxc (Ubuntu Vivid)
no longer affects: lxc (Ubuntu Wily)
Scott Moser (smoser) wrote :

Marking as 'fix-released' in maas in wily and xenial, as the fix went into trunk at revision 4028.
https://code.launchpad.net/~rvb/maas/lp-1313550/+merge/262314

Changed in maas (Ubuntu Wily):
status: New → Fix Released
Changed in maas (Ubuntu):
status: Confirmed → Fix Released
Changed in maas (Ubuntu Vivid):
status: New → Confirmed
no longer affects: lxc (Ubuntu)
Changed in maas (Ubuntu Vivid):
importance: Undecided → High
Changed in maas (Ubuntu Wily):
importance: Undecided → High
Changed in maas (Ubuntu Vivid):
status: Confirmed → Won't Fix

The remaining trusty task is fixed in curtin according to c#29 as well, updating the tasks to match that.

Changed in maas (Ubuntu Trusty):
status: Confirmed → Fix Released

This bug is believed to be fixed in curtin in 17.1. If this is still a problem for you, please make a comment and set the state back to New

Thank you.

Changed in curtin:
status: Fix Committed → Fix Released
Scott Moser (smoser) on 2018-06-01
description: updated
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers