Hi,
On Mon, Jun 24, 2019 at 03:39:57PM +0200, Marcin Smoczynski wrote:
> Introduce new function for IPv6 header extension parsing able to
> determine extension length and next protocol number.
>
> This function is helpful when implementing IPv6 header traversing.
>
> Signed-off-by: Marcin Smoczynski <marcinx.smoczyn...@intel.com>
> ---
> lib/librte_net/rte_ip.h | 49 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
> index ae3b7e730..c2c67b85d 100644
> --- a/lib/librte_net/rte_ip.h
> +++ b/lib/librte_net/rte_ip.h
> @@ -428,6 +428,55 @@ rte_ipv6_udptcp_cksum(const struct rte_ipv6_hdr
> *ipv6_hdr, const void *l4_hdr)
> return (uint16_t)cksum;
> }
>
> +/* IPv6 fragmentation header size */
> +#define RTE_IPV6_FRAG_HDR_SIZE 8
> +
> +/**
> + * Parse next IPv6 header extension
> + *
> + * This function checks if proto number is an IPv6 extensions and parses its
> + * data if so, providing information on next header and extension length.
> + *
> + * @param p
> + * Pointer to an extension raw data.
> + * @param proto
> + * Protocol number extracted from the "next header" field from
> + * the IPv6 header or the previous extension.
> + * @param ext_len
> + * Extension data length.
> + * @return
> + * next protocol number if proto is an IPv6 extension, -EINVAL otherwise
> + */
> +static inline int __rte_experimental
> +rte_ipv6_get_next_ext(uint8_t *p, int proto, size_t *ext_len)
> +{
> + int next_proto;
> +
> + switch (proto) {
> + case IPPROTO_AH:
> + next_proto = *p++;
> + *ext_len = (*p + 2) * sizeof(uint32_t);
> + break;
> +
> + case IPPROTO_HOPOPTS:
> + case IPPROTO_ROUTING:
> + case IPPROTO_DSTOPTS:
> + next_proto = *p++;
> + *ext_len = (*p + 1) * sizeof(uint64_t);
> + break;
> +
> + case IPPROTO_FRAGMENT:
> + next_proto = *p;
> + *ext_len = RTE_IPV6_FRAG_HDR_SIZE;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + return next_proto;
> +}
> +
> #ifdef __cplusplus
> }
> #endif
I just want to highlight the potential danger of this kind of function: then
length is read from the packet (controlled by the peer), and returned without
check.
My initial fear was that an attacker forge an IPv6 packet, resulting in *ext_len
being 0. For instance, calling the function with (proto = IPPROTO_ROUTING,
buffer = [ 0, 255, ... ]). Fortunatly, this does not happen, due to the implicit
promotion of *p to a larger integer in the (*p + 1) expression.
That said, what about adding some comments in the description, saying that:
- the pointer p must point to a buffer whose length is at least 2
- the returned length must be checked by the caller to ensure it does not
overflow the packet buffer
You can add my ack in the next version.
Thanks,
Olivier
