From 45e032754808fb1dfb896d01a17ce4c126a466dc Mon Sep 17 00:00:00 2001 From: Andrey Meshkov Date: Tue, 18 Dec 2018 13:03:57 +0300 Subject: [PATCH] Added Client.AdjustPayloadSize field and more tests --- dnscrypt.go | 44 ++++++++++++++++++------------ dnscrypt_test.go | 71 ++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 85 insertions(+), 30 deletions(-) diff --git a/dnscrypt.go b/dnscrypt.go index 1c3bcb4..384371d 100644 --- a/dnscrypt.go +++ b/dnscrypt.go @@ -41,22 +41,24 @@ var ( ) const ( - clientMagicLen = 8 - nonceSize = xsecretbox.NonceSize - halfNonceSize = xsecretbox.NonceSize / 2 - tagSize = xsecretbox.TagSize - publicKeySize = 32 - queryOverhead = clientMagicLen + publicKeySize + halfNonceSize + tagSize - responseOverhead = len(serverMagic) + nonceSize + tagSize + clientMagicLen = 8 + nonceSize = xsecretbox.NonceSize + halfNonceSize = xsecretbox.NonceSize / 2 + tagSize = xsecretbox.TagSize + publicKeySize = 32 + queryOverhead = clientMagicLen + publicKeySize + halfNonceSize + tagSize + // is a variable length, initially set to 256 bytes, and + // must be a multiple of 64 bytes. (see https://dnscrypt.info/protocol) // Some servers do not work if padded length is less than 256. Example: Quad9 minUdpQuestionSize = 256 ) // Client contains parameters for a DNSCrypt client type Client struct { - Proto string // Protocol ("udp" or "tcp") - Timeout time.Duration // Timeout for read/write operations + Proto string // Protocol ("udp" or "tcp"). Empty means "udp". + Timeout time.Duration // Timeout for read/write operations + AdjustPayloadSize bool // If true, the client will automatically add a EDNS0 RR that will advertise a larger buffer } // CertInfo contains DnsCrypt server certificate data retrieved from the server @@ -113,7 +115,12 @@ func (c *Client) DialStamp(stamp dnsstamps.ServerStamp) (*ServerInfo, time.Durat curve25519.ScalarBaseMult(&serverInfo.PublicKey, &serverInfo.SecretKey) // Set the provider properties - serverInfo.Proto = c.Proto + proto := c.Proto + if proto == "" { + proto = "udp" + } + + serverInfo.Proto = proto serverInfo.ServerPublicKey = stamp.ServerPk serverInfo.ServerAddress = stamp.ServerAddrStr serverInfo.ProviderName = stamp.ProviderName @@ -138,7 +145,7 @@ func (c *Client) DialStamp(stamp dnsstamps.ServerStamp) (*ServerInfo, time.Durat func (c *Client) Exchange(m *dns.Msg, s *ServerInfo) (*dns.Msg, time.Duration, error) { now := time.Now() - conn, err := net.Dial(c.Proto, s.ServerAddress) + conn, err := net.Dial(s.Proto, s.ServerAddress) if err != nil { return nil, 0, err } @@ -158,7 +165,9 @@ func (c *Client) Exchange(m *dns.Msg, s *ServerInfo) (*dns.Msg, time.Duration, e func (c *Client) ExchangeConn(m *dns.Msg, s *ServerInfo, conn net.Conn) (*dns.Msg, time.Duration, error) { now := time.Now() - c.adjustPayloadSize(m) + if c.AdjustPayloadSize { + c.adjustPayloadSize(m) + } query, err := m.Pack() if err != nil { return nil, 0, err @@ -213,21 +222,20 @@ func (c *Client) ExchangeConn(m *dns.Msg, s *ServerInfo, conn net.Conn) (*dns.Ms // Adjusts the maximum payload size advertised in queries sent to upstream servers // See https://github.com/jedisct1/dnscrypt-proxy/blob/master/dnscrypt-proxy/plugin_get_set_payload_size.go -// TODO: I don't really understand why it is required :) +// See here also: https://github.com/jedisct1/dnscrypt-proxy/issues/667 func (c *Client) adjustPayloadSize(msg *dns.Msg) { - originalMaxPayloadSize := 512 - responseOverhead + originalMaxPayloadSize := dns.MinMsgSize edns0 := msg.IsEdns0() dnssec := false if edns0 != nil { - originalMaxPayloadSize = min(int(edns0.UDPSize())-responseOverhead, originalMaxPayloadSize) + originalMaxPayloadSize = int(edns0.UDPSize()) dnssec = edns0.Do() } var options *[]dns.EDNS0 - maxPayloadSize := maxDNSUDPPacketSize - responseOverhead - maxPayloadSize = min(maxDNSUDPPacketSize-responseOverhead, max(originalMaxPayloadSize, maxPayloadSize)) + maxPayloadSize := min(maxDNSUDPPacketSize, originalMaxPayloadSize) - if maxPayloadSize > 512 { + if maxPayloadSize > dns.MinMsgSize { var extra2 []dns.RR for _, extra := range msg.Extra { if extra.Header().Rrtype != dns.TypeOPT { diff --git a/dnscrypt_test.go b/dnscrypt_test.go index 9db6926..232fc87 100644 --- a/dnscrypt_test.go +++ b/dnscrypt_test.go @@ -42,61 +42,108 @@ func TestParseStamp(t *testing.T) { log.Println("") } +func TestInvalidStamp(t *testing.T) { + + client := Client{} + _, _, err := client.Dial("sdns://AQIAAAAAAAAAFDE") + if err == nil { + t.Fatalf("Dial must not have been possible") + } +} + +func TestTimeoutOnDialError(t *testing.T) { + + // AdGuard DNS pointing to a wrong IP + stampStr := "sdns://AQIAAAAAAAAADDguOC44Ljg6NTQ0MyDRK0fyUtzywrv4mRCG6vec5EldixbIoMQyLlLKPzkIcyIyLmRuc2NyeXB0LmRlZmF1bHQubnMxLmFkZ3VhcmQuY29t" + client := Client{Timeout: 300 * time.Millisecond} + + _, _, err := client.Dial(stampStr) + if err == nil { + t.Fatalf("Dial must not have been possible") + } + + if err, ok := err.(net.Error); !ok || !err.Timeout() { + t.Fatalf("Not the timeout error") + } +} + +func TestTimeoutOnDialExchange(t *testing.T) { + + // AdGuard DNS + stampStr := "sdns://AQIAAAAAAAAAFDE3Ni4xMDMuMTMwLjEzMDo1NDQzINErR_JS3PLCu_iZEIbq95zkSV2LFsigxDIuUso_OQhzIjIuZG5zY3J5cHQuZGVmYXVsdC5uczEuYWRndWFyZC5jb20" + client := Client{Timeout: 300 * time.Millisecond} + + serverInfo, _, err := client.Dial(stampStr) + if err != nil { + t.Fatalf("Could not establish connection with %s", stampStr) + } + + // Point it to an IP where there's no DNSCrypt server + serverInfo.ServerAddress = "8.8.8.8:5443" + req := dns.Msg{} + req.Id = dns.Id() + req.RecursionDesired = true + req.Question = []dns.Question{ + {Name: "google-public-dns-a.google.com.", Qtype: dns.TypeA, Qclass: dns.ClassINET}, + } + + // + _, _, err = client.Exchange(&req, serverInfo) + + if err == nil { + t.Fatalf("Exchange must not have been possible") + } + + if err, ok := err.(net.Error); !ok || !err.Timeout() { + t.Fatalf("Not the timeout error") + } +} + func TestDnsCryptResolver(t *testing.T) { stamps := []struct { stampStr string - udp bool }{ { // AdGuard DNS stampStr: "sdns://AQIAAAAAAAAAFDE3Ni4xMDMuMTMwLjEzMDo1NDQzINErR_JS3PLCu_iZEIbq95zkSV2LFsigxDIuUso_OQhzIjIuZG5zY3J5cHQuZGVmYXVsdC5uczEuYWRndWFyZC5jb20", - udp: true, }, { // AdGuard DNS Family stampStr: "sdns://AQIAAAAAAAAAFDE3Ni4xMDMuMTMwLjEzMjo1NDQzILgxXdexS27jIKRw3C7Wsao5jMnlhvhdRUXWuMm1AFq6ITIuZG5zY3J5cHQuZmFtaWx5Lm5zMS5hZGd1YXJkLmNvbQ", - udp: true, }, { // Cisco OpenDNS stampStr: "sdns://AQAAAAAAAAAADjIwOC42Ny4yMjAuMjIwILc1EUAgbyJdPivYItf9aR6hwzzI1maNDL4Ev6vKQ_t5GzIuZG5zY3J5cHQtY2VydC5vcGVuZG5zLmNvbQ", - udp: true, }, { // Cisco OpenDNS Family Shield stampStr: "sdns://AQAAAAAAAAAADjIwOC42Ny4yMjAuMTIzILc1EUAgbyJdPivYItf9aR6hwzzI1maNDL4Ev6vKQ_t5GzIuZG5zY3J5cHQtY2VydC5vcGVuZG5zLmNvbQ", - udp: true, }, { // Quad9 (anycast) dnssec/no-log/filter 9.9.9.9 stampStr: "sdns://AQMAAAAAAAAADDkuOS45Ljk6ODQ0MyBnyEe4yHWM0SAkVUO-dWdG3zTfHYTAC4xHA2jfgh2GPhkyLmRuc2NyeXB0LWNlcnQucXVhZDkubmV0", - udp: true, }, { // https://securedns.eu/ stampStr: "sdns://AQcAAAAAAAAAEzE0Ni4xODUuMTY3LjQzOjUzNTMgs6WXaRRXWwSJ4Z-unEPmefryjFcYlwAxf3u0likfsJUcMi5kbnNjcnlwdC1jZXJ0LnNlY3VyZWRucy5ldQ", - udp: true, }, { // Yandex DNS stampStr: "sdns://AQQAAAAAAAAAEDc3Ljg4LjguNzg6MTUzNTMg04TAccn3RmKvKszVe13MlxTUB7atNgHhrtwG1W1JYyciMi5kbnNjcnlwdC1jZXJ0LmJyb3dzZXIueWFuZGV4Lm5ldA", - udp: true, }, } for _, test := range stamps { - if test.udp { - checkDnsCryptServer(t, test.stampStr, "udp") - } + checkDnsCryptServer(t, test.stampStr, "") checkDnsCryptServer(t, test.stampStr, "tcp") } } func checkDnsCryptServer(t *testing.T, stampStr string, proto string) { - client := Client{Proto: proto, Timeout: 10 * time.Second} + client := Client{Proto: proto, Timeout: 10 * time.Second, AdjustPayloadSize: true} serverInfo, rtt, err := client.Dial(stampStr) if err != nil { t.Fatalf("Could not establish connection with %s", stampStr)