the-tcpdump-group / tcpdump

the TCPdump network dissector
https://www.tcpdump.org/
Other
2.73k stars 852 forks source link

Type Mismatch #690

Closed Bugcheckers closed 6 years ago

Bugcheckers commented 6 years ago

https://github.com/the-tcpdump-group/tcpdump/blob/master/print-hncp.c#L323

const u_char tlv, value; uint16_t type, optlen;

https://github.com/the-tcpdump-group/tcpdump/blob/master/print-hncp.c#L271

const uint8_t tlv, value; uint8_t type, optlen;

Operations on the value variable are the same in dhcpv6_print and dhcpv4_print functions, but the types are slightly different (uint8_t and u_char). u_char is not guaranteed to be an 8-bit type for all platforms, uint8_t is. Additionally, optlen has uint16_t and uint8_t types that have different length and it might lead to bug if it has to be 16 bits.

Please let me know!

Thanks!

guyharris commented 6 years ago

u_char is not guaranteed to be an 8-bit type for all platforms,

For historical reasons (namely "uint8_t didn't exist when libpcap was created"), libpcap uses u_char * as the type for the pointer to a raw packet buffer, and for "trying to support the Univac 1100/2200 series is too much work" reasons, libpcap and tcpdump assume 8-bit bytes, so it has to be an 8-bit type on all platforms on which libpcap and tcpdump work.

guyharris commented 6 years ago

Additionally, optlen has uint16_t and uint8_t types that have different length and it might lead to bug if it has to be 16 bits.

RFC 7788, "Home Networking Control Protocol", says in section 10.2.2 "DHCPv6-Data TLV" "DHCPv6 option stream: DHCPv6 options encoded as specified in [RFC3315].", and says in section 10.2.3 "DHCPv4-Data TLV" "DHCPv4 option stream: DHCPv4 options encoded as specified in [RFC2131]."

RFC 3315, "Dynamic Host Configuration Protocol for IPv6 (DHCPv6)" says, in section 22.1 "Format of DHCP Options", that the option code and option length are 2 bytes long (see the diagram).

RFC 2131, "Dynamic Host Configuration Protocol" says, in section 2 "Protocol Summary":

DHCP introduces a small change in terminology intended to clarify the meaning of one of the fields. What was the "vendor extensions" field in BOOTP has been re-named the "options" field in DHCP. Similarly, the tagged data items that were used inside the BOOTP "vendor extensions" field, which were formerly referred to as "vendor extensions," are now termed simply "options."

The BOOTP "vendor extensions" field was originally documented in RFC 1497 "BOOTP Vendor Information Extensions"; it says

  The vendor information field has been implemented as a free
  format, with extendable tagged sub-fields.  These sub-fields are
  length tagged (with exceptions; see below), allowing clients not
  implementing certain types to correctly skip fields they cannot
  interpret.  Lengths are exclusive of the tag and length octets;
  all multi-byte quantities are in network byte-order.

  Fixed Length Data

     The fixed length data are comprised of two formats.  Those that
     have no data consist of a single tag octet and are implicitly
     of one-octet length, while those that contain data consist of
     one tag octet, one length octet, and length octets of data.

That was obsoleted by RFC 1533 "DHCP Options and BOOTP Vendor Extensions", which says in section 2 "BOOTP Extension/DHCP Option Field Format"

DHCP options have the same format as the BOOTP "vendor extensions" defined in RFC 1497 [2]. Options may be fixed length or variable length. All options begin with a tag octet, which uniquely identifies the option. Fixed-length options without data consist of only a tag octet. Only options 0 and 255 are fixed length. All other options are variable-length with a length octet following the tag octet.

That was, in turn, obsoleted by RFC 2132 "DHCP Options and BOOTP Vendor Extensions", which says, in section 2 "BOOTP Extension/DHCP Option Field Format", the same thing.

So the option type and length field are 16 bits for DHCPv6, and thus for the DHCPv6-Data TLV in HNCP, and they're 8 bits for DHCPv4, and thus for the DHCPv4-Data TLV in HNCP.

Bugcheckers commented 6 years ago

For historical reasons (namely "uint8_t didn't exist when libpcap was created"), libpcap uses u_char * as the type for the pointer to a raw packet buffer, and for "trying to support the Univac 1100/2200 series is too much work" reasons, libpcap and tcpdump assume 8-bit bytes, so it has to be an 8-bit type on all platforms on which libpcap and tcpdump work.

Thank you for your description. So isn't it more consistent if u_char is now converted to uint8_t or vice versa (namely uint8_t is converted to u_char)?