Merge remote-tracking branch 'base/master'

CHANGELOG.md

@@ -32,6 +32,14 @@ NOTE: Add new changes BELOW THIS COMMENT.
 
 - The release executables are now signed.
 
+### Added
+
+- The `--no-permcheck` command-line option to disable checking and migration of
+  permissions for the security-sensitive files and directories, which caused
+  issues on Windows ([#7400]).
+
+[#7400]: https://github.com/AdguardTeam/AdGuardHome/issues/7400
+
 [go-1.23.3]: https://groups.google.com/g/golang-announce/c/X5KodEJYuqI
 
 <!--

internal/client/storage.go

@@ -15,7 +15,6 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/whois"
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/hostsfile"
-	"github.com/AdguardTeam/golibs/log"
 	"github.com/AdguardTeam/golibs/logutil/slogutil"
 )
 
@@ -506,7 +505,7 @@ func (s *Storage) FindByMAC(mac net.HardwareAddr) (p *Persistent, ok bool) {
 
 // RemoveByName removes persistent client information.  ok is false if no such
 // client exists by that name.
-func (s *Storage) RemoveByName(name string) (ok bool) {
+func (s *Storage) RemoveByName(ctx context.Context, name string) (ok bool) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -516,7 +515,7 @@ func (s *Storage) RemoveByName(name string) (ok bool) {
 	}
 
 	if err := p.CloseUpstreams(); err != nil {
-		log.Error("client storage: removing client %q: %s", p.Name, err)
+		s.logger.ErrorContext(ctx, "removing client", "name", p.Name, slogutil.KeyError, err)
 	}
 
 	s.index.remove(p)

internal/client/storage_test.go

@@ -735,7 +735,7 @@ func TestStorage_RemoveByName(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			tc.want(t, s.RemoveByName(tc.cliName))
+			tc.want(t, s.RemoveByName(ctx, tc.cliName))
 		})
 	}
 
@@ -744,8 +744,8 @@ func TestStorage_RemoveByName(t *testing.T) {
 		err = s.Add(ctx, existingClient)
 		require.NoError(t, err)
 
-		assert.True(t, s.RemoveByName(existingName))
-		assert.False(t, s.RemoveByName(existingName))
+		assert.True(t, s.RemoveByName(ctx, existingName))
+		assert.False(t, s.RemoveByName(ctx, existingName))
 	})
 }
 

internal/home/clientshttp.go

@@ -445,7 +445,7 @@ func (clients *clientsContainer) handleDelClient(w http.ResponseWriter, r *http.
 		return
 	}
 
