ICMP type getting overwritten.
Closed this issue · 6 comments
I'm continuing to replicate conntrack(1) with your library and have hit another issue regarding ICMP type/code values. What seems to be happening is that inside extractTuple(), while it gets called twice for the two IP directions, in both cases it contains an ICMP protocol Tuple which gets decoded. The first (for a ping) has a type of 8, then the second has a type of 0, which overwrites the 8. My understanding of ICMP and how conntrack handles it is a little light, but the linux header files indicates that it makes no sense to have both a Orig/Repl for these fields. So I suspect the correct solution is to only update them when 'dir' is 0.
It seems to me that there should not even be a second embedded protocol tuple for ICMP.
This patch replicates the output from conntrack(1).
diff --git a/attribute.go b/attribute.go
index 15d4841..15effef 100644
--- a/attribute.go
+++ b/attribute.go
@@ -224,17 +224,29 @@ func extractProtocolTuple(conn Conn, dir int, data []byte) error {
eleType := map[int]ConnAttrType{dirOrig: AttrOrigPortDst, dirReply: AttrReplPortDst, dirMaster: AttrMasterPortDst}[dir]
conn[eleType] = attr.Data
case ctaProtoIcmpID:
- conn[AttrIcmpID] = attr.Data
+ if dir == 0 {
+ conn[AttrIcmpID] = attr.Data
+ }
case ctaProtoIcmpType:
- conn[AttrIcmpType] = attr.Data
+ if dir == 0 {
+ conn[AttrIcmpType] = attr.Data
+ }
case ctaProtoIcmpCode:
- conn[AttrIcmpCode] = attr.Data
+ if dir == 0 {
+ conn[AttrIcmpCode] = attr.Data
+ }
case ctaProtoIcmpv6ID:
- conn[AttrIcmpID] = attr.Data
+ if dir == 0 {
+ conn[AttrIcmpID] = attr.Data
+ }
case ctaProtoIcmpv6Type:
- conn[AttrIcmpType] = attr.Data
+ if dir == 0 {
+ conn[AttrIcmpType] = attr.Data
+ }
case ctaProtoIcmpv6Code:
- conn[AttrIcmpCode] = attr.Data
+ if dir == 0 {
+ conn[AttrIcmpCode] = attr.Data
+ }
default:
fmt.Printf("Tuple %d is not yet implemented: %v\n", attr.Type, attr.Data)
}
Thanks for the input.
How do you catch these pings? So far I did not notice, that attributes like AttrIcmpType are contained more than once. So I'm very interested to reproduce this.
I'm doing a clone of the conntrack(1) tool, so I'm looking at it's output as I look at mine :-).
Actually following the conntrack.c code is rather hard, so I don't know if they are explicitly doing this.
Since I'm printing each event, I can follow what I'm receiving and then decoding.
Can you describe in more detail, how you noticed the problem?
I'm still trying to reproduce the issue for debugging, but always get only one ICMP element from the kernel. Which kernel version are you using? Can you preproduce it on different kernel versions?
I'm using 'Linux version 4.15.0-55-generic (buildd@lcy01-amd64-029) (gcc version 7.4.0 (Ubuntu 7.4.0-1ubuntu1~18.04.1)) #60-Ubuntu SMP Tue Jul 2 18:22:20 UTC 2019' in a VM for testing.
If I ping localhost, I get two events, which my patched code maps to
1564446895.290168 [NEW] icmp 1 30 src=127.0.0.1 dst=127.0.0.1 type=8 code=0 id=11877 [UNREPLIED] id=1750613504
1564446895.290884 [UPDATE] icmp 1 30 src=127.0.0.1 dst=127.0.0.1 type=8 code=0 id=11877 id=1750613504
Using the unix conntrack command, I get
sudo conntrack -E
[NEW] icmp 1 30 src=127.0.0.1 dst=127.0.0.1 type=8 code=0 id=12042 [UNREPLIED] src=127.0.0.1 dst=127.0.0.1 type=0 code=0 id=12042
[UPDATE] icmp 1 30 src=127.0.0.1 dst=127.0.0.1 type=8 code=0 id=12042 src=127.0.0.1 dst=127.0.0.1 type=0 code=0 id=12042
If I reverse out my patch, my code produces
1564447166.465628 [NEW] icmp 1 30 src=127.0.0.1 dst=127.0.0.1 type=0 code=0 id=12549 [UNREPLIED] id=1110428800
1564447166.465969 [UPDATE] icmp 1 30 src=127.0.0.1 dst=127.0.0.1 type=0 code=0 id=12549 id=1110428800
Putting in instrumentation of you extractProtooclTuple, for a 'ping localhost'
extractProtocolTuple dir = 0
attr.Type = 1 attr.Data = [1]
attr.Type = 4 attr.Data = [55 111]
attr.Type = 5 attr.Data = [8]
attr.Type = 6 attr.Data = [0]
extractProtocolTuple dir = 1
attr.Type = 1 attr.Data = [1]
attr.Type = 4 attr.Data = [55 111]
attr.Type = 5 attr.Data = [0]
attr.Type = 6 attr.Data = [0]
1564447502.105415 [NEW] icmp 1 30 src=127.0.0.1 dst=127.0.0.1 type=0 code=0 id=14191 [UNREPLIED] id=2453676672
extractProtocolTuple dir = 0
attr.Type = 1 attr.Data = [1]
attr.Type = 4 attr.Data = [55 111]
attr.Type = 5 attr.Data = [8]
attr.Type = 6 attr.Data = [0]
extractProtocolTuple dir = 1
attr.Type = 1 attr.Data = [1]
attr.Type = 4 attr.Data = [55 111]
attr.Type = 5 attr.Data = [0]
attr.Type = 6 attr.Data = [0]
1564447502.106161 [UPDATE] icmp 1 30 src=127.0.0.1 dst=127.0.0.1 type=0 code=0 id=14191 id=2453676672
So for each ICMP packet, we have data for both directions, but the ctaProtoIcmpType overwrites the single location with a 0 value the second call.
The data structures have src and dst for IP/port, but not the Icmp variables. I'm not sure if the kernel is bing lazy and putting the ICMP values in both ctaTupleOrig and ctaTupleReply, or if it actually means something.
thank you very much for the detailed investigation!
And yes, I can confirm your report.
As you might have noticed, I've started to improve the API of this library - see #10.
In the future [1] there will be an IP tuple for origin and one for reply, which contain this data in separate structs.
[1] https://github.com/florianl/go-conntrack/pull/11/files#diff-e871404fd69cab40521d6b2178847cf7R64
As the new API has been merged into master some time ago, I will close this issue.
Please don't hesitate to reopen this issue or create a new one, if I can help you further on this topic.