m13253 / dns-over-https

High performance DNS over HTTPS client & server
https://developers.google.com/speed/public-dns/docs/dns-over-https
MIT License
1.97k stars 221 forks source link

use golang net lib to parse google json param [ecs] #84

Open sanyo0714 opened 4 years ago

sanyo0714 commented 4 years ago

use net.ParseCIDR to parse edns_client_subnet.

m13253 commented 4 years ago

Thank you for your contribution!

I remember I once used net.ParseCIDR for this, but later I rewrote it without net.ParseCIDR. I forget the exact reason why net.ParseCIDR did not work. Perhaps it's because it couldn't handle IPv4-mapped-IPv6 well, or any other security or functional problems.

Perhaps the problem which prevented me from using net.ParseCIDR has already been fixed by Golang. So would you please check whether the new code can handle any strange inputs you can come up with?

Thank you again!

sanyo0714 commented 4 years ago

test for net.ParseCIDR

func TestParseCIDR(t *testing.T) {
    parseIp("2001:db8::/0")
    parseIp("2001:db8::/56")
    parseIp("2001:db8::/129")
    parseIp("2001:db8::")

    parseIp("127.0.0.1/0")
    parseIp("127.0.0.1/24")
    parseIp("127.0.0.1/33")
    parseIp("127.0.0.1")

    parseIp("test")
    parseIp("test/0")
    parseIp("test/24")
    parseIp("test/34")
    parseIp("test/56")
    parseIp("test/129")
}

func parseIp(ednsClientSubnet string) {
    ip, ipNet, err := net.ParseCIDR(ednsClientSubnet)

    if err != nil {
        log.Println(fmt.Sprintf("[error] ecs:%s ip:[%v]  ipNet:[%v]  err:[%v]\n", ednsClientSubnet, ip, ipNet, err))
        return
    }

    netmask, ipLen := ipNet.Mask.Size()
    log.Println(fmt.Sprintf("[info ] ecs:%s ip:[%v]  ipNet:[%v]  netmask:[%v]  ipLen:[%v]   err:[%v]\n", ednsClientSubnet, ip, ipNet, netmask, ipLen, err))
}

The results of

=== RUN   TestParseCIDR
2020/07/30 10:06:11 [info ] ecs:2001:db8::/0 ip:[2001:db8::]  ipNet:[::/0]  netmask:[0]  ipLen:[128]   err:[<nil>]
2020/07/30 10:06:11 [info ] ecs:2001:db8::/56 ip:[2001:db8::]  ipNet:[2001:db8::/56]  netmask:[56]  ipLen:[128]   err:[<nil>]
2020/07/30 10:06:11 [error] ecs:2001:db8::/129 ip:[<nil>]  ipNet:[<nil>]  err:[invalid CIDR address: 2001:db8::/129]
2020/07/30 10:06:11 [error] ecs:2001:db8:: ip:[<nil>]  ipNet:[<nil>]  err:[invalid CIDR address: 2001:db8::]
2020/07/30 10:06:11 [info ] ecs:127.0.0.1/0 ip:[127.0.0.1]  ipNet:[0.0.0.0/0]  netmask:[0]  ipLen:[32]   err:[<nil>]
2020/07/30 10:06:11 [info ] ecs:127.0.0.1/24 ip:[127.0.0.1]  ipNet:[127.0.0.0/24]  netmask:[24]  ipLen:[32]   err:[<nil>]
2020/07/30 10:06:11 [error] ecs:127.0.0.1/33 ip:[<nil>]  ipNet:[<nil>]  err:[invalid CIDR address: 127.0.0.1/33]
2020/07/30 10:06:11 [error] ecs:127.0.0.1 ip:[<nil>]  ipNet:[<nil>]  err:[invalid CIDR address: 127.0.0.1]
2020/07/30 10:06:11 [error] ecs:test ip:[<nil>]  ipNet:[<nil>]  err:[invalid CIDR address: test]
2020/07/30 10:06:11 [error] ecs:test/0 ip:[<nil>]  ipNet:[<nil>]  err:[invalid CIDR address: test/0]
2020/07/30 10:06:11 [error] ecs:test/24 ip:[<nil>]  ipNet:[<nil>]  err:[invalid CIDR address: test/24]
2020/07/30 10:06:11 [error] ecs:test/34 ip:[<nil>]  ipNet:[<nil>]  err:[invalid CIDR address: test/34]
2020/07/30 10:06:11 [error] ecs:test/56 ip:[<nil>]  ipNet:[<nil>]  err:[invalid CIDR address: test/56]
2020/07/30 10:06:11 [error] ecs:test/129 ip:[<nil>]  ipNet:[<nil>]  err:[invalid CIDR address: test/129]
--- PASS: TestParseCIDR (0.00s)
PASS
m13253 commented 4 years ago

