Bug 467936 - getfacl/setfacl remove SUID/SGID/Sticky-Bits
getfacl/setfacl remove SUID/SGID/Sticky-Bits
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: acl (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kamil Dudka
Fedora Extras Quality Assurance
: Triaged
Depends On:
Blocks: 571751 580572
  Show dependency treegraph
 
Reported: 2008-10-21 14:47 EDT by M. Steinborn
Modified: 2011-12-12 05:52 EST (History)
7 users (show)

See Also:
Fixed In Version: 2.2.49-3.fc13
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 571751 580572 (view as bug list)
Environment:
Last Closed: 2010-01-05 08:16:02 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
[PATCH] setfacl: changing owner and when S_ISUID should be set --restore fix (2.36 KB, patch)
2009-12-03 05:00 EST, Brandon Philips
no flags Details | Diff
setfacl: changing owner and when S_ISUID should be set --restore fix (2.72 KB, patch)
2009-12-17 15:19 EST, Brandon Philips
no flags Details | Diff

  None (edit)
Description M. Steinborn 2008-10-21 14:47:53 EDT
Note: This bug is also present in Fedora 8 and 9, RHEL 5.2.


Suppose, you want to create a backup using GNU tar and getfacl (the former for the contents of the files, the latter for the ACLs). So far that is working quite good.

Needing the backup, you'll restore it with "tar -xnpf ..." and restore the ACLs with "setfacl --restore ..." and what a pain, all SUID-Bits, GUID-Bits etc. are deleted.

The fastest test is the command line getfacl -R . | setfacl --restore -. Warning:The whole directory tree is processed recursivly!

A german discussion concerning this bug can be read here (http://tinyurl.com/442evz). The link is pointing to google groups.

That discussion lead to ideas, which I implemented by the following patches:

(Note: You can find the patches on http://www.markus-steinborn.de/acl/index.enu.htm )

To the user, the changes look as follows:

msteinbo@aasiaat:/privat/msteinbo/tmp>touch test.sh; chmod 4725 test.sh
msteinbo@aasiaat:/privat/msteinbo/tmp>ls -l
total 0
-rws-w-r-x 1 msteinbo infoabc 0 2008-02-15 12:48 test.sh

msteinbo@aasiaat:/privat/msteinbo/tmp>getfacl test.sh | tee test.acls
# file: test.sh
# owner: msteinbo
# group: infoabc
# mode: 04725
user::rwx
group::-w-
other::r-x

[Notice the new comment line with the octal mode of the file. Using a new comment line should ensure maximum compatibility to existing software.]

msteinbo@aasiaat:/privat/msteinbo/tmp>chmod 0000 test.sh ; ls -l test.sh
---------- 1 msteinbo infoabc 0 2008-02-15 12:48 test.sh

msteinbo@aasiaat:/privat/msteinbo/tmp>setfacl --restore test.acls ; ls -l
test.sh
-rws-w-r-x 1 msteinbo infoabc 0 2008-02-15 12:48 test.sh

[The Mode-Comment triggers a "chmod(2)", which is executed last and therefore it's effects are not overwritten.

Furthermore, the patch introduces the following new options:

getfacl:
      --with-mode         include mode in output
      --without-mode      do not include mode in output - also available
                          via environment variable ACL_OUTPUT_WO_MODE

setfacl:
       --keep-smodes       for --restore: keep suid/sgid/sticky-bits
       --no-keep-smodes    for --restore: do not keep suid/sgid/sticky-bits

The option "--keep-smodes" enables the user to restore backups that are created as described above on an unpatched system without any errors.
Comment 1 Jiri Moskovcak 2008-10-31 09:22:58 EDT
Hi,
this is quite a big change so it should be accepted by upstream first. I'm going to ask them.
Comment 2 Bug Zapper 2008-11-25 23:05:51 EST
This bug appears to have been reported against 'rawhide' during the Fedora 10 development cycle.
Changing version to '10'.

More information and reason for this action is here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping
Comment 3 M. Steinborn 2009-04-05 12:43:38 EDT
Did you get any reply from upstream?
Comment 4 Brandon Philips 2009-05-08 19:00:33 EDT
The third field [of getfacl] contains the discretionary access permissions.
This
standard defines the following symbolic discretionary access
permissions:

        r Read access
        w Write access
        x Execute/search access
        - No access by this ACL entry

The discretionary access permissions field shall contain exactly one
each of the following characters in the following order: r, w, and x.
Each of these may be replaced by the "-" character to indicate no
access. An implementation may define additional characters following the
required characters that represent implementation-defined permissions.

Also there is this:
 http://acl.bestbits.at/pipermail/acl-devel/2004-September/001743.html

[1]
http://wt.xpilot.org/publications/posix.1e/download/Posix_1003.2c-990310.pdf.gz
Comment 5 Bug Zapper 2009-11-18 03:37:09 EST
This message is a reminder that Fedora 10 is nearing its end of life.
Approximately 30 (thirty) days from now Fedora will stop maintaining
and issuing updates for Fedora 10.  It is Fedora's policy to close all
bug reports from releases that are no longer maintained.  At that time
this bug will be closed as WONTFIX if it remains open with a Fedora 
'version' of '10'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version prior to Fedora 10's end of life.

Bug Reporter: Thank you for reporting this issue and we are sorry that 
we may not be able to fix it before Fedora 10 is end of life.  If you 
would still like to see this bug fixed and are able to reproduce it 
against a later version of Fedora please change the 'version' of this 
bug to the applicable version.  If you are unable to change the version, 
please add a comment here and someone will do it for you.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events.  Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

The process we are following is described here: 
http://fedoraproject.org/wiki/BugZappers/HouseKeeping
Comment 6 Brandon Philips 2009-11-29 15:12:04 EST
A fix for this is available in acl 2.2.49: 
 http://download.savannah.gnu.org/releases-noredirect/acl/acl-2.2.49.src.tar.gz
Comment 7 M. Steinborn 2009-11-30 05:21:32 EST
Well, the standard situations seem to work now. But once again I managed to construct a situation where the SUID-Bit is lost using setfacl version 2.2.49:

-------------- cut here --------------------
root@acer:/tmp/test>ls -l
insgesamt 28
-rwsr-xr-x 1 bin  root 22984 30. Nov 10:37 passwd
-rw-r--r-- 1 root root    89 30. Nov 10:38 passwd.acl
root@acer:/tmp/test>cat passwd.acl
# file: passwd
# owner: root
# group: root
# flags: s--
user::rwx
group::r-x
other::r-x

root@acer:/tmp/test>/tmp/1/acl-2.2.49/setfacl/setfacl --restore passwd.acl
root@acer:/tmp/test>ls -l
insgesamt 28
-rwxr-xr-x 1 root root 22984 30. Nov 10:37 passwd
-rw-r--r-- 1 root root    89 30. Nov 10:38 passwd.acl
root@acer:/tmp/test>
-------------- cut here --------------------

PS: Set version to rawhide as this bug applies to every red hat linux (incl. RHEL). So it is not wise to let this bug expire.
Comment 8 Brandon Philips 2009-12-02 13:59:41 EST
Works fine here. Something about how you are running that binary from a directory must be breaking something. Please try again after installing the binary properly.

jenkins:/home/philips/test # touch passwd
jenkins:/home/philips/test # cat > passwd.acl
# file: passwd
# owner: root
# group: root
# flags: s--
user::rwx
group::r-x
other::r-x
jenkins:/home/philips/test # ls -la
total 20
drwxr-xr-x   2 philips users  4096 2009-12-02 10:57 .
drwxr-xr-x 111 philips users 12288 2009-12-02 10:52 ..
-rw-r--r--   1 root    root      0 2009-12-02 10:57 passwd
-rw-r--r--   1 root    root     88 2009-12-02 10:57 passwd.acl
jenkins:/home/philips/test # setfacl --restore passwd.acl 
jenkins:/home/philips/test # ls -la
total 20
drwxr-xr-x   2 philips users  4096 2009-12-02 10:57 .
drwxr-xr-x 111 philips users 12288 2009-12-02 10:52 ..
-rwsr-xr-x   1 root    root      0 2009-12-02 10:57 passwd
-rw-r--r--   1 root    root     88 2009-12-02 10:57 passwd.acl
Comment 9 M. Steinborn 2009-12-03 02:28:43 EST
I am sorry to tell you, but you have missed  an important point: My test starts with a "passwd"-file owned by user "bin" (woith suid flag set), not "root"! After setfacl it is owned by root and should be suid.

Your test also works for me.

BTW: Running the setfacl command twice helps in my test case. This is quite a good workaround.

THe man page states that this sould work:
           setfacl attempts to restore the owner  and  owning  group.  If  the
           input contains flags comments (which define the setuid, setgid, and
           sticky bits), setfacl sets those three bits accordingly

Note: In my above test, "bin" and "root" are arbitraty, ordinary users could also been used. But you have to run the test as root.

Is this test case usefull? IMHO yes, suppose you want to change some User IDs. You can do it the following way:

            1) getfacl -R --absolute-names /home > /tmp/acl.backup.txt
            2) Change UIDs in /etc/passwd
            3) Corrent owners by
                  setfacl --restore /tmp/acl.backup.txt
Comment 10 Brandon Philips 2009-12-03 05:00:21 EST
Created attachment 375723 [details]
[PATCH] setfacl: changing owner and when S_ISUID should be set --restore fix 

Could you try this patch out. I think it should fix the issue.
Comment 11 M. Steinborn 2009-12-05 13:31:47 EST
Works like a charm for me.


BTW: The bug http://savannah.nongnu.org/bugs/?28131 keeps me from updating acl from version 2.2.39 + improved version of my patch from the initial posting of this bug report.


@Redhat: What about updating to version 2.2.49 + above patch?
Comment 12 M. Steinborn 2009-12-05 15:13:00 EST
With an invalid flags line you can cause a core dump. But this is a new bug. so there is a new bug report on https://savannah.nongnu.org/bugs/index.php?28185 .
Comment 13 Brandon Philips 2009-12-17 15:19:39 EST
Created attachment 379082 [details]
setfacl: changing owner and when S_ISUID should be set --restore fix 

This patch avoids yet another stat() and does the same thing. This should be the final fix for this.
Comment 14 M. Steinborn 2009-12-27 04:40:48 EST
Just as a reminder: The patch attached to this bug report (which is by the way from upstream) is missing in the latest koji builds.
Comment 15 Kamil Dudka 2009-12-27 05:36:41 EST
I still can't see the patch in the upstream git.

Brandon, is there any reason to hold on with applying the patch?
Comment 16 Brandon Philips 2009-12-31 22:18:03 EST
(In reply to comment #15)
> I still can't see the patch in the upstream git.
> 
> Brandon, is there any reason to hold on with applying the patch?  

Forgot to push. It should be there now. The patch should be fine.
Comment 17 Kamil Dudka 2010-01-05 08:16:02 EST
Thank you, Brandon!

Built as acl-2.2.49-3.fc13.
Comment 18 Bruno Cornec 2010-03-06 20:11:43 EST
Will this fix also find its way to RHEL ? Currently RHEL5.4 also suffers from that bug (as reported here: http://trac.mondorescue.org/ticket/399)

Is it too late for RHEL5.5 ?
Comment 19 Ondrej Vasik 2010-03-09 03:11:49 EST
(In reply to comment #18)
> Is it too late for RHEL5.5 ?

Yep, it's too late... but feel free to clone that bugzilla against RHEL-5 to increase chances to get this fixed in future RHEL-5 acl update.

Note You need to log in before you can comment on or make changes to this bug.