-
Notifications
You must be signed in to change notification settings - Fork 251
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
Add support for -k/--fwmark option #202
Conversation
Satisfies Issue #158 I am not configured to test ipv6 but it should work. |
For --fwmark to work either run as root or set the cap_net_raw and cap_net_admin file capabilities. setuid does not work for fwmark. sudo chmod u-s /usr/bin/fping README.md ci/prepare-linux.sh and contrib/fping.spec can be updated to reflect this. I will add those changes. |
Yes it looks like the same thing. At least for fwmark there is cap_net_admin as a workaround. For SO_BINDTODEVICE I don't see a capability that will work documented anywhere. |
Checked the kernel source. cap_net_raw should work for SO_BINDTODEVICE, but I didn't try it. |
Yes, cap_net_raw works for SO_BINDTODEVICE. However, without my extra changes to unbreak setuid (or some other change to the same effect), fping remains half-broken on a very large set of machines that cannot operate with capabilites. E.g. every small OpenWrt router. They simply don't use filesystems (and in some case, deployment and update methods) that can deal with extended attributes. That means effectively no capabilities support, even if it is Linux, and even if capabilities are enabled in the kernel. I am not against merging this PR as-is. I will just adjust mine to also fix -k/--fwmark if it lands first, because I believe fping should work in setuid mode just as well as it does with capabilities. |
So, to be very clear, I have no objections against the merge of this PR in its current state. |
What's holding up this from being merged? #200 shows a status of merged. Status checks show that all checks have passed. |
I'd also like to see this merged, and I see it has some conflicts. If no one else has time, I'd be happy to reimplement this on top of the current HEAD. |
Done: #289 |
No description provided.