all: improve permissions, add safe_fs_patterns

This commit is contained in:
Ainar Garipov 2024-10-02 18:09:57 +03:00
parent 5578987884
commit 8ce81f918c
30 changed files with 835 additions and 51 deletions

View File

@ -29,6 +29,20 @@ NOTE: Add new changes BELOW THIS COMMENT.
### Security ### Security
- Previuos versions of AdGuard Home allowed users to add any system it had
access to as filters, exposing them to be world-readable. To prevent this,
AdGuard Home now allows adding filtering-rule list files only from files
matching the patterns enumerated in the `filtering.safe_fs_patterns` property
in the configuration file.
We thank @itz-d0dgy for reporting this vulnerability, designated
CVE-2024-36814, to us.
- Additionally, AdGuard Home will now try to change the permissions of its files
and directories to more restrictive ones to prevent similar vulnerabilities
as well as limit the access to the configuration.
We thank @go-compile for reporting this vulnerability, designated
CVE-2024-36586, to us.
- Go version has been updated to prevent the possibility of exploiting the Go - Go version has been updated to prevent the possibility of exploiting the Go
vulnerabilities fixed in [1.23.2][go-1.23.2]. vulnerabilities fixed in [1.23.2][go-1.23.2].
@ -42,6 +56,15 @@ NOTE: Add new changes BELOW THIS COMMENT.
- Upstream server URL domain names requirements has been relaxed and now follow - Upstream server URL domain names requirements has been relaxed and now follow
the same rules as their domain specifications. the same rules as their domain specifications.
#### Configuration changes
In this release, the schema version has changed from 28 to 29.
- The new array `filtering.safe_fs_patterns` contains glob patterns for paths of
files that can be added as local filtering-rule lists. The migration should
add list files that have already been added, as well as the default value,
`$DATA_DIR/userfilters/*`.
### Fixed ### Fixed
- Property `clients.runtime_sources.dhcp` in the configuration file not taking - Property `clients.runtime_sources.dhcp` in the configuration file not taking
@ -50,6 +73,22 @@ NOTE: Add new changes BELOW THIS COMMENT.
- Enforce Bing safe search from Edge sidebar ([#7154]). - Enforce Bing safe search from Edge sidebar ([#7154]).
- Text overflow on the query log page ([#7119]). - Text overflow on the query log page ([#7119]).
### Known issues
- Due to the complexity of the Windows permissions architecture and poor support
from the standard Go library, we have to postpone the proper automated Windows
fix until the next release.
**Temporary workaround:** Set the permissions of the `AdGuardHome` directory
to more restrictive ones manually. To do that:
1. Locate the `AdGuardHome` directory.
2. Right-click on it and navigate to _Properties → Security → Advanced._
3. (You might need to disable permission inheritance to make them more
restricted.)
4. Adjust to give the `Full control` access to only the user which runs
AdGuard Home. Typically, `Administrator`.
[#5009]: https://github.com/AdguardTeam/AdGuardHome/issues/5009 [#5009]: https://github.com/AdguardTeam/AdGuardHome/issues/5009
[#5704]: https://github.com/AdguardTeam/AdGuardHome/issues/5704 [#5704]: https://github.com/AdguardTeam/AdGuardHome/issues/5704
[#7119]: https://github.com/AdguardTeam/AdGuardHome/issues/7119 [#7119]: https://github.com/AdguardTeam/AdGuardHome/issues/7119

View File

@ -19,6 +19,12 @@ import (
"github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/log"
) )
// Default file and directory permissions.
const (
DefaultPermDir = 0o700
DefaultPermFile = 0o600
)
// Unsupported is a helper that returns a wrapped [errors.ErrUnsupported]. // Unsupported is a helper that returns a wrapped [errors.ErrUnsupported].
func Unsupported(op string) (err error) { func Unsupported(op string) (err error) {
return fmt.Errorf("%s: not supported on %s: %w", op, runtime.GOOS, errors.ErrUnsupported) return fmt.Errorf("%s: not supported on %s: %w", op, runtime.GOOS, errors.ErrUnsupported)

View File

@ -2,4 +2,4 @@
package configmigrate package configmigrate
// LastSchemaVersion is the most recent schema version. // LastSchemaVersion is the most recent schema version.
const LastSchemaVersion uint = 28 const LastSchemaVersion uint = 29

View File

@ -19,6 +19,7 @@ func TestUpgradeSchema1to2(t *testing.T) {
m := New(&Config{ m := New(&Config{
WorkingDir: "", WorkingDir: "",
DataDir: "",
}) })
err := m.migrateTo2(diskConf) err := m.migrateTo2(diskConf)

View File

@ -10,20 +10,24 @@ import (
// Config is a the configuration for initializing a [Migrator]. // Config is a the configuration for initializing a [Migrator].
type Config struct { type Config struct {
// WorkingDir is an absolute path to the working directory of AdGuardHome. // WorkingDir is the absolute path to the working directory of AdGuardHome.
WorkingDir string WorkingDir string
// DataDir is the absolute path to the data directory of AdGuardHome.
DataDir string
} }
// Migrator performs the YAML configuration file migrations. // Migrator performs the YAML configuration file migrations.
type Migrator struct { type Migrator struct {
// workingDir is an absolute path to the working directory of AdGuardHome.
workingDir string workingDir string
dataDir string
} }
// New creates a new Migrator. // New creates a new Migrator.
func New(cfg *Config) (m *Migrator) { func New(c *Config) (m *Migrator) {
return &Migrator{ return &Migrator{
workingDir: cfg.WorkingDir, workingDir: c.WorkingDir,
dataDir: c.DataDir,
} }
} }
@ -120,6 +124,7 @@ func (m *Migrator) upgradeConfigSchema(current, target uint, diskConf yobj) (err
25: migrateTo26, 25: migrateTo26,
26: migrateTo27, 26: migrateTo27,
27: migrateTo28, 27: migrateTo28,
28: m.migrateTo29,
} }
for i, migrate := range upgrades[current:target] { for i, migrate := range upgrades[current:target] {

View File

@ -4,6 +4,7 @@ import (
"io/fs" "io/fs"
"os" "os"
"path" "path"
"path/filepath"
"testing" "testing"
"github.com/AdguardTeam/AdGuardHome/internal/configmigrate" "github.com/AdguardTeam/AdGuardHome/internal/configmigrate"
@ -190,6 +191,10 @@ func TestMigrateConfig_Migrate(t *testing.T) {
yamlEqFunc: require.YAMLEq, yamlEqFunc: require.YAMLEq,
name: "v27", name: "v27",
targetVersion: 27, targetVersion: 27,
}, {
yamlEqFunc: require.YAMLEq,
name: "v29",
targetVersion: 29,
}} }}
for _, tc := range testCases { for _, tc := range testCases {
@ -202,6 +207,7 @@ func TestMigrateConfig_Migrate(t *testing.T) {
migrator := configmigrate.New(&configmigrate.Config{ migrator := configmigrate.New(&configmigrate.Config{
WorkingDir: t.Name(), WorkingDir: t.Name(),
DataDir: filepath.Join(t.Name(), "data"),
}) })
newBody, upgraded, err := migrator.Migrate(body, tc.targetVersion) newBody, upgraded, err := migrator.Migrate(body, tc.targetVersion)
require.NoError(t, err) require.NoError(t, err)

View File

@ -0,0 +1,117 @@
http:
address: 127.0.0.1:3000
session_ttl: 3h
pprof:
enabled: true
port: 6060
users:
- name: testuser
password: testpassword
dns:
bind_hosts:
- 127.0.0.1
port: 53
parental_sensitivity: 0
upstream_dns:
- tls://1.1.1.1
- tls://1.0.0.1
- quic://8.8.8.8:784
bootstrap_dns:
- 8.8.8.8:53
edns_client_subnet:
enabled: true
use_custom: false
custom_ip: ""
filtering:
filtering_enabled: true
parental_enabled: false
safebrowsing_enabled: false
safe_search:
enabled: false
bing: true
duckduckgo: true
google: true
pixabay: true
yandex: true
youtube: true
protection_enabled: true
blocked_services:
schedule:
time_zone: Local
ids:
- 500px
blocked_response_ttl: 10
filters:
- url: https://adaway.org/hosts.txt
name: AdAway
enabled: false
- url: /path/to/file.txt
name: Local Filter
enabled: false
clients:
persistent:
- name: localhost
ids:
- 127.0.0.1
- aa:aa:aa:aa:aa:aa
use_global_settings: true
use_global_blocked_services: true
filtering_enabled: false
parental_enabled: false
safebrowsing_enabled: false
safe_search:
enabled: true
bing: true
duckduckgo: true
google: true
pixabay: true
yandex: true
youtube: true
blocked_services:
schedule:
time_zone: Local
ids:
- 500px
runtime_sources:
whois: true
arp: true
rdns: true
dhcp: true
hosts: true
dhcp:
enabled: false
interface_name: vboxnet0
local_domain_name: local
dhcpv4:
gateway_ip: 192.168.0.1
subnet_mask: 255.255.255.0
range_start: 192.168.0.10
range_end: 192.168.0.250
lease_duration: 1234
icmp_timeout_msec: 10
schema_version: 28
user_rules: []
querylog:
enabled: true
file_enabled: true
interval: 720h
size_memory: 1000
ignored:
- '|.^'
statistics:
enabled: true
interval: 240h
ignored:
- '|.^'
os:
group: ''
rlimit_nofile: 123
user: ''
log:
file: ""
max_backups: 0
max_size: 100
max_age: 3
compress: true
local_time: false
verbose: true

View File

@ -0,0 +1,120 @@
http:
address: 127.0.0.1:3000
session_ttl: 3h
pprof:
enabled: true
port: 6060
users:
- name: testuser
password: testpassword
dns:
bind_hosts:
- 127.0.0.1
port: 53
parental_sensitivity: 0
upstream_dns:
- tls://1.1.1.1
- tls://1.0.0.1
- quic://8.8.8.8:784
bootstrap_dns:
- 8.8.8.8:53
edns_client_subnet:
enabled: true
use_custom: false
custom_ip: ""
filtering:
filtering_enabled: true
parental_enabled: false
safebrowsing_enabled: false
safe_fs_patterns:
- TestMigrateConfig_Migrate/v29/data/userfilters/*
- /path/to/file.txt
safe_search:
enabled: false
bing: true
duckduckgo: true
google: true
pixabay: true
yandex: true
youtube: true
protection_enabled: true
blocked_services:
schedule:
time_zone: Local
ids:
- 500px
blocked_response_ttl: 10
filters:
- url: https://adaway.org/hosts.txt
name: AdAway
enabled: false
- url: /path/to/file.txt
name: Local Filter
enabled: false
clients:
persistent:
- name: localhost
ids:
- 127.0.0.1
- aa:aa:aa:aa:aa:aa
use_global_settings: true
use_global_blocked_services: true
filtering_enabled: false
parental_enabled: false
safebrowsing_enabled: false
safe_search:
enabled: true
bing: true
duckduckgo: true
google: true
pixabay: true
yandex: true
youtube: true
blocked_services:
schedule:
time_zone: Local
ids:
- 500px
runtime_sources:
whois: true
arp: true
rdns: true
dhcp: true
hosts: true
dhcp:
enabled: false
interface_name: vboxnet0
local_domain_name: local
dhcpv4:
gateway_ip: 192.168.0.1
subnet_mask: 255.255.255.0
range_start: 192.168.0.10
range_end: 192.168.0.250
lease_duration: 1234
icmp_timeout_msec: 10
schema_version: 29
user_rules: []
querylog:
enabled: true
file_enabled: true
interval: 720h
size_memory: 1000
ignored:
- '|.^'
statistics:
enabled: true
interval: 240h
ignored:
- '|.^'
os:
group: ''
rlimit_nofile: 123
user: ''
log:
file: ""
max_backups: 0
max_size: 100
max_age: 3
compress: true
local_time: false
verbose: true

View File

@ -0,0 +1,63 @@
package configmigrate
import (
"fmt"
"path/filepath"
)
// migrateTo29 performs the following changes:
//
// # BEFORE:
// 'filters':
// - 'enabled': true
// 'url': /path/to/file.txt
// 'name': My FS Filter
// 'id': 1234
//
// # AFTER:
// 'filters':
// - 'enabled': true
// 'url': /path/to/file.txt
// 'name': My FS Filter
// 'id': 1234
// # …
// 'filtering':
// 'safe_fs_patterns':
// - '/opt/AdGuardHome/data/userfilters/*'
// - '/path/to/file.txt'
// # …
func (m Migrator) migrateTo29(diskConf yobj) (err error) {
diskConf["schema_version"] = 29
filterVals, ok, err := fieldVal[[]any](diskConf, "filters")
if !ok {
return err
}
paths := []string{
filepath.Join(m.dataDir, "userfilters", "*"),
}
for i, v := range filterVals {
var f yobj
f, ok = v.(yobj)
if !ok {
return fmt.Errorf("filters: at index %d: expected object, got %T", i, v)
}
var u string
u, ok, _ = fieldVal[string](f, "url")
if ok && filepath.IsAbs(u) {
paths = append(paths, u)
}
}
fltConf, ok, err := fieldVal[yobj](diskConf, "filtering")
if !ok {
return err
}
fltConf["safe_fs_patterns"] = paths
return nil
}

View File

@ -12,6 +12,7 @@ import (
"strings" "strings"
"time" "time"
"github.com/AdguardTeam/AdGuardHome/internal/aghos"
"github.com/AdguardTeam/AdGuardHome/internal/dhcpsvc" "github.com/AdguardTeam/AdGuardHome/internal/dhcpsvc"
"github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/log"
@ -185,7 +186,7 @@ func writeDB(path string, leases []*dbLease) (err error) {
return err return err
} }
err = maybe.WriteFile(path, buf, 0o644) err = maybe.WriteFile(path, buf, aghos.DefaultPermFile)
if err != nil { if err != nil {
// Don't wrap the error since it's informative enough as is. // Don't wrap the error since it's informative enough as is.
return err return err

View File

@ -11,6 +11,7 @@ import (
"strings" "strings"
"time" "time"
"github.com/AdguardTeam/AdGuardHome/internal/aghos"
"github.com/AdguardTeam/AdGuardHome/internal/aghrenameio" "github.com/AdguardTeam/AdGuardHome/internal/aghrenameio"
"github.com/AdguardTeam/AdGuardHome/internal/filtering/rulelist" "github.com/AdguardTeam/AdGuardHome/internal/filtering/rulelist"
"github.com/AdguardTeam/golibs/container" "github.com/AdguardTeam/golibs/container"
@ -448,11 +449,7 @@ func (d *DNSFilter) updateIntl(flt *FilterYAML) (ok bool, err error) {
var res *rulelist.ParseResult var res *rulelist.ParseResult
// Change the default 0o600 permission to something more acceptable by end tmpFile, err := aghrenameio.NewPendingFile(flt.Path(d.conf.DataDir), aghos.DefaultPermFile)
// users.
//
// See https://github.com/AdguardTeam/AdGuardHome/issues/3198.
tmpFile, err := aghrenameio.NewPendingFile(flt.Path(d.conf.DataDir), 0o644)
if err != nil { if err != nil {
return false, err return false, err
} }
@ -522,6 +519,11 @@ func (d *DNSFilter) reader(fltURL string) (r io.ReadCloser, err error) {
return r, nil return r, nil
} }
fltURL = filepath.Clean(fltURL)
if !pathMatchesAny(d.safeFSPatterns, fltURL) {
return nil, fmt.Errorf("path %q does not match safe patterns", fltURL)
}
r, err = os.Open(fltURL) r, err = os.Open(fltURL)
if err != nil { if err != nil {
return nil, fmt.Errorf("opening file: %w", err) return nil, fmt.Errorf("opening file: %w", err)

View File

@ -19,6 +19,7 @@ import (
"time" "time"
"github.com/AdguardTeam/AdGuardHome/internal/aghhttp" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
"github.com/AdguardTeam/AdGuardHome/internal/aghos"
"github.com/AdguardTeam/AdGuardHome/internal/filtering/rulelist" "github.com/AdguardTeam/AdGuardHome/internal/filtering/rulelist"
"github.com/AdguardTeam/golibs/container" "github.com/AdguardTeam/golibs/container"
"github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/errors"
@ -130,6 +131,10 @@ type Config struct {
// UserRules is the global list of custom rules. // UserRules is the global list of custom rules.
UserRules []string `yaml:"-"` UserRules []string `yaml:"-"`
// SafeFSPatterns are the patterns for matching which local filtering-rule
// files can be added.
SafeFSPatterns []string `yaml:"safe_fs_patterns"`
SafeBrowsingCacheSize uint `yaml:"safebrowsing_cache_size"` // (in bytes) SafeBrowsingCacheSize uint `yaml:"safebrowsing_cache_size"` // (in bytes)
SafeSearchCacheSize uint `yaml:"safesearch_cache_size"` // (in bytes) SafeSearchCacheSize uint `yaml:"safesearch_cache_size"` // (in bytes)
ParentalCacheSize uint `yaml:"parental_cache_size"` // (in bytes) ParentalCacheSize uint `yaml:"parental_cache_size"` // (in bytes)
@ -257,6 +262,8 @@ type DNSFilter struct {
refreshLock *sync.Mutex refreshLock *sync.Mutex
hostCheckers []hostChecker hostCheckers []hostChecker
safeFSPatterns []string
} }
// Filter represents a filter list // Filter represents a filter list
@ -987,13 +994,22 @@ func New(c *Config, blockFilters []Filter) (d *DNSFilter, err error) {
d = &DNSFilter{ d = &DNSFilter{
idGen: newIDGenerator(int32(time.Now().Unix())), idGen: newIDGenerator(int32(time.Now().Unix())),
bufPool: syncutil.NewSlicePool[byte](rulelist.DefaultRuleBufSize), bufPool: syncutil.NewSlicePool[byte](rulelist.DefaultRuleBufSize),
safeSearch: c.SafeSearch,
refreshLock: &sync.Mutex{}, refreshLock: &sync.Mutex{},
safeBrowsingChecker: c.SafeBrowsingChecker, safeBrowsingChecker: c.SafeBrowsingChecker,
parentalControlChecker: c.ParentalControlChecker, parentalControlChecker: c.ParentalControlChecker,
confMu: &sync.RWMutex{}, confMu: &sync.RWMutex{},
} }
d.safeSearch = c.SafeSearch for i, p := range c.SafeFSPatterns {
// Use Match to validate the patterns here.
_, err = filepath.Match(p, "test")
if err != nil {
return nil, fmt.Errorf("safe_fs_patterns: at index %d: %w", i, err)
}
d.safeFSPatterns = append(d.safeFSPatterns, p)
}
d.hostCheckers = []hostChecker{{ d.hostCheckers = []hostChecker{{
check: d.matchSysHosts, check: d.matchSysHosts,
@ -1022,7 +1038,7 @@ func New(c *Config, blockFilters []Filter) (d *DNSFilter, err error) {
err = d.prepareRewrites() err = d.prepareRewrites()
if err != nil { if err != nil {
return nil, fmt.Errorf("rewrites: preparing: %s", err) return nil, fmt.Errorf("rewrites: preparing: %w", err)
} }
if d.conf.BlockedServices != nil { if d.conf.BlockedServices != nil {
@ -1037,11 +1053,16 @@ func New(c *Config, blockFilters []Filter) (d *DNSFilter, err error) {
if err != nil { if err != nil {
d.Close() d.Close()
return nil, fmt.Errorf("initializing filtering subsystem: %s", err) return nil, fmt.Errorf("initializing filtering subsystem: %w", err)
} }
} }
_ = os.MkdirAll(filepath.Join(d.conf.DataDir, filterDir), 0o755) err = os.MkdirAll(filepath.Join(d.conf.DataDir, filterDir), aghos.DefaultPermDir)
if err != nil {
d.Close()
return nil, fmt.Errorf("making filtering directory: %w", err)
}
d.loadFilters(d.conf.Filters) d.loadFilters(d.conf.Filters)
d.loadFilters(d.conf.WhitelistFilters) d.loadFilters(d.conf.WhitelistFilters)

View File

@ -20,14 +20,22 @@ import (
) )
// validateFilterURL validates the filter list URL or file name. // validateFilterURL validates the filter list URL or file name.
func validateFilterURL(urlStr string) (err error) { func (d *DNSFilter) validateFilterURL(urlStr string) (err error) {
defer func() { err = errors.Annotate(err, "checking filter: %w") }() defer func() { err = errors.Annotate(err, "checking filter: %w") }()
if filepath.IsAbs(urlStr) { if filepath.IsAbs(urlStr) {
urlStr = filepath.Clean(urlStr)
_, err = os.Stat(urlStr) _, err = os.Stat(urlStr)
if err != nil {
// Don't wrap the error since it's informative enough as is.
return err
}
// Don't wrap the error since it's informative enough as is. if !pathMatchesAny(d.safeFSPatterns, urlStr) {
return err return fmt.Errorf("path %q does not match safe patterns", urlStr)
}
return nil
} }
u, err := url.ParseRequestURI(urlStr) u, err := url.ParseRequestURI(urlStr)
@ -65,7 +73,7 @@ func (d *DNSFilter) handleFilteringAddURL(w http.ResponseWriter, r *http.Request
return return
} }
err = validateFilterURL(fj.URL) err = d.validateFilterURL(fj.URL)
if err != nil { if err != nil {
aghhttp.Error(r, w, http.StatusBadRequest, "%s", err) aghhttp.Error(r, w, http.StatusBadRequest, "%s", err)
@ -225,7 +233,7 @@ func (d *DNSFilter) handleFilteringSetURL(w http.ResponseWriter, r *http.Request
return return
} }
err = validateFilterURL(fj.Data.URL) err = d.validateFilterURL(fj.Data.URL)
if err != nil { if err != nil {
aghhttp.Error(r, w, http.StatusBadRequest, "invalid url: %s", err) aghhttp.Error(r, w, http.StatusBadRequest, "invalid url: %s", err)

View File

@ -0,0 +1,37 @@
package filtering
import (
"fmt"
"path/filepath"
)
// pathMatchesAny returns true if filePath matches one of globs. globs must be
// valid. filePath must be absolute and clean. If globs are empty,
// pathMatchesAny returns false.
//
// TODO(a.garipov): Move to golibs?
func pathMatchesAny(globs []string, filePath string) (ok bool) {
if len(globs) == 0 {
return false
}
clean, err := filepath.Abs(filePath)
if err != nil {
panic(fmt.Errorf("pathMatchesAny: %w", err))
} else if clean != filePath {
panic(fmt.Errorf("pathMatchesAny: filepath %q is not absolute", filePath))
}
for _, g := range globs {
ok, err = filepath.Match(g, filePath)
if err != nil {
panic(fmt.Errorf("pathMatchesAny: bad pattern: %w", err))
}
if ok {
return true
}
}
return false
}

View File

@ -0,0 +1,78 @@
//go:build unix
package filtering
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestPathInAnyDir(t *testing.T) {
t.Parallel()
const (
filePath = "/path/to/file.txt"
filePathGlob = "/path/to/*"
otherFilePath = "/otherpath/to/file.txt"
)
testCases := []struct {
want assert.BoolAssertionFunc
filePath string
name string
globs []string
}{{
want: assert.False,
filePath: filePath,
name: "nil_pats",
globs: nil,
}, {
want: assert.True,
filePath: filePath,
name: "match",
globs: []string{
filePath,
otherFilePath,
},
}, {
want: assert.False,
filePath: filePath,
name: "no_match",
globs: []string{
otherFilePath,
},
}, {
want: assert.True,
filePath: filePath,
name: "match_star",
globs: []string{
filePathGlob,
},
}}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
tc.want(t, pathMatchesAny(tc.globs, tc.filePath))
})
}
require.True(t, t.Run("panic_on_unabs_file_path", func(t *testing.T) {
t.Parallel()
assert.Panics(t, func() {
_ = pathMatchesAny([]string{"/home/user"}, "../../etc/passwd")
})
}))
require.True(t, t.Run("panic_on_bad_pat", func(t *testing.T) {
t.Parallel()
assert.Panics(t, func() {
_ = pathMatchesAny([]string{`\`}, filePath)
})
}))
}

View File

@ -0,0 +1,73 @@
//go:build windows
package filtering
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestPathInAnyDir(t *testing.T) {
t.Parallel()
const (
filePath = `C:\path\to\file.txt`
filePathGlob = `C:\path\to\*`
otherFilePath = `C:\otherpath\to\file.txt`
)
testCases := []struct {
want assert.BoolAssertionFunc
filePath string
name string
globs []string
}{{
want: assert.False,
filePath: filePath,
name: "nil_pats",
globs: nil,
}, {
want: assert.True,
filePath: filePath,
name: "match",
globs: []string{
filePath,
otherFilePath,
},
}, {
want: assert.False,
filePath: filePath,
name: "no_match",
globs: []string{
otherFilePath,
},
}, {
want: assert.True,
filePath: filePath,
name: "match_star",
globs: []string{
filePathGlob,
},
}}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
tc.want(t, pathMatchesAny(tc.globs, tc.filePath))
})
}
require.True(t, t.Run("panic_on_unabs_file_path", func(t *testing.T) {
t.Parallel()
assert.Panics(t, func() {
_ = pathMatchesAny([]string{`C:\home\user`}, `..\..\etc\passwd`)
})
}))
// TODO(a.garipov): See if there is anything for which filepath.Match
// returns ErrBadPattern on Windows.
}

View File

@ -11,6 +11,7 @@ import (
"path/filepath" "path/filepath"
"time" "time"
"github.com/AdguardTeam/AdGuardHome/internal/aghos"
"github.com/AdguardTeam/AdGuardHome/internal/aghrenameio" "github.com/AdguardTeam/AdGuardHome/internal/aghrenameio"
"github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/ioutil" "github.com/AdguardTeam/golibs/ioutil"
@ -196,7 +197,7 @@ func (f *Filter) readFromHTTP(
return "", nil, fmt.Errorf("got status code %d, want %d", resp.StatusCode, http.StatusOK) return "", nil, fmt.Errorf("got status code %d, want %d", resp.StatusCode, http.StatusOK)
} }
fltFile, err := aghrenameio.NewPendingFile(cachePath, 0o644) fltFile, err := aghrenameio.NewPendingFile(cachePath, aghos.DefaultPermFile)
if err != nil { if err != nil {
return "", nil, fmt.Errorf("creating temp file: %w", err) return "", nil, fmt.Errorf("creating temp file: %w", err)
} }
@ -271,7 +272,7 @@ func parseIntoCache(
filePath string, filePath string,
cachePath string, cachePath string,
) (parseRes *ParseResult, err error) { ) (parseRes *ParseResult, err error) {
tmpFile, err := aghrenameio.NewPendingFile(cachePath, 0o644) tmpFile, err := aghrenameio.NewPendingFile(cachePath, aghos.DefaultPermFile)
if err != nil { if err != nil {
return nil, fmt.Errorf("creating temp file: %w", err) return nil, fmt.Errorf("creating temp file: %w", err)
} }

View File

@ -9,6 +9,7 @@ import (
"sync" "sync"
"time" "time"
"github.com/AdguardTeam/AdGuardHome/internal/aghos"
"github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/log"
"github.com/AdguardTeam/golibs/netutil" "github.com/AdguardTeam/golibs/netutil"
@ -89,7 +90,7 @@ func InitAuth(
trustedProxies: trustedProxies, trustedProxies: trustedProxies,
} }
var err error var err error
a.db, err = bbolt.Open(dbFilename, 0o644, nil) a.db, err = bbolt.Open(dbFilename, aghos.DefaultPermFile, nil)
if err != nil { if err != nil {
log.Error("auth: open DB: %s: %s", dbFilename, err) log.Error("auth: open DB: %s: %s", dbFilename, err)
if err.Error() == "invalid argument" { if err.Error() == "invalid argument" {

View File

@ -9,6 +9,7 @@ import (
"sync" "sync"
"github.com/AdguardTeam/AdGuardHome/internal/aghalg" "github.com/AdguardTeam/AdGuardHome/internal/aghalg"
"github.com/AdguardTeam/AdGuardHome/internal/aghos"
"github.com/AdguardTeam/AdGuardHome/internal/aghtls" "github.com/AdguardTeam/AdGuardHome/internal/aghtls"
"github.com/AdguardTeam/AdGuardHome/internal/configmigrate" "github.com/AdguardTeam/AdGuardHome/internal/configmigrate"
"github.com/AdguardTeam/AdGuardHome/internal/dhcpd" "github.com/AdguardTeam/AdGuardHome/internal/dhcpd"
@ -26,9 +27,15 @@ import (
yaml "gopkg.in/yaml.v3" yaml "gopkg.in/yaml.v3"
) )
// dataDir is the name of a directory under the working one to store some const (
// persistent data. // dataDir is the name of a directory under the working one to store some
const dataDir = "data" // persistent data.
dataDir = "data"
// userFilterDataDir is the name of the directory used to store users'
// FS-based rule lists.
userFilterDataDir = "userfilters"
)
// logSettings are the logging settings part of the configuration file. // logSettings are the logging settings part of the configuration file.
type logSettings struct { type logSettings struct {
@ -520,6 +527,7 @@ func parseConfig() (err error) {
migrator := configmigrate.New(&configmigrate.Config{ migrator := configmigrate.New(&configmigrate.Config{
WorkingDir: Context.workDir, WorkingDir: Context.workDir,
DataDir: Context.getDataDir(),
}) })
var upgraded bool var upgraded bool
@ -534,7 +542,7 @@ func parseConfig() (err error) {
confPath := configFilePath() confPath := configFilePath()
log.Debug("writing config file %q after config upgrade", confPath) log.Debug("writing config file %q after config upgrade", confPath)
err = maybe.WriteFile(confPath, config.fileData, 0o644) err = maybe.WriteFile(confPath, config.fileData, aghos.DefaultPermFile)
if err != nil { if err != nil {
return fmt.Errorf("writing new config: %w", err) return fmt.Errorf("writing new config: %w", err)
} }
@ -700,7 +708,7 @@ func (c *configuration) write() (err error) {
return fmt.Errorf("generating config file: %w", err) return fmt.Errorf("generating config file: %w", err)
} }
err = maybe.WriteFile(confPath, buf.Bytes(), 0o644) err = maybe.WriteFile(confPath, buf.Bytes(), aghos.DefaultPermFile)
if err != nil { if err != nil {
return fmt.Errorf("writing config file: %w", err) return fmt.Errorf("writing config file: %w", err)
} }

View File

@ -270,9 +270,9 @@ DNSStubListener=no
const resolvConfPath = "/etc/resolv.conf" const resolvConfPath = "/etc/resolv.conf"
// Deactivate DNSStubListener // Deactivate DNSStubListener
func disableDNSStubListener() error { func disableDNSStubListener() (err error) {
dir := filepath.Dir(resolvedConfPath) dir := filepath.Dir(resolvedConfPath)
err := os.MkdirAll(dir, 0o755) err = os.MkdirAll(dir, 0o755)
if err != nil { if err != nil {
return fmt.Errorf("os.MkdirAll: %s: %w", dir, err) return fmt.Errorf("os.MkdirAll: %s: %w", dir, err)
} }
@ -413,9 +413,12 @@ func (web *webAPI) handleInstallConfigure(w http.ResponseWriter, r *http.Request
copyInstallSettings(curConfig, config) copyInstallSettings(curConfig, config)
Context.firstRun = false Context.firstRun = false
config.HTTPConfig.Address = netip.AddrPortFrom(req.Web.IP, req.Web.Port)
config.DNS.BindHosts = []netip.Addr{req.DNS.IP} config.DNS.BindHosts = []netip.Addr{req.DNS.IP}
config.DNS.Port = req.DNS.Port config.DNS.Port = req.DNS.Port
config.Filtering.SafeFSPatterns = []string{
filepath.Join(Context.workDir, userFilterDataDir, "*"),
}
config.HTTPConfig.Address = netip.AddrPortFrom(req.Web.IP, req.Web.Port)
u := &webUser{ u := &webUser{
Name: req.Username, Name: req.Username,

View File

@ -47,14 +47,9 @@ func onConfigModified() {
// initDNS updates all the fields of the [Context] needed to initialize the DNS // initDNS updates all the fields of the [Context] needed to initialize the DNS
// server and initializes it at last. It also must not be called unless // server and initializes it at last. It also must not be called unless
// [config] and [Context] are initialized. l must not be nil. // [config] and [Context] are initialized. l must not be nil.
func initDNS(l *slog.Logger) (err error) { func initDNS(l *slog.Logger, statsDir, querylogDir string) (err error) {
anonymizer := config.anonymizer() anonymizer := config.anonymizer()
statsDir, querylogDir, err := checkStatsAndQuerylogDirs(&Context, config)
if err != nil {
return err
}
statsConf := stats.Config{ statsConf := stats.Config{
Logger: l.With(slogutil.KeyPrefix, "stats"), Logger: l.With(slogutil.KeyPrefix, "stats"),
Filename: filepath.Join(statsDir, "stats.db"), Filename: filepath.Join(statsDir, "stats.db"),

View File

@ -31,6 +31,7 @@ import (
"github.com/AdguardTeam/AdGuardHome/internal/filtering" "github.com/AdguardTeam/AdGuardHome/internal/filtering"
"github.com/AdguardTeam/AdGuardHome/internal/filtering/hashprefix" "github.com/AdguardTeam/AdGuardHome/internal/filtering/hashprefix"
"github.com/AdguardTeam/AdGuardHome/internal/filtering/safesearch" "github.com/AdguardTeam/AdGuardHome/internal/filtering/safesearch"
"github.com/AdguardTeam/AdGuardHome/internal/permcheck"
"github.com/AdguardTeam/AdGuardHome/internal/querylog" "github.com/AdguardTeam/AdGuardHome/internal/querylog"
"github.com/AdguardTeam/AdGuardHome/internal/stats" "github.com/AdguardTeam/AdGuardHome/internal/stats"
"github.com/AdguardTeam/AdGuardHome/internal/updater" "github.com/AdguardTeam/AdGuardHome/internal/updater"
@ -630,9 +631,9 @@ func run(opts options, clientBuildFS fs.FS, done chan struct{}) {
} }
} }
dir := Context.getDataDir() dataDir := Context.getDataDir()
err = os.MkdirAll(dir, 0o755) err = os.MkdirAll(dataDir, aghos.DefaultPermDir)
fatalOnError(errors.Annotate(err, "creating DNS data dir at %s: %w", dir)) fatalOnError(errors.Annotate(err, "creating DNS data dir at %s: %w", dataDir))
GLMode = opts.glinetMode GLMode = opts.glinetMode
@ -649,8 +650,11 @@ func run(opts options, clientBuildFS fs.FS, done chan struct{}) {
Context.web, err = initWeb(opts, clientBuildFS, upd, slogLogger) Context.web, err = initWeb(opts, clientBuildFS, upd, slogLogger)
fatalOnError(err) fatalOnError(err)
statsDir, querylogDir, err := checkStatsAndQuerylogDirs(&Context, config)
fatalOnError(err)
if !Context.firstRun { if !Context.firstRun {
err = initDNS(slogLogger) err = initDNS(slogLogger, statsDir, querylogDir)
fatalOnError(err) fatalOnError(err)
Context.tls.start() Context.tls.start()
@ -671,6 +675,12 @@ func run(opts options, clientBuildFS fs.FS, done chan struct{}) {
} }
} }
if permcheck.NeedsMigration(confPath) {
permcheck.Migrate(Context.workDir, dataDir, statsDir, querylogDir, confPath)
}
permcheck.Check(Context.workDir, dataDir, statsDir, querylogDir, confPath)
Context.web.start() Context.web.start()
// Wait for other goroutines to complete their job. // Wait for other goroutines to complete their job.
@ -714,7 +724,12 @@ func (c *configuration) anonymizer() (ipmut *aghnet.IPMut) {
// startMods initializes and starts the DNS server after installation. l must // startMods initializes and starts the DNS server after installation. l must
// not be nil. // not be nil.
func startMods(l *slog.Logger) (err error) { func startMods(l *slog.Logger) (err error) {
err = initDNS(l) statsDir, querylogDir, err := checkStatsAndQuerylogDirs(&Context, config)
if err != nil {
return err
}
err = initDNS(l, statsDir, querylogDir)
if err != nil { if err != nil {
return err return err
} }

View File

@ -14,6 +14,7 @@ import (
"sync" "sync"
"time" "time"
"github.com/AdguardTeam/AdGuardHome/internal/aghos"
"github.com/AdguardTeam/AdGuardHome/internal/next/agh" "github.com/AdguardTeam/AdGuardHome/internal/next/agh"
"github.com/AdguardTeam/AdGuardHome/internal/next/dnssvc" "github.com/AdguardTeam/AdGuardHome/internal/next/dnssvc"
"github.com/AdguardTeam/AdGuardHome/internal/next/websvc" "github.com/AdguardTeam/AdGuardHome/internal/next/websvc"
@ -182,7 +183,7 @@ func (m *Manager) write() (err error) {
return fmt.Errorf("encoding: %w", err) return fmt.Errorf("encoding: %w", err)
} }
err = maybe.WriteFile(m.fileName, b, 0o644) err = maybe.WriteFile(m.fileName, b, aghos.DefaultPermFile)
if err != nil { if err != nil {
return fmt.Errorf("writing: %w", err) return fmt.Errorf("writing: %w", err)
} }

View File

@ -0,0 +1,93 @@
package permcheck
import (
"io/fs"
"os"
"path/filepath"
"github.com/AdguardTeam/AdGuardHome/internal/aghos"
"github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/log"
)
// NeedsMigration returns true if AdGuard Home files need permission migration.
//
// TODO(a.garipov): Consider ways to detect this better.
func NeedsMigration(confFilePath string) (ok bool) {
s, err := os.Stat(confFilePath)
if err != nil {
if errors.Is(err, os.ErrNotExist) {
// Likely a first run. Don't check.
return false
}
log.Error("permcheck: checking if files need migration: %s", err)
// Unexpected error. Try to migrate just in case.
return true
}
return s.Mode().Perm() != aghos.DefaultPermFile
}
// Migrate attempts to change the permissions of AdGuard Home's files. It logs
// the results at an appropriate level.
func Migrate(workDir, dataDir, statsDir, querylogDir, confFilePath string) {
chmodDir(workDir)
chmodFile(confFilePath)
// TODO(a.garipov): Put all paths in one place and remove this duplication.
chmodDir(dataDir)
chmodDir(filepath.Join(dataDir, "filters"))
chmodFile(filepath.Join(dataDir, "sessions.db"))
chmodFile(filepath.Join(dataDir, "leases.json"))
if dataDir != querylogDir {
chmodDir(querylogDir)
}
chmodFile(filepath.Join(querylogDir, "querylog.json"))
chmodFile(filepath.Join(querylogDir, "querylog.json.1"))
if dataDir != statsDir {
chmodDir(statsDir)
}
chmodFile(filepath.Join(statsDir, "stats.db"))
}
// chmodDir changes the permissions of a single directory. The results are
// logged at the appropriate level.
func chmodDir(dirPath string) {
chmodPath(dirPath, typeDir, aghos.DefaultPermDir)
}
// chmodFile changes the permissions of a single file. The results are logged
// at the appropriate level.
func chmodFile(filePath string) {
chmodPath(filePath, typeFile, aghos.DefaultPermFile)
}
// chmodPath changes the permissions of a single filesystem entity. The results
// are logged at the appropriate level.
func chmodPath(entPath, fileType string, fm fs.FileMode) {
err := os.Chmod(entPath, fm)
if err == nil {
log.Info("permcheck: changed permissions for %s %q", fileType, entPath)
return
} else if errors.Is(err, os.ErrNotExist) {
log.Debug("permcheck: changing permissions for %s %q: %s", fileType, entPath, err)
return
}
log.Error(
"permcheck: SECURITY WARNING: cannot change permissions for %s %q to %#o: %s; "+
"this can leave your system vulnerable, see "+
"https://adguard-dns.io/kb/adguard-home/running-securely/#os-service-concerns",
fileType,
entPath,
fm,
err,
)
}

View File

@ -0,0 +1,86 @@
// Package permcheck contains code for simplifying permissions checks on files
// and directories.
//
// TODO(a.garipov): Improve the approach on Windows.
package permcheck
import (
"io/fs"
"os"
"path/filepath"
"github.com/AdguardTeam/AdGuardHome/internal/aghos"
"github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/log"
)
// File type constants for logging.
const (
typeDir = "directory"
typeFile = "file"
)
// Check checks the permissions on important files. It logs the results at
// appropriate levels.
func Check(workDir, dataDir, statsDir, querylogDir, confFilePath string) {
checkDir(workDir)
checkFile(confFilePath)
// TODO(a.garipov): Put all paths in one place and remove this duplication.
checkDir(dataDir)
checkDir(filepath.Join(dataDir, "filters"))
checkFile(filepath.Join(dataDir, "sessions.db"))
checkFile(filepath.Join(dataDir, "leases.json"))
if dataDir != querylogDir {
checkDir(querylogDir)
}
checkFile(filepath.Join(querylogDir, "querylog.json"))
checkFile(filepath.Join(querylogDir, "querylog.json.1"))
if dataDir != statsDir {
checkDir(statsDir)
}
checkFile(filepath.Join(statsDir, "stats.db"))
}
// checkDir checks the permissions of a single directory. The results are
// logged at the appropriate level.
func checkDir(dirPath string) {
checkPath(dirPath, typeDir, aghos.DefaultPermDir)
}
// checkFile checks the permissions of a single file. The results are logged at
// the appropriate level.
func checkFile(filePath string) {
checkPath(filePath, typeFile, aghos.DefaultPermFile)
}
// checkPath checks the permissions of a single filesystem entity. The results
// are logged at the appropriate level.
func checkPath(entPath, fileType string, want fs.FileMode) {
s, err := os.Stat(entPath)
if err != nil {
logFunc := log.Error
if errors.Is(err, os.ErrNotExist) {
logFunc = log.Debug
}
logFunc("permcheck: checking %s %q: %s", fileType, entPath, err)
return
}
// TODO(a.garipov): Add a more fine-grained check and result reporting.
perm := s.Mode().Perm()
if perm != want {
log.Info(
"permcheck: SECURITY WARNING: %s %q has unexpected permissions %#o; want %#o",
fileType,
entPath,
perm,
want,
)
}
}

View File

@ -8,6 +8,7 @@ import (
"sync" "sync"
"time" "time"
"github.com/AdguardTeam/AdGuardHome/internal/aghos"
"github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/log"
) )
@ -56,7 +57,7 @@ type qLogFile struct {
// newQLogFile initializes a new instance of the qLogFile. // newQLogFile initializes a new instance of the qLogFile.
func newQLogFile(path string) (qf *qLogFile, err error) { func newQLogFile(path string) (qf *qLogFile, err error) {
f, err := os.OpenFile(path, os.O_RDONLY, 0o644) f, err := os.OpenFile(path, os.O_RDONLY, aghos.DefaultPermFile)
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@ -7,6 +7,7 @@ import (
"os" "os"
"time" "time"
"github.com/AdguardTeam/AdGuardHome/internal/aghos"
"github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/log"
) )
@ -70,7 +71,7 @@ func (l *queryLog) flushToFile(b *bytes.Buffer) (err error) {
filename := l.logFile filename := l.logFile
f, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0o644) f, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_APPEND, aghos.DefaultPermFile)
if err != nil { if err != nil {
return fmt.Errorf("creating file %q: %w", filename, err) return fmt.Errorf("creating file %q: %w", filename, err)
} }

View File

@ -15,6 +15,7 @@ import (
"github.com/AdguardTeam/AdGuardHome/internal/aghhttp" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
"github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/AdGuardHome/internal/aghnet"
"github.com/AdguardTeam/AdGuardHome/internal/aghos"
"github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/logutil/slogutil"
"github.com/AdguardTeam/golibs/timeutil" "github.com/AdguardTeam/golibs/timeutil"
@ -383,7 +384,7 @@ func (s *StatsCtx) openDB() (err error) {
s.logger.Debug("opening database") s.logger.Debug("opening database")
var db *bbolt.DB var db *bbolt.DB
db, err = bbolt.Open(s.filename, 0o644, nil) db, err = bbolt.Open(s.filename, aghos.DefaultPermFile, nil)
if err != nil { if err != nil {
if err.Error() == "invalid argument" { if err.Error() == "invalid argument" {
const lines = `AdGuard Home cannot be initialized due to an incompatible file system. const lines = `AdGuard Home cannot be initialized due to an incompatible file system.

View File

@ -15,6 +15,7 @@ import (
"sync" "sync"
"time" "time"
"github.com/AdguardTeam/AdGuardHome/internal/aghos"
"github.com/AdguardTeam/AdGuardHome/internal/version" "github.com/AdguardTeam/AdGuardHome/internal/version"
"github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/ioutil" "github.com/AdguardTeam/golibs/ioutil"
@ -263,7 +264,7 @@ func (u *Updater) check() (err error) {
// ignores the configuration file if firstRun is true. // ignores the configuration file if firstRun is true.
func (u *Updater) backup(firstRun bool) (err error) { func (u *Updater) backup(firstRun bool) (err error) {
log.Debug("updater: backing up current configuration") log.Debug("updater: backing up current configuration")
_ = os.Mkdir(u.backupDir, 0o755) _ = os.Mkdir(u.backupDir, aghos.DefaultPermDir)
if !firstRun { if !firstRun {
err = copyFile(u.confName, filepath.Join(u.backupDir, "AdGuardHome.yaml")) err = copyFile(u.confName, filepath.Join(u.backupDir, "AdGuardHome.yaml"))
if err != nil { if err != nil {
@ -337,10 +338,10 @@ func (u *Updater) downloadPackageFile() (err error) {
return fmt.Errorf("io.ReadAll() failed: %w", err) return fmt.Errorf("io.ReadAll() failed: %w", err)
} }
_ = os.Mkdir(u.updateDir, 0o755) _ = os.Mkdir(u.updateDir, aghos.DefaultPermDir)
log.Debug("updater: saving package to file") log.Debug("updater: saving package to file")
err = os.WriteFile(u.packageName, body, 0o644) err = os.WriteFile(u.packageName, body, aghos.DefaultPermFile)
if err != nil { if err != nil {
return fmt.Errorf("os.WriteFile() failed: %w", err) return fmt.Errorf("os.WriteFile() failed: %w", err)
} }
@ -527,7 +528,7 @@ func copyFile(src, dst string) error {
if e != nil { if e != nil {
return e return e
} }
e = os.WriteFile(dst, d, 0o644) e = os.WriteFile(dst, d, aghos.DefaultPermFile)
if e != nil { if e != nil {
return e return e
} }

View File

@ -497,7 +497,7 @@ download() {
# Function unpack unpacks the passed archive depending on it's extension. # Function unpack unpacks the passed archive depending on it's extension.
unpack() { unpack() {
log "unpacking package from $pkg_name into $out_dir" log "unpacking package from $pkg_name into $out_dir"
if ! mkdir -p "$out_dir" if ! mkdir -m 0700 -p "$out_dir"
then then
error_exit "cannot create directory $out_dir" error_exit "cannot create directory $out_dir"
fi fi