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.

Sign up for GitHub

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

Jump to bottom

fix(renderer): add more common protocols to protoname renderer #341

Merged
merged 6 commits into from
Aug 13, 2024
Merged

fix(renderer): add more common protocols to protoname renderer #341

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
33ae373
fix(renderer): add more common protocols to protoname renderer, add u…
DerInti Jul 29, 2024
38fdf10
Update producer/proto/render.go
DerInti Jul 30, 2024
c3242eb
Update producer/proto/render.go with complete ip protolist
DerInti Jul 30, 2024
223d4d2
Update producer/proto/render.go
lspgn Jul 31, 2024
95fb1c7
Merge branch 'netsampler:main' into protonames-rendering
DerInti Jul 31, 2024
7203564
fix(protoname): follow iana naming scheme for ipv6-icmp in protorenderer
DerInti Jul 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
165 changes: 162 additions & 3 deletions producer/proto/render.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,152 @@ var (
0x86dd: "IPv6",
}
protoName = map[uint32]string{
0: "HOPOPT",
1: "ICMP",
2: "IGMP",
3: "GGP",
4: "IPv4",
5: "ST",
6: "TCP",
7: "CBT",
8: "EGP",
9: "IGP",
10: "BBN-RCC-MON",
11: "NVP-II",
12: "PUP",
13: "ARGUS",
14: "EMCON",
15: "XNET",
16: "CHAOS",
17: "UDP",
58: "ICMPv6",
18: "MUX",
19: "DCN-MEAS",
20: "HMP",
21: "PRM",
22: "XNS-IDP",
23: "TRUNK-1",
24: "TRUNK-2",
25: "LEAF-1",
26: "LEAF-2",
27: "RDP",
28: "IRTP",
29: "ISO-TP4",
30: "NETBLT",
31: "MFE-NSP",
32: "MERIT-INP",
33: "DCCP",
34: "3PC",
35: "IDPR",
36: "XTP",
37: "DDP",
38: "IDPR-CMTP",
39: "TP++",
40: "IL",
41: "IPv6",
42: "SDRP",
43: "IPv6-Route",
Copy link
Member

Choose a reason for hiding this comment

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

IPv6-EH-Routing maybe?

Copy link
Contributor

@marioschaefer marioschaefer Jul 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

IPv6-Route is the official IANA name, same as with ICMPv6 vs. IPv6-ICMP - we can deviate from the standard, as you like.
But in my opinion, we should stick to the standard.

--- update ---
commit was not planned, i thought i deleted my change request... will change again, according to your decision.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1 to sticking to official IANA protocol names

Copy link
Member

Choose a reason for hiding this comment

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

Good point

44: "IPv6-Frag",
45: "IDRP",
46: "RSVP",
47: "GRE",
48: "DSR",
49: "BNA",
50: "ESP",
51: "AH",
52: "I-NLSP",
53: "SWIPE",
54: "NARP",
55: "Min-IPv4",
56: "TLSP",
57: "SKIP",
58: "IPv6-ICMP",
Copy link
Member

Choose a reason for hiding this comment

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

Would rather keep ICMPv6 since it's already being used

Copy link
Contributor

@marioschaefer marioschaefer Jul 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

see comment above

Copy link
Member

@lspgn lspgn Aug 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

While I agree it's IANA, I would prefer not breaking existing implementations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my 2 cents:

  • I dont think we can mitigate the absence of information without some kind of breakage of existing implementations. As the Protocol Name is the only hint to the protocol field of the actually sampled packet (when utilizing JSON output) I'd prefer IANA names.
  • imho a viable alternative would be to add a field to the json output (proto_num?) with the int32 protonum - which would allow a clear mapping to official protocol names (for example in an enricher) and wont break existing implementations relying on the strings
    I'd keep all other protocols in protoName with their official names - with "ICMPv6" as only deviation.
    @lspgn @marioschaefer wdyt

Copy link
Contributor

Choose a reason for hiding this comment

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

@DerInti
point 1:
i'm on the complete same page

point 2:
I also thought about that and its probably the cleanest solution, bcs you have the number and the name.
But this will also break the existing implementation, since we will stick to the IANA-Names and the existing implementation doesn't use them.
on the other hand: how much longer do we want to carry the wrong name around with us. sooner or later it will be changed. the longer you wait, the more it hurts. that's why i'm a big fan of "sooner rather than later".

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
58: "IPv6-ICMP",
58: "ICMPv6",

59: "IPv6-NoNxt",
60: "IPv6-Opts",
61: "any-host-internal-protocol",
62: "CFTP",
63: "any-local-network",
64: "SAT-EXPAK",
65: "KRYPTOLAN",
66: "RVD",
67: "IPPC",
68: "any-distributed-file-system",
69: "SAT-MON",
70: "VISA",
71: "IPCV",
72: "CPNX",
73: "CPHB",
74: "WSN",
75: "PVP",
76: "BR-SAT-MON",
77: "SUN-ND",
78: "WB-MON",
79: "WB-EXPAK",
80: "ISO-IP",
81: "VMTP",
82: "SECURE-VMTP",
83: "VINES",
84: "IPTM",
85: "NSFNET-IGP",
86: "DGP",
87: "TCF",
88: "EIGRP",
89: "OSPFIGP",
90: "Sprite-RPC",
91: "LARP",
92: "MTP",
93: "AX.25",
94: "IPIP",
95: "MICP",
96: "SCC-SP",
97: "ETHERIP",
98: "ENCAP",
99: "any-private-encryption-scheme",
100: "GMTP",
101: "IFMP",
102: "PNNI",
103: "PIM",
104: "ARIS",
105: "SCPS",
106: "QNX",
107: "A/N",
108: "IPComp",
109: "SNP",
110: "Compaq-Peer",
111: "IPX-in-IP",
112: "VRRP",
113: "PGM",
114: "any-0-hop-protocol",
115: "L2TP",
116: "DDX",
117: "IATP",
118: "STP",
119: "SRP",
120: "UTI",
121: "SMP",
122: "SM",
123: "PTP",
124: "ISIS over IPv4",
125: "FIRE",
126: "CRTP",
127: "CRUDP",
128: "SSCOPMCE",
129: "IPLT",
130: "SPS",
131: "PIPE",
132: "SCTP",
133: "FC",
134: "RSVP-E2E-IGNORE",
135: "Mobility Header",
136: "UDPLite",
137: "MPLS-in-IP",
138: "manet",
139: "HIP",
140: "Shim6",
141: "WESP",
142: "ROHC",
143: "Ethernet",
144: "AGGFRAG",
145: "NSH",
}
icmpTypeName = map[uint32]string{
0: "EchoReply",
Expand Down Expand Up @@ -169,11 +310,29 @@ func EtypeRenderer(msg *ProtoProducerMessage, fieldName string, data interface{}
return "unknown"
}

func ProtoName(protoNumber uint32) string {
if (protoNumber >= 146) && (protoNumber <= 252) {
return "unassigned"
}
if (protoNumber >= 253) && (protoNumber <= 254) {
return "experimental"
}
if protoNumber == 255 {
return "reserved"
}
dataC, ok := protoName[protoNumber]
if ok {
return dataC
} else {
return "unknown"
}
}

func ProtoRenderer(msg *ProtoProducerMessage, fieldName string, data interface{}) interface{} {
if dataC, ok := data.(uint32); ok {
return protoName[dataC]
return ProtoName(dataC)
} else if dataC, ok := data.(uint64); ok {
return protoName[uint32(dataC)]
return ProtoName(uint32(dataC))
}
return "unknown"
}
Expand Down
Loading