Skip to content

Commit 0bd5438

Browse files
Refactor jail architecture with integrated services (#15)
* start * Refactor jail to use integrated services architecture - Updated namespace implementations to include environment setup in Command method - Migrated Execute function logic from namespaces into Command method with proper env, credentials, and I/O setup - Enhanced jail.Jail to integrate proxy server, certificate manager, rule engine, and auditor - Simplified CLI to use unified jail.Jail instead of managing separate components - Maintained all existing functionality while improving code organization - All tests pass and binary builds successfully Co-authored-by: f0ssel <[email protected]> * Move environment setup to Open method in namespace implementations - Move environment preparation from Command() to Open() method in both Linux and macOS namespaces - Add preparedEnv field to store the prepared environment once during setup - Simplify Command() methods to just inject the prepared environment - Improves performance by avoiding repeated environment setup on each command - Maintains all existing functionality while improving efficiency Co-authored-by: f0ssel <[email protected]> * Move sudo credential setup to Open method in namespace implementations - Move sudo credential preparation from Command() to Open() method in both Linux and macOS namespaces - Add procAttr field to store prepared syscall.SysProcAttr with credentials - Simplify Command() methods to just use the prepared process attributes - Eliminates repeated sudo environment checks and credential parsing on each command - Further improves performance and code organization - All credential handling now happens once during setup phase Co-authored-by: f0ssel <[email protected]> * Move construction logic from jail.New to CLI caller - Simplify jail.Config to only contain dependencies (Commander, ProxyServer, etc) - Move all component construction logic from jail.New into CLI - jail.New now only accepts pre-constructed dependencies and assembles them - CLI handles rule parsing, certificate manager creation, proxy server setup - Better separation of concerns: jail package focuses on orchestration, CLI handles construction - Eliminates error handling in jail.New since dependencies are pre-validated - Makes jail package more testable with dependency injection Co-authored-by: f0ssel <[email protected]> * Remove unneeded dependencies from jail.Config - Remove RuleEngine, Auditor, and CertManager from jail.Config - These dependencies are only needed by ProxyServer, not by Jail directly - Simplify jail.Config to only contain CommandExecutor, ProxyServer, and Logger - Remove GetCACertPEM method since CertManager is no longer available in Jail - Clean up unused imports (audit, rules, tls packages) - CA certificate handling remains in CLI where CertManager is constructed - Further simplifies the Jail orchestration layer Co-authored-by: f0ssel <[email protected]> * remove no cleanup * setenv * Convert preparedEnv to map and implement SetEnv methods - Convert preparedEnv from []string to map[string]string for better environment management - Implement SetEnv methods in both Linux and macOS namespace implementations - Add CommandExecutor accessor method to Jail for SetEnv access - Update CLI to use SetEnv method for CA certificate environment variables - Remove Env field from namespace.Config since SetEnv is used instead - Environment variables now properly managed through SetEnv interface - Allows dynamic environment variable setting after initialization - Better encapsulation and control over environment variables Co-authored-by: f0ssel <[email protected]> * fix * fix * fix * fix * Move TLS setup logic to CertificateManager method - Add SetupTLSAndWriteCACert method to CertificateManager - Combines getting TLS config, CA cert PEM, and writing CA cert to file - Returns TLS config, CA cert path, and CA cert PEM in one call - Update CLI to use the new method instead of separate calls - Reduces complexity in CLI Run function - Better encapsulation of TLS-related setup logic - Remove unused filepath import from CLI - Clean separation between TLS setup and CLI orchestration Co-authored-by: f0ssel <[email protected]> * Move GetConfigDir call into CertificateManager and remove unused return value - Update SetupTLSAndWriteCACert to call tls.GetConfigDir() internally - Return config directory as part of the method's return values - Remove unused []byte (CA cert PEM) from return values since it's never used - Update CLI to handle new return signature with configDir - Remove separate GetConfigDir call from CLI - Pass empty string to NewCertificateManager since configDir is determined internally - Further simplifies CLI by removing another external dependency call - Better encapsulation of config directory management within TLS package Co-authored-by: f0ssel <[email protected]> * fix --------- Co-authored-by: blink-so[bot] <211532188+blink-so[bot]@users.noreply.github.com> Co-authored-by: f0ssel <[email protected]>
1 parent 5678372 commit 0bd5438

File tree

11 files changed

+376
-343
lines changed

11 files changed

+376
-343
lines changed

cli/cli.go

Lines changed: 55 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,12 @@ import (
77
"log/slog"
88
"os"
99
"os/signal"
10-
"path/filepath"
1110
"strings"
1211
"syscall"
13-
"time"
1412

13+
"github.com/coder/jail"
1514
"github.com/coder/jail/audit"
16-
"github.com/coder/jail/network"
15+
"github.com/coder/jail/namespace"
1716
"github.com/coder/jail/proxy"
1817
"github.com/coder/jail/rules"
1918
"github.com/coder/jail/tls"
@@ -25,7 +24,6 @@ type Config struct {
2524
AllowStrings []string
2625
NoTLSIntercept bool
2726
LogLevel string
28-
NoJailCleanup bool
2927
}
3028

3129
// NewCommand creates and returns the root serpent command
@@ -70,14 +68,6 @@ Examples:
7068
Default: "warn",
7169
Value: serpent.StringOf(&config.LogLevel),
7270
},
73-
{
74-
Name: "no-jail-cleanup",
75-
Flag: "no-jail-cleanup",
76-
Env: "JAIL_NO_JAIL_CLEANUP",
77-
Description: "Skip jail cleanup (hidden flag for testing).",
78-
Value: serpent.BoolOf(&config.NoJailCleanup),
79-
Hidden: true,
80-
},
8171
},
8272
Handler: func(inv *serpent.Invocation) error {
8373
return Run(config, inv.Args)
@@ -123,90 +113,85 @@ func Run(config Config, args []string) error {
123113
logger.Warn("No allow rules specified; all network traffic will be denied by default")
124114
}
125115

116+
// Parse allow rules
126117
allowRules, err := rules.ParseAllowSpecs(config.AllowStrings)
127118
if err != nil {
128119
logger.Error("Failed to parse allow rules", "error", err)
129120
return fmt.Errorf("failed to parse allow rules: %v", err)
130121
}
131122

132-
// Implicit final deny-all is handled by the RuleEngine default behavior when no rules match.
133-
// Build final rules slice in order: user allows only.
134-
ruleList := allowRules
135-
136123
// Create rule engine
137-
ruleEngine := rules.NewRuleEngine(ruleList, logger)
124+
ruleEngine := rules.NewRuleEngine(allowRules, logger)
125+
126+
// Create auditor
127+
auditor := audit.NewLoggingAuditor(logger)
138128

139-
// Get configuration directory
140-
configDir, err := tls.GetConfigDir()
129+
// Create network namespace configuration
130+
nsConfig := namespace.Config{
131+
HTTPPort: 8040,
132+
HTTPSPort: 8043,
133+
}
134+
135+
// Create commander
136+
commander, err := namespace.New(nsConfig, logger)
141137
if err != nil {
142-
logger.Error("Failed to get config directory", "error", err)
143-
return fmt.Errorf("failed to get config directory: %v", err)
138+
logger.Error("Failed to create network namespace", "error", err)
139+
return fmt.Errorf("failed to create network namespace: %v", err)
144140
}
145141

146142
// Create certificate manager (if TLS interception is enabled)
147-
var certManager *tls.CertificateManager
148143
var tlsConfig *cryptotls.Config
149-
var extraEnv map[string]string = make(map[string]string)
150-
151144
if !config.NoTLSIntercept {
152-
certManager, err = tls.NewCertificateManager(configDir, logger)
145+
certManager, err := tls.NewCertificateManager("", logger) // Empty configDir since it will be determined internally
153146
if err != nil {
154147
logger.Error("Failed to create certificate manager", "error", err)
155148
return fmt.Errorf("failed to create certificate manager: %v", err)
156149
}
157150

158-
tlsConfig = certManager.GetTLSConfig()
159-
160-
// Get CA certificate for environment
161-
caCertPEM, err := certManager.GetCACertPEM()
162-
if err != nil {
163-
logger.Error("Failed to get CA certificate", "error", err)
164-
return fmt.Errorf("failed to get CA certificate: %v", err)
165-
}
166-
167-
// Write CA certificate to a temporary file for tools that need a file path
168-
caCertPath := filepath.Join(configDir, "ca-cert.pem")
169-
err = os.WriteFile(caCertPath, caCertPEM, 0644)
151+
// Setup TLS config and write CA certificate to file
152+
var caCertPath, configDir string
153+
tlsConfig, caCertPath, configDir, err = certManager.SetupTLSAndWriteCACert()
170154
if err != nil {
171-
logger.Error("Failed to write CA certificate file", "error", err)
172-
return fmt.Errorf("failed to write CA certificate file: %v", err)
155+
logger.Error("Failed to setup TLS and CA certificate", "error", err)
156+
return fmt.Errorf("failed to setup TLS and CA certificate: %v", err)
173157
}
174158

175159
// Set standard CA certificate environment variables for common tools
176160
// This makes tools like curl, git, etc. trust our dynamically generated CA
177-
extraEnv["SSL_CERT_FILE"] = caCertPath // OpenSSL/LibreSSL-based tools
178-
extraEnv["SSL_CERT_DIR"] = configDir // OpenSSL certificate directory
179-
extraEnv["CURL_CA_BUNDLE"] = caCertPath // curl
180-
extraEnv["GIT_SSL_CAINFO"] = caCertPath // Git
181-
extraEnv["REQUESTS_CA_BUNDLE"] = caCertPath // Python requests
182-
extraEnv["NODE_EXTRA_CA_CERTS"] = caCertPath // Node.js
183-
extraEnv["JAIL_CA_CERT"] = string(caCertPEM) // Keep for backward compatibility
161+
commander.SetEnv("SSL_CERT_FILE", caCertPath) // OpenSSL/LibreSSL-based tools
162+
commander.SetEnv("SSL_CERT_DIR", configDir) // OpenSSL certificate directory
163+
commander.SetEnv("CURL_CA_BUNDLE", caCertPath) // curl
164+
commander.SetEnv("GIT_SSL_CAINFO", caCertPath) // Git
165+
commander.SetEnv("REQUESTS_CA_BUNDLE", caCertPath) // Python requests
166+
commander.SetEnv("NODE_EXTRA_CA_CERTS", caCertPath) // Node.js
184167
}
185168

186-
// Create network jail configuration
187-
networkConfig := network.JailConfig{
188-
HTTPPort: 8040,
189-
HTTPSPort: 8043,
190-
NetJailName: "jail",
191-
SkipCleanup: config.NoJailCleanup,
192-
}
169+
// Create proxy server
170+
proxyServer := proxy.NewProxyServer(proxy.Config{
171+
HTTPPort: 8040,
172+
HTTPSPort: 8043,
173+
RuleEngine: ruleEngine,
174+
Auditor: auditor,
175+
Logger: logger,
176+
TLSConfig: tlsConfig,
177+
})
193178

194-
// Create network jail
195-
networkInstance, err := network.NewJail(networkConfig, logger)
196-
if err != nil {
197-
logger.Error("Failed to create network jail", "error", err)
198-
return fmt.Errorf("failed to create network jail: %v", err)
199-
}
179+
// Create jail instance
180+
jailInstance := jail.New(jail.Config{
181+
Commander: commander,
182+
ProxyServer: proxyServer,
183+
Logger: logger,
184+
})
200185

201-
// Setup signal handling BEFORE any network setup
186+
// Setup signal handling BEFORE any setup
202187
sigChan := make(chan os.Signal, 1)
203188
signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM)
204189

