kern/39018: ipsec code doesn't handle specific icmp codes

[mlelstv%serpens.de@localhost]

>Number:         39018
>Category:       kern
>Synopsis:       ipsec code doesn't handle specific icmp codes
>Confidential:   no
>Severity:       critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Jun 22 15:10:00 +0000 2008
>Originator:     Michael van Elst
>Release:        NetBSD 4.0_STABLE
>Organization:
-- 
                                Michael van Elst
Internet: mlelstv%serpens.de@localhost
                                "A potential Snark may lurk in every tree."
>Environment:
        
        
System: NetBSD henery 4.0_STABLE NetBSD 4.0_STABLE (HENERY) #10: Sun Jun 15 
18:35:06 CEST 2008 
mlelstv@henery:/home/netbsd4/obj.i386/home/netbsd4/src/sys/arch/i386/compile/HENERY
 i386
Architecture: i386
Machine: i386
>Description:
IPSEC allows to specify rules for specific ICMP packets by qualifying
these with type and code values. This is necessary for example to
use unencrypted packets for IPv6 neighbour detection but encrypted packets
for any other communication.

However, neither the KAME nor FASTIPSEC code honor such rules.

setkey(8) stores ICMP type and code values in the source and destination
port fields of an IPSEC rule. But when searching for such a rule the
secpolicyindex structure is filled with zero values, ignoring the
type and code values of the packet.

>How-To-Repeat:

Try to use IPSEC with IPv6 and specify a rule for ICMP codes 135 and 136.

>Fix:

The problem is in ipsec{4,6}_get_ulp that ignore type and code values.

NetBSD ignores the packet data:
[...]
        case IPPROTO_ICMPV6:
        default:
                /* XXX intermediate headers??? */
                spidx->ul_proto = nxt;
                break;
[...]


FreeBSD does it correctly for V6 (still ignores it for V4):

[...]
        case IPPROTO_ICMPV6:
                spidx->ul_proto = nxt;
                if (off + sizeof(struct icmp6_hdr) > m->m_pkthdr.len)
                        break;
                m_copydata(m, off, sizeof(ih), (caddr_t)&ih);
                ((struct sockaddr_in6 *)&spidx->src)->sin6_port =
                    htons((uint16_t)ih.icmp6_type);
                ((struct sockaddr_in6 *)&spidx->dst)->sin6_port =
                    htons((uint16_t)ih.icmp6_code);
                break;
        default:
                /* XXX intermediate headers??? */
                spidx->ul_proto = nxt;
                break;
[...]

>Unformatted: