| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 13 Nov 2007 08:32:25 -0800
From: "Templin, Fred L" <Fred.L.Templin@...ing.com>
To: "Vlad Yasevich" <vladislav.yasevich@...com>
Cc: <netdev@...r.kernel.org>,
YOSHIFUJI Hideaki / 吉藤英明
<yoshfuji@...ux-ipv6.org>
Subject: RE: [PATCH 01/01] ipv6: RFC4214 Support (v2.0)
HI Vlad,
> -----Original Message-----
> From: Vlad Yasevich [mailto:vladislav.yasevich@...com]
> Sent: Tuesday, November 13, 2007 7:51 AM
> To: Templin, Fred L
> Cc: netdev@...r.kernel.org; YOSHIFUJI Hideaki / 吉藤英明
> Subject: Re: [PATCH 01/01] ipv6: RFC4214 Support (v2.0)
>
> Hi Fred
>
> Some comments.
>
> Templin, Fred L wrote:
> > From: Fred L. Templin <fred.l.templin@...ing.com>
> >
> > This patch includes support for the Intra-Site Automatic Tunnel
> > Addressing Protocol (ISATAP) per RFC4214. It uses the SIT
> > module, and is configured using extensions to the "iproute2"
> > utility.
> >
> > The following diffs are specific to the Linux 2.6.24-rc2 kernel
> > distribution. This message includes the full and patchable
> diff text;
> > please use this version to apply patches.
> >
> > Signed-off-by: Fred L. Templin <fred.l.templin@...ing.com>
> >
> > ---
> >
> > --- linux-2.6.24-rc2/include/linux/if.h.orig
> 2007-11-08 12:05:47.000000000 -0800
> > +++ linux-2.6.24-rc2/include/linux/if.h 2007-11-08
> 08:26:44.000000000 -0800
> > @@ -61,6 +61,7 @@
> > #define IFF_MASTER_ALB 0x10 /* bonding
> master, balance-alb. */
> > #define IFF_BONDING 0x20 /* bonding
> master or slave */
> > #define IFF_SLAVE_NEEDARP 0x40 /* need ARPs
> for validation */
> > +#define IFF_ISATAP 0x80 /* ISATAP interface
> (RFC4214) */
> >
> > #define IF_GET_IFACE 0x0001 /* for querying only */
> > #define IF_GET_PROTO 0x0002
> > --- linux-2.6.24-rc2/include/linux/in.h.orig
> 2007-11-09 08:00:32.000000000 -0800
> > +++ linux-2.6.24-rc2/include/linux/in.h 2007-11-12
> 07:37:05.000000000 -0800
> > @@ -253,6 +253,14 @@ struct sockaddr_in {
> > #define ZERONET(x) (((x) & htonl(0xff000000)) == htonl(0x00000000))
> > #define LOCAL_MCAST(x) (((x) & htonl(0xFFFFFF00)) ==
> htonl(0xE0000000))
> >
> > +/* Special-Use IPv4 Addresses (RFC3330) */
> > +#define PRIVATE_10(x) (((x) & htonl(0xff000000)) ==
> htonl(0x0A000000))
> > +#define LINKLOCAL_169(x) (((x) & htonl(0xffff0000)) ==
> htonl(0xA9FE0000))
> > +#define PRIVATE_172(x) (((x) & htonl(0xfff00000)) ==
> htonl(0xAC100000))
> > +#define TEST_192(x) (((x) & htonl(0xffffff00)) ==
> htonl(0xC0000200))
> > +#define ANYCAST_6TO4(x) (((x) & htonl(0xffffff00)) ==
> htonl(0xC0586300))
> > +#define PRIVATE_192(x) (((x) & htonl(0xffff0000)) ==
> htonl(0xC0A80000))
> > +#define TEST_198(x) (((x) & htonl(0xfffe0000)) ==
> htonl(0xC6120000))
> > #endif
> >
> > #endif /* _LINUX_IN_H */
> > --- linux-2.6.24-rc2/include/net/addrconf.h.orig
> 2007-11-08 12:06:17.000000000 -0800
> > +++ linux-2.6.24-rc2/include/net/addrconf.h 2007-11-12
> 14:29:51.000000000 -0800
> > @@ -241,6 +241,12 @@ static inline int ipv6_addr_is_ll_all_ro
> > addr->s6_addr32[3] == htonl(0x00000002));
> > }
> >
> > +/* only for IFF_ISATAP interfaces */
> > +static inline int ipv6_addr_is_isatap(const struct in6_addr *addr)
> > +{
> > + return ((addr->s6_addr32[2] | htonl(0x02000000)) ==
> htonl(0x02005EFE));
> > +}
> > +
> > #ifdef CONFIG_PROC_FS
> > extern int if6_proc_init(void);
> > extern void if6_proc_exit(void);
> > --- linux-2.6.24-rc2/net/ipv6/addrconf.c.orig
> 2007-11-08 11:59:35.000000000 -0800
> > +++ linux-2.6.24-rc2/net/ipv6/addrconf.c 2007-11-12
> 14:32:43.000000000 -0800
> > @@ -75,7 +75,7 @@
> > #include <net/ip.h>
> > #include <net/netlink.h>
> > #include <net/pkt_sched.h>
> > -#include <linux/if_tunnel.h>
> > +#include <net/ipip.h>
> > #include <linux/rtnetlink.h>
> >
> > #ifdef CONFIG_IPV6_PRIVACY
> > @@ -1424,6 +1424,20 @@ static int addrconf_ifid_infiniband(u8 *
> > return 0;
> > }
> >
> > +static int addrconf_ifid_isatap(u8 *eui, __be32 addr)
> > +{
> > +
> > + eui[0] = 0x02; eui[1] = 0; eui[2] = 0x5E; eui[3] = 0xFE;
> > + memcpy (eui+4, &addr, 4);
> > +
> > + if (ZERONET(addr) || PRIVATE_10(addr) || LOOPBACK(addr) ||
> > + LINKLOCAL_169(addr) || PRIVATE_172(addr) ||
> TEST_192(addr) ||
> > + ANYCAST_6TO4(addr) || PRIVATE_192(addr) || TEST_198(addr) ||
> > + MULTICAST(addr) || BADCLASS(addr)) eui[0] &= ‾0x02;
>
> Please put the assignment on its own line.
OK.
> > +
> > + return 0;
> > +}
> > +
> > static int ipv6_generate_eui64(u8 *eui, struct net_device *dev)
> > {
> > switch (dev->type) {
> > @@ -1435,6 +1449,9 @@ static int ipv6_generate_eui64(u8 *eui,
> > return addrconf_ifid_arcnet(eui, dev);
> > case ARPHRD_INFINIBAND:
> > return addrconf_ifid_infiniband(eui, dev);
> > + case ARPHRD_SIT:
> > + if (dev->priv_flags & IFF_ISATAP)
> > + return addrconf_ifid_isatap(eui,
> *(__be32 *)dev->dev_addr);
> > }
> > return -1;
> > }
> > @@ -1470,8 +1487,7 @@ regen:
> > *
> > * - Reserved subnet anycast (RFC 2526)
> > * 11111101 11....11 1xxxxxxx
> > - * - ISATAP (draft-ietf-ngtrans-isatap-13.txt) 5.1
> > - * 00-00-5E-FE-xx-xx-xx-xx
> > + * - ISATAP (RFC4214) 00-00-5E-FE-xx-xx-xx-xx
> > * - value 0
> > * - XXX: already assigned to an address on the device
> > */
> > @@ -2201,6 +2217,29 @@ static void addrconf_sit_config(struct n
> > return;
> > }
> >
> > + /* ISATAP (RFC4214) - NBMA link */
> > + if (dev->priv_flags & IFF_ISATAP) {
> > + struct in6_addr addr;
> > +
> > + addrconf_add_lroute(dev);
> > +
> > + ipv6_addr_set(&addr, htonl(0xFE800000), 0, 0, 0);
> > +
> > + if (ipv6_generate_eui64(addr.s6_addr + 8, dev) == 0) {
> > + struct inet6_ifaddr *ifp;
> > +
> > + ifp = ipv6_add_addr(idev, &addr, 64,
> > + IFA_LINK, IFA_F_PERMANENT);
> > + if (!IS_ERR(ifp)) {
> > + addrconf_prefix_route(&ifp->addr,
> > + ifp->prefix_len,
> idev->dev, 0, 0);
> > + addrconf_dad_start(ifp, 0);
> > + in6_ifa_put(ifp);
> > + }
> > +
>
> If ipv6_generate_eui64() or ipv6_add_addr() fail, you will
> still have a link-local
> prefix route on the device.
>
> You might want to pull out the above code into a separate
> function and do correct
> clean-ups on failures.
OK.
> > + return;
> > + }
> > +
> > sit_add_v4_addrs(idev);
> >
> > if (dev->flags&IFF_POINTOPOINT) {
> > @@ -2531,6 +2570,18 @@ static void addrconf_rs_timer(unsigned l
> > * Announcement received after solicitation
> > * was sent
> > */
> > +
> > + /* ISATAP (RFC4214) - schedule next RS/RA */
> > + if (ifp->idev->dev->priv_flags & IFF_ISATAP) {
> > + struct ip_tunnel *t =
> netdev_priv(ifp->idev->dev);
> > + if (t->parms.i_key != INADDR_NONE) {
> > + spin_lock(&ifp->lock);
> > + ifp->probes = 0;
> > + ifp->idev->if_flags &=
> ‾(IF_RS_SENT|IF_RA_RCVD);
> > + addrconf_mod_timer(ifp, AC_DAD,
> t->parms.o_key*HZ);
>
> You are using a DAD timer to schedule RS?
I am using the DAD timer to re-DAD the link local, which
in turn schedules RS.
> > + spin_unlock(&ifp->lock);
> > + }
> > + }
> > goto out;
> > }
> >
> > @@ -2545,10 +2596,28 @@ static void addrconf_rs_timer(unsigned l
> > ifp->idev->cnf.rtr_solicit_interval);
> > spin_unlock(&ifp->lock);
> >
> > - ipv6_addr_all_routers(&all_routers);
> > + /* ISATAP (RFC4214) - unicast RS */
> > + if (ifp->idev->dev->priv_flags & IFF_ISATAP) {
> > + struct ip_tunnel *t =
> netdev_priv(ifp->idev->dev);
> > +
> > + if (t->parms.i_key == INADDR_NONE) goto out;
> > +
> > + ipv6_addr_set(&all_routers,
> htonl(0xFE800000), 0, 0, 0);
> > +
> addrconf_ifid_isatap(all_routers.s6_addr + 8, t->parms.i_key);
> > + } else
> > + ipv6_addr_all_routers(&all_routers);
> >
> > ndisc_send_rs(ifp->idev->dev, &ifp->addr, &all_routers);
> > } else {
> > + /* ISATAP (RFC4214) - try again later */
> > + if (ifp->idev->dev->priv_flags & IFF_ISATAP) {
> > + struct ip_tunnel *t =
> netdev_priv(ifp->idev->dev);
> > + if (t->parms.i_key != INADDR_NONE) {
> > + ifp->probes = 0;
> > + ifp->idev->if_flags &=
> ‾(IF_RS_SENT|IF_RA_RCVD);
> > + addrconf_mod_timer(ifp, AC_DAD,
> t->parms.o_key*HZ);
>
> Again, using DAD timer?
Same as above.
> > + }
> > + }
> > spin_unlock(&ifp->lock);
> > /*
> > * Note: we do not support deprecated "all on-link"
> > @@ -2594,6 +2663,7 @@ static void addrconf_dad_start(struct in
> > spin_lock_bh(&ifp->lock);
> >
> > if (dev->flags&(IFF_NOARP|IFF_LOOPBACK) ||
> > + dev->priv_flags&IFF_ISATAP ||
> > !(ifp->flags&IFA_F_TENTATIVE) ||
> > ifp->flags & IFA_F_NODAD) {
> > ifp->flags &= ‾(IFA_F_TENTATIVE|IFA_F_OPTIMISTIC);
> > @@ -2690,7 +2760,16 @@ static void addrconf_dad_completed(struc
> > (ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL)) {
> > struct in6_addr all_routers;
> >
> > - ipv6_addr_all_routers(&all_routers);
> > + /* ISATAP (RFC4214) - unicast RS */
> > + if (ifp->idev->dev->priv_flags & IFF_ISATAP) {
> > + struct ip_tunnel *t =
> netdev_priv(ifp->idev->dev);
> > +
> > + if (t->parms.i_key == INADDR_NONE) return;
> > +
> > + ipv6_addr_set(&all_routers,
> htonl(0xFE800000), 0, 0, 0);
> > +
> addrconf_ifid_isatap(all_routers.s6_addr + 8, t->parms.i_key);
> > + } else
> > + ipv6_addr_all_routers(&all_routers);
> >
> > /*
> > * If a host as already performed a random delay
> > --- linux-2.6.24-rc2/net/ipv6/sit.c.orig 2007-11-08
> 12:03:41.000000000 -0800
> > +++ linux-2.6.24-rc2/net/ipv6/sit.c 2007-11-12
> 14:30:52.000000000 -0800
> > @@ -16,6 +16,7 @@
> > * Changes:
> > * Roger Venning <r.venning@...stra.com>: 6to4 support
> > * Nate Thompson <nate@...bog.net>: 6to4 support
> > + * Fred L. Templin <fltemplin@....org>: isatap support
> > */
> >
> > #include <linux/module.h>
> > @@ -182,6 +183,8 @@ static struct ip_tunnel * ipip6_tunnel_l
> > dev->init = ipip6_tunnel_init;
> > nt->parms = *parms;
> >
> > + if (parms->i_key) dev->priv_flags |= IFF_ISATAP;
> > +
> > if (register_netdevice(dev) < 0) {
> > free_netdev(dev);
> > goto failed;
> > @@ -382,6 +385,47 @@ static int ipip6_rcv(struct sk_buff *skb
> > IPCB(skb)->flags = 0;
> > skb->protocol = htons(ETH_P_IPV6);
> > skb->pkt_type = PACKET_HOST;
> > +
> > + /* ISATAP (RFC4214) - check source address */
> > + if (tunnel->dev->priv_flags & IFF_ISATAP) {
> > + struct neighbour *neigh;
> > + struct dst_entry *dst;
> > + struct flowi fl;
> > + struct in6_addr *addr6;
> > + struct ipv6hdr *iph6;
> > +
> > + /* from ISATAP router */
> > + if ((tunnel->parms.i_key != INADDR_NONE) &&
> > + (iph->saddr ==
> tunnel->parms.i_key)) goto accept;
> > +
> > + iph6 = ipv6_hdr(skb);
> > + addr6 = &iph6->saddr;
> > +
> > + /* from legitimate previous hop */
> > + memset(&fl, 0, sizeof(fl));
> > + fl.proto = iph6->nexthdr;
> > + ipv6_addr_copy(&fl.fl6_dst, addr6);
> > + fl.oif = tunnel->dev->ifindex;
> > + security_skb_classify_flow(skb, &fl);
> > +
> > + if (!(dst = ip6_route_output(NULL, &fl)) ||
> > + (dst->dev != tunnel->dev) ||
> > + ((neigh = dst->neighbour) ==
> NULL)) goto drop;
>
> You are catching the error conditions incorrectly.
> ip6_route_output will return
> a pointer to dst whose error field will be set if the route
> lookup failed. You need
> to do something like:
> dst = ip6_route_output(NULL, &fl);
> if (dst->error || dst->dev !=
> tunnel->dev || ...)
OK.
> Also, please put the 'goto' on its own line.
OK.
> > +
> > + addr6 = (struct in6_addr*)&neigh->primary_key;
> > +
> > + if (!(ipv6_addr_is_isatap(addr6)) ||
> > + (addr6->s6_addr32[3] != iph->saddr)) {
> > +drop:
> > + tunnel->stat.rx_errors++;
> > + read_unlock(&ipip6_lock);
> > + dst_release(dst);
> > + kfree_skb(skb);
> > + return 0;
> > + }
> > + dst_release(dst);
> > + }
> > +accept:
>
> You never use the 'drop' or 'accept' tags. You can remove
> them.
OK.
> Also, it appears
> that you are doing some validations on the tunnel. Might
> want to split that out into its
> own function and just call that.
OK.
Fred
fred.l.templin@...ing.com
> -vlad
>
> > tunnel->stat.rx_packets++;
> > tunnel->stat.rx_bytes += skb->len;
> > skb->dev = tunnel->dev;
> > @@ -444,6 +488,29 @@ static int ipip6_tunnel_xmit(struct sk_b
> > if (skb->protocol != htons(ETH_P_IPV6))
> > goto tx_error;
> >
> > + /* ISATAP (RFC4214) - must come before 6to4 */
> > + if (dev->priv_flags & IFF_ISATAP) {
> > + struct neighbour *neigh = NULL;
> > +
> > + if (skb->dst)
> > + neigh = skb->dst->neighbour;
> > +
> > + if (neigh == NULL) {
> > + if (net_ratelimit())
> > + printk(KERN_DEBUG "sit: nexthop
> == NULL¥n");
> > + goto tx_error;
> > + }
> > +
> > + addr6 = (struct in6_addr*)&neigh->primary_key;
> > + addr_type = ipv6_addr_type(addr6);
> > +
> > + if ((addr_type & IPV6_ADDR_UNICAST) &&
> > + ipv6_addr_is_isatap(addr6))
> > + dst = addr6->s6_addr32[3];
> > + else
> > + goto tx_error;
> > + }
> > +
> > if (!dst)
> > dst = try_6to4(&iph6->daddr);
> >
> > @@ -651,6 +718,8 @@ ipip6_tunnel_ioctl (struct net_device *d
> > ipip6_tunnel_unlink(t);
> > t->parms.iph.saddr = p.iph.saddr;
> > t->parms.iph.daddr = p.iph.daddr;
> > + t->parms.i_key = p.i_key;
> > + t->parms.o_key = p.o_key;
> > memcpy(dev->dev_addr, &p.iph.saddr, 4);
> > memcpy(dev->broadcast, &p.iph.daddr, 4);
> > ipip6_tunnel_link(t);
> > @@ -663,6 +732,8 @@ ipip6_tunnel_ioctl (struct net_device *d
> > if (cmd == SIOCCHGTUNNEL) {
> > t->parms.iph.ttl = p.iph.ttl;
> > t->parms.iph.tos = p.iph.tos;
> > + t->parms.i_key = p.i_key;
> > + t->parms.o_key = p.o_key;
> > }
> > if
> (copy_to_user(ifr->ifr_ifru.ifru_data, &t->parms, sizeof(p)))
> > err = -EFAULT;
> > -
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@...r.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists