From fcb7c47bd9da36790c7d2cdaf35b938f025eab85 Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Tue, 15 Sep 2015 10:51:56 +0000 Subject: [PATCH 1/2] Always include endpoint nodes brought in by procspy, even if they get merged with conntracked endpoints. --- probe/endpoint/reporter.go | 44 ++++++++++++++++++++++---------------- render/mapping.go | 12 +++++++---- report/topology.go | 2 +- 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/probe/endpoint/reporter.go b/probe/endpoint/reporter.go index 0f58b41cb..b95e40934 100644 --- a/probe/endpoint/reporter.go +++ b/probe/endpoint/reporter.go @@ -17,6 +17,7 @@ const ( Addr = "addr" // typically IPv4 Port = "port" Conntracked = "conntracked" + Procspied = "procspied" ) // Reporter generates Reports containing the Endpoint topology. @@ -95,26 +96,31 @@ func (r *Reporter) Report() (report.Report, error) { hostNodeID := report.MakeHostNodeID(r.hostID) rpt := report.MakeReport() - conns, err := procspy.Connections(r.includeProcesses) - if err != nil { - return rpt, err - } - for conn := conns.Next(); conn != nil; conn = conns.Next() { - var ( - localPort = conn.LocalPort - remotePort = conn.RemotePort - localAddr = conn.LocalAddress.String() - remoteAddr = conn.RemoteAddress.String() - ) - extraNodeInfo := report.MakeNode() - if conn.Proc.PID > 0 { - extraNodeInfo = extraNodeInfo.WithMetadata(report.Metadata{ - process.PID: strconv.FormatUint(uint64(conn.Proc.PID), 10), - report.HostNodeID: hostNodeID, - }) + { + conns, err := procspy.Connections(r.includeProcesses) + if err != nil { + return rpt, err + } + commonNodeInfo := report.MakeNode().WithMetadata(report.Metadata{ + Procspied: "true", + }) + for conn := conns.Next(); conn != nil; conn = conns.Next() { + var ( + localPort = conn.LocalPort + remotePort = conn.RemotePort + localAddr = conn.LocalAddress.String() + remoteAddr = conn.RemoteAddress.String() + ) + extraNodeInfo := commonNodeInfo.Copy() + if conn.Proc.PID > 0 { + extraNodeInfo = extraNodeInfo.WithMetadata(report.Metadata{ + process.PID: strconv.FormatUint(uint64(conn.Proc.PID), 10), + report.HostNodeID: hostNodeID, + }) + } + r.addConnection(&rpt, localAddr, remoteAddr, localPort, remotePort, &extraNodeInfo, &commonNodeInfo) } - r.addConnection(&rpt, localAddr, remoteAddr, localPort, remotePort, &extraNodeInfo, nil) } if r.conntracker != nil { @@ -136,7 +142,7 @@ func (r *Reporter) Report() (report.Report, error) { r.natmapper.applyNAT(rpt, r.hostID) } - return rpt, err + return rpt, nil } func (r *Reporter) addConnection(rpt *report.Report, localAddr, remoteAddr string, localPort, remotePort uint16, extraLocalNode, extraRemoteNode *report.Node) { diff --git a/render/mapping.go b/render/mapping.go index 626bd6d21..66c88fb2c 100644 --- a/render/mapping.go +++ b/render/mapping.go @@ -48,10 +48,14 @@ func MapEndpointIdentity(m RenderableNode, local report.Networks) RenderableNode } // We need to filter out short lived connections from this view, - // if they don't also have a pid; see #447 + // if they don't also have a pid; see #447. Note if they + // we're introduced by proc spy then they are guaranteed to + // have a pid on the other end of the adjacency, so we include them + // no matter what. pid, pidOK := m.Metadata[process.PID] _, conntracked := m.Metadata[endpoint.Conntracked] - if !pidOK && conntracked { + _, procspied := m.Metadata[endpoint.Procspied] + if !procspied && !pidOK && conntracked { return RenderableNodes{} } @@ -64,8 +68,8 @@ func MapEndpointIdentity(m RenderableNode, local report.Networks) RenderableNode } // We are a 'client' pseudo node if the port is in the ephemeral port range. - // Linux uses 32768 to 61000. - if p, err := strconv.Atoi(port); err == nil && len(m.Adjacency) > 0 && p >= 32768 && p < 61000 { + // Linux uses 32768 to 61000, IANA suggests 49152 to 65535. + if p, err := strconv.Atoi(port); err == nil && len(m.Adjacency) > 0 && p >= 32768 && p < 65535 { // We only exist if there is something in our adjacency // Generate a single pseudo node for every (client ip, server ip, server port) dstNodeID := m.Adjacency[0] diff --git a/report/topology.go b/report/topology.go index 0b4e75b00..55e817e95 100644 --- a/report/topology.go +++ b/report/topology.go @@ -102,7 +102,7 @@ func MakeNodeWith(m map[string]string) Node { // WithMetadata returns a fresh copy of n, with Metadata set to m func (n Node) WithMetadata(m map[string]string) Node { result := n.Copy() - result.Metadata = m + result.Metadata = result.Metadata.Merge(m) return result } From 8b424df2be7b34674a9762d2956dd6f578cfabb0 Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Wed, 16 Sep 2015 09:08:00 +0000 Subject: [PATCH 2/2] Review feedback. --- render/mapping.go | 11 ++------ render/mapping_test.go | 10 +++---- report/topology.go | 2 +- test/report_fixture.go | 63 +++++++++++++++++++++++++----------------- 4 files changed, 46 insertions(+), 40 deletions(-) diff --git a/render/mapping.go b/render/mapping.go index 66c88fb2c..1c7e50ff5 100644 --- a/render/mapping.go +++ b/render/mapping.go @@ -47,15 +47,9 @@ func MapEndpointIdentity(m RenderableNode, local report.Networks) RenderableNode return RenderableNodes{} } - // We need to filter out short lived connections from this view, - // if they don't also have a pid; see #447. Note if they - // we're introduced by proc spy then they are guaranteed to - // have a pid on the other end of the adjacency, so we include them - // no matter what. - pid, pidOK := m.Metadata[process.PID] - _, conntracked := m.Metadata[endpoint.Conntracked] + // We only show nodes found through procspy in this view. _, procspied := m.Metadata[endpoint.Procspied] - if !procspied && !pidOK && conntracked { + if !procspied { return RenderableNodes{} } @@ -93,6 +87,7 @@ func MapEndpointIdentity(m RenderableNode, local report.Networks) RenderableNode rank = major ) + pid, pidOK := m.Metadata[process.PID] if pidOK { minor = fmt.Sprintf("%s (%s)", minor, pid) } diff --git a/render/mapping_test.go b/render/mapping_test.go index ef17dcf2d..eaf1a299d 100644 --- a/render/mapping_test.go +++ b/render/mapping_test.go @@ -18,11 +18,11 @@ func nrn(nmd report.Node) render.RenderableNode { func TestMapEndpointIdentity(t *testing.T) { for _, input := range []testcase{ {nrn(report.MakeNode()), false}, - {nrn(report.MakeNodeWith(map[string]string{endpoint.Addr: "1.2.3.4"})), false}, - {nrn(report.MakeNodeWith(map[string]string{endpoint.Port: "1234"})), false}, - {nrn(report.MakeNodeWith(map[string]string{endpoint.Addr: "1.2.3.4", endpoint.Port: "1234"})), true}, - {nrn(report.MakeNodeWith(map[string]string{endpoint.Addr: "1.2.3.4", endpoint.Port: "40000"})), true}, - {nrn(report.MakeNodeWith(map[string]string{report.HostNodeID: report.MakeHostNodeID("foo"), endpoint.Addr: "10.0.0.1", endpoint.Port: "20001"})), true}, + {nrn(report.MakeNodeWith(map[string]string{endpoint.Addr: "1.2.3.4", endpoint.Procspied: "true"})), false}, + {nrn(report.MakeNodeWith(map[string]string{endpoint.Port: "1234", endpoint.Procspied: "true"})), false}, + {nrn(report.MakeNodeWith(map[string]string{endpoint.Addr: "1.2.3.4", endpoint.Port: "1234", endpoint.Procspied: "true"})), true}, + {nrn(report.MakeNodeWith(map[string]string{endpoint.Addr: "1.2.3.4", endpoint.Port: "40000", endpoint.Procspied: "true"})), true}, + {nrn(report.MakeNodeWith(map[string]string{report.HostNodeID: report.MakeHostNodeID("foo"), endpoint.Addr: "10.0.0.1", endpoint.Port: "20001", endpoint.Procspied: "true"})), true}, } { testMap(t, render.MapEndpointIdentity, input) } diff --git a/report/topology.go b/report/topology.go index 55e817e95..01c7e7712 100644 --- a/report/topology.go +++ b/report/topology.go @@ -99,7 +99,7 @@ func MakeNodeWith(m map[string]string) Node { return MakeNode().WithMetadata(m) } -// WithMetadata returns a fresh copy of n, with Metadata set to m +// WithMetadata returns a fresh copy of n, with Metadata m merged in. func (n Node) WithMetadata(m map[string]string) Node { result := n.Copy() result.Metadata = result.Metadata.Merge(m) diff --git a/test/report_fixture.go b/test/report_fixture.go index 65cf19e5d..986643ed5 100644 --- a/test/report_fixture.go +++ b/test/report_fixture.go @@ -48,6 +48,8 @@ var ( ServerComm = "apache" NonContainerComm = "bash" + True = "true" + ClientHostNodeID = report.MakeHostNodeID(ClientHostID) ServerHostNodeID = report.MakeHostNodeID(ServerHostID) @@ -92,75 +94,84 @@ var ( // care to test into the fixture. Just be sure to include the bits // that the mapping funcs extract :) Client54001NodeID: report.MakeNode().WithMetadata(map[string]string{ - endpoint.Addr: ClientIP, - endpoint.Port: ClientPort54001, - process.PID: Client1PID, - report.HostNodeID: ClientHostNodeID, + endpoint.Addr: ClientIP, + endpoint.Port: ClientPort54001, + process.PID: Client1PID, + report.HostNodeID: ClientHostNodeID, + endpoint.Procspied: True, }).WithEdge(Server80NodeID, report.EdgeMetadata{ EgressPacketCount: newu64(10), EgressByteCount: newu64(100), }), Client54002NodeID: report.MakeNode().WithMetadata(map[string]string{ - endpoint.Addr: ClientIP, - endpoint.Port: ClientPort54002, - process.PID: Client2PID, - report.HostNodeID: ClientHostNodeID, + endpoint.Addr: ClientIP, + endpoint.Port: ClientPort54002, + process.PID: Client2PID, + report.HostNodeID: ClientHostNodeID, + endpoint.Procspied: True, }).WithEdge(Server80NodeID, report.EdgeMetadata{ EgressPacketCount: newu64(20), EgressByteCount: newu64(200), }), Server80NodeID: report.MakeNode().WithMetadata(map[string]string{ - endpoint.Addr: ServerIP, - endpoint.Port: ServerPort, - process.PID: ServerPID, - report.HostNodeID: ServerHostNodeID, + endpoint.Addr: ServerIP, + endpoint.Port: ServerPort, + process.PID: ServerPID, + report.HostNodeID: ServerHostNodeID, + endpoint.Procspied: True, }), NonContainerNodeID: report.MakeNode().WithMetadata(map[string]string{ - endpoint.Addr: ServerIP, - endpoint.Port: NonContainerClientPort, - process.PID: NonContainerPID, - report.HostNodeID: ServerHostNodeID, + endpoint.Addr: ServerIP, + endpoint.Port: NonContainerClientPort, + process.PID: NonContainerPID, + report.HostNodeID: ServerHostNodeID, + endpoint.Procspied: True, }).WithAdjacent(GoogleEndpointNodeID), // Probe pseudo nodes UnknownClient1NodeID: report.MakeNode().WithMetadata(map[string]string{ - endpoint.Addr: UnknownClient1IP, - endpoint.Port: UnknownClient1Port, + endpoint.Addr: UnknownClient1IP, + endpoint.Port: UnknownClient1Port, + endpoint.Procspied: True, }).WithEdge(Server80NodeID, report.EdgeMetadata{ EgressPacketCount: newu64(30), EgressByteCount: newu64(300), }), UnknownClient2NodeID: report.MakeNode().WithMetadata(map[string]string{ - endpoint.Addr: UnknownClient2IP, - endpoint.Port: UnknownClient2Port, + endpoint.Addr: UnknownClient2IP, + endpoint.Port: UnknownClient2Port, + endpoint.Procspied: True, }).WithEdge(Server80NodeID, report.EdgeMetadata{ EgressPacketCount: newu64(40), EgressByteCount: newu64(400), }), UnknownClient3NodeID: report.MakeNode().WithMetadata(map[string]string{ - endpoint.Addr: UnknownClient3IP, - endpoint.Port: UnknownClient3Port, + endpoint.Addr: UnknownClient3IP, + endpoint.Port: UnknownClient3Port, + endpoint.Procspied: True, }).WithEdge(Server80NodeID, report.EdgeMetadata{ EgressPacketCount: newu64(50), EgressByteCount: newu64(500), }), RandomClientNodeID: report.MakeNode().WithMetadata(map[string]string{ - endpoint.Addr: RandomClientIP, - endpoint.Port: RandomClientPort, + endpoint.Addr: RandomClientIP, + endpoint.Port: RandomClientPort, + endpoint.Procspied: True, }).WithEdge(Server80NodeID, report.EdgeMetadata{ EgressPacketCount: newu64(60), EgressByteCount: newu64(600), }), GoogleEndpointNodeID: report.MakeNode().WithMetadata(map[string]string{ - endpoint.Addr: GoogleIP, - endpoint.Port: GooglePort, + endpoint.Addr: GoogleIP, + endpoint.Port: GooglePort, + endpoint.Procspied: True, }), }, },