-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Heartbeat] Rootless pings #13795
Merged
Merged
[Heartbeat] Rootless pings #13795
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
e1c90e8
Adding non-privileged icmp on linux and darwin
leopucci ccdaafc
trying to understand why it does not run on travis
leopucci 5a4eac5
basic test
leopucci 5d27bbe
adding timeout
leopucci a9f616c
=)
leopucci a790ea7
group permission comparison
leopucci 579dd2c
osx trial
leopucci 609a5f4
adjust
leopucci 6daf6f2
=)
leopucci a185b2d
go go go
leopucci 7fb8291
Merge remote-tracking branch 'origin/master' into rootless-pings
andrewvc e5a51ba
Improvements on tests and error reporting
andrewvc 6afb77f
Use errorf
andrewvc b50106c
Incorporate PR feedback
andrewvc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,14 +24,14 @@ import ( | |
"fmt" | ||
"math/rand" | ||
"net" | ||
"os" | ||
"runtime" | ||
"sync" | ||
"time" | ||
|
||
"golang.org/x/net/icmp" | ||
"golang.org/x/net/ipv4" | ||
"golang.org/x/net/ipv6" | ||
|
||
"github.com/elastic/beats/libbeat/logp" | ||
) | ||
|
||
type icmpLoop struct { | ||
|
@@ -85,26 +85,52 @@ var ( | |
loop *icmpLoop | ||
) | ||
|
||
func noPingCapabilityError(message string) error { | ||
return fmt.Errorf(fmt.Sprintf("Insufficient privileges to perform ICMP ping. %s", message)) | ||
} | ||
|
||
func newICMPLoop() (*icmpLoop, error) { | ||
// Log errors at info level, as the loop is setup globally when ICMP module is loaded | ||
// first (not yet configured). | ||
// With multiple configurations using the icmp loop, we have to postpose | ||
// IPv4/IPv6 checking | ||
conn4 := createListener("IPv4", "ip4:icmp") | ||
conn6 := createListener("IPv6", "ip6:ipv6-icmp") | ||
|
||
unprivilegedPossible := false | ||
l := &icmpLoop{ | ||
conn4: conn4, | ||
conn6: conn6, | ||
recv: make(chan packet, 16), | ||
requests: map[requestID]*requestContext{}, | ||
} | ||
|
||
if conn4 != nil { | ||
go l.runICMPRecv(conn4, protocolICMP) | ||
if l.conn4 == nil && l.conn6 == nil { | ||
switch runtime.GOOS { | ||
case "linux", "darwin": | ||
unprivilegedPossible = true | ||
//This is non-privileged ICMP, not udp | ||
l.conn4 = createListener("Unprivileged IPv4", "udp4") | ||
l.conn6 = createListener("Unprivileged IPv6", "udp6") | ||
} | ||
} | ||
if conn6 != nil { | ||
go l.runICMPRecv(conn6, protocolIPv6ICMP) | ||
|
||
if l.conn4 != nil { | ||
go l.runICMPRecv(l.conn4, protocolICMP) | ||
} | ||
if l.conn6 != nil { | ||
go l.runICMPRecv(l.conn6, protocolIPv6ICMP) | ||
} | ||
|
||
if l.conn4 == nil && l.conn6 == nil { | ||
if unprivilegedPossible { | ||
var buffer bytes.Buffer | ||
path, _ := os.Executable() | ||
buffer.WriteString("You can run without root by setting cap_net_raw:\n sudo setcap cap_net_raw+eip ") | ||
buffer.WriteString(path + " \n") | ||
buffer.WriteString("Your system allows the use of unprivileged ping by setting net.ipv4.ping_group_range \n sysctl -w net.ipv4.ping_group_range='<min-uid> <max-uid>' ") | ||
return nil, noPingCapabilityError(buffer.String()) | ||
} | ||
return nil, noPingCapabilityError("You must provide the appropriate permissions to this executable") | ||
} | ||
|
||
return l, nil | ||
|
@@ -124,10 +150,10 @@ func (l *icmpLoop) checkNetworkMode(mode string) error { | |
} | ||
|
||
if ip4 && l.conn4 == nil { | ||
return errors.New("failed to initiate IPv4 support") | ||
return errors.New("failed to initiate IPv4 support. Check log details for permission configuration") | ||
} | ||
if ip6 && l.conn6 == nil { | ||
return errors.New("failed to initiate IPv6 support") | ||
return errors.New("failed to initiate IPv6 support. Check log details for permission configuration") | ||
} | ||
|
||
return nil | ||
|
@@ -202,6 +228,7 @@ func (l *icmpLoop) ping( | |
timeout time.Duration, | ||
interval time.Duration, | ||
) (time.Duration, int, error) { | ||
debugf("INVOKE PING") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. leftover? |
||
|
||
var err error | ||
toTimer := time.NewTimer(timeout) | ||
|
@@ -272,6 +299,8 @@ func (l *icmpLoop) ping( | |
if !success { | ||
return 0, requests, timeoutError{} | ||
} | ||
|
||
debugf("PING DONE %v", rtt) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. leftover? |
||
return rtt, requests, nil | ||
} | ||
|
||
|
@@ -344,7 +373,6 @@ func createListener(name, network string) *icmp.PacketConn { | |
// true, even if error value itself is `nil`. Checking for conn suppresses | ||
// misleading log message. | ||
if conn == nil && err != nil { | ||
logp.Info("%v ICMP not supported: %v", name, err) | ||
return nil | ||
} | ||
return conn | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
import os | ||
import unittest | ||
import platform | ||
import socket | ||
import sys | ||
from heartbeat import BaseTest | ||
from elasticsearch import Elasticsearch | ||
from beat.beat import INTEGRATION_TESTS | ||
import nose.tools | ||
import logging | ||
import subprocess | ||
import time | ||
|
||
|
||
class Test(BaseTest): | ||
def test_base(self): | ||
""" | ||
Basic test with icmp root non privilege ICMP test. | ||
|
||
""" | ||
|
||
config = { | ||
"monitors": [ | ||
{ | ||
"type": "icmp", | ||
"schedule": "*/5 * * * * * *", | ||
"hosts": ["127.0.0.1"], | ||
} | ||
] | ||
} | ||
|
||
self.render_config_template( | ||
path=os.path.abspath(self.working_dir) + "/log/*", | ||
**config | ||
) | ||
|
||
proc = self.start_beat() | ||
|
||
def has_started_message(): return self.log_contains("ICMP loop successfully initialized") | ||
|
||
def has_failed_message(): return self.log_contains("Failed to initialize ICMP loop") | ||
|
||
# We don't know if the system tests are running is configured to support or not support ping, but we can at least check that the ICMP loop | ||
# was initiated. In the future we should start up VMs with the correct perms configured and be more specific. In addition to that | ||
# we should run pings on those machines and make sure they work. | ||
self.wait_until(lambda: has_started_message() or has_failed_message(), 30) | ||
|
||
if has_failed_message(): | ||
proc.check_kill_and_wait(1) | ||
else: | ||
# Check that documents are moving through | ||
self.wait_until(lambda: self.output_has(lines=1)) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not checking for the error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there's much to do with the error here. We're already reporting an error, so the worst case is we can't print the path. WDYT the behavior here should be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did not check the code when extacly we would get an error. Was thinking to have the error as part of the message below if there is one. But I'm find with what you have now. If the path is empty, it will just print an empty line.