Skip to content

Commit 8789653

Browse files
committed
feat(metrics): validate endpoint TLS file permissions
1 parent b0717e4 commit 8789653

File tree

5 files changed

+163
-11
lines changed

5 files changed

+163
-11
lines changed

cmd/run.go

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,9 @@ func Run(log *logrus.Logger, conf *config.Config, externGeoDataDirs []string) (e
151151
}(endpointServer, cfg)
152152
}
153153
endpointCfg := endpointConfigFromGlobal(conf, log)
154+
if err = validateEndpointTLSFiles(endpointCfg); err != nil {
155+
return fmt.Errorf("invalid endpoint tls config: %w", err)
156+
}
154157
startEndpointServer(endpointCfg)
155158

156159
// Serve tproxy TCP/UDP server util signals.
@@ -264,6 +267,16 @@ loop:
264267
log.SetOutput(oldLogOutput) // FIXME: THIS IS A HACK.
265268
logrus.SetOutput(oldLogOutput)
266269

270+
newEndpointCfg := endpointConfigFromGlobal(newConf, log)
271+
if err = validateEndpointTLSFiles(newEndpointCfg); err != nil {
272+
log.WithFields(logrus.Fields{
273+
"err": err,
274+
}).Errorln("[Reload] Failed to reload")
275+
sdnotify.Ready()
276+
_ = os.WriteFile(SignalProgressFilePath, append([]byte{consts.ReloadError}, []byte("\n"+err.Error())...), 0644)
277+
continue
278+
}
279+
267280
// New control plane.
268281
obj := c.EjectBpf()
269282
var dnsCache map[string]*control.DnsCache
@@ -315,7 +328,6 @@ loop:
315328
}
316329
oldC.Close()
317330

