From 1d61f69704c01a0a3202b6b22baae32bd6232caa Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Thu, 16 Nov 2023 13:14:00 +0000 Subject: [PATCH 1/5] Restrict NAP file/dir permissions --- src/extensions/nginx-app-protect/nap/nap.go | 7 +++---- src/extensions/nginx-app-protect/nap/nap_metadata.go | 2 +- src/extensions/nginx-app-protect/nap/nap_metadata_test.go | 1 + src/extensions/nginx-app-protect/nap/nap_test.go | 4 ++-- .../agent/v2/src/extensions/nginx-app-protect/nap/nap.go | 7 +++---- .../src/extensions/nginx-app-protect/nap/nap_metadata.go | 2 +- 6 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/extensions/nginx-app-protect/nap/nap.go b/src/extensions/nginx-app-protect/nap/nap.go index 7726cf31c..c51d14d88 100644 --- a/src/extensions/nginx-app-protect/nap/nap.go +++ b/src/extensions/nginx-app-protect/nap/nap.go @@ -15,17 +15,16 @@ import ( "strings" "time" - log "github.com/sirupsen/logrus" - "github.com/nginx/agent/v2/src/core" + + log "github.com/sirupsen/logrus" ) const ( DefaultOptNAPDir = "/opt/app_protect" DefaultNMSCompilerDir = "/opt/nms-nap-compiler" compilerDirPrefix = "app_protect-" - - dirPerm = 0o755 + dirPerm = 0o644 ) var ( diff --git a/src/extensions/nginx-app-protect/nap/nap_metadata.go b/src/extensions/nginx-app-protect/nap/nap_metadata.go index a3ed2a09f..92cbec038 100644 --- a/src/extensions/nginx-app-protect/nap/nap_metadata.go +++ b/src/extensions/nginx-app-protect/nap/nap_metadata.go @@ -91,7 +91,7 @@ func UpdateMetadata( directory := filepath.Dir(appProtectWAFDetails.GetWafLocation()) _, err = os.Stat(directory) if os.IsNotExist(err) { - err = os.MkdirAll(directory, 0o755) + err = os.MkdirAll(directory, 0o644) if err != nil { return err } diff --git a/src/extensions/nginx-app-protect/nap/nap_metadata_test.go b/src/extensions/nginx-app-protect/nap/nap_metadata_test.go index cd5b03ade..6ab13e492 100644 --- a/src/extensions/nginx-app-protect/nap/nap_metadata_test.go +++ b/src/extensions/nginx-app-protect/nap/nap_metadata_test.go @@ -14,6 +14,7 @@ import ( "github.com/nginx/agent/sdk/v2" "github.com/nginx/agent/sdk/v2/proto" + "github.com/stretchr/testify/assert" ) diff --git a/src/extensions/nginx-app-protect/nap/nap_test.go b/src/extensions/nginx-app-protect/nap/nap_test.go index 4aa807c7a..ff0212134 100644 --- a/src/extensions/nginx-app-protect/nap/nap_test.go +++ b/src/extensions/nginx-app-protect/nap/nap_test.go @@ -12,10 +12,10 @@ import ( "os" "testing" + testutils "github.com/nginx/agent/v2/test/utils" + "github.com/google/uuid" "github.com/stretchr/testify/assert" - - testutils "github.com/nginx/agent/v2/test/utils" ) const ( diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap.go b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap.go index 7726cf31c..c51d14d88 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap.go @@ -15,17 +15,16 @@ import ( "strings" "time" - log "github.com/sirupsen/logrus" - "github.com/nginx/agent/v2/src/core" + + log "github.com/sirupsen/logrus" ) const ( DefaultOptNAPDir = "/opt/app_protect" DefaultNMSCompilerDir = "/opt/nms-nap-compiler" compilerDirPrefix = "app_protect-" - - dirPerm = 0o755 + dirPerm = 0o644 ) var ( diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go index a3ed2a09f..92cbec038 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go @@ -91,7 +91,7 @@ func UpdateMetadata( directory := filepath.Dir(appProtectWAFDetails.GetWafLocation()) _, err = os.Stat(directory) if os.IsNotExist(err) { - err = os.MkdirAll(directory, 0o755) + err = os.MkdirAll(directory, 0o644) if err != nil { return err } From b954a514aeb63941d28d3ae19924cf97237a9ccb Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Thu, 16 Nov 2023 14:38:20 +0000 Subject: [PATCH 2/5] Lint --- src/extensions/php_fpm_metrics.go | 2 +- src/extensions/php_fpm_metrics_test.go | 1 + src/extensions/prometheus-metrics/prometheus_exporter.go | 2 +- src/extensions/prometheus-metrics/prometheus_exporter_test.go | 2 +- .../github.com/nginx/agent/v2/src/extensions/php_fpm_metrics.go | 2 +- .../v2/src/extensions/prometheus-metrics/prometheus_exporter.go | 2 +- 6 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/extensions/php_fpm_metrics.go b/src/extensions/php_fpm_metrics.go index 24b76f846..5dbec172a 100644 --- a/src/extensions/php_fpm_metrics.go +++ b/src/extensions/php_fpm_metrics.go @@ -13,11 +13,11 @@ import ( agent_config "github.com/nginx/agent/sdk/v2/agent/config" "github.com/nginx/agent/v2/src/core" "github.com/nginx/agent/v2/src/core/config" + log "github.com/sirupsen/logrus" ) const ( - // Version phpFpmMetricsExtensionPluginVersion = "v0.1.0" ) diff --git a/src/extensions/php_fpm_metrics_test.go b/src/extensions/php_fpm_metrics_test.go index d83c8654d..7b44a7764 100644 --- a/src/extensions/php_fpm_metrics_test.go +++ b/src/extensions/php_fpm_metrics_test.go @@ -13,6 +13,7 @@ import ( "github.com/nginx/agent/v2/src/core/config" "github.com/nginx/agent/v2/src/extensions" tutils "github.com/nginx/agent/v2/test/utils" + "github.com/stretchr/testify/assert" ) diff --git a/src/extensions/prometheus-metrics/prometheus_exporter.go b/src/extensions/prometheus-metrics/prometheus_exporter.go index 313611bbc..a7688a48d 100644 --- a/src/extensions/prometheus-metrics/prometheus_exporter.go +++ b/src/extensions/prometheus-metrics/prometheus_exporter.go @@ -4,8 +4,8 @@ import ( "strings" "github.com/nginx/agent/sdk/v2/proto" - "github.com/nginx/agent/v2/src/core/metrics" + "github.com/prometheus/client_golang/prometheus" ) diff --git a/src/extensions/prometheus-metrics/prometheus_exporter_test.go b/src/extensions/prometheus-metrics/prometheus_exporter_test.go index 889b4fb92..ef896d268 100644 --- a/src/extensions/prometheus-metrics/prometheus_exporter_test.go +++ b/src/extensions/prometheus-metrics/prometheus_exporter_test.go @@ -3,9 +3,9 @@ package prometheus_metrics import ( "testing" + "github.com/nginx/agent/sdk/v2/proto" "github.com/nginx/agent/v2/src/core/metrics" - "github.com/nginx/agent/sdk/v2/proto" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" ) diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/php_fpm_metrics.go b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/php_fpm_metrics.go index 24b76f846..5dbec172a 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/php_fpm_metrics.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/php_fpm_metrics.go @@ -13,11 +13,11 @@ import ( agent_config "github.com/nginx/agent/sdk/v2/agent/config" "github.com/nginx/agent/v2/src/core" "github.com/nginx/agent/v2/src/core/config" + log "github.com/sirupsen/logrus" ) const ( - // Version phpFpmMetricsExtensionPluginVersion = "v0.1.0" ) diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/prometheus-metrics/prometheus_exporter.go b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/prometheus-metrics/prometheus_exporter.go index 313611bbc..a7688a48d 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/prometheus-metrics/prometheus_exporter.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/prometheus-metrics/prometheus_exporter.go @@ -4,8 +4,8 @@ import ( "strings" "github.com/nginx/agent/sdk/v2/proto" - "github.com/nginx/agent/v2/src/core/metrics" + "github.com/prometheus/client_golang/prometheus" ) From 78c9dddb25dabd707ff0fec9ccd2c4c05438cf1e Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Thu, 16 Nov 2023 15:28:03 +0000 Subject: [PATCH 3/5] Improve error messages. Debug. Tweak permissions --- .../nginx-app-protect/nap/nap_metadata.go | 23 +++++++++++++------ .../nap/nap_metadata_test.go | 5 ++-- .../nginx-app-protect/nap/nap_metadata.go | 23 +++++++++++++------ 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/extensions/nginx-app-protect/nap/nap_metadata.go b/src/extensions/nginx-app-protect/nap/nap_metadata.go index 92cbec038..c9e8f92a0 100644 --- a/src/extensions/nginx-app-protect/nap/nap_metadata.go +++ b/src/extensions/nginx-app-protect/nap/nap_metadata.go @@ -11,6 +11,7 @@ import ( "encoding/json" "errors" "os" + "fmt" "path/filepath" "github.com/nginx/agent/sdk/v2" @@ -33,11 +34,11 @@ func UpdateMetadata( data, err := os.ReadFile(appProtectWAFDetails.GetWafLocation()) if err != nil { if !errors.Is(err, os.ErrNotExist) { - return err + return fmt.Errorf("failed to update metadata: %v", err) } } else { if err := json.Unmarshal(data, &previousMeta); err != nil { - return err + return fmt.Errorf("failed to unmarshal current metadata: %v", err) } previousPrecompiledPublication = previousMeta.PrecompiledPublication } @@ -53,7 +54,6 @@ func UpdateMetadata( policies, profiles := sdk.GetAppProtectPolicyAndSecurityLogFilesWithIgnoreDirectives(cfg, ignoreDirectives) policyBundles := []*BundleMetadata{} - profileBundles := []*BundleMetadata{} for _, policy := range policies { bundle := &BundleMetadata{ @@ -61,6 +61,9 @@ func UpdateMetadata( } policyBundles = append(policyBundles, bundle) } + + profileBundles := []*BundleMetadata{} + for _, profile := range profiles { bundle := &BundleMetadata{ Name: profile, @@ -84,21 +87,27 @@ func UpdateMetadata( m, err := json.Marshal(metadata) if err != nil { - return err + return fmt.Errorf("failed to marshal metadata update: %v", err) } // Make dir if not exists directory := filepath.Dir(appProtectWAFDetails.GetWafLocation()) _, err = os.Stat(directory) if os.IsNotExist(err) { - err = os.MkdirAll(directory, 0o644) + err = os.MkdirAll(directory, 0o755) if err != nil { - return err + return fmt.Errorf("failed to create directory for metadata update: %v", err) } } log.Debugf("Writing NAP Metadata %s", m) - return os.WriteFile(appProtectWAFDetails.GetWafLocation(), m, 0o644) + + err = os.WriteFile(appProtectWAFDetails.GetWafLocation(), m, 0o664) + if err != nil { + return fmt.Errorf("failed to write NAP Metadata update: %v", err ) + } + + return nil } // metadataAreEqual compares the metadata for equality diff --git a/src/extensions/nginx-app-protect/nap/nap_metadata_test.go b/src/extensions/nginx-app-protect/nap/nap_metadata_test.go index 6ab13e492..2e0333ce9 100644 --- a/src/extensions/nginx-app-protect/nap/nap_metadata_test.go +++ b/src/extensions/nginx-app-protect/nap/nap_metadata_test.go @@ -171,9 +171,10 @@ func TestUpdateNapMetadata(t *testing.T) { WafLocation: metadataFile, PrecompiledPublication: tc.precompPub, } - ignoreDirecitves := []string{} + + ignoreDirectives = []string{} - err = UpdateMetadata(cfg, appProtectWAFDetails, ignoreDirecitves) + err = UpdateMetadata(cfg, appProtectWAFDetails, ignoreDirectives) assert.NoError(t, err) data, err := os.ReadFile(metadataFile) diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go index 92cbec038..c9e8f92a0 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go @@ -11,6 +11,7 @@ import ( "encoding/json" "errors" "os" + "fmt" "path/filepath" "github.com/nginx/agent/sdk/v2" @@ -33,11 +34,11 @@ func UpdateMetadata( data, err := os.ReadFile(appProtectWAFDetails.GetWafLocation()) if err != nil { if !errors.Is(err, os.ErrNotExist) { - return err + return fmt.Errorf("failed to update metadata: %v", err) } } else { if err := json.Unmarshal(data, &previousMeta); err != nil { - return err + return fmt.Errorf("failed to unmarshal current metadata: %v", err) } previousPrecompiledPublication = previousMeta.PrecompiledPublication } @@ -53,7 +54,6 @@ func UpdateMetadata( policies, profiles := sdk.GetAppProtectPolicyAndSecurityLogFilesWithIgnoreDirectives(cfg, ignoreDirectives) policyBundles := []*BundleMetadata{} - profileBundles := []*BundleMetadata{} for _, policy := range policies { bundle := &BundleMetadata{ @@ -61,6 +61,9 @@ func UpdateMetadata( } policyBundles = append(policyBundles, bundle) } + + profileBundles := []*BundleMetadata{} + for _, profile := range profiles { bundle := &BundleMetadata{ Name: profile, @@ -84,21 +87,27 @@ func UpdateMetadata( m, err := json.Marshal(metadata) if err != nil { - return err + return fmt.Errorf("failed to marshal metadata update: %v", err) } // Make dir if not exists directory := filepath.Dir(appProtectWAFDetails.GetWafLocation()) _, err = os.Stat(directory) if os.IsNotExist(err) { - err = os.MkdirAll(directory, 0o644) + err = os.MkdirAll(directory, 0o755) if err != nil { - return err + return fmt.Errorf("failed to create directory for metadata update: %v", err) } } log.Debugf("Writing NAP Metadata %s", m) - return os.WriteFile(appProtectWAFDetails.GetWafLocation(), m, 0o644) + + err = os.WriteFile(appProtectWAFDetails.GetWafLocation(), m, 0o664) + if err != nil { + return fmt.Errorf("failed to write NAP Metadata update: %v", err ) + } + + return nil } // metadataAreEqual compares the metadata for equality From 08b55077270b6045aac687bb6130e1058ea546e8 Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Thu, 16 Nov 2023 15:31:48 +0000 Subject: [PATCH 4/5] go mod vendor --- src/extensions/nginx-app-protect/nap/nap_metadata.go | 6 +++--- src/extensions/nginx-app-protect/nap/nap_metadata_test.go | 2 +- .../v2/src/extensions/nginx-app-protect/nap/nap_metadata.go | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/extensions/nginx-app-protect/nap/nap_metadata.go b/src/extensions/nginx-app-protect/nap/nap_metadata.go index c9e8f92a0..c1068fd4b 100644 --- a/src/extensions/nginx-app-protect/nap/nap_metadata.go +++ b/src/extensions/nginx-app-protect/nap/nap_metadata.go @@ -10,8 +10,8 @@ package nap import ( "encoding/json" "errors" - "os" "fmt" + "os" "path/filepath" "github.com/nginx/agent/sdk/v2" @@ -101,10 +101,10 @@ func UpdateMetadata( } log.Debugf("Writing NAP Metadata %s", m) - + err = os.WriteFile(appProtectWAFDetails.GetWafLocation(), m, 0o664) if err != nil { - return fmt.Errorf("failed to write NAP Metadata update: %v", err ) + return fmt.Errorf("failed to write NAP Metadata update: %v", err) } return nil diff --git a/src/extensions/nginx-app-protect/nap/nap_metadata_test.go b/src/extensions/nginx-app-protect/nap/nap_metadata_test.go index 2e0333ce9..7be283f2d 100644 --- a/src/extensions/nginx-app-protect/nap/nap_metadata_test.go +++ b/src/extensions/nginx-app-protect/nap/nap_metadata_test.go @@ -171,7 +171,7 @@ func TestUpdateNapMetadata(t *testing.T) { WafLocation: metadataFile, PrecompiledPublication: tc.precompPub, } - + ignoreDirectives = []string{} err = UpdateMetadata(cfg, appProtectWAFDetails, ignoreDirectives) diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go index c9e8f92a0..c1068fd4b 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go @@ -10,8 +10,8 @@ package nap import ( "encoding/json" "errors" - "os" "fmt" + "os" "path/filepath" "github.com/nginx/agent/sdk/v2" @@ -101,10 +101,10 @@ func UpdateMetadata( } log.Debugf("Writing NAP Metadata %s", m) - + err = os.WriteFile(appProtectWAFDetails.GetWafLocation(), m, 0o664) if err != nil { - return fmt.Errorf("failed to write NAP Metadata update: %v", err ) + return fmt.Errorf("failed to write NAP Metadata update: %v", err) } return nil From 416cd226026cd471003684775b12602b910078b3 Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Thu, 16 Nov 2023 15:41:18 +0000 Subject: [PATCH 5/5] Min change --- src/extensions/nginx-app-protect/nap/nap.go | 2 +- src/extensions/nginx-app-protect/nap/nap_metadata.go | 4 ++-- .../agent/v2/src/extensions/nginx-app-protect/nap/nap.go | 2 +- .../v2/src/extensions/nginx-app-protect/nap/nap_metadata.go | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/extensions/nginx-app-protect/nap/nap.go b/src/extensions/nginx-app-protect/nap/nap.go index c51d14d88..9d87c8fa9 100644 --- a/src/extensions/nginx-app-protect/nap/nap.go +++ b/src/extensions/nginx-app-protect/nap/nap.go @@ -24,7 +24,7 @@ const ( DefaultOptNAPDir = "/opt/app_protect" DefaultNMSCompilerDir = "/opt/nms-nap-compiler" compilerDirPrefix = "app_protect-" - dirPerm = 0o644 + dirPerm = 0o750 ) var ( diff --git a/src/extensions/nginx-app-protect/nap/nap_metadata.go b/src/extensions/nginx-app-protect/nap/nap_metadata.go index c1068fd4b..582d93bf9 100644 --- a/src/extensions/nginx-app-protect/nap/nap_metadata.go +++ b/src/extensions/nginx-app-protect/nap/nap_metadata.go @@ -94,7 +94,7 @@ func UpdateMetadata( directory := filepath.Dir(appProtectWAFDetails.GetWafLocation()) _, err = os.Stat(directory) if os.IsNotExist(err) { - err = os.MkdirAll(directory, 0o755) + err = os.MkdirAll(directory, 0o750) if err != nil { return fmt.Errorf("failed to create directory for metadata update: %v", err) } @@ -102,7 +102,7 @@ func UpdateMetadata( log.Debugf("Writing NAP Metadata %s", m) - err = os.WriteFile(appProtectWAFDetails.GetWafLocation(), m, 0o664) + err = os.WriteFile(appProtectWAFDetails.GetWafLocation(), m, 0o644) if err != nil { return fmt.Errorf("failed to write NAP Metadata update: %v", err) } diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap.go b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap.go index c51d14d88..9d87c8fa9 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap.go @@ -24,7 +24,7 @@ const ( DefaultOptNAPDir = "/opt/app_protect" DefaultNMSCompilerDir = "/opt/nms-nap-compiler" compilerDirPrefix = "app_protect-" - dirPerm = 0o644 + dirPerm = 0o750 ) var ( diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go index c1068fd4b..582d93bf9 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/extensions/nginx-app-protect/nap/nap_metadata.go @@ -94,7 +94,7 @@ func UpdateMetadata( directory := filepath.Dir(appProtectWAFDetails.GetWafLocation()) _, err = os.Stat(directory) if os.IsNotExist(err) { - err = os.MkdirAll(directory, 0o755) + err = os.MkdirAll(directory, 0o750) if err != nil { return fmt.Errorf("failed to create directory for metadata update: %v", err) } @@ -102,7 +102,7 @@ func UpdateMetadata( log.Debugf("Writing NAP Metadata %s", m) - err = os.WriteFile(appProtectWAFDetails.GetWafLocation(), m, 0o664) + err = os.WriteFile(appProtectWAFDetails.GetWafLocation(), m, 0o644) if err != nil { return fmt.Errorf("failed to write NAP Metadata update: %v", err) }