This repository was archived by the owner on Jul 10, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 38
[POA-155] Capture client/server timeouts as well as Agent parsing errors #245
Open
shreys7
wants to merge
7
commits into
main
Choose a base branch
from
shrey/capture-timeouts-and-agent-parsing-errors
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
bbad830
[POA-155] Capture client/server timeouts as well as Agent parsing errors
shreys7 a810d14
[POA-155] Fix the mock module
shreys7 eba7fd8
[POA-155] Address few PR comments
shreys7 65409f6
[POA-155] Do not error out when we dont find a partial witness for th…
shreys7 405def1
[POA-155] Address akinet related PR review comments
shreys7 03b07fa
[POA-155] Fix unit tests
shreys7 d84c32c
[POA-155] Rename the client and server timeout metadata pnc structs t…
shreys7 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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 |
|---|---|---|
|
|
@@ -55,6 +55,9 @@ type tcpFlow struct { | |
| // Context for the FIRST packet that currentParser is processing. | ||
| currentParserCtx *assemblerCtxWithSeq | ||
|
|
||
| // Indicates if this flow has seen any packet | ||
| firstPacketSeen bool | ||
|
|
||
| // Data that was left unused when determining parser, awaiting for more data. | ||
| // This is a hack to flush data when the flow terminates before a parser has | ||
| // been selected since reassembled does not get invoked on stream end even if | ||
|
|
@@ -71,6 +74,7 @@ func newTCPFlow(clock clockWrapper, bidiID akinet.TCPBidiID, nf, tf gopacket.Flo | |
| bidiID: bidiID, | ||
| outChan: outChan, | ||
| factorySelector: fs, | ||
| firstPacketSeen: false, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -94,6 +98,12 @@ func (f *tcpFlow) reassembledWithIgnore(ignoreCount int, sg reassembly.ScatterGa | |
|
|
||
| printer.V(6).Infof("reassembled with %d bytes, isEnd=%v\n", bytesAvailable-ignoreCount, isEnd) | ||
|
|
||
| // since we are ending this flow, mark the firstPacketSeen as false for the current flow for the new request/response pair | ||
| // Is there a race condition here where Accept and reassembledWithIgnore will be called together for the same flow? | ||
shreys7 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if isEnd { | ||
| f.firstPacketSeen = false | ||
| } | ||
|
|
||
| if f.currentParser == nil { | ||
| // Try to create a new parser. | ||
| fact, decision, discardFront := f.factorySelector.Select(pktData, isEnd) | ||
|
|
@@ -209,6 +219,7 @@ func (f *tcpFlow) reassemblyComplete() { | |
| } | ||
| f.currentParser = nil | ||
| f.currentParserCtx = nil | ||
| f.firstPacketSeen = false | ||
| } else if f.unusedAcceptBuf.Len() > 0 { | ||
| // The flow terminated before a parser has been selected, flush any bytes | ||
| // that were buffered waiting for more data to determine parse. | ||
|
|
@@ -294,6 +305,14 @@ func (c *tcpStream) Accept(tcp *layers.TCP, _ gopacket.CaptureInfo, dir reassemb | |
| } | ||
| } | ||
|
|
||
| currFlow := c.flows[dir] | ||
| revFlow := c.flows[dir.Reverse()] | ||
|
|
||
| // Mark the firstPacketSeen as true for the current flow | ||
| if !currFlow.firstPacketSeen { | ||
| currFlow.firstPacketSeen = true | ||
| } | ||
|
|
||
| // Output some metadata for the current packet. | ||
| { | ||
| srcE, dstE := c.netFlow.Endpoints() | ||
|
|
@@ -314,6 +333,32 @@ func (c *tcpStream) Accept(tcp *layers.TCP, _ gopacket.CaptureInfo, dir reassemb | |
| } | ||
| } | ||
|
|
||
| // One of the flows initiated a connection close request | ||
| if tcp.FIN { | ||
|
Contributor
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. I do think it is important to handle RST too, or at least leave a FIXME about abnormal termination. If the server crashes hard (a kernel failure) or there is a load balancer problem directing traffic to the wrong server, then the client will get a RST back instead of a FIN. Of course, we might not be still around to see the traffic in that case. The server getting a RST is a bit more obscure. The comments here do not make it clear how we handle the FIN-ACK from the side that did not initiate the shutdown. |
||
| // Confirm with mark if we need this new variable or we can achieve the same | ||
| // using revFlow.currentParser and revFlow.currentParserCtx | ||
| if !revFlow.firstPacketSeen { | ||
shreys7 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| currFlowParser := currFlow.currentParser | ||
|
|
||
| // The current flow is the one that initiated the connection close request | ||
| // and no packets are yet arrived on the reverse flow | ||
| // this would indicate a timeout on the current side | ||
| if currFlowParser != nil { | ||
shreys7 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if currFlowParser.Name() == "HTTP/1.x Request Parser Factory" { | ||
shreys7 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| c.outChan <- currFlow.toPNT(ac.GetCaptureInfo().Timestamp, ac.GetCaptureInfo().Timestamp, akinet.ClientTimeoutMetadata{ | ||
| StreamID: uuid.UUID(currFlow.bidiID), | ||
| Seq: int(tcp.Seq), | ||
| }) | ||
| } else if currFlowParser.Name() == "HTTP/1.x Response Parser Factory" { | ||
| c.outChan <- currFlow.toPNT(ac.GetCaptureInfo().Timestamp, ac.GetCaptureInfo().Timestamp, akinet.ServerTimeoutMetadata{ | ||
| StreamID: uuid.UUID(currFlow.bidiID), | ||
| Seq: int(tcp.Seq), | ||
| }) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Accept everything, even if the packet might violate the TCP state machine | ||
| // and get rejected by the client or server's TCP stack. We do this because we | ||
| // are interested in detecting all dataflows, not just ones from valid TCP | ||
|
|
||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.