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

[Heartbeat] Rootless pings #13795

Merged
merged 14 commits into from
Sep 25, 2019
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Add support to set the document id in the json reader. {pull}5844[5844]

*Heartbeat*
- Add non-privileged icmp on linux and darwin(mac). {pull}11610[11610] {issue}11498[11498]

- Enable `add_observer_metadata` processor in default config. {pull}11394[11394]
- Record HTTP body metadata and optionally contents in `http.response.body.*` fields. {pull}13022[13022]
Expand Down
11 changes: 3 additions & 8 deletions heartbeat/monitors/active/icmp/icmp.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,6 @@ func create(
return nil, 0, err
}

// TODO: check icmp is support by OS + check we've
// got required credentials (implementation uses RAW socket, requires root +
// not supported on all OSes)
// TODO: replace icmp package base reader/sender using raw sockets with
// OS specific solution

ipVersion := config.Mode.Network()
if len(config.Hosts) > 0 && ipVersion == "" {
err := fmt.Errorf("pinging hosts requires ipv4 or ipv6 mode enabled")
Expand All @@ -61,13 +55,14 @@ func create(

var loopErr error
loopInit.Do(func() {
debugf("initialize icmp handler")
debugf("initializing ICMP loop")
loop, loopErr = newICMPLoop()
})
if loopErr != nil {
debugf("Failed to initialize ICMP loop %v", loopErr)
logp.Warn("Failed to initialize ICMP loop %v", loopErr)
return nil, 0, loopErr
}
debugf("ICMP loop successfully initialized")

if err := loop.checkNetworkMode(ipVersion); err != nil {
return nil, 0, err
Expand Down
48 changes: 38 additions & 10 deletions heartbeat/monitors/active/icmp/loop.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -85,26 +85,52 @@ var (
loop *icmpLoop
)

func noPingCapabilityError(message string) error {
return errors.New(fmt.Sprintf("Insufficient privileges to perform ICMP ping. %s", message))
andrewvc marked this conversation as resolved.
Show resolved Hide resolved
}

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()
Copy link
Contributor

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?

Copy link
Contributor Author

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?

Copy link
Contributor

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.

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
Expand All @@ -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
Expand Down Expand Up @@ -202,6 +228,7 @@ func (l *icmpLoop) ping(
timeout time.Duration,
interval time.Duration,
) (time.Duration, int, error) {
debugf("INVOKE PING")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leftover?


var err error
toTimer := time.NewTimer(timeout)
Expand Down Expand Up @@ -272,6 +299,8 @@ func (l *icmpLoop) ping(
if !success {
return 0, requests, timeoutError{}
}

debugf("PING DONE %v", rtt)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leftover?

return rtt, requests, nil
}

Expand Down Expand Up @@ -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
Expand Down
52 changes: 52 additions & 0 deletions heartbeat/tests/system/test_icmp.py
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))