From 3e9edd9eac774f54eaa2041fb703ddb12dacd4b4 Mon Sep 17 00:00:00 2001 From: Andrey Meshkov Date: Wed, 27 Jan 2021 15:21:39 +0300 Subject: [PATCH] Pull request: Minor code improvement - added tests for safebrowsing and parental control services Merge in DNS/adguard-home from sbpctests to master Squashed commit of the following: commit 56df4dac20fcdba319543e8b15d9420ec247b68d Merge: 0dcec1d8 1c1a7683 Author: Andrey Meshkov Date: Wed Jan 27 15:06:53 2021 +0300 Merge branch 'master' into sbpctests commit 0dcec1d85362142b51443ab2ae0de0b4fd2a9676 Author: Andrey Meshkov Date: Wed Jan 27 14:13:18 2021 +0300 applied suggestion commit 166b659458a68e0c395028aa3dea580c5118544c Merge: e15e5cf6 dde5d798 Author: Andrey Meshkov Date: Wed Jan 27 14:02:47 2021 +0300 merge commit e15e5cf6a7a9cbf4ffdb24197fc7062e931bdf34 Author: Andrey Meshkov Date: Wed Jan 27 14:00:27 2021 +0300 Fix review comments commit dde5d798faa0f20b41f9252acfd4a83f9b8b19e3 Author: Andrey Meshkov Date: Wed Jan 27 13:10:03 2021 +0300 fixed comment style commit 3a77bc89cc0d1994d7a9de450b2796b6c27bd3dc Author: Andrey Meshkov Date: Wed Jan 27 12:57:11 2021 +0300 Minor code improvement - added tests for safebrowsing and parental control services --- internal/dnsfilter/safebrowsing.go | 1 - internal/dnsfilter/safebrowsing_test.go | 118 ++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 1 deletion(-) diff --git a/internal/dnsfilter/safebrowsing.go b/internal/dnsfilter/safebrowsing.go index cec3081b..ad99bf80 100644 --- a/internal/dnsfilter/safebrowsing.go +++ b/internal/dnsfilter/safebrowsing.go @@ -200,7 +200,6 @@ func (c *sbCtx) processTXT(resp *dns.Msg) (bool, [][]byte) { log.Debug("%s: received hashes for %s: %v", c.svc, c.host, txt.Txt) for _, t := range txt.Txt { - if len(t) != 32*2 { continue } diff --git a/internal/dnsfilter/safebrowsing_test.go b/internal/dnsfilter/safebrowsing_test.go index 060664b5..21ea2277 100644 --- a/internal/dnsfilter/safebrowsing_test.go +++ b/internal/dnsfilter/safebrowsing_test.go @@ -2,6 +2,7 @@ package dnsfilter import ( "crypto/sha256" + "encoding/hex" "strings" "testing" @@ -134,3 +135,120 @@ func TestSBPC_checkErrorUpstream(t *testing.T) { _, err = d.checkParental("smthng.com") assert.NotNil(t, err) } + +// testSbUpstream implements upstream.Upstream interface for replacing real +// upstream in tests. +type testSbUpstream struct { + hostname string + block bool + requestsCount int +} + +// Exchange returns a message depending on the upstream settings (hostname, block) +func (u *testSbUpstream) Exchange(r *dns.Msg) (*dns.Msg, error) { + u.requestsCount++ + + hash := sha256.Sum256([]byte(u.hostname)) + prefix := hash[0:2] + hashToReturn := hex.EncodeToString(prefix) + strings.Repeat("ab", 28) + if u.block { + hashToReturn = hex.EncodeToString(hash[:]) + } + + m := &dns.Msg{} + m.Answer = []dns.RR{ + &dns.TXT{ + Hdr: dns.RR_Header{ + Name: r.Question[0].Name, + }, + Txt: []string{ + hashToReturn, + }, + }, + } + + return m, nil +} + +func (u *testSbUpstream) Address() string { + return "" +} + +func TestSBPC_sbValidResponse(t *testing.T) { + d := NewForTest(&Config{SafeBrowsingEnabled: true}, nil) + defer d.Close() + + ups := &testSbUpstream{} + d.safeBrowsingUpstream = ups + d.parentalUpstream = ups + + // Prepare the upstream + ups.hostname = "example.org" + ups.block = false + ups.requestsCount = 0 + + // First - check that the request is not blocked + res, err := d.checkSafeBrowsing("example.org") + assert.Nil(t, err) + assert.False(t, res.IsFiltered) + + // Check the cache state, check that the response is now cached + assert.Equal(t, 1, gctx.safebrowsingCache.Stats().Count) + assert.Equal(t, 0, gctx.safebrowsingCache.Stats().Hit) + + // There was one request to an upstream + assert.Equal(t, 1, ups.requestsCount) + + // Now make the same request to check that the cache was used + res, err = d.checkSafeBrowsing("example.org") + assert.Nil(t, err) + assert.False(t, res.IsFiltered) + + // Check the cache state, it should've been used + assert.Equal(t, 1, gctx.safebrowsingCache.Stats().Count) + assert.Equal(t, 1, gctx.safebrowsingCache.Stats().Hit) + + // Check that there were no additional requests + assert.Equal(t, 1, ups.requestsCount) +} + +func TestSBPC_pcBlockedResponse(t *testing.T) { + d := NewForTest(&Config{SafeBrowsingEnabled: true}, nil) + defer d.Close() + + ups := &testSbUpstream{} + d.safeBrowsingUpstream = ups + d.parentalUpstream = ups + + // Prepare the upstream + // Make sure that the upstream will return a response that matches the queried domain + ups.hostname = "example.com" + ups.block = true + ups.requestsCount = 0 + + // Make a lookup + res, err := d.checkParental("example.com") + assert.Nil(t, err) + assert.True(t, res.IsFiltered) + assert.Len(t, res.Rules, 1) + + // Check the cache state, check that the response is now cached + assert.Equal(t, 1, gctx.parentalCache.Stats().Count) + assert.Equal(t, 1, gctx.parentalCache.Stats().Hit) + + // There was one request to an upstream + assert.Equal(t, 1, ups.requestsCount) + + // Make a second lookup for the same domain + res, err = d.checkParental("example.com") + assert.Nil(t, err) + assert.True(t, res.IsFiltered) + assert.Len(t, res.Rules, 1) + + // Check the cache state, it should've been used + assert.Equal(t, 1, gctx.parentalCache.Stats().Count) + assert.Equal(t, 2, gctx.parentalCache.Stats().Hit) + + // Check that there were no additional requests + assert.Equal(t, 1, ups.requestsCount) +}