From 2dd62b63e959acf78eb46d668123fc439278bff2 Mon Sep 17 00:00:00 2001 From: Michael Reber Date: Fri, 14 Nov 2025 11:44:23 +0100 Subject: [PATCH] replace base64 payload bash -c with a stdin via here-doc to prevent false pharsing --- internal/fail2ban/connector_ssh.go | 31 +++++++++++++++++++++++++++--- internal/fail2ban/manager.go | 11 +++++++++++ pkg/web/handlers.go | 23 ++++++++++++++-------- 3 files changed, 54 insertions(+), 11 deletions(-) diff --git a/internal/fail2ban/connector_ssh.go b/internal/fail2ban/connector_ssh.go index 00c19c4..f91b89b 100644 --- a/internal/fail2ban/connector_ssh.go +++ b/internal/fail2ban/connector_ssh.go @@ -204,9 +204,34 @@ func (sc *SSHConnector) ensureAction(ctx context.Context) error { script := strings.ReplaceAll(sshEnsureActionScript, "__PAYLOAD__", payload) // Base64 encode the entire script to avoid shell escaping issues scriptB64 := base64.StdEncoding.EncodeToString([]byte(script)) - cmd := fmt.Sprintf("echo %s | base64 -d | bash", scriptB64) - _, err := sc.runRemoteCommand(ctx, []string{"bash", "-lc", cmd}) - return err + + // Use sh -s to read commands from stdin, then pass the base64 string via stdin + // This is the most reliable way to pass data via SSH + args := sc.buildSSHArgs([]string{"sh", "-s"}) + cmd := exec.CommandContext(ctx, "ssh", args...) + + // Create a script that reads the base64 string from stdin and pipes it through base64 -d | bash + // We use a here-document to pass the base64 string + scriptContent := fmt.Sprintf("cat <<'ENDBASE64' | base64 -d | bash\n%s\nENDBASE64\n", scriptB64) + cmd.Stdin = strings.NewReader(scriptContent) + + settingSnapshot := config.GetSettings() + if settingSnapshot.Debug { + config.DebugLog("SSH ensureAction command [%s]: ssh %s (with here-doc via stdin)", sc.server.Name, strings.Join(args, " ")) + } + + out, err := cmd.CombinedOutput() + output := strings.TrimSpace(string(out)) + if err != nil { + config.DebugLog("Failed to ensure action file for server %s: %v (output: %s)", sc.server.Name, err, output) + return fmt.Errorf("failed to ensure action file on remote server %s: %w (remote output: %s)", sc.server.Name, err, output) + } + if output != "" { + config.DebugLog("Successfully ensured action file for server %s (output: %s)", sc.server.Name, output) + } else { + config.DebugLog("Successfully ensured action file for server %s (no output)", sc.server.Name) + } + return nil } func (sc *SSHConnector) getJails(ctx context.Context) ([]string, error) { diff --git a/internal/fail2ban/manager.go b/internal/fail2ban/manager.go index 0b7e78c..9353d61 100644 --- a/internal/fail2ban/manager.go +++ b/internal/fail2ban/manager.go @@ -131,6 +131,17 @@ func (m *Manager) UpdateActionFiles(ctx context.Context) error { return lastErr } +// UpdateActionFileForServer updates the action file for a specific server by ID. +func (m *Manager) UpdateActionFileForServer(ctx context.Context, serverID string) error { + m.mu.RLock() + conn, ok := m.connectors[serverID] + m.mu.RUnlock() + if !ok { + return fmt.Errorf("connector for server %s not found or not enabled", serverID) + } + return updateConnectorAction(ctx, conn) +} + // updateConnectorAction updates the action file for a specific connector. func updateConnectorAction(ctx context.Context, conn Connector) error { switch c := conn.(type) { diff --git a/pkg/web/handlers.go b/pkg/web/handlers.go index 23f4f2d..f9cdaf3 100644 --- a/pkg/web/handlers.go +++ b/pkg/web/handlers.go @@ -271,25 +271,32 @@ func UpsertServerHandler(c *gin.Context) { return } + // Check if server exists and was previously disabled + oldServer, wasEnabled := config.GetServerByID(req.ID) + wasDisabled := !wasEnabled || !oldServer.Enabled + server, err := config.UpsertServer(req) if err != nil { c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) return } + // Check if server was just enabled (transition from disabled to enabled) + justEnabled := wasDisabled && server.Enabled + if err := fail2ban.GetManager().ReloadFromSettings(config.GetSettings()); err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } - // Update action file for this server if it's a remote server (SSH or Agent) and enabled - if server.Enabled && (server.Type == "ssh" || server.Type == "agent") { - // ReloadFromSettings already created the connector, so we can update its action file - // We need to trigger an action file update for this specific server - // Since UpdateActionFiles updates all, we can call it, or we can add a single-server method - // For now, we'll update all remote servers (it's idempotent and ensures consistency) - if err := fail2ban.GetManager().UpdateActionFiles(c.Request.Context()); err != nil { - config.DebugLog("Warning: failed to update some remote action files: %v", err) + // Only update action files if: + // 1. Server was just enabled (transition from disabled to enabled) + // 2. Server is a remote server (SSH or Agent) + // Note: ReloadFromSettings already calls ensureAction when creating connectors, + // but we need to update if the server was just enabled to ensure it has the latest callback URL + if justEnabled && (server.Type == "ssh" || server.Type == "agent") { + if err := fail2ban.GetManager().UpdateActionFileForServer(c.Request.Context(), server.ID); err != nil { + config.DebugLog("Warning: failed to update action file for server %s: %v", server.Name, err) // Don't fail the request, just log the warning } }