Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore directives feature #343

Merged
merged 3 commits into from
Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions sdk/config_apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type ConfigApply struct {
func NewConfigApply(
confFile string,
allowedDirectories map[string]struct{},
ignoreDirectives []string,
) (*ConfigApply, error) {
w, err := zip.NewWriter("/")
if err != nil {
Expand All @@ -47,7 +48,7 @@ func NewConfigApply(
notExistDirs: make(map[string]struct{}),
}
if confFile != "" {
return b, b.mapCurrentFiles(confFile, allowedDirectories)
return b, b.mapCurrentFiles(confFile, allowedDirectories, ignoreDirectives)
}
return b, nil
}
Expand Down Expand Up @@ -179,10 +180,11 @@ func (b *ConfigApply) RemoveFromNotExists(fullPath string) {

// mapCurrentFiles parse the provided file via cross-plane, generate a list of files, which should be identical to the
// DirectoryMap, will mark off the files as the config is being applied, any leftovers after complete should be deleted.
func (b *ConfigApply) mapCurrentFiles(confFile string, allowedDirectories map[string]struct{}) error {
func (b *ConfigApply) mapCurrentFiles(confFile string, allowedDirectories map[string]struct{}, ignoreDirectives []string) error {
log.Debugf("parsing %s", confFile)
payload, err := crossplane.Parse(confFile,
&crossplane.ParseOptions{
IgnoreDirectives: ignoreDirectives,
SingleFile: false,
StopParsingOnError: true,
},
Expand Down
10 changes: 8 additions & 2 deletions sdk/config_apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ func TestNewConfigApply(t *testing.T) {
name string
confFile string
allowedDirectories map[string]struct{}
ignoreDirectives []string
expectedConfigApply *ConfigApply
expectError bool
}{
Expand All @@ -107,6 +108,7 @@ func TestNewConfigApply(t *testing.T) {
allowedDirectories: map[string]struct{}{
tmpDir: {},
},
ignoreDirectives: []string{},
expectedConfigApply: &ConfigApply{
existing: map[string]struct{}{
defaultConfFile: {},
Expand All @@ -124,6 +126,7 @@ func TestNewConfigApply(t *testing.T) {
name: "no config file present",
confFile: "",
allowedDirectories: map[string]struct{}{},
ignoreDirectives: []string{},
expectedConfigApply: &ConfigApply{
existing: map[string]struct{}{},
notExists: map[string]struct{}{},
Expand All @@ -135,6 +138,7 @@ func TestNewConfigApply(t *testing.T) {
name: "empty config file present",
confFile: emptyConfFile,
allowedDirectories: map[string]struct{}{},
ignoreDirectives: []string{},
expectedConfigApply: &ConfigApply{
existing: map[string]struct{}{},
notExists: map[string]struct{}{},
Expand All @@ -146,6 +150,7 @@ func TestNewConfigApply(t *testing.T) {
name: "unknown config file present",
confFile: "/tmp/unknown.conf",
allowedDirectories: map[string]struct{}{},
ignoreDirectives: []string{},
expectedConfigApply: &ConfigApply{
existing: map[string]struct{}{},
notExists: map[string]struct{}{},
Expand All @@ -157,7 +162,7 @@ func TestNewConfigApply(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
configApply, err := NewConfigApply(tc.confFile, tc.allowedDirectories)
configApply, err := NewConfigApply(tc.confFile, tc.allowedDirectories, tc.ignoreDirectives)
assert.Equal(t, tc.expectedConfigApply.existing, configApply.GetExisting())
assert.Equal(t, tc.expectedConfigApply.notExists, configApply.GetNotExists())
assert.Equal(t, tc.expectedConfigApply.notExistDirs, configApply.GetNotExistDirs())
Expand Down Expand Up @@ -266,8 +271,9 @@ func TestConfigApplyCompleteAndRollback(t *testing.T) {
require.NoError(t, os.WriteFile(confFile, []byte(confFileContent), 0644))

allowedDirectories := map[string]struct{}{tmpDir: {}}
ignoreDirectives := []string{}

configApply, err := NewConfigApply(confFile, allowedDirectories)
configApply, err := NewConfigApply(confFile, allowedDirectories, ignoreDirectives)
assert.Equal(t, 5, len(configApply.GetExisting()))
assert.Nil(t, err)

Expand Down
11 changes: 8 additions & 3 deletions sdk/config_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,11 @@ func GetNginxConfig(
nginxId,
systemId string,
allowedDirectories map[string]struct{},
ignoreDirectives []string,
) (*proto.NginxConfig, error) {
payload, err := crossplane.Parse(confFile,
&crossplane.ParseOptions{
IgnoreDirectives: ignoreDirectives,
SingleFile: false,
StopParsingOnError: true,
},
Expand Down Expand Up @@ -728,9 +730,10 @@ func pingStatusAPIEndpoint(statusAPI string) bool {
return true
}

func GetStatusApiInfo(confFile string) (statusApi string, err error) {
func GetStatusApiInfo(confFile string, ignoreDirectives []string) (statusApi string, err error) {
payload, err := crossplane.Parse(confFile,
&crossplane.ParseOptions{
IgnoreDirectives: ignoreDirectives,
SingleFile: false,
StopParsingOnError: true,
CombineConfigs: true,
Expand All @@ -749,7 +752,7 @@ func GetStatusApiInfo(confFile string) (statusApi string, err error) {
return "", errors.New("no status api reachable from the agent found")
}

func GetErrorAndAccessLogs(confFile string) (*proto.ErrorLogs, *proto.AccessLogs, error) {
func GetErrorAndAccessLogs(confFile string, ignoreDirectives []string) (*proto.ErrorLogs, *proto.AccessLogs, error) {
nginxConfig := &proto.NginxConfig{
Action: proto.NginxConfigAction_RETURN,
ConfigData: nil,
Expand All @@ -763,6 +766,7 @@ func GetErrorAndAccessLogs(confFile string) (*proto.ErrorLogs, *proto.AccessLogs

payload, err := crossplane.Parse(confFile,
&crossplane.ParseOptions{
IgnoreDirectives: ignoreDirectives,
SingleFile: false,
StopParsingOnError: true,
},
Expand Down Expand Up @@ -828,7 +832,7 @@ func convertToHexFormat(hexString string) string {
return formatted
}

func GetAppProtectPolicyAndSecurityLogFiles(cfg *proto.NginxConfig) ([]string, []string) {
func GetAppProtectPolicyAndSecurityLogFiles(cfg *proto.NginxConfig, ignoreDirectives []string) ([]string, []string) {
policyMap := make(map[string]bool)
profileMap := make(map[string]bool)

Expand All @@ -838,6 +842,7 @@ func GetAppProtectPolicyAndSecurityLogFiles(cfg *proto.NginxConfig) ([]string, [

payload, err := crossplane.Parse(confFile,
&crossplane.ParseOptions{
IgnoreDirectives: ignoreDirectives,
SingleFile: false,
StopParsingOnError: true,
},
Expand Down
14 changes: 9 additions & 5 deletions sdk/config_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -633,12 +633,13 @@ func TestGetNginxConfig(t *testing.T) {
assert.NoError(t, err)

allowedDirs := map[string]struct{}{}
ignoreDirectives := []string{}

if test.expected.Zaux != nil {
allowedDirs[test.expected.Zaux.RootDirectory] = struct{}{}
allowedDirs["/tmp/testdata/nginx/"] = struct{}{}
}
result, err := GetNginxConfig(test.fileName, nginxID, systemID, allowedDirs)
result, err := GetNginxConfig(test.fileName, nginxID, systemID, allowedDirs, ignoreDirectives)
assert.NoError(t, err)

assert.Equal(t, test.expected.Action, result.Action)
Expand Down Expand Up @@ -750,7 +751,8 @@ func TestGetStatusApiInfo(t *testing.T) {
output := bytes.Replace(input, []byte("127.0.0.1:80"), []byte(splitUrl), -1)
assert.NoError(t, os.WriteFile(test.fileName, output, 0664))

result, err := GetStatusApiInfo(test.fileName)
ignoreDirectives := []string{}
result, err := GetStatusApiInfo(test.fileName, ignoreDirectives)

//Update port in expected plusApi with the port of the mock server
test.plusApi = strings.Replace(test.plusApi, ":80", ":"+strings.Split(splitUrl, ":")[1], 1)
Expand Down Expand Up @@ -979,8 +981,9 @@ func TestGetErrorAndAccessLogs(t *testing.T) {

err = setUpFile(test.fileName, []byte(test.config))
assert.NoError(t, err)
ignoreDirectives := []string{}

errorLogs, accessLogs, err := GetErrorAndAccessLogs(test.fileName)
errorLogs, accessLogs, err := GetErrorAndAccessLogs(test.fileName, ignoreDirectives)
assert.NoError(t, err)

for index, accessLog := range accessLogs.AccessLog {
Expand Down Expand Up @@ -1535,11 +1538,12 @@ func TestGetAppProtectPolicyAndSecurityLogFiles(t *testing.T) {
assert.NoError(t, err)

allowedDirs := map[string]struct{}{}
ignoreDirectives := []string{}

cfg, err := GetNginxConfig(tc.file, nginxID, systemID, allowedDirs)
cfg, err := GetNginxConfig(tc.file, nginxID, systemID, allowedDirs, ignoreDirectives)
assert.NoError(t, err)

policies, profiles := GetAppProtectPolicyAndSecurityLogFiles(cfg)
policies, profiles := GetAppProtectPolicyAndSecurityLogFiles(cfg, ignoreDirectives)
assert.ElementsMatch(t, tc.expPolicies, policies)
assert.ElementsMatch(t, tc.expProfiles, profiles)
})
Expand Down
1 change: 1 addition & 0 deletions src/core/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ func GetConfig(clientId string) (*Config, error) {
AllowedDirectoriesMap: map[string]struct{}{},
DisplayName: Viper.GetString(DisplayNameKey),
InstanceGroup: Viper.GetString(InstanceGroupKey),
IgnoreDirectives: Viper.GetStringSlice(IgnoreDirectivesKey),
}

for _, dir := range strings.Split(config.ConfigDirs, ":") {
Expand Down
19 changes: 13 additions & 6 deletions src/core/config/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ var (
TreatWarningsAsErrors: false,
},
ConfigDirs: "/etc/nginx:/usr/local/etc/nginx:/usr/share/nginx/modules:/etc/nms",
IgnoreDirectives: []string{},
AllowedDirectoriesMap: map[string]struct{}{},
TLS: TLSConfig{
Enable: false,
Expand Down Expand Up @@ -100,12 +101,13 @@ const (
ConfigPathKey = "path"
DynamicConfigPathKey = "dynamic-config-path"

CloudAccountIdKey = "cloudaccountid"
LocationKey = "location"
DisplayNameKey = "display_name"
InstanceGroupKey = "instance_group"
ConfigDirsKey = "config_dirs"
TagsKey = "tags"
CloudAccountIdKey = "cloudaccountid"
LocationKey = "location"
DisplayNameKey = "display_name"
InstanceGroupKey = "instance_group"
ConfigDirsKey = "config_dirs"
TagsKey = "tags"
IgnoreDirectivesKey = "ignore_directives"

// viper keys used in config
LogKey = "log"
Expand Down Expand Up @@ -268,6 +270,11 @@ var (
DefaultValue: agent_config.GetDefaultFeatures(),
},
// NGINX Config
&StringSliceFlag{
Name: IgnoreDirectivesKey,
Usage: "A comma-separated list of ignoring directives which contain sensitive info.",
DefaultValue: Defaults.IgnoreDirectives,
},
&StringFlag{
Name: NginxExcludeLogs,
Usage: "One or more NGINX access log paths that you want to exclude from metrics collection. This key is formatted as a string and multiple values should be provided as a comma-separated list.",
Expand Down
1 change: 1 addition & 0 deletions src/core/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type Config struct {
AllowedDirectoriesMap map[string]struct{} `yaml:"-"`
DisplayName string `mapstructure:"display_name" yaml:"display_name,omitempty"`
InstanceGroup string `mapstructure:"instance_group" yaml:"instance_group,omitempty"`
IgnoreDirectives []string `mapstructure:"ignore_directives" yaml:"ignore_directives,omitempty"`
}

func (c *Config) IsGrpcServerConfigured() bool {
Expand Down
8 changes: 4 additions & 4 deletions src/core/environment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ func TestWriteFiles(t *testing.T) {
for _, file := range files {
assert.NoFileExists(t, file.GetName())
}
backup, err := sdk.NewConfigApply("", nil)
backup, err := sdk.NewConfigApply("", nil, []string{})
assert.NoError(t, err)

env := EnvironmentType{}
Expand Down Expand Up @@ -656,7 +656,7 @@ func TestWriteFilesWhenExists(t *testing.T) {

AllowedDirectoriesMap := map[string]struct{}{"/tmp": {}}

backup, err := sdk.NewConfigApply("", nil)
backup, err := sdk.NewConfigApply("", nil, []string{})
assert.NoError(t, err)
for _, file := range files {
assert.NoFileExists(t, file.GetName())
Expand Down Expand Up @@ -686,7 +686,7 @@ func TestWriteFilesNotAllowed(t *testing.T) {
Permissions: "0644",
},
}
backup, err := sdk.NewConfigApply("", nil)
backup, err := sdk.NewConfigApply("", nil, []string{})
assert.NoError(t, err)

AllowedDirectoriesMap := map[string]struct{}{"/opt": {}}
Expand All @@ -705,7 +705,7 @@ func TestWriteFile(t *testing.T) {
Contents: []byte("contents"),
Permissions: "0777",
}
backup, err := sdk.NewConfigApply("", nil)
backup, err := sdk.NewConfigApply("", nil, []string{})
assert.NoError(t, err)
assert.NoError(t, writeFile(backup, file, "/tmp"))
assert.FileExists(t, file.GetName())
Expand Down
9 changes: 5 additions & 4 deletions src/core/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func (n *NginxBinaryType) GetNginxDetailsFromProcess(nginxProcess Process) *prot
nginxDetailsFacade.ConfPath = path
}

url, err := sdk.GetStatusApiInfo(nginxDetailsFacade.ConfPath)
url, err := sdk.GetStatusApiInfo(nginxDetailsFacade.ConfPath, n.config.IgnoreDirectives)
if err != nil {
log.Tracef("Unable to get status api from the configuration: NGINX metrics will be unavailable for this system. please configure a status API to get NGINX metrics: %v", err)
}
Expand Down Expand Up @@ -352,6 +352,7 @@ func (n *NginxBinaryType) WriteConfig(config *proto.NginxConfig) (*sdk.ConfigApp
config.ConfigData.NginxId,
config.ConfigData.SystemId,
n.config.AllowedDirectoriesMap,
n.config.IgnoreDirectives,
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -387,7 +388,7 @@ func (n *NginxBinaryType) WriteConfig(config *proto.NginxConfig) (*sdk.ConfigApp

log.Info("Updating NGINX config")
var configApply *sdk.ConfigApply
configApply, err = sdk.NewConfigApply(details.ConfPath, n.config.AllowedDirectoriesMap)
configApply, err = sdk.NewConfigApply(details.ConfPath, n.config.AllowedDirectoriesMap, n.config.IgnoreDirectives)
if err != nil {
log.Warnf("config_apply error: %s", err)
return nil, err
Expand Down Expand Up @@ -482,7 +483,7 @@ func generateDeleteFromDirectoryMap(
}

func (n *NginxBinaryType) ReadConfig(confFile, nginxId, systemId string) (*proto.NginxConfig, error) {
configPayload, err := sdk.GetNginxConfig(confFile, nginxId, systemId, n.config.AllowedDirectoriesMap)
configPayload, err := sdk.GetNginxConfig(confFile, nginxId, systemId, n.config.AllowedDirectoriesMap, n.config.IgnoreDirectives)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -534,7 +535,7 @@ func (n *NginxBinaryType) writeBackup(config *proto.NginxConfig, confFiles []*pr
allowedDirs := map[string]struct{}{"/tmp": {}}
path := filepath.Join("/tmp", strconv.FormatInt(time.Now().Unix(), 10))

configApply, err := sdk.NewConfigApply("/tmp", n.config.AllowedDirectoriesMap)
configApply, err := sdk.NewConfigApply("/tmp", n.config.AllowedDirectoriesMap, n.config.IgnoreDirectives)
if err != nil {
log.Warnf("config_apply error: %s", err)
return
Expand Down
3 changes: 2 additions & 1 deletion src/extensions/nginx-app-protect/nap/nap_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
func UpdateMetadata(
cfg *proto.NginxConfig,
appProtectWAFDetails *proto.AppProtectWAFDetails,
ignoreDirectives []string,
) error {
previousPrecompiledPublication := false
previousMeta := Metadata{}
Expand All @@ -49,7 +50,7 @@ func UpdateMetadata(
return nil
}

policies, profiles := sdk.GetAppProtectPolicyAndSecurityLogFiles(cfg)
policies, profiles := sdk.GetAppProtectPolicyAndSecurityLogFiles(cfg, ignoreDirectives)

policyBundles := []*BundleMetadata{}
profileBundles := []*BundleMetadata{}
Expand Down
6 changes: 4 additions & 2 deletions src/extensions/nginx-app-protect/nap/nap_metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,9 @@ func TestUpdateNapMetadata(t *testing.T) {
err := setUpFile(configFile, []byte(config))
assert.NoError(t, err)
allowedDirs := map[string]struct{}{}
ignoreDirectives := []string{}

cfg, err := sdk.GetNginxConfig(configFile, nginxID, systemID, allowedDirs)
cfg, err := sdk.GetNginxConfig(configFile, nginxID, systemID, allowedDirs, ignoreDirectives)
assert.NoError(t, err)

appProtectWAFDetails := &proto.AppProtectWAFDetails{
Expand All @@ -170,8 +171,9 @@ func TestUpdateNapMetadata(t *testing.T) {
WafLocation: metadataFile,
PrecompiledPublication: tc.precompPub,
}
ignoreDirecitves := []string{}

err = UpdateMetadata(cfg, appProtectWAFDetails)
err = UpdateMetadata(cfg, appProtectWAFDetails, ignoreDirecitves)
assert.NoError(t, err)

data, err := os.ReadFile(metadataFile)
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/agent_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ func (h *NginxHandler) applyNginxConfig(nginxDetail *proto.NginxDetails, buf *by
Contents: buf.Bytes(),
}

configApply, err := sdk.NewConfigApply(protoFile.GetName(), h.config.AllowedDirectoriesMap)
configApply, err := sdk.NewConfigApply(protoFile.GetName(), h.config.AllowedDirectoriesMap, h.config.IgnoreDirectives)
if err != nil {
return fmt.Errorf("unable to write config: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ func createCollectorConfigsMap(config *config.Config, env core.Environment, bina
stubStatusApi = detail.StatusUrl
}

errorLogs, accessLogs, err := sdk.GetErrorAndAccessLogs(detail.ConfPath)
errorLogs, accessLogs, err := sdk.GetErrorAndAccessLogs(detail.ConfPath, config.IgnoreDirectives)
if err != nil {
log.Warnf("Error reading access and error logs from config %s %v", detail.ConfPath, err)
}
Expand Down
Loading