Skip to content

Commit 5e3f0a5

Browse files
aphralGaphralG
committed
change allowed dir
1 parent f9e231d commit 5e3f0a5

File tree

11 files changed

+321
-180
lines changed

11 files changed

+321
-180
lines changed

‎internal/config/config.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,8 @@ func registerFlags() {
161161

162162
fs.StringSlice(AllowedDirectoriesKey,
163163
DefaultAllowedDirectories(),
164-
"A comma-separated list of paths that you want to grant NGINX Agent read/write access to")
164+
"A comma-separated list of paths that you want to grant NGINX Agent read/write access to. Allowed "+
165+
"directories are case sensitive")
165166

166167
fs.Duration(
167168
InstanceWatcherMonitoringFrequencyKey,

‎internal/config/config_test.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,8 @@ func getAgentConfig() *Config {
625625
},
626626
},
627627
AllowedDirectories: []string{
628-
"/etc/nginx", "/usr/local/etc/nginx", "/var/run/nginx", "/var/log/nginx", "/usr/share/nginx/modules",
628+
"/etc/nginx", "/etc/nginx-agent", "/usr/local/etc/nginx", "/var/run/nginx", "/var/log/nginx",
629+
"/usr/share/nginx/modules",
629630
},
630631
Collector: &Collector{
631632
ConfigPath: "/etc/nginx-agent/nginx-agent-otelcol.yaml",
@@ -755,7 +756,8 @@ func createConfig() *Config {
755756
},
756757
},
757758
AllowedDirectories: []string{
758-
"/etc/nginx", "/usr/local/etc/nginx", "/var/run/nginx", "/usr/share/nginx/modules", "/var/log/nginx",
759+
"/etc/nginx", "/etc/nginx-agent", "/usr/local/etc/nginx",
760+
"/var/run/nginx", "/usr/share/nginx/modules", "/var/log/nginx",
759761
},
760762
DataPlaneConfig: &DataPlaneConfig{
761763
Nginx: &NginxDataPlaneConfig{

‎internal/config/testdata/nginx-agent.conf‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ client:
4444

4545
allowed_directories:
4646
- /etc/nginx
47+
- /etc/nginx-agent
4748
- /usr/local/etc/nginx
4849
- /var/run/nginx
4950
- /usr/share/nginx/modules

‎internal/config/types.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,9 @@ func (c *Config) AreReceiversConfigured() bool {
382382

383383
func isAllowedDir(dir string, allowedDirs []string) bool {
384384
for _, allowedDirectory := range allowedDirs {
385+
if !strings.HasSuffix(allowedDirectory, "/") {
386+
allowedDirectory += "/"
387+
}
385388
if strings.HasPrefix(dir, allowedDirectory) {
386389
return true
387390
}

‎internal/config/types_test.go‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// Copyright (c) F5, Inc.
2+
//
3+
// This source code is licensed under the Apache License, Version 2.0 license found in the
4+
// LICENSE file in the root directory of this source tree.
5+
6+
package config
7+
8+
import (
9+
"testing"
10+
11+
"github.com/stretchr/testify/assert"
12+
)
13+
14+
func TestTypes_IsDirectoryAllowed(t *testing.T) {
15+
config := getAgentConfig()
16+
17+
tests := []struct {
18+
name string
19+
fileDir string
20+
allowedDirs []string
21+
allowed bool
22+
}{
23+
{
24+
name: "Test 1: directory allowed",
25+
allowed: true,
26+
allowedDirs: []string{
27+
"/etc/nginx",
28+
"/var/log/nginx/",
29+
},
30+
fileDir: "/etc/nginx/nginx.conf",
31+
},
32+
{
33+
name: "Test 2: directory not allowed",
34+
allowed: false,
35+
allowedDirs: []string{
36+
"/etc/nginx/",
37+
"/var/log/nginx/",
38+
},
39+
fileDir: "/etc/nginx-agent/nginx-agent.conf",
40+
},
41+
{
42+
name: "Test 3: directory allowed",
43+
allowed: true,
44+
allowedDirs: []string{
45+
"/etc/nginx/",
46+
"/var/log/nginx/",
47+
},
48+
fileDir: "/etc/nginx/conf.d/nginx-agent.conf",
49+
},
50+
{
51+
name: "Test 4: directory not allowed",
52+
allowed: false,
53+
allowedDirs: []string{
54+
"/etc/nginx",
55+
"/var/log/nginx",
56+
},
57+
fileDir: "~/test.conf",
58+
},
59+
{
60+
name: "Test 5: directory not allowed",
61+
allowed: false,
62+
allowedDirs: []string{
63+
"/etc/nginx/",
64+
"/var/log/nginx/",
65+
},
66+
fileDir: "//test.conf",
67+
},
68+
{
69+
name: "Test 6: directory allowed",
70+
allowed: true,
71+
allowedDirs: []string{
72+
"/etc/nginx/",
73+
"/var/log/nginx/",
74+
"/",
75+
},
76+
fileDir: "/test.conf",
77+
},
78+
}
79+
80+
for _, test := range tests {
81+
t.Run(test.name, func(t *testing.T) {
82+
config.AllowedDirectories = test.allowedDirs
83+
result := config.IsDirectoryAllowed(test.fileDir)
84+
assert.Equal(t, test.allowed, result)
85+
})
86+
}
87+
}

‎internal/watcher/instance/nginx_config_parser.go‎

Lines changed: 71 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"encoding/json"
1111
"fmt"
1212
"io"
13-
"io/fs"
1413
"log/slog"
1514
"net"
1615
"net/http"
@@ -113,102 +112,101 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
113112
rootDir := filepath.Dir(instance.GetInstanceRuntime().GetConfigPath())
114113

115114
for _, conf := range payload.Config {
116-
if ncp.agentConfig.IsDirectoryAllowed(strings.ToLower(conf.File)) {
117-
formatMap := make(map[string]string)
118-
err := ncp.crossplaneConfigTraverse(ctx, &conf,
119-
func(ctx context.Context, parent, directive *crossplane.Directive) error {
120-
switch directive.Directive {
121-
case "log_format":
122-
formatMap = ncp.formatMap(directive)
123-
case "access_log":
124-
if !ncp.ignoreLog(directive.Args[0]) {
125-
accessLog := ncp.accessLog(directive.Args[0], ncp.accessLogDirectiveFormat(directive),
126-
formatMap)
127-
nginxConfigContext.AccessLogs = append(nginxConfigContext.AccessLogs, accessLog)
128-
}
129-
case "error_log":
130-
if !ncp.ignoreLog(directive.Args[0]) {
131-
errorLog := ncp.errorLog(directive.Args[0], ncp.errorLogDirectiveLevel(directive))
132-
nginxConfigContext.ErrorLogs = append(nginxConfigContext.ErrorLogs, errorLog)
133-
} else {
134-
slog.WarnContext(ctx, fmt.Sprintf("Currently error log outputs to %s. Log monitoring "+
135-
"is disabled while applying a config; "+"log errors to file to enable error monitoring",
136-
directive.Args[0]), "error_log", directive.Args[0])
137-
}
138-
case "root":
139-
rootFiles := ncp.rootFiles(ctx, directive.Args[0])
140-
nginxConfigContext.Files = append(nginxConfigContext.Files, rootFiles...)
141-
case "ssl_certificate", "proxy_ssl_certificate", "ssl_client_certificate",
142-
"ssl_trusted_certificate":
143-
sslCertFile := ncp.sslCert(ctx, directive.Args[0], rootDir)
144-
if !ncp.isDuplicateFile(nginxConfigContext.Files, sslCertFile) {
145-
nginxConfigContext.Files = append(nginxConfigContext.Files, sslCertFile)
146-
}
115+
if !ncp.agentConfig.IsDirectoryAllowed(conf.File) {
116+
slog.WarnContext(ctx, "File included in NGINX config is outside of allowed directories, "+
117+
"excluding from config",
118+
"file", conf.File)
119+
120+
continue
121+
}
122+
123+
formatMap := make(map[string]string)
124+
err := ncp.crossplaneConfigTraverse(ctx, &conf,
125+
func(ctx context.Context, parent, directive *crossplane.Directive) error {
126+
switch directive.Directive {
127+
case "log_format":
128+
formatMap = ncp.formatMap(directive)
129+
case "access_log":
130+
if !ncp.ignoreLog(directive.Args[0]) {
131+
accessLog := ncp.accessLog(directive.Args[0], ncp.accessLogDirectiveFormat(directive),
132+
formatMap)
133+
nginxConfigContext.AccessLogs = append(nginxConfigContext.AccessLogs, accessLog)
134+
}
135+
case "error_log":
136+
if !ncp.ignoreLog(directive.Args[0]) {
137+
errorLog := ncp.errorLog(directive.Args[0], ncp.errorLogDirectiveLevel(directive))
138+
nginxConfigContext.ErrorLogs = append(nginxConfigContext.ErrorLogs, errorLog)
139+
} else {
140+
slog.WarnContext(ctx, fmt.Sprintf("Currently error log outputs to %s. Log monitoring "+
141+
"is disabled while applying a config; "+"log errors to file to enable error monitoring",
142+
directive.Args[0]), "error_log", directive.Args[0])
143+
}
144+
case "ssl_certificate", "proxy_ssl_certificate", "ssl_client_certificate",
145+
"ssl_trusted_certificate":
146+
sslCertFile := ncp.sslCert(ctx, directive.Args[0], rootDir)
147+
if !ncp.isDuplicateFile(nginxConfigContext.Files, sslCertFile) {
148+
nginxConfigContext.Files = append(nginxConfigContext.Files, sslCertFile)
149+
}
147150

148-
case "app_protect_security_log":
149-
if len(directive.Args) > 1 {
150-
syslogArg := directive.Args[1]
151-
re := regexp.MustCompile(`syslog:server=([\S]+)`)
152-
matches := re.FindStringSubmatch(syslogArg)
153-
if len(matches) > 1 {
154-
syslogServer := matches[1]
155-
if !napSyslogServersFound[syslogServer] {
156-
nginxConfigContext.NAPSysLogServers = append(
157-
nginxConfigContext.NAPSysLogServers,
158-
syslogServer,
159-
)
160-
napSyslogServersFound[syslogServer] = true
161-
slog.DebugContext(ctx, "Found NAP syslog server", "address", syslogServer)
162-
}
151+
case "app_protect_security_log":
152+
if len(directive.Args) > 1 {
153+
syslogArg := directive.Args[1]
154+
re := regexp.MustCompile(`syslog:server=([\S]+)`)
155+
matches := re.FindStringSubmatch(syslogArg)
156+
if len(matches) > 1 {
157+
syslogServer := matches[1]
158+
if !napSyslogServersFound[syslogServer] {
159+
nginxConfigContext.NAPSysLogServers = append(
160+
nginxConfigContext.NAPSysLogServers,
161+
syslogServer,
162+
)
163+
napSyslogServersFound[syslogServer] = true
164+
slog.DebugContext(ctx, "Found NAP syslog server", "address", syslogServer)
163165
}
164166
}
165167
}
168+
}
166169

167-
return nil
168-
},
169-
)
170-
if err != nil {
171-
return nginxConfigContext, fmt.Errorf("traverse nginx config: %w", err)
172-
}
170+
return nil
171+
},
172+
)
173+
if err != nil {
174+
return nginxConfigContext, fmt.Errorf("traverse nginx config: %w", err)
175+
}
173176

174-
stubStatus := ncp.crossplaneConfigTraverseAPIDetails(ctx, &conf, ncp.apiCallback, stubStatusAPIDirective)
175-
if stubStatus.URL != "" {
176-
nginxConfigContext.StubStatus = stubStatus
177-
}
177+
stubStatus := ncp.crossplaneConfigTraverseAPIDetails(ctx, &conf, ncp.apiCallback, stubStatusAPIDirective)
178+
if stubStatus.URL != "" {
179+
nginxConfigContext.StubStatus = stubStatus
180+
}
178181

179-
plusAPI := ncp.crossplaneConfigTraverseAPIDetails(ctx, &conf, ncp.apiCallback, plusAPIDirective)
180-
if plusAPI.URL != "" {
181-
nginxConfigContext.PlusAPI = plusAPI
182-
}
182+
plusAPI := ncp.crossplaneConfigTraverseAPIDetails(ctx, &conf, ncp.apiCallback, plusAPIDirective)
183+
if plusAPI.URL != "" {
184+
nginxConfigContext.PlusAPI = plusAPI
185+
}
183186

184-
fileMeta, err := files.FileMeta(conf.File)
185-
if err != nil {
186-
slog.WarnContext(ctx, "Unable to get file metadata", "file_name", conf.File, "error", err)
187-
} else {
188-
nginxConfigContext.Files = append(nginxConfigContext.Files, &mpi.File{FileMeta: fileMeta})
189-
}
187+
fileMeta, err := files.FileMeta(conf.File)
188+
if err != nil {
189+
slog.WarnContext(ctx, "Unable to get file metadata", "file_name", conf.File, "error", err)
190190
} else {
191-
slog.WarnContext(ctx, "File in NGINX config not in allowed directories, excluding from config",
192-
"file", conf.File)
191+
nginxConfigContext.Files = append(nginxConfigContext.Files, &mpi.File{FileMeta: fileMeta})
193192
}
194193
}
195194

196195
return nginxConfigContext, nil
197196
}
198197

199198
func (ncp *NginxConfigParser) ignoreLog(logPath string) bool {
200-
logLower := strings.ToLower(logPath)
201199
ignoreLogs := []string{"off", "/dev/stderr", "/dev/stdout", "/dev/null", "stderr", "stdout"}
202200

203-
if strings.HasPrefix(logLower, "syslog:") || slices.Contains(ignoreLogs, logLower) {
201+
if strings.HasPrefix(logPath, "syslog:") || slices.Contains(ignoreLogs, logPath) {
204202
return true
205203
}
206204

207-
if ncp.isExcludeLog(logLower) {
205+
if ncp.isExcludeLog(logPath) {
208206
return true
209207
}
210208

211-
if !ncp.agentConfig.IsDirectoryAllowed(logLower) {
209+
if !ncp.agentConfig.IsDirectoryAllowed(logPath) {
212210
slog.Warn("Log being read is outside of allowed directories", "log_path", logPath)
213211
}
214212

@@ -317,39 +315,6 @@ func (ncp *NginxConfigParser) errorLogDirectiveLevel(directive *crossplane.Direc
317315
return ""
318316
}
319317

320-
func (ncp *NginxConfigParser) rootFiles(ctx context.Context, rootDir string) (rootFiles []*mpi.File) {
321-
if !ncp.agentConfig.IsDirectoryAllowed(rootDir) {
322-
slog.DebugContext(ctx, "Root directory not in allowed directories", "root_directory", rootDir)
323-
return rootFiles
324-
}
325-
326-
err := filepath.WalkDir(rootDir,
327-
func(path string, d fs.DirEntry, err error) error {
328-
if err != nil {
329-
return err
330-
}
331-
332-
if d.IsDir() {
333-
return nil
334-
}
335-
336-
rootFileMeta, fileMetaErr := files.FileMeta(path)
337-
if fileMetaErr != nil {
338-
return fileMetaErr
339-
}
340-
341-
rootFiles = append(rootFiles, &mpi.File{FileMeta: rootFileMeta})
342-
343-
return nil
344-
},
345-
)
346-
if err != nil {
347-
slog.WarnContext(ctx, "Unable to walk root directory", "root_directory", rootDir)
348-
}
349-
350-
return rootFiles
351-
}
352-
353318
func (ncp *NginxConfigParser) sslCert(ctx context.Context, file, rootDir string) (sslCertFile *mpi.File) {
354319
if strings.Contains(file, "$") {
355320
// cannot process any filepath with variables

0 commit comments

Comments
 (0)