205190
// Handle signals immediately in background
206191
go func() {
207192
sig := <-sigChan
208193
logger.Info("Received signal during setup, cleaning up...", "signal", sig)
209-
err := networkInstance.Cleanup()
194+
err := jailInstance.Close()
210195
if err != nil {
211196
logger.Error("Emergency cleanup failed", "error", err)
212197
}
@@ -216,55 +201,29 @@ func Run(config Config, args []string) error {
216201
// Ensure cleanup happens no matter what
217202
defer func() {
218203
logger.Debug("Starting cleanup process")
219-
err := networkInstance.Cleanup()
204+
err := jailInstance.Close()
220205
if err != nil {
221-
logger.Error("Failed to cleanup network jail", "error", err)
206+
logger.Error("Failed to cleanup jail", "error", err)
222207
} else {
223208
logger.Debug("Cleanup completed successfully")
224209
}
225210
}()
226211

227-
// Setup network jail
228-
err = networkInstance.Setup(networkConfig.HTTPPort, networkConfig.HTTPSPort)
212+
// Open jail (starts network namespace and proxy server)
213+
err = jailInstance.Open()
229214
if err != nil {
230-
logger.Error("Failed to setup network jail", "error", err)
231-
return fmt.Errorf("failed to setup network jail: %v", err)
232-
}
233-
234-
// Create auditor
235-
auditor := audit.NewLoggingAuditor(logger)
236-
237-
// Create proxy server
238-
proxyConfig := proxy.Config{
239-
HTTPPort: networkConfig.HTTPPort,
240-
HTTPSPort: networkConfig.HTTPSPort,
241-
RuleEngine: ruleEngine,
242-
Auditor: auditor,
243-
Logger: logger,
244-
TLSConfig: tlsConfig,
215+
logger.Error("Failed to open jail", "error", err)
216+
return fmt.Errorf("failed to open jail: %v", err)
245217
}
246218

247-
proxyServer := proxy.NewProxyServer(proxyConfig)
248-
249219
// Create context for graceful shutdown
250220
ctx, cancel := context.WithCancel(context.Background())
251221
defer cancel()
252222

253-
// Start proxy server in background
254-
go func() {
255-
err := proxyServer.Start(ctx)
256-
if err != nil {
257-
logger.Error("Proxy server error", "error", err)
258-
}
259-
}()
260-
261-
// Give proxy time to start
262-
time.Sleep(100 * time.Millisecond)
263-
264-
// Execute command in network jail
223+
// Execute command in jail
265224
go func() {
266225
defer cancel()
267-
err := networkInstance.Execute(args, extraEnv)
226+
err := jailInstance.Command(args).Run()
268227
if err != nil {
269228
logger.Error("Command execution failed", "error", err)
270229
}
@@ -277,12 +236,7 @@ func Run(config Config, args []string) error {
277236
cancel()
278237
case <-ctx.Done():
279238
// Context cancelled by command completion
280-
}
281-
282-
// Stop proxy server
283-
err = proxyServer.Stop()
284-
if err != nil {
285-
logger.Error("Failed to stop proxy server", "error", err)
239+
logger.Info("Command completed, shutting down...")
286240
}
287241

288242
return nil

jail.go

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package jail
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"log/slog"
7+
"os/exec"
8+
"time"
9+
10+
"github.com/coder/jail/proxy"
11+
)
12+
13+
type Commander interface {
14+
Open() error
15+
SetEnv(key string, value string)
16+
Command(command []string) *exec.Cmd
17+
Close() error
18+
}
19+
20+
type Config struct {
21+
Commander Commander
22+
ProxyServer *proxy.ProxyServer
23+
Logger *slog.Logger
24+
}
25+
26+
type Jail struct {
27+
commandExecutor Commander
28+
proxyServer *proxy.ProxyServer
29+
logger *slog.Logger
30+
cancel context.CancelFunc
31+
ctx context.Context
32+
}
33+
34+
func New(config Config) *Jail {
35+
ctx, cancel := context.WithCancel(context.Background())
36+
37+
return &Jail{
38+
commandExecutor: config.Commander,
39+
proxyServer: config.ProxyServer,
40+
logger: config.Logger,
41+
ctx: ctx,
42+
cancel: cancel,
43+
}
44+
}
45+
46+
func (j *Jail) Open() error {
47+
// Open the command executor (network namespace)
48+
err := j.commandExecutor.Open()
49+
if err != nil {
50+
return fmt.Errorf("failed to open command executor: %v", err)
51+
}
52+
53+
// Start proxy server in background
54+
go func() {
55+
err := j.proxyServer.Start(j.ctx)
56+
if err != nil {
57+
j.logger.Error("Proxy server error", "error", err)
58+
}
59+
}()
60+
61+
// Give proxy time to start
62+
time.Sleep(100 * time.Millisecond)
63+
64+
return nil
65+
}
66+
67+
func (j *Jail) Command(command []string) *exec.Cmd {
68+
return j.commandExecutor.Command(command)
69+
}
70+
71+
func (j *Jail) Close() error {
72+
// Cancel context to stop proxy server
73+
if j.cancel != nil {
74+
j.cancel()
75+
}
76+
77+
// Stop proxy server
78+
if j.proxyServer != nil {
79+
err := j.proxyServer.Stop()
80+
if err != nil {
81+
j.logger.Error("Failed to stop proxy server", "error", err)
82+
}
83+
}
84+
85+
// Close command executor
86+
return j.commandExecutor.Close()
87+
}

0 commit comments

Comments
 (0)