318-
newEndpointCfg := endpointConfigFromGlobal(newConf, log)
319331
if endpointConfigChanged(endpointCfg, newEndpointCfg) {
320332
if endpointServer != nil {
321333
_ = endpointServer.Shutdown(context.Background())
@@ -364,6 +376,42 @@ func endpointConfigChanged(a, b metrics.EndpointConfig) bool {
364376
return a != b
365377
}
366378

379+
func validateEndpointTLSFiles(cfg metrics.EndpointConfig) error {
380+
if cfg.TlsCertificate == "" && cfg.TlsKey == "" {
381+
return nil
382+
}
383+
if cfg.TlsCertificate == "" || cfg.TlsKey == "" {
384+
return fmt.Errorf("endpoint_tls_certificate and endpoint_tls_key must be configured together")
385+
}
386+
387+
certFile, err := os.Open(cfg.TlsCertificate)
388+
if err != nil {
389+
return fmt.Errorf("cannot open endpoint_tls_certificate '%s': %w", cfg.TlsCertificate, err)
390+
}
391+
certFi, err := certFile.Stat()
392+
certFile.Close()
393+
if err != nil {
394+
return fmt.Errorf("cannot stat endpoint_tls_certificate '%s': %w", cfg.TlsCertificate, err)
395+
}
396+
if err = common.ValidateFilePermissionAllowed(cfg.TlsCertificate, certFi, 0o640, 0o644); err != nil {
397+
return fmt.Errorf("invalid endpoint_tls_certificate: %w", err)
398+
}
399+
400+
keyFile, err := os.Open(cfg.TlsKey)
401+
if err != nil {
402+
return fmt.Errorf("cannot open endpoint_tls_key '%s': %w", cfg.TlsKey, err)
403+
}
404+
keyFi, err := keyFile.Stat()
405+
keyFile.Close()
406+
if err != nil {
407+
return fmt.Errorf("cannot stat endpoint_tls_key '%s': %w", cfg.TlsKey, err)
408+
}
409+
if err = common.ValidateFilePermissionAllowed(cfg.TlsKey, keyFi, 0o600); err != nil {
410+
return fmt.Errorf("invalid endpoint_tls_key: %w", err)
411+
}
412+
return nil
413+
}
414+
367415
func newControlPlane(log *logrus.Logger, bpf interface{}, dnsCache map[string]*control.DnsCache, conf *config.Config, externGeoDataDirs []string) (c *control.ControlPlane, err error) {
368416
// Deep copy to prevent modification.
369417
conf = deepcopy.Copy(conf).(*config.Config)

common/file_permission.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* SPDX-License-Identifier: AGPL-3.0-only
3+
* Copyright (c) 2022-2025, daeuniverse Organization <dae@v2raya.org>
4+
*/
5+
6+
package common
7+
8+
import (
9+
"fmt"
10+
"os"
11+
"sort"
12+
"strings"
13+
)
14+
15+
func ValidateFilePermissionNotTooOpen(path string, fi os.FileInfo) error {
16+
if fi.IsDir() {
17+
return fmt.Errorf("cannot read a directory: %v", path)
18+
}
19+
if fi.Mode()&0o037 > 0 {
20+
return fmt.Errorf("permissions %04o for '%v' are too open; requires the file is NOT writable by the same group and NOT accessible by others; suggest 0640 or 0600", fi.Mode()&0o777, path)
21+
}
22+
return nil
23+
}
24+
25+
func ValidateFilePermissionAllowed(path string, fi os.FileInfo, allowedModes ...os.FileMode) error {
26+
if fi.IsDir() {
27+
return fmt.Errorf("cannot read a directory: %v", path)
28+
}
29+
perm := fi.Mode().Perm()
30+
for _, mode := range allowedModes {
31+
if perm == mode.Perm() {
32+
return nil
33+
}
34+
}
35+
if len(allowedModes) == 0 {
36+
return fmt.Errorf("permissions %04o for '%v' are invalid", perm, path)
37+
}
38+
allowed := make([]string, 0, len(allowedModes))
39+
for _, mode := range allowedModes {
40+
allowed = append(allowed, fmt.Sprintf("%04o", mode.Perm()))
41+
}
42+
sort.Strings(allowed)
43+
return fmt.Errorf("permissions %04o for '%v' are invalid; allowed: %s", perm, path, strings.Join(allowed, ", "))
44+
}

common/file_permission_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/*
2+
* SPDX-License-Identifier: AGPL-3.0-only
3+
* Copyright (c) 2022-2025, daeuniverse Organization <dae@v2raya.org>
4+
*/
5+
6+
package common
7+
8+
import (
9+
"os"
10+
"path/filepath"
11+
"testing"
12+
)
13+
14+
func writeTempFile(t *testing.T, mode os.FileMode) (string, os.FileInfo) {
15+
t.Helper()
16+
dir := t.TempDir()
17+
path := filepath.Join(dir, "x")
18+
if err := os.WriteFile(path, []byte("x"), 0o600); err != nil {
19+
t.Fatalf("write temp file: %v", err)
20+
}
21+
if err := os.Chmod(path, mode); err != nil {
22+
t.Fatalf("chmod temp file: %v", err)
23+
}
24+
fi, err := os.Stat(path)
25+
if err != nil {
26+
t.Fatalf("stat temp file: %v", err)
27+
}
28+
return path, fi
29+
}
30+
31+
func TestValidateFilePermissionNotTooOpen(t *testing.T) {
32+
_, fi0600 := writeTempFile(t, 0o600)
33+
if err := ValidateFilePermissionNotTooOpen("test-0600", fi0600); err != nil {
34+
t.Fatalf("0600 should pass: %v", err)
35+
}
36+
37+
_, fi0640 := writeTempFile(t, 0o640)
38+
if err := ValidateFilePermissionNotTooOpen("test-0640", fi0640); err != nil {
39+
t.Fatalf("0640 should pass: %v", err)
40+
}
41+
42+
_, fi0644 := writeTempFile(t, 0o644)
43+
if err := ValidateFilePermissionNotTooOpen("test-0644", fi0644); err == nil {
44+
t.Fatal("0644 should fail as too open")
45+
}
46+
}
47+
48+
func TestValidateFilePermissionAllowed(t *testing.T) {
49+
_, fi0600 := writeTempFile(t, 0o600)
50+
if err := ValidateFilePermissionAllowed("key", fi0600, 0o600); err != nil {
51+
t.Fatalf("0600 should pass for key: %v", err)
52+
}
53+
if err := ValidateFilePermissionAllowed("key", fi0600, 0o640, 0o644); err == nil {
54+
t.Fatal("0600 should fail for cert-only allowed modes")
55+
}
56+
57+
_, fi0640 := writeTempFile(t, 0o640)
58+
if err := ValidateFilePermissionAllowed("cert", fi0640, 0o640, 0o644); err != nil {
59+
t.Fatalf("0640 should pass for cert: %v", err)
60+
}
61+
62+
_, fi0644 := writeTempFile(t, 0o644)
63+
if err := ValidateFilePermissionAllowed("cert", fi0644, 0o640, 0o644); err != nil {
64+
t.Fatalf("0644 should pass for cert: %v", err)
65+
}
66+
}

common/subscription/subscription.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,8 @@ func ResolveFile(u *url.URL, configDir string) (b []byte, err error) {
112112
if err != nil {
113113
return nil, err
114114
}
115-
if fi.IsDir() {
116-
return nil, fmt.Errorf("subscription file cannot be a directory: %v", path)
117-
}
118-
if fi.Mode()&0037 > 0 {
119-
return nil, fmt.Errorf("permissions %04o for '%v' are too open; requires the file is NOT writable by the same group and NOT accessible by others; suggest 0640 or 0600", fi.Mode()&0777, path)
115+
if err = common.ValidateFilePermissionNotTooOpen(path, fi); err != nil {
116+
return nil, err
120117
}
121118
// Resolve the first line instruction.
122119
fReader := bufio.NewReader(f)

config/config_merger.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,8 @@ func (m *Merger) readEntry(entry string) (err error) {
7171
if err != nil {
7272
return err
7373
}
74-
if fi.IsDir() {
75-
return fmt.Errorf("cannot include a directory: %v", entry)
76-
}
77-
if fi.Mode()&0037 > 0 {
78-
return fmt.Errorf("permissions %04o for '%v' are too open; requires the file is NOT writable by the same group and NOT accessible by others; suggest 0640 or 0600", fi.Mode()&0777, entry)
74+
if err = common.ValidateFilePermissionNotTooOpen(entry, fi); err != nil {
75+
return err
7976
}
8077
// Read and parse.
8178
b, err := io.ReadAll(f)

0 commit comments

Comments
 (0)