The test fails with IPv4-mapped-IPv6 addresses:

parseIp("::ffff:7f00:1/0")
parseIp("::ffff:7f00:1/120")
parseIp("::ffff:7f00:1")
parseIp("127.0.0.1/0")
parseIp("127.0.0.1/24")
parseIp("127.0.0.1")

net.ParseCIDR cannot distinguish between these two cases. And if we write additional logic to address this issue, we might write more code than the original split-then-parse version.

sanyo0714 commented 4 years ago
func TestEdns0SubnetParseCIDR(t *testing.T) {
    // init dns Msg
    msg := new(dns.Msg)
    msg.Id = dns.Id()
    msg.SetQuestion(dns.Fqdn("example.com"), 1)

    // init edns0Subnet
    edns0Subnet := new(dns.EDNS0_SUBNET)
    edns0Subnet.Code = dns.EDNS0SUBNET
    edns0Subnet.SourceScope = 0

    // init opt
    opt := new(dns.OPT)
    opt.Hdr.Name = "."
    opt.Hdr.Rrtype = dns.TypeOPT
    opt.SetUDPSize(dns.DefaultMsgSize)

    opt.Option = append(opt.Option, edns0Subnet)
    msg.Extra = append(msg.Extra, opt)

    // ------------------------------------------
    var ipLen int
    //edns0Subnet.Address, edns0Subnet.SourceNetmask, ipLen = parseSubnet("::ffff:7f00:1/120")
    edns0Subnet.Address, edns0Subnet.SourceNetmask, ipLen = parseSubnet("127.0.0.1/24")

    switch ipLen {
    case 128: // ipv6
        edns0Subnet.Family = 2
    case 32: // ipv4
        edns0Subnet.Family = 1
    default:

    }
    log.Println(msg.Pack())

    // ------127.0.0.1/24-----
    // [143 29 1 0 0 1 0 0 0 0 0 1 7 101 120 97 109 112 108 101 3 99 111 109 0 0 1 0 1 0
    // opt start   0 41 16 0 0 0 0 0 0 11
    // subnet start 0 8 0 7 0 1 24 0
    // client subnet start 127 0 0]

    // -----::ffff:7f00:1/120----
    // [111 113 1 0 0 1 0 0 0 0 0 1 7 101 120 97 109 112 108 101 3 99 111 109 0 0 1 0 1 0
    // opt start  0 41 16 0 0 0 0 0 0 23
    // subnet start  0 8 0 19 0 2 120 0
    // client subnet start 0 0 0 0 0 0 0 0 0 0 255 255 127 0 0]

}

func parseSubnet(ednsClientSubnet string) (net.IP, uint8, int) {
    ip, ipNet, err := net.ParseCIDR(ednsClientSubnet)

    if err != nil {
        return nil, 0, 0
    }

    netmask, ipLen := ipNet.Mask.Size()
    return ip, uint8(netmask), ipLen
}

The result of msg.Pack() is ok~

// ------127.0.0.1/24-----
// [143 29 1 0 0 1 0 0 0 0 0 1 7 101 120 97 109 112 108 101 3 99 111 109 0 0 1 0 1 0
// opt start   0 41 16 0 0 0 0 0 0 11
// subnet start 0 8 0 7 0 1 24 0
// client subnet start 127 0 0]

// -----::ffff:7f00:1/120----
// [111 113 1 0 0 1 0 0 0 0 0 1 7 101 120 97 109 112 108 101 3 99 111 109 0 0 1 0 1 0
// opt start  0 41 16 0 0 0 0 0 0 23
// subnet start  0 8 0 19 0 2 120 0
// client subnet start 0 0 0 0 0 0 0 0 0 0 255 255 127 0 0]
m13253 commented 4 years ago

The result of msg.Pack() is ok~

A recent pull does not pass the test. Please update your code to fix the problem.