From 2a629880fd0ef26ca8696431c3b120f5e27837f3 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Mon, 19 Aug 2019 10:28:30 +0100 Subject: [PATCH 1/4] Rename crypto-key config options, improve control flow --- cmd/yggdrasil/main.go | 18 ++++++++ src/config/config.go | 10 ++-- src/tuntap/admin.go | 24 +++++----- src/tuntap/ckr.go | 74 ++++++++++++++--------------- src/tuntap/iface.go | 105 +++++++++++++++++++++++++++--------------- 5 files changed, 140 insertions(+), 91 deletions(-) diff --git a/cmd/yggdrasil/main.go b/cmd/yggdrasil/main.go index 15d7d226..2fa10720 100644 --- a/cmd/yggdrasil/main.go +++ b/cmd/yggdrasil/main.go @@ -80,6 +80,24 @@ func readConfig(useconf *bool, useconffile *string, normaliseconf *bool) *config if listen, ok := dat["Listen"].(string); ok { dat["Listen"] = []string{listen} } + if tunnelrouting, ok := dat["TunnelRouting"].(map[string]interface{}); ok { + if c, ok := tunnelrouting["IPv4Sources"]; ok { + delete(tunnelrouting, "IPv4Sources") + tunnelrouting["IPv4LocalSubnets"] = c + } + if c, ok := tunnelrouting["IPv6Sources"]; ok { + delete(tunnelrouting, "IPv6Sources") + tunnelrouting["IPv6LocalSubnets"] = c + } + if c, ok := tunnelrouting["IPv4Destinations"]; ok { + delete(tunnelrouting, "IPv4Destinations") + tunnelrouting["IPv4RemoteSubnets"] = c + } + if c, ok := tunnelrouting["IPv6Destinations"]; ok { + delete(tunnelrouting, "IPv6Destinations") + tunnelrouting["IPv6RemoteSubnets"] = c + } + } // Sanitise the config confJson, err := json.Marshal(dat) if err != nil { diff --git a/src/config/config.go b/src/config/config.go index a7bbbacf..6c127552 100644 --- a/src/config/config.go +++ b/src/config/config.go @@ -74,11 +74,11 @@ type SessionFirewall struct { // TunnelRouting contains the crypto-key routing tables for tunneling type TunnelRouting struct { - Enable bool `comment:"Enable or disable tunnel routing."` - IPv6Destinations map[string]string `comment:"IPv6 CIDR subnets, mapped to the EncryptionPublicKey to which they\nshould be routed, e.g. { \"aaaa:bbbb:cccc::/e\": \"boxpubkey\", ... }"` - IPv6Sources []string `comment:"Optional IPv6 source subnets which are allowed to be tunnelled in\naddition to this node's Yggdrasil address/subnet. If not\nspecified, only traffic originating from this node's Yggdrasil\naddress or subnet will be tunnelled."` - IPv4Destinations map[string]string `comment:"IPv4 CIDR subnets, mapped to the EncryptionPublicKey to which they\nshould be routed, e.g. { \"a.b.c.d/e\": \"boxpubkey\", ... }"` - IPv4Sources []string `comment:"IPv4 source subnets which are allowed to be tunnelled. Unlike for\nIPv6, this option is required for bridging IPv4 traffic. Only\ntraffic with a source matching these subnets will be tunnelled."` + Enable bool `comment:"Enable or disable tunnel routing."` + IPv6RemoteSubnets map[string]string `comment:"IPv6 subnets belonging to remote nodes, mapped to the node's public\nkey, e.g. { \"aaaa:bbbb:cccc::/e\": \"boxpubkey\", ... }"` + IPv6LocalSubnets []string `comment:"IPv6 subnets belonging to this node's end of the tunnels. Only traffic\nfrom these ranges (or the Yggdrasil node's IPv6 address/subnet)\nwill be tunnelled."` + IPv4RemoteSubnets map[string]string `comment:"IPv4 subnets belonging to remote nodes, mapped to the node's public\nkey, e.g. { \"a.b.c.d/e\": \"boxpubkey\", ... }"` + IPv4LocalSubnets []string `comment:"IPv4 subnets belonging to this node's end of the tunnels. Only traffic\nfrom these ranges will be tunnelled."` } // SwitchOptions contains tuning options for the switch diff --git a/src/tuntap/admin.go b/src/tuntap/admin.go index 21c7048d..778c03ae 100644 --- a/src/tuntap/admin.go +++ b/src/tuntap/admin.go @@ -66,15 +66,15 @@ func (t *TunAdapter) SetupAdminHandlers(a *admin.AdminSocket) { t.ckr.setEnabled(enabled) return admin.Info{"enabled": enabled}, nil }) - a.AddHandler("addSourceSubnet", []string{"subnet"}, func(in admin.Info) (admin.Info, error) { - if err := t.ckr.addSourceSubnet(in["subnet"].(string)); err == nil { + a.AddHandler("addLocalSubnet", []string{"subnet"}, func(in admin.Info) (admin.Info, error) { + if err := t.ckr.addLocalSubnet(in["subnet"].(string)); err == nil { return admin.Info{"added": []string{in["subnet"].(string)}}, nil } else { return admin.Info{"not_added": []string{in["subnet"].(string)}}, errors.New("Failed to add source subnet") } }) - a.AddHandler("addRoute", []string{"subnet", "box_pub_key"}, func(in admin.Info) (admin.Info, error) { - if err := t.ckr.addRoute(in["subnet"].(string), in["box_pub_key"].(string)); err == nil { + a.AddHandler("addRemoteSubnet", []string{"subnet", "box_pub_key"}, func(in admin.Info) (admin.Info, error) { + if err := t.ckr.addRemoteSubnet(in["subnet"].(string), in["box_pub_key"].(string)); err == nil { return admin.Info{"added": []string{fmt.Sprintf("%s via %s", in["subnet"].(string), in["box_pub_key"].(string))}}, nil } else { return admin.Info{"not_added": []string{fmt.Sprintf("%s via %s", in["subnet"].(string), in["box_pub_key"].(string))}}, errors.New("Failed to add route") @@ -87,8 +87,8 @@ func (t *TunAdapter) SetupAdminHandlers(a *admin.AdminSocket) { subnets = append(subnets, subnet.String()) } } - getSourceSubnets(t.ckr.ipv4sources) - getSourceSubnets(t.ckr.ipv6sources) + getSourceSubnets(t.ckr.ipv4locals) + getSourceSubnets(t.ckr.ipv6locals) return admin.Info{"source_subnets": subnets}, nil }) a.AddHandler("getRoutes", []string{}, func(in admin.Info) (admin.Info, error) { @@ -98,19 +98,19 @@ func (t *TunAdapter) SetupAdminHandlers(a *admin.AdminSocket) { routes[ckr.subnet.String()] = hex.EncodeToString(ckr.destination[:]) } } - getRoutes(t.ckr.ipv4routes) - getRoutes(t.ckr.ipv6routes) + getRoutes(t.ckr.ipv4remotes) + getRoutes(t.ckr.ipv6remotes) return admin.Info{"routes": routes}, nil }) - a.AddHandler("removeSourceSubnet", []string{"subnet"}, func(in admin.Info) (admin.Info, error) { - if err := t.ckr.removeSourceSubnet(in["subnet"].(string)); err == nil { + a.AddHandler("removeLocalSubnet", []string{"subnet"}, func(in admin.Info) (admin.Info, error) { + if err := t.ckr.removeLocalSubnet(in["subnet"].(string)); err == nil { return admin.Info{"removed": []string{in["subnet"].(string)}}, nil } else { return admin.Info{"not_removed": []string{in["subnet"].(string)}}, errors.New("Failed to remove source subnet") } }) - a.AddHandler("removeRoute", []string{"subnet", "box_pub_key"}, func(in admin.Info) (admin.Info, error) { - if err := t.ckr.removeRoute(in["subnet"].(string), in["box_pub_key"].(string)); err == nil { + a.AddHandler("removeRemoteSubnet", []string{"subnet", "box_pub_key"}, func(in admin.Info) (admin.Info, error) { + if err := t.ckr.removeRemoteSubnet(in["subnet"].(string), in["box_pub_key"].(string)); err == nil { return admin.Info{"removed": []string{fmt.Sprintf("%s via %s", in["subnet"].(string), in["box_pub_key"].(string))}}, nil } else { return admin.Info{"not_removed": []string{fmt.Sprintf("%s via %s", in["subnet"].(string), in["box_pub_key"].(string))}}, errors.New("Failed to remove route") diff --git a/src/tuntap/ckr.go b/src/tuntap/ckr.go index 80e3e42f..f93cb7cf 100644 --- a/src/tuntap/ckr.go +++ b/src/tuntap/ckr.go @@ -21,12 +21,12 @@ type cryptokey struct { tun *TunAdapter enabled atomic.Value // bool reconfigure chan chan error - ipv4routes []cryptokey_route - ipv6routes []cryptokey_route + ipv4remotes []cryptokey_route + ipv6remotes []cryptokey_route ipv4cache map[address.Address]cryptokey_route ipv6cache map[address.Address]cryptokey_route - ipv4sources []net.IPNet - ipv6sources []net.IPNet + ipv4locals []net.IPNet + ipv6locals []net.IPNet mutexroutes sync.RWMutex mutexcaches sync.RWMutex mutexsources sync.RWMutex @@ -66,42 +66,42 @@ func (c *cryptokey) configure() error { // Clear out existing routes c.mutexroutes.Lock() - c.ipv6routes = make([]cryptokey_route, 0) - c.ipv4routes = make([]cryptokey_route, 0) + c.ipv6remotes = make([]cryptokey_route, 0) + c.ipv4remotes = make([]cryptokey_route, 0) c.mutexroutes.Unlock() // Add IPv6 routes - for ipv6, pubkey := range current.TunnelRouting.IPv6Destinations { - if err := c.addRoute(ipv6, pubkey); err != nil { + for ipv6, pubkey := range current.TunnelRouting.IPv6RemoteSubnets { + if err := c.addRemoteSubnet(ipv6, pubkey); err != nil { return err } } // Add IPv4 routes - for ipv4, pubkey := range current.TunnelRouting.IPv4Destinations { - if err := c.addRoute(ipv4, pubkey); err != nil { + for ipv4, pubkey := range current.TunnelRouting.IPv4RemoteSubnets { + if err := c.addRemoteSubnet(ipv4, pubkey); err != nil { return err } } // Clear out existing sources c.mutexsources.Lock() - c.ipv6sources = make([]net.IPNet, 0) - c.ipv4sources = make([]net.IPNet, 0) + c.ipv6locals = make([]net.IPNet, 0) + c.ipv4locals = make([]net.IPNet, 0) c.mutexsources.Unlock() // Add IPv6 sources - c.ipv6sources = make([]net.IPNet, 0) - for _, source := range current.TunnelRouting.IPv6Sources { - if err := c.addSourceSubnet(source); err != nil { + c.ipv6locals = make([]net.IPNet, 0) + for _, source := range current.TunnelRouting.IPv6LocalSubnets { + if err := c.addLocalSubnet(source); err != nil { return err } } // Add IPv4 sources - c.ipv4sources = make([]net.IPNet, 0) - for _, source := range current.TunnelRouting.IPv4Sources { - if err := c.addSourceSubnet(source); err != nil { + c.ipv4locals = make([]net.IPNet, 0) + for _, source := range current.TunnelRouting.IPv4LocalSubnets { + if err := c.addLocalSubnet(source); err != nil { return err } } @@ -128,8 +128,8 @@ func (c *cryptokey) isEnabled() bool { // Check whether the given address (with the address length specified in bytes) // matches either the current node's address, the node's routed subnet or the -// list of subnets specified in IPv4Sources/IPv6Sources. -func (c *cryptokey) isValidSource(addr address.Address, addrlen int) bool { +// list of subnets specified in ipv4locals/ipv6locals. +func (c *cryptokey) isValidLocalAddress(addr address.Address, addrlen int) bool { c.mutexsources.RLock() defer c.mutexsources.RUnlock() @@ -154,9 +154,9 @@ func (c *cryptokey) isValidSource(addr address.Address, addrlen int) bool { // Check if the prefix is IPv4 or IPv6 if addrlen == net.IPv6len { - routingsources = &c.ipv6sources + routingsources = &c.ipv6locals } else if addrlen == net.IPv4len { - routingsources = &c.ipv4sources + routingsources = &c.ipv4locals } else { return false } @@ -174,7 +174,7 @@ func (c *cryptokey) isValidSource(addr address.Address, addrlen int) bool { // Adds a source subnet, which allows traffic with these source addresses to // be tunnelled using crypto-key routing. -func (c *cryptokey) addSourceSubnet(cidr string) error { +func (c *cryptokey) addLocalSubnet(cidr string) error { c.mutexsources.Lock() defer c.mutexsources.Unlock() @@ -192,9 +192,9 @@ func (c *cryptokey) addSourceSubnet(cidr string) error { // Check if the prefix is IPv4 or IPv6 if prefixsize == net.IPv6len*8 { - routingsources = &c.ipv6sources + routingsources = &c.ipv6locals } else if prefixsize == net.IPv4len*8 { - routingsources = &c.ipv4sources + routingsources = &c.ipv4locals } else { return errors.New("Unexpected prefix size") } @@ -214,7 +214,7 @@ func (c *cryptokey) addSourceSubnet(cidr string) error { // Adds a destination route for the given CIDR to be tunnelled to the node // with the given BoxPubKey. -func (c *cryptokey) addRoute(cidr string, dest string) error { +func (c *cryptokey) addRemoteSubnet(cidr string, dest string) error { c.mutexroutes.Lock() c.mutexcaches.Lock() defer c.mutexroutes.Unlock() @@ -235,10 +235,10 @@ func (c *cryptokey) addRoute(cidr string, dest string) error { // Check if the prefix is IPv4 or IPv6 if prefixsize == net.IPv6len*8 { - routingtable = &c.ipv6routes + routingtable = &c.ipv6remotes routingcache = &c.ipv6cache } else if prefixsize == net.IPv4len*8 { - routingtable = &c.ipv4routes + routingtable = &c.ipv4remotes routingcache = &c.ipv4cache } else { return errors.New("Unexpected prefix size") @@ -328,9 +328,9 @@ func (c *cryptokey) getPublicKeyForAddress(addr address.Address, addrlen int) (c // Check if the prefix is IPv4 or IPv6 if addrlen == net.IPv6len { - routingtable = &c.ipv6routes + routingtable = &c.ipv6remotes } else if addrlen == net.IPv4len { - routingtable = &c.ipv4routes + routingtable = &c.ipv4remotes } else { return crypto.BoxPubKey{}, errors.New("Unexpected prefix size") } @@ -339,7 +339,7 @@ func (c *cryptokey) getPublicKeyForAddress(addr address.Address, addrlen int) (c ip := make(net.IP, addrlen) copy(ip[:addrlen], addr[:]) - // Check if we have a route. At this point c.ipv6routes should be + // Check if we have a route. At this point c.ipv6remotes should be // pre-sorted so that the most specific routes are first for _, route := range *routingtable { // Does this subnet match the given IP? @@ -371,7 +371,7 @@ func (c *cryptokey) getPublicKeyForAddress(addr address.Address, addrlen int) (c // Removes a source subnet, which allows traffic with these source addresses to // be tunnelled using crypto-key routing. -func (c *cryptokey) removeSourceSubnet(cidr string) error { +func (c *cryptokey) removeLocalSubnet(cidr string) error { c.mutexsources.Lock() defer c.mutexsources.Unlock() @@ -389,9 +389,9 @@ func (c *cryptokey) removeSourceSubnet(cidr string) error { // Check if the prefix is IPv4 or IPv6 if prefixsize == net.IPv6len*8 { - routingsources = &c.ipv6sources + routingsources = &c.ipv6locals } else if prefixsize == net.IPv4len*8 { - routingsources = &c.ipv4sources + routingsources = &c.ipv4locals } else { return errors.New("Unexpected prefix size") } @@ -409,7 +409,7 @@ func (c *cryptokey) removeSourceSubnet(cidr string) error { // Removes a destination route for the given CIDR to be tunnelled to the node // with the given BoxPubKey. -func (c *cryptokey) removeRoute(cidr string, dest string) error { +func (c *cryptokey) removeRemoteSubnet(cidr string, dest string) error { c.mutexroutes.Lock() c.mutexcaches.Lock() defer c.mutexroutes.Unlock() @@ -430,10 +430,10 @@ func (c *cryptokey) removeRoute(cidr string, dest string) error { // Check if the prefix is IPv4 or IPv6 if prefixsize == net.IPv6len*8 { - routingtable = &c.ipv6routes + routingtable = &c.ipv6remotes routingcache = &c.ipv6cache } else if prefixsize == net.IPv4len*8 { - routingtable = &c.ipv4routes + routingtable = &c.ipv4remotes routingcache = &c.ipv4cache } else { return errors.New("Unexpected prefix size") diff --git a/src/tuntap/iface.go b/src/tuntap/iface.go index 670f7829..55ddd1b4 100644 --- a/src/tuntap/iface.go +++ b/src/tuntap/iface.go @@ -21,25 +21,50 @@ func (tun *TunAdapter) writer() error { if n == 0 { continue } - if tun.iface.IsTAP() { - var dstAddr address.Address - if b[0]&0xf0 == 0x60 { - if len(b) < 40 { - //panic("Tried to send a packet shorter than an IPv6 header...") - util.PutBytes(b) - continue - } - copy(dstAddr[:16], b[24:]) - } else if b[0]&0xf0 == 0x40 { - if len(b) < 20 { - //panic("Tried to send a packet shorter than an IPv4 header...") - util.PutBytes(b) - continue - } - copy(dstAddr[:4], b[16:]) - } else { - return errors.New("Invalid address family") + var dstAddr address.Address + var addrlen int + // Check whether the packet is IPv4, IPv6 or neither + if b[0]&0xf0 == 0x60 { + // IPv6 packet found + if len(b) < 40 { + // Packet was too short + util.PutBytes(b) + continue } + // Extract the destination IPv6 address + copy(dstAddr[:16], b[24:]) + addrlen = 16 + } else if b[0]&0xf0 == 0x40 { + // IPv4 packet found + if len(b) < 20 { + // Packet was too short + util.PutBytes(b) + continue + } + // Extract the destination IPv4 address + copy(dstAddr[:4], b[16:]) + addrlen = 4 + } else { + // Neither IPv4 nor IPv6 + return errors.New("Invalid address family") + } + // Check the crypto-key routing rules next + if tun.ckr.isEnabled() { + if !tun.ckr.isValidLocalAddress(dstAddr, addrlen) { + util.PutBytes(b) + continue + } + } else { + if addrlen != 16 { + util.PutBytes(b) + continue + } + if !bytes.Equal(tun.addr[:16], dstAddr[:16]) && !bytes.Equal(tun.subnet[:8], dstAddr[:8]) { + util.PutBytes(b) + continue + } + } + if tun.iface.IsTAP() { sendndp := func(dstAddr address.Address) { neigh, known := tun.icmpv6.getNeighbor(dstAddr) known = known && (time.Since(neigh.lastsolicitation).Seconds() < 30) @@ -69,7 +94,6 @@ func (tun *TunAdapter) writer() error { } else { // Nothing has been discovered, try to discover the destination sendndp(tun.addr) - } if peerknown { var proto ethernet.Ethertype @@ -187,26 +211,33 @@ func (tun *TunAdapter) readerPacketHandler(ch chan []byte) { tun.log.Traceln("Unknown packet type, dropping") continue } - if tun.ckr.isEnabled() && !tun.ckr.isValidSource(srcAddr, addrlen) { - // The packet had a source address that doesn't belong to us or our - // configured crypto-key routing source subnets - continue - } - if !dstAddr.IsValid() && !dstSnet.IsValid() { - if key, err := tun.ckr.getPublicKeyForAddress(dstAddr, addrlen); err == nil { - // A public key was found, get the node ID for the search - dstNodeID = crypto.GetNodeID(&key) - // Do a quick check to ensure that the node ID refers to a vaild - // Yggdrasil address or subnet - this might be superfluous - addr := *address.AddrForNodeID(dstNodeID) - copy(dstAddr[:], addr[:]) - copy(dstSnet[:], addr[:]) - // Are we certain we looked up a valid node? - if !dstAddr.IsValid() && !dstSnet.IsValid() { + if tun.ckr.isEnabled() { + if !tun.ckr.isValidLocalAddress(srcAddr, addrlen) { + continue + } + if !dstAddr.IsValid() && !dstSnet.IsValid() { + if key, err := tun.ckr.getPublicKeyForAddress(dstAddr, addrlen); err == nil { + // A public key was found, get the node ID for the search + dstNodeID = crypto.GetNodeID(&key) + // Do a quick check to ensure that the node ID refers to a vaild + // Yggdrasil address or subnet - this might be superfluous + addr := *address.AddrForNodeID(dstNodeID) + copy(dstAddr[:], addr[:]) + copy(dstSnet[:], addr[:]) + // Are we certain we looked up a valid node? + if !dstAddr.IsValid() && !dstSnet.IsValid() { + continue + } + } else { + // No public key was found in the CKR table so we've exhausted our options continue } - } else { - // No public key was found in the CKR table so we've exhausted our options + } + } else { + if addrlen != 16 { + continue + } + if !dstAddr.IsValid() && !dstSnet.IsValid() { continue } } From 2b6462c8a9c73376f0849d81d050fae3736453d4 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Tue, 20 Aug 2019 09:38:27 +0100 Subject: [PATCH 2/4] Strict checking of Yggdrasil source/destination addresses --- src/tuntap/conn.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/tuntap/conn.go b/src/tuntap/conn.go index 61cdb2b4..d1dc60dc 100644 --- a/src/tuntap/conn.go +++ b/src/tuntap/conn.go @@ -1,6 +1,7 @@ package tuntap import ( + "bytes" "errors" "time" @@ -70,6 +71,17 @@ func (s *tunConn) reader() (err error) { return e } } else if len(bs) > 0 { + if bs[0]&0xf0 == 0x60 { + switch { + case bs[8] == 0x02 && !bytes.Equal(s.addr[:16], bs[8:24]): // source + case bs[8] == 0x03 && !bytes.Equal(s.snet[:8], bs[8:16]): // source + case bs[24] == 0x02 && !bytes.Equal(s.tun.addr[:16], bs[24:40]): // destination + case bs[24] == 0x03 && !bytes.Equal(s.tun.subnet[:8], bs[24:32]): // destination + util.PutBytes(bs) + continue + default: + } + } s.tun.send <- bs s.stillAlive() } else { @@ -96,6 +108,16 @@ func (s *tunConn) writer() error { if !ok { return errors.New("send closed") } + if bs[0]&0xf0 == 0x60 { + switch { + case bs[8] == 0x02 && !bytes.Equal(s.tun.addr[:16], bs[8:24]): // source + case bs[8] == 0x03 && !bytes.Equal(s.tun.subnet[:8], bs[8:16]): // source + case bs[24] == 0x02 && !bytes.Equal(s.addr[:16], bs[24:40]): // destination + case bs[24] == 0x03 && !bytes.Equal(s.snet[:8], bs[24:32]): // destination + continue + default: + } + } msg := yggdrasil.FlowKeyMessage{ FlowKey: util.GetFlowKey(bs), Message: bs, From b6e67bc0ba9b56d1cde8609617f4b0b23a57bc00 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Tue, 20 Aug 2019 09:38:46 +0100 Subject: [PATCH 3/4] Check CKR remotes when receiving traffic --- src/tuntap/ckr.go | 38 +++++++++++++++++++------------------- src/tuntap/iface.go | 21 +++++++++++++++++---- 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/tuntap/ckr.go b/src/tuntap/ckr.go index f93cb7cf..9ce0120b 100644 --- a/src/tuntap/ckr.go +++ b/src/tuntap/ckr.go @@ -27,9 +27,9 @@ type cryptokey struct { ipv6cache map[address.Address]cryptokey_route ipv4locals []net.IPNet ipv6locals []net.IPNet - mutexroutes sync.RWMutex + mutexremotes sync.RWMutex mutexcaches sync.RWMutex - mutexsources sync.RWMutex + mutexlocals sync.RWMutex } type cryptokey_route struct { @@ -65,10 +65,10 @@ func (c *cryptokey) configure() error { c.setEnabled(current.TunnelRouting.Enable) // Clear out existing routes - c.mutexroutes.Lock() + c.mutexremotes.Lock() c.ipv6remotes = make([]cryptokey_route, 0) c.ipv4remotes = make([]cryptokey_route, 0) - c.mutexroutes.Unlock() + c.mutexremotes.Unlock() // Add IPv6 routes for ipv6, pubkey := range current.TunnelRouting.IPv6RemoteSubnets { @@ -85,10 +85,10 @@ func (c *cryptokey) configure() error { } // Clear out existing sources - c.mutexsources.Lock() + c.mutexlocals.Lock() c.ipv6locals = make([]net.IPNet, 0) c.ipv4locals = make([]net.IPNet, 0) - c.mutexsources.Unlock() + c.mutexlocals.Unlock() // Add IPv6 sources c.ipv6locals = make([]net.IPNet, 0) @@ -130,8 +130,8 @@ func (c *cryptokey) isEnabled() bool { // matches either the current node's address, the node's routed subnet or the // list of subnets specified in ipv4locals/ipv6locals. func (c *cryptokey) isValidLocalAddress(addr address.Address, addrlen int) bool { - c.mutexsources.RLock() - defer c.mutexsources.RUnlock() + c.mutexlocals.RLock() + defer c.mutexlocals.RUnlock() ip := net.IP(addr[:addrlen]) @@ -175,8 +175,8 @@ func (c *cryptokey) isValidLocalAddress(addr address.Address, addrlen int) bool // Adds a source subnet, which allows traffic with these source addresses to // be tunnelled using crypto-key routing. func (c *cryptokey) addLocalSubnet(cidr string) error { - c.mutexsources.Lock() - defer c.mutexsources.Unlock() + c.mutexlocals.Lock() + defer c.mutexlocals.Unlock() // Is the CIDR we've been given valid? _, ipnet, err := net.ParseCIDR(cidr) @@ -215,9 +215,9 @@ func (c *cryptokey) addLocalSubnet(cidr string) error { // Adds a destination route for the given CIDR to be tunnelled to the node // with the given BoxPubKey. func (c *cryptokey) addRemoteSubnet(cidr string, dest string) error { - c.mutexroutes.Lock() + c.mutexremotes.Lock() c.mutexcaches.Lock() - defer c.mutexroutes.Unlock() + defer c.mutexremotes.Unlock() defer c.mutexcaches.Unlock() // Is the CIDR we've been given valid? @@ -323,8 +323,8 @@ func (c *cryptokey) getPublicKeyForAddress(addr address.Address, addrlen int) (c c.mutexcaches.RUnlock() - c.mutexroutes.RLock() - defer c.mutexroutes.RUnlock() + c.mutexremotes.RLock() + defer c.mutexremotes.RUnlock() // Check if the prefix is IPv4 or IPv6 if addrlen == net.IPv6len { @@ -366,14 +366,14 @@ func (c *cryptokey) getPublicKeyForAddress(addr address.Address, addrlen int) (c } // No route was found if we got to this point - return crypto.BoxPubKey{}, errors.New(fmt.Sprintf("No route to %s", ip.String())) + return crypto.BoxPubKey{}, fmt.Errorf("no route to %s", ip.String()) } // Removes a source subnet, which allows traffic with these source addresses to // be tunnelled using crypto-key routing. func (c *cryptokey) removeLocalSubnet(cidr string) error { - c.mutexsources.Lock() - defer c.mutexsources.Unlock() + c.mutexlocals.Lock() + defer c.mutexlocals.Unlock() // Is the CIDR we've been given valid? _, ipnet, err := net.ParseCIDR(cidr) @@ -410,9 +410,9 @@ func (c *cryptokey) removeLocalSubnet(cidr string) error { // Removes a destination route for the given CIDR to be tunnelled to the node // with the given BoxPubKey. func (c *cryptokey) removeRemoteSubnet(cidr string, dest string) error { - c.mutexroutes.Lock() + c.mutexremotes.Lock() c.mutexcaches.Lock() - defer c.mutexroutes.Unlock() + defer c.mutexremotes.Unlock() defer c.mutexcaches.Unlock() // Is the CIDR we've been given valid? diff --git a/src/tuntap/iface.go b/src/tuntap/iface.go index 55ddd1b4..f5b88133 100644 --- a/src/tuntap/iface.go +++ b/src/tuntap/iface.go @@ -21,6 +21,7 @@ func (tun *TunAdapter) writer() error { if n == 0 { continue } + var srcAddr address.Address var dstAddr address.Address var addrlen int // Check whether the packet is IPv4, IPv6 or neither @@ -31,8 +32,9 @@ func (tun *TunAdapter) writer() error { util.PutBytes(b) continue } - // Extract the destination IPv6 address - copy(dstAddr[:16], b[24:]) + // Extract the IPv6 addresses + copy(srcAddr[:16], b[8:24]) + copy(dstAddr[:16], b[24:40]) addrlen = 16 } else if b[0]&0xf0 == 0x40 { // IPv4 packet found @@ -41,8 +43,9 @@ func (tun *TunAdapter) writer() error { util.PutBytes(b) continue } - // Extract the destination IPv4 address - copy(dstAddr[:4], b[16:]) + // Extract the IPv4 addresses + copy(srcAddr[:4], b[12:16]) + copy(dstAddr[:4], b[16:20]) addrlen = 4 } else { // Neither IPv4 nor IPv6 @@ -54,6 +57,16 @@ func (tun *TunAdapter) writer() error { util.PutBytes(b) continue } + if srcAddr[0] != 0x02 && srcAddr[0] != 0x03 { + // TODO: is this check useful? this doesn't actually guarantee that the + // packet came from the configured public key for that remote, just that + // it came from *a* configured remote. at this stage we have no ability + // to know which Conn or public key was involved + if _, err := tun.ckr.getPublicKeyForAddress(srcAddr, addrlen); err != nil { + util.PutBytes(b) + continue + } + } } else { if addrlen != 16 { util.PutBytes(b) From 4156aa30034f50f848725627dcf42cbdb6ef53bd Mon Sep 17 00:00:00 2001 From: Arceliar Date: Tue, 20 Aug 2019 18:10:08 -0500 Subject: [PATCH 4/4] move ckr checks into the tunConn code --- src/tuntap/ckr.go | 16 +----- src/tuntap/conn.go | 128 ++++++++++++++++++++++++++++++++++++++------ src/tuntap/iface.go | 97 ++++----------------------------- 3 files changed, 123 insertions(+), 118 deletions(-) diff --git a/src/tuntap/ckr.go b/src/tuntap/ckr.go index 9ce0120b..ad8c89d4 100644 --- a/src/tuntap/ckr.go +++ b/src/tuntap/ckr.go @@ -132,23 +132,9 @@ func (c *cryptokey) isEnabled() bool { func (c *cryptokey) isValidLocalAddress(addr address.Address, addrlen int) bool { c.mutexlocals.RLock() defer c.mutexlocals.RUnlock() - - ip := net.IP(addr[:addrlen]) - - if addrlen == net.IPv6len { - // Does this match our node's address? - if bytes.Equal(addr[:16], c.tun.addr[:16]) { - return true - } - - // Does this match our node's subnet? - if bytes.Equal(addr[:8], c.tun.subnet[:8]) { - return true - } - } - // Does it match a configured CKR source? if c.isEnabled() { + ip := net.IP(addr[:addrlen]) // Build our references to the routing sources var routingsources *[]net.IPNet diff --git a/src/tuntap/conn.go b/src/tuntap/conn.go index d1dc60dc..3fb7a544 100644 --- a/src/tuntap/conn.go +++ b/src/tuntap/conn.go @@ -6,6 +6,7 @@ import ( "time" "github.com/yggdrasil-network/yggdrasil-go/src/address" + "github.com/yggdrasil-network/yggdrasil-go/src/crypto" "github.com/yggdrasil-network/yggdrasil-go/src/util" "github.com/yggdrasil-network/yggdrasil-go/src/yggdrasil" "golang.org/x/net/icmp" @@ -71,16 +72,62 @@ func (s *tunConn) reader() (err error) { return e } } else if len(bs) > 0 { - if bs[0]&0xf0 == 0x60 { - switch { - case bs[8] == 0x02 && !bytes.Equal(s.addr[:16], bs[8:24]): // source - case bs[8] == 0x03 && !bytes.Equal(s.snet[:8], bs[8:16]): // source - case bs[24] == 0x02 && !bytes.Equal(s.tun.addr[:16], bs[24:40]): // destination - case bs[24] == 0x03 && !bytes.Equal(s.tun.subnet[:8], bs[24:32]): // destination - util.PutBytes(bs) - continue - default: + ipv4 := len(bs) > 20 && bs[0]&0xf0 == 0x40 + ipv6 := len(bs) > 40 && bs[0]&0xf0 == 0x60 + isCGA := true + // Check source addresses + switch { + case ipv6 && bs[8] == 0x02 && bytes.Equal(s.addr[:16], bs[8:24]): // source + case ipv6 && bs[8] == 0x03 && bytes.Equal(s.snet[:8], bs[8:16]): // source + default: + isCGA = false + } + // Check destiantion addresses + switch { + case ipv6 && bs[24] == 0x02 && bytes.Equal(s.tun.addr[:16], bs[24:40]): // destination + case ipv6 && bs[24] == 0x03 && bytes.Equal(s.tun.subnet[:8], bs[24:32]): // destination + default: + isCGA = false + } + // Decide how to handle the packet + var skip bool + switch { + case isCGA: // Allowed + case s.tun.ckr.isEnabled() && (ipv4 || ipv6): + var srcAddr address.Address + var dstAddr address.Address + var addrlen int + if ipv4 { + copy(srcAddr[:], bs[12:16]) + copy(dstAddr[:], bs[16:20]) + addrlen = 4 } + if ipv6 { + copy(srcAddr[:], bs[8:24]) + copy(dstAddr[:], bs[24:40]) + addrlen = 16 + } + if !s.tun.ckr.isValidLocalAddress(dstAddr, addrlen) { + // The destination address isn't in our CKR allowed range + skip = true + } else if key, err := s.tun.ckr.getPublicKeyForAddress(srcAddr, addrlen); err == nil { + srcNodeID := crypto.GetNodeID(&key) + if s.conn.RemoteAddr() == *srcNodeID { + // This is the one allowed CKR case, where source and destination addresses are both good + } else { + // The CKR key associated with this address doesn't match the sender's NodeID + skip = true + } + } else { + // We have no CKR route for this source address + skip = true + } + default: + skip = true + } + if skip { + util.PutBytes(bs) + continue } s.tun.send <- bs s.stillAlive() @@ -108,15 +155,62 @@ func (s *tunConn) writer() error { if !ok { return errors.New("send closed") } - if bs[0]&0xf0 == 0x60 { - switch { - case bs[8] == 0x02 && !bytes.Equal(s.tun.addr[:16], bs[8:24]): // source - case bs[8] == 0x03 && !bytes.Equal(s.tun.subnet[:8], bs[8:16]): // source - case bs[24] == 0x02 && !bytes.Equal(s.addr[:16], bs[24:40]): // destination - case bs[24] == 0x03 && !bytes.Equal(s.snet[:8], bs[24:32]): // destination - continue - default: + v4 := len(bs) > 20 && bs[0]&0xf0 == 0x40 + v6 := len(bs) > 40 && bs[0]&0xf0 == 0x60 + isCGA := true + // Check source addresses + switch { + case v6 && bs[8] == 0x02 && bytes.Equal(s.tun.addr[:16], bs[8:24]): // source + case v6 && bs[8] == 0x03 && bytes.Equal(s.tun.subnet[:8], bs[8:16]): // source + default: + isCGA = false + } + // Check destiantion addresses + switch { + case v6 && bs[24] == 0x02 && bytes.Equal(s.addr[:16], bs[24:40]): // destination + case v6 && bs[24] == 0x03 && bytes.Equal(s.snet[:8], bs[24:32]): // destination + default: + isCGA = false + } + // Decide how to handle the packet + var skip bool + switch { + case isCGA: // Allowed + case s.tun.ckr.isEnabled() && (v4 || v6): + var srcAddr address.Address + var dstAddr address.Address + var addrlen int + if v4 { + copy(srcAddr[:], bs[12:16]) + copy(dstAddr[:], bs[16:20]) + addrlen = 4 } + if v6 { + copy(srcAddr[:], bs[8:24]) + copy(dstAddr[:], bs[24:40]) + addrlen = 16 + } + if !s.tun.ckr.isValidLocalAddress(srcAddr, addrlen) { + // The source address isn't in our CKR allowed range + skip = true + } else if key, err := s.tun.ckr.getPublicKeyForAddress(dstAddr, addrlen); err == nil { + dstNodeID := crypto.GetNodeID(&key) + if s.conn.RemoteAddr() == *dstNodeID { + // This is the one allowed CKR case, where source and destination addresses are both good + } else { + // The CKR key associated with this address doesn't match the sender's NodeID + skip = true + } + } else { + // We have no CKR route for this destination address... why do we have the packet in the first place? + skip = true + } + default: + skip = true + } + if skip { + util.PutBytes(bs) + continue } msg := yggdrasil.FlowKeyMessage{ FlowKey: util.GetFlowKey(bs), diff --git a/src/tuntap/iface.go b/src/tuntap/iface.go index f5b88133..35657a9f 100644 --- a/src/tuntap/iface.go +++ b/src/tuntap/iface.go @@ -2,7 +2,6 @@ package tuntap import ( "bytes" - "errors" "net" "time" @@ -21,62 +20,6 @@ func (tun *TunAdapter) writer() error { if n == 0 { continue } - var srcAddr address.Address - var dstAddr address.Address - var addrlen int - // Check whether the packet is IPv4, IPv6 or neither - if b[0]&0xf0 == 0x60 { - // IPv6 packet found - if len(b) < 40 { - // Packet was too short - util.PutBytes(b) - continue - } - // Extract the IPv6 addresses - copy(srcAddr[:16], b[8:24]) - copy(dstAddr[:16], b[24:40]) - addrlen = 16 - } else if b[0]&0xf0 == 0x40 { - // IPv4 packet found - if len(b) < 20 { - // Packet was too short - util.PutBytes(b) - continue - } - // Extract the IPv4 addresses - copy(srcAddr[:4], b[12:16]) - copy(dstAddr[:4], b[16:20]) - addrlen = 4 - } else { - // Neither IPv4 nor IPv6 - return errors.New("Invalid address family") - } - // Check the crypto-key routing rules next - if tun.ckr.isEnabled() { - if !tun.ckr.isValidLocalAddress(dstAddr, addrlen) { - util.PutBytes(b) - continue - } - if srcAddr[0] != 0x02 && srcAddr[0] != 0x03 { - // TODO: is this check useful? this doesn't actually guarantee that the - // packet came from the configured public key for that remote, just that - // it came from *a* configured remote. at this stage we have no ability - // to know which Conn or public key was involved - if _, err := tun.ckr.getPublicKeyForAddress(srcAddr, addrlen); err != nil { - util.PutBytes(b) - continue - } - } - } else { - if addrlen != 16 { - util.PutBytes(b) - continue - } - if !bytes.Equal(tun.addr[:16], dstAddr[:16]) && !bytes.Equal(tun.subnet[:8], dstAddr[:8]) { - util.PutBytes(b) - continue - } - } if tun.iface.IsTAP() { sendndp := func(dstAddr address.Address) { neigh, known := tun.icmpv6.getNeighbor(dstAddr) @@ -86,6 +29,7 @@ func (tun *TunAdapter) writer() error { } } peermac := net.HardwareAddr{0x00, 0x00, 0x00, 0x00, 0x00, 0x00} + var dstAddr address.Address var peerknown bool if b[0]&0xf0 == 0x40 { dstAddr = tun.addr @@ -183,10 +127,7 @@ func (tun *TunAdapter) readerPacketHandler(ch chan []byte) { // From the IP header, work out what our source and destination addresses // and node IDs are. We will need these in order to work out where to send // the packet - var srcAddr address.Address var dstAddr address.Address - var dstNodeID *crypto.NodeID - var dstNodeIDMask *crypto.NodeID var dstSnet address.Subnet var addrlen int n := len(bs) @@ -203,7 +144,6 @@ func (tun *TunAdapter) readerPacketHandler(ch chan []byte) { } // IPv6 address addrlen = 16 - copy(srcAddr[:addrlen], bs[8:]) copy(dstAddr[:addrlen], bs[24:]) copy(dstSnet[:addrlen/2], bs[24:]) } else if bs[0]&0xf0 == 0x40 { @@ -217,7 +157,6 @@ func (tun *TunAdapter) readerPacketHandler(ch chan []byte) { } // IPv4 address addrlen = 4 - copy(srcAddr[:addrlen], bs[12:]) copy(dstAddr[:addrlen], bs[16:]) } else { // Unknown address length or protocol, so drop the packet and ignore it @@ -225,36 +164,22 @@ func (tun *TunAdapter) readerPacketHandler(ch chan []byte) { continue } if tun.ckr.isEnabled() { - if !tun.ckr.isValidLocalAddress(srcAddr, addrlen) { - continue - } - if !dstAddr.IsValid() && !dstSnet.IsValid() { + if addrlen != 16 || (!dstAddr.IsValid() && !dstSnet.IsValid()) { if key, err := tun.ckr.getPublicKeyForAddress(dstAddr, addrlen); err == nil { // A public key was found, get the node ID for the search - dstNodeID = crypto.GetNodeID(&key) - // Do a quick check to ensure that the node ID refers to a vaild - // Yggdrasil address or subnet - this might be superfluous - addr := *address.AddrForNodeID(dstNodeID) - copy(dstAddr[:], addr[:]) - copy(dstSnet[:], addr[:]) - // Are we certain we looked up a valid node? - if !dstAddr.IsValid() && !dstSnet.IsValid() { - continue - } - } else { - // No public key was found in the CKR table so we've exhausted our options - continue + dstNodeID := crypto.GetNodeID(&key) + dstAddr = *address.AddrForNodeID(dstNodeID) + dstSnet = *address.SubnetForNodeID(dstNodeID) + addrlen = 16 } } - } else { - if addrlen != 16 { - continue - } - if !dstAddr.IsValid() && !dstSnet.IsValid() { - continue - } + } + if addrlen != 16 || (!dstAddr.IsValid() && !dstSnet.IsValid()) { + // Couldn't find this node's ygg IP + continue } // Do we have an active connection for this node address? + var dstNodeID, dstNodeIDMask *crypto.NodeID tun.mutex.RLock() session, isIn := tun.addrToConn[dstAddr] if !isIn || session == nil {