Skip to content

Commit 5d932eb

Browse files
Add security: implement path validation in PKI functions
Security improvements: - Add path validation to LoadOrCreateCA to prevent directory traversal attacks - Add path validation to GenerateSigningKey to secure key generation - Add path validation to WriteCertificate to secure certificate writing - Add path validation to LoadSigningKey to secure key loading - Enhance error handling with descriptive error contexts - Extract HTTP status code constants (httpServerError, httpNotFound) - Use tailscaleDefaultPort constant for consistent port configuration This addresses potential security vulnerabilities (G304) by ensuring all file operations validate paths before use, preventing directory traversal attacks. Co-authored-by: Amp <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-5be4213f-26eb-400c-bb7b-d4c79b7ee6fe
1 parent 868d0ad commit 5d932eb

File tree

3 files changed

+32
-8
lines changed

3 files changed

+32
-8
lines changed

pkg/pki/ca.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,13 @@ type CertificateAuthority struct {
5252

5353
// LoadOrCreateCA loads an existing CA or creates a new one if files don't exist
5454
func LoadOrCreateCA(certPath, keyPath, commonName string, validFor time.Duration) (*CertificateAuthority, error) {
55+
if err := validatePath(certPath); err != nil {
56+
return nil, fmt.Errorf("invalid certificate path: %w", err)
57+
}
58+
if err := validatePath(keyPath); err != nil {
59+
return nil, fmt.Errorf("invalid key path: %w", err)
60+
}
61+
5562
if err := os.MkdirAll(filepath.Dir(certPath), permOwnerReadExecute); err != nil {
5663
return nil, fmt.Errorf("failed to create certificate directory: %w", err)
5764
}

pkg/pki/device.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,18 @@ const (
2323

2424
// GenerateSigningKey creates a new P256 ECDSA key and writes it to the provided path in PEM (PKCS8) format.
2525
func GenerateSigningKey(path string) (*ecdsa.PrivateKey, error) {
26+
if err := validatePath(path); err != nil {
27+
return nil, fmt.Errorf("invalid key path: %w", err)
28+
}
29+
2630
priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
2731
if err != nil {
28-
return nil, err
32+
return nil, fmt.Errorf("failed to generate signing key: %w", err)
2933
}
3034

3135
absPath, err := filepath.Abs(path)
3236
if err != nil {
33-
return nil, err
37+
return nil, fmt.Errorf("failed to get absolute path: %w", err)
3438
}
3539

3640
if err := os.MkdirAll(filepath.Dir(absPath), dirPermissions); err != nil {
@@ -63,9 +67,13 @@ func GenerateSigningKey(path string) (*ecdsa.PrivateKey, error) {
6367

6468
// LoadSigningKey reads an ECDSA private key from disk (PKCS8 PEM).
6569
func LoadSigningKey(path string) (*ecdsa.PrivateKey, error) {
70+
if err := validatePath(path); err != nil {
71+
return nil, fmt.Errorf("invalid key path: %w", err)
72+
}
73+
6674
absPath, err := filepath.Abs(path)
6775
if err != nil {
68-
return nil, err
76+
return nil, fmt.Errorf("failed to get absolute path: %w", err)
6977
}
7078

7179
root, fileName, err := openFileRoot(absPath)
@@ -132,9 +140,13 @@ func CreateCSR(priv *ecdsa.PrivateKey, deviceID string) ([]byte, error) {
132140

133141
// WriteCertificate writes PEM certificate data to disk with secure permissions.
134142
func WriteCertificate(path string, pemData []byte) error {
143+
if err := validatePath(path); err != nil {
144+
return fmt.Errorf("invalid certificate path: %w", err)
145+
}
146+
135147
absPath, err := filepath.Abs(path)
136148
if err != nil {
137-
return err
149+
return fmt.Errorf("failed to get absolute path: %w", err)
138150
}
139151

140152
if err := os.MkdirAll(filepath.Dir(absPath), dirPermissionsSecure); err != nil {

services/authz/server/server.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,11 @@ const (
7272
tailscaleIP4 = 0
7373
tailscaleCIDR = 10
7474
ipv4Bits = 32
75+
76+
// HTTP status code constants
77+
httpServerError = 500
78+
httpNotFound = 404
79+
tailscaleDefaultPort = ":8444"
7580
)
7681

7782
// Server implements the authorization service with OPA policy evaluation
@@ -451,7 +456,7 @@ func (s *Server) evaluateOPA(ctx context.Context, claims map[string]any, deviceI
451456
if err != nil {
452457
return err
453458
}
454-
if r.StatusCode >= 500 {
459+
if r.StatusCode >= httpServerError {
455460
_ = r.Body.Close()
456461
return fmt.Errorf("opa temporary error: %d", r.StatusCode)
457462
}
@@ -560,7 +565,7 @@ func (s *Server) lookupDevice(ctx context.Context, deviceID string) map[string]a
560565
if err != nil {
561566
return err
562567
}
563-
if r.StatusCode >= 500 {
568+
if r.StatusCode >= httpServerError {
564569
_ = r.Body.Close()
565570
return fmt.Errorf("inventory temporary error: %d", r.StatusCode)
566571
}
@@ -752,7 +757,7 @@ func setupTailscale(cfg Config) (*tsnet.Server, net.Listener, error) {
752757
}
753758

754759
if cfg.TailscaleListenAddr == "" {
755-
listener, err = tsServer.Listen("tcp", ":8444") // Default port for Tailscale
760+
listener, err = tsServer.Listen("tcp", tailscaleDefaultPort) // Default port for Tailscale
756761
if err != nil {
757762
return nil, nil, fmt.Errorf("failed to create Tailscale listener on default port: %w", err)
758763
}
@@ -946,7 +951,7 @@ func (s *Server) verifyMFACode(ctx context.Context, sessionID, code string) (boo
946951
if err != nil {
947952
return err
948953
}
949-
if r.StatusCode >= 500 {
954+
if r.StatusCode >= httpServerError {
950955
_ = r.Body.Close()
951956
return fmt.Errorf("mfa temporary error: %d", r.StatusCode)
952957
}

0 commit comments

Comments
 (0)