diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f36a61ec..2b9ed08b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,7 +1,7 @@ 'name': 'build' 'env': - 'GO_VERSION': '1.23.1' + 'GO_VERSION': '1.23.2' 'NODE_VERSION': '16' 'on': diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index f5e99131..526c70fe 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1,7 +1,7 @@ 'name': 'lint' 'env': - 'GO_VERSION': '1.23.1' + 'GO_VERSION': '1.23.2' 'on': 'push': diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a01a9b2..90f031de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,19 +18,43 @@ TODO(a.garipov): Use the common markdown formatting tools. + + + + +## [v0.107.53] - 2024-10-03 + +See also the [v0.107.53 GitHub milestone][ms-v0.107.53]. + ### Security +- Previous 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 - vulnerabilities fixed in [1.23.1][go-1.23.1]. + vulnerabilities fixed in [1.23.2][go-1.23.2]. ### Added @@ -42,6 +66,15 @@ NOTE: Add new changes BELOW THIS COMMENT. - Upstream server URL domain names requirements has been relaxed and now follow 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 - Property `clients.runtime_sources.dhcp` in the configuration file not taking @@ -50,17 +83,30 @@ NOTE: Add new changes BELOW THIS COMMENT. - Enforce Bing safe search from Edge sidebar ([#7154]). - 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 [#5704]: https://github.com/AdguardTeam/AdGuardHome/issues/5704 [#7119]: https://github.com/AdguardTeam/AdGuardHome/issues/7119 [#7154]: https://github.com/AdguardTeam/AdGuardHome/pull/7154 [#7155]: https://github.com/AdguardTeam/AdGuardHome/pull/7155 -[go-1.23.1]: https://groups.google.com/g/golang-announce/c/K-cEzDeCtpc - - +[go-1.23.2]: https://groups.google.com/g/golang-announce/c/NKEc8VT7Fz0 +[ms-v0.107.53]: https://github.com/AdguardTeam/AdGuardHome/milestone/88?closed=1 @@ -3098,11 +3144,12 @@ See also the [v0.104.2 GitHub milestone][ms-v0.104.2]. -[Unreleased]: https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.52...HEAD +[Unreleased]: https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.53...HEAD +[v0.107.53]: https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.52...v0.107.53 [v0.107.52]: https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.51...v0.107.52 [v0.107.51]: https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.50...v0.107.51 [v0.107.50]: https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.49...v0.107.50 diff --git a/Makefile b/Makefile index 6501e375..2d891860 100644 --- a/Makefile +++ b/Makefile @@ -27,8 +27,7 @@ DEPLOY_SCRIPT_PATH = not/a/real/path DIST_DIR = dist GOAMD64 = v1 GOPROXY = https://proxy.golang.org|direct -GOSUMDB = sum.golang.google.cn -GOTOOLCHAIN = go1.23.1 +GOTOOLCHAIN = go1.23.2 GOTELEMETRY = off GPG_KEY = devteam@adguard.com GPG_KEY_PASSPHRASE = not-a-real-password @@ -67,7 +66,6 @@ ENV = env\ GO="$(GO.MACRO)"\ GOAMD64='$(GOAMD64)'\ GOPROXY='$(GOPROXY)'\ - GOSUMDB='$(GOSUMDB)'\ GOTELEMETRY='$(GOTELEMETRY)'\ GOTOOLCHAIN='$(GOTOOLCHAIN)'\ GPG_KEY='$(GPG_KEY)'\ diff --git a/bamboo-specs/release.yaml b/bamboo-specs/release.yaml index 5ace2365..182f327d 100644 --- a/bamboo-specs/release.yaml +++ b/bamboo-specs/release.yaml @@ -8,7 +8,7 @@ 'variables': 'channel': 'edge' 'dockerFrontend': 'adguard/home-js-builder:2.0' - 'dockerGo': 'adguard/go-builder:1.23.1--1' + 'dockerGo': 'adguard/go-builder:1.23.2--1' 'stages': - 'Build frontend': @@ -276,7 +276,7 @@ 'variables': 'channel': 'beta' 'dockerFrontend': 'adguard/home-js-builder:2.0' - 'dockerGo': 'adguard/go-builder:1.23.1--1' + 'dockerGo': 'adguard/go-builder:1.23.2--1' # release-vX.Y.Z branches are the branches from which the actual final # release is built. - '^release-v[0-9]+\.[0-9]+\.[0-9]+': @@ -292,4 +292,4 @@ 'variables': 'channel': 'release' 'dockerFrontend': 'adguard/home-js-builder:2.0' - 'dockerGo': 'adguard/go-builder:1.23.1--1' + 'dockerGo': 'adguard/go-builder:1.23.2--1' diff --git a/bamboo-specs/test.yaml b/bamboo-specs/test.yaml index 29a8bae5..2f672e2d 100644 --- a/bamboo-specs/test.yaml +++ b/bamboo-specs/test.yaml @@ -6,7 +6,7 @@ 'name': 'AdGuard Home - Build and run tests' 'variables': 'dockerFrontend': 'adguard/home-js-builder:2.0' - 'dockerGo': 'adguard/go-builder:1.23.1--1' + 'dockerGo': 'adguard/go-builder:1.23.2--1' 'channel': 'development' 'stages': @@ -196,5 +196,5 @@ # may need to build a few of these. 'variables': 'dockerFrontend': 'adguard/home-js-builder:2.0' - 'dockerGo': 'adguard/go-builder:1.23.1--1' + 'dockerGo': 'adguard/go-builder:1.23.2--1' 'channel': 'candidate' diff --git a/go.mod b/go.mod index ad0b523b..9b5a942c 100644 --- a/go.mod +++ b/go.mod @@ -1,10 +1,10 @@ module github.com/AdguardTeam/AdGuardHome -go 1.23.1 +go 1.23.2 require ( - github.com/AdguardTeam/dnsproxy v0.73.0 - github.com/AdguardTeam/golibs v0.26.0 + github.com/AdguardTeam/dnsproxy v0.73.2 + github.com/AdguardTeam/golibs v0.27.0 github.com/AdguardTeam/urlfilter v0.19.0 github.com/NYTimes/gziphandler v1.1.1 github.com/ameshkov/dnscrypt/v2 v2.3.0 diff --git a/go.sum b/go.sum index cc2d4459..763e14a0 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ -github.com/AdguardTeam/dnsproxy v0.73.0 h1:E1fxzosMqExZH8h7OJnKXLxyktcAFRJapLF4+nKULms= -github.com/AdguardTeam/dnsproxy v0.73.0/go.mod h1:ZcvmyQY2EiX5B0yCTkiYTgtm+1lBWA0lajbEI9dOhW4= -github.com/AdguardTeam/golibs v0.26.0 h1:uLL0XggEjB+87lL1tPpEAQNoKAlHDq5AyBUVWEgf63E= -github.com/AdguardTeam/golibs v0.26.0/go.mod h1:iWdjXPCwmK2g2FKIb/OwEPnovSXeMqRhI8FWLxF5oxE= +github.com/AdguardTeam/dnsproxy v0.73.2 h1:O6wRXzHsnWL5TkhYcuLWCShVFF0X5RFI6qUmq1ZFVsQ= +github.com/AdguardTeam/dnsproxy v0.73.2/go.mod h1:zD5WfTctbRvYYk8PS39h6/OT84NTu6QxKbAiBN5PUcI= +github.com/AdguardTeam/golibs v0.27.0 h1:YxCFK6HBGp/ZXp3bv5uei+oLH12UfIYB8u2rh1B6nnU= +github.com/AdguardTeam/golibs v0.27.0/go.mod h1:iWdjXPCwmK2g2FKIb/OwEPnovSXeMqRhI8FWLxF5oxE= github.com/AdguardTeam/urlfilter v0.19.0 h1:q7eH13+yNETlpD/VD3u5rLQOripcUdEktqZFy+KiQLk= github.com/AdguardTeam/urlfilter v0.19.0/go.mod h1:+N54ZvxqXYLnXuvpaUhK2exDQW+djZBRSb6F6j0rkBY= github.com/NYTimes/gziphandler v1.1.1 h1:ZUDjpQae29j0ryrS0u/B8HZfJBtBQHjqw2rQ2cqUQ3I= diff --git a/internal/aghos/os.go b/internal/aghos/os.go index f2933a98..2ac99464 100644 --- a/internal/aghos/os.go +++ b/internal/aghos/os.go @@ -19,6 +19,12 @@ import ( "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]. func Unsupported(op string) (err error) { return fmt.Errorf("%s: not supported on %s: %w", op, runtime.GOOS, errors.ErrUnsupported) diff --git a/internal/configmigrate/configmigrate.go b/internal/configmigrate/configmigrate.go index 6e8845e0..cba61247 100644 --- a/internal/configmigrate/configmigrate.go +++ b/internal/configmigrate/configmigrate.go @@ -2,4 +2,4 @@ package configmigrate // LastSchemaVersion is the most recent schema version. -const LastSchemaVersion uint = 28 +const LastSchemaVersion uint = 29 diff --git a/internal/configmigrate/migrations_internal_test.go b/internal/configmigrate/migrations_internal_test.go index 5349102f..34bbb847 100644 --- a/internal/configmigrate/migrations_internal_test.go +++ b/internal/configmigrate/migrations_internal_test.go @@ -19,6 +19,7 @@ func TestUpgradeSchema1to2(t *testing.T) { m := New(&Config{ WorkingDir: "", + DataDir: "", }) err := m.migrateTo2(diskConf) diff --git a/internal/configmigrate/migrator.go b/internal/configmigrate/migrator.go index ebdf6ba7..70a80f8b 100644 --- a/internal/configmigrate/migrator.go +++ b/internal/configmigrate/migrator.go @@ -10,20 +10,24 @@ import ( // Config is a the configuration for initializing a [Migrator]. 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 + + // DataDir is the absolute path to the data directory of AdGuardHome. + DataDir string } // Migrator performs the YAML configuration file migrations. type Migrator struct { - // workingDir is an absolute path to the working directory of AdGuardHome. workingDir string + dataDir string } // New creates a new Migrator. -func New(cfg *Config) (m *Migrator) { +func New(c *Config) (m *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, 26: migrateTo27, 27: migrateTo28, + 28: m.migrateTo29, } for i, migrate := range upgrades[current:target] { diff --git a/internal/configmigrate/migrator_test.go b/internal/configmigrate/migrator_test.go index 442713a4..cc6672ad 100644 --- a/internal/configmigrate/migrator_test.go +++ b/internal/configmigrate/migrator_test.go @@ -4,6 +4,7 @@ import ( "io/fs" "os" "path" + "path/filepath" "testing" "github.com/AdguardTeam/AdGuardHome/internal/configmigrate" @@ -190,6 +191,10 @@ func TestMigrateConfig_Migrate(t *testing.T) { yamlEqFunc: require.YAMLEq, name: "v27", targetVersion: 27, + }, { + yamlEqFunc: require.YAMLEq, + name: "v29", + targetVersion: 29, }} for _, tc := range testCases { @@ -202,6 +207,7 @@ func TestMigrateConfig_Migrate(t *testing.T) { migrator := configmigrate.New(&configmigrate.Config{ WorkingDir: t.Name(), + DataDir: filepath.Join(t.Name(), "data"), }) newBody, upgraded, err := migrator.Migrate(body, tc.targetVersion) require.NoError(t, err) diff --git a/internal/configmigrate/testdata/TestMigrateConfig_Migrate/v29/input.yml b/internal/configmigrate/testdata/TestMigrateConfig_Migrate/v29/input.yml new file mode 100644 index 00000000..909fdd5f --- /dev/null +++ b/internal/configmigrate/testdata/TestMigrateConfig_Migrate/v29/input.yml @@ -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 diff --git a/internal/configmigrate/testdata/TestMigrateConfig_Migrate/v29/output.yml b/internal/configmigrate/testdata/TestMigrateConfig_Migrate/v29/output.yml new file mode 100644 index 00000000..e698551b --- /dev/null +++ b/internal/configmigrate/testdata/TestMigrateConfig_Migrate/v29/output.yml @@ -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 diff --git a/internal/configmigrate/v29.go b/internal/configmigrate/v29.go new file mode 100644 index 00000000..fcd04bef --- /dev/null +++ b/internal/configmigrate/v29.go @@ -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 +} diff --git a/internal/dhcpd/db.go b/internal/dhcpd/db.go index aa482f2e..d3ca53bf 100644 --- a/internal/dhcpd/db.go +++ b/internal/dhcpd/db.go @@ -12,6 +12,7 @@ import ( "strings" "time" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/AdGuardHome/internal/dhcpsvc" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" @@ -185,7 +186,7 @@ func writeDB(path string, leases []*dbLease) (err error) { return err } - err = maybe.WriteFile(path, buf, 0o644) + err = maybe.WriteFile(path, buf, aghos.DefaultPermFile) if err != nil { // Don't wrap the error since it's informative enough as is. return err diff --git a/internal/filtering/filter.go b/internal/filtering/filter.go index 15bc14d9..0dd3471c 100644 --- a/internal/filtering/filter.go +++ b/internal/filtering/filter.go @@ -11,6 +11,7 @@ import ( "strings" "time" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/AdGuardHome/internal/aghrenameio" "github.com/AdguardTeam/AdGuardHome/internal/filtering/rulelist" "github.com/AdguardTeam/golibs/container" @@ -448,11 +449,7 @@ func (d *DNSFilter) updateIntl(flt *FilterYAML) (ok bool, err error) { var res *rulelist.ParseResult - // Change the default 0o600 permission to something more acceptable by end - // users. - // - // See https://github.com/AdguardTeam/AdGuardHome/issues/3198. - tmpFile, err := aghrenameio.NewPendingFile(flt.Path(d.conf.DataDir), 0o644) + tmpFile, err := aghrenameio.NewPendingFile(flt.Path(d.conf.DataDir), aghos.DefaultPermFile) if err != nil { return false, err } @@ -522,6 +519,11 @@ func (d *DNSFilter) reader(fltURL string) (r io.ReadCloser, err error) { 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) if err != nil { return nil, fmt.Errorf("opening file: %w", err) diff --git a/internal/filtering/filtering.go b/internal/filtering/filtering.go index 55404b74..8836515c 100644 --- a/internal/filtering/filtering.go +++ b/internal/filtering/filtering.go @@ -19,6 +19,7 @@ import ( "time" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/AdGuardHome/internal/filtering/rulelist" "github.com/AdguardTeam/golibs/container" "github.com/AdguardTeam/golibs/errors" @@ -130,6 +131,10 @@ type Config struct { // UserRules is the global list of custom rules. 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) SafeSearchCacheSize uint `yaml:"safesearch_cache_size"` // (in bytes) ParentalCacheSize uint `yaml:"parental_cache_size"` // (in bytes) @@ -257,6 +262,8 @@ type DNSFilter struct { refreshLock *sync.Mutex hostCheckers []hostChecker + + safeFSPatterns []string } // Filter represents a filter list @@ -987,13 +994,22 @@ func New(c *Config, blockFilters []Filter) (d *DNSFilter, err error) { d = &DNSFilter{ idGen: newIDGenerator(int32(time.Now().Unix())), bufPool: syncutil.NewSlicePool[byte](rulelist.DefaultRuleBufSize), + safeSearch: c.SafeSearch, refreshLock: &sync.Mutex{}, safeBrowsingChecker: c.SafeBrowsingChecker, parentalControlChecker: c.ParentalControlChecker, 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{{ check: d.matchSysHosts, @@ -1022,7 +1038,7 @@ func New(c *Config, blockFilters []Filter) (d *DNSFilter, err error) { err = d.prepareRewrites() if err != nil { - return nil, fmt.Errorf("rewrites: preparing: %s", err) + return nil, fmt.Errorf("rewrites: preparing: %w", err) } if d.conf.BlockedServices != nil { @@ -1037,11 +1053,16 @@ func New(c *Config, blockFilters []Filter) (d *DNSFilter, err error) { if err != nil { 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.WhitelistFilters) diff --git a/internal/filtering/http.go b/internal/filtering/http.go index 3a13ccb3..e459cb2f 100644 --- a/internal/filtering/http.go +++ b/internal/filtering/http.go @@ -20,14 +20,22 @@ import ( ) // 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") }() if filepath.IsAbs(urlStr) { + urlStr = filepath.Clean(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. - return err + if !pathMatchesAny(d.safeFSPatterns, urlStr) { + return fmt.Errorf("path %q does not match safe patterns", urlStr) + } + + return nil } u, err := url.ParseRequestURI(urlStr) @@ -65,7 +73,7 @@ func (d *DNSFilter) handleFilteringAddURL(w http.ResponseWriter, r *http.Request return } - err = validateFilterURL(fj.URL) + err = d.validateFilterURL(fj.URL) if err != nil { aghhttp.Error(r, w, http.StatusBadRequest, "%s", err) @@ -225,7 +233,7 @@ func (d *DNSFilter) handleFilteringSetURL(w http.ResponseWriter, r *http.Request return } - err = validateFilterURL(fj.Data.URL) + err = d.validateFilterURL(fj.Data.URL) if err != nil { aghhttp.Error(r, w, http.StatusBadRequest, "invalid url: %s", err) diff --git a/internal/filtering/path.go b/internal/filtering/path.go new file mode 100644 index 00000000..a52a6ebe --- /dev/null +++ b/internal/filtering/path.go @@ -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 +} diff --git a/internal/filtering/path_unix_internal_test.go b/internal/filtering/path_unix_internal_test.go new file mode 100644 index 00000000..09139979 --- /dev/null +++ b/internal/filtering/path_unix_internal_test.go @@ -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) + }) + })) +} diff --git a/internal/filtering/path_windows_internal_test.go b/internal/filtering/path_windows_internal_test.go new file mode 100644 index 00000000..cbabc243 --- /dev/null +++ b/internal/filtering/path_windows_internal_test.go @@ -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. +} diff --git a/internal/filtering/rulelist/filter.go b/internal/filtering/rulelist/filter.go index 5f3fa6be..a29897de 100644 --- a/internal/filtering/rulelist/filter.go +++ b/internal/filtering/rulelist/filter.go @@ -11,6 +11,7 @@ import ( "path/filepath" "time" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/AdGuardHome/internal/aghrenameio" "github.com/AdguardTeam/golibs/errors" "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) } - fltFile, err := aghrenameio.NewPendingFile(cachePath, 0o644) + fltFile, err := aghrenameio.NewPendingFile(cachePath, aghos.DefaultPermFile) if err != nil { return "", nil, fmt.Errorf("creating temp file: %w", err) } @@ -271,7 +272,7 @@ func parseIntoCache( filePath string, cachePath string, ) (parseRes *ParseResult, err error) { - tmpFile, err := aghrenameio.NewPendingFile(cachePath, 0o644) + tmpFile, err := aghrenameio.NewPendingFile(cachePath, aghos.DefaultPermFile) if err != nil { return nil, fmt.Errorf("creating temp file: %w", err) } diff --git a/internal/home/auth.go b/internal/home/auth.go index 18bc5668..5b6cf9b8 100644 --- a/internal/home/auth.go +++ b/internal/home/auth.go @@ -9,6 +9,7 @@ import ( "sync" "time" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/netutil" @@ -89,7 +90,7 @@ func InitAuth( trustedProxies: trustedProxies, } var err error - a.db, err = bbolt.Open(dbFilename, 0o644, nil) + a.db, err = bbolt.Open(dbFilename, aghos.DefaultPermFile, nil) if err != nil { log.Error("auth: open DB: %s: %s", dbFilename, err) if err.Error() == "invalid argument" { diff --git a/internal/home/clients.go b/internal/home/clients.go index 66a44a62..5a30d6bc 100644 --- a/internal/home/clients.go +++ b/internal/home/clients.go @@ -72,10 +72,13 @@ func (clients *clientsContainer) Init( return errors.Error("clients container already initialized") } + clients.safeSearchCacheSize = filteringConf.SafeSearchCacheSize + clients.safeSearchCacheTTL = time.Minute * time.Duration(filteringConf.CacheTime) + confClients := make([]*client.Persistent, 0, len(objects)) for i, o := range objects { var p *client.Persistent - p, err = o.toPersistent(filteringConf) + p, err = o.toPersistent(clients.safeSearchCacheSize, clients.safeSearchCacheTTL) if err != nil { return fmt.Errorf("init persistent client at index %d: %w", i, err) } @@ -165,7 +168,8 @@ type clientObject struct { // toPersistent returns an initialized persistent client if there are no errors. func (o *clientObject) toPersistent( - filteringConf *filtering.Config, + safeSearchCacheSize uint, + safeSearchCacheTTL time.Duration, ) (cli *client.Persistent, err error) { cli = &client.Persistent{ Name: o.Name, @@ -201,8 +205,8 @@ func (o *clientObject) toPersistent( if o.SafeSearchConf.Enabled { err = cli.SetSafeSearch( o.SafeSearchConf, - filteringConf.SafeSearchCacheSize, - time.Minute*time.Duration(filteringConf.CacheTime), + safeSearchCacheSize, + safeSearchCacheTTL, ) if err != nil { return nil, fmt.Errorf("init safesearch %q: %w", cli.Name, err) diff --git a/internal/home/config.go b/internal/home/config.go index b8d69dae..20d2eb97 100644 --- a/internal/home/config.go +++ b/internal/home/config.go @@ -9,6 +9,7 @@ import ( "sync" "github.com/AdguardTeam/AdGuardHome/internal/aghalg" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/AdGuardHome/internal/aghtls" "github.com/AdguardTeam/AdGuardHome/internal/configmigrate" "github.com/AdguardTeam/AdGuardHome/internal/dhcpd" @@ -26,9 +27,15 @@ import ( yaml "gopkg.in/yaml.v3" ) -// dataDir is the name of a directory under the working one to store some -// persistent data. -const dataDir = "data" +const ( + // dataDir is the name of a directory under the working one to store some + // 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. type logSettings struct { @@ -520,6 +527,7 @@ func parseConfig() (err error) { migrator := configmigrate.New(&configmigrate.Config{ WorkingDir: Context.workDir, + DataDir: Context.getDataDir(), }) var upgraded bool @@ -534,7 +542,7 @@ func parseConfig() (err error) { confPath := configFilePath() 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 { 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) } - err = maybe.WriteFile(confPath, buf.Bytes(), 0o644) + err = maybe.WriteFile(confPath, buf.Bytes(), aghos.DefaultPermFile) if err != nil { return fmt.Errorf("writing config file: %w", err) } diff --git a/internal/home/controlinstall.go b/internal/home/controlinstall.go index f94457d0..3051f0f6 100644 --- a/internal/home/controlinstall.go +++ b/internal/home/controlinstall.go @@ -270,9 +270,9 @@ DNSStubListener=no const resolvConfPath = "/etc/resolv.conf" // Deactivate DNSStubListener -func disableDNSStubListener() error { +func disableDNSStubListener() (err error) { dir := filepath.Dir(resolvedConfPath) - err := os.MkdirAll(dir, 0o755) + err = os.MkdirAll(dir, 0o755) if err != nil { 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) Context.firstRun = false - config.HTTPConfig.Address = netip.AddrPortFrom(req.Web.IP, req.Web.Port) config.DNS.BindHosts = []netip.Addr{req.DNS.IP} 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{ Name: req.Username, diff --git a/internal/home/dns.go b/internal/home/dns.go index 9dd711f5..f0c6d4de 100644 --- a/internal/home/dns.go +++ b/internal/home/dns.go @@ -47,14 +47,9 @@ func onConfigModified() { // 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 // [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() - statsDir, querylogDir, err := checkStatsAndQuerylogDirs(&Context, config) - if err != nil { - return err - } - statsConf := stats.Config{ Logger: l.With(slogutil.KeyPrefix, "stats"), Filename: filepath.Join(statsDir, "stats.db"), diff --git a/internal/home/home.go b/internal/home/home.go index 69a67223..df4e4296 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -31,6 +31,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/filtering" "github.com/AdguardTeam/AdGuardHome/internal/filtering/hashprefix" "github.com/AdguardTeam/AdGuardHome/internal/filtering/safesearch" + "github.com/AdguardTeam/AdGuardHome/internal/permcheck" "github.com/AdguardTeam/AdGuardHome/internal/querylog" "github.com/AdguardTeam/AdGuardHome/internal/stats" "github.com/AdguardTeam/AdGuardHome/internal/updater" @@ -630,9 +631,9 @@ func run(opts options, clientBuildFS fs.FS, done chan struct{}) { } } - dir := Context.getDataDir() - err = os.MkdirAll(dir, 0o755) - fatalOnError(errors.Annotate(err, "creating DNS data dir at %s: %w", dir)) + dataDir := Context.getDataDir() + err = os.MkdirAll(dataDir, aghos.DefaultPermDir) + fatalOnError(errors.Annotate(err, "creating DNS data dir at %s: %w", dataDir)) 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) fatalOnError(err) + statsDir, querylogDir, err := checkStatsAndQuerylogDirs(&Context, config) + fatalOnError(err) + if !Context.firstRun { - err = initDNS(slogLogger) + err = initDNS(slogLogger, statsDir, querylogDir) fatalOnError(err) 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() // 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 // not be nil. 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 { return err } diff --git a/internal/next/configmgr/configmgr.go b/internal/next/configmgr/configmgr.go index 84e4e61a..a75f1304 100644 --- a/internal/next/configmgr/configmgr.go +++ b/internal/next/configmgr/configmgr.go @@ -14,6 +14,7 @@ import ( "sync" "time" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/AdGuardHome/internal/next/agh" "github.com/AdguardTeam/AdGuardHome/internal/next/dnssvc" "github.com/AdguardTeam/AdGuardHome/internal/next/websvc" @@ -182,7 +183,7 @@ func (m *Manager) write() (err error) { return fmt.Errorf("encoding: %w", err) } - err = maybe.WriteFile(m.fileName, b, 0o644) + err = maybe.WriteFile(m.fileName, b, aghos.DefaultPermFile) if err != nil { return fmt.Errorf("writing: %w", err) } diff --git a/internal/permcheck/migrate.go b/internal/permcheck/migrate.go new file mode 100644 index 00000000..b052bffa --- /dev/null +++ b/internal/permcheck/migrate.go @@ -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, + ) +} diff --git a/internal/permcheck/permcheck.go b/internal/permcheck/permcheck.go new file mode 100644 index 00000000..aea4a743 --- /dev/null +++ b/internal/permcheck/permcheck.go @@ -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, + ) + } +} diff --git a/internal/querylog/qlogfile.go b/internal/querylog/qlogfile.go index 397840c9..175420ea 100644 --- a/internal/querylog/qlogfile.go +++ b/internal/querylog/qlogfile.go @@ -8,6 +8,7 @@ import ( "sync" "time" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" ) @@ -56,7 +57,7 @@ type qLogFile struct { // newQLogFile initializes a new instance of the qLogFile. 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 { return nil, err } diff --git a/internal/querylog/querylogfile.go b/internal/querylog/querylogfile.go index 3783376f..412a364b 100644 --- a/internal/querylog/querylogfile.go +++ b/internal/querylog/querylogfile.go @@ -7,6 +7,7 @@ import ( "os" "time" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" ) @@ -70,7 +71,7 @@ func (l *queryLog) flushToFile(b *bytes.Buffer) (err error) { 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 { return fmt.Errorf("creating file %q: %w", filename, err) } diff --git a/internal/stats/stats.go b/internal/stats/stats.go index c6de4c66..a3a0f62e 100644 --- a/internal/stats/stats.go +++ b/internal/stats/stats.go @@ -15,6 +15,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" "github.com/AdguardTeam/AdGuardHome/internal/aghnet" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/timeutil" @@ -383,7 +384,7 @@ func (s *StatsCtx) openDB() (err error) { s.logger.Debug("opening database") 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.Error() == "invalid argument" { const lines = `AdGuard Home cannot be initialized due to an incompatible file system. diff --git a/internal/tools/go.mod b/internal/tools/go.mod index 597d929c..91263fef 100644 --- a/internal/tools/go.mod +++ b/internal/tools/go.mod @@ -1,6 +1,6 @@ module github.com/AdguardTeam/AdGuardHome/internal/tools -go 1.23.1 +go 1.23.2 require ( github.com/fzipp/gocyclo v0.6.0 @@ -20,7 +20,7 @@ require ( require ( cloud.google.com/go v0.115.1 // indirect cloud.google.com/go/ai v0.8.2 // indirect - cloud.google.com/go/auth v0.9.5 // indirect + cloud.google.com/go/auth v0.9.7 // indirect cloud.google.com/go/auth/oauth2adapt v0.2.4 // indirect cloud.google.com/go/compute/metadata v0.5.2 // indirect cloud.google.com/go/longrunning v0.6.1 // indirect @@ -59,7 +59,7 @@ require ( google.golang.org/api v0.199.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240930140551-af27646dc61f // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240930140551-af27646dc61f // indirect - google.golang.org/grpc v1.67.0 // indirect + google.golang.org/grpc v1.67.1 // indirect google.golang.org/protobuf v1.34.2 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/internal/tools/go.sum b/internal/tools/go.sum index 2415c9f0..3b923a2d 100644 --- a/internal/tools/go.sum +++ b/internal/tools/go.sum @@ -3,8 +3,8 @@ cloud.google.com/go v0.115.1 h1:Jo0SM9cQnSkYfp44+v+NQXHpcHqlnRJk2qxh6yvxxxQ= cloud.google.com/go v0.115.1/go.mod h1:DuujITeaufu3gL68/lOFIirVNJwQeyf5UXyi+Wbgknc= cloud.google.com/go/ai v0.8.2 h1:LEaQwqBv+k2ybrcdTtCTc9OPZXoEdcQaGrfvDYS6Bnk= cloud.google.com/go/ai v0.8.2/go.mod h1:Wb3EUUGWwB6yHBaUf/+oxUq/6XbCaU1yh0GrwUS8lr4= -cloud.google.com/go/auth v0.9.5 h1:4CTn43Eynw40aFVr3GpPqsQponx2jv0BQpjvajsbbzw= -cloud.google.com/go/auth v0.9.5/go.mod h1:Xo0n7n66eHyOWWCnitop6870Ilwo3PiZyodVkkH1xWM= +cloud.google.com/go/auth v0.9.7 h1:ha65jNwOfI48YmUzNfMaUDfqt5ykuYIUnSartpU1+BA= +cloud.google.com/go/auth v0.9.7/go.mod h1:Xo0n7n66eHyOWWCnitop6870Ilwo3PiZyodVkkH1xWM= cloud.google.com/go/auth/oauth2adapt v0.2.4 h1:0GWE/FUsXhf6C+jAkWgYm7X9tK8cuEIfy19DBn6B6bY= cloud.google.com/go/auth/oauth2adapt v0.2.4/go.mod h1:jC/jOpwFP6JBxhB3P5Rr0a9HLMC/Pe3eaL4NmdvqPtc= cloud.google.com/go/compute/metadata v0.5.2 h1:UxK4uu/Tn+I3p2dYWTfiX4wva7aYlKixAHn3fyqngqo= @@ -224,8 +224,8 @@ google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyac google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY= google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= google.golang.org/grpc v1.33.2/go.mod h1:JMHMWHQWaTccqQQlmk3MJZS+GWXOdAesneDmEnv2fbc= -google.golang.org/grpc v1.67.0 h1:IdH9y6PF5MPSdAntIcpjQ+tXO41pcQsfZV2RxtQgVcw= -google.golang.org/grpc v1.67.0/go.mod h1:1gLDyUQU7CTLJI90u3nXZ9ekeghjeM7pTDZlqFNg2AA= +google.golang.org/grpc v1.67.1 h1:zWnc1Vrcno+lHZCOofnIMvycFcc0QRGIzm9dhnDX68E= +google.golang.org/grpc v1.67.1/go.mod h1:1gLDyUQU7CTLJI90u3nXZ9ekeghjeM7pTDZlqFNg2AA= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= diff --git a/internal/updater/updater.go b/internal/updater/updater.go index 4e4a21d4..b3e85dee 100644 --- a/internal/updater/updater.go +++ b/internal/updater/updater.go @@ -15,6 +15,7 @@ import ( "sync" "time" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/AdGuardHome/internal/version" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/ioutil" @@ -263,7 +264,7 @@ func (u *Updater) check() (err error) { // ignores the configuration file if firstRun is true. func (u *Updater) backup(firstRun bool) (err error) { log.Debug("updater: backing up current configuration") - _ = os.Mkdir(u.backupDir, 0o755) + _ = os.Mkdir(u.backupDir, aghos.DefaultPermDir) if !firstRun { err = copyFile(u.confName, filepath.Join(u.backupDir, "AdGuardHome.yaml")) if err != nil { @@ -337,10 +338,10 @@ func (u *Updater) downloadPackageFile() (err error) { 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") - err = os.WriteFile(u.packageName, body, 0o644) + err = os.WriteFile(u.packageName, body, aghos.DefaultPermFile) if err != nil { return fmt.Errorf("os.WriteFile() failed: %w", err) } @@ -527,7 +528,7 @@ func copyFile(src, dst string) error { if e != nil { return e } - e = os.WriteFile(dst, d, 0o644) + e = os.WriteFile(dst, d, aghos.DefaultPermFile) if e != nil { return e } diff --git a/scripts/install.sh b/scripts/install.sh index 08c54e87..31184cb0 100644 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -497,7 +497,7 @@ download() { # Function unpack unpacks the passed archive depending on it's extension. unpack() { log "unpacking package from $pkg_name into $out_dir" - if ! mkdir -p "$out_dir" + if ! mkdir -m 0700 -p "$out_dir" then error_exit "cannot create directory $out_dir" fi