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

Use go for the ping plugin #496

Closed
xor-gate opened this issue Jan 8, 2016 · 8 comments · Fixed by #629
Closed

Use go for the ping plugin #496

xor-gate opened this issue Jan 8, 2016 · 8 comments · Fixed by #629

Comments

@xor-gate
Copy link

xor-gate commented Jan 8, 2016

I'm running telegraf in a systemd container with debian jessie. It seems it calls exec directly on /bin/ping. Telegraf runs under the user telegraf so the suid bit needs to be set. Maybe it should check this before invoking ?. And maybe it would be much nicer if we can directly ICMP from golang instead of executing a binary?

Jan 08 12:24:00 dev03 telegraf[14763]: ping: icmp open socket: Operation not permitted, exit status 2
[INCORRECT]
ls -al /bin/ping
-rwxr-xr-x 1 root root 40760 May 20 2011 /bin/ping

[CORRECT]
[root@pccadmin ~]# chmod 4755 /bin/ping
[root@pccadmin ~]# ls -tlr /bin/ping
-rwsr-xr-x. 1 root root 40760 Jul 18 2011 /bin/ping

then it works:

Jan 08 12:27:10 dev03 telegraf[14763]: 2016/01/08 12:27:10 Flushed 460 metrics to output influxdb in 62.249947ms
Jan 08 12:27:13 dev03 telegraf[14763]: 2016/01/08 12:27:13 Error in plugin [ping]: PING psu.local (192.168.23.65) 56(84) bytes of data.
Jan 08 12:27:13 dev03 telegraf[14763]: From 192.168.22.180 icmp_seq=1 Destination Host Unreachable
Jan 08 12:27:13 dev03 telegraf[14763]: --- psu.local ping statistics ---
Jan 08 12:27:13 dev03 telegraf[14763]: 1 packets transmitted, 0 received, +1 errors, 100% packet loss, time 0ms, exit status 1
@zstyblik
Copy link
Contributor

zstyblik commented Jan 8, 2016

Maybe it should check this before invoking ?

Do you mean Telegraf should check every 10, or so, seconds? Also, this is imho Debian's problem and not the Telegraf's problem. Don't get me wrong. I had the same problem with Nagios checks and wasn't happy about it either.

And maybe it would be much nicer if we can directly ICMP from golang instead of executing a binary?

That would be awesome.

@sparrc
Copy link
Contributor

sparrc commented Jan 8, 2016

looks like we may be able to use this library: https://github.com/tatsushid/go-fastping, that would be a better solution than executing the ping binary

@zstyblik
Copy link
Contributor

zstyblik commented Jan 8, 2016

@xor-gate btw the correct solution is to use/set capabilities % setcap cap_net_raw+p /bin/ping

@xor-gate
Copy link
Author

@zstyblik, you are right i'm not very familiar with the capabilities under linux. It is good to mention because setuid bit is kind of evil.

@zstyblik
Copy link
Contributor

@xor-gate no worries. It's not from my head either ;)

@xor-gate xor-gate changed the title Ping not permitted when setuid bit is not set [Debian] Ping not permitted when setuid/cap_net_raw bit is not set for /bin/ping Jan 14, 2016
@sparrc sparrc changed the title [Debian] Ping not permitted when setuid/cap_net_raw bit is not set for /bin/ping Use go for the ping plugin Jan 15, 2016
@sparrc
Copy link
Contributor

sparrc commented Jan 15, 2016

Changing this case to just track using golang for the ping plugin, which is probably the only real solution to this issue

@sparrc
Copy link
Contributor

sparrc commented Feb 1, 2016

I wrote up a library for pinging via Go: https://github.com/sparrc/go-ping

BUT unfortunately this doesn't help much at all. Pinging relies on the setuid (or set capabilities) bits to execute certain system calls with root permission. Unless you gave the entire telegraf binary this permission (very very bad idea), pinging from Go has even less capabilities than using the binary. There are "unpriviliged" pings possible, but in linux this requires setting a kernel variable: https://github.com/sparrc/go-ping#note-on-linux-support

I'm going to close this case with #629, in which I'm also putting a note in the config that some users may need to set capabilities on their ping binary for this plugin to work.

@xor-gate
Copy link
Author

xor-gate commented Feb 2, 2016

@sparrc this looks good 👍 , and unfortunately we need to set the capabilities but it is much much better than the previous exec and should be more portable when running under BSDs (chrooted, restricted etcetera).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants