From 95676e8d7276d99e5297fe74d8ea49bfffcd7e83 Mon Sep 17 00:00:00 2001 From: dangerousplay Date: Wed, 11 Sep 2024 19:21:27 -0300 Subject: [PATCH 1/3] [Added] GPO list timeout daemon configuration option Signed-off-by: Davi Henrique --- cmd/adsysd/daemon/daemon.go | 12 +++++++++--- conf.example/adsys.yaml | 3 +++ docs/reference/adsys-daemon.md | 6 ++++++ internal/ad/ad.go | 2 +- internal/adsysservice/adsysservice.go | 12 +++++++++++- internal/consts/consts.go | 4 +--- 6 files changed, 31 insertions(+), 8 deletions(-) diff --git a/cmd/adsysd/daemon/daemon.go b/cmd/adsysd/daemon/daemon.go index df1677a76..936847709 100644 --- a/cmd/adsysd/daemon/daemon.go +++ b/cmd/adsysd/daemon/daemon.go @@ -51,9 +51,10 @@ type daemonConfig struct { SystemUnitDir string `mapstructure:"systemunit_dir"` GlobalTrustDir string `mapstructure:"global_trust_dir"` - AdBackend string `mapstructure:"ad_backend"` - SSSdConfig sss.Config `mapstructure:"sssd"` - WinbindConfig winbind.Config `mapstructure:"winbind"` + AdBackend string `mapstructure:"ad_backend"` + SSSdConfig sss.Config `mapstructure:"sssd"` + WinbindConfig winbind.Config `mapstructure:"winbind"` + GpoListTimeout int `mapstructure:"gpo_list_timeout"` ServiceTimeout int `mapstructure:"service_timeout"` } @@ -125,6 +126,7 @@ func New() *App { adsysservice.WithADBackend(a.config.AdBackend), adsysservice.WithSSSConfig(a.config.SSSdConfig), adsysservice.WithWinbindConfig(a.config.WinbindConfig), + adsysservice.WithGpoListTimeout(time.Second*time.Duration(a.config.GpoListTimeout)), ) if err != nil { close(a.ready) @@ -163,6 +165,10 @@ func New() *App { err = a.viper.BindPFlag("service_timeout", a.rootCmd.PersistentFlags().Lookup("timeout")) decorate.LogOnError(&err) + a.rootCmd.PersistentFlags().IntP("gpo-list-timeout", "", consts.DefaultGpoListTimeout, gotext.Get("time in seconds for the gpo list. 0 for no timeout.")) + err = a.viper.BindPFlag("gpo_list_timeout", a.rootCmd.PersistentFlags().Lookup("gpo-list-timeout")) + decorate.LogOnError(&err) + a.rootCmd.PersistentFlags().StringP("ad-backend", "", "sssd", gotext.Get("Active Directory authentication backend")) err = a.viper.BindPFlag("ad_backend", a.rootCmd.PersistentFlags().Lookup("ad-backend")) decorate.LogOnError(&err) diff --git a/conf.example/adsys.yaml b/conf.example/adsys.yaml index 28257ed71..f465d7ee5 100644 --- a/conf.example/adsys.yaml +++ b/conf.example/adsys.yaml @@ -36,3 +36,6 @@ detect_cached_ticket: false # Client only configuration client_timeout: 60 + +# GPO List timeout +gpo_list_timeout: 30 diff --git a/docs/reference/adsys-daemon.md b/docs/reference/adsys-daemon.md index 5912efca3..81b89d86e 100644 --- a/docs/reference/adsys-daemon.md +++ b/docs/reference/adsys-daemon.md @@ -177,6 +177,12 @@ A custom domain can be used to override the C API call that ADSys executes to de A custom domain controller can be used to override the C API call that ADSys executes to determine the AD controller FQDN -- which is returned by `wbinfo --dsgetdcname domain.com` (e.g. `adc.example.com`). +### GPO configuration: + +* **gpo_list_timeout** + +Maximum time in seconds for the GPO list to finish otherwise the GPO list is aborted. This can be overridden by the `--gpo-list-timeout` option. Defaults to 10 seconds. + ### Client only configuration:** * **client_timeout** diff --git a/internal/ad/ad.go b/internal/ad/ad.go index 6036a20a5..cf9bddfe2 100644 --- a/internal/ad/ad.go +++ b/internal/ad/ad.go @@ -254,7 +254,7 @@ func (ad *AD) GetPolicies(ctx context.Context, objectName string, objectClass Ob args := append([]string{}, ad.gpoListCmd...) // Copy gpoListCmd to prevent data race scriptArgs := []string{"--objectclass", string(objectClass), adServerFQDN, objectName} cmdArgs := append(args, scriptArgs...) - cmdCtx, cancel := context.WithTimeout(ctx, time.Second*10) + cmdCtx, cancel := context.WithTimeout(ctx, ad.gpoListTimeout) defer cancel() log.Debugf(ctx, "Getting gpo list with arguments: %q", strings.Join(scriptArgs, " ")) // #nosec G204 - cmdArgs is under our control (python embedded script or mock for tests) diff --git a/internal/adsysservice/adsysservice.go b/internal/adsysservice/adsysservice.go index 6955e201c..a6902a66f 100644 --- a/internal/adsysservice/adsysservice.go +++ b/internal/adsysservice/adsysservice.go @@ -71,6 +71,7 @@ type options struct { systemUnitDir string globalTrustDir string adBackend string + gpoListTimeout time.Duration sssConfig sss.Config winbindConfig winbind.Config authorizer authorizerer @@ -187,6 +188,14 @@ func WithWinbindConfig(c winbind.Config) func(o *options) error { } } +// WithGpoListTimeout specifies the timeout for the gpo list +func WithGpoListTimeout(t time.Duration) func(o *options) error { + return func(o *options) error { + o.gpoListTimeout = t + return nil + } +} + // New returns a new instance of an AD service. // If url or domain is empty, we load the missing parameters from sssd.conf, taking first // domain in the list if not provided. @@ -241,7 +250,8 @@ func New(ctx context.Context, opts ...option) (s *Service, err error) { if args.runDir != "" { adOptions = append(adOptions, ad.WithRunDir(args.runDir)) } - adOptions = append(adOptions, ad.WithGpoListTimeout(consts.DefaultGpoListTimeout)) + + adOptions = append(adOptions, ad.WithGpoListTimeout(args.gpoListTimeout)) hostname, err := os.Hostname() if err != nil { diff --git a/internal/consts/consts.go b/internal/consts/consts.go index b0350c7ed..39c2c1c49 100644 --- a/internal/consts/consts.go +++ b/internal/consts/consts.go @@ -2,8 +2,6 @@ package consts import ( - "time" - log "github.com/sirupsen/logrus" ) @@ -41,7 +39,7 @@ const ( DefaultServiceTimeout = 120 // DefaultGpoListTimeout is the default time to wait for the GPO list subcommand to finish. - DefaultGpoListTimeout = 10 * time.Second + DefaultGpoListTimeout = 10 // DistroID is the distro ID which can be overridden at build time. DistroID = "Ubuntu" From 50024225de93e61c7699db2b7b9c9e75a13d2011 Mon Sep 17 00:00:00 2001 From: Davi Henrique Date: Fri, 13 Sep 2024 10:22:08 -0300 Subject: [PATCH 2/3] [Changed] Adsys configuration example gpo list timeout to 10 seconds Signed-off-by: Davi Henrique --- conf.example/adsys.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conf.example/adsys.yaml b/conf.example/adsys.yaml index f465d7ee5..a5052c6d6 100644 --- a/conf.example/adsys.yaml +++ b/conf.example/adsys.yaml @@ -38,4 +38,4 @@ detect_cached_ticket: false client_timeout: 60 # GPO List timeout -gpo_list_timeout: 30 +gpo_list_timeout: 10 From 73212e13da76a59df7f25994b2ad48ad066e259f Mon Sep 17 00:00:00 2001 From: Didier Roche Date: Fri, 13 Sep 2024 15:47:53 +0200 Subject: [PATCH 3/3] Capitalize GPO for spelling check and fix gpo false positive check Generated documentation should have capitalized GPO for consistency across the doc. However, as gpo is also used as an argument of the config, put it in the allowed list. --- cmd/adsysd/daemon/daemon.go | 2 +- docs/.custom_wordlist.txt | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/adsysd/daemon/daemon.go b/cmd/adsysd/daemon/daemon.go index 936847709..aefa7a582 100644 --- a/cmd/adsysd/daemon/daemon.go +++ b/cmd/adsysd/daemon/daemon.go @@ -165,7 +165,7 @@ func New() *App { err = a.viper.BindPFlag("service_timeout", a.rootCmd.PersistentFlags().Lookup("timeout")) decorate.LogOnError(&err) - a.rootCmd.PersistentFlags().IntP("gpo-list-timeout", "", consts.DefaultGpoListTimeout, gotext.Get("time in seconds for the gpo list. 0 for no timeout.")) + a.rootCmd.PersistentFlags().IntP("gpo-list-timeout", "", consts.DefaultGpoListTimeout, gotext.Get("time in seconds for the GPO list. 0 for no timeout.")) err = a.viper.BindPFlag("gpo_list_timeout", a.rootCmd.PersistentFlags().Lookup("gpo-list-timeout")) decorate.LogOnError(&err) diff --git a/docs/.custom_wordlist.txt b/docs/.custom_wordlist.txt index c8d3cc258..ac081cd62 100644 --- a/docs/.custom_wordlist.txt +++ b/docs/.custom_wordlist.txt @@ -42,6 +42,7 @@ FQDN GDM gdm GPL +gpo GPO gpolist GPOs