From bd45344d191236bd03f58927093baa3a74e3ba19 Mon Sep 17 00:00:00 2001 From: toto Date: Wed, 4 Mar 2026 15:55:00 +0100 Subject: [PATCH] v1.1.11: Fix exclude_source_ips config loading and debug logging Major fixes: - Add exclude_source_ips to mergeConfigs() - config file values now properly loaded - Add validation for exclude_source_ips (IP/CIDR format validation) - Remove JA4SENTINEL_LOG_LEVEL env var from systemd service - Config file log_level now respected without env override Debug logging improvements: - Log IP filter entries at startup (debug mode) - Track filtered packet count with atomic counter - Display filter statistics at shutdown via GetFilterStats() - New debug logs in tlsparse component Testing: - Add 6 new unit tests for exclude_source_ips and log_level config loading - Test mergeConfigs() behavior with empty/override values - Test validation of invalid IPs and CIDR ranges Documentation: - Update architecture.yml with ipfilter module - Document config loading priority and notes - Update api.Config fields (LocalIPs, ExcludeSourceIPs, LogLevel) Files changed: - internal/config/loader.go (merge, validation, helpers) - internal/config/loader_test.go (6 new tests) - internal/tlsparse/parser.go (GetFilterStats, counter) - cmd/ja4sentinel/main.go (debug logging) - packaging/systemd/ja4sentinel.service (remove env var) - architecture.yml (ipfilter module, config_loading section) Co-authored-by: Qwen-Coder --- architecture.yml | 39 +++- cmd/ja4sentinel/main.go | 25 ++- internal/config/loader.go | 70 +++++++ internal/config/loader_test.go | 259 ++++++++++++++++++++++++++ internal/tlsparse/parser.go | 13 ++ packaging/rpm/ja4sentinel.spec | 26 ++- packaging/systemd/ja4sentinel.service | 1 - 7 files changed, 426 insertions(+), 7 deletions(-) diff --git a/architecture.yml b/architecture.yml index 5ba31d3..ffd5834 100644 --- a/architecture.yml +++ b/architecture.yml @@ -57,14 +57,17 @@ modules: path: "internal/tlsparse" description: "Extraction des ClientHello TLS côté client à partir des paquets capturés." responsibilities: - - "Décoder les couches IP/TCP jusqu’au payload TLS." + - "Décoder les couches IP/TCP jusqu'au payload TLS." - "Identifier le ClientHello TLS du client sur les ports configurés." - "Assembler les segments si nécessaire pour obtenir un ClientHello complet." - "Produire des TLSClientHello enrichis avec IPMeta et TCPMeta." + - "Filtrer les IPs source exclues via le module ipfilter (avant parsing TLS)." + - "Compter les paquets filtrés pour statistiques (GetFilterStats)." allowed_dependencies: - "config" - "capture" - "api" + - "ipfilter" forbidden_dependencies: - "output" @@ -117,6 +120,22 @@ modules: - "fingerprint" - "output" + - name: ipfilter + path: "internal/ipfilter" + description: "Filtrage des adresses IP source par correspondance IP/CIDR." + responsibilities: + - "Charger une liste d'IPs ou plages CIDR à exclure." + - "Vérifier si une IP source correspond à une entrée de la liste d'exclusion." + - "Supporter IPv4 et IPv6." + - "Validation des formats IP et CIDR lors du chargement de la config." + allowed_dependencies: [] + forbidden_dependencies: + - "config" + - "capture" + - "tlsparse" + - "fingerprint" + - "output" + - name: cmd_ja4sentinel path: "cmd/ja4sentinel" description: "Point d'entrée de l'application (main)." @@ -126,6 +145,7 @@ modules: - "Brancher les modules entre eux selon l'architecture pipeline." - "Gérer les signaux système (arrêt propre)." - "Gérer le signal SIGHUP pour la rotation des logs (systemctl reload)." + - "Logger les statistiques du filtre IP au démarrage et à l'arrêt (debug)." allowed_dependencies: - "config" - "capture" @@ -155,9 +175,11 @@ api: - { name: Interface, type: "string", description: "Nom de l'interface réseau (ex: eth0)." } - { name: ListenPorts, type: "[]uint16", description: "Ports TCP à surveiller (ex: [443, 8443])." } - { name: BPFFilter, type: "string", description: "Filtre BPF optionnel pour la capture." } + - { name: LocalIPs, type: "[]string", description: "IPs locales à surveiller (vide = auto-détection, exclut loopback)." } + - { name: ExcludeSourceIPs,type: "[]string", description: "IPs sources ou plages CIDR à exclure (ex: [\"10.0.0.0/8\", \"192.168.1.1\"]). Validé par le module config." } - { name: FlowTimeoutSec, type: "int", description: "Timeout en secondes pour l'extraction du handshake TLS (défaut: 30)." } - { name: PacketBufferSize,type: "int", description: "Taille du buffer du canal de paquets (défaut: 1000). Pour les environnements à fort trafic." } - - { name: LogLevel, type: "string", description: "Niveau de log : debug, info, warn, error (défaut: info). Extension pour configuration runtime." } + - { name: LogLevel, type: "string", description: "Niveau de log : debug, info, warn, error (défaut: info). Configuration via fichier YAML uniquement (pas d'override env dans systemd)." } - name: "api.IPMeta" description: "Métadonnées IP pour fingerprinting de stack." @@ -478,7 +500,7 @@ packaging: note: "Fourni par le RPM, configure la rotation quotidienne avec compression." - path: "/etc/systemd/system/ja4sentinel.service" description: "Unité systemd pour la gestion du service." - note: "Doit inclure Type=notify et ExecReload=/bin/kill -HUP $MAINPID pour supporter systemctl reload." + note: "Doit inclure Type=notify et ExecReload=/bin/kill -HUP $MAINPID pour supporter systemctl reload. PAS de variable Environment=JA4SENTINEL_LOG_LEVEL pour respecter la config fichier." logrotate: description: "Configuration logrotate pour la rotation des logs." behavior: @@ -489,3 +511,14 @@ packaging: - "systemctl reload ja4sentinel déclenche le handler SIGHUP." - "Le service réouvre ses fichiers de log sans redémarrage complet." +config_loading: + priority: + - "1. Fichier de configuration YAML (config.yml)" + - "2. Variables d'environnement JA4SENTINEL_* (sauf log_level depuis v1.1.11)" + - "3. Arguments CLI (--config)" + notes: + - "Depuis v1.1.11, la variable JA4SENTINEL_LOG_LEVEL n'est plus définie dans le service systemd." + - "Le log_level doit être configuré exclusivement dans le fichier YAML." + - "exclude_source_ips est uniquement chargé depuis le fichier YAML (pas d'override env)." + - "La fusion des configs utilise mergeConfigs() qui préserve les valeurs non-overridées." + diff --git a/cmd/ja4sentinel/main.go b/cmd/ja4sentinel/main.go index 3e6d895..8002143 100644 --- a/cmd/ja4sentinel/main.go +++ b/cmd/ja4sentinel/main.go @@ -23,7 +23,7 @@ import ( var ( // Version information (set via ldflags) - Version = "1.1.9" + Version = "1.1.11" BuildTime = "unknown" GitCommit = "unknown" ) @@ -120,12 +120,25 @@ func main() { ) fingerprintEngine := fingerprint.NewEngine() - // Log exclusion configuration + // Log exclusion configuration with debug details if len(appConfig.Core.ExcludeSourceIPs) > 0 { appLogger.Info("main", "Source IP exclusion enabled", map[string]string{ "exclude_count": fmt.Sprintf("%d", len(appConfig.Core.ExcludeSourceIPs)), "exclude_ips": strings.Join(appConfig.Core.ExcludeSourceIPs, ", "), }) + appLogger.Debug("tlsparse", "IP filter configured", map[string]string{ + "filter_entries": strings.Join(appConfig.Core.ExcludeSourceIPs, ", "), + }) + } else { + appLogger.Debug("tlsparse", "IP filter disabled (no exclusions configured)", nil) + } + + // Log filter stats at startup (debug mode) + filteredCount, hasFilter := parser.GetFilterStats() + if hasFilter { + appLogger.Debug("tlsparse", "IP filter initialized", map[string]string{ + "filtered_packets": fmt.Sprintf("%d", filteredCount), + }) } // Create output builder with error callback for socket connection errors @@ -291,6 +304,14 @@ shutdown: }) } + // Log final filter stats + filteredCount, hasFilter = parser.GetFilterStats() + if hasFilter { + appLogger.Info("tlsparse", "IP filter statistics", map[string]string{ + "total_filtered_packets": fmt.Sprintf("%d", filteredCount), + }) + } + if mw, ok := outputWriter.(interface{ CloseAll() error }); ok { if err := mw.CloseAll(); err != nil { appLogger.Error("main", "Failed to close output writers", map[string]string{ diff --git a/internal/config/loader.go b/internal/config/loader.go index 40845bd..065622b 100644 --- a/internal/config/loader.go +++ b/internal/config/loader.go @@ -175,6 +175,11 @@ func mergeConfigs(base, override api.AppConfig) api.AppConfig { result.Core.LogLevel = override.Core.LogLevel } + // Merge exclude_source_ips (override takes precedence) + if len(override.Core.ExcludeSourceIPs) > 0 { + result.Core.ExcludeSourceIPs = override.Core.ExcludeSourceIPs + } + if len(override.Outputs) > 0 { result.Outputs = override.Outputs } @@ -218,6 +223,27 @@ func (l *LoaderImpl) validate(config api.AppConfig) error { } } + // Validate exclude_source_ips (if provided) + if len(config.Core.ExcludeSourceIPs) > 0 { + for i, ip := range config.Core.ExcludeSourceIPs { + if ip == "" { + return fmt.Errorf("exclude_source_ips[%d]: entry cannot be empty", i) + } + // Basic validation: check if it looks like an IP or CIDR + if !strings.Contains(ip, "/") { + // Single IP - basic check + if !isValidIP(ip) { + return fmt.Errorf("exclude_source_ips[%d]: invalid IP address %q", i, ip) + } + } else { + // CIDR - basic check + if !isValidCIDR(ip) { + return fmt.Errorf("exclude_source_ips[%d]: invalid CIDR %q", i, ip) + } + } + } + } + allowedTypes := map[string]struct{}{ "stdout": {}, "file": {}, @@ -257,3 +283,47 @@ func ToJSON(config api.AppConfig) string { } return string(data) } + +// isValidIP checks if a string is a valid IP address +func isValidIP(ip string) bool { + if ip == "" { + return false + } + // Simple validation: check if it contains only valid IP characters + for _, ch := range ip { + if !((ch >= '0' && ch <= '9') || ch == '.') { + // Could be IPv6 + if ch == ':' { + return true // Accept IPv6 without detailed validation + } + return false + } + } + return true +} + +// isValidCIDR checks if a string is a valid CIDR notation +func isValidCIDR(cidr string) bool { + if cidr == "" { + return false + } + parts := strings.Split(cidr, "/") + if len(parts) != 2 { + return false + } + // Check IP part + if !isValidIP(parts[0]) { + return false + } + // Check prefix length + prefix, err := strconv.Atoi(parts[1]) + if err != nil { + return false + } + if strings.Contains(parts[0], ":") { + // IPv6 + return prefix >= 0 && prefix <= 128 + } + // IPv4 + return prefix >= 0 && prefix <= 32 +} diff --git a/internal/config/loader_test.go b/internal/config/loader_test.go index 8ca4ad8..901f3b8 100644 --- a/internal/config/loader_test.go +++ b/internal/config/loader_test.go @@ -741,3 +741,262 @@ func TestMergeConfigs_OutputMerge(t *testing.T) { t.Errorf("Merged Outputs[0].Type = %q, want 'file'", result.Outputs[0].Type) } } + +// TestMergeConfigs_ExcludeSourceIPs tests that exclude_source_ips is properly merged +func TestMergeConfigs_ExcludeSourceIPs(t *testing.T) { + base := api.AppConfig{ + Core: api.Config{ + Interface: "eth0", + ListenPorts: []uint16{443}, + ExcludeSourceIPs: []string{}, // Empty base + }, + } + override := api.AppConfig{ + Core: api.Config{ + ExcludeSourceIPs: []string{"10.0.0.0/8", "192.168.1.1"}, + }, + } + + result := mergeConfigs(base, override) + + if len(result.Core.ExcludeSourceIPs) != 2 { + t.Errorf("Merged ExcludeSourceIPs length = %d, want 2", len(result.Core.ExcludeSourceIPs)) + } + if result.Core.ExcludeSourceIPs[0] != "10.0.0.0/8" { + t.Errorf("Merged ExcludeSourceIPs[0] = %q, want '10.0.0.0/8'", result.Core.ExcludeSourceIPs[0]) + } + if result.Core.ExcludeSourceIPs[1] != "192.168.1.1" { + t.Errorf("Merged ExcludeSourceIPs[1] = %q, want '192.168.1.1'", result.Core.ExcludeSourceIPs[1]) + } +} + +// TestMergeConfigs_ExcludeSourceIPs_EmptyOverride tests that empty override doesn't replace +func TestMergeConfigs_ExcludeSourceIPs_EmptyOverride(t *testing.T) { + base := api.AppConfig{ + Core: api.Config{ + Interface: "eth0", + ListenPorts: []uint16{443}, + ExcludeSourceIPs: []string{"10.0.0.0/8"}, + }, + } + override := api.AppConfig{ + Core: api.Config{ + ExcludeSourceIPs: []string{}, // Empty override + }, + } + + result := mergeConfigs(base, override) + + // Empty override should NOT replace base + if len(result.Core.ExcludeSourceIPs) != 1 { + t.Errorf("Merged ExcludeSourceIPs length = %d, want 1", len(result.Core.ExcludeSourceIPs)) + } + if result.Core.ExcludeSourceIPs[0] != "10.0.0.0/8" { + t.Errorf("Merged ExcludeSourceIPs[0] = %q, want '10.0.0.0/8'", result.Core.ExcludeSourceIPs[0]) + } +} + +// TestLoadFromFile_ExcludeSourceIPs tests loading exclude_source_ips from YAML file +func TestLoadFromFile_ExcludeSourceIPs(t *testing.T) { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yml") + + yamlContent := ` +core: + interface: eth0 + listen_ports: + - 443 + exclude_source_ips: + - "10.0.0.0/8" + - "192.168.1.100" + log_level: debug +outputs: + - type: stdout + enabled: true +` + if err := os.WriteFile(configPath, []byte(yamlContent), 0600); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + loader := NewLoader(configPath) + cfg, err := loader.Load() + + if err != nil { + t.Fatalf("Load() error = %v", err) + } + + if len(cfg.Core.ExcludeSourceIPs) != 2 { + t.Errorf("ExcludeSourceIPs length = %d, want 2", len(cfg.Core.ExcludeSourceIPs)) + } + if cfg.Core.ExcludeSourceIPs[0] != "10.0.0.0/8" { + t.Errorf("ExcludeSourceIPs[0] = %q, want '10.0.0.0/8'", cfg.Core.ExcludeSourceIPs[0]) + } + if cfg.Core.ExcludeSourceIPs[1] != "192.168.1.100" { + t.Errorf("ExcludeSourceIPs[1] = %q, want '192.168.1.100'", cfg.Core.ExcludeSourceIPs[1]) + } + if cfg.Core.LogLevel != "debug" { + t.Errorf("LogLevel = %q, want 'debug'", cfg.Core.LogLevel) + } +} + +// TestLoadFromFile_LogLevel tests loading log_level from YAML file +func TestLoadFromFile_LogLevel(t *testing.T) { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yml") + + tests := []struct { + name string + logLevel string + want string + wantErr bool + }{ + {"debug level", "debug", "debug", false}, + {"info level", "info", "info", false}, + {"warn level", "warn", "warn", false}, + {"error level", "error", "error", false}, + {"invalid level", "invalid", "", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + yamlContent := ` +core: + interface: eth0 + listen_ports: + - 443 + log_level: ` + tt.logLevel + ` +outputs: + - type: stdout + enabled: true +` + if err := os.WriteFile(configPath, []byte(yamlContent), 0600); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + loader := NewLoader(configPath) + cfg, err := loader.Load() + + if (err != nil) != tt.wantErr { + t.Fatalf("Load() error = %v, wantErr %v", err, tt.wantErr) + } + + if !tt.wantErr && cfg.Core.LogLevel != tt.want { + t.Errorf("LogLevel = %q, want %q", cfg.Core.LogLevel, tt.want) + } + }) + } +} + +// TestValidate_ExcludeSourceIPs tests validation of exclude_source_ips entries +func TestValidate_ExcludeSourceIPs(t *testing.T) { + loader := &LoaderImpl{} + + tests := []struct { + name string + ips []string + wantErr bool + errContains string + }{ + { + name: "empty list", + ips: []string{}, + wantErr: false, + }, + { + name: "valid single IP", + ips: []string{"192.168.1.1"}, + wantErr: false, + }, + { + name: "valid CIDR", + ips: []string{"10.0.0.0/8"}, + wantErr: false, + }, + { + name: "multiple valid entries", + ips: []string{"10.0.0.0/8", "192.168.1.1", "172.16.0.0/12"}, + wantErr: false, + }, + { + name: "empty entry", + ips: []string{""}, + wantErr: true, + errContains: "entry cannot be empty", + }, + { + name: "invalid IP", + ips: []string{"999.999.999.999"}, + wantErr: true, + errContains: "invalid IP address", + }, + { + name: "invalid CIDR format", + ips: []string{"10.0.0.0/33"}, + wantErr: true, + errContains: "invalid CIDR", + }, + { + name: "invalid CIDR syntax", + ips: []string{"10.0.0.0/"}, + wantErr: true, + errContains: "invalid CIDR", + }, + { + name: "mixed valid and invalid", + ips: []string{"10.0.0.0/8", "invalid"}, + wantErr: true, + errContains: "invalid", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := api.AppConfig{ + Core: api.Config{ + Interface: "eth0", + ListenPorts: []uint16{443}, + ExcludeSourceIPs: tt.ips, + }, + Outputs: []api.OutputConfig{ + {Type: "stdout", Enabled: true}, + }, + } + err := loader.validate(cfg) + + if (err != nil) != tt.wantErr { + t.Fatalf("validate() error = %v, wantErr %v", err, tt.wantErr) + } + + if tt.wantErr && tt.errContains != "" { + if err == nil || !strings.Contains(err.Error(), tt.errContains) { + t.Errorf("validate() error = %v, should contain %q", err, tt.errContains) + } + } + }) + } +} + +// TestLoadFromEnv_ExcludeSourceIPs tests that exclude_source_ips can be set via env (future feature) +// Currently exclude_source_ips is NOT loaded from env, only from config file +func TestLoadFromEnv_ExcludeSourceIPs_NotSupported(t *testing.T) { + // This test documents that exclude_source_ips is NOT loaded from env + // It's only loaded from config file + t.Setenv("JA4SENTINEL_LOG_LEVEL", "debug") + + loader := NewLoader("") + cfg, err := loader.Load() + + if err != nil { + t.Fatalf("Load() error = %v", err) + } + + // exclude_source_ips should be empty (not loaded from env) + if len(cfg.Core.ExcludeSourceIPs) != 0 { + t.Errorf("ExcludeSourceIPs should be empty from env, got %v", cfg.Core.ExcludeSourceIPs) + } + + // But log_level should be loaded from env + if cfg.Core.LogLevel != "debug" { + t.Errorf("LogLevel = %q, want 'debug'", cfg.Core.LogLevel) + } +} diff --git a/internal/tlsparse/parser.go b/internal/tlsparse/parser.go index 2726568..8fb7f61 100644 --- a/internal/tlsparse/parser.go +++ b/internal/tlsparse/parser.go @@ -6,6 +6,7 @@ import ( "fmt" "strings" "sync" + "sync/atomic" "time" "ja4sentinel/api" @@ -65,6 +66,7 @@ type ParserImpl struct { maxTrackedFlows int maxHelloBufferBytes int sourceIPFilter *ipfilter.Filter + filteredCount uint64 // Counter for filtered packets (debug) } // NewParser creates a new TLS parser with connection state tracking @@ -98,9 +100,11 @@ func NewParserWithTimeoutAndFilter(timeout time.Duration, excludeSourceIPs []str flowTimeout: timeout, cleanupDone: make(chan struct{}), cleanupClose: make(chan struct{}), + closeOnce: sync.Once{}, maxTrackedFlows: DefaultMaxTrackedFlows, maxHelloBufferBytes: DefaultMaxHelloBufferBytes, sourceIPFilter: filter, + filteredCount: 0, } go p.cleanupLoop() return p @@ -243,6 +247,7 @@ func (p *ParserImpl) Process(pkt api.RawPacket) (*api.TLSClientHello, error) { // Check if source IP should be excluded if p.sourceIPFilter != nil && p.sourceIPFilter.ShouldExclude(srcIP) { + atomic.AddUint64(&p.filteredCount, 1) return nil, nil // Source IP is excluded } @@ -406,6 +411,14 @@ func (p *ParserImpl) getOrCreateFlow(key string, srcIP string, srcPort uint16, d return flow } +// GetFilterStats returns statistics about the IP filter (for debug/monitoring) +func (p *ParserImpl) GetFilterStats() (filteredCount uint64, hasFilter bool) { + if p.sourceIPFilter == nil { + return 0, false + } + return atomic.LoadUint64(&p.filteredCount), true +} + // Close cleans up the parser and stops background goroutines func (p *ParserImpl) Close() error { p.closeOnce.Do(func() { diff --git a/packaging/rpm/ja4sentinel.spec b/packaging/rpm/ja4sentinel.spec index a72ba83..91adf8a 100644 --- a/packaging/rpm/ja4sentinel.spec +++ b/packaging/rpm/ja4sentinel.spec @@ -3,7 +3,7 @@ %if %{defined build_version} %define spec_version %{build_version} %else -%define spec_version 1.1.9 +%define spec_version 1.1.11 %endif Name: ja4sentinel @@ -123,6 +123,30 @@ fi %changelog +* Wed Mar 04 2026 Jacquin Antoine - 1.1.11-1 +- FIX: Remove JA4SENTINEL_LOG_LEVEL environment variable from systemd service +- Config file log_level now respected (no env var override) +- FIX: Add exclude_source_ips to config merge function (mergeConfigs) +- FIX: Add validation for exclude_source_ips entries (IP/CIDR) +- New isValidIP() and isValidCIDR() helper functions +- Config file exclude_source_ips now properly loaded and validated +- DEBUG: Add IP filter debug logging for troubleshooting +- Log filter entries at startup in debug mode +- Track filtered packet count with atomic counter +- Display filter statistics at shutdown +- New GetFilterStats() method on parser for monitoring +- Added unit tests for exclude_source_ips and log_level config loading + +* Wed Mar 04 2026 Jacquin Antoine - 1.1.10-1 +- DEBUG: Add IP filter debug logging for troubleshooting +- Log filter entries at startup in debug mode +- Track filtered packet count with atomic counter +- Display filter statistics at shutdown +- New GetFilterStats() method on parser for monitoring +- FIX: Add exclude_source_ips to config merge function +- FIX: Add validation for exclude_source_ips entries (IP/CIDR) +- Helps diagnose exclude_source_ips filtering issues + * Wed Mar 04 2026 Jacquin Antoine - 1.1.9-1 - FEATURE: Add source IP exclusion with CIDR support - New exclude_source_ips configuration option diff --git a/packaging/systemd/ja4sentinel.service b/packaging/systemd/ja4sentinel.service index dc1a748..f401ad6 100644 --- a/packaging/systemd/ja4sentinel.service +++ b/packaging/systemd/ja4sentinel.service @@ -16,7 +16,6 @@ RestartSec=5 WatchdogSec=30 TimeoutStopSec=2 NotifyAccess=main -Environment=JA4SENTINEL_LOG_LEVEL=info # Security hardening (compatible with root for packet capture) ProtectSystem=strict