Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

btrfs send | btrfs receive adds "security.capability" xattr to some files #292

Closed
Nox996 opened this issue Sep 1, 2020 · 6 comments
Closed
Labels
bug kernel something in kernel has to be done too

Comments

@Nox996
Copy link

Nox996 commented Sep 1, 2020

On my arch linux built server, I am experiencing inconsistencies with my btrfs volumes and backup strategy using btrfs send | btrfs receive, to be specific, xattrs seem to be added or corrupted for some files.

Environment
System: Arch Linux using

  • Kernel 5.8.5-arch1-1
  • btrfs-progs 5.7-1

Root is on a btrfs subvolume "@" on a btrfs partition, residing on a luks container on /dev/sda
Snapshots of "@" are created on the "@snapshots" subvolume that is mounted on "/.snapshots"

These snapshots are sent to another "backup" btrfs volume residing on the same PC for backup reasons, consisting of

  • two LUKS containers on /dev/sdc and /dev/sdd, that are combined in a RAID1 btrfs volume. The root of this btrfs volume is mounted on /mnt/pool_Tank.

Problem:
When sending a snapshot from the root btrfs volume to the backup volume using the btrfs send | btrfs receive command chain, some binaries receive xattrs from the security.capability domain.

Example (this is also reproducible on my machine):

  1. Send / Receive the snapshot:
/mnt/pool_Tank/temp # btrfs send /.snapshots/@.20200809 | btrfs receive ./
    At subvol /.snapshots/@.20200809
    At subvol @.20200809
btrfs send /.snapshots/@.20200809  0.21s user 23.65s system 38% cpu 1:01.23 total
btrfs receive ./  6.47s user 14.55s system 32% cpu 1:03.73 total
  1. Check the xattrs for e.g. /usr/bin/newusers:
% sudo getfattr -d -m "-" --absolute-names /.snapshots/@.20200809/usr/bin/newusers
% sudo getfattr -d -m "-" --absolute-names /mnt/pool_Tank/temp/@.20200809/usr/bin/newusers
	# file: /mnt/pool_Tank/temp/@.20200809/usr/bin/newusers
	security.capability=0sAQAAAoAAAAAAAAAAAAAAAAAAAAA=
@Seb35
Copy link

Seb35 commented Sep 1, 2020

I don’t have an extensive knowledge of btrfs, and I will let someone more knowledgeable answer, but I mention these two other issues with the inverse issue: #85 and #202.

Although, you can provide the relevant lines (or their absence) of the verbose output of receive to better understand the situation: btrfs send /.snapshots/@.20200809 | btrfs receive -vv ./ 2>log-file.txt.

@Nox996
Copy link
Author

Nox996 commented Sep 1, 2020

Hello @Seb35,

Here are the (at least seeming to me) relevant lines of the verbose output of btrfs receive:

mkfile o19227-19-0
rename o19227-19-0 -> usr/bin/newuidmap
utimes usr/bin
write usr/bin/newuidmap - offset=0 length=36992
chown usr/bin/newuidmap - uid=0, gid=0
chown: restore capabilities
chmod usr/bin/newuidmap - mode=0755
set_xattr usr/bin/newuidmap - name=security.capability data_len=20 data=�
utimes usr/bin/newuidmap
mkfile o19228-19-0
rename o19228-19-0 -> usr/bin/newusers
utimes usr/bin
write usr/bin/newusers - offset=0 length=49152
write usr/bin/newusers - offset=49152 length=43208
chown usr/bin/newusers - uid=0, gid=0
chown: restore capabilities
chmod usr/bin/newusers - mode=0755
utimes usr/bin/newusers
mkfile o26949-20-0
rename o26949-20-0 -> usr/bin/ping
utimes usr/bin
write usr/bin/ping - offset=0 length=49152
write usr/bin/ping - offset=49152 length=19216
chown usr/bin/ping - uid=0, gid=0
chmod usr/bin/ping - mode=0755
set_xattr usr/bin/ping - name=security.capability data_len=20 data=�
utimes usr/bin/ping
mkfile o26950-20-0
rename o26950-20-0 -> usr/bin/rarpd
utimes usr/bin
write usr/bin/rarpd - offset=0 length=22536
chown usr/bin/rarpd - uid=0, gid=0
chown: restore capabilities
chmod usr/bin/rarpd - mode=0755
utimes usr/bin/rarpd
mkfile o39055-21-0
rename o39055-21-0 -> usr/bin/rcp
utimes usr/bin
write usr/bin/rcp - offset=0 length=49152
write usr/bin/rcp - offset=49152 length=14720
chown usr/bin/rcp - uid=0, gid=0
chmod usr/bin/rcp - mode=0755
set_xattr usr/bin/rcp - name=security.capability data_len=20 data=�
utimes usr/bin/rcp
mkfile o39056-21-0
rename o39056-21-0 -> usr/bin/rlogin
utimes usr/bin
write usr/bin/rlogin - offset=0 length=49152
write usr/bin/rlogin - offset=49152 length=10808
chown usr/bin/rlogin - uid=0, gid=0
chown: restore capabilities
chmod usr/bin/rlogin - mode=0755
set_xattr usr/bin/rlogin - name=security.capability data_len=20 data=�
utimes usr/bin/rlogin
mkfile o39057-21-0
rename o39057-21-0 -> usr/bin/rlogind
utimes usr/bin
write usr/bin/rlogind - offset=0 length=49152
write usr/bin/rlogind - offset=49152 length=23136
chown usr/bin/rlogind - uid=0, gid=0
chown: restore capabilities
chmod usr/bin/rlogind - mode=0755
utimes usr/bin/rlogind
mkfile o39058-21-0
rename o39058-21-0 -> usr/bin/rsh
utimes usr/bin
write usr/bin/rsh - offset=0 length=49152
write usr/bin/rsh - offset=49152 length=6520
chown usr/bin/rsh - uid=0, gid=0
chmod usr/bin/rsh - mode=0755
set_xattr usr/bin/rsh - name=security.capability data_len=20 data=�
utimes usr/bin/rsh
mkfile o39059-21-0
rename o39059-21-0 -> usr/bin/rshd
utimes usr/bin
write usr/bin/rshd - offset=0 length=49152
write usr/bin/rshd - offset=49152 length=15328
chown usr/bin/rshd - uid=0, gid=0
chown: restore capabilities
chmod usr/bin/rshd - mode=0755
utimes usr/bin/rshd

It seems to me that setting the wrong xattrs depends on the received file "before", that is why I have included these in the trace above.

Wrong xattrs (i.e. a security.capability is set) are existing for:

/usr/bin/newusers
/usr/bin/rarpd
/usr/bin/rlogind
/usr/bin/rshd

Rsync comparison:

% sudo rsync -n --itemize-changes --checksum -a --delete --numeric-ids --hard-links --acls --xattrs --devices --specials -v /.snapshots/@.20200809/ /mnt/pool_Tank/temp/@.20200809
sending incremental file list
.d..t...... ./
cd+++++++++ srv/
.f........x usr/bin/newusers
.f........x usr/bin/rarpd
.f........x usr/bin/rlogind
.f........x usr/bin/rshd
cd+++++++++ var/abs/ 
cd+++++++++ var/cache/pacman/pkg/
cd+++++++++ var/lib/machines/
cd+++++++++ var/lib/portables/
cd+++++++++ var/tmp/

sent 2,646,540 bytes  received 4,863 bytes  62,385.95 bytes/sec
total size is 2,175,012,332  speedup is 820.33 (DRY RUN)
sudo rsync -n --itemize-changes --checksum -a --delete --numeric-ids  --acls   2.43s user 11.59s system 30% cpu 45.811 total

@Nox996
Copy link
Author

Nox996 commented Sep 6, 2020

I was able to do some more experiments and have also created a tiny script with which I can reproduce the issue to 100% on my machine.

The kernel versions below are for arch linux vanilla kernel

My findings are:

  1. I am able to reproduce the issue with the following combinations of Kernel & btrfs-progs:

    • Linux 5.7.12 + btrfs-progs 5.7
    • Linux 5.8.5 + btrfs-progs 5.7
  2. I am not able to reproduce the issue with Linux 5.6.15 + btrfs-progs 5.7

  3. It seems that in the error case, the file that is sent / received after a file with security capabilities gets same security capabilities

  4. I am attaching my tiny script for issue reproduction here (rename to .sh):
    btrfs_capability_issues.txt

  5. Here is a btrfs-receive -vv full log in the error case:
    btrfs_receive_log.txt

As it seems to me this is dependent on the kernel version, I am creating an issue on the kernel.org bugzilla for this now.

@Damenly
Copy link
Contributor

Damenly commented Sep 11, 2020

Figured it out.
The kernel commit btrfs: send: emit file capabilities after chown corrected the behavior caused issue202.
Now the kernel emits "security.capability" ATTR after CHOWN in send stream.
However, progs still expects ATTR before CHOWN. See btrfs-progs: receive: restore capabilities after chown.

Simply reverting the progs commit could solve the issue, but should compatibility with old kernels be considered?

