[RFC,v2,08/10] virtio-net rsc: Sanity check & More bypass cases check

From: Wei Xu <wei@wei-thinkpad.nay.redhat.com>

More general exception cases check
1. Incorrect version in IP header
2. IP options & IP fragment
3. Not a TCP packets
4. Sanity size check to prevent buffer overflow attack.

Signed-off-by: Wei Xu <wexu@redhat.com>
---
 hw/net/virtio-net.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

On 02/01/2016 02:13 AM, wexu@redhat.com wrote:
> From: Wei Xu <wei@wei-thinkpad.nay.redhat.com>
>
> More general exception cases check
> 1. Incorrect version in IP header
> 2. IP options & IP fragment
> 3. Not a TCP packets
> 4. Sanity size check to prevent buffer overflow attack.
>
> Signed-off-by: Wei Xu <wexu@redhat.com>

Let's squash this into previous patches too for a better bisection
ability and complete implementation.

> ---
>  hw/net/virtio-net.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index b0987d0..9b44762 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1948,6 +1948,46 @@ static size_t virtio_net_rsc_drain_one(NetRscChain *chain, NetClientState *nc,
>  
>      return virtio_net_do_receive(nc, buf, size);
>  }
> +
> +static int32_t virtio_net_rsc_filter4(NetRscChain *chain, struct ip_header *ip,
> +                                      const uint8_t *buf, size_t size)

This function checks for ip header, so need rename it to something like
"virtio_net_rsc_ipv4_filter()"

> +{
> +    uint16_t ip_len;
> +
> +    if (size < (TCP4_OFFSET + sizeof(tcp_header))) {
> +        return RSC_BYPASS;
> +    }
> +
> +    /* Not an ipv4 one */
> +    if (0x4 != ((0xF0 & ip->ip_ver_len) >> 4)) {

Let's don't use magic value like 0x4 here.

> +        return RSC_BYPASS;
> +    }
> +
> +    /* Don't handle packets with ip option */
> +    if (5 != (0xF & ip->ip_ver_len)) {
> +        return RSC_BYPASS;
> +    }
> +
> +    /* Don't handle packets with ip fragment */
> +    if (!(htons(ip->ip_off) & IP_DF)) {
> +        return RSC_BYPASS;
> +    }
> +
> +    if (ip->ip_p != IPPROTO_TCP) {
> +        return RSC_BYPASS;
> +    }
> +
> +    /* Sanity check */
> +    ip_len = htons(ip->ip_len);
> +    if (ip_len < (sizeof(struct ip_header) + sizeof(struct tcp_header))
> +        || ip_len > (size - IP_OFFSET)) {
> +        return RSC_BYPASS;
> +    }
> +
> +    return RSC_WANT;
> +}
> +
> +
>  static size_t virtio_net_rsc_receive4(void *opq, NetClientState* nc,
>                                        const uint8_t *buf, size_t size)
>  {
> @@ -1958,6 +1998,10 @@ static size_t virtio_net_rsc_receive4(void *opq, NetClientState* nc,
>      chain = (NetRscChain *)opq;
>      ip = (struct ip_header *)(buf + IP_OFFSET);
>  
> +    if (RSC_WANT != virtio_net_rsc_filter4(chain, ip, buf, size)) {
> +        return virtio_net_do_receive(nc, buf, size);
> +    }
> +
>      ret = virtio_net_rsc_parse_tcp_ctrl((uint8_t *)ip,
>                                          (0xF & ip->ip_ver_len) << 2);
>      if (RSC_BYPASS == ret) {

On 02/01/2016 02:58 PM, Jason Wang wrote:
>
> On 02/01/2016 02:13 AM, wexu@redhat.com wrote:
>> From: Wei Xu <wei@wei-thinkpad.nay.redhat.com>
>>
>> More general exception cases check
>> 1. Incorrect version in IP header
>> 2. IP options & IP fragment
>> 3. Not a TCP packets
>> 4. Sanity size check to prevent buffer overflow attack.
>>
>> Signed-off-by: Wei Xu <wexu@redhat.com>
> Let's squash this into previous patches too for a better bisection
> ability and complete implementation.
ok.
>
>> ---
>>   hw/net/virtio-net.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 44 insertions(+)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index b0987d0..9b44762 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -1948,6 +1948,46 @@ static size_t virtio_net_rsc_drain_one(NetRscChain *chain, NetClientState *nc,
>>   
>>       return virtio_net_do_receive(nc, buf, size);
>>   }
>> +
>> +static int32_t virtio_net_rsc_filter4(NetRscChain *chain, struct ip_header *ip,
>> +                                      const uint8_t *buf, size_t size)
> This function checks for ip header, so need rename it to something like
> "virtio_net_rsc_ipv4_filter()"
OK.
>
>> +{
>> +    uint16_t ip_len;
>> +
>> +    if (size < (TCP4_OFFSET + sizeof(tcp_header))) {
>> +        return RSC_BYPASS;
>> +    }
>> +
>> +    /* Not an ipv4 one */
>> +    if (0x4 != ((0xF0 & ip->ip_ver_len) >> 4)) {
> Let's don't use magic value like 0x4 here.
OK.
>
>> +        return RSC_BYPASS;
>> +    }
>> +
>> +    /* Don't handle packets with ip option */
>> +    if (5 != (0xF & ip->ip_ver_len)) {
>> +        return RSC_BYPASS;
>> +    }
>> +
>> +    /* Don't handle packets with ip fragment */
>> +    if (!(htons(ip->ip_off) & IP_DF)) {
>> +        return RSC_BYPASS;
>> +    }
>> +
>> +    if (ip->ip_p != IPPROTO_TCP) {
>> +        return RSC_BYPASS;
>> +    }
>> +
>> +    /* Sanity check */
>> +    ip_len = htons(ip->ip_len);
>> +    if (ip_len < (sizeof(struct ip_header) + sizeof(struct tcp_header))
>> +        || ip_len > (size - IP_OFFSET)) {
>> +        return RSC_BYPASS;
>> +    }
>> +
>> +    return RSC_WANT;
>> +}
>> +
>> +
>>   static size_t virtio_net_rsc_receive4(void *opq, NetClientState* nc,
>>                                         const uint8_t *buf, size_t size)
>>   {
>> @@ -1958,6 +1998,10 @@ static size_t virtio_net_rsc_receive4(void *opq, NetClientState* nc,
>>       chain = (NetRscChain *)opq;
>>       ip = (struct ip_header *)(buf + IP_OFFSET);
>>   
>> +    if (RSC_WANT != virtio_net_rsc_filter4(chain, ip, buf, size)) {
>> +        return virtio_net_do_receive(nc, buf, size);
>> +    }
>> +
>>       ret = virtio_net_rsc_parse_tcp_ctrl((uint8_t *)ip,
>>                                           (0xF & ip->ip_ver_len) << 2);
>>       if (RSC_BYPASS == ret) {
>