-	if !clients.storage.RemoveByName(cj.Name) {
+	if !clients.storage.RemoveByName(r.Context(), cj.Name) {
 		aghhttp.Error(r, w, http.StatusBadRequest, "Client not found")
 
 		return

internal/home/dns.go

@@ -48,11 +48,11 @@ 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, statsDir, querylogDir string) (err error) {
+func initDNS(baseLogger *slog.Logger, statsDir, querylogDir string) (err error) {
 	anonymizer := config.anonymizer()
 
 	statsConf := stats.Config{
-		Logger:            l.With(slogutil.KeyPrefix, "stats"),
+		Logger:            baseLogger.With(slogutil.KeyPrefix, "stats"),
 		Filename:          filepath.Join(statsDir, "stats.db"),
 		Limit:             config.Stats.Interval.Duration,
 		ConfigModified:    onConfigModified,
@@ -73,6 +73,7 @@ func initDNS(l *slog.Logger, statsDir, querylogDir string) (err error) {
 	}
 
 	conf := querylog.Config{
+		Logger:              baseLogger.With(slogutil.KeyPrefix, "querylog"),
 		Anonymizer:          anonymizer,
 		ConfigModified:      onConfigModified,
 		HTTPRegister:        httpRegister,
@@ -114,7 +115,7 @@ func initDNS(l *slog.Logger, statsDir, querylogDir string) (err error) {
 		anonymizer,
 		httpRegister,
 		tlsConf,
-		l,
+		baseLogger,
 	)
 }
 
@@ -464,7 +465,8 @@ func startDNSServer() error {
 	Context.filters.EnableFilters(false)
 
 	// TODO(s.chzhen):  Pass context.
-	err := Context.clients.Start(context.TODO())
+	ctx := context.TODO()
+	err := Context.clients.Start(ctx)
 	if err != nil {
 		return fmt.Errorf("starting clients container: %w", err)
 	}
@@ -476,7 +478,11 @@ func startDNSServer() error {
 
 	Context.filters.Start()
 	Context.stats.Start()
-	Context.queryLog.Start()
+
+	err = Context.queryLog.Start(ctx)
+	if err != nil {
+		return fmt.Errorf("starting query log: %w", err)
+	}
 
 	return nil
 }
@@ -532,12 +538,16 @@ func closeDNSServer() {
 	if Context.stats != nil {
 		err := Context.stats.Close()
 		if err != nil {
-			log.Debug("closing stats: %s", err)
+			log.Error("closing stats: %s", err)
 		}
 	}
 
 	if Context.queryLog != nil {
-		Context.queryLog.Close()
+		// TODO(s.chzhen):  Pass context.
+		err := Context.queryLog.Shutdown(context.TODO())
+		if err != nil {
+			log.Error("closing query log: %s", err)
+		}
 	}
 
 	log.Debug("all dns modules are closed")

internal/home/home.go

@@ -159,7 +159,7 @@ func setupContext(opts options) (err error) {
 
 	if Context.firstRun {
 		log.Info("This is the first time AdGuard Home is launched")
-		checkPermissions()
+		checkNetworkPermissions()
 
 		return nil
 	}
@@ -687,18 +687,26 @@ func run(opts options, clientBuildFS fs.FS, done chan struct{}) {
 		}
 	}
 
-	if permcheck.NeedsMigration(confPath) {
-		permcheck.Migrate(Context.workDir, dataDir, statsDir, querylogDir, confPath)
+	if !opts.noPermCheck {
+		checkPermissions(Context.workDir, confPath, dataDir, statsDir, querylogDir)
 	}
 
-	permcheck.Check(Context.workDir, dataDir, statsDir, querylogDir, confPath)
-
 	Context.web.start()
 
 	// Wait for other goroutines to complete their job.
 	<-done
 }
 
+// checkPermissions checks and migrates permissions of the files and directories
+// used by AdGuard Home, if needed.
+func checkPermissions(workDir, confPath, dataDir, statsDir, querylogDir string) {
+	if permcheck.NeedsMigration(confPath) {
+		permcheck.Migrate(workDir, dataDir, statsDir, querylogDir, confPath)
+	}
+
+	permcheck.Check(workDir, dataDir, statsDir, querylogDir, confPath)
+}
+
 // initUsers initializes context auth module.  Clears config users field.
 func initUsers() (auth *Auth, err error) {
 	sessFilename := filepath.Join(Context.getDataDir(), "sessions.db")
@@ -758,8 +766,9 @@ func startMods(l *slog.Logger) (err error) {
 	return nil
 }
 
-// Check if the current user permissions are enough to run AdGuard Home
-func checkPermissions() {
+// checkNetworkPermissions checks if the current user permissions are enough to
+// use the required networking functionality.
+func checkNetworkPermissions() {
 	log.Info("Checking if AdGuard Home has necessary permissions")
 
 	if ok, err := aghnet.CanBindPrivilegedPorts(); !ok || err != nil {

internal/home/options.go

@@ -78,6 +78,10 @@ type options struct {
 	// localFrontend forces AdGuard Home to use the frontend files from disk
 	// rather than the ones that have been compiled into the binary.
 	localFrontend bool
+
+	// noPermCheck disables checking and migration of permissions for the
+	// security-sensitive files.
+	noPermCheck bool
 }
 
 // initCmdLineOpts completes initialization of the global command-line option
@@ -305,6 +309,15 @@ var cmdLineOpts = []cmdLineOpt{{
 	description:     "Run in GL-Inet compatibility mode.",
 	longName:        "glinet",
 	shortName:       "",
+}, {
+	updateWithValue: nil,
+	updateNoValue:   func(o options) (options, error) { o.noPermCheck = true; return o, nil },
+	effect:          nil,
+	serialize:       func(o options) (val string, ok bool) { return "", o.noPermCheck },
+	description: "Skip checking and migration of permissions " +
+		"of security-sensitive files.",
+	longName:  "no-permcheck",
+	shortName: "",
 }, {
 	updateWithValue: nil,
 	updateNoValue:   nil,

internal/next/cmd/opt.go

@@ -89,6 +89,12 @@ type options struct {
 	// TODO(a.garipov): Use.
 	performUpdate bool
 
+	// noPermCheck, if true, instructs AdGuard Home to skip checking and
+	// migrating the permissions of its security-sensitive files.
+	//
+	// TODO(e.burkov):  Use.
+	noPermCheck bool
+
 	// verbose, if true, instructs AdGuard Home to enable verbose logging.
 	verbose bool
 
@@ -110,7 +116,8 @@ const (
 	disableUpdateIdx
 	glinetModeIdx
 	helpIdx
-	localFrontend
+	localFrontendIdx
+	noPermCheckIdx
 	performUpdateIdx
 	verboseIdx
 	versionIdx
@@ -214,14 +221,22 @@ var commandLineOptions = []*commandLineOption{
 		valueType:    "",
 	},
 
-	localFrontend: {
+	localFrontendIdx: {
 		defaultValue: false,
 		description:  "Use local frontend directories.",
 		long:         "local-frontend",
 		short:        "",
 		valueType:    "",
 	},
 
+	noPermCheckIdx: {
+		defaultValue: false,
+		description:  "Skip checking the permissions of security-sensitive files.",
+		long:         "no-permcheck",
+		short:        "",
+		valueType:    "",
+	},
+
 	performUpdateIdx: {
 		defaultValue: false,
 		description:  "Update the current binary and restart the service in case it's installed.",
@@ -264,7 +279,8 @@ func parseOptions(cmdName string, args []string) (opts *options, err error) {
 		disableUpdateIdx: &opts.disableUpdate,
 		glinetModeIdx:    &opts.glinetMode,
 		helpIdx:          &opts.help,
-		localFrontend:    &opts.localFrontend,
+		localFrontendIdx: &opts.localFrontend,
+		noPermCheckIdx:   &opts.noPermCheck,
 		performUpdateIdx: &opts.performUpdate,
 		verboseIdx:       &opts.verbose,
 		versionIdx:       &opts.version,