@kdave
Copy link
Owner

kdave commented Sep 11, 2020

Yes the compatibility should be considered, there are various combinations of progs/kernel out there. The workaround will be deleted eventually but it seems that now we need to enhance it to work with the new kernel behaviour. I've seen some send+capabilities fstests fail so that seems be still unfixed. Thanks for additional info.

@kdave kdave added the bug label Sep 11, 2020
@kdave kdave added the kernel something in kernel has to be done too label Feb 11, 2021
@kdave
Copy link
Owner

kdave commented Feb 11, 2021

The workaround is going to be deleted in the next release, fixed by patch "btrfs-progs: receive: remove workaround for setting capabilities", the kernel patch has been backported to all live stable kernel versions.

@kdave kdave closed this as completed Feb 11, 2021
kdave pushed a commit that referenced this issue Feb 11, 2021
We had a few bugs on the kernel side of send/receive where capabilities
ended up being lost after receiving a send stream. They all stem from the
fact that the kernel used to send all xattrs before issuing the chown
command, and the later clears any existing capabilities in a file or
directory.

Initially a workaround was added to btrfs-progs' receive command, in commit
123a2a0 ("btrfs-progs: receive: restore capabilities after chown"),
and that fixed some instances of the problem. More recently, other instances
of the problem were found, a proper fix for the kernel was made, which fixes
the root problem by making send always emit the setxattr command for setting
capabilities after issuing a chown command. This was done in kernel commit
89efda52e6b693 ("btrfs: send: emit file capabilities after chown"), which
landed in kernel 5.8.

However, the workaround on the receive command now causes us to incorrectly
set a capability on a file that should not have it, because it assumes all
setxattr commands for a file always comes before a chown.

Example reproducer:

  $ cat send-caps.sh
  #!/bin/bash

  DEV1=/dev/sdh
  DEV2=/dev/sdi

  MNT1=/mnt/sdh
  MNT2=/mnt/sdi

  mkfs.btrfs -f $DEV1 > /dev/null
  mkfs.btrfs -f $DEV2 > /dev/null

  mount $DEV1 $MNT1
  mount $DEV2 $MNT2

  touch $MNT1/foo
  touch $MNT1/bar
  setcap cap_net_raw=p $MNT1/foo

  btrfs subvolume snapshot -r $MNT1 $MNT1/snap1

  btrfs send $MNT1/snap1 | btrfs receive $MNT2

  echo
  echo "capabilities on destination filesystem:"
  echo
  getcap $MNT2/snap1/foo
  getcap $MNT2/snap1/bar

  umount $MNT1
  umount $MNT2

When running the test script, we can see that both files foo and bar get
the capability set, when only file foo should have it:

  $ ./send-caps.sh
  Create a readonly snapshot of '/mnt/sdh' in '/mnt/sdh/snap1'
  At subvol /mnt/sdh/snap1
  At subvol snap1

  capabilities on destination filesystem:

  /mnt/sdi/snap1/foo cap_net_raw=p
  /mnt/sdi/snap1/bar cap_net_raw=p

Since the kernel fix was backported to all currently supported stable
releases (5.10.x, 5.4.x, 4.19.x, 4.14.x, 4.9.x and 4.4.x), remove the
workaround from receive. Having such a workaround relying on the order
of commands in a send stream is always troublesome and doomed to break
one day.

A test case for fstests will come soon.

Issue: #85
Issue: #202
Issue: #292
Reported-by: Richard Brown <rbrown@suse.de>
Reviewed-by: Su Yue <l@damenly.su>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
kdave pushed a commit that referenced this issue Feb 19, 2021
We had a few bugs on the kernel side of send/receive where capabilities
ended up being lost after receiving a send stream. They all stem from the
fact that the kernel used to send all xattrs before issuing the chown
command, and the later clears any existing capabilities in a file or
directory.

Initially a workaround was added to btrfs-progs' receive command, in commit
123a2a0 ("btrfs-progs: receive: restore capabilities after chown"),
and that fixed some instances of the problem. More recently, other instances
of the problem were found, a proper fix for the kernel was made, which fixes
the root problem by making send always emit the setxattr command for setting
capabilities after issuing a chown command. This was done in kernel commit
89efda52e6b693 ("btrfs: send: emit file capabilities after chown"), which
landed in kernel 5.8.

However, the workaround on the receive command now causes us to incorrectly
set a capability on a file that should not have it, because it assumes all
setxattr commands for a file always comes before a chown.

