gcash / bchd

An alternative full node bitcoin cash implementation written in Go (golang)
ISC License
280 stars 101 forks source link

better remote node selection [split network attack] #149

Closed nshopik closed 5 years ago

nshopik commented 5 years ago

Interesting read about splinting network https://ripe77.ripe.net/presentations/19-ripe_15_10.pdf

I didn't personally check bchd but I assume remote node selection just simple random from seed (same as bitcoind), but there high chance it could select all 8 nodes within same /24 range. IPv6 need different network mask for that /48 or even /32 to be sure

Maybe we should improve that process and avoid connecting nodes that close within each other in term IP space, thoughts?

zquestz commented 5 years ago

@nshopik this is something I would love, basically if there are enough nodes in different ip ranges we should prioritize those, and only fall back to connecting to nodes in the same ip range. In addition I think it would be good to increase the default number of outgoing connections to something slightly higher than 8.

nshopik commented 5 years ago

Also its probably wise treat IPv4 connections and IPv6 connections count as separate networks since they are.

I think base number of outgoing connection could stay within 8-12.

While typical node doesn't have problems to handle 100+ connections and most nodes doesn't run on low memory embedded devices where you count every byte of memory which even idle TCP connection would eat. But if we able to provide quality over quantity we probably could stay within sane amount of tcp connections instead just raising it for sake "just to be safe".

zquestz commented 5 years ago

Well we should at least allow the outgoing connection limit to be easily changed. Perhaps as a configuration option. Right now I want more than 8 and there is no way to do that without using bchctl.

zquestz commented 5 years ago

This already is happening. After looking at the code we restrict connections to subnets we are already connected to.

func GroupKey(na *wire.NetAddress) string {
    if IsLocal(na) {
        return "local"
    }
    if !IsRoutable(na) {
        return "unroutable"
    }
    if IsIPv4(na) {
        return na.IP.Mask(net.CIDRMask(16, 32)).String()
    }
    if IsRFC6145(na) || IsRFC6052(na) {
        // last four bytes are the ip address
        ip := na.IP[12:16]
        return ip.Mask(net.CIDRMask(16, 32)).String()
    }

    if IsRFC3964(na) {
        ip := na.IP[2:6]
        return ip.Mask(net.CIDRMask(16, 32)).String()

    }
    if IsRFC4380(na) {
        // teredo tunnels have the last 4 bytes as the v4 address XOR
        // 0xff.
        ip := net.IP(make([]byte, 4))
        for i, ipByte := range na.IP[12:16] {
            ip[i] = ipByte ^ 0xff
        }
        return ip.Mask(net.CIDRMask(16, 32)).String()
    }
    if IsOnionCatTor(na) {
        // group is keyed off the first 4 bits of the actual onion key.
        return fmt.Sprintf("tor:%d", na.IP[6]&((1<<4)-1))
    }

    // OK, so now we know ourselves to be a IPv6 address.
    // bitcoind uses /32 for everything, except for Hurricane Electric's
    // (he.net) IP range, which it uses /36 for.
    bits := 32
    if heNet.Contains(na.IP) {
        bits = 36
    }

    return na.IP.Mask(net.CIDRMask(bits, 128)).String()
}