Skip to content

Commit 8a05e9a

Browse files
authored
Add nil check for SSL certificate file paths (#1027)
* Add nil check for SSL certificate file paths * Add unit test
1 parent 632970f commit 8a05e9a

File tree

4 files changed

+68
-3
lines changed

4 files changed

+68
-3
lines changed

internal/watcher/instance/nginx_config_parser.go

+7-3
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
116116
rootDir := filepath.Dir(instance.GetInstanceRuntime().GetConfigPath())
117117

118118
for _, conf := range payload.Config {
119+
slog.DebugContext(ctx, "Traversing NGINX config file", "config", conf)
119120
if !ncp.agentConfig.IsDirectoryAllowed(conf.File) {
120121
slog.WarnContext(ctx, "File included in NGINX config is outside of allowed directories, "+
121122
"excluding from config",
@@ -148,8 +149,11 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
148149
case "ssl_certificate", "proxy_ssl_certificate", "ssl_client_certificate",
149150
"ssl_trusted_certificate":
150151
sslCertFile := ncp.sslCert(ctx, directive.Args[0], rootDir)
151-
if !ncp.isDuplicateFile(nginxConfigContext.Files, sslCertFile) {
152-
nginxConfigContext.Files = append(nginxConfigContext.Files, sslCertFile)
152+
if sslCertFile != nil {
153+
if !ncp.isDuplicateFile(nginxConfigContext.Files, sslCertFile) {
154+
slog.DebugContext(ctx, "Adding SSL certificate file", "ssl_cert", sslCertFile)
155+
nginxConfigContext.Files = append(nginxConfigContext.Files, sslCertFile)
156+
}
153157
}
154158

155159
case "app_protect_security_log":
@@ -321,7 +325,7 @@ func (ncp *NginxConfigParser) errorLogDirectiveLevel(directive *crossplane.Direc
321325

322326
func (ncp *NginxConfigParser) sslCert(ctx context.Context, file, rootDir string) (sslCertFile *mpi.File) {
323327
if strings.Contains(file, "$") {
324-
// cannot process any filepath with variables
328+
slog.DebugContext(ctx, "Cannot process SSL certificate file path with variables", "file", file)
325329
return nil
326330
}
327331

internal/watcher/instance/nginx_config_parser_test.go

+15
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,21 @@ func TestNginxConfigParser_Parse(t *testing.T) {
368368
},
369369
allowedDirectories: []string{dir},
370370
},
371+
{
372+
name: "Test 4: SSL Certificate file path containing variables",
373+
instance: protos.GetNginxPlusInstance([]string{}),
374+
content: testconfig.GetNginxConfWithSSLCertsWithVariables(),
375+
expectedConfigContext: &model.NginxConfigContext{
376+
StubStatus: &model.APIDetails{},
377+
PlusAPI: &model.APIDetails{},
378+
InstanceID: protos.GetNginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(),
379+
Files: []*mpi.File{},
380+
AccessLogs: []*model.AccessLog{},
381+
ErrorLogs: []*model.ErrorLog{},
382+
NAPSysLogServers: nil,
383+
},
384+
allowedDirectories: []string{dir},
385+
},
371386
}
372387

373388
for _, test := range tests {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
worker_processes 1;
2+
events {
3+
worker_connections 1024;
4+
}
5+
6+
http {
7+
default_type application/octet-stream;
8+
9+
sendfile on;
10+
keepalive_timeout 65;
11+
12+
server {
13+
listen 80;listen [::]:80;
14+
listen 443 ssl;listen [::]:443 ssl;
15+
ssl_certificate $secret_dir_path/default-cafe-secret;
16+
ssl_certificate_key $secret_dir_path/default-cafe-secret;
17+
18+
location / {
19+
root /usr/share/nginx/html;
20+
index index.html index.htm;
21+
}
22+
23+
##
24+
# Enable Metrics
25+
##
26+
location /api {
27+
stub_status;
28+
allow 127.0.0.1;
29+
deny all;
30+
}
31+
32+
# redirect server error pages to the static page /50x.html
33+
#
34+
error_page 500 502 503 504 /50x.html;
35+
location = /50x.html {
36+
root /usr/share/nginx/html;
37+
}
38+
}
39+
}

test/config/nginx_config.go

+7
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ var embedNginxConfWithMultipleAccessLogs string
1616
//go:embed nginx/nginx-not-allowed-dir.conf
1717
var embedNginxConfWithNotAllowedDir string
1818

19+
//go:embed nginx/nginx-ssl-certs-with-variables.conf
20+
var embedNginxConfWithSSLCertsWithVariables string
21+
1922
func GetNginxConfigWithMultipleAccessLogs(
2023
errorLogName,
2124
accessLogName,
@@ -34,3 +37,7 @@ func GetNginxConfigWithMultipleAccessLogs(
3437
func GetNginxConfigWithNotAllowedDir(errorLogFile, notAllowedFile, allowedFileDir, accessLogFile string) string {
3538
return fmt.Sprintf(embedNginxConfWithNotAllowedDir, errorLogFile, notAllowedFile, allowedFileDir, accessLogFile)
3639
}
40+
41+
func GetNginxConfWithSSLCertsWithVariables() string {
42+
return embedNginxConfWithSSLCertsWithVariables
43+
}

0 commit comments

Comments
 (0)