Example reproducer:

  $ cat send-caps.sh
  #!/bin/bash

  DEV1=/dev/sdh
  DEV2=/dev/sdi

  MNT1=/mnt/sdh
  MNT2=/mnt/sdi

  mkfs.btrfs -f $DEV1 > /dev/null
  mkfs.btrfs -f $DEV2 > /dev/null

  mount $DEV1 $MNT1
  mount $DEV2 $MNT2

  touch $MNT1/foo
  touch $MNT1/bar
  setcap cap_net_raw=p $MNT1/foo

  btrfs subvolume snapshot -r $MNT1 $MNT1/snap1

  btrfs send $MNT1/snap1 | btrfs receive $MNT2

  echo
  echo "capabilities on destination filesystem:"
  echo
  getcap $MNT2/snap1/foo
  getcap $MNT2/snap1/bar

  umount $MNT1
  umount $MNT2

When running the test script, we can see that both files foo and bar get
the capability set, when only file foo should have it:

  $ ./send-caps.sh
  Create a readonly snapshot of '/mnt/sdh' in '/mnt/sdh/snap1'
  At subvol /mnt/sdh/snap1
  At subvol snap1

  capabilities on destination filesystem:

  /mnt/sdi/snap1/foo cap_net_raw=p
  /mnt/sdi/snap1/bar cap_net_raw=p

Since the kernel fix was backported to all currently supported stable
releases (5.10.x, 5.4.x, 4.19.x, 4.14.x, 4.9.x and 4.4.x), remove the
workaround from receive. Having such a workaround relying on the order
of commands in a send stream is always troublesome and doomed to break
one day.

A test case for fstests will come soon.

Issue: #85
Issue: #202
Issue: #292
Reported-by: Richard Brown <rbrown@suse.de>
Reviewed-by: Su Yue <l@damenly.su>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
kdave pushed a commit that referenced this issue Feb 22, 2021
We had a few bugs on the kernel side of send/receive where capabilities
ended up being lost after receiving a send stream. They all stem from the
fact that the kernel used to send all xattrs before issuing the chown
command, and the later clears any existing capabilities in a file or
directory.

Initially a workaround was added to btrfs-progs' receive command, in commit
123a2a0 ("btrfs-progs: receive: restore capabilities after chown"),
and that fixed some instances of the problem. More recently, other instances
of the problem were found, a proper fix for the kernel was made, which fixes
the root problem by making send always emit the setxattr command for setting
capabilities after issuing a chown command. This was done in kernel commit
89efda52e6b693 ("btrfs: send: emit file capabilities after chown"), which
landed in kernel 5.8.

However, the workaround on the receive command now causes us to incorrectly
set a capability on a file that should not have it, because it assumes all
setxattr commands for a file always comes before a chown.

Example reproducer:

  $ cat send-caps.sh
  #!/bin/bash

  DEV1=/dev/sdh
  DEV2=/dev/sdi

  MNT1=/mnt/sdh
  MNT2=/mnt/sdi

  mkfs.btrfs -f $DEV1 > /dev/null
  mkfs.btrfs -f $DEV2 > /dev/null

  mount $DEV1 $MNT1
  mount $DEV2 $MNT2

  touch $MNT1/foo
  touch $MNT1/bar
  setcap cap_net_raw=p $MNT1/foo

  btrfs subvolume snapshot -r $MNT1 $MNT1/snap1

  btrfs send $MNT1/snap1 | btrfs receive $MNT2

  echo
  echo "capabilities on destination filesystem:"
  echo
  getcap $MNT2/snap1/foo
  getcap $MNT2/snap1/bar

  umount $MNT1
  umount $MNT2

When running the test script, we can see that both files foo and bar get
the capability set, when only file foo should have it:

  $ ./send-caps.sh
  Create a readonly snapshot of '/mnt/sdh' in '/mnt/sdh/snap1'
  At subvol /mnt/sdh/snap1
  At subvol snap1

  capabilities on destination filesystem:

  /mnt/sdi/snap1/foo cap_net_raw=p
  /mnt/sdi/snap1/bar cap_net_raw=p

Since the kernel fix was backported to all currently supported stable
releases (5.10.x, 5.4.x, 4.19.x, 4.14.x, 4.9.x and 4.4.x), remove the
workaround from receive. Having such a workaround relying on the order
of commands in a send stream is always troublesome and doomed to break
one day.

A test case for fstests will come soon.

Issue: #85
Issue: #202
Issue: #292
Reported-by: Richard Brown <rbrown@suse.de>
Reviewed-by: Su Yue <l@damenly.su>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug kernel something in kernel has to be done too
Projects
None yet
Development

No branches or pull requests

4 participants