Bug 455713 - remove suid from ping
remove suid from ping
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: iputils (Show other bugs)
rawhide
All Linux
low Severity low
: ---
: ---
Assigned To: Jiri Skala
Fedora Extras Quality Assurance
: Reopened, Security
Depends On:
Blocks: 449984 rancid
  Show dependency treegraph
 
Reported: 2008-07-17 07:05 EDT by Peter Vrabec
Modified: 2014-11-09 17:31 EST (History)
12 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-03-09 11:04:13 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)

  None (edit)
Description Peter Vrabec 2008-07-17 07:05:01 EDT
Description of problem:
Since there is file capabilities support in F9, we  can remove suid from ping
command.

Use setcap command to set proper capabilities.
Comment 1 Boyan Anastasov 2008-09-09 04:33:16 EDT
This may seem as a good change, but look what's happening for kernel
not supporting those capabilities:

/usr/sbin/setcap cap_net_raw=ep /bin/ping ; echo "status=$?"
Failed to set capabilities on file `/bin/ping' (Operation not supported)
usage: setcap [-q] (-r|-|<caps>) <filename> [ ... (-r|-|<capsN>) <filenameN> ]

 Note <filename> must be a regular (non-symlink) file.
status=1


Maybe it's better to check the status of the commands in the postinstall
script, and if they fail - use the old behavior with suid root?

Something like this:
/usr/sbin/setcap cap_net_raw=ep /bin/ping
[ $? -ne 0 ] && chmod u+s /bin/ping
/usr/sbin/setcap cap_net_raw=ep /bin/ping6
[ $? -ne 0 ] && chmod u+s /bin/ping6
Comment 2 Adam Tkac 2008-09-09 09:37:41 EDT
This change has broken my configuration (iputils-20071127-5.fc10.x86_64):

$ ping google.com
ping: icmp open socket: Operation not permitted

I think this change is absolutely useless, look into ping{,6} code, main() function:

        setlocale(LC_ALL, "");

        icmp_sock = socket(AF_INET, SOCK_RAW, IPPROTO_ICMP);
        socket_errno = errno;

        uid = getuid();
        if (setuid(uid)) {
                perror("ping: setuid");
                exit(-1);
        }

It immediately drops root privileges so I don't see any reason for using capabilities. If you think that setuid is not sufficient you can use setresuid(2). Why do you think that using capabilities brings any benefit?
Comment 3 David Zeuthen 2008-09-09 16:13:40 EDT
Comment 2 seems pretty insightful, can we get the setuid bit back instead of randomly breaking systems for the fun of it? Thanks.
Comment 4 David Zeuthen 2008-09-09 16:16:04 EDT
... or at the _very_ least, if you don't want to add the setuid back, then you need something like "Conflicts: kernel < XYZ" where XYZ is the earliest kernel version with the stuff you depend on.
Comment 5 Valdis Kletnieks 2008-09-09 17:07:20 EDT
You also need to keep in mind that people *will* run self-compiled kernels, which may not exactly match the Fedora kernel in configuration.

% uname -a
Linux turing-police.cc.vt.edu 2.6.27-rc5-mmotm0829 #13 SMP PREEMPT Mon Sep 1 05:13:29 EDT 2008 x86_64 x86_64 x86_64 GNU/Linux
% zgrep CAPAB /proc/config.gz
# CONFIG_SECURITY_FILE_CAPABILITIES is not set

Blam.  Instant non-functional ping.

(Yes, sometimes software needs to exactly match the kernel - but that's mostly things like udev or modutils.  Nobody expects that changing the kernel will break ping, that's violating the Principle of Least Surprise)...
Comment 6 Steve Grubb 2008-09-10 09:45:36 EDT
We need to move ping to filesystem capabilities in order to find out what dragons lurk within. Ping is a pretty simple case. If we can't do it right for ping, then we will have problems for more complicated apps. So, in that spirit, I think this is a legitimate bug in how it is being implemented. We should adjust the post script to try to set the capabilities and then fallback to setuid on error. That is a reasonable compromise. 

We want to move several more apps, so we need to work out the recipe. If we cannot get this right by beta2, then we should go back to setuid and try again when rawhide opens for F-11 work. Thanks for helping to work through this.
Comment 7 Pierre Ossman 2008-09-14 13:24:06 EDT
Doing it in post will not solve the case of custom kernels as the user might be temporarily running a standard kernel at the time the package gets installed.
Comment 8 Pete Zaitcev 2008-09-14 17:30:11 EDT
For some weird reason, capabilites are not getting installed here:
getcap /bin/ping returns nothing (iputils-20071127-5.fc10.x86_64).
If I run the command from Boyan's comment #1 from shell, everything
works. Maybe some %postin are buggy in iputils, although rpm --scripts
shows the exact same command.

BTW, I think it would be better to add the capabilities into RPM
and describe them in %files rather than have them in %postinstall.
The fallback idea sounds too much like sweeping the problem under
the carpet.
Comment 9 Adam Jackson 2008-09-16 10:32:38 EDT
Would be pleasant to see this addressed one way or the other by F10.  Adding to the blocker.
Comment 10 Robert Scheck 2008-09-16 15:19:35 EDT
Regular pings without any specials seem to work for me with -5 currently.
Comment 11 Mamoru TASAKA 2008-09-24 04:23:24 EDT
Current rawhide ping (iputils-20071127-5.fc10) breaks srpm which checks the existence of ping like:

$ ping -c 1 -v 127.0.0.1 > /dev/null 2>&1

with dist-f10 scratch build on koji, which runs on 2.6.18(?) kernel (review request
bug 451189: build succeeds with dist-f9-updates-candidate).
Comment 12 Jindrich Novy 2008-09-25 06:13:12 EDT
Todays' update of iputils broke my ping as well. Any unpriviledged ping fails due to missing suid.
Comment 13 Adam Tkac 2008-09-25 06:44:55 EDT
From comments (especially comment #5 and comment #7) it seems that usage of file system capabilities in Fedora brings more problems than it solves.

People tune their systems so I think we can rely on suid binaries, review their code and use cap_{set,get}_proc(3) and setuid(2) functions instead file system based capabilities.

File system capabilities might be used in RHEL where you can tell that "your custom compiled kernel is not supported, please install distribution one" but this approach is speculative as well.
Comment 14 Jiri Skala 2008-09-26 02:22:10 EDT
I decided to make one step back due to following problems:
- strange behaviour %post section on x86_64 arch
- the file capability was missed once during some operations (permanently). Unfortunately I was not able to reproduce it to initiate fixing this problem.

The ping will use suid flag again. I'd like to put my opinion and remark to comments above. If somebody downgrade kernel against initial state she/he should await problems and writting one chmod command is only little subset of downgrading operations.
Comment 15 Lubomir Rintel 2010-03-09 10:29:08 EST
Reopening this, since it was closed with rather unclean reasoning while dropping setuid anywhere we could has rather strong benefits.

(In reply to comment #14)
> I decided to make one step back due to following problems:
> - strange behaviour %post section on x86_64 arch

What kind of strange behavior did you exactly experience? Is it reproducible, does it still happen?

> - the file capability was missed once during some operations (permanently).
> Unfortunately I was not able to reproduce it to initiate fixing this problem.

That's a rather weak proof that an issue exists. If you manage to reproduce the issue or provide a proof that it exist, I'll gladly put all my efforts into solving it.

> The ping will use suid flag again. I'd like to put my opinion and remark to
> comments above. If somebody downgrade kernel against initial state she/he
> should await problems and writting one chmod command is only little subset of
> downgrading operations.

If we applied the logic about custom kernels not working, we would need to stop relying on ext[234], since my custom compiled kernel without doesn't boot. My other kernel is a 2.0 and does not work with current glibc, so we should probably downgrade to libc5. That doesn't sound very right, does it?

For what it's worth, argument about "immediate" drop of suid is rather weak. main() is by far not the first code executed in a process, stuff's happening in dynamic linker and C library as well. By the way, do you remember pulseaudio also dropping setuid very early?
Comment 16 Lubomir Rintel 2010-03-09 10:33:16 EST
By the way, RPM now supports capabilities in %files section (use %caps()), no need to mess with scriptlets and setpcap anymore.
Comment 17 Steve Grubb 2010-03-09 11:04:13 EST
This bug was closed back when support for this was a problem. Since then we decided not to pursue this avenue. If we want to start changing from setuid to file system based attributes, I would suggest making a F-14 project page and running it by FESCO. But this bug report is not needed to do that, so let's close it back. The fedora-devel mail list would be a better place to discuss this.

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