Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 490956 - net-analyzer/wireshark has some unneccessary configure options
Summary: net-analyzer/wireshark has some unneccessary configure options
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal with 1 vote (vote)
Assignee: Netmon Herd
URL:
Whiteboard:
Keywords: PATCH
Depends on:
Blocks:
 
Reported: 2013-11-10 20:17 UTC by Mira Ressel
Modified: 2017-07-20 07:12 UTC (History)
1 user (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
Patch for the net-analyzer/wireshark-1.10.3 ebuild (wireshark-1.10.3.ebuild.patch,1.53 KB, patch)
2013-11-10 20:17 UTC, Mira Ressel
Details | Diff
Updated capability patch for wireshark ebuild (wireshark-1.11.0-r2.ebuild.patch,1.57 KB, patch)
2013-11-11 20:09 UTC, Mira Ressel
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mira Ressel 2013-11-10 20:17:09 UTC
Created attachment 363018 [details, diff]
Patch for the net-analyzer/wireshark-1.10.3 ebuild

The net-analyzer/wireshark ebuild includes 3 configure options in src_configure() which are uneccessary because the don't affect compilation, but only duplicate the behavior of the fcaps call in pkg_postinst().

These options cause problems on SELinux-enabled systems (as they disallow setfcap calls in src_install), and as they are unneccessary anyway, I removed them. My patch also removes the 2nd enewgroup call in pkg_setup, as it was only required when using the said configure options.

Furthermore, I discovered that bug 357237 doesn't apply anymore, and finally, I changed the perms in the fcaps call according to the default settings in fcaps.eclass, as suid and cap binaries shouldn't be world-readable (the read bits I removed would otherwise be removed by Portage anyway, that's the sfperms FEATURE).
Comment 1 Jeroen Roovers (RETIRED) gentoo-dev 2013-11-11 19:54:58 UTC
Comment on attachment 363018 [details, diff]
Patch for the net-analyzer/wireshark-1.10.3 ebuild

>--- c/net-analyzer/wireshark/wireshark-1.10.3.ebuild
>+++ w/net-analyzer/wireshark/wireshark-1.10.3.ebuild

I'd rather see a patch against the unstable 1.11 branch.

> 	if use pcap; then
>-		fcaps -o 0 -g wireshark -m 4550 -M 0750 \
>+		fcaps -o root -g wireshark -m 4710 -M 0710 \
> 			cap_dac_read_search,cap_net_raw,cap_net_admin \
> 			"${EROOT}"/usr/bin/dumpcap
> 	fi

Why are you replacing UID=0 with root? This breaks prefix support.
Comment 2 Mira Ressel 2013-11-11 20:09:58 UTC
Created attachment 363088 [details, diff]
Updated capability patch for wireshark ebuild

(In reply to Jeroen Roovers from comment #1)

> I'd rather see a patch against the unstable 1.11 branch.

Updating that was easy, only one of the contexts has changed.


> Why are you replacing UID=0 with root? This breaks prefix support.

Sorry, I didn't know about that implication. But if that's the case, shouldn't the default value in the eclass ("root") also be changed? (Btw: Are there other pitfalls related to prefix which could matter for this ebuild?)
Comment 3 Jeroen Roovers (RETIRED) gentoo-dev 2013-12-20 15:24:00 UTC
I fixed the permissions in 1.8.12, 1.10.5 and 1.11.2. The rest of the patch went into 1.11.2 only.
Comment 4 Mike Auty (RETIRED) gentoo-dev 2014-01-25 04:00:31 UTC
/usr/bin/dumpcap now doesn't get its group set as wireshark, so starting wireshark says "no interfaces found: dumpcap permission denied".  My guess is, it's the removal of the "$(use_with pcap dumpcap-group wireshark)" line.

This is not on an SE Linux system, and happens with the current 1.11.2 and 1.11.3_pre54663.  I don't think fcaps actually sets the group unless something goes wrong (at least, that's what the comment in the eclass suggests).  I can't tell whether this is a bug in fcaps.eclass or the correct behaviour (and therefore this ebuild is wrong), but I figured I'd try here first.  I'm more than happy to file a new bug if that's deemed necessary, just let me know...
Comment 5 Mira Ressel 2014-02-06 22:33:20 UTC
(In reply to Mike Auty from comment #4)

I'm sorry about this. Your analysis is correct; this is this the intented behaviour of the eclass. If this has to be fixed fast, a proper fix would be to but the "enewgroup" and "dumpcap-group" lines back in.

However, I'll talk to the eclass maintainer about this because I think I'd probably make sense to change the eclass semantics and/or include extra switches for this kind of thing.

@Jeroen: Sorry for breaking your package. I'll try the eclass route first for fixing as I think others might also profit from this kind of functionality, but I'll ping back if I don't reach a speedy solution.
Comment 6 Jeroen Roovers (RETIRED) gentoo-dev 2014-02-06 23:43:57 UTC
(In reply to Luis Ressel from comment #5)
> (In reply to Mike Auty from comment #4)
> 
> I'm sorry about this. Your analysis is correct; this is this the intented
> behaviour of the eclass. If this has to be fixed fast, a proper fix would be
> to but the "enewgroup" and "dumpcap-group" lines back in.

I have put both back in 1.11.2-r1 and 1.11.3_pre5537.

> However, I'll talk to the eclass maintainer about this because I think I'd
> probably make sense to change the eclass semantics and/or include extra
> switches for this kind of thing.

That's for another bug report.

> @Jeroen: Sorry for breaking your package. I'll try the eclass route first
> for fixing as I think others might also profit from this kind of
> functionality, but I'll ping back if I don't reach a speedy solution.

Please check the new ebuilds. It should be all right now.
Comment 7 Mira Ressel 2014-02-07 17:46:48 UTC
(In reply to Jeroen Roovers from comment #6)
> I have put both back in 1.11.2-r1 and 1.11.3_pre5537.

I'm also fine with that, as it means I'll have more time to discuss about the eclass.

> Please check the new ebuilds. It should be all right now.

You've forgot a slight subtlety: Letting the build system set the right group will only work if that group already exists; that's why we had pkg_setup(){enewgroup wireshark;}. You'll either have to put that back in, or just remove the "use_with pcap dumpcap-group wireshark" thingie again and do a manual chgrp in pkg_postinst, right after the fcaps call.

I'd go with the second solution - It doesn't matter for direct builds, but setting the group on the target system seems saner for binary packages.
Comment 8 Jeroen Roovers (RETIRED) gentoo-dev 2014-02-07 18:13:39 UTC
(In reply to Luis Ressel from comment #7)
> (In reply to Jeroen Roovers from comment #6)
> > I have put both back in 1.11.2-r1 and 1.11.3_pre5537.
> 
> I'm also fine with that, as it means I'll have more time to discuss about
> the eclass.
> 
> > Please check the new ebuilds. It should be all right now.
> 
> You've forgot a slight subtlety: Letting the build system set the right
> group will only work if that group already exists; that's why we had
> pkg_setup(){enewgroup wireshark;}. You'll either have to put that back in,
> or just remove the "use_with pcap dumpcap-group wireshark" thingie again and
> do a manual chgrp in pkg_postinst, right after the fcaps call.

You mean it's in pkg_postinst but not in pkg_setup?
Comment 9 Mira Ressel 2014-02-07 18:24:26 UTC
(In reply to Jeroen Roovers from comment #8)
> You mean it's in pkg_postinst but not in pkg_setup?

Yup, exactly. So we'll have to either restore the latter or set the group manually in pkg_postinst. I propose to take the second route.
Comment 10 Jeroen Roovers (RETIRED) gentoo-dev 2017-07-20 07:12:14 UTC
(In reply to Luis Ressel from comment #9)
> (In reply to Jeroen Roovers from comment #8)
> > You mean it's in pkg_postinst but not in pkg_setup?
> 
> Yup, exactly. So we'll have to either restore the latter or set the group
> manually in pkg_postinst. I propose to take the second route.

I think that was fixed a long time ago.