NAI in the host id may be incorrectly coded

Please check this Artturi.

From: Stefan Götz <email address hidden>
Subject: [hipl-dev] Re: [Branch ~hipl-core/hipl/trunk] Rev 5263: Hooked in functions for compressing/decompressing host identites in every function that receives ...
Date: 9. Dezember 2010 16:05:59 MEZ
To: <email address hidden>

> /* copy hostname to hadb entry if local copy is empty */
> if (strlen((char *) (ctx->hadb_entry->peer_hostname)) == 0) {
> memcpy(ctx->hadb_entry->peer_hostname,
> - hip_get_param_host_id_hostname(peer_host_id),
> + hip_get_param_host_id_hostname(&peer_host_id),
> HIP_HOST_ID_HOSTNAME_LEN_MAX - 1);

I looked this up in rfc5201, Section 5.2.8 or rather RFC 1035 Section 3.1.

1) on the wire, the host name is supposed to be 0-terminated but otherwise not
in a regular string format (the wire format of the HOST_ID parameter follows the
somewhat peculiar DNS resource record encoding convention that happens to make
strings 0-terminated). It might make sense to check whether HIPL treats this
right - I haven't. For example, the above memcpy() with the number of copied
bytes being HIP_HOST_ID_HOSTNAME_LEN_MAX - 1 is not kosher as there are cases
where it does not ensure the 0-termination of the string, I think.

2) HIP_HOST_ID_HOSTNAME_LEN_MAX is 64 but should be 256 to provide enough room
for FQDNs longer than 63 bytes because those are valid according to the RFC.
Note that this is not only a 'oh HIPL doesn't support longer names' kind of
problem but a 'an RFC-compliant HIP packet might lead to buffer overflows in the
current HIPL implementation' kind of problem.

Cheers,
 Stefan

From: Stefan Götz <email address hidden>
Subject: [hipl-dev] Re: [Branch ~hipl-core/hipl/trunk] Rev 5263: Hooked in functions for compressing/decompressing host identites in every function that receives ...
Date: 9. Dezember 2010 17:23:15 MEZ
To: <email address hidden>

Another problem lurks on the input side where packet contents are not properly checked, in particular for damaged or forged packet contents. For example, hipd/input.c:766 can be tricked into copying a non-zero terminated string. While I haven't checked whether this may lead to actual buffer overruns in code that assumes it to be 0-terminated, this behavior not correct.

See also the last message in https://code.launchpad.net/~stefan.goetz/hipl/remove-peername/+merge/50801

(Btw, I just recalled that this problem has not been reported as an interoperability problem by Jeff)

> (Btw, I just recalled that this problem has not been reported as an
> interoperability problem by Jeff)

I would take a guess and assume that other implementations do not complain when
HIPL sends only a short hostname instead of an FQDN. On the receiving side, if
HIPL receives and prints another implementation's FQDN in the wire encoding, the
only chance for Jeff to notice would be to check the hipconf output, so maybe he
didn't do that. Or other implementations do not send DIs or maybe also only
short hostnames instead of FQDNs. Just guessing wildly :)

Stefan