Merge: * dhcpd: check if subnet mask is correct

Close #887

* commit '79a5c920a40180b7291d94535e50017d98eb3a63':
  * dhcpd: check if subnet mask is correct

README.md

@@ -176,6 +176,7 @@ or you can use the Docker image from the `edge` tag, which is synced with the re
     * [Windows 32-bit](https://static.adguard.com/adguardhome/beta/AdGuardHome_Windows_386.zip)
     * [Linux 64-bit](https://static.adguard.com/adguardhome/beta/AdGuardHome_linux_amd64.tar.gz)
     * [Linux 32-bit](https://static.adguard.com/adguardhome/beta/AdGuardHome_linux_386.tar.gz)
+    * [FreeBSD 64-bit](https://static.adguard.com/adguardhome/beta/AdGuardHome_freebsd_amd64.tar.gz)
     * [64-bit ARM](https://static.adguard.com/adguardhome/beta/AdGuardHome_linux_arm64.tar.gz)
     * [MIPS](https://static.adguard.com/adguardhome/beta/AdGuardHome_linux_mips.tar.gz)
     * [MIPSLE](https://static.adguard.com/adguardhome/beta/AdGuardHome_linux_mipsle.tar.gz)

dhcpd/dhcpd.go

@@ -127,7 +127,7 @@ func (s *Server) setConfig(config ServerConfig) error {
 	}
 
 	subnet, err := parseIPv4(config.SubnetMask)
-	if err != nil {
+	if err != nil || !isValidSubnetMask(subnet) {
 		return wrapErrPrint(err, "Failed to parse subnet mask %s", config.SubnetMask)
 	}
 

dhcpd/dhcpd_test.go

@@ -197,3 +197,15 @@ func TestDB(t *testing.T) {
 
 	os.Remove("leases.db")
 }
+
+func TestIsValidSubnetMask(t *testing.T) {
+	if !isValidSubnetMask([]byte{255, 255, 255, 0}) {
+		t.Fatalf("isValidSubnetMask([]byte{255,255,255,0})")
+	}
+	if isValidSubnetMask([]byte{255, 255, 253, 0}) {
+		t.Fatalf("isValidSubnetMask([]byte{255,255,253,0})")
+	}
+	if isValidSubnetMask([]byte{0, 255, 255, 255}) {
+		t.Fatalf("isValidSubnetMask([]byte{255,255,253,0})")
+	}
+}

dhcpd/helpers.go

@@ -1,6 +1,7 @@
 package dhcpd
 
 import (
+	"encoding/binary"
 	"fmt"
 	"net"
 
@@ -65,3 +66,19 @@ func parseIPv4(text string) (net.IP, error) {
 	}
 	return result.To4(), nil
 }
+
+// Return TRUE if subnet mask is correct (e.g. 255.255.255.0)
+func isValidSubnetMask(mask net.IP) bool {
+	var n uint32
+	n = binary.BigEndian.Uint32(mask)
+	for i := 0; i != 32; i++ {
+		if n == 0 {
+			break
+		}
+		if (n & 0x80000000) == 0 {
+			return false
+		}
+		n <<= 1
+	}
+	return true
+}

dnsfilter/dnsfilter.go

@@ -731,13 +731,13 @@ func New(c *Config, filters map[int]string) *Dnsfilter {
 
 	if c != nil {
 		// initialize objects only once
-		if c.SafeBrowsingEnabled && gctx.safebrowsingCache == nil {
+		if gctx.safebrowsingCache == nil {
 			gctx.safebrowsingCache = gcache.New(defaultCacheSize).LRU().Expiration(defaultCacheTime).Build()
 		}
-		if c.SafeSearchEnabled && gctx.safeSearchCache == nil {
+		if gctx.safeSearchCache == nil {
 			gctx.safeSearchCache = gcache.New(defaultCacheSize).LRU().Expiration(defaultCacheTime).Build()
 		}
-		if c.ParentalEnabled && gctx.parentalCache == nil {
+		if gctx.parentalCache == nil {
 			gctx.parentalCache = gcache.New(defaultCacheSize).LRU().Expiration(defaultCacheTime).Build()
 		}
 		if len(c.ResolverAddress) != 0 && gctx.dialCache == nil {

dnsfilter/dnsfilter_test.go

@@ -455,13 +455,16 @@ func TestMatching(t *testing.T) {
 func applyClientSettings(clientAddr string, setts *RequestFilteringSettings) {
 	setts.FilteringEnabled = false
 	setts.ParentalEnabled = false
+	setts.SafeBrowsingEnabled = true
 }
 
+// Check behaviour without any per-client settings,
+//  then apply per-client settings and check behaviour once again
 func TestClientSettings(t *testing.T) {
 	var r Result
 	filters := make(map[int]string)
 	filters[0] = "||example.org^\n"
-	d := NewForTest(&Config{ParentalEnabled: true}, filters)
+	d := NewForTest(&Config{ParentalEnabled: true, SafeBrowsingEnabled: false}, filters)
 	defer d.Destroy()
 	d.ParentalSensitivity = 3
 
@@ -479,6 +482,12 @@ func TestClientSettings(t *testing.T) {
 		t.Fatalf("CheckHost FilteredParental")
 	}
 
+	// safesearch is disabled
+	r, _ = d.CheckHost("wmconvirus.narod.ru", dns.TypeA, "1.1.1.1")
+	if r.IsFiltered {
+		t.Fatalf("CheckHost safesearch")
+	}
+
 	// override client settings:
 	d.FilterHandler = applyClientSettings
 
@@ -488,11 +497,17 @@ func TestClientSettings(t *testing.T) {
 		t.Fatalf("CheckHost")
 	}
 
-	// override parental settings
+	// override parental settings (force disable parental)
 	r, _ = d.CheckHost("pornhub.com", dns.TypeA, "1.1.1.1")
 	if r.IsFiltered {
 		t.Fatalf("CheckHost")
 	}
+
+	// override safesearch settings (force enable safesearch)
+	r, _ = d.CheckHost("wmconvirus.narod.ru", dns.TypeA, "1.1.1.1")
+	if !r.IsFiltered || r.Reason != FilteredSafeBrowsing {
+		t.Fatalf("CheckHost FilteredSafeBrowsing")
+	}
 }
 
 // BENCHMARKS

home/control.go

@@ -722,6 +722,11 @@ func handleFilteringRemoveURL(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Stop DNS server:
+	//  we close urlfilter object which in turn closes file descriptors to filter files.
+	// Otherwise, Windows won't allow us to remove the file which is being currently used.
+	_ = dnsServer.Stop()
+
 	// go through each element and delete if url matches
 	config.Lock()
 	newFilters := config.Filters[:0]
@@ -732,9 +737,11 @@ func handleFilteringRemoveURL(w http.ResponseWriter, r *http.Request) {
 			// Remove the filter file
 			err := os.Remove(filter.Path())
 			if err != nil && !os.IsNotExist(err) {
+				config.Unlock()
 				httpError(w, http.StatusInternalServerError, "Couldn't remove the filter file: %s", err)
 				return
 			}
+			log.Debug("os.Remove(%s)", filter.Path())
 		}
 	}
 	// Update the configuration after removing filter files

home/dns.go

@@ -179,6 +179,9 @@ func generateServerConfig() (dnsforward.ServerConfig, error) {
 		Data: userFilter.Data,
 	})
 	for _, filter := range config.Filters {
+		if !filter.Enabled {
+			continue
+		}
 		filters = append(filters, dnsfilter.Filter{
 			ID:       filter.ID,
 			FilePath: filter.Path(),
@@ -253,10 +256,6 @@ func startDNSServer() error {
 }
 
 func reconfigureDNSServer() error {
-	if !isRunning() {
-		return fmt.Errorf("Refusing to reconfigure forwarding DNS server: not running")
-	}
-
 	config, err := generateServerConfig()
 	if err != nil {
 		return errorx.Decorate(err, "Couldn't start forwarding DNS server")
@@ -271,7 +270,7 @@ func reconfigureDNSServer() error {
 
 func stopDNSServer() error {
 	if !isRunning() {
-		return fmt.Errorf("Refusing to stop forwarding DNS server: not running")
+		return nil
 	}
 
 	err := dnsServer.Stop()

home/filter.go

@@ -170,12 +170,15 @@ func periodicallyRefreshFilters() {
 // Algorithm:
 // . Get the list of filters to be updated
 // . For each filter run the download and checksum check operation
-//  . If filter data hasn't changed, set new update time
-//  . If filter data has changed, parse it, save it on disk, set new update time
+// . Stop server
+// . For each filter:
+//  . If filter data hasn't changed, just set new update time on file
+//  . If filter data has changed, save it on disk
 //  . Apply changes to the current configuration
-// . Restart server
+// . Start server
 func refreshFiltersIfNecessary(force bool) int {
 	var updateFilters []filter
+	var updateFlags []bool // 'true' if filter data has changed
 
 	if config.firstRun {
 		return 0
@@ -206,25 +209,39 @@ func refreshFiltersIfNecessary(force bool) int {
 	for i := range updateFilters {
 		uf := &updateFilters[i]
 		updated, err := uf.update()
+		updateFlags = append(updateFlags, updated)
 		if err != nil {
 			log.Printf("Failed to update filter %s: %s\n", uf.URL, err)
 			continue
 		}
+		uf.LastUpdated = time.Now()
+		if updated {
+			updateCount++
+		}
+	}
+
+	stopped := false
+	if updateCount != 0 {
+		_ = dnsServer.Stop()
+		stopped = true
+	}
+
+	updateCount = 0
+	for i := range updateFilters {
+		uf := &updateFilters[i]
+		updated := updateFlags[i]
 		if updated {
 			// Saving it to the filters dir now
-			err = uf.save()
+			err := uf.save()
 			if err != nil {
 				log.Printf("Failed to save the updated filter %d: %s", uf.ID, err)
 				continue
 			}
-
 		} else {
-			mtime := time.Now()
-			e := os.Chtimes(uf.Path(), mtime, mtime)
+			e := os.Chtimes(uf.Path(), uf.LastUpdated, uf.LastUpdated)
 			if e != nil {
 				log.Error("os.Chtimes(): %v", e)
 			}
-			uf.LastUpdated = mtime
 		}
 
 		config.Lock()
@@ -249,11 +266,10 @@ func refreshFiltersIfNecessary(force bool) int {
 		config.Unlock()
 	}
 
-	if updateCount > 0 && isRunning() {
+	if stopped {
 		err := reconfigureDNSServer()
 		if err != nil {
-			msg := fmt.Sprintf("SHOULD NOT HAPPEN: cannot reconfigure DNS server with the new filters: %s", err)
-			panic(msg)
+			log.Error("cannot reconfigure DNS server with the new filters: %s", err)
 		}
 	}
 	return updateCount