From 22e257d4e4f79246c3b1651b77b65a6c90995bf8 Mon Sep 17 00:00:00 2001 From: galitskiy Date: Thu, 10 Mar 2022 19:12:23 +0000 Subject: [PATCH 1/2] add cli argument to configure NAP log level --- build/Dockerfile | 10 ------ cmd/nginx-ingress/main.go | 33 ++++++++++++++++++- cmd/nginx-ingress/main_test.go | 30 +++++++++++++++++ .../templates/controller-daemonset.yaml | 3 ++ .../templates/controller-deployment.yaml | 3 ++ deployments/helm-chart/values.yaml | 2 ++ .../command-line-arguments.md | 11 +++++++ internal/nginx/fake_manager.go | 2 +- internal/nginx/manager.go | 28 +++++----------- 9 files changed, 91 insertions(+), 31 deletions(-) diff --git a/build/Dockerfile b/build/Dockerfile index c7998526c9..bc8d0dcc59 100644 --- a/build/Dockerfile +++ b/build/Dockerfile @@ -219,16 +219,6 @@ RUN --mount=target=/tmp [ -n "${BUILD_OS##*plus*}" ] && exit 0; mkdir -p etc/ngi RUN --mount=target=/tmp [ -n "${NAP_MODULES##*waf*}" ] && exit 0; mkdir -p /etc/nginx/waf/nac-policies /etc/nginx/waf/nac-logconfs /etc/nginx/waf/nac-usersigs /var/log/app_protect /opt/app_protect \ && chown -R nginx:0 /etc/app_protect /usr/share/ts /var/log/app_protect/ /opt/app_protect/ /var/log/nginx/ \ && touch /etc/nginx/waf/nac-usersigs/index.conf \ - && printf "%s\n" "MODULE = ALL;" "LOG_LEVEL = TS_CRIT;" "FILE = 2;" > /etc/app_protect/bd/logger.cfg \ - && printf "%s\n" "[config_set_compiler]" "log_level=fatal" >> /etc/app_protect/tools/asm_logging.conf \ - && for v in \ - asm_config_server \ - lock_factory \ - bd_agent \ - import_export_policy \ - set_active \ - ; do sed -i "/\[$v/a log_level=fatal" "/etc/app_protect/tools/asm_logging.conf" \ - ; done \ && cp -a /tmp/build/log-default.json /etc/nginx # run only on nap dos build diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index f2a93c6f19..c53f571046 100644 --- a/cmd/nginx-ingress/main.go +++ b/cmd/nginx-ingress/main.go @@ -71,6 +71,9 @@ var ( appProtect = flag.Bool("enable-app-protect", false, "Enable support for NGINX App Protect. Requires -nginx-plus.") + appProtectLogLevel = flag.String("app-protect-log-level", appProtectLogLevelDefault, + `Sets log level for App Protect. Allowed values: fatal, error, warn, info, debug, trace. Requires -nginx-plus and -enable-app-protect.`) + appProtectDos = flag.Bool("enable-app-protect-dos", false, "Enable support for NGINX App Protect dos. Requires -nginx-plus.") appProtectDosDebug = flag.Bool("app-protect-dos-debug", false, "Enable debugging for App Protect Dos. Requires -nginx-plus and -enable-app-protect-dos.") @@ -248,6 +251,17 @@ func main() { glog.Fatal("NGINX App Protect support is for NGINX Plus only") } + if *appProtectLogLevel != appProtectLogLevelDefault && !*appProtect && !*nginxPlus { + glog.Fatal("app-protect-log-level support is for NGINX Plus only and App Protect is enable") + } + + if *appProtectLogLevel != appProtectLogLevelDefault && *appProtect && *nginxPlus { + logLevelValidationError := validateAppProtectLogLevel(*appProtectLogLevel) + if logLevelValidationError != nil { + glog.Fatalf("Invalid value for app-protect-log-level: %v", *appProtectLogLevel) + } + } + if *appProtectDos && !*nginxPlus { glog.Fatal("NGINX App Protect Dos support is for NGINX Plus only") } @@ -449,7 +463,7 @@ func main() { aPPluginDone = make(chan error, 1) aPAgentDone = make(chan error, 1) - nginxManager.AppProtectAgentStart(aPAgentDone, *nginxDebug) + nginxManager.AppProtectAgentStart(aPAgentDone, *appProtectLogLevel) nginxManager.AppProtectPluginStart(aPPluginDone) } @@ -783,6 +797,23 @@ func validatePort(port int) error { return nil } +const appProtectLogLevelDefault = "fatal" + +// validateAppProtectLogLevel makes sure a given logLevel is one of the allowed values +func validateAppProtectLogLevel(logLevel string) error { + switch strings.ToLower(logLevel) { + case + "fatal", + "error", + "warn", + "info", + "debug", + "trace": + return nil + } + return fmt.Errorf("invalid App Protect log level: %v", logLevel) +} + // parseNginxStatusAllowCIDRs converts a comma separated CIDR/IP address string into an array of CIDR/IP addresses. // It returns an array of the valid CIDR/IP addresses or an error if given an invalid address. func parseNginxStatusAllowCIDRs(input string) (cidrs []string, err error) { diff --git a/cmd/nginx-ingress/main_test.go b/cmd/nginx-ingress/main_test.go index 2f780db2e2..0b98cbed23 100644 --- a/cmd/nginx-ingress/main_test.go +++ b/cmd/nginx-ingress/main_test.go @@ -123,3 +123,33 @@ func TestValidateLocation(t *testing.T) { } } } + +func TestValidateAppProtectLogLevel(t *testing.T) { + badLogLevels := []string{ + "", + "critical", + "none", + "info;", + } + for _, badLogLevel := range badLogLevels { + err := validateAppProtectLogLevel(badLogLevel) + if err == nil { + t.Errorf("validateAppProtectLogLevel(%v) returned no error when it should have returned an error", badLogLevel) + } + } + + goodLogLevels := []string{ + "fatal", + "Error", + "WARN", + "info", + "debug", + "trace", + } + for _, goodLogLevel := range goodLogLevels { + err := validateAppProtectLogLevel(goodLogLevel) + if err != nil { + t.Errorf("validateAppProtectLogLevel(%v) returned an error when it should have returned no error: %v", goodLogLevel, err) + } + } +} diff --git a/deployments/helm-chart/templates/controller-daemonset.yaml b/deployments/helm-chart/templates/controller-daemonset.yaml index 9797d327bf..243f8f04f5 100644 --- a/deployments/helm-chart/templates/controller-daemonset.yaml +++ b/deployments/helm-chart/templates/controller-daemonset.yaml @@ -105,6 +105,9 @@ spec: - -nginx-plus={{ .Values.controller.nginxplus }} - -nginx-reload-timeout={{ .Values.controller.nginxReloadTimeout }} - -enable-app-protect={{ .Values.controller.appprotect.enable }} +{{- if and .Values.controller.appprotect.enable .Values.controller.appprotect.logLevel }} + - -app-protect-log-level={{ .Values.controller.appprotect.logLevel }} +{{ end }} - -enable-app-protect-dos={{ .Values.controller.appprotectdos.enable }} {{- if .Values.controller.appprotectdos.enable }} - -app-protect-dos-debug={{ .Values.controller.appprotectdos.debug }} diff --git a/deployments/helm-chart/templates/controller-deployment.yaml b/deployments/helm-chart/templates/controller-deployment.yaml index 0626d3dd12..5974a6416f 100644 --- a/deployments/helm-chart/templates/controller-deployment.yaml +++ b/deployments/helm-chart/templates/controller-deployment.yaml @@ -103,6 +103,9 @@ spec: - -nginx-plus={{ .Values.controller.nginxplus }} - -nginx-reload-timeout={{ .Values.controller.nginxReloadTimeout }} - -enable-app-protect={{ .Values.controller.appprotect.enable }} +{{- if and .Values.controller.appprotect.enable .Values.controller.appprotect.logLevel }} + - -app-protect-log-level={{ .Values.controller.appprotect.logLevel }} +{{ end }} - -enable-app-protect-dos={{ .Values.controller.appprotectdos.enable }} {{- if .Values.controller.appprotectdos.enable }} - -app-protect-dos-debug={{ .Values.controller.appprotectdos.debug }} diff --git a/deployments/helm-chart/values.yaml b/deployments/helm-chart/values.yaml index 26100aa40e..6075cf4b34 100644 --- a/deployments/helm-chart/values.yaml +++ b/deployments/helm-chart/values.yaml @@ -16,6 +16,8 @@ controller: appprotect: ## Enable the App Protect module in the Ingress Controller. enable: false + ## Sets log level for App Protect. Allowed values: fatal, error, warn, info, debug, trace + # logLevel: fatal ## Support for App Protect Dos appprotectdos: diff --git a/docs/content/configuration/global-configuration/command-line-arguments.md b/docs/content/configuration/global-configuration/command-line-arguments.md index ae5000002c..5dba4fd960 100644 --- a/docs/content/configuration/global-configuration/command-line-arguments.md +++ b/docs/content/configuration/global-configuration/command-line-arguments.md @@ -337,6 +337,17 @@ Requires [-nginx-plus](#cmdoption-nginx-plus). * If the argument is set, but `nginx-plus` is set to false, the Ingress Controller will fail to start. +  + + +### -app-protect-log-level `` + +Sets log level for App Protect. Allowed values: fatal, error, warn, info, debug, trace. + +Requires [-nginx-plus](#cmdoption-nginx-plus) and [-enable-app-protect](#cmdoption-enable-app-protect). + +* If the argument is set, but `nginx-plus` and `enable-app-protect` are set to false, the Ingress Controller will fail to start. +   diff --git a/internal/nginx/fake_manager.go b/internal/nginx/fake_manager.go index d2448600f2..0f0a644c6e 100644 --- a/internal/nginx/fake_manager.go +++ b/internal/nginx/fake_manager.go @@ -151,7 +151,7 @@ func (*FakeManager) SetOpenTracing(_ bool) { } // AppProtectAgentStart is a fake implementation of AppProtectAgentStart -func (*FakeManager) AppProtectAgentStart(_ chan error, _ bool) { +func (*FakeManager) AppProtectAgentStart(_ chan error, _ string) { glog.V(3).Infof("Starting FakeAppProtectAgent") } diff --git a/internal/nginx/manager.go b/internal/nginx/manager.go index 38d3cfb3c8..77d0be1338 100644 --- a/internal/nginx/manager.go +++ b/internal/nginx/manager.go @@ -32,16 +32,11 @@ const ( appProtectPluginStartCmd = "/usr/share/ts/bin/bd-socket-plugin" appProtectAgentStartCmd = "/opt/app_protect/bin/bd_agent" + appProtectLogLevelCmd = "/opt/app_protect/bin/set_log_level" // appPluginParams is the configuration of App-Protect plugin appPluginParams = "tmm_count 4 proc_cpuinfo_cpu_mhz 2000000 total_xml_memory 307200000 total_umu_max_size 3129344 sys_max_account_id 1024 no_static_config" - // appProtectDebugLogConfigFileContent holds the content of the file to be written when nginx debug is enabled. It will enable NGINX App Protect debug logs - appProtectDebugLogConfigFileContent = "MODULE = IO_PLUGIN;\nLOG_LEVEL = TS_INFO | TS_DEBUG;\nFILE = 2;\nMODULE = ECARD_POLICY;\nLOG_LEVEL = TS_INFO | TS_DEBUG;\nFILE = 2;\n" - - // appProtectLogConfigFileName is the location of the NGINX App Protect logging configuration file - appProtectLogConfigFileName = "/etc/app_protect/bd/logger.cfg" - appProtectDosAgentInstallCmd = "/usr/bin/adminstall" appProtectDosAgentStartCmd = "/usr/bin/admd -d --standalone" appProtectDosAgentStartDebugCmd = "/usr/bin/admd -d --standalone --log debug" @@ -81,7 +76,7 @@ type Manager interface { UpdateServersInPlus(upstream string, servers []string, config ServerConfig) error UpdateStreamServersInPlus(upstream string, servers []string) error SetOpenTracing(openTracing bool) - AppProtectAgentStart(apaDone chan error, debug bool) + AppProtectAgentStart(apaDone chan error, logLevel string) AppProtectAgentQuit() AppProtectPluginStart(appDone chan error) AppProtectPluginQuit() @@ -461,20 +456,15 @@ func (lm *LocalManager) SetOpenTracing(openTracing bool) { } // AppProtectAgentStart starts the AppProtect agent -func (lm *LocalManager) AppProtectAgentStart(apaDone chan error, debug bool) { - if debug { - glog.V(3).Info("Starting AppProtect Agent in debug mode") - err := os.Remove(appProtectLogConfigFileName) - if err != nil { - glog.Fatalf("Failed removing App Protect Log configuration file") - } - err = createFileAndWrite(appProtectLogConfigFileName, []byte(appProtectDebugLogConfigFileContent)) - if err != nil { - glog.Fatalf("Failed Writing App Protect Log configuration file") - } +func (lm *LocalManager) AppProtectAgentStart(apaDone chan error, logLevel string) { + glog.V(3).Info("Setting log level for App Protect - ", logLevel) + appProtectLogLevelCmdfull := fmt.Sprintf("%v %v", appProtectLogLevelCmd, logLevel) + logLevelCmd := exec.Command("sh", "-c", appProtectLogLevelCmdfull) + if err := logLevelCmd.Run(); err != nil { + glog.Fatalf("Failed to set log level for AppProtect: %v", err) } - glog.V(3).Info("Starting AppProtect Agent") + glog.V(3).Info("Starting AppProtect Agent") cmd := exec.Command(appProtectAgentStartCmd) if err := cmd.Start(); err != nil { glog.Fatalf("Failed to start AppProtect Agent: %v", err) From c04e00258f420fedaabb2022b9ccc23273adaeb2 Mon Sep 17 00:00:00 2001 From: galitskiy Date: Mon, 21 Mar 2022 19:00:02 +0000 Subject: [PATCH 2/2] disable lint g204 for appProtectLogLevelCmdfull since input argument already velidated --- internal/nginx/manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/nginx/manager.go b/internal/nginx/manager.go index 77d0be1338..66c4991103 100644 --- a/internal/nginx/manager.go +++ b/internal/nginx/manager.go @@ -459,7 +459,7 @@ func (lm *LocalManager) SetOpenTracing(openTracing bool) { func (lm *LocalManager) AppProtectAgentStart(apaDone chan error, logLevel string) { glog.V(3).Info("Setting log level for App Protect - ", logLevel) appProtectLogLevelCmdfull := fmt.Sprintf("%v %v", appProtectLogLevelCmd, logLevel) - logLevelCmd := exec.Command("sh", "-c", appProtectLogLevelCmdfull) + logLevelCmd := exec.Command("sh", "-c", appProtectLogLevelCmdfull) // #nosec G204 if err := logLevelCmd.Run(); err != nil { glog.Fatalf("Failed to set log level for AppProtect: %v", err) }