Nmap Development mailing list archives
Re: IPv6 packet.lua review
From: Xu Weilin <mzweilin () gmail com>
Date: Thu, 21 Jul 2011 21:26:00 +0800
On Thu, Jul 21, 2011 at 2:14 PM, David Fifield <david () bamsoftware com>wrote:
This is a quick review of the changes Weilin has made to the packet.lua library to support IPv6 packet construction. The file is in /nmap-exp/weilin/host_discovery_scripts/packet.lua. +--- Build an Ethernet frame. +-- @param mac_dst six-byte string of the destination MAC address. +-- @param mac_src six-byte string of the source MAC address. +-- @param packet string of the payload. +-- @return frame string of the Ether frame. +function Frame:build_ether_frame(mac_dst, mac_src, packet) + self.mac_dst = mac_dst or self.mac_dst + self.mac_src = mac_src or self.mac_src + self.buf = packet or self.buf + local l3_type + if self.ip_v == 4 then + l3_type = string.char(0x08, 0x00) + elseif self.ip_v == 6 then + l3_type = string.char(0x86, 0xdd) + else + return nil, "Unknown packet." + end + self.frame_buf = self.mac_dst..self.mac_src..l3_type..self.buf +end How does self.ip_v get set? Why should IPv4 aznd IPv6 be given special status here? Shouldn't the upper-layer protocol be a parameter?
Sorry, I neglected the other layer3 protocols.
function Packet:new(packet, packet_len, force_continue) local o = setmetatable({}, {__index = Packet}) + if not packet then + return o + end o.buf = packet Why is this handling for packet == nil now necessary?
We need a Packet object before building the packet. The previous code is just designed for parsing packets.
+ self.buf = + set_u32("....",0,ver_tc_fl) .. + set_u16("..",0,#(self.exheader or "")+#(self.l4_packet or "")) ..--string.char(0x00,0x10) .. --payload length Using set_u32 and set_u16 in this way is confusing. If necessary, define another function that just returns the string you want, instead of forcing the caller to provide a dummy string of the proper length.
Agree.
+function Packet:build_invalid_extension_header(exheader_type) + local ex_invalid_opt = string.char(0x80,0x01,0xfe,0x18,0xfe,0x18,0xfe,0x18,0x0,0x0,0x0,0x0,0x0,0x0) +function Packet:build_router_advert(mac_src,prefix,prefix_len,valid_time,preferred_time) + self.ip6_src = mac_to_lladdr(mac_src) These functions are too specialized to be in the packet.lua library. Special parameters like these belong in a script.
Reasonable.
+function inet6_pton_simple(str) +function ipv6tobin(str) These functions are hard to understand for what they do. They don't handle addresses of the form "::ffff:a.b.c.d". There is not enough error checking; try nonsense addresses like "1::2::3" and "1:2:3" and "1:2:3:4:5:6:7:8:9". Rather than trying to debug all these cases; perhaps it would be better to provide a wrapper around getaddrinfo?
Toni has a better implementation of ip6tobin() which is nearer to our goal.
+function mactobin(str) + if not str then + return mactobin("00:00:00:00:00:00") + end +function toipv6(raw_ipv6_addr) + if not raw_ipv6_addr then + return "?::?" + end +function mac_to_lladdr(mac) + if not mac then + return "?::?" + end I would rather see an error about a nil variable than silently insert default values like this.
The style is inherited from the original packet.lua. So we have to change other functions at the same time, such as toip().
+--- Pare an IPv6 extension header. Just jump over it at the moment. +-- @param force_continue Ignored. +-- @return Whether the parsing succeeded. +function Packet:ipv6_ext_header_parse(force_continue) + local ext_hdr_len = self.u8(self.ip6_data_offset + 1) + ext_hdr_len = ext_hdr_len*8 + 8 + self.ip6_data_offset = self.ip6_data_offset + ext_hdr_len + self.ip6_nhdr = self.u8(self.ip6_data_offset) +end I think we must be more careful than this with extension headers. All the currently defined extension headers have the nh value in the first byte, but I hven't seen a guarantee that it will always be so. Also, the way the function is called, the code will attempt to skip over any nh value that isn't IPPROTO_TCP, IPPROTO_UDP, and IPPROTO_ICMPV6. What about other things like IPPROTO_SCTP? That will produce nonsense if parsed this way. Instead of looping until you get a known upper-layer protocol (assuming everything else is an extension header), loop as long as you have a known extension header (and assume everything else is an upper-layer protocol). See ipv6_get_data_primitive in netutil.cc.
Thanks for your advice. -- Regards Xu Weilin 许伟林 _______________________________________________ Sent through the nmap-dev mailing list http://cgi.insecure.org/mailman/listinfo/nmap-dev Archived at http://seclists.org/nmap-dev/
Current thread:
- IPv6 packet.lua review David Fifield (Jul 20)
- Re: IPv6 packet.lua review Xu Weilin (Jul 21)
- Re: IPv6 packet.lua review Xu Weilin (Jul 22)
- Re: IPv6 packet.lua review Xu Weilin (Jul 21)