diff --git a/internal/fail2ban/connector_local.go b/internal/fail2ban/connector_local.go index 97cd98e..bb8029b 100644 --- a/internal/fail2ban/connector_local.go +++ b/internal/fail2ban/connector_local.go @@ -147,9 +147,18 @@ func (lc *LocalConnector) Reload(ctx context.Context) error { // Include the output in the error message for better debugging return fmt.Errorf("fail2ban reload error: %w (output: %s)", err, strings.TrimSpace(out)) } + // Check if output indicates success (fail2ban-client returns "OK" on success) - if strings.TrimSpace(out) != "OK" && strings.TrimSpace(out) != "" { + outputTrimmed := strings.TrimSpace(out) + if outputTrimmed != "OK" && outputTrimmed != "" { config.DebugLog("fail2ban reload output: %s", out) + + // Check for jail errors in output even when command succeeds + // Look for patterns like "Errors in jail 'jailname'. Skipping..." + if strings.Contains(out, "Errors in jail") || strings.Contains(out, "Unable to read the filter") { + // Return an error that includes the output so handler can parse it + return fmt.Errorf("fail2ban reload completed but with errors (output: %s)", strings.TrimSpace(out)) + } } return nil } @@ -285,6 +294,9 @@ func (lc *LocalConnector) UpdateDefaultSettings(ctx context.Context, settings co // EnsureJailLocalStructure implements Connector. func (lc *LocalConnector) EnsureJailLocalStructure(ctx context.Context) error { + // Note: Migration is handled in newConnectorForServer() before + // config.EnsureLocalFail2banAction() is called, so migration has already + // run by the time this method is called. return config.EnsureJailLocalStructure() } diff --git a/internal/fail2ban/connector_ssh.go b/internal/fail2ban/connector_ssh.go index 158bc81..d3ec1a2 100644 --- a/internal/fail2ban/connector_ssh.go +++ b/internal/fail2ban/connector_ssh.go @@ -13,6 +13,7 @@ import ( "strconv" "strings" "sync" + "time" "github.com/swissmakers/fail2ban-ui/internal/config" ) @@ -1241,10 +1242,112 @@ action = %%(action_mwlg)s PY`, escapeForShell(jailLocalPath), escapeForShell(ignoreIPStr), escapeForShell(banactionVal), escapeForShell(banactionAllportsVal), escapeForShell(config.JailLocalBanner()), settings.BantimeIncrement, escapeForShell(settings.Bantime), escapeForShell(settings.Findtime), settings.Maxretry, escapeForShell(settings.Destemail)) + // IMPORTANT: Run migration FIRST before ensuring structure + // This is because ensureJailLocalStructure may overwrite jail.local, + // which would destroy any jail sections that need to be migrated + if err := sc.MigrateJailsFromJailLocalRemote(ctx); err != nil { + config.DebugLog("Warning: No migration done (may be normal if no jails to migrate): %v", err) + // Don't fail - continue with ensuring structure + } + + // Then ensure the basic structure _, err := sc.runRemoteCommand(ctx, []string{"bash", "-lc", ensureScript}) return err } +// MigrateJailsFromJailLocalRemote migrates non-commented jail sections from jail.local to jail.d/*.local files on remote system. +func (sc *SSHConnector) MigrateJailsFromJailLocalRemote(ctx context.Context) error { + jailLocalPath := "/etc/fail2ban/jail.local" + jailDPath := "/etc/fail2ban/jail.d" + + // Check if jail.local exists + checkScript := fmt.Sprintf("test -f %s && echo 'exists' || echo 'notfound'", jailLocalPath) + out, err := sc.runRemoteCommand(ctx, []string{"sh", "-c", checkScript}) + if err != nil || strings.TrimSpace(out) != "exists" { + return nil // Nothing to migrate + } + + // Read jail.local content + content, err := sc.runRemoteCommand(ctx, []string{"cat", jailLocalPath}) + if err != nil { + return fmt.Errorf("failed to read jail.local: %w", err) + } + + // Parse content locally to extract non-commented sections + sections, defaultContent, err := parseJailSectionsUncommented(content) + if err != nil { + return fmt.Errorf("failed to parse jail.local: %w", err) + } + + // If no non-commented, non-DEFAULT jails found, nothing to migrate + if len(sections) == 0 { + config.DebugLog("No jails to migrate from jail.local on remote system") + return nil + } + + // Create backup + backupPath := jailLocalPath + ".backup." + fmt.Sprintf("%d", time.Now().Unix()) + backupScript := fmt.Sprintf("cp %s %s", jailLocalPath, backupPath) + if _, err := sc.runRemoteCommand(ctx, []string{"sh", "-c", backupScript}); err != nil { + return fmt.Errorf("failed to create backup: %w", err) + } + config.DebugLog("Created backup of jail.local at %s on remote system", backupPath) + + // Ensure jail.d directory exists + ensureDirScript := fmt.Sprintf("mkdir -p %s", jailDPath) + if _, err := sc.runRemoteCommand(ctx, []string{"sh", "-c", ensureDirScript}); err != nil { + return fmt.Errorf("failed to create jail.d directory: %w", err) + } + + // Write each jail to its own .local file + migratedCount := 0 + for jailName, jailContent := range sections { + if jailName == "" { + continue + } + + jailFilePath := fmt.Sprintf("%s/%s.local", jailDPath, jailName) + + // Check if .local file already exists + checkFileScript := fmt.Sprintf("test -f %s && echo 'exists' || echo 'notfound'", jailFilePath) + fileOut, err := sc.runRemoteCommand(ctx, []string{"sh", "-c", checkFileScript}) + if err == nil && strings.TrimSpace(fileOut) == "exists" { + config.DebugLog("Skipping migration for jail %s: .local file already exists", jailName) + continue + } + + // Write jail content to .local file using heredoc + // Escape single quotes in content for shell + escapedContent := strings.ReplaceAll(jailContent, "'", "'\"'\"'") + writeScript := fmt.Sprintf(`cat > %s <<'JAILEOF' +%s +JAILEOF +`, jailFilePath, escapedContent) + if _, err := sc.runRemoteCommand(ctx, []string{"bash", "-c", writeScript}); err != nil { + return fmt.Errorf("failed to write jail file %s: %w", jailFilePath, err) + } + config.DebugLog("Migrated jail %s to %s on remote system", jailName, jailFilePath) + migratedCount++ + } + + // Only rewrite jail.local if we migrated something + if migratedCount > 0 { + // Rewrite jail.local with only DEFAULT section + // Escape single quotes in defaultContent for shell + escapedDefault := strings.ReplaceAll(defaultContent, "'", "'\"'\"'") + writeLocalScript := fmt.Sprintf(`cat > %s <<'LOCALEOF' +%s +LOCALEOF +`, jailLocalPath, escapedDefault) + if _, err := sc.runRemoteCommand(ctx, []string{"bash", "-c", writeLocalScript}); err != nil { + return fmt.Errorf("failed to rewrite jail.local: %w", err) + } + config.DebugLog("Migration completed on remote system: moved %d jails to jail.d/", migratedCount) + } + + return nil +} + // parseJailConfigContent parses jail configuration content and returns JailInfo slice. func parseJailConfigContent(content string) []JailInfo { var jails []JailInfo diff --git a/internal/fail2ban/jail_management.go b/internal/fail2ban/jail_management.go index 79cc614..ae14edd 100644 --- a/internal/fail2ban/jail_management.go +++ b/internal/fail2ban/jail_management.go @@ -8,6 +8,7 @@ import ( "regexp" "strings" "sync" + "time" "github.com/swissmakers/fail2ban-ui/internal/config" ) @@ -466,6 +467,236 @@ func parseJailSections(content string) (map[string]string, string, error) { return sections, defaultContent.String(), scanner.Err() } +// parseJailSectionsUncommented parses jail.local content and returns: +// - map of jail name to jail content (excluding DEFAULT, INCLUDES, and commented sections) +// - DEFAULT section content (including commented lines) +// Only extracts non-commented jail sections +func parseJailSectionsUncommented(content string) (map[string]string, string, error) { + sections := make(map[string]string) + var defaultContent strings.Builder + + // Sections that should be ignored (not jails) + ignoredSections := map[string]bool{ + "DEFAULT": true, + "INCLUDES": true, + } + + scanner := bufio.NewScanner(strings.NewReader(content)) + var currentSection string + var currentContent strings.Builder + inDefault := false + sectionIsCommented := false + + for scanner.Scan() { + line := scanner.Text() + trimmed := strings.TrimSpace(line) + + // Check if this is a section header + if strings.HasPrefix(trimmed, "[") && strings.HasSuffix(trimmed, "]") { + // Check if the section is commented + originalLine := strings.TrimSpace(line) + isCommented := strings.HasPrefix(originalLine, "#") + + // Save previous section + if currentSection != "" { + sectionContent := strings.TrimSpace(currentContent.String()) + if inDefault { + // Always include DEFAULT section content (even if commented) + defaultContent.WriteString(sectionContent) + if !strings.HasSuffix(sectionContent, "\n") { + defaultContent.WriteString("\n") + } + } else if !ignoredSections[currentSection] && !sectionIsCommented { + // Only save non-commented, non-ignored sections + sections[currentSection] = sectionContent + } + } + + // Start new section + if isCommented { + // Remove the # from the section name + sectionName := strings.Trim(trimmed, "[]") + if strings.HasPrefix(sectionName, "#") { + sectionName = strings.TrimSpace(strings.TrimPrefix(sectionName, "#")) + } + currentSection = sectionName + sectionIsCommented = true + } else { + currentSection = strings.Trim(trimmed, "[]") + sectionIsCommented = false + } + currentContent.Reset() + currentContent.WriteString(line) + currentContent.WriteString("\n") + inDefault = (currentSection == "DEFAULT") + } else { + currentContent.WriteString(line) + currentContent.WriteString("\n") + } + } + + // Save final section + if currentSection != "" { + sectionContent := strings.TrimSpace(currentContent.String()) + if inDefault { + defaultContent.WriteString(sectionContent) + } else if !ignoredSections[currentSection] && !sectionIsCommented { + // Only save if it's not an ignored section and not commented + sections[currentSection] = sectionContent + } + } + + return sections, defaultContent.String(), scanner.Err() +} + +// MigrateJailsFromJailLocal migrates non-commented jail sections from jail.local to jail.d/*.local files. +// This should be called when a server is added or enabled to migrate legacy jails. +func MigrateJailsFromJailLocal() error { + localPath := "/etc/fail2ban/jail.local" + jailDPath := "/etc/fail2ban/jail.d" + + // Check if jail.local exists + if _, err := os.Stat(localPath); os.IsNotExist(err) { + return nil // Nothing to migrate + } + + // Read jail.local content + content, err := os.ReadFile(localPath) + if err != nil { + return fmt.Errorf("failed to read jail.local: %w", err) + } + + // Parse content to extract non-commented sections + sections, defaultContent, err := parseJailSectionsUncommented(string(content)) + if err != nil { + return fmt.Errorf("failed to parse jail.local: %w", err) + } + + // If no non-commented, non-DEFAULT jails found, nothing to migrate + if len(sections) == 0 { + config.DebugLog("No jails to migrate from jail.local") + return nil + } + + // Create backup of jail.local + backupPath := localPath + ".backup." + fmt.Sprintf("%d", time.Now().Unix()) + if err := os.WriteFile(backupPath, content, 0644); err != nil { + return fmt.Errorf("failed to create backup: %w", err) + } + config.DebugLog("Created backup of jail.local at %s", backupPath) + + // Ensure jail.d directory exists + if err := os.MkdirAll(jailDPath, 0755); err != nil { + return fmt.Errorf("failed to create jail.d directory: %w", err) + } + + // Write each jail to its own .local file in jail.d/ + migratedCount := 0 + for jailName, jailContent := range sections { + // Skip empty jail names + if jailName == "" { + continue + } + + jailFilePath := filepath.Join(jailDPath, jailName+".local") + + // Check if .local file already exists + if _, err := os.Stat(jailFilePath); err == nil { + // File already exists - skip migration for this jail + config.DebugLog("Skipping migration for jail %s: .local file already exists", jailName) + continue + } + + // Ensure enabled = false is set by default for migrated jails + // Check if enabled is already set in the content + enabledSet := strings.Contains(jailContent, "enabled") || strings.Contains(jailContent, "Enabled") + if !enabledSet { + // Add enabled = false at the beginning of the jail section + // Find the first line after [jailName] + lines := strings.Split(jailContent, "\n") + modifiedContent := "" + for i, line := range lines { + modifiedContent += line + "\n" + // After the section header, add enabled = false + if i == 0 && strings.HasPrefix(strings.TrimSpace(line), "[") && strings.HasSuffix(strings.TrimSpace(line), "]") { + modifiedContent += "enabled = false\n" + } + } + jailContent = modifiedContent + } else { + // If enabled is set, ensure it's false by replacing any enabled = true + jailContent = regexp.MustCompile(`(?m)^\s*enabled\s*=\s*true\s*$`).ReplaceAllString(jailContent, "enabled = false") + } + + // Write jail content to .local file + if err := os.WriteFile(jailFilePath, []byte(jailContent), 0644); err != nil { + return fmt.Errorf("failed to write jail file %s: %w", jailFilePath, err) + } + config.DebugLog("Migrated jail %s to %s (enabled = false)", jailName, jailFilePath) + migratedCount++ + } + + // Only rewrite jail.local if we actually migrated something + if migratedCount > 0 { + // Rewrite jail.local with only DEFAULT section and commented jails + // We need to preserve commented sections, so we'll reconstruct the file + newLocalContent := defaultContent + + // Add back commented sections that weren't migrated + scanner := bufio.NewScanner(strings.NewReader(string(content))) + var inCommentedJail bool + var commentedJailContent strings.Builder + var commentedJailName string + for scanner.Scan() { + line := scanner.Text() + trimmed := strings.TrimSpace(line) + + if strings.HasPrefix(trimmed, "[") && strings.HasSuffix(trimmed, "]") { + // Check if this is a commented section + originalLine := strings.TrimSpace(line) + if strings.HasPrefix(originalLine, "#[") { + // Save previous commented jail if any + if inCommentedJail && commentedJailName != "" { + newLocalContent += commentedJailContent.String() + } + inCommentedJail = true + commentedJailContent.Reset() + commentedJailName = strings.Trim(trimmed, "[]") + if strings.HasPrefix(commentedJailName, "#") { + commentedJailName = strings.TrimSpace(strings.TrimPrefix(commentedJailName, "#")) + } + commentedJailContent.WriteString(line) + commentedJailContent.WriteString("\n") + } else { + // Non-commented section - save previous commented jail if any + if inCommentedJail && commentedJailName != "" { + newLocalContent += commentedJailContent.String() + inCommentedJail = false + commentedJailContent.Reset() + } + } + } else if inCommentedJail { + commentedJailContent.WriteString(line) + commentedJailContent.WriteString("\n") + } + } + // Save final commented jail if any + if inCommentedJail && commentedJailName != "" { + newLocalContent += commentedJailContent.String() + } + + if !strings.HasSuffix(newLocalContent, "\n") { + newLocalContent += "\n" + } + if err := os.WriteFile(localPath, []byte(newLocalContent), 0644); err != nil { + return fmt.Errorf("failed to rewrite jail.local: %w", err) + } + config.DebugLog("Migration completed: moved %d jails to jail.d/", migratedCount) + } + + return nil +} + // parseJailConfigFileOnlyDefault parses only the DEFAULT section from a jail config file. func parseJailConfigFileOnlyDefault(path string) ([]JailInfo, error) { var jails []JailInfo diff --git a/internal/fail2ban/manager.go b/internal/fail2ban/manager.go index 1e15f67..153d297 100644 --- a/internal/fail2ban/manager.go +++ b/internal/fail2ban/manager.go @@ -169,6 +169,14 @@ func updateConnectorAction(ctx context.Context, conn Connector) error { func newConnectorForServer(server config.Fail2banServer) (Connector, error) { switch server.Type { case "local": + // IMPORTANT: Run migration FIRST before ensuring structure + // This ensures any legacy jails in jail.local are migrated to jail.d/*.local + // before ensureJailLocalStructure() overwrites jail.local + if err := MigrateJailsFromJailLocal(); err != nil { + config.DebugLog("Warning: migration check failed (may be normal if no jails to migrate): %v", err) + // Don't fail - continue with ensuring structure + } + if err := config.EnsureLocalFail2banAction(server); err != nil { fmt.Printf("warning: failed to ensure local fail2ban action: %v\n", err) } diff --git a/pkg/web/handlers.go b/pkg/web/handlers.go index 72fe67f..b6db0a6 100644 --- a/pkg/web/handlers.go +++ b/pkg/web/handlers.go @@ -1041,6 +1041,51 @@ func getJailNames(jails map[string]bool) []string { return names } +func contains(slice []string, item string) bool { + for _, s := range slice { + if s == item { + return true + } + } + return false +} + +// parseJailErrorsFromReloadOutput extracts jail names that have errors from reload output. +// Looks for patterns like "Errors in jail 'jailname'. Skipping..." or "Unable to read the filter 'filtername'" +func parseJailErrorsFromReloadOutput(output string) []string { + var problematicJails []string + lines := strings.Split(output, "\n") + + for _, line := range lines { + // Look for "Errors in jail 'jailname'. Skipping..." + if strings.Contains(line, "Errors in jail") && strings.Contains(line, "Skipping") { + // Extract jail name between single quotes + re := regexp.MustCompile(`Errors in jail '([^']+)'`) + matches := re.FindStringSubmatch(line) + if len(matches) > 1 { + problematicJails = append(problematicJails, matches[1]) + } + } + // Also check for filter errors that might indicate jail problems + // "Unable to read the filter 'filtername'" - this might be referenced by a jail + // Note: Filter errors are often associated with jails, but we primarily track + // jail errors directly via "Errors in jail" messages above + _ = strings.Contains(line, "Unable to read the filter") // Track for future enhancement + } + + // Remove duplicates + seen := make(map[string]bool) + uniqueJails := []string{} + for _, jail := range problematicJails { + if !seen[jail] { + seen[jail] = true + uniqueJails = append(uniqueJails, jail) + } + } + + return uniqueJails +} + // After updating, fail2ban is reloaded to apply the changes. func UpdateJailManagementHandler(c *gin.Context) { config.DebugLog("----------------------------") @@ -1081,9 +1126,75 @@ func UpdateJailManagementHandler(c *gin.Context) { config.DebugLog("Successfully updated jail enabled states") // Reload fail2ban to apply the changes (reload is sufficient for jail enable/disable) - if err := conn.Reload(c.Request.Context()); err != nil { - config.DebugLog("Error: failed to reload fail2ban after updating jail settings: %v", err) - errMsg := err.Error() + reloadErr := conn.Reload(c.Request.Context()) + + // Check for errors in reload output even if reload "succeeded" + var problematicJails []string + var detailedErrorOutput string + if reloadErr != nil { + errMsg := reloadErr.Error() + config.DebugLog("Error: failed to reload fail2ban after updating jail settings: %v", reloadErr) + + // Extract output from error message (format: "fail2ban reload completed but with errors (output: ...)") + if strings.Contains(errMsg, "(output:") { + // Extract the output part + outputStart := strings.Index(errMsg, "(output:") + 8 + outputEnd := strings.LastIndex(errMsg, ")") + if outputEnd > outputStart { + detailedErrorOutput = errMsg[outputStart:outputEnd] + problematicJails = parseJailErrorsFromReloadOutput(detailedErrorOutput) + } + } else if strings.Contains(errMsg, "output:") { + // Alternative format: "fail2ban reload error: ... (output: ...)" + outputStart := strings.Index(errMsg, "output:") + 7 + if outputStart < len(errMsg) { + detailedErrorOutput = strings.TrimSpace(errMsg[outputStart:]) + problematicJails = parseJailErrorsFromReloadOutput(detailedErrorOutput) + } + } + + // If we found problematic jails, disable them + if len(problematicJails) > 0 { + config.DebugLog("Found %d problematic jail(s) in reload output: %v", len(problematicJails), problematicJails) + + // Create disable update for problematic jails + disableUpdate := make(map[string]bool) + for _, jailName := range problematicJails { + disableUpdate[jailName] = false + } + + // Also disable any jails that were enabled in this request if they're in the problematic list + for jailName := range enabledJails { + if contains(problematicJails, jailName) { + disableUpdate[jailName] = false + } + } + + if len(disableUpdate) > 0 { + if disableErr := conn.UpdateJailEnabledStates(c.Request.Context(), disableUpdate); disableErr != nil { + config.DebugLog("Error disabling problematic jails: %v", disableErr) + } else { + // Reload again after disabling + if reloadErr2 := conn.Reload(c.Request.Context()); reloadErr2 != nil { + config.DebugLog("Error: failed to reload fail2ban after disabling problematic jails: %v", reloadErr2) + } + } + } + + // Update enabledJails to include problematic jails for response + for _, jailName := range problematicJails { + enabledJails[jailName] = true + } + } + + // Update errMsg with detailed error output when debug mode is enabled + settings := config.GetSettings() + if settings.Debug && detailedErrorOutput != "" { + errMsg = fmt.Sprintf("%s\n\nDetailed error output:\n%s", errMsg, detailedErrorOutput) + } else if detailedErrorOutput != "" { + // Even without debug mode, include basic error info + errMsg = fmt.Sprintf("%s (check debug mode for details)", errMsg) + } // If any jails were enabled in this request and reload failed, disable them all if len(enabledJails) > 0 { @@ -1106,7 +1217,7 @@ func UpdateJailManagementHandler(c *gin.Context) { } // Reload again after disabling - if reloadErr := conn.Reload(c.Request.Context()); reloadErr != nil { + if reloadErr = conn.Reload(c.Request.Context()); reloadErr != nil { config.DebugLog("Error: failed to reload fail2ban after disabling jails: %v", reloadErr) c.JSON(http.StatusOK, gin.H{ "error": fmt.Sprintf("Failed to reload fail2ban after disabling jails: %v", reloadErr),