diff --git a/.env.example b/.env.example index 696afb8..0cc1716 100644 --- a/.env.example +++ b/.env.example @@ -63,19 +63,52 @@ INGEST_PORT=5001 AGENT_SHARED_SECRET= # How long a one-time enrolment token stays valid. # ENROLLMENT_TOKEN_TTL_HOURS=24 -# Port the server calls back on for agent-inbound requests (config push, -# SOAR execute, screen stream). -# AGENT_PORT=5000 +# AGENT_PORT is gone, along with the listener it named. The server no longer +# dials endpoints: config push, SOAR execute and the screen stream all travel +# back down the channel the agent opens (/agent-link), so there is no port to +# open on a monitored host and no address for the server to guess. -# ─── TLS (optional) ────────────────────────────────────────────────────── +# ─── TLS ───────────────────────────────────────────────────────────────── # Off by default: the stack is expected to sit behind a reverse proxy that -# terminates TLS. Set TLS_ENABLED=1 to have Sanic serve HTTPS directly. -# Remember SESSION_COOKIE_SECURE=1 once TLS is in play, wherever it -# terminates. +# terminates TLS. TLS_ENABLED=1 makes this process serve HTTPS itself, on +# 8000 for the console and 5011 for telemetry. +# +# These three were documented here, and in docs/production-deployment.md, and +# in the closing output of certs/generate_certs.py - and read by nothing. +# Setting TLS_ENABLED=1 produced a console that came up cleanly on plain HTTP +# while its operator had every reason to believe it was HTTPS. They are wired +# up now, and a certificate that cannot be loaded stops the server instead of +# quietly downgrading it. # TLS_ENABLED=0 +# +# Leave TLS_CERT/TLS_KEY unset with TLS_ENABLED=1 and a self-signed +# certificate is generated on first boot, one per deployment, in the +# sentora_certs volume. Browsers will warn - nothing trusts that CA - and +# agents need certs/rootCA.crt to verify the server. Use a real certificate +# for anything reachable beyond your own network. # TLS_CERT=certs/server.crt # TLS_KEY=certs/server.key +# TLS_CN=soc.example.com + +# Telemetry transport. 5001 is the original plaintext listener; 5011 carries +# the same protocol inside TLS, and an agent chooses between them from its +# server_url scheme, so there is no second setting on the endpoint to get +# wrong. +# +# Both listen by default, because cutting to TLS before the fleet is rebuilt +# makes telemetry stop with no symptom on either side. server.py names each +# agent still arriving in the clear, once. When nothing is named any more, set +# INGEST_TLS_REQUIRED=1: the plaintext port is not opened at all, and an +# un-migrated agent is refused rather than silently accepted. +# INGEST_TLS_PORT=5011 +# INGEST_TLS_REQUIRED=0 + +# SESSION_COOKIE_SECURE follows TLS_ENABLED when it is not set. It used to +# default to 0 regardless, so turning on TLS and leaving this alone gave an +# HTTPS console whose session cookie was still allowed onto plain HTTP. Set it +# explicitly to 1 when a proxy in front of this process terminates TLS - that +# is invisible from in here. # ─── Sessions ──────────────────────────────────────────────────────────── diff --git a/Sentora/conf/file_scan.yaml b/Sentora/conf/file_scan.yaml index a245fe6..31ec762 100644 --- a/Sentora/conf/file_scan.yaml +++ b/Sentora/conf/file_scan.yaml @@ -6,6 +6,15 @@ target_dirs: windows: - "C:\\Users" +# Directories that churn on every host and mean nothing when they do. +# +# `windows` was empty, and `modules/fim.py` did not read this key at all - so +# the real-time watcher recorded every temp file, browser cache entry and pip +# install under C:\Users as an integrity violation. Thousands an hour, with a +# real change to a real file somewhere among them. +# +# The module now merges these with its own defaults rather than replacing +# them, so adding one directory here does not silently un-exclude the rest. exclude_dirs: linux: - /proc @@ -13,7 +22,11 @@ exclude_dirs: - /dev - /var/lib/docker - /run - windows: [] + windows: + - "*\\AppData\\Local\\Temp\\*" + - "*\\AppData\\Local\\Packages\\*" + - "*\\node_modules\\*" + - "*\\$Recycle.Bin\\*" backup_extensions: - sql.gz diff --git a/Sentora/main.py b/Sentora/main.py index 1420188..faa3b11 100644 --- a/Sentora/main.py +++ b/Sentora/main.py @@ -1,4 +1,5 @@ import socket +import ssl import struct import time import asyncio @@ -320,6 +321,23 @@ def run(self): AGENT_NAME = None SERVER_IP = None SERVER_PORT = 5001 + +#: Telemetry transport. Derived at startup from the enrolment config's +#: `server_url` scheme - see `_resolve_ingest_transport`. An https console +#: means an https-grade ingest channel, because the two carry the same data +#: and there is nothing to be gained from encrypting the smaller half. +INGEST_TLS = False +INGEST_TLS_PORT = 5011 +#: PEM bundle for a self-signed server. Verification is not optional: an +#: unverified TLS connection is encrypted to whoever answered, which against +#: an attacker on the path is what not encrypting it would have achieved. +INGEST_CA = None +#: The name in the server's certificate. Separate from SERVER_IP because the +#: certificate is issued for a hostname and the connection is usually made to +#: an address, and TLS checks the name. +SERVER_HOSTNAME = None +#: An unbounded blocking read is how a collector thread disappears for good. +INGEST_TIMEOUT_S = 30 AUTOMATIONS_API_URL = None AUTOMATIONS_MODE = "auto" AGENT_SHARED_SECRET = None @@ -508,6 +526,72 @@ def send_alert(source, severity, message, metadata=None): print(f"[Alert] Failed to send alert: {e}") +def _ingest_socket(): + """A connected socket to the ingest listener, TLS when the server is https. + + Derived from the enrolment config's `server_url` rather than configured + separately - the same reasoning `modules/link.channel_url` gives for the + control channel. A second setting is a second thing to get wrong, and it + would be wrong in exactly the deployments where the first one was right. + + The certificate is verified. A self-signed server needs its CA here + (`server_ca` in config.json, or SENTORA_CA_CERT); without it the + connection fails, and it should - an unverified TLS connection encrypts + the traffic to whoever answered, which against an attacker on the path is + the same as not encrypting it. + """ + raw = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + raw.settimeout(INGEST_TIMEOUT_S) + try: + raw.connect((SERVER_IP, SERVER_PORT)) + if not INGEST_TLS: + return raw + context = ssl.create_default_context(cafile=INGEST_CA or None) + return context.wrap_socket(raw, server_hostname=SERVER_HOSTNAME or SERVER_IP) + except Exception: + raw.close() + raise + + +def _read_receipt(sock): + """The server's reply, or None if it did not send one. + + None is a real answer and not a failure: a server built before the reply + frame stores the batch and closes, which arrives here as a clean EOF. It + must not be reported as data loss, and it must not be reported as success + either - `record_send` keeps the difference. + """ + try: + header = _recv_exactly(sock, 4) + if header is None: + return None + (length,) = struct.unpack('!I', header) + # A sane bound. Nothing legitimate sends a megabyte of receipt, and + # allocating on a length an attacker chose is how a reply frame turns + # into a denial of service against the agent. + if length == 0 or length > 64 * 1024: + return None + body = _recv_exactly(sock, length) + if body is None: + return None + return json.loads(body.decode('utf-8')) + except Exception: + # A malformed or truncated reply is treated as no reply. The batch is + # already stored or already lost by this point; guessing which from a + # broken frame would be worse than saying "unacknowledged". + return None + + +def _recv_exactly(sock, n: int): + buf = b'' + while len(buf) < n: + chunk = sock.recv(n - len(buf)) + if not chunk: + return None + buf += chunk + return buf + + def send_table(table: str): rows = fetch_unsent(table, limit=50) if not rows: @@ -518,8 +602,7 @@ def send_table(table: str): fsize = len(data_bytes) try: - with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: - s.connect((SERVER_IP, SERVER_PORT)) + with _ingest_socket() as s: agent_bytes = AGENT_NAME.encode('utf-8') s.sendall(struct.pack('!I', len(agent_bytes))) @@ -540,22 +623,45 @@ def send_table(table: str): s.sendall(struct.pack('!Q', fsize)) s.sendall(data_bytes) - # Marked sent because `sendall` returned, which means the bytes - # reached the socket buffer. The ingest protocol has no reply, so - # this is the strongest thing the agent can know - and it is weaker - # than it reads. A server that accepts a batch and stores none of it - # looks identical from here, which is exactly how four tables came to - # be permanently empty while this line said "sent (50 rows)" every - # cycle. `modules/telemetry_health` exists so the server can put its - # own row count beside this number. + # Read before the socket closes. This used to mark rows sent the + # moment `sendall` returned, which only ever meant the bytes + # reached the operating system's socket buffer - a server that + # accepted a batch and stored none of it looked identical from + # here. That is how four tables stayed permanently empty for days + # while this function logged "sent (50 rows)" every cycle. + receipt = _read_receipt(s) + + if receipt is not None and receipt.get("error"): + # The server took the bytes and could not store them. Leaving the + # rows unsent is the entire point of the reply frame: they are + # retried, and the reason is on the endpoint where somebody + # asking "why is this table empty" will find it. + telemetry_health.record_send_failure( + table, f"server refused the batch: {receipt['error']}") + if debug: + print(f"[!] {table}: server stored nothing " + f"({receipt['error']}); rows kept for retry") + return + mark_sent(table, [r['id'] for r in rows]) - telemetry_health.record_send(table, len(rows)) + telemetry_health.record_send(table, len(rows), receipt=receipt) if debug: # The IP/OS/HOST/MAC banner used to be repeated here. It is # constant for the life of the process and was the single largest # thing in agent.log: 4376 copies of the same 120-character string # in thirteen hours. It is logged once at startup instead. - print(f"[+] {table} sent ({len(rows)} rows)") + if receipt is None: + # An older server. Said as much rather than printed as a + # success, because "sent" here means the same weak thing it + # always did. + print(f"[+] {table} sent ({len(rows)} rows, unacknowledged - " + f"server did not reply)") + else: + rejected = receipt.get("rejected", 0) + note = (f", {rejected} REJECTED" if rejected else "") + print(f"[+] {table} sent ({len(rows)} rows, " + f"{receipt.get('stored', 0)} stored, " + f"{receipt.get('duplicates', 0)} already held{note})") except Exception as e: telemetry_health.record_send_failure(table, e) @@ -601,6 +707,23 @@ def periodic_wrapped(func, interval: int, name: str): if failures: print(f"[+] {name} recovered after {failures} failure(s)", flush=True) failures = 0 + except SystemExit as e: + # A library calling `sys.exit` must not end the collector. + # + # SystemExit inherits from BaseException, so `except Exception` + # does not see it - and in a thread Python discards it silently. + # `portscanner` and `info_collector` both did this on their error + # paths, so each died on its first failure and never ran again: + # `portscan_result sent` appeared zero times in the log of a host + # that had been up for days, and the Ports tab was empty with + # nothing anywhere saying why. + # + # Both are fixed to raise instead. This stays because the next + # module to do it should be a reported failure, not a thread that + # quietly stops existing. + failures += 1 + print(f"[!] {name} called sys.exit({e.code}) - treating it as a " + f"failure rather than ending the collector", flush=True) except Exception as e: failures += 1 if failures == 1 or failures % _FAILURE_REPEAT_EVERY == 0: @@ -1695,11 +1818,55 @@ def _parse_args(): 'Generated by the server installer; required for the agent to run.') parser.add_argument('--api', '-p', type=str, default=None, help='Automations API base (Default: http://:8000)') - parser.add_argument('--ingest-port', type=int, default=5001, - help='Ingest TCP port (Default: 5001)') + parser.add_argument('--ingest-port', type=int, default=None, + help='Ingest TCP port. Default follows the server URL ' + 'scheme: 5011 for https, 5001 for http.') + parser.add_argument('--ca', type=str, default=None, + help='PEM bundle used to verify the server certificate. ' + 'Needed when the server uses the self-signed CA ' + 'from certs/generate_certs.py.') return parser.parse_args() +def _resolve_ingest_transport(cfg: dict, args) -> tuple[bool, int, str | None, str | None]: + """Whether telemetry goes over TLS, and to which port. + + Derived from `server_url`, the same rule `modules/link.channel_url` uses + for the control channel: an `https://` console means an encrypted ingest + channel. Deriving beats configuring here because the two settings would + have to agree, nothing would make them, and the deployment where they + disagreed would be the one that thought it was encrypted. + + An explicit `--ingest-port` still wins. Somebody port-forwarding through a + bastion has a real reason to override this, and the derivation is a + default rather than a rule. + """ + server_url = str(cfg.get("server_url") or "") + tls = server_url.lower().startswith("https://") + + if args.ingest_port is not None: + port = int(args.ingest_port) + elif cfg.get("ingest_port"): + port = int(cfg["ingest_port"]) + else: + port = INGEST_TLS_PORT if tls else 5001 + + ca = (args.ca or cfg.get("server_ca") or os.getenv("SENTORA_CA_CERT") or "").strip() + if ca and not os.path.exists(ca): + print(f"[!] server_ca points at {ca!r}, which does not exist. TLS " + f"verification will use the system trust store and will fail " + f"against a self-signed server.", flush=True) + ca = "" + + # The name the certificate is issued for. Taken from the URL rather than + # from SERVER_IP: `certs/generate_certs.py` puts a hostname in the SAN, + # and verifying an address against it fails with a message that reads + # like a certificate problem when it is an addressing one. + host = server_url.split("://", 1)[-1].split("/", 1)[0].split(":")[0] or None + + return tls, port, (ca or None), host + + def _load_identity_from_config(path: str) -> dict: """Load agent identity (agent_name, agent_key, server_url) from JSON config.""" with open(path, "r", encoding="utf-8-sig") as f: @@ -1985,6 +2152,7 @@ def dispatch_channel_request(method: str, path: str, body) -> tuple[dict, int]: def main(): global AGENT_NAME, SERVER_IP, SERVER_PORT, AGENT_SHARED_SECRET, AUTOMATIONS_API_URL, AUTOMATIONS_MODE + global INGEST_TLS, INGEST_CA, SERVER_HOSTNAME if hasattr(signal, 'SIGTERM'): try: @@ -2031,9 +2199,11 @@ def main(): AGENT_NAME = args.agent or cfg.get("agent_name") SERVER_IP = args.server or cfg.get("server_ip") or (cfg.get("server_url", "").replace("http://", "").replace("https://", "").split(":")[0] or "127.0.0.1") - SERVER_PORT = int(args.ingest_port) AGENT_SHARED_SECRET = cfg.get("agent_key") + INGEST_TLS, SERVER_PORT, INGEST_CA, SERVER_HOSTNAME = _resolve_ingest_transport( + cfg, args) + if not AGENT_NAME: AGENT_NAME = "agent" if not AGENT_SHARED_SECRET or not cfg.get("server_url"): @@ -2064,7 +2234,8 @@ def main(): print(f"[*] Agent Name: {AGENT_NAME}") print(f"[*] Server IP: {SERVER_IP}") - print(f"[*] Ingest Port: {SERVER_PORT}") + print(f"[*] Ingest: {SERVER_IP}:{SERVER_PORT} " + f"({'TLS' if INGEST_TLS else 'PLAINTEXT - telemetry crosses the network in the clear'})") print(f"[*] API Base: {AUTOMATIONS_API_URL}") print(f"[*] Automations Mode: {AUTOMATIONS_MODE} " f"({'server' if (AUTOMATIONS_MODE == 'server' or (AUTOMATIONS_MODE == 'auto' and AUTOMATIONS_API_URL)) else 'db'})") diff --git a/Sentora/modules/fim.py b/Sentora/modules/fim.py index 5dc7e24..e2e087f 100644 --- a/Sentora/modules/fim.py +++ b/Sentora/modules/fim.py @@ -1,11 +1,35 @@ -import os +"""Real-time file integrity monitoring. + +What this used to do: watch all of `C:\\Users` recursively, with no +exclusions, and record every `.py`, `.exe`, `.dll`, `.sh` or `.php` written +anywhere beneath it. + +On a real Windows host that is `AppData\\Local\\Temp`, browser caches, pip and +npm installs, build output and every DLL a program unpacks - thousands of +"integrity violation" events an hour, none of them about integrity. The local +queue never drained: `fim_data sent (50 rows)` every ten seconds for ever, +while `network_connections` and `hardware_inventory` waited behind it. And a +real change to `/etc/shadow` arrived in the console among ten thousand rows +about a temp directory, which is the same as not arriving. + +The exclusions were not missing by oversight. `conf/file_scan.yaml` has an +`exclude_dirs` key, the console pushes it, and `check_permissions` honours it - +this module never read the file at all, so the setting was configurable, +documented, and connected to nothing. +""" + +import fnmatch import hashlib +import os import time -import threading from datetime import datetime + +import yaml from watchdog.observers import Observer from watchdog.events import FileSystemEventHandler -from .db import insert_record, fetch_where + +from . import agent_paths +from .db import insert_record DEFAULT_MONITOR_PATHS = [ "/etc", @@ -14,11 +38,99 @@ "C:\\Users", ] +#: Directories that churn on every host and mean nothing when they do. +#: +#: Watching `C:\\Users` at all is only defensible with these: the interesting +#: things under it are a user's own documents, startup folders and ssh keys, +#: and none of those live here. A deployment that genuinely wants one of them +#: watched can shorten this list in `file_scan.yaml`. +DEFAULT_EXCLUDES = { + "windows": [ + "*\\AppData\\Local\\Temp\\*", + "*\\AppData\\Local\\Packages\\*", + "*\\AppData\\Local\\Microsoft\\Windows\\INetCache\\*", + "*\\AppData\\Local\\Microsoft\\Windows\\WebCache\\*", + "*\\AppData\\Local\\Google\\Chrome\\User Data\\*\\Cache\\*", + "*\\AppData\\Local\\Mozilla\\Firefox\\Profiles\\*\\cache2\\*", + "*\\AppData\\Local\\pip\\cache\\*", + "*\\AppData\\Roaming\\npm-cache\\*", + "*\\node_modules\\*", + "*\\$Recycle.Bin\\*", + "*\\.git\\*", + "*\\__pycache__\\*", + "*\\venv\\*", + "*\\.venv\\*", + ], + "linux": [ + "/proc/*", "/sys/*", "/dev/*", "/run/*", + "/var/lib/docker/*", + "*/node_modules/*", "*/.git/*", "*/__pycache__/*", + "*/venv/*", "*/.venv/*", + ], +} + CRITICAL_FILES = [ - "passwd", "shadow", "hosts", "authorized_keys", "sshd_config", "config.php", ".env" + "passwd", "shadow", "hosts", "authorized_keys", "sshd_config", + "config.php", ".env", ] +WATCHED_EXTENSIONS = (".php", ".py", ".sh", ".exe", ".dll") + +IS_WINDOWS = os.name == "nt" + + +def _load_scan_config() -> tuple[list, list]: + """(paths to watch, patterns to ignore) from `conf/file_scan.yaml`. + + Read at start rather than hardcoded, because the console pushes this file + and an operator changing it expects something to happen. Falls back to the + defaults above if the file is missing or unreadable - a FIM that refuses + to start because a config file moved is worse than one watching the + default set. + """ + platform_key = "windows" if IS_WINDOWS else "linux" + targets = [p for p in DEFAULT_MONITOR_PATHS if os.path.exists(p)] + excludes = list(DEFAULT_EXCLUDES[platform_key]) + + try: + with open(agent_paths.config_path("file_scan"), "r", encoding="utf-8") as fh: + cfg = yaml.safe_load(fh) or {} + except Exception as e: + print(f"[FIM] file_scan.yaml unreadable ({e}); using defaults", flush=True) + return targets, excludes + + configured = (cfg.get("target_dirs") or {}).get(platform_key) or [] + if configured: + targets = [p for p in configured if os.path.exists(p)] + + # Added to the defaults rather than replacing them. Somebody adding one + # directory should not silently lose the exclusion of every temp folder. + for pattern in (cfg.get("exclude_dirs") or {}).get(platform_key) or []: + pattern = str(pattern) + if not any(ch in pattern for ch in "*?"): + pattern = pattern.rstrip("/\\") + ("\\*" if IS_WINDOWS else "/*") + if pattern not in excludes: + excludes.append(pattern) + + return targets, excludes + + +def is_excluded(path: str, patterns) -> bool: + """Whether a path is somewhere nobody wants an integrity alert about.""" + text = str(path) + lowered = text.lower() + for pattern in patterns: + if fnmatch.fnmatch(lowered, str(pattern).lower()): + return True + return False + + class FIMHandler(FileSystemEventHandler): + def __init__(self, excludes=None): + super().__init__() + self.excludes = list(excludes or []) + self.suppressed = 0 + def on_modified(self, event): if not event.is_directory: self.process(event.src_path, "modified") @@ -32,66 +144,74 @@ def on_deleted(self, event): self.process(event.src_path, "deleted") def process(self, path, status): + # Before anything else, including the hash. Hashing a file in a temp + # directory costs a read of it, and there are thousands. + if is_excluded(path, self.excludes): + self.suppressed += 1 + return + filename = os.path.basename(path) - is_critical = any(c in filename for c in CRITICAL_FILES) or any(path.endswith(ext) for ext in [".php", ".py", ".sh", ".exe", ".dll"]) - + is_critical = (any(c in filename for c in CRITICAL_FILES) + or path.endswith(WATCHED_EXTENSIONS)) if not is_critical: return current_hash = calculate_sha256(path) if status != "deleted" else "DELETED" - - rec = { + insert_record("fim_data", { "path": path, "hash_sha256": current_hash or "ERROR", "status": status, "last_seen": datetime.now().strftime("%Y-%m-%d %H:%M:%S"), - "sent": False - } - insert_record("fim_data", rec) + "sent": False, + }) print(f"[!] FIM REALTIME: File {path} was {status.upper()}!") + def calculate_sha256(file_path): - if not os.path.exists(file_path): return None + if not os.path.exists(file_path): + return None sha256_hash = hashlib.sha256() try: with open(file_path, "rb") as f: for byte_block in iter(lambda: f.read(4096), b""): sha256_hash.update(byte_block) return sha256_hash.hexdigest() - except Exception: return None + except Exception: + return None + def start_realtime_monitoring(): - event_handler = FIMHandler() + targets, excludes = _load_scan_config() + if not targets: + print("[!] FIM: No valid paths found to monitor.", flush=True) + return + + handler = FIMHandler(excludes) observer = Observer() - - monitored_count = 0 - for path in DEFAULT_MONITOR_PATHS: - if os.path.exists(path): - observer.schedule(event_handler, path, recursive=True) - monitored_count += 1 - - if monitored_count > 0: - observer.start() - print(f"[*] FIM: Real-time monitoring started for {monitored_count} base paths.") - try: - while True: - time.sleep(1) - except KeyboardInterrupt: - observer.stop() - observer.join() - else: - print("[!] FIM: No valid paths found to monitor.") - -def scan_baseline(): - """Initial baseline scan if needed.""" - print("[*] FIM: Performing initial baseline scan...") - for path in DEFAULT_MONITOR_PATHS: - if not os.path.exists(path): continue - pass + for path in targets: + observer.schedule(handler, path, recursive=True) + + observer.start() + print(f"[*] FIM: watching {len(targets)} path(s), " + f"{len(excludes)} exclusion(s).", flush=True) + try: + while True: + time.sleep(60) + # How much noise the exclusions are absorbing. Without this the + # only evidence that they are doing anything is the absence of + # events, which is also what a broken watcher produces. + if handler.suppressed: + count = handler.suppressed + print(f"[FIM] suppressed {count} events in excluded directories", + flush=True) + except KeyboardInterrupt: + observer.stop() + observer.join() + def main(): - scan_baseline() start_realtime_monitoring() + if __name__ == "__main__": main() diff --git a/Sentora/modules/find_vulns/info_collector.py b/Sentora/modules/find_vulns/info_collector.py index 76fc5fd..52687ed 100644 --- a/Sentora/modules/find_vulns/info_collector.py +++ b/Sentora/modules/find_vulns/info_collector.py @@ -59,8 +59,13 @@ def list_linux_packages(pm): result = subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, encoding='utf-8', errors='replace') if result.returncode != 0: - print(f"Error getting Linux packages: {result.stderr}", file=sys.stderr) - sys.exit(1) + # Raised, not `sys.exit(1)`. This runs on a collector thread under + # `periodic_wrapped`, which catches Exception - and SystemExit + # inherits from BaseException, so it slipped past and killed the + # thread permanently on the first failure, without a line anywhere. + raise RuntimeError( + f"{' '.join(cmd)} exited {result.returncode}: " + f"{(result.stderr or '').strip()[:200]}") if pm == 'pacman': return [line.split(' ', 1) for line in result.stdout.splitlines() if line.strip()] @@ -118,11 +123,9 @@ def save_packages(packages): insert_record_enc(TABLE, payload) def main(): - try: - system = detect_platform() - except RuntimeError as e: - print(e, file=sys.stderr) - sys.exit(1) + # Not caught and turned into `sys.exit`: the caller is a collector loop + # that reports and retries, and SystemExit would end the thread instead. + system = detect_platform() if system == 'windows': packages = list_windows_packages() diff --git a/Sentora/modules/link.py b/Sentora/modules/link.py index d2b7c2d..6103658 100644 --- a/Sentora/modules/link.py +++ b/Sentora/modules/link.py @@ -82,6 +82,23 @@ def decode_stream_frame(raw: bytes): # dying quietly while both ends believe it is up. PING_INTERVAL_S = 30.0 +# How long any single socket operation may block. Separate from the ping +# cadence, which it used to be the same number as: `create_connection` was +# handed PING_INTERVAL_S, so "ping every thirty seconds" quietly also meant +# "one send may block for thirty seconds" - six times the deadline of the +# control requests queued behind it. A config read then timed out at 5s +# against an agent that was connected and healthy, and neither side logged +# anything, because a reply that is never sent logs nothing. +SOCKET_TIMEOUT_S = 10.0 + +# How long a *stream* payload will wait for the send lock before giving up. +# +# The asymmetry this whole arrangement rests on: a screen frame is stale the +# moment it is delayed and the next one supersedes it, while nothing +# supersedes the answer somebody is waiting for. So stream payloads yield and +# control frames do not. +STREAM_SEND_WAIT_S = 0.25 + RECONNECT_MIN_S = 2.0 RECONNECT_MAX_S = 60.0 @@ -112,6 +129,15 @@ def __init__(self, server_url: str, agent_key: str, dispatch, self._send_lock = threading.Lock() self._streams: dict[int, object] = {} self._streams_lock = threading.Lock() + #: Stream payloads that yielded to a control frame. Counted rather + #: than logged per frame, and reported once a stream ends - a number + #: here is a slow link, not a fault. + self._dropped_frames = 0 + #: How many control frames are waiting for the wire. A stream payload + #: does not even queue while this is non-zero, which is what stops a + #: continuous capture from pushing a reply past the server's deadline. + self._control_waiting = 0 + self._control_lock = threading.Lock() self.connected = False self.last_error = "" @@ -183,7 +209,7 @@ def _serve_once(self) -> None: url, header=[f"X-Agent-Key: {self.agent_key}", f"X-Agent-Version: {AGENT_VERSION}"], - timeout=PING_INTERVAL_S) + timeout=SOCKET_TIMEOUT_S) self._ws = ws self.connected = True self.last_error = "" @@ -227,18 +253,55 @@ def _serve_once(self) -> None: # descriptor the agent keeps for the rest of its life. print(f"[link] socket would not close: {e}", flush=True) - def _send(self, frame) -> None: + def _send(self, frame, *, droppable: bool = False) -> None: + """Put one frame on the wire. + + Serialised, because two frames interleaved on one socket is a protocol + error rather than a slow response. That single lock is also what made + a screen stream able to starve a config reply: the pump holds it for + every frame it sends, it never stops sending, and a request waiting + behind it has five seconds before the server gives up on the agent. + + `droppable` is how the two are told apart. A stream payload that + cannot get onto the wire promptly is already stale - the next capture + supersedes it - so it yields. A reply does not: dropping one would + produce the same timeout by a quieter route. + """ ws = self._ws if ws is None: return - # Serialised: replies and stream payloads are produced on worker - # threads, and two frames interleaved on one socket is a protocol - # error rather than a slow response. - with self._send_lock: + + if droppable: + # Yield to anything already queued. Without this a screen pump + # and a reply race for the same lock on equal terms, and Python + # locks are not fair - a pump producing a frame every 200ms will + # win often enough to push a reply past the five seconds the + # server waits. + if self._control_waiting: + self._dropped_frames += 1 + return + if not self._send_lock.acquire(timeout=STREAM_SEND_WAIT_S): + # Not worth logging per frame: on a slow link this is the + # normal, healthy outcome, and a line for each one would be + # the noisiest thing in agent.log. + self._dropped_frames += 1 + return + else: + with self._control_lock: + self._control_waiting += 1 + try: + self._send_lock.acquire() + finally: + with self._control_lock: + self._control_waiting -= 1 + + try: if isinstance(frame, (bytes, bytearray)): ws.send_binary(bytes(frame)) else: ws.send(json.dumps(frame)) + finally: + self._send_lock.release() def _handle(self, raw) -> None: # Binary means stream payload; the socket keeps the two apart so @@ -324,18 +387,45 @@ def _open_stream(self, frame: dict) -> None: self._send({"t": "stream_opened", "ch": channel}) why = "the stream ended" + dropped_at_start = self._dropped_frames try: while not self._stop.is_set(): + # Whether this pump still owns the channel it was opened on. + # + # Without it a pump outlives its stream. A screen capture has + # no natural end, so when the viewer went away and the server + # forgot the channel, this loop carried on capturing and + # sending for the life of the connection - competing for the + # send lock with every config read and restart, on behalf of + # nobody. The server's side of it printed "data for a stream + # that is gone" once per frame and changed nothing. + with self._streams_lock: + still_ours = self._streams.get(channel) is stream + if not still_ours: + why = "the stream was closed" + return + chunk = stream.read(0.2) if chunk is None: why = getattr(stream, "exit_reason", "") or why break if chunk: - self._send(encode_stream_frame(channel, chunk)) + # Droppable: a screen frame that cannot get onto the wire + # promptly is stale, and the next one supersedes it. + self._send(encode_stream_frame(channel, chunk), + droppable=True) except Exception as e: why = f"the stream failed: {e}" print(f"[link] {kind} stream failed: {e}", flush=True) finally: + dropped = self._dropped_frames - dropped_at_start + if dropped: + # One line per stream, not per frame. A count here means the + # link could not keep up with the capture rate, which is a + # slow network rather than a fault - but it is the number to + # look at when a screen view is choppy. + print(f"[link] {kind} stream: {dropped} frame(s) yielded to " + f"control traffic", flush=True) self._close_stream(channel, why, tell_server=True) def _close_stream(self, channel, why: str = "", *, tell_server: bool = False) -> None: diff --git a/Sentora/modules/portscanner/portscanner.py b/Sentora/modules/portscanner/portscanner.py index d4312e8..335719f 100644 --- a/Sentora/modules/portscanner/portscanner.py +++ b/Sentora/modules/portscanner/portscanner.py @@ -647,10 +647,19 @@ async def main_async(): print(f"{'='*70}\n") except KeyboardInterrupt: print("\n[!] Scan interrupted by user.") - sys.exit(1) + raise except Exception as e: + # Raised, not `sys.exit(1)`. + # + # This runs on a collector thread under `periodic_wrapped`, which + # catches Exception. `sys.exit` raises SystemExit, which inherits from + # BaseException and slips straight past that - so the thread died on + # the first failed scan and never ran again, silently, for the life of + # the agent. `portscan_result sent` appears zero times in the log of a + # host that had been up for days, and the Ports tab was empty with + # nothing anywhere saying why. print(f"\n[!] Unexpected error: {e}") - sys.exit(1) + raise def main(): import sys diff --git a/Sentora/modules/telemetry_health.py b/Sentora/modules/telemetry_health.py index 8bf875c..f7ef20e 100644 --- a/Sentora/modules/telemetry_health.py +++ b/Sentora/modules/telemetry_health.py @@ -10,10 +10,16 @@ The last one is why none of it was visible from here. `sendall` returning means the bytes reached the operating system's socket buffer. It says nothing -about whether the server stored them, and the ingest protocol has no reply - -so the agent cannot know, and has always reported success. +about whether the server stored them. -This module does not fix that. It makes the two halves *comparable*: the agent +The protocol now carries a reply frame, so a batch can be acknowledged and +`record_send` can report what was actually stored - but only against a server +new enough to send one. `acknowledged: False` in the report below means the +server said nothing, which is not the same claim as "nothing arrived", and +the console has to be able to tell those apart. + +That still leaves the comparison worth making. This module makes the two +halves *comparable*: the agent says what it holds and what it has shipped, the server says what it holds, and the difference names the broken link. `[+] network_connections sent (50 rows)` against a server table with zero rows is not ambiguous once somebody puts the @@ -37,12 +43,32 @@ _lock = threading.Lock() -def record_send(table: str, rows: int) -> None: +def record_send(table: str, rows: int, receipt: dict | None = None) -> None: + """A batch left this agent, and what the server said about it. + + `receipt` is the thing the module docstring above says does not exist. It + does now: the ingest protocol carries a reply frame, so a send can report + what was *stored* rather than only what was written to a socket. It stays + optional because an agent can be pointed at a server older than the frame, + and `None` means exactly that - not "nothing was stored", which is a + different and much worse claim. + """ with _lock: state = _send_state.setdefault(table, {}) state["last_sent_at"] = time.time() state["last_sent_rows"] = int(rows) state["last_error"] = None + if receipt is None: + state["acknowledged"] = False + state.pop("last_stored_rows", None) + else: + state["acknowledged"] = True + state["last_stored_rows"] = int(receipt.get("stored") or 0) + state["last_duplicate_rows"] = int(receipt.get("duplicates") or 0) + # Rows the server judged unstorable. Not retried - a row it could + # not store will not become storable - so this count is the only + # trace they leave on this side. + state["last_rejected_rows"] = int(receipt.get("rejected") or 0) def record_send_failure(table: str, error: Exception | str) -> None: @@ -94,11 +120,20 @@ def report(tables) -> dict: out[table] = { "held": held, "unsent": unsent, - # Rows this agent believes it has shipped. Belief, not fact: - # nothing acknowledges an ingest batch. + # Rows this agent believes it has shipped. "shipped": max(held - unsent, 0), "last_sent_at": sent.get("last_sent_at"), "last_sent_rows": sent.get("last_sent_rows"), + # Whether the server confirmed the last batch, and how much of it + # it kept. `acknowledged: False` means the server said nothing - + # either it predates the reply frame, or it hung up - and the + # number beside it is belief rather than fact. Reported as its own + # field so the console can say which, instead of showing a count + # whose reliability is invisible. + "acknowledged": sent.get("acknowledged", False), + "last_stored_rows": sent.get("last_stored_rows"), + "last_duplicate_rows": sent.get("last_duplicate_rows"), + "last_rejected_rows": sent.get("last_rejected_rows"), "last_error": sent.get("last_error"), "last_error_at": sent.get("last_error_at"), } diff --git a/app.py b/app.py index 0622e43..28ff854 100644 --- a/app.py +++ b/app.py @@ -116,6 +116,7 @@ def password_complexity(cls, v: str): from core import login_guard from core import netloc from core import threat_feeds +from core import tls as product_tls from core import version as product_version # Installer templating: pure string generation, extracted so app.py is not # also 420 lines of PowerShell. See core/installers.py. @@ -125,7 +126,13 @@ def password_complexity(cls, v: str): # TLS; a Secure cookie on a plain-http lab deployment is silently dropped by # the browser and nobody can log in. Set SESSION_COOKIE_SECURE=1 in .env once # you terminate TLS in front of the app. -SESSION_COOKIE_SECURE = os.getenv("SESSION_COOKIE_SECURE", "0").lower() in ("1", "true", "yes", "on") +# +# Unset now means "follow TLS_ENABLED" rather than "0". The pair had to agree +# and nothing made them: an operator who turned on TLS and left this alone got +# an HTTPS console whose session cookie was still allowed onto plain HTTP. +# An explicit value still wins, because TLS terminating at a proxy is +# invisible from in here and that deployment needs the flag by hand. +SESSION_COOKIE_SECURE = product_tls.cookie_should_be_secure() # 'Lax' blocks the cookie on cross-site POST/XHR, which is the CSRF case that # matters, while staying compatible with normal same-origin use. 'Strict' is @@ -357,6 +364,42 @@ def load_or_create_fernet_from_env(): import zipfile import io +@app.get("/api/agent/ca") +async def download_ca(request): + """The self-signed CA certificate, for agents that have to verify this + server. + + Public, and safe to be. This serves `rootCA.crt` and never `rootCA.key`: + the certificate is the half of the pair whose entire purpose is to be + distributed, and publishing it grants nothing - it lets a client check a + signature it could not otherwise check. + + Fetching it over the connection it is meant to secure is trust on first + use, and worth being honest about rather than dressing up. It is the same + trust the enrolment token and the agent binary already travel on in the + same installer run: anyone able to intercept this exchange can hand over a + binary of their choosing, and the CA is the smallest of those problems. + Deployments that cannot accept that should ship `rootCA.crt` out of band + and set `server_ca` by hand. + + 404 rather than an empty body when there is no CA, because a zero-byte + PEM written to disk fails later with a message about the certificate + format, at a point far from the cause. + """ + ca = pathlib.Path(__file__).resolve().parent / "certs" / "rootCA.crt" + if not ca.exists(): + return json({ + "error": "no_local_ca", + "message": "This server has no self-signed CA. That is expected " + "when TLS is off, or when TLS_CERT points at a " + "certificate from a real CA - in which case the agent " + "needs no extra trust and server_ca can stay unset.", + }, status=404) + return await response.file( + str(ca), mime_type="application/x-pem-file", + filename="rootCA.crt") + + @app.get("/api/agent/download/") async def download_agent(request, os_type): user_id = current_user_id(request) @@ -1142,6 +1185,11 @@ async def audit_log(request, action: str, resource: str, details: str = ""): # before the agent polled them, so they never ran and the UI showed them # green. They authenticate now — see _require_agent. "download_agent", "register_agent", "agent_bootstrap", + # The CA certificate, and only the certificate. It is the half of the pair + # whose purpose is to be handed out, an installer needs it before it holds + # any credential, and publishing it grants nothing - it lets a client check + # a signature it could not otherwise check. + "download_ca", "deploy_agent_linux", "deploy_agent_windows", "get_pending_automations_for_agent", "report_automation_result", "report_automation_result_by_id", @@ -1178,6 +1226,10 @@ async def audit_log(request, action: str, resource: str, details: str = ""): # here only because the operator session middleware would otherwise reject # the handshake before the handler is ever reached. "/agent-link", + # The CA *certificate*, so an agent can verify a self-signed server. + # Public on purpose: a certificate is the half of the pair meant to be + # handed out, and an installer needs it before it has any credential. + "/api/agent/ca", } _AGENT_PATH_SUFFIXES = ("/automations/pending", "/automations/report") @@ -6249,17 +6301,47 @@ async def restart_db(request): +#: Tables this endpoint may empty. Both are deduplicated on ingest, which is +#: why `_clear_one_table` exists rather than a bare TRUNCATE. +CLEARABLE_TABLES = ["events_alert", "siem_events"] + + +async def _clear_one_table(cursor, table: str) -> None: + """Empty a table *and* forget what it has already seen. + + The TRUNCATE alone left every ingest fingerprint behind, and that is not a + tidiness problem - it permanently blinds the table. + + Ingest writes a fingerprint per row into `ingest_fingerprint` and skips any + row whose fingerprint it already holds. Clearing the rows without clearing + the fingerprints therefore means every future row of that table is skipped + as a duplicate of a row that no longer exists. The table stays empty for + ever, and the agent is told the server already has the data - so it marks + those rows sent and never offers them again. + + Seen live: an agent shipped 507 new `siem_events` rows into a table + holding zero, and every layer reported success. One click on Clear had + turned the most important table on the console into a permanent silence + that looks exactly like a quiet host. + """ + await cursor.execute(f"TRUNCATE TABLE {table}") + # Scoped to this table: the fingerprints for everything else describe rows + # that are still there, and dropping them would re-admit every duplicate + # the ingest path exists to reject. + await cursor.execute( + "DELETE FROM ingest_fingerprint WHERE table_name=%s", (table,)) + + @require_permission("clear_logs") @app.route("//clear/", methods=["POST"]) async def clear_table(request, agent, table): - allowed_tables = ["events_alert", "siem_events"] - if table not in allowed_tables: + if table not in CLEARABLE_TABLES: return sanic_json({"status": "error", "message": f"{table} Can't be cleaned."}, status=403) try: cnx = await connect_db_for_agent(agent) cursor = await cnx.cursor() - await cursor.execute(f"TRUNCATE TABLE {table}") + await _clear_one_table(cursor, table) await cnx.commit() await cursor.close(); await cnx.close() return sanic_json({"status": "success", "message": f"{table} tablosu temizlendi."}) @@ -6269,11 +6351,10 @@ async def clear_table(request, agent, table): @require_permission("clear_logs") @app.route("//clear_delayed/
", methods=["POST"]) async def clear_table_delayed(request, agent, table): - allowed_tables = ["events_alert", "siem_events"] data = request.json or {} delay = int(data.get("delay", 0)) - if table not in allowed_tables: + if table not in CLEARABLE_TABLES: return sanic_json({"status": "error", "message": f"{table} is not a table this endpoint may clear"}, status=403) if delay <= 0: @@ -6284,7 +6365,10 @@ async def delayed_clear(): try: cnx = await connect_db_for_agent(agent) cursor = await cnx.cursor() - await cursor.execute(f"TRUNCATE TABLE {table}") + # The same clear as the immediate route, through the same + # function. Two copies of a TRUNCATE is how one of them came to + # forget the fingerprints in the first place. + await _clear_one_table(cursor, table) await cnx.commit() await cursor.close(); await cnx.close() print(f"{table} {delay} cleaned after this.") @@ -9641,6 +9725,10 @@ async def send(frame) -> None: f"agent opened a channel ({state}: {detail})") reason = "the agent disconnected" + #: Channels we have already told this agent to stop. Frames sent before + #: that close arrived are ordinary, and one line each would be the + #: noisiest thing in the log at exactly the wrong moment. + orphaned: set[int] = set() try: while True: raw = await ws.recv() @@ -9655,8 +9743,26 @@ async def send(frame) -> None: continue channel, payload = parsed if not link.deliver_stream(channel, payload): - print(f"[agent-link] {agent}: data for a stream that is " - f"gone (ch {channel})", flush=True) + # Say so, rather than only noticing. This printed a line + # per frame and changed nothing, so an agent whose viewer + # had gone kept capturing and sending for the life of the + # connection - and every one of those frames competed for + # the agent's single send lock with the config reads and + # restarts queued behind them. That is how a healthy, + # connected agent came to answer GET /config/rules in + # more than five seconds. + # + # Once per channel: the close is already travelling, and + # the frames still in flight were sent before it arrived. + if channel not in orphaned: + orphaned.add(channel) + print(f"[agent-link] {agent}: data for a stream that " + f"is gone (ch {channel}); telling it to stop", + flush=True) + await _ws_notify(ws, pyjson.dumps({ + "t": "close", "ch": channel, + "why": "nothing is attached to this stream", + })) continue try: @@ -9863,48 +9969,288 @@ async def vnc_proxy(request, ws, agent): async def serve_root(request): return await response.file("./frontend/dist/index.html") +#: Page size. Capped because `size` reaches OpenSearch directly, and a +#: request for a million rows is answered by building a million rows. +SEARCH_MAX_SIZE = 200 +SEARCH_DEFAULT_SIZE = 50 + +#: How far back a search looks when nothing says otherwise. +#: +#: A default at all, rather than "everything", is the single biggest change +#: here. The previous version had no time filter of any kind, so every search +#: scanned the whole retention and sorted it - and no operator could ask the +#: question they actually had, which is nearly always about a window. +SEARCH_DEFAULT_WINDOW = "now-24h" + +_RELATIVE_TIME = re.compile(r"^now(-\d+[smhdwMy])?(/[smhdwMy])?$") + + +def _time_bound(value: str | None, fallback: str) -> str: + """An OpenSearch date-math bound, or the fallback. + + `now-7d` and an ISO timestamp are both accepted; anything else is + refused rather than passed through, because an unparseable bound sent to + the engine comes back as a 400 the operator has to decode. + """ + text = (value or "").strip() + if not text: + return fallback + if _RELATIVE_TIME.match(text): + return text + try: + datetime.fromisoformat(text.replace("Z", "+00:00")) + return text + except ValueError: + return fallback + + +def _positive_int(value, default: int, maximum: int) -> int: + """A bounded integer from a query string. + + `int(request.args.get("limit", 100))` was the previous version: `?limit=abc` + raised inside the handler and came back as a bodyless 500, and + `?limit=999999` was honoured. + """ + try: + parsed = int(str(value)) + except (TypeError, ValueError): + return default + return max(1, min(parsed, maximum)) + + @require_permission("read_telemetry") @app.route("/api/logs/search") async def search_logs_api(request): - agent = request.args.get("agent", "*") - table = request.args.get("table", "*") - query_str = request.args.get("q", "*") - limit = int(request.args.get("limit", 100)) - - if query_str == "*": - query_body = { - "size": limit, - "query": { - "bool": { - "must": [ - {"wildcard": {"agent_name": agent}} - ] - } - }, - "sort": [{"@timestamp": {"order": "desc"}}] - } - else: - query_body = { - "size": limit, - "query": { - "bool": { - "must": [ - {"wildcard": {"agent_name": agent}}, - {"multi_match": {"query": query_str, "fields": ["*"]}} - ] - } - }, - "sort": [{"@timestamp": {"order": "desc"}}] - } - - index_mask = f"sentora-logs-{table.replace('_', '-')}" if table != "*" else "sentora-logs-*" - - resp = os_utils.search_logs(query_body, index_mask=index_mask) - if not resp: - return sanic_json({"hits": []}) - - hits = [h["_source"] for h in resp.get("hits", {}).get("hits", [])] - return sanic_json({"hits": hits, "total": resp.get("hits", {}).get("total", {}).get("value", 0)}) + """Search indexed telemetry: a window, a query, and a page of results. + + What this replaces could do one thing - free text across every field, over + the whole retention, first N results, no way to see the N+1th. Three of + those are what an operator asks for first. + + The query is Lucene syntax (`severity:critical AND source:sshd`), which is + what the filter builder in the console writes and what somebody who knows + the syntax can edit by hand. Leading wildcards are refused: `*foo` forces + a scan of every term in the index and is the classic way to take a search + cluster down from a text box. + """ + query_text = (request.args.get("q") or "").strip() + agent = (request.args.get("agent") or "").strip() + table = (request.args.get("table") or "").strip() + size = _positive_int(request.args.get("size"), + SEARCH_DEFAULT_SIZE, SEARCH_MAX_SIZE) + page = _positive_int(request.args.get("page"), 1, 10_000) - 1 + + filters: list = [{ + "range": {"@timestamp": { + "gte": _time_bound(request.args.get("from"), SEARCH_DEFAULT_WINDOW), + "lte": _time_bound(request.args.get("to"), "now"), + }}, + }] + if agent: + # An exact term, not a wildcard. The previous version put the caller's + # string straight into a `wildcard` clause, so the default `*` matched + # every term in the index and a leading `*` was one keystroke away. + hyphen, underscore, _ = _agent_name_forms(agent) + filters.append({"terms": {"agent_name.keyword": + sorted({agent, hyphen, underscore})}}) + + must: list = [] + if query_text: + must.append({"query_string": { + "query": query_text, + "default_operator": "AND", + "analyze_wildcard": True, + "allow_leading_wildcard": False, + "lenient": True, + }}) + + body = { + "from": page * size, + "size": size, + "track_total_hits": True, + "query": {"bool": {"must": must or [{"match_all": {}}], "filter": filters}}, + "sort": [{"@timestamp": {"order": "desc"}}], + } + + index_mask = (f"sentora-logs-{table.replace('_', '-')}" + if table else "sentora-logs-*") + try: + resp = await asyncio.to_thread( + os_utils.search_logs, body, index_mask) + except os_utils.SearchError as e: + # 400, not an empty result. A search that could not run and a search + # that found nothing were the same screen, and on a security console + # one of those is an all-clear and the other is no answer at all. + return sanic_json({ + "status": "error", + "message": f"The query could not be run: {e}", + "hits": [], "total": 0, + }, status=400) + + hits = resp.get("hits", {}) + total = (hits.get("total") or {}).get("value", 0) + return sanic_json({ + "status": "success", + "hits": [{**h.get("_source", {}), "_index": h.get("_index")} + for h in hits.get("hits", [])], + "total": total, + "page": page + 1, + "size": size, + "pages": max(1, -(-total // size)), + "took_ms": resp.get("took"), + }) + + +#: Columns the Events search may filter and sort on, per table. +#: +#: An allowlist, because these become SQL identifiers. Everything the operator +#: types goes in as a parameter; only the *column* comes from here. +EVENT_SEARCH_TABLES = { + "siem_events": ("id", "source", "severity", "message", "techniques", + "created_at"), + "events_alert": ("id", "source", "severity", "message", "created_at"), +} + +EVENT_SEARCH_MAX_SIZE = 200 + + +def _fernet() -> "Fernet": + """The key this server decrypts telemetry with.""" + key = getattr(app.ctx, "fernet_key", None) or bootstrap_client.get_fernet_key() + return Fernet(key.encode("utf-8") if isinstance(key, str) else key) + + +def _search_events_sync(table: str, term: str, agent: str | None, + severity: str | None, hours: int, + page: int, size: int) -> tuple[list, int]: + """Search the per-agent MySQL tables. Returns (rows, total). + + Separate from the log search rather than merged with it. The two stores + answer different questions - OpenSearch holds raw telemetry, these hold + what the rules decided about it - and merging them means one ranking over + two things that are not comparable. Tabs, not a union. + """ + columns = EVENT_SEARCH_TABLES[table] + encrypted = set(ENCRYPTED_FIELDS_MAP.get(table) or ()) + + rows: list = [] + total = 0 + with sync_mysql_conn() as conn: + cursor = conn.cursor(dictionary=True) + try: + cursor.execute("SHOW DATABASES") + dbs = [r["Database"] for r in cursor.fetchall() + if str(r["Database"]).endswith("_db")] + if agent: + wanted = _agent_name_forms(agent)[1] + "_db" + dbs = [d for d in dbs if d == wanted] + + for db in dbs: + where = ["created_at >= NOW() - INTERVAL %s HOUR"] + params: list = [hours] + # `severity` is encrypted in some tables, so it cannot be + # filtered in SQL there - it is applied after decryption below. + if severity and "severity" in columns and "severity" not in encrypted: + where.append("severity = %s") + params.append(severity) + clause = " AND ".join(where) + # Through the escaper even though `columns` is an allowlist and + # `db` came from SHOW DATABASES. Relying on where a name came + # from is how the rule rots: the next person adds one that came + # from somewhere else and the shape looks identical. + select = ", ".join(_quote_identifier(c) for c in columns) + try: + cursor.execute( + f"SELECT {select} FROM {_quote_identifier(db)}" + f".{_quote_identifier(table)} " + f"WHERE {clause} ORDER BY created_at DESC LIMIT 2000", + params) + except Exception: + continue # table not provisioned on this agent + for row in cursor.fetchall(): + row["agent"] = db[:-3] + rows.append(row) + finally: + cursor.close() + + # Decrypt, then filter. The searchable text is inside the ciphertext, so + # a `LIKE` in SQL would match nothing and report it as no results - which + # is the failure this whole page is being rebuilt around. + if encrypted: + fernet_obj = _fernet() + rows = [decrypt_row_fields(r, list(encrypted), fernet_obj) for r in rows] + + if term: + needle = term.lower() + rows = [r for r in rows + if any(needle in str(v).lower() for v in r.values())] + if severity and "severity" in encrypted: + rows = [r for r in rows + if str(r.get("severity") or "").lower() == severity.lower()] + + rows.sort(key=lambda r: str(r.get("created_at") or ""), reverse=True) + total = len(rows) + start = (page - 1) * size + return rows[start:start + size], total + + +@require_permission("read_telemetry") +@app.route("/api/events/search") +async def search_events_api(request): + """Search what the rules decided, as opposed to the raw telemetry. + + The Events half of the search page. Kept as its own endpoint and its own + tab because the two stores are not comparable: one ranking over both would + have to decide whether a raw log line outranks a detection, and there is + no answer to that. + """ + table = (request.args.get("table") or "siem_events").strip() + if table not in EVENT_SEARCH_TABLES: + return sanic_json({ + "status": "error", + "message": f"{table!r} is not searchable; " + f"try {sorted(EVENT_SEARCH_TABLES)}", + }, status=400) + + size = _positive_int(request.args.get("size"), 50, EVENT_SEARCH_MAX_SIZE) + page = _positive_int(request.args.get("page"), 1, 10_000) + hours = _positive_int(request.args.get("hours"), 24, 24 * 365) + + try: + rows, total = await asyncio.to_thread( + _search_events_sync, table, + (request.args.get("q") or "").strip(), + (request.args.get("agent") or "").strip() or None, + (request.args.get("severity") or "").strip() or None, + hours, page, size) + except Exception as e: + print(f"[!] event search failed: {e}", flush=True) + return sanic_json({"status": "error", + "message": f"The search could not be run: {e}"}, + status=500) + + return sanic_json({ + "status": "success", + "hits": rows, + "total": total, + "page": page, + "size": size, + "pages": max(1, -(-total // size)), + "tables": sorted(EVENT_SEARCH_TABLES), + }) + + +@require_permission("read_telemetry") +@app.route("/api/logs/fields") +async def search_log_fields(request): + """The field names the filter builder offers. + + Read from the index mapping rather than a hand-kept list: the indices are + created by dynamic mapping from whatever the agents send, so a list here + would describe a schema nobody wrote. + """ + fields = await asyncio.to_thread(os_utils.log_fields) + return sanic_json({"status": "success", "fields": fields}) @app.route("/", name="frontend_spa") async def serve_index(request, path=""): @@ -9957,11 +10303,34 @@ async def health_check(request): f"live in one worker only; set WORKERS=1 until the registry is " f"shared between them.", flush=True) + # TLS_ENABLED / TLS_CERT / TLS_KEY were documented in .env.example, in + # docs/production-deployment.md and in the output of + # certs/generate_certs.py, and read by nothing. Setting them produced a + # console that came up cleanly on plain HTTP while its operator had every + # reason to think it was HTTPS - so the login form and the session cookie + # crossed the network in clear text with no symptom at all. + # + # `core.tls.resolve` raises rather than falling back. A server that cannot + # do what its configuration says must not quietly do something weaker. + try: + ssl_config = product_tls.resolve() + except product_tls.TLSConfigError as e: + print(f"[!] {e}", flush=True) + raise SystemExit(2) + + print(f"[*] Serving on port 8000 over {product_tls.describe()}", flush=True) + if ssl_config and not SESSION_COOKIE_SECURE: + # Reachable only when SESSION_COOKIE_SECURE=0 is set by hand against + # an HTTPS listener, which is a downgrade rather than a default. + print("[!] TLS is on but SESSION_COOKIE_SECURE=0 is set explicitly; " + "the session cookie is allowed onto plain HTTP.", flush=True) + app.run( host="0.0.0.0", port=8000, single_process=(num_workers == 1), workers=num_workers, access_log=True, - auto_reload=False + auto_reload=False, + ssl=ssl_config, ) diff --git a/core/installers.py b/core/installers.py index 3b607ed..84044e7 100644 --- a/core/installers.py +++ b/core/installers.py @@ -114,14 +114,40 @@ def _render_linux_install(server_url: str, server_ip: str, token: str) -> str: unzip -q -o agent.zip chmod +x main 2>/dev/null || true +# The CA this server signs with, if it signs with its own. +# +# Only for an https server: against http there is nothing to verify, and +# against a certificate from a real CA the endpoint's trust store already has +# it (the server answers 404 and this stays empty). Fetching it over the +# connection it secures is trust on first use - the same trust the enrolment +# token and the agent binary above already travel on in this very script. +SERVER_CA="" +case "$SERVER_URL" in + https://*) + if curl -fsSk -o "$INSTALL_DIR/rootCA.crt" "$SERVER_URL/api/agent/ca" && [ -s "$INSTALL_DIR/rootCA.crt" ]; then + SERVER_CA="$INSTALL_DIR/rootCA.crt" + echo "[*] Stored the server CA for certificate verification." + else + rm -f "$INSTALL_DIR/rootCA.crt" + echo "[i] Server has no local CA; verifying against the system trust store." + fi + ;; +esac + # Write identity config +# +# No ingest_port: the agent derives it from server_url, so an https server +# means telemetry goes to the TLS listener. Pinning 5001 here would have kept +# every installed agent on the plaintext port no matter what the server was +# configured to do. umask 077 cat > "$INSTALL_DIR/config.json" < str: return }} + # See the Linux script for the reasoning. Only for an https server, + # and trust on first use over the same connection that just delivered + # main.exe. + $ServerCa = "" + if ($ServerUrl -like "https://*") {{ + $CaPath = Join-Path $InstallDir "rootCA.crt" + try {{ + Invoke-WebRequest -Uri "$ServerUrl/api/agent/ca" -OutFile $CaPath -UseBasicParsing -TimeoutSec 30 + if ((Test-Path $CaPath) -and ((Get-Item $CaPath).Length -gt 0)) {{ + $ServerCa = $CaPath + Write-Host "[*] Stored the server CA for certificate verification." + }} + }} catch {{ + Write-Host "[i] Server has no local CA; verifying against the system trust store." -ForegroundColor DarkGray + }} + }} + + # `ingest_port` is deliberately absent. It was pinned to 5001 here, + # which would have held every Windows agent on the plaintext listener + # regardless of how the server was configured - the agent derives the + # port from server_url instead, so https means the TLS one. $Config = @{{ agent_name = $AgentName agent_key = $AgentKey server_url = $ServerUrl server_ip = $ServerIp - ingest_port = 5001 + server_ca = $ServerCa }} | ConvertTo-Json -Depth 3 $ConfigPath = Join-Path $InstallDir "config.json" # PS5.1 `Set-Content -Encoding UTF8` writes a BOM which Python's diff --git a/core/opensearch.py b/core/opensearch.py index 6f3669a..b99e5db 100644 --- a/core/opensearch.py +++ b/core/opensearch.py @@ -39,16 +39,57 @@ async def index_log(agent: str, table: str, item: dict): print(f"[OpenSearch] Error indexing log: {e}") return None -def search_logs(query_body: dict, index_mask: str = "sentora-logs-*"): +class SearchError(Exception): + """A search that could not run, with the reason the engine gave. + + Separate from "found nothing" on purpose. This used to catch everything + and return None, which the endpoint turned into `{"hits": []}` - so a + malformed query, an unreachable OpenSearch and a genuinely empty result + were the same screen. On a security console the difference between "no + matches" and "the search did not run" is the difference between an + all-clear and no answer at all. """ - Search logs in OpenSearch. + + +def search_logs(query_body: dict, index_mask: str = "sentora-logs-*"): + """Run a query. Raises SearchError if it could not run.""" + try: + return client.search(index=index_mask, body=query_body) + except Exception as e: + # The engine's own message is what tells an operator that they wrote + # `severtiy:high` - a summary of it does not. + detail = getattr(e, "info", None) or {} + reason = "" + if isinstance(detail, dict): + root = (detail.get("error") or {}).get("root_cause") or [] + if root: + reason = root[0].get("reason") or "" + raise SearchError(reason or str(e)) from e + + +def log_fields(index_mask: str = "sentora-logs-*") -> list: + """Field names present in the log indices, for the filter builder. + + Read from the mapping rather than kept as a list: the indices are created + by dynamic mapping from whatever the agents send, so any hand-maintained + list would describe a schema nobody wrote. """ try: - response = client.search( - index=index_mask, - body=query_body - ) - return response + mapping = client.indices.get_mapping(index=index_mask) except Exception as e: - print(f"[OpenSearch] Search error: {e}") - return None + print(f"[OpenSearch] mapping unavailable: {e}", flush=True) + return [] + + names: set[str] = set() + + def walk(properties: dict, prefix: str = "") -> None: + for name, spec in (properties or {}).items(): + path = f"{prefix}{name}" + if "properties" in spec: + walk(spec["properties"], f"{path}.") + else: + names.add(path) + + for index in mapping.values(): + walk(((index.get("mappings") or {}).get("properties")) or {}) + return sorted(names) diff --git a/core/tls.py b/core/tls.py new file mode 100644 index 0000000..db8076c --- /dev/null +++ b/core/tls.py @@ -0,0 +1,201 @@ +"""Turning `TLS_ENABLED=1` into an actual TLS listener. + +This module exists because the setting did not do anything. `TLS_ENABLED`, +`TLS_CERT` and `TLS_KEY` were in `.env.example`, in the production guide, and +in the instructions `certs/generate_certs.py` prints when it finishes — and no +line of code read any of them. An operator who set `TLS_ENABLED=1`, restarted, +and saw the console come up had every reason to believe the console was served +over HTTPS. It was served over plain HTTP on 0.0.0.0:8000, with the session +cookie and every credential posted to the login form travelling in clear text +across whatever network sat in front of it. + +That is the worst shape a security control can have: not missing, but +*documented*. A missing feature gets noticed the first time someone looks for +it. A documented one gets trusted and never looked at again. + +Two rules follow from that, and they are the whole design here: + +**Asking for TLS and not getting it is a startup failure, never a fallback.** +If the certificate cannot be read, the server must refuse to start rather than +listen on plain HTTP. Falling back is how the original problem would come +back wearing an error message nobody reads in a container log. + +**A cert nobody configured is generated, not assumed.** `certs.generate_certs` +already produces a per-deployment key that never leaves the machine. Wiring it +in here means the lab case works with one setting, and the honest warning +about a self-signed CA is printed rather than implied. +""" +from __future__ import annotations + +import os +import ssl + + +TRUTHY = ("1", "true", "yes", "on") + + +class TLSConfigError(RuntimeError): + """TLS was asked for and cannot be delivered. + + Raised rather than handled: see the module docstring. The one behaviour + this must never have is continuing on plain HTTP. + """ + + +def tls_enabled(env=None) -> bool: + env = os.environ if env is None else env + return str(env.get("TLS_ENABLED", "")).strip().lower() in TRUTHY + + +def resolve(env=None, generate=True, log=print) -> dict | None: + """The `ssl` argument for `app.run()`, or None to serve plain HTTP. + + `generate=False` is for callers that only want to know what is configured + without creating key material as a side effect — the boot-time report and + the tests both need that. + """ + env = os.environ if env is None else env + if not tls_enabled(env): + return None + + cert = (env.get("TLS_CERT") or "").strip() + key = (env.get("TLS_KEY") or "").strip() + + # Half a configuration is the dangerous case: it looks deliberate, and + # whichever half is missing would otherwise be filled in from a generated + # certificate the operator did not ask for and will not know is in use. + if bool(cert) != bool(key): + missing = "TLS_KEY" if cert else "TLS_CERT" + raise TLSConfigError( + f"TLS_ENABLED=1 with only one half of the pair set; {missing} is " + f"empty. Set both, or neither to use a generated certificate." + ) + + if not cert: + if not generate: + return {"cert": None, "key": None, "generated": True} + cert, key = _generate(env, log) + + for label, path in (("TLS_CERT", cert), ("TLS_KEY", key)): + if not os.path.exists(path): + raise TLSConfigError( + f"{label}={path!r} does not exist. TLS was requested and " + f"cannot be served; refusing to fall back to plain HTTP." + ) + # Existing-but-unreadable is its own case. In a container it usually + # means the key was mounted with the host's ownership, and the error + # from the TLS layer further down is a bare PermissionError with no + # path in it. + try: + with open(path, "rb") as handle: + handle.read(1) + except OSError as e: + raise TLSConfigError(f"{label}={path!r} cannot be read: {e}") from e + + # Load it here rather than leaving it to the server. A malformed PEM, or a + # key that does not match its certificate, is a clear message at startup + # instead of a handshake that fails per-connection once traffic arrives. + try: + context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) + context.load_cert_chain(certfile=cert, keyfile=key) + except ssl.SSLError as e: + raise TLSConfigError( + f"TLS_CERT={cert!r} and TLS_KEY={key!r} were found but do not form " + f"a usable pair: {e}" + ) from e + + return {"cert": cert, "key": key} + + +def _generate(env, log) -> tuple[str, str]: + """Fall back to a self-signed certificate for this deployment. + + Deliberately noisy. A generated certificate is the right default for a lab + and the wrong one for anything an operator has to trust, and the only + moment they will read about the difference is here. + """ + try: + from certs.generate_certs import ensure_certs + except ImportError as e: + raise TLSConfigError( + f"TLS_ENABLED=1, no TLS_CERT/TLS_KEY set, and the certificate " + f"generator is unavailable ({e}). Install `cryptography` or point " + f"TLS_CERT/TLS_KEY at a real certificate." + ) from e + + cn = (env.get("TLS_CN") or "").strip() or "localhost" + try: + paths = ensure_certs(cn=cn, log=log) + except Exception as e: + raise TLSConfigError(f"could not generate a certificate: {e}") from e + + log("[tls] Serving HTTPS with a self-signed certificate. Browsers will " + "warn, because nothing trusts this CA - that warning is accurate. " + "Point TLS_CERT/TLS_KEY at a real certificate for anything reachable " + "beyond your own network.") + log(f"[tls] Agents need certs/rootCA.crt to verify this server; see " + f"SENTORA_CA_CERT in the agent configuration.") + return paths["crt"], paths["key"] + + +def existing(env=None) -> dict | None: + """The certificate pair that is already on disk, without creating one. + + For the second process. `app` owns generation the way it owns the Fernet + key, and `ingest` mounts the same directory read-only - so `ingest` has to + be able to ask "is it there yet" without the answer being "it is now, + because I made a different one". Two processes generating independently + would each hold a certificate the other does not, and an agent that + verified the console would fail against ingest for no visible reason. + """ + env = os.environ if env is None else env + if not tls_enabled(env): + return None + + cert = (env.get("TLS_CERT") or "").strip() + key = (env.get("TLS_KEY") or "").strip() + if not cert or not key: + here = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + cert = os.path.join(here, "certs", "server.crt") + key = os.path.join(here, "certs", "server.key") + + if not (os.path.exists(cert) and os.path.exists(key)): + return None + return {"cert": cert, "key": key} + + +def describe(env=None) -> str: + """One line for the boot banner, so the transport is visible at a glance. + + The reason this is worth a line: an operator who believed they had TLS had + nothing on screen that would have told them otherwise. + """ + env = os.environ if env is None else env + if not tls_enabled(env): + return ("HTTP (no TLS in this process - correct only if something in " + "front of it terminates TLS)") + if (env.get("TLS_CERT") or "").strip(): + return f"HTTPS (TLS_CERT={env.get('TLS_CERT')})" + return "HTTPS (self-signed, generated for this deployment)" + + +def cookie_should_be_secure(env=None) -> bool: + """Whether the session cookie may carry `Secure`. + + Both directions of this are a real outage, which is why it is derived + rather than left to a second setting that has to agree with the first: + + `Secure` on a plain-HTTP deployment means the browser drops the cookie and + nobody can log in. No `Secure` on an HTTPS deployment means the cookie that + authenticates an operator will travel on any plain-HTTP request that can be + provoked. + + `SESSION_COOKIE_SECURE` still wins when it is set, because TLS terminating + at a proxy is invisible from in here - that deployment serves HTTP from + this process and still needs the flag. + """ + env = os.environ if env is None else env + explicit = str(env.get("SESSION_COOKIE_SECURE", "")).strip().lower() + if explicit: + return explicit in TRUTHY + return tls_enabled(env) diff --git a/db/init.sql b/db/init.sql index fff9bb4..df9091f 100644 --- a/db/init.sql +++ b/db/init.sql @@ -393,3 +393,71 @@ CREATE TABLE IF NOT EXISTS network_connections ( KEY idx_nc_port (local_port), KEY idx_nc_dup (dup_fp) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- ================== registry_logs ================== +-- +-- These three tables were missing from this file while `ALLOWED_TABLES` and +-- `DEDUP_TABLES` in server.py both named them and the agent collected them. +-- The result was a batch rejected with 1146 on every cycle, on the one side +-- where nobody was looking: the agent's log said `registry_logs sent (3 +-- rows)`, the console showed an empty table, and the only place the two +-- numbers appeared together was the telemetry health view that eventually +-- surfaced it -- 7,788 rows held on the endpoint against a table that did not +-- exist. +-- +-- `hive`, `key_path`, `value_name` and `value_data` are encrypted by the +-- agent (see core/telemetry_crypto.ENCRYPTED_FIELDS), so every one of them +-- has to be wide enough for Fernet ciphertext rather than for the value a +-- human would read. A VARCHAR sized for the plaintext fails with 1406 the +-- first time a real row arrives, which is how `network_connections` lost a +-- day of data. +CREATE TABLE IF NOT EXISTS registry_logs ( + id BIGINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + hive VARCHAR(512), + key_path TEXT, + value_name VARCHAR(512), + value_data TEXT, + status VARCHAR(32), + `timestamp` TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + sent TINYINT(1) DEFAULT 0, + dup_fp CHAR(64) NULL, + KEY idx_reg_status (status), + KEY idx_reg_dup (dup_fp) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- ================== process_events ================== +-- +-- `name`, `cmdline` and `username` are encrypted. `cmdline` is TEXT because a +-- command line is unbounded before encryption and more so after it. +CREATE TABLE IF NOT EXISTS process_events ( + id BIGINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + pid INT, + ppid INT, + name VARCHAR(512), + cmdline TEXT, + username VARCHAR(512), + status VARCHAR(32), + `timestamp` TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + sent TINYINT(1) DEFAULT 0, + dup_fp CHAR(64) NULL, + KEY idx_proc_pid (pid), + KEY idx_proc_status (status), + KEY idx_proc_dup (dup_fp) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- ================== security_audit ================== +-- +-- `finding` and `details` are encrypted; `severity` and `category` are not, +-- so the console can filter on them without a key. +CREATE TABLE IF NOT EXISTS security_audit ( + id BIGINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + category VARCHAR(64), + finding TEXT, + severity VARCHAR(32), + details TEXT, + `timestamp` TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + sent TINYINT(1) DEFAULT 0, + dup_fp CHAR(64) NULL, + KEY idx_audit_sev (severity), + KEY idx_audit_dup (dup_fp) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; diff --git a/docker-compose.yaml b/docker-compose.yaml index d3e439a..8b40f32 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -130,6 +130,18 @@ services: # made all previously encrypted agent telemetry unreadable. Back this # volume up — there is no key rotation path. - sentora_data:/app/data + # TLS material, on the same footing as the Fernet key above: generated + # on first boot by this container and needed for the life of the + # deployment. Without a volume it lived in the image layer and was + # regenerated on every `--build`, which changes the deployment's TLS + # identity - every agent that had been given the CA to trust would stop + # verifying, and stop reporting, after a routine rebuild. + # + # `app` mounts it read-write and is the only writer. `ingest` mounts the + # same volume read-only, for exactly the reason the data volume gives: + # a second process that can generate its own would hold a certificate + # the first one does not. + - sentora_certs:/app/certs # The whole host filesystem used to be bind-mounted here as # `/:/host_disk:ro`, so that the dashboard could show one disk-usage # percentage. That handed /etc/shadow, every SSH key and every other @@ -163,7 +175,17 @@ services: ollama-init: condition: service_completed_successfully ports: + # 5001 is the original plaintext listener; 5011 carries the same + # protocol inside TLS. Both are published because a fleet does not move + # in one step, and closing 5001 before the agents have been rebuilt + # makes their telemetry stop with no symptom on either side - a host + # that went quiet looks exactly like a host with nothing to say. + # + # server.py names every agent still arriving in the clear, once each. + # When that list is empty, set INGEST_TLS_REQUIRED=1 and drop the 5001 + # line below; a connection refused is a symptom somebody can see. - "5001:5001" + - "5011:5011" volumes: # Read-only, and required. The agent encrypts `message` before sending; # this key is what turns it back into a log. Without it the worker @@ -174,6 +196,9 @@ services: # read-only here means a container that starts before that cannot # quietly generate a different one and decrypt nothing. - sentora_data:/app/data:ro + # Same arrangement, same reason: `app` generates the certificate, this + # process reads it. See `server._ingest_tls_context`. + - sentora_certs:/app/certs:ro rabbitmq: image: rabbitmq:3-management @@ -341,3 +366,7 @@ volumes: mysql_data: opensearch_data: sentora_data: + # Back this up with the Fernet key. Losing it is not fatal - a new + # certificate is generated - but every agent holding the old CA stops + # verifying the server and stops sending until it is re-pointed. + sentora_certs: diff --git a/docs/production-deployment.md b/docs/production-deployment.md index 5f2cb99..40cc063 100644 --- a/docs/production-deployment.md +++ b/docs/production-deployment.md @@ -97,10 +97,30 @@ while the queue backed up behind it. | Direction | Port | Protocol | Required when | | :--- | :--- | :--- | :--- | -| Agent → server | 5001/tcp | length-framed binary | always | -| Agent → server | 8000/tcp | HTTPS REST + WS | always (SOAR polling, deploy, screen stream) | -| Operator → server | 8000/tcp | HTTPS | UI access | -| Server → agent | 5000/tcp | HTTP | config push, SOAR execute, screen stream (`AGENT_PORT`) | +| Agent → server | 5011/tcp | length-framed binary inside TLS | `TLS_ENABLED=1` (the target state) | +| Agent → server | 5001/tcp | length-framed binary, **in the clear** | until every agent is rebuilt | +| Agent → server | 8000/tcp | HTTP(S) REST + WS | always — the agent's own channel (`/agent-link`) | +| Operator → server | 8000/tcp | HTTP(S) | UI access | + +**Nothing connects to an agent.** There is no inbound port on a monitored +host: config push, SOAR execute, the console and the screen stream all travel +back down the websocket the agent opens. A firewall rule allowing the server +to reach endpoints is not needed and should be removed if one exists. + +`5001` and `5011` carry the same protocol; `5011` wraps it in TLS. Both listen +by default because a fleet does not migrate in one step, and closing `5001` +before the agents are rebuilt stops their telemetry with no symptom on either +side — a host that went quiet is indistinguishable from a host with nothing to +report. The ingest log names each agent still arriving in the clear, once +each: + +``` +[!] web-01 is sending telemetry in the clear on port 5001. ... +``` + +When that stops appearing, set `INGEST_TLS_REQUIRED=1` and stop publishing +`5001`. An un-migrated agent is then refused, which is a symptom somebody can +see. Only `app` and `ingest` publish on all interfaces. Everything else binds to `BIND_ADDR`, which defaults to `127.0.0.1`: @@ -165,10 +185,22 @@ attribute the source IP. ## 3. TLS / Certificate Handling +> **These settings did nothing until recently.** `TLS_ENABLED`, `TLS_CERT` and +> `TLS_KEY` were described in this guide, in `.env.example`, and in the +> instructions `certs/generate_certs.py` prints when it finishes — and read by +> no line of code. Setting `TLS_ENABLED=1` produced a console that came up +> cleanly on plain HTTP while its operator had every reason to believe it was +> HTTPS, with the login form and the session cookie crossing the network in +> clear text. If you are upgrading from a build before this section changed, +> **assume the console was never encrypted** and rotate any credential that was +> typed into it. + ### 3.1 Three patterns 1. **Reverse proxy terminates TLS (recommended).** App listens on plain HTTP - at `:8000`, ingest plain TCP at `:5001`. Rotation is a proxy reload. + at `:8000`, ingest plain TCP at `:5001`. Rotation is a proxy reload. Set + `SESSION_COOKIE_SECURE=1` by hand — TLS terminating at a proxy is invisible + from inside the app, so nothing can infer it for you. 2. **App container terminates TLS.** Cert at `certs/server.crt` + `.key`, and in `.env`: ```ini @@ -176,14 +208,47 @@ attribute the source IP. TLS_CERT=/app/certs/server.crt TLS_KEY=/app/certs/server.key ``` - Rotation requires `docker compose restart app`. + Rotation requires `docker compose restart app ingest`. Both read the same + pair; `app` is the only writer. 3. **Self-signed, lab only.** With `TLS_ENABLED=1` and no certificate - present, the app generates one on first boot; `python certs/generate_certs.py - --force` replaces it. Each deployment gets its own key, and the key never - leaves the machine that made it — nothing is bundled any more, because a - committed private key gave every install the same published TLS identity. - Browsers will warn: the CA is self-signed and trusts nothing. Use a real - certificate for anything reachable beyond your own network. + present, the app generates one on first boot into the `sentora_certs` + volume; `python certs/generate_certs.py --force` replaces it. Each + deployment gets its own key, and the key never leaves the machine that made + it — nothing is bundled any more, because a committed private key gave + every install the same published TLS identity. Browsers will warn: the CA + is self-signed and trusts nothing. Use a real certificate for anything + reachable beyond your own network. + +**A certificate that cannot be loaded stops the server.** `TLS_ENABLED=1` with +a missing, unreadable or mismatched pair exits with a message naming the file, +rather than falling back to plain HTTP. Falling back is how the original +problem would come back wearing an error nobody reads in a container log. + +### 3.1.1 What the agent has to trust + +Against a self-signed server the agent verifies and therefore fails, which is +correct — an unverified TLS connection is encrypted to whoever answered. Give +it the CA: + +```jsonc +// config.json, beside agent_name / agent_key / server_url +{ + "server_url": "https://soc.example.com:8000", + "server_ca": "C:\Sentora\rootCA.crt" +} +``` + +`SENTORA_CA_CERT` and `--ca` do the same thing. Copy `certs/rootCA.crt` from +the server — the certificate, never `rootCA.key`. + +The transport is derived from `server_url`, not configured separately: an +`https://` console means telemetry goes to `5011` inside TLS and the channel +goes to `wss://`. Two settings that had to agree, with nothing making them +agree, would be wrong in exactly the deployment that believed it was +encrypted. `--ingest-port` still overrides the port for a bastion setup. + +With a real certificate from a public CA, `server_ca` is unnecessary — the +system trust store already has it. ### 3.2 Session cookies require this @@ -192,8 +257,13 @@ terminating anywhere in front of the UI.** The session cookie is what authenticates the operator; without `Secure` it can travel over plain HTTP. The inverse is also a trap: setting it to `1` while serving plain HTTP means -the browser silently drops the cookie and nobody can log in. That is why the -default is `0`. +the browser silently drops the cookie and nobody can log in. + +Unset, it now follows `TLS_ENABLED` rather than defaulting to `0`. The two had +to agree and nothing made them, so turning on TLS and leaving this alone gave +an HTTPS console whose session cookie was still allowed onto plain HTTP. An +explicit value still wins — which is what pattern 1 above needs, because a +proxy terminating TLS is invisible from inside this process. `SESSION_COOKIE_SAMESITE` defaults to `Lax`, which blocks the cross-site POST/XHR that CSRF needs. Only a split-origin deployment needs `None`, and @@ -210,13 +280,52 @@ nginx -t && nginx -s reload # no downtime App-terminated: ```bash -cp new.crt /opt/sentora/certs/server.crt -cp new.key /opt/sentora/certs/server.key -docker compose restart app +docker compose cp new.crt app:/app/certs/server.crt +docker compose cp new.key app:/app/certs/server.key +docker compose restart app ingest ``` -**Agent trust:** agents do not pin a server cert — they trust the system trust -store. Rotation is server-side only; no re-enrolment needed. +`ingest` reads the same volume read-only, so it has to be restarted too — a +console on the new certificate and telemetry still presenting the old one is +the kind of half-migration that shows up as a few hosts going quiet. + +**Agent trust:** agents verify against the system trust store, plus `server_ca` +if one is configured. Rotating a certificate issued by a public CA is +server-side only. Rotating the *self-signed CA* — which `--force` does — is +not: every agent holding the old `rootCA.crt` will refuse the new server and +stop sending. Replace the file on the endpoints first, or re-enrol them. + +### 3.4 Why not mTLS, and what it would take + +Client certificates were considered for agent authentication and deliberately +not implemented. The reasoning, so it does not have to be reconstructed later: + +**What authenticates an agent today.** Every agent route checks `X-Agent-Key` +against a per-agent secret issued at enrolment. The control channel refuses +the fleet-wide key outright, because it carries `/self_destruct` among other +things and a leaked master key should not be able to open one against every +endpoint at once. Ingest is the weaker half — it identifies an agent by the +name in the frame, and TLS gives it a private channel but not a proof of who +is on the other end. + +**What mTLS would add.** A key that cannot be replayed from a captured log or +a config file, and revocation that does not depend on the server remembering +to reject a secret. + +**Why it is not in yet.** A client certificate needs an enrolment flow that +issues one, an agent that stores it as securely as it stores the key it +already has, a CA whose private key lives on the server that signs them, and a +revocation path — a CRL or a short lifetime with renewal. Every one of those +is a way for an agent to lose the ability to report, and an agent that cannot +report is invisible in precisely the way this platform exists to prevent. The +enrolment flow is the part to build first, and it is not a smaller change than +the transport work above. + +**The honest summary:** TLS here protects the telemetry in transit and proves +the *server's* identity to the agent. It does not prove the agent's identity +to the server beyond a bearer secret. If your threat model includes an +attacker who can read an endpoint's config file, that secret is what they get, +and mTLS is the thing that would help. --- @@ -228,6 +337,7 @@ store. Rotation is server-side only; no re-enrolment needed. | :--- | :--- | :--- | :--- | | Volume `mysql_data` | All telemetry, agents, AI insights, automations, sessions | High | `mysqldump --all-databases` daily | | Volume `sentora_data` | `data/fernet.key` — the **agent** telemetry key | **Critical** | Encrypted vault | +| Volume `sentora_certs` | The deployment's TLS certificate and self-signed CA | High | Encrypted vault (it holds `server.key`) | | File `.env` | `FERNET_KEY` (server key), DB and broker passwords, agent shared secret | **Critical** | Encrypted vault | | Volume `opensearch_data` | Log search index | Medium | Rebuildable from MySQL | | Volume `ollama` | Model weights | Low | Re-pull, or restore for air-gap | @@ -241,6 +351,12 @@ telemetry. If you are restoring a deployment from before that change, expect some historical rows to be undecryptable — the UI marks them `` rather than showing ciphertext. +Losing `sentora_certs` is recoverable but not free: a new certificate and a +new CA are generated, and every agent holding the old `rootCA.crt` refuses the +new server and stops sending until the file is replaced on the endpoint. The +backup script tars every volume `docker-compose.yaml` declares, so this one is +picked up without anybody having to remember it. + ### 4.2 Backup script `scripts/backup_state.py` does all of this, cross-platform, and its restore diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 1ecbdba..66eeaa7 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -22,7 +22,7 @@ import Deployment from './pages/Deployment'; import SoarHub from './pages/SoarHub'; import Assets from './pages/Assets'; import FileIntegrity from './pages/FileIntegrity'; -import LogSearch from './pages/LogSearch'; +import Search from './pages/Search'; import { authService } from './services/api'; const ProtectedRoute: React.FC<{ children: React.ReactNode }> = ({ children }) => { @@ -167,11 +167,15 @@ const App: React.FC = () => { } /> - - + } /> + + {/* The old path, so a bookmark still lands somewhere rather than on + the catch-all redirect to the dashboard. */} + } /> } /> diff --git a/frontend/src/components/Sidebar.tsx b/frontend/src/components/Sidebar.tsx index bf72a81..82be319 100644 --- a/frontend/src/components/Sidebar.tsx +++ b/frontend/src/components/Sidebar.tsx @@ -136,7 +136,7 @@ const Sidebar: React.FC = ({ isOpen }) => { {authService.hasPermission('read_telemetry') && } label="Global Alerts" />} {authService.hasPermission('read_telemetry') && } label="Asset Inventory" />} {authService.hasPermission('read_telemetry') && } label="File Integrity" />} - {authService.hasPermission('read_telemetry') && } label="Log Explorer (OS)" />} + {authService.hasPermission('read_telemetry') && } label="Search" />} {authService.hasPermission('read_telemetry') && } label="Threat Intelligence" />} {authService.hasPermission('read_telemetry') && } label="ATT&CK Coverage" />} {authService.hasPermission('manage_agent') && } label="Deploy Agent" />} diff --git a/frontend/src/components/ui/charts.tsx b/frontend/src/components/ui/charts.tsx index 23fccb6..f7f246c 100644 --- a/frontend/src/components/ui/charts.tsx +++ b/frontend/src/components/ui/charts.tsx @@ -133,11 +133,55 @@ export function TrendChart({ ); } +/** Several measures over time, on one axis because they share a unit. + * + * The reason a host page needs this rather than three numbers: a single + * sample of "CPU 91%" and a host that has been pinned at 91% for an hour look + * identical as a number and are completely different problems. Only the shape + * separates a spike from a plateau. + * + * `domain` defaults to 0-100 because the callers so far are percentages, and + * a percentage axis that rescales itself to the data makes 4% and 40% draw + * the same picture. + */ +export function SeriesLines({ + data, series, height, domain, unit, +}: { + data: Record[]; + series: { key: string; name: string }[]; + height?: number; + domain?: [number, number]; + unit?: string; +}) { + const empty = !data.some((row) => series.some((s) => Number(row[s.key]) > 0)); + return ( + + + + + + + + {series.map((s, i) => ( + + ))} + + + ); +} + /** A share of a whole. Used sparingly: a donut answers "what proportion", * and almost every other question on this console is better as bars. */ export function ShareDonut({ - data, height, -}: { data: { name: string; value: number }[]; height?: number }) { + data, height, colorFor, +}: { + data: { name: string; value: number }[]; + height?: number; + /** Per-slice colour, for the donuts where the slice *is* an outcome. */ + colorFor?: (row: { name: string; value: number }, index: number) => string; +}) { return ( d.value > 0)}> @@ -145,7 +189,8 @@ export function ShareDonut({ innerRadius="55%" outerRadius="80%" paddingAngle={2} stroke={token('--bg-color', '#000')}> {data.map((row, i) => ( - + ))} diff --git a/frontend/src/pages/AgentDetail.tsx b/frontend/src/pages/AgentDetail.tsx index 3b04c71..fc2469a 100644 --- a/frontend/src/pages/AgentDetail.tsx +++ b/frontend/src/pages/AgentDetail.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useState, useMemo } from 'react'; import { createPortal } from 'react-dom'; import { useParams, Link } from 'react-router-dom'; import { @@ -33,6 +33,30 @@ import AgentConsole from '../components/AgentConsole'; import Fuse from 'fuse.js'; import { isUnanswered, countUnanswered } from '../lib/insightTriage'; import { summariseEventMessage } from '../lib/windowsEvent'; +import { Card } from '../components/ui'; +import { CategoryBars } from '../components/ui/charts'; + +const chartNote: React.CSSProperties = { + margin: '0 0 var(--space-3)', color: 'var(--text-muted)', + fontSize: 'var(--text-xs)', maxWidth: '58ch', +}; + +const SEVERITY_TONE: Record = { + CRITICAL: 'var(--sev-critical)', + HIGH: 'var(--sev-high)', + MEDIUM: 'var(--sev-medium)', + LOW: 'var(--sev-low)', + INFO: 'var(--sev-info)', +}; +const SEVERITY_ORDER = ['CRITICAL', 'HIGH', 'MEDIUM', 'LOW', 'INFO']; + +/** Disk is the one host metric where the number alone is a decision: under + * 80% nobody cares, over 90% something is about to stop working. */ +const diskTone = (pct: number) => + pct >= 90 ? 'var(--sev-critical)' + : pct >= 80 ? 'var(--sev-high)' + : pct >= 60 ? 'var(--sev-medium)' + : 'var(--chart-1)'; // log_extractor often stuffs the enriched event into the `message` column as a // JSON string. Surface the inner fields so the table shows real columns instead @@ -757,6 +781,34 @@ const OverviewTab: React.FC<{ data: any }> = ({ data }) => { const latestResources = data.resources[0] || {}; const latestDisk = data.disks[0] || {}; + /* `resource_usage` is a snapshot table - the server keeps one row per agent + - so there is no CPU history to plot here, and a line drawn through a + single sample would be a chart pretending to be a trend. What this host + does have more than one of is alerts and open ports, so those are what + get drawn. */ + const alertsBySeverity = useMemo(() => { + const counts = new Map(); + for (const a of data.alerts || []) { + const sev = String(parseSiemRow(a).severity || 'INFO').toUpperCase(); + counts.set(sev, (counts.get(sev) ?? 0) + 1); + } + return SEVERITY_ORDER + .filter((name) => counts.has(name)) + .map((name) => ({ name, value: counts.get(name) as number })); + }, [data.alerts]); + + const portsByService = useMemo(() => { + const counts = new Map(); + for (const r of data.portscans || []) { + const key = String(r.service || 'unknown'); + counts.set(key, (counts.get(key) ?? 0) + 1); + } + return [...counts.entries()] + .map(([name, value]) => ({ name, value })) + .sort((a, b) => b.value - a.value) + .slice(0, 6); + }, [data.portscans]); + return (
@@ -801,6 +853,23 @@ const OverviewTab: React.FC<{ data: any }> = ({ data }) => {
+ +

+ Three criticals and three hundred lows are the same "300 alerts" + in a counter and completely different afternoons. +

+ SEVERITY_TONE[row.name] ?? 'var(--chart-1)'} /> + + + +

+ What this host exposes, grouped by service. Anything here that the + machine has no reason to run is attack surface nobody chose. +

+ + +

Threat Summary

@@ -867,6 +936,36 @@ const VulnsTab: React.FC<{ agentName: string, data: any[], onRefresh: () => void const rows = data.map((r: any) => [r.package_name, r.package_version, r.vulnerability_id, r.summary]); + /* OSV gives us an advisory id and a summary, not a CVSS score - so there is + no severity to chart here, and inventing one from the summary text would + be worse than not having it. What the rows do support is *where* the + findings land: which package carries them, and which advisory database + produced them. */ + const byPackage = useMemo(() => { + const counts = new Map(); + for (const r of data) { + const key = String(r.package_name || 'unknown'); + counts.set(key, (counts.get(key) ?? 0) + 1); + } + return [...counts.entries()] + .map(([name, value]) => ({ name, value })) + .sort((a, b) => b.value - a.value) + .slice(0, 6); + }, [data]); + + const bySource = useMemo(() => { + const counts = new Map(); + for (const r of data) { + const id = String(r.vulnerability_id || ''); + const prefix = id.split('-')[0].toUpperCase() || 'UNKNOWN'; + counts.set(prefix, (counts.get(prefix) ?? 0) + 1); + } + return [...counts.entries()] + .map(([name, value]) => ({ name, value })) + .sort((a, b) => b.value - a.value) + .slice(0, 6); + }, [data]); + return (
@@ -905,6 +1004,24 @@ const VulnsTab: React.FC<{ agentName: string, data: any[], onRefresh: () => void {scanning ? 'Scanning…' : 'Scan Now'}
+
+ +

+ A list of two hundred advisories is usually a handful of packages. + Upgrading the top bar tends to clear most of the table. +

+ + + +

+ Which database matched. All of one prefix means OSV only resolved + one ecosystem - the rest of the installed software went unchecked + rather than came back clean. +

+ + +
+ = ({ data }) => (
+ +

+ Every mount on one axis. Six separate meters make you compare six + numbers by eye; one chart puts the mount that is about to fill up at + the top of the picture. +

+ ({ + name: String(d.mountpoint || d.device || '?'), + value: Number(d.percent) || 0, + })) + .sort((a: any, b: any) => b.value - a.value) + .slice(0, 8)} + height={200} + colorFor={(row) => diskTone(row.value)} + /> + +

Disk Inventory

diff --git a/frontend/src/pages/Agents.tsx b/frontend/src/pages/Agents.tsx index 095fabf..9bfb521 100644 --- a/frontend/src/pages/Agents.tsx +++ b/frontend/src/pages/Agents.tsx @@ -1,4 +1,18 @@ -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useMemo, useState } from 'react'; +import { Card } from '../components/ui'; +import { CategoryBars } from '../components/ui/charts'; + +const chartNote: React.CSSProperties = { + margin: '0 0 var(--space-3)', color: 'var(--text-muted)', + fontSize: 'var(--text-xs)', maxWidth: '58ch', +}; + +/** Reachability is a state, so it takes the semantic colours rather than the + * chart palette. */ +const REACH_TONE: Record = { + Commandable: 'var(--accent-success)', + 'No channel': 'var(--sev-critical)', +}; import { Monitor, Search, @@ -75,7 +89,29 @@ const Agents: React.FC = () => { } }; - const filteredAgents = agents.filter(agent => + /* Both derived from the list already loaded. A chart with its own request + is a chart that can disagree with the table under it. */ + const osSpread = useMemo(() => { + const counts = new Map(); + for (const a of agents) { + // "Windows-11-10.0.26200-SP0" -> "Windows 11". The build number is what + // the row is for; the chart is about which platforms exist. + const raw = String(a.os_info || 'Unknown'); + const family = raw.split('-').slice(0, 2).join(' ').trim() || 'Unknown'; + counts.set(family, (counts.get(family) ?? 0) + 1); + } + return [...counts.entries()] + .map(([name, value]) => ({ name, value })) + .sort((a, b) => b.value - a.value) + .slice(0, 6); + }, [agents]); + + const reachability = useMemo(() => ([ + { name: 'Commandable', value: agents.filter(a => a.channel_connected).length }, + { name: 'No channel', value: agents.filter(a => !a.channel_connected).length }, + ]), [agents]); + + const filteredAgents = agents.filter(agent => agent.name.toLowerCase().includes(searchTerm.toLowerCase()) || (agent.display_name || '').toLowerCase().includes(searchTerm.toLowerCase()) || (agent.public_ip || '').includes(searchTerm) || @@ -101,6 +137,28 @@ const Agents: React.FC = () => {
+ {/* The shape of the estate, before the list of it. Deliberately not the + dashboard's cards again: that one answers "what needs attention", + this one answers "what am I actually running" — which is the + question you have when you open the agent list. */} +
+ +

+ What the fleet is made of. A single host on an old build is a + different problem from half the estate on one. +

+ + + +

+ Whether a command would reach the host now. Telemetry travels on a + separate connection, so an agent can be reporting and still be + uncommandable. +

+ REACH_TONE[row.name]} /> + +
+
diff --git a/frontend/src/pages/Assets.tsx b/frontend/src/pages/Assets.tsx index f7d60a5..a08edba 100644 --- a/frontend/src/pages/Assets.tsx +++ b/frontend/src/pages/Assets.tsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect } from 'react'; +import React, { useState, useEffect, useMemo } from 'react'; import { Database, Search, @@ -11,6 +11,22 @@ import { Info } from 'lucide-react'; import { agentService } from '../services/api'; +import { Card } from '../components/ui'; +import { CategoryBars } from '../components/ui/charts'; + +const chartNote: React.CSSProperties = { + margin: '0 0 var(--space-3)', color: 'var(--text-muted)', + fontSize: 'var(--text-xs)', maxWidth: '58ch', +}; + +/** A socket state is a state, so it takes a semantic colour. A listening + * socket is surface; an established one is a conversation already underway. */ +const STATE_TONE: Record = { + LISTEN: 'var(--sev-medium)', + ESTABLISHED: 'var(--sev-high)', + CLOSE_WAIT: 'var(--sev-low)', + TIME_WAIT: 'var(--sev-info)', +}; const Assets: React.FC = () => { const [agents, setAgents] = useState([]); @@ -63,6 +79,38 @@ const Assets: React.FC = () => { return searchStr.includes(searchTerm.toLowerCase()); }); + /* One chart per tab, because the three tabs answer three different + questions - and all of them are asked of the rows already on screen, + so the picture and the table can never disagree. */ + const breakdown = useMemo(() => { + const field = + activeTab === 'hardware' ? (i: any) => i.type + : activeTab === 'software' ? (i: any) => i.vendor + : (i: any) => i.state; + + const counts = new Map(); + for (const item of filteredData) { + const key = String(field(item) || 'unknown').trim() || 'unknown'; + counts.set(key, (counts.get(key) ?? 0) + 1); + } + return [...counts.entries()] + .map(([name, value]) => ({ + name: name.length > 22 ? name.slice(0, 21) + '…' : name, + value, + })) + .sort((a, b) => b.value - a.value) + .slice(0, 8); + }, [filteredData, activeTab]); + + const breakdownCopy = activeTab === 'hardware' + ? { title: 'Hardware by type', + note: 'What the inventory is made of. A category that is missing entirely usually means the collector for it failed rather than the host has none.' } + : activeTab === 'software' + ? { title: 'Packages by vendor', + note: 'Who wrote the software on this host. A long tail of one-off vendors is where unmanaged software lives.' } + : { title: 'Connections by state', + note: 'Listening sockets are attack surface; established ones are traffic already flowing. The mix is the shape of what this host is doing.' }; + return (
{/* Header Section */} @@ -159,6 +207,19 @@ const Assets: React.FC = () => { setActiveTab('network')} icon={} label="Network" />
+
+ +

{breakdownCopy.note}

+ STATE_TONE[row.name.toUpperCase()] ?? 'var(--chart-1)' + : undefined} + /> + +
+ {/* Table View */}
{ }); const tactics = [...TACTIC_ORDER, 'other'].filter(t => byTactic.has(t)); + /* In kill-chain order, not sorted by height. A bar chart of tactics sorted + by count answers a question nobody has; sorted by the chain it answers + "where in an intrusion would this estate not see anything". */ + const coverageByTactic = tactics.map(tactic => ({ + name: tactic.replace(/-/g, ' ').replace(/\b\w/g, c => c.toUpperCase()), + value: (byTactic.get(tactic) ?? []) + .filter(t => data?.covered.includes(t)).length, + })); + const cellStyle = (technique: string): React.CSSProperties => { const seen = eventsFor(technique) > 0; const covered = data?.covered.includes(technique) ?? false; @@ -183,6 +193,26 @@ const AttackCoverage: React.FC = () => { />
+ {/* The grid answers "which cell". This answers "which stage of an + intrusion am I blind in", which is the question you ask before + reading a grid of eighty cells. Drawn in kill-chain order, so the + gap is where in an attack it sits rather than where in an alphabet. */} + {!loading && tactics.length > 0 && ( +
+

+ Coverage by tactic +

+

+ Techniques a rule addresses, per stage. A short bar early in the + chain is worse than a short bar late in it: nothing you fail to + detect at initial access gets easier to catch afterwards. +

+ +
+ )} + {!loading && !!data?.covered_only_by_a_sibling?.length && (
{ if (!raw) return '-'; @@ -36,6 +43,25 @@ const AuditLogs: React.FC = () => { .finally(() => setLoading(false)); }; + /* Both derived from the rows already fetched - a chart with its own query + is a chart that can disagree with the table under it. */ + const _top = (pick: (l: any) => any) => { + const counts = new Map(); + for (const l of logs) { + const key = String(pick(l) || 'unknown').trim() || 'unknown'; + counts.set(key, (counts.get(key) ?? 0) + 1); + } + return [...counts.entries()] + .map(([name, value]) => ({ + name: name.length > 22 ? name.slice(0, 21) + '…' : name, + value, + })) + .sort((a, b) => b.value - a.value) + .slice(0, 6); + }; + const byAction = useMemo(() => _top((l) => l.action), [logs]); + const byUser = useMemo(() => _top((l) => l.username), [logs]); + const handleSearch = (e: React.KeyboardEvent) => { if (e.key === 'Enter') { fetchLogs(searchTerm || '*'); @@ -49,6 +75,27 @@ const AuditLogs: React.FC = () => {

Comprehensive history of all administrative actions, resource modifications, and system changes.

+ {/* An audit log is read after something went wrong, and by then the + question is never "what happened" but "who was doing what". A + hundred rows in reverse-chronological order answers the first and + hides the second. */} +
+ +

+ What this window is mostly made of. A destructive action appearing + at all is worth more attention than a common one appearing often. +

+ + + +

+ Who is making the changes. An account here that nobody is sitting + at is the finding. +

+ + +
+

diff --git a/frontend/src/pages/FileIntegrity.tsx b/frontend/src/pages/FileIntegrity.tsx index 9febba0..beaaa45 100644 --- a/frontend/src/pages/FileIntegrity.tsx +++ b/frontend/src/pages/FileIntegrity.tsx @@ -1,4 +1,22 @@ -import React, { useState, useEffect } from 'react'; +import React, { useState, useEffect, useMemo } from 'react'; +import { Card } from '../components/ui'; +import { CategoryBars } from '../components/ui/charts'; + +const chartNote: React.CSSProperties = { + margin: '0 0 var(--space-3)', color: 'var(--text-muted)', + fontSize: 'var(--text-xs)', maxWidth: '58ch', +}; + +/** These are states, so they take the semantic colours. A deletion is not + * worse than a modification because it is drawn in a different hue - it is + * worse, and the hue says so. */ +const STATUS_TONE: Record = { + deleted: 'var(--sev-critical)', + changed: 'var(--sev-high)', + modified: 'var(--sev-high)', + created: 'var(--sev-medium)', + baseline: 'var(--accent-neutral)', +}; import { ShieldCheck, Search, @@ -62,6 +80,38 @@ const FileIntegrity: React.FC = () => { return searchStr.includes(searchTerm.toLowerCase()); }); + /* Derived from the rows already loaded, so the charts and the timeline + below can never disagree about the same events. */ + const byStatus = useMemo(() => { + const counts = new Map(); + for (const e of data) { + const key = String(e.status || 'unknown'); + counts.set(key, (counts.get(key) ?? 0) + 1); + } + return [...counts.entries()] + .map(([name, value]) => ({ name, value })) + .sort((a, b) => b.value - a.value); + }, [data]); + + const byDirectory = useMemo(() => { + const counts = new Map(); + for (const e of data) { + const path = String(e.path || ''); + const cut = Math.max(path.lastIndexOf('\\'), path.lastIndexOf('/')); + const dir = cut > 0 ? path.slice(0, cut) : path || 'unknown'; + counts.set(dir, (counts.get(dir) ?? 0) + 1); + } + return [...counts.entries()] + // Only the tail of a long path fits on an axis, and the tail is + // the part that distinguishes them. + .map(([name, value]) => ({ + name: name.length > 28 ? '…' + name.slice(-27) : name, + value, + })) + .sort((a, b) => b.value - a.value) + .slice(0, 6); + }, [data]); + return (
{/* Header Section */} @@ -151,7 +201,31 @@ const FileIntegrity: React.FC = () => { {/* Main Content: Timeline */}
- + + {/* A timeline reads one event at a time, which is right + once you know which event. These two say which: what + kind of change, and where they are concentrated. A + directory producing hundreds of "modified" is a watch + list that needs narrowing, not an intrusion. */} +
+ +

+ Created and deleted are rarer than modified, and + worth more when they happen. +

+ STATUS_TONE[row.name] ?? 'var(--chart-1)'} /> + + +

+ Where the events cluster. One directory carrying + most of them usually means the watch list is too + wide rather than the host is busy. +

+ + +
+
diff --git a/frontend/src/pages/GlobalAlerts.tsx b/frontend/src/pages/GlobalAlerts.tsx index 3c8967d..a455cbf 100644 --- a/frontend/src/pages/GlobalAlerts.tsx +++ b/frontend/src/pages/GlobalAlerts.tsx @@ -15,9 +15,25 @@ import { RefreshCw } from 'lucide-react'; import { agentService } from '../services/api'; +import { Card } from '../components/ui'; +import { CategoryBars } from '../components/ui/charts'; import Fuse from 'fuse.js'; +const chartNote: React.CSSProperties = { + margin: '0 0 var(--space-3)', color: 'var(--text-muted)', + fontSize: 'var(--text-xs)', maxWidth: '58ch', +}; + +/** These categories *are* severities, so they take the semantic colours + * rather than the chart palette. */ +const SEVERITY_BAR: Record = { + Critical: 'var(--sev-critical)', + High: 'var(--sev-high)', + Medium: 'var(--sev-medium)', + Low: 'var(--sev-low)', +}; + const GlobalAlerts: React.FC = () => { const [alerts, setAlerts] = useState([]); const [loading, setLoading] = useState(true); @@ -117,6 +133,27 @@ const GlobalAlerts: React.FC = () => { low: alerts.filter(a => a.severity === 'LOW').length }; + /* Derived from the rows already on the page. A chart that needs its own + request is a chart that disagrees with the table under it. */ + const severityBars = React.useMemo(() => ([ + { name: 'Critical', value: stats.critical }, + { name: 'High', value: stats.high }, + { name: 'Medium', value: stats.medium }, + { name: 'Low', value: stats.low }, + ]), [stats.critical, stats.high, stats.medium, stats.low]); + + const byAgent = React.useMemo(() => { + const counts = new Map(); + for (const a of alerts) { + const host = a.agent || 'unknown'; + counts.set(host, (counts.get(host) ?? 0) + 1); + } + return [...counts.entries()] + .map(([name, value]) => ({ name, value })) + .sort((a, b) => b.value - a.value) + .slice(0, 6); + }, [alerts]); + return (
@@ -132,11 +169,31 @@ const GlobalAlerts: React.FC = () => {
{/* Severity Stats */} -
- } /> - } /> - } /> - } /> +
+ } /> + } /> + } /> + } /> +
+ + {/* The two questions asked before reading a single alert: how bad, and + where. Both are answered by the rows already on this page, so they + cost nothing but the drawing. */} +
+ +

+ The shape matters more than the total: a hundred alerts that are + all low is a quiet day, and three criticals is not. +

+ SEVERITY_BAR[row.name]} /> + + +

+ Where the alerts are concentrated. One host carrying most of them + is usually one misconfiguration rather than an estate on fire. +

+ +
diff --git a/frontend/src/pages/LogSearch.tsx b/frontend/src/pages/LogSearch.tsx deleted file mode 100644 index 71cc6f1..0000000 --- a/frontend/src/pages/LogSearch.tsx +++ /dev/null @@ -1,232 +0,0 @@ -import React, { useState, useEffect } from 'react'; -import { Search, Filter, Database, Calendar, User, Eye, BarChart3 } from 'lucide-react'; -import { agentService } from '../services/api'; - -const LogSearch: React.FC = () => { - const [agents, setAgents] = useState([]); - const [selectedAgent, setSelectedAgent] = useState('*'); - const [selectedTable, setSelectedTable] = useState('*'); - const [query, setQuery] = useState('*'); - const [logs, setLogs] = useState([]); - const [loading, setLoading] = useState(false); - const [total, setTotal] = useState(0); - const [selectedLog, setSelectedLog] = useState(null); - - const tables = [ - { id: '*', name: 'All Types' }, - { id: 'siem_events', name: 'SIEM Events' }, - { id: 'events_alert', name: 'Security Alerts' }, - { id: 'process_events', name: 'Process Events' }, - { id: 'network_connections', name: 'Network' }, - { id: 'fim_data', name: 'File Integrity' }, - { id: 'audit_logs', name: 'Audit Logs' } - ]; - - useEffect(() => { - agentService.getAgents().then(setAgents).catch(console.error); - handleSearch(); - }, []); - - const handleSearch = () => { - setLoading(true); - agentService.searchLogs({ - agent: selectedAgent, - table: selectedTable, - q: query, - limit: 100 - }) - .then(res => { - setLogs(res.hits || []); - setTotal(res.total || 0); - }) - .catch(err => { - console.error(err); - setLogs([]); - }) - .finally(() => setLoading(false)); - }; - - return ( -
- - -
-
-
- -
- - setQuery(e.target.value || '*')} - onKeyDown={(e) => e.key === 'Enter' && handleSearch()} - style={{ width: '100%', backgroundColor: 'var(--bg-color)', border: '1px solid var(--border-color)', borderRadius: '10px', padding: '12px 12px 12px 42px', color: 'white', fontSize: '0.9rem', transition: 'border-color 0.2s' }} - /> -
-
- -
- - -
- -
- - -
- - -
-
- -
-
-
- - Results from OpenSearch -
- Showing top 100 matches -
- -
-

- - - - - - - - - - {logs.map((log, i) => ( - e.currentTarget.style.backgroundColor = 'rgba(255,255,255,0.02)'} - onMouseLeave={e => e.currentTarget.style.backgroundColor = 'transparent'} - > - - - - - - ))} - {logs.length === 0 && !loading && ( - - - - )} - -
TimestampAgentLog Content / DetailsAction
-
- - {new Date(log['@timestamp']).toLocaleString()} -
- 		-
-
- -
- {log.agent_name} -
- 		-
- - {log.file_path || log.path || log.process_name || log.remote_ip || 'TELEMETRY_DATA'} - - - {JSON.stringify(log).substring(0, 150)}... - -
- 		- -
- -

No log entries found for specific criteria.

-
- - - - {selectedLog && ( -
setSelectedLog(null)}> -
e.stopPropagation()}> -
-

Raw JSON Data

- -
-
- 
-                                {JSON.stringify(selectedLog, null, 2)}
-
-
-
-
- )} - - ); -}; - -export default LogSearch; diff --git a/frontend/src/pages/LoginLogs.tsx b/frontend/src/pages/LoginLogs.tsx index bd23f09..36ee18b 100644 --- a/frontend/src/pages/LoginLogs.tsx +++ b/frontend/src/pages/LoginLogs.tsx @@ -1,6 +1,13 @@ -import React, { useState, useEffect } from 'react'; +import React, { useState, useEffect, useMemo } from 'react'; import { ShieldCheck, ShieldAlert, Globe, Search } from 'lucide-react'; import { adminService } from '../services/api'; +import { Card } from '../components/ui'; +import { CategoryBars } from '../components/ui/charts'; + +const chartNote: React.CSSProperties = { + margin: '0 0 var(--space-3)', color: 'var(--text-muted)', + fontSize: 'var(--text-xs)', maxWidth: '58ch', +}; const LoginLogs: React.FC = () => { const [logs, setLogs] = useState([]); @@ -12,6 +19,34 @@ const LoginLogs: React.FC = () => { .finally(() => setLoading(false)); }, []); + /* A hundred rows scrolled past is not a picture of who is trying to get in. + These two are: which accounts are being attempted, and where from. Both + read the rows already loaded, so neither can disagree with the table. */ + const failedByUser = useMemo(() => { + const counts = new Map(); + for (const l of logs) { + if (l.status === 'success') continue; + const key = String(l.username || 'unknown'); + counts.set(key, (counts.get(key) ?? 0) + 1); + } + return [...counts.entries()] + .map(([name, value]) => ({ name, value })) + .sort((a, b) => b.value - a.value) + .slice(0, 6); + }, [logs]); + + const bySourceIp = useMemo(() => { + const counts = new Map(); + for (const l of logs) { + const key = String(l.ip_address || 'unknown'); + counts.set(key, (counts.get(key) ?? 0) + 1); + } + return [...counts.entries()] + .map(([name, value]) => ({ name, value })) + .sort((a, b) => b.value - a.value) + .slice(0, 6); + }, [logs]); + return (
@@ -19,6 +54,24 @@ const LoginLogs: React.FC = () => {

Detailed history of all authentication attempts, including local and LDAP logins.

+
+ +

+ Empty is the healthy picture. One account taking most of the bars + is either a stuck client or someone guessing at it. +

+ 'var(--sev-critical)'} /> + + +

+ Where the logins come from, successes included. An address nobody + recognises near the top is worth a look before it succeeds. +

+ + +
+

Login History

diff --git a/frontend/src/pages/Search.tsx b/frontend/src/pages/Search.tsx new file mode 100644 index 0000000..0a32c25 --- /dev/null +++ b/frontend/src/pages/Search.tsx @@ -0,0 +1,302 @@ +/** + * Search, with the three things it could not do. + * + * The previous version was free text across every field, over the whole + * retention, first N results, no way to reach the N+1th. So the question an + * operator actually has — "what happened on that host between two and four, + * with severity critical" — could not be asked at all. + * + * Filters and text are the same thing here. Clicking a filter writes the + * query; the query stays editable, so somebody who knows the syntax is never + * fighting the builder, and somebody who does not can learn it by watching + * what their clicks produce. A builder that hides the query teaches nothing + * and a bare text box helps nobody on their first day. + * + * Two tabs, not one ranked list. Logs are raw telemetry and Events are what + * the rules decided about it; merging them would mean deciding whether a log + * line outranks a detection, and there is no answer to that. + */ +import { useCallback, useEffect, useMemo, useState } from 'react'; +import { Link } from 'react-router-dom'; +import { Search as SearchIcon, Plus, X, ChevronLeft, ChevronRight } from 'lucide-react'; +import api from '../services/api'; +import { + PageHeader, Card, Badge, DataTable, Row, Cell, + EmptyState, ErrorState, LoadingState, Tone, +} from '../components/ui'; + +type Tab = 'logs' | 'events'; + +type Filter = { field: string; op: ':' | '!='; value: string }; + +const RANGES = [ + { label: '1h', from: 'now-1h' }, + { label: '24h', from: 'now-24h' }, + { label: '7d', from: 'now-7d' }, + { label: '30d', from: 'now-30d' }, +]; + +const SEVERITY_TONE: Record = { + critical: 'critical', high: 'high', medium: 'medium', + low: 'low', info: 'info', +}; + +/** Filters -> the Lucene the server runs. The one place the two agree. */ +function toQuery(filters: Filter[]): string { + return filters + .filter((f) => f.field && f.value) + .map((f) => { + const value = /[\s"]/.test(f.value) ? `"${f.value.replace(/"/g, '\\"')}"` + : f.value; + return f.op === ':' ? `${f.field}:${value}` : `NOT ${f.field}:${value}`; + }) + .join(' AND '); +} + +export default function Search() { + const [tab, setTab] = useState('logs'); + const [filters, setFilters] = useState([]); + const [query, setQuery] = useState(''); + const [range, setRange] = useState('now-24h'); + const [fields, setFields] = useState([]); + const [hits, setHits] = useState([]); + const [total, setTotal] = useState(0); + const [pages, setPages] = useState(1); + const [page, setPage] = useState(1); + const [loading, setLoading] = useState(false); + const [error, setError] = useState(''); + const [ran, setRan] = useState(false); + + useEffect(() => { + api.get('/api/logs/fields') + .then((r) => setFields(r.data?.fields ?? [])) + .catch(() => setFields([])); + }, []); + + const run = useCallback(async (nextPage = 1) => { + setLoading(true); + setError(''); + try { + const path = tab === 'logs' ? '/api/logs/search' : '/api/events/search'; + const params: any = { q: query, page: nextPage, size: 50 }; + if (tab === 'logs') params.from = range; + else params.hours = { 'now-1h': 1, 'now-24h': 24, 'now-7d': 168, 'now-30d': 720 }[range] ?? 24; + + const res = await api.get(path, { params }); + setHits(res.data?.hits ?? []); + setTotal(res.data?.total ?? 0); + setPages(res.data?.pages ?? 1); + setPage(nextPage); + } catch (e: any) { + // A search that could not run must never render as one that found + // nothing. The engine's own message is what tells somebody they wrote + // `severtiy:high`. + setError(e?.response?.data?.message || e?.message || 'Search failed'); + setHits([]); + setTotal(0); + } finally { + setLoading(false); + setRan(true); + } + }, [tab, query, range]); + + const addFilter = (field: string, value: string, op: Filter['op'] = ':') => { + const next = [...filters, { field, op, value: String(value) }]; + setFilters(next); + setQuery(toQuery(next)); + }; + + const dropFilter = (index: number) => { + const next = filters.filter((_, i) => i !== index); + setFilters(next); + setQuery(toQuery(next)); + }; + + const columns = useMemo(() => { + if (!hits.length) return []; + const seen = new Set(); + for (const hit of hits.slice(0, 20)) Object.keys(hit).forEach((k) => seen.add(k)); + // Time and host first; they are what a result is read by. + const preferred = ['@timestamp', 'created_at', 'agent_name', 'agent', + 'severity', 'source', 'message']; + const rest = [...seen].filter((c) => !preferred.includes(c) && !c.startsWith('_')); + return [...preferred.filter((c) => seen.has(c)), ...rest].slice(0, 8); + }, [hits]); + + const agentOf = (hit: any) => hit.agent_name || hit.agent || ''; + + return ( +
+ } + title="Search" + subtitle="Logs are the raw telemetry; Events are what the rules decided + about it. Filters write the query below, and the query stays + editable." + /> + + +
+ {(['logs', 'events'] as Tab[]).map((t) => ( + + ))} +
+ {RANGES.map((r) => ( + + ))} +
+ + + + {filters.length > 0 && ( +
+ {filters.map((f, i) => ( + + {f.op === '!=' && NOT} + {f.field}:{f.value} + + + ))} +
+ )} + +
{ e.preventDefault(); run(1); }} + style={{ display: 'flex', gap: 'var(--space-2)', marginTop: 'var(--space-3)' }}> + setQuery(e.target.value)} + placeholder={tab === 'logs' + ? 'severity:critical AND source:sshd' + : 'free text across the decrypted fields'} + style={{ flex: 1, minWidth: 0 }} + /> + +
+ {tab === 'events' && ( +
+ Events are stored encrypted, so this matches after decryption rather + than in SQL — free text, not field syntax. +
+ )} + + + {loading && } + {!loading && error && } + {!loading && !error && ran && hits.length === 0 && ( + + )} + + {!loading && !error && hits.length > 0 && ( + + + + {page} / {pages} + + +
+ } + > + + {hits.map((hit, i) => ( + + {columns.map((col) => ( + + {col === 'severity' && hit[col] ? ( + + {hit[col]} + + ) : ( + /* Click a value to narrow, which is how a search + becomes an investigation rather than one question. */ + + )} + + ))} + + {agentOf(hit) && ( + + Open host + + )} + + + ))} + + + )} +
+ ); +} + +function FilterBuilder({ + fields, onAdd, +}: { fields: string[]; onAdd: (f: string, v: string, op: Filter['op']) => void }) { + const [field, setField] = useState(''); + const [op, setOp] = useState(':'); + const [value, setValue] = useState(''); + + const add = () => { + if (!field || !value) return; + onAdd(field, value, op); + setValue(''); + }; + + return ( +
+ setField(e.target.value)} style={{ minWidth: '180px' }} /> + + {fields.map((f) => + + setValue(e.target.value)} + onKeyDown={(e) => { if (e.key === 'Enter') { e.preventDefault(); add(); } }} + style={{ minWidth: '180px' }} /> + +
+ ); +} diff --git a/frontend/src/pages/SoarHub.tsx b/frontend/src/pages/SoarHub.tsx index 15d9569..e4f33ef 100644 --- a/frontend/src/pages/SoarHub.tsx +++ b/frontend/src/pages/SoarHub.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useState, useMemo } from 'react'; import { Zap, Activity, @@ -19,6 +19,22 @@ import { } from 'lucide-react'; import { agentService } from '../services/api'; import { Link } from 'react-router-dom'; +import { Card } from '../components/ui'; +import { CategoryBars, ShareDonut } from '../components/ui/charts'; + +const chartNote: React.CSSProperties = { + margin: '0 0 var(--space-3)', color: 'var(--text-muted)', + fontSize: 'var(--text-xs)', maxWidth: '58ch', +}; + +/** Outcomes are states, so they take the semantic colours rather than a + * series palette - a failed response should not be teal because it happened + * to be the third slice. */ +const OUTCOME_TONE: Record = { + Failed: 'var(--sev-critical)', + Pending: 'var(--sev-medium)', + Succeeded: 'var(--accent-success)', +}; // ── Action intelligence ────────────────────────────────────────────────────── type RiskLevel = 'LOW' | 'MEDIUM' | 'HIGH' | 'CRITICAL'; @@ -215,6 +231,51 @@ const SoarHub: React.FC = () => { }; const [searchTerm, setSearchTerm] = useState(''); + /* Derived from the same rows the stream below renders, so the charts can + never disagree with the table under them. */ + const _label = (a: any) => String(a.action || 'unknown').toUpperCase(); + const _succeeded = (a: any) => a.status === 'success' || a.status === 'completed'; + const _pending = (a: any) => a.status === 'active' || a.status === 'pending'; + + const byAction = useMemo(() => { + const counts = new Map(); + for (const a of allActionsCache) { + const key = _label(a); + counts.set(key, (counts.get(key) ?? 0) + 1); + } + return [...counts.entries()] + .map(([name, value]) => ({ name, value })) + .sort((a, b) => b.value - a.value) + .slice(0, 6); + }, [allActionsCache]); + + const byOutcome = useMemo(() => { + let ok = 0, failed = 0, pending = 0; + for (const a of allActionsCache) { + if (_succeeded(a)) ok++; + else if (a.status === 'failed') failed++; + else if (_pending(a)) pending++; + } + return [ + { name: 'Succeeded', value: ok }, + { name: 'Failed', value: failed }, + { name: 'Pending', value: pending }, + ]; + }, [allActionsCache]); + + const failuresByAction = useMemo(() => { + const counts = new Map(); + for (const a of allActionsCache) { + if (a.status !== 'failed') continue; + const key = _label(a); + counts.set(key, (counts.get(key) ?? 0) + 1); + } + return [...counts.entries()] + .map(([name, value]) => ({ name, value })) + .sort((a, b) => b.value - a.value) + .slice(0, 6); + }, [allActionsCache]); + const filteredActions = (filterMode === 'failed' ? allActionsCache.filter(a => a.status === 'failed') : allActionsCache @@ -309,6 +370,37 @@ const SoarHub: React.FC = () => { } color="var(--accent-color)" />
+ {/* Four counters say how much automation ran. They do not say whether + it worked, and a playbook that fires constantly and fails every time + scores well on all four. These two answer that: which actions the + rules actually reach for, and how those attempts end. */} +
+ +

+ Which responses the rules reach for. One action carrying everything + usually means one rule is doing all the firing. +

+ + + +

+ A response that was attempted is not a response that landed. Failures + here are threats that were detected and then not contained. +

+ OUTCOME_TONE[row.name] ?? 'var(--chart-1)'} /> + + +

+ Empty is the healthy picture. A single action dominating this chart + is a broken playbook rather than a busy week - the same call failing + the same way every time. +

+ 'var(--sev-critical)'} /> + +
+
{/* Real-time Action Stream */}
diff --git a/frontend/src/pages/ThreatIntel.tsx b/frontend/src/pages/ThreatIntel.tsx index 48f451f..716ebb6 100644 --- a/frontend/src/pages/ThreatIntel.tsx +++ b/frontend/src/pages/ThreatIntel.tsx @@ -1,5 +1,6 @@ -import React, { useEffect, useState, useCallback } from 'react'; -import { StatCard } from '../components/ui'; +import React, { useEffect, useState, useCallback, useMemo } from 'react'; +import { StatCard, Card } from '../components/ui'; +import { CategoryBars, ShareDonut } from '../components/ui/charts'; import { Radar, RefreshCw, Search, AlertTriangle, Globe, FileDigit, Link2, Server } from 'lucide-react'; import { agentService, adminService, authService } from '../services/api'; @@ -14,6 +15,11 @@ type Indicator = { last_seen?: string; }; +const chartNote: React.CSSProperties = { + margin: '0 0 var(--space-3)', color: 'var(--text-muted)', + fontSize: 'var(--text-xs)', maxWidth: '58ch', +}; + const SEVERITY_STYLE: Record = { CRITICAL: { color: '#ef4444', bg: 'rgba(239,68,68,0.10)' }, HIGH: { color: '#f97316', bg: 'rgba(249,115,22,0.10)' }, @@ -98,6 +104,22 @@ const ThreatIntel: React.FC = () => { const stats = data?.stats; const indicators: Indicator[] = data?.indicators || []; + + /* Both read the same `stats` the counters and the source list read, so a + chart cannot disagree with the row beside it. */ + const typeShare = useMemo(() => ( + (stats?.by_type || []) + .map((t: any) => ({ name: String(t.type || 'unknown'), value: Number(t.n) || 0 })) + .sort((a: any, b: any) => b.value - a.value) + .slice(0, 8) + ), [stats]); + + const sourceShare = useMemo(() => ( + (stats?.by_source || []) + .map((t: any) => ({ name: String(t.source || 'unknown'), value: Number(t.n) || 0 })) + .sort((a: any, b: any) => b.value - a.value) + .slice(0, 6) + ), [stats]); const feedsOff = stats?.mode === 'off'; return ( @@ -174,6 +196,27 @@ const ThreatIntel: React.FC = () => { />
+ {/* The counters say how many indicators are held; these say what kind + they are and where they came from. Both matter, because a feed list + that is 99% one source is one outage away from being empty, and a + store that is all file hashes will not match network telemetry. */} +
+ +

+ What the store can actually match on. Hashes match process + telemetry; addresses and domains match connections. +

+ + + +

+ How concentrated the intel is. One feed carrying nearly all of it + means one broken key takes the coverage with it. +

+ + +
+ {/* Per-source breakdown: a feed that stopped refreshing shows an old timestamp here instead of just quietly shrinking as pruning runs. */} {!!stats?.by_source?.length && ( diff --git a/server.py b/server.py index ec24f07..a34ff17 100644 --- a/server.py +++ b/server.py @@ -1,4 +1,7 @@ import asyncio +import contextlib +import functools +import ssl import struct import os import mysql.connector @@ -22,6 +25,29 @@ SERVER_PORT = int(os.getenv('INGEST_PORT', '5001')) BUFFER_SIZE = 16 * 1024 +# Telemetry over TLS, on a second port beside the plaintext one. +# +# A second port rather than a flag on the first, because the two cannot share +# one: the existing protocol opens with a 4-byte big-endian length, so its +# first byte is always 0x00, and a TLS ClientHello opens with 0x16. They are +# trivially distinguishable - and distinguishing them means reading a byte +# that then cannot be handed back to the TLS layer, so one port can serve one +# of them and not both. +# +# Both listeners run by default, on purpose. Cutting straight to TLS would +# blackhole every agent built before this, and telemetry going quiet is the +# exact failure this codebase keeps finding: a host that stopped reporting and +# a host with nothing to report render identically. So the plaintext listener +# stays, counts who is still using it, and says so - and +# INGEST_TLS_REQUIRED=1 closes it once the fleet has moved. +INGEST_TLS_PORT = int(os.getenv('INGEST_TLS_PORT', '5011')) +INGEST_TLS_REQUIRED = os.getenv('INGEST_TLS_REQUIRED', '0').strip().lower() in ('1', 'true', 'yes', 'on') + +#: Agents seen on the plaintext listener since this process started. Not a +#: counter of connections - a set of names, because "nine agents are still in +#: the clear" is the number an operator can act on, and 40,000 is not. +PLAINTEXT_AGENTS: set[str] = set() + DB_HOST = os.getenv('DB_HOST', '127.0.0.1') DB_USER = os.getenv('DB_USER', 'root') DB_PASSWORD = os.getenv('DB_PASSWORD', 'my-secret-pw') @@ -30,8 +56,24 @@ #: Tables that hold the *current* state of a host rather than a history of #: events. Each batch replaces the contents: the agent re-sends the whole #: picture, and the previous picture is no longer true. +#: +#: "The agent re-sends the whole picture" is the load-bearing half of that +#: sentence, and it is a claim about the collector, not a preference. Snapshot +#: semantics are only correct for a collector that re-reports everything every +#: cycle; against one that reports deltas, DELETE-then-insert throws away +#: everything the delta did not mention. +#: +#: `critical_files` used to be here and is the reason the paragraph exists. +#: `check_permissions` writes a row only when a file's owner, group or mode +#: has actually changed - it is a change log, and the agent even keeps a +#: UNIQUE index on its `dup_fp` to guarantee that. So three permission changes +#: meant the server deleted the 1,200 rows it held and stored three, and after +#: a server-side database reset the table stayed empty for ever: there was +#: nothing new to send, and by design a sent row is never re-offered. Six of +#: the seven tables the agent deduplicates locally were already in +#: DEDUP_TABLES; this was the one that was not. SNAPSHOT_TABLES = { - "resource_usage", "disk_usage", "critical_files", + "resource_usage", "disk_usage", "network_connections", "hardware_inventory", "docker_containers", } @@ -54,7 +96,7 @@ #: `_assert_ingest_modes_are_exclusive` below makes adding a table to both a #: startup failure rather than a silently empty tab. DEDUP_TABLES = { - "portscan_result", "packages", + "critical_files", "portscan_result", "packages", "vulnerabilities_report", "siem_events", "events_alert", "soar_actions", "fim_data", "registry_logs", "process_events", "security_audit", "software_inventory", "network_inventory" @@ -711,7 +753,23 @@ def _parse_os_info_tail(os_info: str | None): async def insert_data(agent: str, table: str, data: list, public_ip: str = None, os_info: str = None, - reported_ip: str = None): + reported_ip: str = None) -> dict: + """Store a batch and report what actually happened to it. + + The return value is the point. This used to return None and swallow every + insertion failure into a debug line, which was fine for a caller that + ignored it - and the caller ignored it because the wire protocol had no + way to say anything back. The agent therefore marked rows sent the moment + `sendall` returned, and a server that accepted a batch and stored none of + it was indistinguishable from one that stored all of it. + + That is not hypothetical: four tables were permanently empty for days + while the agent logged "sent (50 rows)" every cycle, because the column + was too narrow for the ciphertext and every INSERT raised 1406 into this + `except`. + + So: a receipt, and an `error` that survives to the other end. + """ db_name = create_agent_db_if_not_exists(agent) create_tables_if_not_exist(db_name) @@ -725,7 +783,7 @@ async def insert_data(agent: str, table: str, data: list, public_ip: str = None, try: if table not in ALLOWED_TABLES: print(f"[!] Unknown table '{table}' received. Skipping.") - return + return {"stored": 0, "error": f"unknown table {table!r}"} # A snapshot replaces what was there. Named rather than written out # here, because the same list existing twice is how it came to @@ -733,6 +791,11 @@ async def insert_data(agent: str, table: str, data: list, public_ip: str = None, if table in SNAPSHOT_TABLES: cursor.execute(f"DELETE FROM `{table}`") + stored = 0 + duplicates = 0 + rejected = 0 + first_error = None + for item in data: item = dict(item) item.pop("id", None) @@ -745,12 +808,65 @@ async def insert_data(agent: str, table: str, data: list, public_ip: str = None, (table, fp) ) if cursor.rowcount == 0: + # Already held. Counted as accepted rather than stored: + # the agent must not resend it, and reporting it as + # stored would make dedup look like data loss. + duplicates += 1 continue keys = ', '.join(f"`{k}`" for k in item.keys()) values = ', '.join(['%s'] * len(item)) sql = f"INSERT INTO `{table}` ({keys}) VALUES ({values})" - cursor.execute(sql, list(item.values())) + if table in SNAPSHOT_TABLES: + # Last write wins, which is what "snapshot" means. + # + # The agent appends a row per entity per collection cycle and + # ships them in batches of fifty, so one batch routinely + # carries the same container or disk from several cycles. The + # server's DELETE clears the previous picture but not the + # repeats inside the batch itself, and `docker_containers` has + # UNIQUE(container_id) - so the second copy raised 1062, the + # whole batch rolled back, and the table stayed empty while + # the agent logged "docker_containers sent (8 rows)". + updates = ', '.join(f"`{k}`=VALUES(`{k}`)" for k in item.keys()) + sql += f" ON DUPLICATE KEY UPDATE {updates}" + + try: + cursor.execute(sql, list(item.values())) + stored += 1 + except Exception as row_error: + # Per row, because one unstorable row used to take the other + # forty-nine with it. A batch is not a transaction as far as + # the fleet is concerned: it is fifty independent facts, and + # losing all of them because one is malformed is the worst + # available outcome. + rejected += 1 + if first_error is None: + first_error = f"{type(row_error).__name__}: {row_error}" + + # Take the fingerprint back out. + # + # It was written a few lines above, before the row it stands + # for existed. While a failure rolled the whole batch back + # that was harmless - the fingerprint went with it. Now that a + # batch commits despite a rejected row, leaving it behind + # marks the row as already-seen for ever: it is skipped as a + # duplicate on every future attempt, so it can never be stored + # even once the reason it failed is fixed. + # + # `registry_logs` showed exactly this - three fingerprints + # held against a table that did not exist, for three rows that + # would have stayed unstorable after the table was created. + if table in DEDUP_TABLES: + try: + cursor.execute( + "DELETE FROM ingest_fingerprint " + "WHERE table_name=%s AND fp=%s", (table, fp)) + except Exception as cleanup_error: + print(f"[!] {agent}/{table}: could not release the " + f"fingerprint for a rejected row: " + f"{cleanup_error}", flush=True) + continue for _alert in correlate_across_hosts(agent, table, item, cursor): _spawn(publish_to_ai_queue(agent, "events_alert", _alert), @@ -799,9 +915,28 @@ async def insert_data(agent: str, table: str, data: list, public_ip: str = None, label=f"index {agent}/{table}") conn.commit() + if rejected: + # Reported, not hidden behind a successful-looking receipt. The + # agent does not retry these - a row the server cannot store will + # not become storable on the next attempt - so this line is the + # only trace they ever leave. + print(f"[!] {agent}/{table}: stored {stored}, rejected {rejected} " + f"({first_error})", flush=True) + return {"stored": stored, "duplicates": duplicates, + "rejected": rejected, "error": None} except Exception as e: - if debug: - print(f"[!] Data insertion error: {e}") + # Printed unconditionally, not behind `debug`. A batch that fails to + # store is the single most important thing this process can say, and + # it spent that failure inside an `if debug` that is off in + # production. + print(f"[!] {agent}/{table}: data insertion failed: " + f"{type(e).__name__}: {e}", flush=True) + try: + conn.rollback() + except Exception: + pass + return {"stored": 0, "duplicates": 0, "rejected": 0, + "error": f"{type(e).__name__}: {e}"} finally: cursor.close() conn.close() @@ -921,12 +1056,30 @@ def observed_peer_ip(writer) -> str | None: return host -async def handle_client(reader, writer): +async def handle_client(reader, writer, encrypted: bool = True): + """One batch from one agent. + + `encrypted` says which listener the connection arrived on. It is recorded + rather than acted on: refusing here would make a legacy agent's telemetry + vanish with no explanation anywhere, which is the failure mode this whole + module keeps being bitten by. INGEST_TLS_REQUIRED refuses it earlier, by + not opening the plaintext port at all - a connection refused is a symptom + an operator can see. + """ try: raw_len = await recv_all(reader, 4) (agent_name_len,) = struct.unpack('!I', raw_len) agent_name = (await recv_all(reader, agent_name_len)).decode('utf-8') + if not encrypted and agent_name not in PLAINTEXT_AGENTS: + PLAINTEXT_AGENTS.add(agent_name) + print(f"[!] {agent_name} is sending telemetry in the clear on " + f"port {SERVER_PORT}. Its logs, hostnames and process " + f"names cross the network unencrypted. Rebuild the agent " + f"against an https:// server URL to move it to " + f"{INGEST_TLS_PORT}; set INGEST_TLS_REQUIRED=1 to stop " + f"accepting this once the fleet has moved.", flush=True) + raw_ip_len = await recv_all(reader, 4) (ip_len,) = struct.unpack('!I', raw_ip_len) claimed_ip = (await recv_all(reader, ip_len)).decode('utf-8') @@ -962,8 +1115,9 @@ async def handle_client(reader, writer): print(f"[ERROR] JSON decode failed from {agent_name}@{public_ip}: {e}") return - await insert_data(agent_name, fname.replace(".json", ""), data, public_ip, os_info, - reported_ip=claimed_ip) + receipt = await insert_data(agent_name, fname.replace(".json", ""), data, + public_ip, os_info, reported_ip=claimed_ip) + await _send_receipt(writer, receipt) if debug: print(f"[INFO] Data received from {agent_name}@{public_ip} ({os_info}) - File: {fname}, Size: {fsize} bytes") @@ -975,16 +1129,117 @@ async def handle_client(reader, writer): await writer.wait_closed() +async def _send_receipt(writer, receipt) -> None: + """Tell the agent what became of the batch it just sent. + + The ingest protocol had no reply at all, which is why the agent marked + rows sent on `sendall` returning - the strongest thing it could know, and + much weaker than it read. This adds the only frame that lets the agent + distinguish "stored" from "accepted by a socket". + + Appended rather than versioned, and safe in both directions. An agent + built before this reads nothing and closes; the write lands in a socket + buffer nobody drains and is discarded when the connection goes, so the + failure mode for an old fleet is exactly the behaviour it has today. + """ + if receipt is None: + return + try: + blob = json.dumps(receipt).encode("utf-8") + writer.write(struct.pack("!I", len(blob)) + blob) + await writer.drain() + except Exception as e: + # An agent that hung up before reading is the ordinary case for any + # build older than this frame, so this is not worth a line unless + # somebody is debugging. + if debug: + print(f"[INFO] receipt not delivered: {type(e).__name__}: {e}") + + +def _ingest_tls_context(): + """The server-side TLS context for the ingest listener, or None. + + Shares `core.tls` with the console rather than growing a second pair of + certificate settings. One identity for the deployment is also the only + arrangement an agent can be told to trust in one step. + + This process reads the certificate and never creates one. `app` owns + generation, the way it already owns the Fernet key, and mounts the + directory read-only here. Two processes generating independently would + each end up holding a certificate the other does not, and an agent that + verified the console would then fail against ingest for no visible + reason - a host that goes quiet, which is the exact symptom this codebase + keeps chasing. + """ + from core import tls as product_tls + + if not product_tls.tls_enabled(): + return None + + config = product_tls.existing() + if not config: + # Not an error worth a traceback: on a first boot this process + # usually wins the race with `app` by a few seconds. `restart: always` + # brings it back, and the message says what it is waiting for rather + # than looking like a broken configuration. + raise RuntimeError( + "no certificate at certs/server.crt yet. `app` generates it on " + "first boot and this process reads it; if `app` is running and " + "this persists, check that both mount the same certs volume." + ) + + context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) + context.load_cert_chain(certfile=config["cert"], keyfile=config["key"]) + return context + + async def main(): - server = await asyncio.start_server( - handle_client, SERVER_IP, SERVER_PORT, - reuse_address=True, reuse_port=False - ) - addr = server.sockets[0].getsockname() - print(f"[*] TCP ingest server listening on: {addr}") + servers = [] - async with server: - await server.serve_forever() + tls_context = None + try: + tls_context = _ingest_tls_context() + except Exception as e: + # Same rule as the console: asking for TLS and not getting it is a + # startup failure. Telemetry is the more sensitive of the two streams + # - it carries the process names, paths and log lines of every host - + # so falling back to the plaintext port would be the worse half of + # the deployment silently continuing. + print(f"[!] ingest TLS was requested and cannot be served: {e}", flush=True) + raise SystemExit(2) + + if tls_context is not None: + servers.append(await asyncio.start_server( + functools.partial(handle_client, encrypted=True), + SERVER_IP, INGEST_TLS_PORT, ssl=tls_context, + reuse_address=True, reuse_port=False, + )) + print(f"[*] TLS ingest listening on {SERVER_IP}:{INGEST_TLS_PORT}", flush=True) + elif INGEST_TLS_REQUIRED: + print("[!] INGEST_TLS_REQUIRED=1 but TLS_ENABLED is not set, so there " + "is no encrypted listener to require. Set TLS_ENABLED=1.", + flush=True) + raise SystemExit(2) + + if INGEST_TLS_REQUIRED: + print(f"[*] Plaintext ingest on {SERVER_PORT} is closed " + f"(INGEST_TLS_REQUIRED=1). Agents that have not been rebuilt " + f"will be refused rather than silently accepted.", flush=True) + else: + servers.append(await asyncio.start_server( + functools.partial(handle_client, encrypted=False), + SERVER_IP, SERVER_PORT, + reuse_address=True, reuse_port=False, + )) + warning = ("in the clear" if tls_context is None else + "in the clear, for agents not yet rebuilt") + print(f"[*] TCP ingest listening on {SERVER_IP}:{SERVER_PORT} ({warning})", + flush=True) + + async with contextlib.AsyncExitStack() as stack: + for server in servers: + await stack.enter_async_context(server) + await asyncio.gather(*(s.serve_forever() for s in servers)) if __name__ == "__main__": asyncio.run(main()) diff --git a/tests/test_clear_table_dedup.py b/tests/test_clear_table_dedup.py new file mode 100644 index 0000000..e346c09 --- /dev/null +++ b/tests/test_clear_table_dedup.py @@ -0,0 +1,111 @@ +"""Clearing a table has to clear what it has already seen. + +`//clear/` ran a bare `TRUNCATE TABLE siem_events` and left +`ingest_fingerprint` untouched. That is not untidiness - it permanently blinds +the table. + +Ingest writes a fingerprint per row and skips any row whose fingerprint it +already holds. Emptying the rows without emptying the fingerprints means every +future row is skipped as a duplicate of a row that no longer exists. So: + + the table stays empty for ever + the server tells the agent it already holds the data + the agent marks those rows sent and never offers them again + every layer reports success + +Seen live in the telemetry health view, which is the only place the two counts +sit side by side: an agent had shipped 507 new `siem_events` rows into a table +holding zero, with `unsent: 0` on its side. One click on Clear had turned the +most important table on the console into a silence indistinguishable from a +host with nothing to report. + +Both clearable tables are in `DEDUP_TABLES`, so this applies to both of them, +and the delayed route is the same TRUNCATE written a second time - which is +how the two came to disagree. +""" +from __future__ import annotations + +import ast +import pathlib + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +APP = ROOT / "app.py" +SERVER = ROOT / "server.py" + + +def _function(name: str) -> ast.AST: + tree = ast.parse(APP.read_text(encoding="utf-8")) + return next(n for n in ast.walk(tree) + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)) + and n.name == name) + + +def _dedup_tables() -> set: + tree = ast.parse(SERVER.read_text(encoding="utf-8")) + for node in tree.body: + if isinstance(node, ast.Assign) and any( + getattr(t, "id", "") == "DEDUP_TABLES" for t in node.targets): + return set(ast.literal_eval(node.value)) + raise AssertionError("DEDUP_TABLES is not a literal set in server.py") + + +def _clearable() -> list: + tree = ast.parse(APP.read_text(encoding="utf-8")) + for node in tree.body: + if isinstance(node, ast.Assign) and any( + getattr(t, "id", "") == "CLEARABLE_TABLES" for t in node.targets): + return list(ast.literal_eval(node.value)) + raise AssertionError("CLEARABLE_TABLES is not a literal in app.py") + + +def test_clearing_forgets_the_fingerprints_too(): + """The fix. Without it the TRUNCATE is a one-way door.""" + body = ast.unparse(_function("_clear_one_table")) + assert "TRUNCATE TABLE" in body + assert "DELETE FROM ingest_fingerprint" in body + + +def test_only_this_tables_fingerprints_are_dropped(): + """The fingerprints for every other table describe rows that are still + there. Dropping those would re-admit every duplicate the ingest path + exists to reject - a quieter failure than the one being fixed, and a + harder one to notice.""" + body = ast.unparse(_function("_clear_one_table")) + delete = body[body.index("DELETE FROM ingest_fingerprint"):] + assert "table_name=%s" in delete or "table_name = %s" in delete, \ + "the clear drops every table's fingerprints, not just this one's" + + +@pytest.mark.parametrize("route", ["clear_table", "clear_table_delayed"]) +def test_neither_route_truncates_on_its_own(route): + """There were two copies of the same TRUNCATE and only one of them would + ever have been fixed.""" + body = ast.unparse(_function(route)) + assert "TRUNCATE" not in body, ( + f"{route} still empties the table itself; it has to go through " + f"_clear_one_table or it will drift again" + ) + assert "_clear_one_table" in body + + +def test_the_two_routes_agree_on_what_may_be_cleared(): + """They each carried their own `allowed_tables` list. Two lists that have + to match, with nothing making them match.""" + for route in ("clear_table", "clear_table_delayed"): + body = ast.unparse(_function(route)) + assert "allowed_tables = " not in body, \ + f"{route} keeps its own copy of the allow-list" + assert "CLEARABLE_TABLES" in body + + +@pytest.mark.parametrize("table", _clearable()) +def test_every_clearable_table_is_actually_deduplicated(table): + """The reason the fingerprint delete is needed at all. If a table ever + becomes clearable *without* being deduplicated this test says so, and the + extra DELETE is then harmless rather than load-bearing.""" + assert table in _dedup_tables(), ( + f"{table} can be cleared but is not deduplicated - check whether " + f"_clear_one_table still describes what happens to it" + ) diff --git a/tests/test_collector_survival.py b/tests/test_collector_survival.py new file mode 100644 index 0000000..d119267 --- /dev/null +++ b/tests/test_collector_survival.py @@ -0,0 +1,155 @@ +"""A collector that fails must report and retry, never stop existing. + +The Ports tab was empty on a host that had been up for days, and +`portscan_result sent` appeared zero times in its log. Not once - so the +scanner had not merely failed, it had stopped running entirely. + +`main_async` ended its error paths with `sys.exit(1)`. SystemExit inherits +from BaseException, so `periodic_wrapped`'s `except Exception` never saw it, +and in a thread Python discards it without a word. The first failed scan +ended that thread for the life of the agent. `info_collector` did the same on +two paths. + +The other half of the same evening: real-time FIM watched all of `C:\\Users` +with no exclusions and recorded every `.py`, `.exe` and `.dll` written +underneath - temp directories, browser caches, pip installs. Thousands of +"integrity violation" rows an hour, a local queue that never drained, and +`network_connections` and `hardware_inventory` waiting behind them. The +exclusions were not missing: `conf/file_scan.yaml` has the key, the console +pushes it, `check_permissions` honours it, and `fim.py` never read the file. + +Both are the same failure to the operator: a tab that is empty or full of +noise, and nothing anywhere saying which. +""" +from __future__ import annotations + +import ast +import pathlib + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +AGENT = ROOT / "Sentora" +MAIN = AGENT / "main.py" +FIM = AGENT / "modules" / "fim.py" + +COLLECTOR_MODULES = [ + AGENT / "modules" / "portscanner" / "portscanner.py", + AGENT / "modules" / "find_vulns" / "info_collector.py", + AGENT / "modules" / "edr_enforcer.py", + AGENT / "modules" / "inventory.py", + AGENT / "modules" / "fim.py", +] + + +def _code_only(path: pathlib.Path) -> str: + """Source without comments or docstrings. + + Each of these files explains the `sys.exit` it removed, and matching the + text finds the explanation. + """ + tree = ast.parse(path.read_text(encoding="utf-8")) + for node in ast.walk(tree): + if not isinstance(node, (ast.Module, ast.FunctionDef, + ast.AsyncFunctionDef, ast.ClassDef)): + continue + body = node.body + if (body and isinstance(body[0], ast.Expr) + and isinstance(body[0].value, ast.Constant) + and isinstance(body[0].value.value, str)): + del body[0] + return ast.unparse(tree) + + +@pytest.mark.parametrize("path", COLLECTOR_MODULES, ids=lambda p: p.stem) +def test_no_collector_calls_sys_exit(path): + """In a thread, SystemExit ends that thread and Python says nothing.""" + assert "sys.exit" not in _code_only(path), ( + f"{path.name} ends a code path with sys.exit; under `periodic_wrapped` " + f"that kills the collector permanently and silently" + ) + + +def test_the_runner_survives_one_anyway(): + """Both offenders are fixed, so this guards the next one rather than the + last one - which is the only useful place for a guard like this.""" + tree = ast.parse(MAIN.read_text(encoding="utf-8")) + fn = next(n for n in tree.body + if isinstance(n, ast.FunctionDef) and n.name == "periodic_wrapped") + handlers = [ast.unparse(h.type) for h in ast.walk(fn) + if isinstance(h, ast.ExceptHandler) and h.type] + assert "SystemExit" in handlers, \ + "a library calling sys.exit still ends the collector" + + +# -------------------------------------------------------------------------- +# FIM noise +# -------------------------------------------------------------------------- + +def test_fim_reads_the_config_it_is_given(): + """`conf/file_scan.yaml` is pushed from the console. A setting that is + configurable, documented, and connected to nothing is worse than one that + does not exist - somebody sets it and believes it took.""" + code = _code_only(FIM) + assert "file_scan" in code + assert "exclude_dirs" in code + + +def test_fim_excludes_the_directories_that_churn(): + """Watching `C:\\Users` at all is only defensible with these. The + interesting things under it - documents, startup folders, ssh keys - are + not in a temp directory or a browser cache.""" + source = FIM.read_text(encoding="utf-8") + for pattern in ("AppData\\\\Local\\\\Temp", "node_modules", "$Recycle.Bin"): + assert pattern in source, pattern + + +def test_the_exclusion_check_happens_before_the_hash(): + """Hashing a file costs a read of it, and in a temp directory there are + thousands a minute.""" + tree = ast.parse(FIM.read_text(encoding="utf-8")) + fn = next(n for n in ast.walk(tree) + if isinstance(n, ast.FunctionDef) and n.name == "process") + code = ast.unparse(fn) + assert "is_excluded" in code + assert code.index("is_excluded") < code.index("calculate_sha256") + + +def test_configured_exclusions_add_to_the_defaults(): + """Somebody adding one directory should not silently un-exclude every + temp folder.""" + code = _code_only(FIM) + assert "excludes.append" in code, \ + "the configured list replaces the defaults instead of extending them" + + +@pytest.mark.parametrize("path,excluded", [ + ("C:\\Users\\pc\\AppData\\Local\\Temp\\x\\build.py", True), + ("C:\\Users\\pc\\Documents\\notes.py", False), + ("C:\\Users\\pc\\project\\node_modules\\pkg\\index.js", True), + ("C:\\Users\\pc\\.ssh\\authorized_keys", False), +]) +def test_the_matcher_separates_noise_from_signal(path, excluded): + """Compiled and run, because a pattern list that matches nothing looks + exactly like a pattern list that is working.""" + import fnmatch + + tree = ast.parse(FIM.read_text(encoding="utf-8")) + fn = next(n for n in tree.body + if isinstance(n, ast.FunctionDef) and n.name == "is_excluded") + defaults = next(n for n in tree.body + if isinstance(n, ast.Assign) + and getattr(n.targets[0], "id", "") == "DEFAULT_EXCLUDES") + namespace: dict = {"fnmatch": fnmatch} + exec(compile(ast.Module(body=[defaults, fn], type_ignores=[]), + str(FIM), "exec"), namespace) + patterns = namespace["DEFAULT_EXCLUDES"]["windows"] + assert namespace["is_excluded"](path, patterns) is excluded, path + + +def test_the_suppressed_count_is_reported(): + """Without it, the only evidence the exclusions are working is the absence + of events - which is also what a broken watcher produces.""" + code = _code_only(FIM) + assert "suppressed" in code + assert "suppressed {count} events in excluded directories" in code diff --git a/tests/test_design_system.py b/tests/test_design_system.py index d5d5850..7415650 100644 --- a/tests/test_design_system.py +++ b/tests/test_design_system.py @@ -231,6 +231,90 @@ def test_the_dashboard_says_whether_a_command_would_reach_a_host(): assert "channel_connected" in dashboard +@pytest.mark.parametrize("page", PAGES, ids=lambda p: p.stem) +def test_no_page_reaches_past_the_chart_wrappers(page): + """Recharts styles itself with props, not CSS. A page importing it + directly draws a white grid and a light tooltip on a black ground, and + picks its own series colours - which is how the console came to look like + three products the first time.""" + source = page.read_text(encoding="utf-8") + assert "from 'recharts'" not in source, ( + f"{page.name} imports recharts directly; use components/ui/charts so " + f"the palette and the axes stay in one place" + ) + + +def test_the_charted_pages_use_the_shared_palette(): + """A chart is only consistent if its colours come from the tokens. Pages + that hardcode a hex are how a series colour comes to mean two things.""" + charted = [p for p in PAGES + if "components/ui/charts" in p.read_text(encoding="utf-8")] + assert len(charted) >= 4, ( + f"only {len(charted)} pages use the shared charts; the dashboard's " + f"visual language has not reached the rest of the console" + ) + + +#: Pages whose data is a distribution rather than a record. A table is the +#: right way to read one row; it is the wrong way to see that four hundred of +#: them are the same package, or that every failure belongs to one playbook. +CHART_WORTHY = [ + "Dashboard", "GlobalAlerts", "Agents", "AttackCoverage", + "FileIntegrity", "SoarHub", "ThreatIntel", "AgentDetail", + "Assets", "AuditLogs", "LoginLogs", +] + + +@pytest.mark.parametrize("stem", CHART_WORTHY) +def test_the_pages_that_answer_a_distribution_draw_one(stem): + """The console had exactly one page with charts and twenty-three with + tables, so the dashboard looked like a different product from everything + it linked to. This is the list of pages where the shape of the data is + the answer, not a decoration on it.""" + page = FRONTEND / "pages" / f"{stem}.tsx" + source = page.read_text(encoding="utf-8") + assert "components/ui/charts" in source, ( + f"{stem} answers a 'how much of what' question and still only has a " + f"table" + ) + + +@pytest.mark.parametrize("stem", CHART_WORTHY) +def test_the_chart_import_is_actually_rendered(stem): + """Importing a wrapper and never rendering it makes the page pass the + test above while looking exactly as it did before. Grep for the import is + not evidence of a chart; a `data=` prop on a wrapper is.""" + source = (FRONTEND / "pages" / f"{stem}.tsx").read_text(encoding="utf-8") + wrappers = ("CategoryBars", "ShareDonut", "TrendChart", "SeriesLines") + rendered = [w for w in wrappers if re.search(rf"<{w}", source)] + assert rendered, f"{stem} imports the chart wrappers but renders none" + + for wrapper in rendered: + assert re.search(rf"<{wrapper}[^>]*data=", source, re.S), ( + f"{stem} renders <{wrapper}> with no data prop" + ) + + +def test_the_wrappers_let_a_caller_colour_a_state(): + """Most series should take the neutral palette. But when the category *is* + a severity or an outcome, drawing "failed" in whatever colour the third + slice happened to get is the chart lying about its own data.""" + source = CHARTS.read_text(encoding="utf-8") + for wrapper in ("CategoryBars", "ShareDonut"): + block = source[source.index(f"export function {wrapper}"):] + block = block[:block.index(chr(10) + "}" + chr(10))] + assert "colorFor" in block, f"{wrapper} cannot express a semantic colour" + + +def test_the_line_chart_pins_its_axis(): + """A percentage axis that rescales to its data draws 4% and 40% as the + same picture, which is the one thing a resource chart must not do.""" + source = CHARTS.read_text(encoding="utf-8") + lines = source[source.index("export function SeriesLines"): + source.index("export function ShareDonut")] + assert "domain ?? [0, 100]" in lines + + def test_the_attack_chain_is_reachable_from_a_host(): """It answers a question about one machine, so it is reached from that machine rather than from the nav.""" diff --git a/tests/test_encrypted_column_widths.py b/tests/test_encrypted_column_widths.py index 5fefb1c..c870547 100644 --- a/tests/test_encrypted_column_widths.py +++ b/tests/test_encrypted_column_widths.py @@ -88,6 +88,16 @@ def _column_type(table: str, column: str): def test_an_encrypted_column_can_hold_its_ciphertext(table, column): declared = _column_type(table, column) if declared is None: + # A skip here used to cover a real hole. Nine of these skipped for + # months, and the reason was not "that column is spelled differently" + # - it was that `registry_logs`, `process_events` and `security_audit` + # had no server-side table at all, so there was no width to check + # because there was nowhere to put the data. The suite reported + # "nothing to check here" about three tables the agent was filling. + # + # `test_every_ingestible_table_has_somewhere_to_land` below is the + # test that catches that directly; this one stays a skip so a column + # renamed on one side is not reported twice. pytest.skip(f"{table}.{column} is not in db/init.sql") kind, width = declared diff --git a/tests/test_ingest_acknowledgement.py b/tests/test_ingest_acknowledgement.py new file mode 100644 index 0000000..358ea59 --- /dev/null +++ b/tests/test_ingest_acknowledgement.py @@ -0,0 +1,309 @@ +"""The agent may only call a batch sent once the server says it stored it. + +This closes the last of the three failures in `telemetry_health`'s docstring, +and the one that made the other two invisible: + + a collector raised on its first line, so the four after it never ran + the server accepted a batch and discarded every row of it + the agent marked rows sent as soon as `sendall` returned + +`sendall` returning means the bytes reached the operating system's socket +buffer. It says nothing about the server, and the ingest protocol had no reply +frame, so the agent could not have known better - which is how four tables +stayed permanently empty for days while `[+] network_connections sent (50 +rows)` scrolled past every cycle. The rows were marked sent and dropped from +the retry queue on the strength of a socket write. + +The protocol now carries a receipt. Three cases have to stay distinguishable, +and collapsing any two of them recreates the bug: + + a receipt saying rows were stored -> mark sent + a receipt carrying an error -> keep the rows, retry, say why + no receipt at all (an older server) -> mark sent, and say it is belief + +The third is the subtle one. "The server said nothing" is not "the server +stored nothing", and treating it as failure would make every agent pointed at +an older server retry the same rows for ever. +""" +from __future__ import annotations + +import ast +import json +import pathlib +import socket +import struct +import threading + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +MAIN = ROOT / "Sentora" / "main.py" +SERVER = ROOT / "server.py" + + +def _compiled(path: pathlib.Path, *names: str, extra: dict | None = None): + """Named functions, compiled with the module constants they refer to.""" + tree = ast.parse(path.read_text(encoding="utf-8")) + wanted = [n for n in tree.body + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)) + and n.name in names] + assert len(wanted) == len(names), f"missing one of {names} in {path.name}" + + referenced = set() + for fn in wanted: + referenced |= {n.id for n in ast.walk(fn) if isinstance(n, ast.Name)} + constants = [n for n in tree.body + if isinstance(n, ast.Assign) + and any(getattr(t, "id", "") in referenced for t in n.targets)] + + namespace: dict = {"socket": socket, "struct": struct, "json": json} + namespace.update(extra or {}) + exec(compile(ast.Module(body=[*constants, *wanted], type_ignores=[]), + str(path), "exec"), namespace) + return [namespace[n] for n in names] + + +@pytest.fixture +def pair(): + """A connected socket pair, so the frame is tested on a real socket. + + `socket.socketpair()` is AF_UNIX on POSIX and a loopback TCP pair on + Windows; both give the partial reads this code has to survive. + """ + a, b = socket.socketpair() + a.settimeout(5) + b.settimeout(5) + yield a, b + a.close() + b.close() + + +# -------------------------------------------------------------------------- +# The frame itself +# -------------------------------------------------------------------------- + +def test_a_receipt_survives_the_wire(pair): + server_end, agent_end = pair + read_receipt, _ = _compiled(MAIN, "_read_receipt", "_recv_exactly") + + body = json.dumps({"stored": 12, "duplicates": 3, "error": None}).encode() + server_end.sendall(struct.pack("!I", len(body)) + body) + + assert read_receipt(agent_end) == {"stored": 12, "duplicates": 3, "error": None} + + +def test_a_server_that_says_nothing_reads_as_no_receipt(pair): + """An agent built after the frame, talking to a server built before it. + The server stores the batch and closes; that must not read as data loss.""" + server_end, agent_end = pair + read_receipt, _ = _compiled(MAIN, "_read_receipt", "_recv_exactly") + + server_end.close() + assert read_receipt(agent_end) is None + + +def test_a_truncated_receipt_reads_as_no_receipt(pair): + """Half a frame is not half an answer. The batch is already stored or + already lost by this point, and guessing which from a broken frame would + be worse than saying it went unacknowledged.""" + server_end, agent_end = pair + read_receipt, _ = _compiled(MAIN, "_read_receipt", "_recv_exactly") + + server_end.sendall(struct.pack("!I", 4096) + b'{"stored"') + server_end.close() + assert read_receipt(agent_end) is None + + +def test_an_absurd_length_is_refused_without_allocating(pair): + """The length comes off the network. Allocating on it is how a reply + frame becomes a denial of service against the agent.""" + server_end, agent_end = pair + read_receipt, _ = _compiled(MAIN, "_read_receipt", "_recv_exactly") + + server_end.sendall(struct.pack("!I", 0xFFFFFFFF)) + assert read_receipt(agent_end) is None + + +def test_a_frame_split_across_packets_is_reassembled(pair): + """TCP is a stream. A receipt arriving in two reads must not be treated + as a truncated one.""" + server_end, agent_end = pair + read_receipt, _ = _compiled(MAIN, "_read_receipt", "_recv_exactly") + + body = json.dumps({"stored": 1, "duplicates": 0, "error": None}).encode() + + def dribble(): + header = struct.pack("!I", len(body)) + server_end.sendall(header[:2]) + server_end.sendall(header[2:]) + for i in range(0, len(body), 3): + server_end.sendall(body[i:i + 3]) + + thread = threading.Thread(target=dribble) + thread.start() + try: + assert read_receipt(agent_end) == {"stored": 1, "duplicates": 0, "error": None} + finally: + thread.join() + + +# -------------------------------------------------------------------------- +# What the agent does with it +# -------------------------------------------------------------------------- + +def _send_table_body() -> str: + """`send_table`, comments and docstrings dropped. + + The comments in it quote the old behaviour they warn about, so matching + the raw source finds the warning and reads it as the code. That has caught + several tests in this suite already. + """ + tree = ast.parse(MAIN.read_text(encoding="utf-8")) + fn = next(n for n in tree.body + if isinstance(n, ast.FunctionDef) and n.name == "send_table") + return ast.unparse(fn) + + +def test_an_error_receipt_keeps_the_rows(): + """The entire point of the frame. A server that took the bytes and stored + none of them used to be indistinguishable from one that stored all of + them, and the rows left the retry queue either way.""" + body = _send_table_body() + assert "mark_sent" in body + + guard = body[:body.index("mark_sent")] + assert "receipt" in guard, "mark_sent runs before the receipt is considered" + assert "return" in guard, "an error receipt does not stop the rows being marked sent" + + +def test_the_receipt_is_read_before_the_socket_closes(): + """`with _ingest_socket() as s` closes at the end of the block. A read + placed after it would always return nothing, and every batch would report + as unacknowledged against a server that acknowledged it.""" + body = _send_table_body() + lines = [line.strip() for line in body.splitlines()] + open_at = next(i for i, line in enumerate(lines) if "_ingest_socket()" in line) + read_at = next(i for i, line in enumerate(lines) if "_read_receipt" in line) + decide_at = next(i for i, line in enumerate(lines) if line.startswith("if receipt")) + assert open_at < read_at < decide_at + + +def test_a_missing_receipt_still_marks_the_batch_sent(): + """An agent pointed at a server older than the frame has to keep working. + Treating silence as failure would make it retry the same rows for ever.""" + body = _send_table_body() + # The failure path is entered on an error *in* a receipt, never on its + # absence. + assert "receipt is not None" in body + + +def test_the_agent_says_which_kind_of_success_it_had(): + """'sent (50 rows)' meant the same weak thing for months. Where the server + confirms a count the log should show it, and where it does not the log has + to say so rather than read identically.""" + body = _send_table_body() + assert "unacknowledged" in body + + +# -------------------------------------------------------------------------- +# The server half +# -------------------------------------------------------------------------- + +def _insert_data() -> ast.AsyncFunctionDef: + tree = ast.parse(SERVER.read_text(encoding="utf-8")) + return next(n for n in tree.body + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)) + and n.name == "insert_data") + + +def test_insert_data_reports_what_it_stored(): + """It returned None and swallowed every insertion failure into a line + behind `if debug`, which is off in production. That is how a batch of + fifty rows failing 1406 on every INSERT was reported to nobody.""" + fn = _insert_data() + body = ast.unparse(fn) + + assert "'stored': stored" in body or '"stored": stored' in body + returns = [n for n in ast.walk(fn) if isinstance(n, ast.Return)] + assert returns, "insert_data still returns nothing" + assert all(n.value is not None for n in returns), \ + "a bare `return` leaves the caller unable to tell success from a skip" + + +def test_a_failed_batch_is_reported_outside_debug(): + """The failure that mattered spent days inside `if debug`.""" + fn = _insert_data() + reporting = [h for h in ast.walk(fn) + if isinstance(h, ast.ExceptHandler) + and "data insertion failed" in ast.unparse(h)] + assert reporting, "the insertion failure message is gone" + + for handler in reporting: + for node in ast.walk(handler): + if isinstance(node, ast.If) and "debug" in ast.unparse(node.test): + pytest.fail("the insertion failure is reported only when debug is on") + + +def test_the_receipt_write_cannot_break_an_older_fleet(): + """An agent built before the frame closes the connection without reading. + The write then fails, and that has to be ordinary rather than an error + that makes every legacy batch look broken.""" + tree = ast.parse(SERVER.read_text(encoding="utf-8")) + fn = next(n for n in tree.body + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)) + and n.name == "_send_receipt") + body = ast.unparse(fn) + assert "try:" in body + assert "except Exception" in body + + +# -------------------------------------------------------------------------- +# One bad row must not take the batch with it +# -------------------------------------------------------------------------- + +def test_a_batch_is_fifty_facts_not_one_transaction(): + """A single unstorable row used to roll back the other forty-nine. + + Found by the change above, within seconds of it shipping: making the + insertion failure visible turned up + + docker_containers: Duplicate entry '3d25565c...' for key + 'docker_containers.uniq_container' + + on a live host. The agent appends a row per container per collection + cycle and ships fifty at a time, so one batch routinely carries the same + container several times over. `docker_containers` has UNIQUE(container_id), + the second copy raised 1062, and the whole batch was discarded - while the + agent logged `docker_containers sent (8 rows)` every cycle. + """ + fn = _insert_data() + body = ast.unparse(fn) + assert "rejected += 1" in body, \ + "a failing row still aborts the whole batch" + + # The per-row handler has to be inside the loop, not around it. + loop = next(n for n in ast.walk(fn) + if isinstance(n, ast.For) and "item" in ast.unparse(n.target)) + handlers = [h for h in ast.walk(loop) if isinstance(h, ast.ExceptHandler)] + assert any("rejected" in ast.unparse(h) for h in handlers), \ + "nothing catches a single row's failure inside the loop" + + +def test_a_snapshot_takes_the_last_write(): + """A snapshot holds what is true now, so the same entity arriving twice in + one batch is the newer picture replacing the older - not a conflict.""" + body = ast.unparse(_insert_data()) + assert "ON DUPLICATE KEY UPDATE" in body + assert "SNAPSHOT_TABLES" in body + + +def test_rejected_rows_are_not_retried_for_ever(): + """The other side of reporting them. A row the server cannot store will + not become storable on the next attempt, so `error` - which is what makes + the agent keep a batch - must be reserved for a failure of the batch as a + whole.""" + body = ast.unparse(_insert_data()) + success = body[body.index("conn.commit()"):] + success = success[:success.index("except Exception")] + assert "'error': None" in success or '"error": None' in success, \ + "a batch with rejected rows reports an error and will be resent for ever" diff --git a/tests/test_ingest_modes.py b/tests/test_ingest_modes.py index 06c7f1f..0faf88d 100644 --- a/tests/test_ingest_modes.py +++ b/tests/test_ingest_modes.py @@ -99,7 +99,7 @@ def test_the_snapshot_list_is_not_written_out_twice(): @pytest.mark.parametrize("table", [ "network_connections", "hardware_inventory", "docker_containers", - "critical_files", "resource_usage", "disk_usage", + "resource_usage", "disk_usage", ]) def test_current_state_tables_are_snapshots(table): """These describe what is true on the host right now. The agent re-sends @@ -111,6 +111,14 @@ def test_current_state_tables_are_snapshots(table): @pytest.mark.parametrize("table", [ "siem_events", "events_alert", "fim_data", "packages", "vulnerabilities_report", "portscan_result", + # `critical_files` was in the snapshot list above, and this line is the + # correction. `check_permissions` writes a row only when a file's owner, + # group or mode has changed - it reports deltas, not the current picture - + # so DELETE-then-insert stored the handful of files that had changed and + # discarded the twelve hundred that had not. After a server-side database + # reset it stayed empty for ever: nothing new to send, and a sent row is + # never re-offered. + "critical_files", ]) def test_event_history_tables_are_deduplicated(table): """These accumulate. Emptying one on every batch would discard the @@ -118,6 +126,50 @@ def test_event_history_tables_are_deduplicated(table): assert table in _set_literal("DEDUP_TABLES"), table +# -------------------------------------------------------------------------- +# The signal that decides which mode a table belongs in +# -------------------------------------------------------------------------- + +AGENT_SCHEMA = ROOT / "Sentora" / "db" / "init.sql" + + +def _locally_deduplicated() -> set: + """Tables the agent keeps a UNIQUE index on `dup_fp` for. + + That index is a statement about the collector: it can only hold if the + collector never writes the same fact twice, which means it reports changes + rather than the current picture. A table with this index is a delta + stream, whatever anyone later assumes about it. + """ + import re + + return set(re.findall( + r"CREATE UNIQUE INDEX[^\n]*\n?\s*ON (\w+) \(dup_fp\)", + AGENT_SCHEMA.read_text(encoding="utf-8"))) + + +@pytest.mark.parametrize("table", sorted(_locally_deduplicated())) +def test_a_delta_stream_is_never_a_snapshot(table): + """The invariant that would have caught `critical_files` on the day it was + classified, without anyone having to read the collector. + + Snapshot semantics say "the agent re-sends the whole picture, so replace + what is here". A collector the agent deduplicates locally cannot re-send + the whole picture - the unique index is what stops it. Pairing the two + means every batch deletes everything the delta did not mention. + + Six of the seven tables with this index were already deduplicated on the + server. The seventh was the one showing zero rows against twelve hundred + on the endpoint. + """ + assert table not in _set_literal("SNAPSHOT_TABLES"), ( + f"the agent deduplicates {table} locally, so it only ever ships rows " + f"that changed - but the server empties the table on every batch and " + f"keeps only those. Everything unchanged is discarded, and after a " + f"server-side reset the table never refills." + ) + + def test_every_ingested_table_has_a_mode(): """A table in neither set accumulates without deduplication, which is almost never what anybody wanted - it is the default you get by diff --git a/tests/test_ingest_schema_coverage.py b/tests/test_ingest_schema_coverage.py new file mode 100644 index 0000000..1dabc21 --- /dev/null +++ b/tests/test_ingest_schema_coverage.py @@ -0,0 +1,135 @@ +"""Every table the server accepts has to be a table the server has. + +Found in the telemetry health view, which is the only place the two halves of +the count appear side by side: + + registry_logs NO TABLE held 7788 shipped 7788 on server — + +The agent had been collecting registry changes for weeks and shipping them +every cycle. `ALLOWED_TABLES` in server.py named `registry_logs`, so ingest +accepted the batch. `DEDUP_TABLES` named it, so a fingerprint was written for +every row. And `db/init.sql` — the per-agent schema the server applies before +the first insert — had no such table, so each row failed with + + 1146 (42S02): Table 'DESKTOP_EVS8H9J_3_db.registry_logs' doesn't exist + +Three tables were in this state: `registry_logs`, `process_events` and +`security_audit`. Nothing reported it. The failure was printed behind `if +debug`, the agent's log said `registry_logs sent (3 rows)` because `sendall` +had returned, and the console showed an empty table — which is exactly what a +host with no registry activity looks like. + +The lists were the whole bug. Four of them describe the same set of tables and +they were maintained by hand: + + ALLOWED_TABLES what ingest will accept + SNAPSHOT_TABLES replace the contents + DEDUP_TABLES skip what we have already seen + db/init.sql where it actually goes + +Any name present in the first three and absent from the fourth is data the +platform accepts and then throws away. +""" +from __future__ import annotations + +import ast +import pathlib +import re + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +SERVER = ROOT / "server.py" +SCHEMA = ROOT / "db" / "init.sql" +AGENT_SCHEMA = ROOT / "Sentora" / "db" / "init.sql" + + +def _set_literal(name: str) -> set: + tree = ast.parse(SERVER.read_text(encoding="utf-8")) + for node in tree.body: + if isinstance(node, ast.Assign) and any( + getattr(t, "id", "") == name for t in node.targets): + return set(ast.literal_eval(node.value)) + raise AssertionError(f"{name} is not a literal set in server.py") + + +def _schema_tables(path: pathlib.Path) -> set: + return set(re.findall( + r"CREATE TABLE IF NOT EXISTS [`\"]?(\w+)", path.read_text(encoding="utf-8"))) + + +ALLOWED = _set_literal("ALLOWED_TABLES") + + +@pytest.mark.parametrize("table", sorted(ALLOWED)) +def test_every_ingestible_table_has_somewhere_to_land(table): + """Accepting a batch for a table that does not exist is worse than + refusing it: refusing says so, and this discarded the rows quietly while + every layer above reported success.""" + assert table in _schema_tables(SCHEMA), ( + f"server.py accepts '{table}' and db/init.sql has no such table, so " + f"every row of it fails with 1146 and is discarded" + ) + + +@pytest.mark.parametrize("table", sorted(_set_literal("DEDUP_TABLES"))) +def test_nothing_is_deduplicated_into_a_table_that_does_not_exist(table): + """Worse than the plain case. The fingerprint is written before the row, + so a table that cannot accept the row still accumulates the marks that say + it has already been seen.""" + assert table in _schema_tables(SCHEMA), f"{table} is deduplicated into nothing" + + +@pytest.mark.parametrize("table", sorted(_set_literal("SNAPSHOT_TABLES"))) +def test_nothing_is_snapshotted_into_a_table_that_does_not_exist(table): + assert table in _schema_tables(SCHEMA), f"{table} is snapshotted into nothing" + + +def test_the_agent_collects_nothing_the_server_will_not_take(): + """The other direction, and the one that produced the original hole: a + collector shipping a table ingest has never heard of. + + Reported as a list rather than per-table, because the agent's schema + legitimately holds local-only tables - a queue the server never sees is + fine, a collector the server silently discards is not. + """ + agent_tables = _schema_tables(AGENT_SCHEMA) + shipped = {t for t in agent_tables if t in ALLOWED} + missing = sorted(t for t in shipped if t not in _schema_tables(SCHEMA)) + assert not missing, ( + f"the agent collects and ships {missing}, ingest accepts them, and " + f"there is nowhere to put them" + ) + + +def test_a_rejected_row_does_not_keep_its_fingerprint(): + """The fingerprint is written before the row it stands for. + + While a failed row rolled the whole batch back this was harmless — the + fingerprint went with it. Once a batch commits despite a rejected row, + a fingerprint left behind marks that row as already-seen for ever: it is + skipped as a duplicate on every future attempt, so fixing the reason it + failed does not bring it back. + + Visible in the live data at the time: three `registry_logs` fingerprints + held against a table that did not exist. + """ + tree = ast.parse(SERVER.read_text(encoding="utf-8")) + fn = next(n for n in tree.body + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)) + and n.name == "insert_data") + + # The per-row handler, found by the statement only it contains. + # + # Searching for "rejected" matched the batch-level handler first, because + # its return dict carries `'rejected': 0` - so this test failed against + # correct code while reading a handler thirty lines away. Anchor on an + # exact statement, not on a word that appears in both. + handler = next(h for h in ast.walk(fn) + if isinstance(h, ast.ExceptHandler) + and "rejected += 1" in ast.unparse(h)) + body = ast.unparse(handler) + assert "DELETE FROM ingest_fingerprint" in body, ( + "a row that could not be stored keeps the fingerprint that will stop " + "it ever being stored" + ) diff --git a/tests/test_ingest_tls.py b/tests/test_ingest_tls.py new file mode 100644 index 0000000..d63d62f --- /dev/null +++ b/tests/test_ingest_tls.py @@ -0,0 +1,363 @@ +"""Telemetry over TLS, and a migration that is visible rather than silent. + +Ingest carried every log line, process name, path and hostname the fleet +produces, in the clear, on a port published on all interfaces. The console got +TLS first because a login form is the obvious thing to protect; the larger and +more sensitive stream was the one nobody looked at. + +Two design decisions are worth pinning here, because both are easy to +"simplify" back into a bug. + +**Two ports, not a flag on one.** The existing protocol opens with a 4-byte +big-endian length, so its first byte is always 0x00; a TLS ClientHello opens +with 0x16. They are trivially distinguishable - but distinguishing them means +reading a byte that then cannot be handed back to the TLS layer, so one port +can serve one of them and not both. + +**The plaintext listener stays open until it is closed deliberately.** Cutting +straight to TLS would blackhole every agent built before this, and a host that +stopped reporting looks exactly like a host with nothing to report. That is +the failure this codebase keeps finding. So the plaintext port keeps working, +names the agents still using it, and `INGEST_TLS_REQUIRED=1` closes it once +the fleet has moved. +""" +from __future__ import annotations + +import ast +import pathlib +import socket +import ssl + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +SERVER = ROOT / "server.py" +MAIN = ROOT / "Sentora" / "main.py" + + +def _function(path: pathlib.Path, name: str) -> ast.AST: + tree = ast.parse(path.read_text(encoding="utf-8")) + return next(n for n in tree.body + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)) + and n.name == name) + + +# -------------------------------------------------------------------------- +# The server opens the right listeners +# -------------------------------------------------------------------------- + +def test_both_listeners_exist(): + body = ast.unparse(_function(SERVER, "main")) + assert "INGEST_TLS_PORT" in body + assert "SERVER_PORT" in body + assert "ssl=tls_context" in body, "the TLS listener is not actually encrypted" + + +def test_the_plaintext_listener_can_be_closed(): + """The migration has to be able to end. A permanent plaintext port is a + permanent way to bypass the encrypted one.""" + body = ast.unparse(_function(SERVER, "main")) + assert "INGEST_TLS_REQUIRED" in body + + +def test_requiring_tls_without_configuring_it_is_a_startup_failure(): + """`INGEST_TLS_REQUIRED=1` with `TLS_ENABLED` unset would close the + plaintext port and open nothing - an ingest service listening on nothing + at all, which from the fleet's side is indistinguishable from a quiet + week.""" + body = ast.unparse(_function(SERVER, "main")) + guard = body[body.index("INGEST_TLS_REQUIRED"):] + assert "SystemExit" in guard + + +def test_tls_that_cannot_be_served_is_not_downgraded(): + """The same rule the console follows. Telemetry is the more sensitive of + the two streams, so falling back to the plaintext port would be the worse + half of the deployment quietly continuing.""" + body = ast.unparse(_function(SERVER, "main")) + handler = next(h for h in ast.walk(_function(SERVER, "main")) + if isinstance(h, ast.ExceptHandler) + and "cannot be served" in ast.unparse(h)) + assert "SystemExit" in ast.unparse(handler) + assert "start_server" not in ast.unparse(handler) + assert body.count("SystemExit") >= 2 + + +def test_a_plaintext_agent_is_named_once_not_counted_forever(): + """'Nine agents are still in the clear' is a number an operator can act + on. Forty thousand connection warnings is a log nobody reads.""" + body = ast.unparse(_function(SERVER, "handle_client")) + assert "PLAINTEXT_AGENTS" in body + assert "in the clear" in body + + source = SERVER.read_text(encoding="utf-8") + assert "PLAINTEXT_AGENTS: set[str] = set()" in source, \ + "a set, so the same agent is reported once rather than per batch" + + +def test_the_server_does_not_refuse_plaintext_mid_connection(): + """Refusing here would drop the batch after the agent had already + committed to sending it, with the explanation on the wrong machine. + Closing the port is the honest refusal: a connection refused is a symptom + an operator can see.""" + fn = _function(SERVER, "handle_client") + body = ast.unparse(fn) + guard = body[body.index("PLAINTEXT_AGENTS"):] + head = guard[:guard.index("await insert_data")] if "await insert_data" in guard else guard + assert "return" not in head.split("print")[0] + + +# -------------------------------------------------------------------------- +# The agent picks the same transport without being told twice +# -------------------------------------------------------------------------- + +def _resolve(): + """`_resolve_ingest_transport`, compiled with the constants it names.""" + import os as _os + + tree = ast.parse(MAIN.read_text(encoding="utf-8")) + fn = next(n for n in tree.body + if isinstance(n, ast.FunctionDef) + and n.name == "_resolve_ingest_transport") + referenced = {n.id for n in ast.walk(fn) if isinstance(n, ast.Name)} + constants = [n for n in tree.body + if isinstance(n, ast.Assign) + and any(getattr(t, "id", "") in referenced for t in n.targets)] + + namespace: dict = {"os": _os} + exec(compile(ast.Module(body=[*constants, fn], type_ignores=[]), + str(MAIN), "exec"), namespace) + return namespace["_resolve_ingest_transport"] + + +class _Args: + def __init__(self, ingest_port=None, ca=None): + self.ingest_port = ingest_port + self.ca = ca + + +def test_https_implies_encrypted_telemetry(): + """Derived from `server_url`, the rule `modules/link.channel_url` already + uses for the control channel. Two settings that have to agree, with + nothing making them agree, would be wrong in exactly the deployment that + believed it was encrypted.""" + tls, port, _, host = _resolve()({"server_url": "https://soc.example.com:8000"}, _Args()) + assert tls is True + assert port == 5011 + assert host == "soc.example.com" + + +def test_http_stays_on_the_plaintext_port(): + """A lab pointed at http:// must keep working exactly as it did.""" + tls, port, _, _ = _resolve()({"server_url": "http://127.0.0.1:8000"}, _Args()) + assert tls is False + assert port == 5001 + + +def test_an_explicit_port_still_wins(): + """Somebody forwarding through a bastion has a real reason to override + this, so the derivation is a default rather than a rule.""" + _, port, _, _ = _resolve()({"server_url": "https://soc.example.com"}, + _Args(ingest_port=7001)) + assert port == 7001 + + +def test_the_certificate_name_comes_from_the_url_not_the_address(): + """`certs/generate_certs.py` puts a hostname in the SAN. Verifying an IP + against it fails with a message that reads like a certificate problem when + it is an addressing one.""" + _, _, _, host = _resolve()({"server_url": "https://soc.example.com:8000/"}, _Args()) + assert host == "soc.example.com" + + +def test_a_ca_path_that_does_not_exist_is_reported_not_ignored(capsys, tmp_path): + """Silently falling back to the system trust store gives a connection that + fails later for a reason that has nothing to do with the real cause.""" + _, _, ca, _ = _resolve()( + {"server_url": "https://soc.example.com", "server_ca": str(tmp_path / "absent.pem")}, + _Args()) + assert ca is None + assert "does not exist" in capsys.readouterr().out + + +def test_a_real_ca_path_is_kept(tmp_path): + bundle = tmp_path / "rootCA.crt" + bundle.write_text("-----BEGIN CERTIFICATE-----\n", encoding="utf-8") + _, _, ca, _ = _resolve()( + {"server_url": "https://soc.example.com", "server_ca": str(bundle)}, _Args()) + assert ca == str(bundle) + + +# -------------------------------------------------------------------------- +# The connection the agent actually makes +# -------------------------------------------------------------------------- + +def test_the_agent_verifies_the_server_certificate(): + """An unverified TLS connection is encrypted to whoever answered, which + against an attacker on the path is what not encrypting it would have + achieved. `create_default_context` verifies; the ways to turn that off are + what this guards against.""" + body = ast.unparse(_function(MAIN, "_ingest_socket")) + assert "create_default_context" in body + assert "server_hostname" in body + assert "CERT_NONE" not in body + assert "check_hostname = False" not in body + assert "_create_unverified_context" not in body + + +def test_the_ingest_socket_has_a_timeout(): + """An unbounded blocking read is how a collector thread disappears for + good - and a thread that never returns reports nothing and looks like a + quiet host.""" + body = ast.unparse(_function(MAIN, "_ingest_socket")) + assert "settimeout" in body + + +def test_a_failed_handshake_does_not_leak_the_socket(): + """`wrap_socket` raising leaves the underlying descriptor open. One per + cycle is a file-descriptor leak that ends as a host that stops reporting + for reasons nothing explains.""" + fn = _function(MAIN, "_ingest_socket") + handler = next(h for h in ast.walk(fn) if isinstance(h, ast.ExceptHandler)) + assert "close" in ast.unparse(handler) + assert "raise" in ast.unparse(handler) + + +# -------------------------------------------------------------------------- +# One TLS identity for the deployment +# -------------------------------------------------------------------------- + +def test_ingest_shares_the_consoles_certificate_settings(): + """A second pair of certificate variables is a second thing to rotate and + a second thing to forget. It is also the only arrangement an agent can be + told to trust in one step.""" + body = ast.unparse(_function(SERVER, "_ingest_tls_context")) + assert "product_tls" in body or "core import tls" in body + assert "TLS_INGEST_CERT" not in SERVER.read_text(encoding="utf-8") + + +# -------------------------------------------------------------------------- +# An actual handshake +# -------------------------------------------------------------------------- + +def test_the_agent_can_reach_a_tls_listener_end_to_end(tmp_path): + """The one test that would have caught the bug this work started from. + + Everything above reads source. Source can be entirely correct about a + feature that does not work - `TLS_ENABLED` was documented in three places + and wired to nothing, and no amount of reading the documentation would + have revealed it. So this one runs a real listener with a real generated + certificate and makes the agent's own connect path talk to it. + """ + import ssl as _ssl + import threading + + from certs.generate_certs import ensure_certs + + paths = ensure_certs(outdir=str(tmp_path), cn="localhost", days=30, + log=lambda *_: None) + + server_ctx = _ssl.create_default_context(_ssl.Purpose.CLIENT_AUTH) + server_ctx.load_cert_chain(certfile=paths["crt"], keyfile=paths["key"]) + + listener = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + listener.bind(("127.0.0.1", 0)) + listener.listen(1) + port = listener.getsockname()[1] + + seen: dict = {} + + def serve(): + raw, _ = listener.accept() + try: + with server_ctx.wrap_socket(raw, server_side=True) as tls: + seen["received"] = tls.recv(16) + tls.sendall(b"pong") + except Exception as e: # pragma: no cover - surfaced below + seen["error"] = f"{type(e).__name__}: {e}" + finally: + raw.close() + + thread = threading.Thread(target=serve, daemon=True) + thread.start() + + connect = _compiled_socket_fn( + SERVER_IP="127.0.0.1", SERVER_PORT=port, INGEST_TLS=True, + INGEST_CA=paths["root_crt"], SERVER_HOSTNAME="localhost") + + with connect() as sock: + sock.sendall(b"ping") + assert sock.recv(16) == b"pong" + + thread.join(timeout=5) + assert seen.get("error") is None, seen.get("error") + assert seen.get("received") == b"ping" + + +def test_a_server_the_agent_cannot_verify_is_refused(tmp_path): + """The other half, and the one that is easy to lose. Encryption to an + unverified peer is encryption to whoever answered - against an attacker on + the path, exactly what not encrypting it would have achieved.""" + import ssl as _ssl + import threading + + from certs.generate_certs import ensure_certs + + real = ensure_certs(outdir=str(tmp_path / "server"), cn="localhost", + days=30, log=lambda *_: None) + stranger = ensure_certs(outdir=str(tmp_path / "other"), cn="localhost", + days=30, log=lambda *_: None) + + server_ctx = _ssl.create_default_context(_ssl.Purpose.CLIENT_AUTH) + server_ctx.load_cert_chain(certfile=real["crt"], keyfile=real["key"]) + + listener = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + listener.bind(("127.0.0.1", 0)) + listener.listen(1) + port = listener.getsockname()[1] + + def serve(): + try: + raw, _ = listener.accept() + try: + server_ctx.wrap_socket(raw, server_side=True) + except Exception: + pass + finally: + raw.close() + except Exception: + pass + + threading.Thread(target=serve, daemon=True).start() + + # Handed the wrong CA: a different deployment's, which is what an attacker + # presenting their own certificate looks like from here. + connect = _compiled_socket_fn( + SERVER_IP="127.0.0.1", SERVER_PORT=port, INGEST_TLS=True, + INGEST_CA=stranger["root_crt"], SERVER_HOSTNAME="localhost") + + with pytest.raises(ssl.SSLError): + connect() + + +def _compiled_socket_fn(**globals_): + """`_ingest_socket`, compiled with the module globals it reads replaced.""" + tree = ast.parse(MAIN.read_text(encoding="utf-8")) + fn = next(n for n in tree.body + if isinstance(n, ast.FunctionDef) and n.name == "_ingest_socket") + + # The module-level constants it reads, lifted with it. Compiling the + # function alone raises NameError on the first one, which reads as a bug + # in the agent rather than in the harness. + tree_body = ast.parse(MAIN.read_text(encoding="utf-8")).body + referenced = {n.id for n in ast.walk(fn) if isinstance(n, ast.Name)} + constants = [n for n in tree_body + if isinstance(n, ast.Assign) + and any(getattr(t, "id", "") in referenced for t in n.targets)] + + namespace: dict = {"socket": socket, "ssl": ssl} + exec(compile(ast.Module(body=[*constants, fn], type_ignores=[]), + str(MAIN), "exec"), namespace) + # After the constants, so the test's values win over the shipped defaults. + namespace.update(globals_) + return namespace["_ingest_socket"] diff --git a/tests/test_installer_transport.py b/tests/test_installer_transport.py new file mode 100644 index 0000000..a487ae2 --- /dev/null +++ b/tests/test_installer_transport.py @@ -0,0 +1,179 @@ +"""What the installer writes decides which transport an agent ever uses. + +The Windows script wrote `ingest_port = 5001` into every config.json. The +agent derives its transport from `server_url` — an `https://` console means +telemetry goes to the TLS listener — and an explicit port overrides that +derivation, by design, so somebody forwarding through a bastion can say what +they mean. + +Those two facts together meant every Windows agent would have stayed on the +plaintext port for ever, against a server configured for TLS, with nothing +anywhere reporting a problem: the telemetry arrives, the console fills, and +the only difference is that it crossed the network in the clear. That is the +shape of failure this codebase keeps finding, and a config file is a +particularly good place to hide it, because it is written once at enrolment +and then never looked at again. +""" +from __future__ import annotations + +import json +import pathlib +import re + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent + +from core.installers import _render_linux_install, _render_windows_install # noqa: E402 + + +@pytest.fixture(params=["https://soc.example.com:8000", "http://10.0.0.5:8000"]) +def url(request): + return request.param + + +def _linux(url: str) -> str: + return _render_linux_install(url, "10.0.0.5", "tok-123") + + +def _windows(url: str) -> str: + return _render_windows_install(url, "10.0.0.5", "tok-123") + + +# -------------------------------------------------------------------------- +# The port the agent is left on +# -------------------------------------------------------------------------- + +def test_no_installer_pins_the_ingest_port(url): + """Pinning it defeats the derivation from `server_url`, which is the only + thing that moves a fleet onto TLS without a second setting to get wrong.""" + for name, script in (("linux", _linux(url)), ("windows", _windows(url))): + assert not re.search(r"ingest_port\s*[=:]\s*5001", script), ( + f"the {name} installer pins ingest_port, so this agent will stay " + f"on the plaintext listener whatever the server is configured to do" + ) + + +def test_the_config_still_carries_what_the_agent_needs(url): + """A guard against fixing the above by deleting too much: the identity + fields are what the agent refuses to start without.""" + for script in (_linux(url), _windows(url)): + for field in ("agent_name", "agent_key", "server_url"): + assert field in script + + +# -------------------------------------------------------------------------- +# Trusting a self-signed server +# -------------------------------------------------------------------------- + +def test_an_https_installer_fetches_the_ca(url): + """Against a self-signed server the agent verifies and therefore fails, + which is correct — an unverified TLS connection is encrypted to whoever + answered. Something has to put the CA on the endpoint, and the installer + is the only step that runs there with the server reachable.""" + for script in (_linux(url), _windows(url)): + assert "/api/agent/ca" in script + assert "server_ca" in script + + +def test_the_ca_fetch_is_conditional_on_https(): + """Against plain http there is nothing to verify, and against a real CA + the endpoint's trust store already has it. Writing an empty server_ca + unconditionally would be harmless; fetching unconditionally would put a + 404 body on disk and fail later with a certificate-format error far from + the cause.""" + linux = _linux("http://10.0.0.5:8000") + assert "https://*" in linux, "the linux fetch is not gated on the scheme" + + windows = _windows("http://10.0.0.5:8000") + assert '$ServerUrl -like "https://*"' in windows + + +def test_an_empty_ca_file_is_not_kept(): + """A zero-byte PEM is worse than none: it satisfies every existence check + and fails at the point of use.""" + linux = _linux("https://soc.example.com") + assert "-s " in linux and "rm -f" in linux + windows = _windows("https://soc.example.com") + assert ".Length -gt 0" in windows + + +# -------------------------------------------------------------------------- +# The endpoint it fetches from +# -------------------------------------------------------------------------- + +def _download_ca_code() -> str: + """The handler's statements, with its docstring dropped. + + Its docstring names the file it exists to never serve, so matching the raw + source finds the explanation and reads it as the code. That mistake has + been made repeatedly in this suite; the fix is always to assert on the + parsed body rather than the text. + """ + import ast + + tree = ast.parse((ROOT / "app.py").read_text(encoding="utf-8")) + fn = next(n for n in ast.walk(tree) + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)) + and n.name == "download_ca") + body = list(fn.body) + if (body and isinstance(body[0], ast.Expr) + and isinstance(body[0].value, ast.Constant)): + body = body[1:] + return "\n".join(ast.unparse(n) for n in body) + + +def test_the_server_serves_the_certificate_and_never_the_key(): + """The distinction the whole design rests on. One half of the pair is + meant to be distributed; the other signs everything this deployment + vouches for.""" + code = _download_ca_code() + assert "rootCA.crt" in code + assert "rootCA.key" not in code + + +def test_the_ca_route_is_public_deliberately(): + """An installer needs it before it holds any credential. Listed rather + than left to the path backstop, so `test_auth_wiring` sees a decision + instead of an omission.""" + app_py = (ROOT / "app.py").read_text(encoding="utf-8") + handlers = app_py[app_py.index("_PUBLIC_HANDLERS = {"):] + handlers = handlers[:handlers.index("\n}")] + assert '"download_ca"' in handlers + + +def test_a_server_with_no_local_ca_says_so(): + """404, not an empty body. A deployment using a real certificate has no + local CA and needs none, and that answer has to be distinguishable from a + broken one.""" + assert "status=404" in _download_ca_code() + + +# -------------------------------------------------------------------------- +# The scripts still have to be scripts +# -------------------------------------------------------------------------- + +def test_the_linux_script_has_no_unexpanded_template_braces(url): + """`_render_*` are f-strings, so a literal brace has to be doubled. One + that is not becomes a KeyError at render time, or worse, silently + interpolates something.""" + script = _linux(url) + assert "{{" not in script and "}}" not in script + + +def test_the_windows_script_has_no_unexpanded_template_braces(url): + script = _windows(url) + assert "{{" not in script and "}}" not in script + + +def test_the_written_config_is_valid_json(): + """The heredoc is assembled by hand, and a missing comma between two + fields is a config the agent refuses to load — on the endpoint, after the + installer has reported success.""" + script = _linux("https://soc.example.com:8000") + body = script[script.index('cat > "$INSTALL_DIR/config.json"'):] + body = body[body.index("{"):body.index("EOF\n", body.index("{"))] + + # Shell variables stand in for values; substitute something JSON-safe. + literal = re.sub(r"\$[A-Z_]+", "x", body).strip() + json.loads(literal) diff --git a/tests/test_link_frame_priority.py b/tests/test_link_frame_priority.py new file mode 100644 index 0000000..5b10a4a --- /dev/null +++ b/tests/test_link_frame_priority.py @@ -0,0 +1,201 @@ +"""A screen frame must not be able to starve a config reply. + +Symptom, from a real host: the Config tab returned + + DESKTOP-EVS8H9J-3 did not answer GET /config/rules within 5s + +while the agent was connected, healthy, and shipping telemetry the whole time. +`cmd_get_config` opens a small YAML file and reads it — microseconds. The +agent's own log had no record of the request, because a reply that is never +sent logs nothing. + +Both sides were behaving. The channel multiplexes two very different kinds of +traffic down one websocket: + + control a request and its reply, small, and something is waiting on it + stream console output and screen frames, large, and continuous + +and one `_send_lock` serialises them, because two frames interleaved on one +socket is a protocol error. So a screen pump sending a full screenshot every +200ms competed for the same lock as every config read, restart and health +check — and it never stopped competing, because a screen capture has no +natural end and nothing told it to stop when the viewer went away. + +Worse, `PING_INTERVAL_S` was doing two unrelated jobs: the ping cadence *and* +the socket timeout handed to `create_connection`. One `send_binary` the +network was slow to accept could hold the only send lock for thirty seconds, +against control requests whose deadline is five. + +The asymmetry the fix rests on: **a stream frame is stale the moment it is +delayed, and a control reply is not.** The next capture supersedes the frame +that could not get onto the wire; nothing supersedes the answer somebody is +waiting for. +""" +from __future__ import annotations + +import pathlib +import threading +import time + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +LINK = ROOT / "Sentora" / "modules" / "link.py" + +FRAME = b"\x00\x00\x00\x01payload" + + +@pytest.fixture +def client(): + """A real `AgentLinkClient` with no network under it.""" + import sys + + sys.path.insert(0, str(ROOT / "Sentora")) + from modules.link import AgentLinkClient + + return AgentLinkClient("http://server:8000", "key", lambda *a: ({}, 200)) + + +class _SlowSocket: + """A websocket whose binary sends block, the way a saturated one does. + + Only `send_binary` stalls. Text frames carry the control traffic whose + latency is under test, and making those slow too would measure the fake + socket rather than the lock discipline. + """ + + def __init__(self, delay: float): + self.delay = delay + self.sent: list = [] + self.releasing = threading.Event() + + def send(self, payload): # control frames + self.sent.append(("text", payload)) + + def send_binary(self, payload): # stream payloads + self.releasing.wait(self.delay) + self.sent.append(("binary", payload)) + + +def test_a_reply_waits_for_at_most_one_stream_frame(client): + """The guarantee, stated honestly. + + A frame already on the wire cannot be recalled, so a reply can always be + delayed by that one. What it must never wait behind is a *queue* of them — + and it used to, because a capture that never ends produces frames faster + than a slow link drains them and every one took the same lock. + + So: two stream frames and a reply, the first frame stalled. The reply has + to go out when that frame completes, not after both. + """ + sock = _SlowSocket(delay=1.0) + client._ws = sock + + threading.Thread(target=lambda: client._send(FRAME, droppable=True), + daemon=True).start() + time.sleep(0.2) # let it take the lock and stall + + # A second frame arrives while the first is still going, the way a 200ms + # capture loop produces them. + threading.Thread(target=lambda: client._send(FRAME, droppable=True), + daemon=True).start() + time.sleep(0.05) + + started = time.monotonic() + client._send({"t": "res", "id": "abc", "status": 200, "body": {"ok": True}}) + waited = time.monotonic() - started + + assert waited < 1.8, ( + f"the reply waited {waited:.1f}s - long enough to have queued behind " + f"more than the one frame already on the wire" + ) + assert [k for k, _ in sock.sent].count("text") == 1, "the reply never went out" + + +def test_a_stream_frame_yields_while_a_reply_is_waiting(client): + """Python locks are not fair. A pump producing a frame every 200ms wins + the race often enough to push a reply past the server's five seconds, so + the pump stands down rather than competing on equal terms.""" + sock = _SlowSocket(delay=2.0) + client._ws = sock + + threading.Thread(target=lambda: client._send(FRAME, droppable=True), + daemon=True).start() + time.sleep(0.2) + + reply = threading.Thread( + target=lambda: client._send({"t": "res", "id": "a", "status": 200, "body": {}}), + daemon=True) + reply.start() + time.sleep(0.1) # the reply is now queued for the wire + + before = client._dropped_frames + client._send(FRAME, droppable=True) + assert client._dropped_frames == before + 1, \ + "a stream frame queued ahead of a reply that was already waiting" + + sock.releasing.set() + reply.join(timeout=5) + + +def test_a_control_frame_is_never_dropped(client): + """The other half. Dropping a reply would produce the same timeout by a + quieter route, with nothing logged either.""" + sock = _SlowSocket(delay=0.0) + sock.releasing.set() + client._ws = sock + + client._send({"t": "res", "id": "abc", "status": 200, "body": {}}) + assert [kind for kind, _ in sock.sent] == ["text"] + + +def test_the_socket_deadline_is_not_the_ping_cadence(): + """`PING_INTERVAL_S` was passed as `create_connection(timeout=...)`, so + "ping every 30s" silently also meant "any single send may block for 30 + seconds" — six times the deadline of the requests behind it.""" + source = LINK.read_text(encoding="utf-8") + assert "SOCKET_TIMEOUT_S" in source, \ + "the socket deadline and the ping cadence are still the same number" + + connect = source[source.index("websocket.create_connection("):] + connect = connect[:connect.index(")")] + assert "PING_INTERVAL_S" not in connect, \ + "the connection still takes its deadline from the ping interval" + + +def test_the_pump_stops_when_its_stream_is_gone(): + """An orphaned pump is what made any of this reachable. + + The server printed "data for a stream that is gone" and did nothing else, + so an agent whose viewer had closed kept capturing and sending for the + life of the connection — competing for the send lock with every control + request, for ever, on behalf of nobody. + """ + source = LINK.read_text(encoding="utf-8") + pump = source[source.index(" def _open_stream"):] + pump = pump[:pump.index(" def _close_stream")] + assert "_streams.get(channel) is stream" in pump, \ + "the pump does not notice that its own stream was closed under it" + + +def test_the_server_tells_the_agent_about_an_orphaned_stream(): + """Printing it locally leaves the agent pumping. The close has to travel.""" + app_py = (ROOT / "app.py").read_text(encoding="utf-8") + loop = app_py[app_py.index("async def agent_link_socket"):] + loop = loop[:loop.index("def _validate_agent_auth_sync")] + + orphan = loop[loop.index("deliver_stream("):] + orphan = orphan[:orphan.index("continue")] + assert '"t": "close"' in orphan, \ + "the agent is never told its stream is gone, so it keeps sending" + + +def test_the_orphan_notice_is_sent_once_per_channel(): + """Frames already in flight arrive after the close. One line and one + close each would be the noisiest thing in the log at exactly the moment + somebody is reading it.""" + app_py = (ROOT / "app.py").read_text(encoding="utf-8") + loop = app_py[app_py.index("async def agent_link_socket"):] + loop = loop[:loop.index("def _validate_agent_auth_sync")] + assert "orphaned" in loop + assert "orphaned.add(channel)" in loop diff --git a/tests/test_search.py b/tests/test_search.py new file mode 100644 index 0000000..18230fe --- /dev/null +++ b/tests/test_search.py @@ -0,0 +1,254 @@ +"""Search could not answer the question anybody actually has. + +It was free text across every field, over the whole retention, first N +results, with no way to reach the N+1th. So "what happened on that host +between two and four, with severity critical" could not be asked at all - not +because the data was missing, but because the query had nowhere to put a time, +a field, or a page. + +Three defects underneath that were their own bugs: + + limit = int(request.args.get("limit", 100)) + +`?limit=abc` raised inside the handler and came back as a bodyless 500; +`?limit=999999` was honoured and sent straight to OpenSearch. + + {"wildcard": {"agent_name": agent}} + +The caller's string went into a `wildcard` clause. The default was `*`, so +every search matched every term in the index, and a leading `*` was one +keystroke away from a scan of every term there is. + + resp = os_utils.search_logs(...) + if not resp: return sanic_json({"hits": []}) + +`search_logs` caught everything and returned None, so a malformed query, an +unreachable OpenSearch and a genuinely empty result were the same screen. On a +security console the difference between "no matches" and "the search did not +run" is the difference between an all-clear and no answer at all. +""" +from __future__ import annotations + +import ast +import pathlib + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +APP = ROOT / "app.py" +OS_UTILS = ROOT / "core" / "opensearch.py" +PAGE = ROOT / "frontend" / "src" / "pages" / "Search.tsx" + + +def _function(path: pathlib.Path, name: str) -> str: + tree = ast.parse(path.read_text(encoding="utf-8")) + return next(ast.unparse(n) for n in ast.walk(tree) + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)) + and n.name == name) + + +def _compiled(path: pathlib.Path, name: str, extra: dict | None = None): + """One function, compiled with the module constants it refers to. + + Compiling the function alone raises NameError the moment it uses a + module-level constant - which reads as a bug in the code rather than in + the harness. The constants it names are lifted with it. + """ + import re as _re + + tree = ast.parse(path.read_text(encoding="utf-8")) + fn = next(n for n in tree.body + if isinstance(n, ast.FunctionDef) and n.name == name) + referenced = {n.id for n in ast.walk(fn) if isinstance(n, ast.Name)} + constants = [n for n in tree.body + if isinstance(n, ast.Assign) + and any(getattr(t, "id", "") in referenced for t in n.targets)] + + namespace: dict = {"re": _re, "datetime": __import__("datetime").datetime} + namespace.update(extra or {}) + exec(compile(ast.Module(body=[*constants, fn], type_ignores=[]), + str(path), "exec"), namespace) + return namespace[name] + + +# -------------------------------------------------------------------------- +# A bounded page size +# -------------------------------------------------------------------------- + +@pytest.mark.parametrize("raw,expected", [ + (None, 50), ("", 50), ("abc", 50), ("-4", 1), ("0", 1), + ("20", 20), ("999999", 200), +]) +def test_a_page_size_is_bounded_and_never_raises(raw, expected): + """`int(request.args.get("limit", 100))` turned `?limit=abc` into a + bodyless 500 and honoured `?limit=999999`.""" + positive_int = _compiled(APP, "_positive_int") + assert positive_int(raw, 50, 200) == expected + + +# -------------------------------------------------------------------------- +# A window, which there was none of +# -------------------------------------------------------------------------- + +@pytest.mark.parametrize("raw", ["now-1h", "now-7d", "now", "now-1d/d"]) +def test_relative_windows_are_accepted(raw): + bound = _compiled(APP, "_time_bound") + assert bound(raw, "now-24h") == raw + + +def test_an_absolute_timestamp_is_accepted(): + bound = _compiled(APP, "_time_bound") + assert bound("2026-09-01T12:00:00", "now-24h") == "2026-09-01T12:00:00" + + +@pytest.mark.parametrize("raw", [ + "yesterday", "now-", "'; DROP", "now-1x", "", None, +]) +def test_junk_falls_back_rather_than_reaching_the_engine(raw): + """An unparseable bound sent through comes back as a 400 the operator has + to decode.""" + bound = _compiled(APP, "_time_bound") + assert bound(raw, "now-24h") == "now-24h" + + +def test_there_is_a_default_window_at_all(): + """The single biggest change. With no time filter every search scanned the + whole retention and sorted it, and the question an operator has is nearly + always about a window.""" + code = _function(APP, "search_logs_api") + assert "SEARCH_DEFAULT_WINDOW" in code + assert '"range"' in code or "'range'" in code + + +# -------------------------------------------------------------------------- +# The query +# -------------------------------------------------------------------------- + +def test_the_query_is_field_aware(): + """`multi_match` across `fields: ["*"]` cannot express `severity:critical`, + which is the first thing anybody wants to type.""" + code = _function(APP, "search_logs_api") + assert "query_string" in code + assert "multi_match" not in code + + +def test_a_leading_wildcard_is_refused(): + """`*foo` forces a scan of every term in the index. It is the classic way + to take a search cluster down from a text box.""" + code = _function(APP, "search_logs_api") + assert "'allow_leading_wildcard': False" in code \ + or '"allow_leading_wildcard": False' in code + + +def test_the_agent_filter_is_a_term_not_a_wildcard(): + """The caller's string went straight into a `wildcard` clause, and the + default was `*`.""" + code = _function(APP, "search_logs_api") + # The clause, not the word: `analyze_wildcard` and + # `allow_leading_wildcard` are settings on the query parser and both + # legitimately contain it. + assert "'wildcard':" not in code and '"wildcard":' not in code, \ + "the caller's string is back in a wildcard clause" + assert "terms" in code + assert "_agent_name_forms" in code, \ + "a hyphenated host would not be found under the console's spelling" + + +# -------------------------------------------------------------------------- +# Results you can get past the first page of +# -------------------------------------------------------------------------- + +def test_results_are_paged(): + code = _function(APP, "search_logs_api") + assert "'from'" in code or '"from"' in code + assert "pages" in code + assert "track_total_hits" in code, "the total is capped at 10000 without it" + + +# -------------------------------------------------------------------------- +# A failed search is not an empty one +# -------------------------------------------------------------------------- + +def test_the_engine_reports_failures_instead_of_swallowing_them(): + source = OS_UTILS.read_text(encoding="utf-8") + assert "class SearchError" in source + body = _function(OS_UTILS, "search_logs") + assert "raise SearchError" in body + assert "return None" not in body + + +def test_a_broken_query_is_a_400_with_the_reason(): + """Not `{"hits": []}`. A search that could not run and a search that found + nothing were the same screen.""" + code = _function(APP, "search_logs_api") + assert "SearchError" in code + assert "400" in code + assert "could not be run" in code + + +def test_the_page_tells_the_two_apart(): + page = PAGE.read_text(encoding="utf-8") + assert "The search did not run" in page + assert "No matches in this window" in page + + +# -------------------------------------------------------------------------- +# Two stores, two tabs +# -------------------------------------------------------------------------- + +def test_events_are_searched_separately(): + """One ranking over both would have to decide whether a raw log line + outranks a detection, and there is no answer to that.""" + source = APP.read_text(encoding="utf-8") + assert '"/api/events/search"' in source + page = PAGE.read_text(encoding="utf-8") + assert "'logs'" in page and "'events'" in page + + +def test_event_columns_are_an_allowlist(): + """They become SQL identifiers. Everything the operator types goes in as a + parameter; only the column comes from here.""" + source = APP.read_text(encoding="utf-8") + assert "EVENT_SEARCH_TABLES" in source + code = _function(APP, "search_events_api") + assert "not searchable" in code + + +def test_events_are_matched_after_decryption(): + """The searchable text is inside the ciphertext, so a `LIKE` in SQL + matches nothing and reports it as no results - which is the failure this + page is being rebuilt around.""" + code = _function(APP, "_search_events_sync") + assert "decrypt_row_fields" in code + assert "LIKE" not in code.upper() + + +# -------------------------------------------------------------------------- +# The console +# -------------------------------------------------------------------------- + +def test_the_filters_and_the_text_are_the_same_thing(): + """A builder that hides the query teaches nothing, and a bare text box + helps nobody on their first day.""" + page = PAGE.read_text(encoding="utf-8") + assert "function toQuery" in page + assert "setQuery(toQuery(next))" in page + + +def test_a_result_value_can_be_clicked_to_narrow(): + """How a search becomes an investigation rather than one question.""" + page = PAGE.read_text(encoding="utf-8") + assert "addFilter(col, hit[col])" in page + + +def test_a_result_links_to_its_host(): + page = PAGE.read_text(encoding="utf-8") + assert "/agent/${encodeURIComponent(agentOf(hit))}" in page + + +def test_the_old_path_still_lands_somewhere(): + """A bookmark to /log-search should not fall through to the dashboard + catch-all, which looks like the page was deleted.""" + app_tsx = (ROOT / "frontend" / "src" / "App.tsx").read_text(encoding="utf-8") + assert '/log-search' in app_tsx + assert 'Navigate to="/search"' in app_tsx diff --git a/tests/test_tls_serving.py b/tests/test_tls_serving.py new file mode 100644 index 0000000..7c9d496 --- /dev/null +++ b/tests/test_tls_serving.py @@ -0,0 +1,135 @@ +"""`TLS_ENABLED=1` has to produce TLS, or refuse to start. + +The setting was in `.env.example`, in `docs/production-deployment.md`, and in +the closing instructions of `certs/generate_certs.py`. It was read by nothing. +An operator who set it saw the console come up and had no way, short of +opening a packet capture, to learn that the login form and the session cookie +were crossing the network in clear text. + +A missing control gets noticed the first time somebody looks for it. A +documented one gets trusted. These tests are about the second kind: the +configuration and the transport have to be the same thing. +""" +from __future__ import annotations + +import pathlib +import ssl + +import pytest + +from core import tls + +ROOT = pathlib.Path(__file__).resolve().parent.parent + + +def test_off_by_default(): + assert tls.resolve(env={}) is None + assert tls.tls_enabled(env={}) is False + + +@pytest.mark.parametrize("value", ["1", "true", "TRUE", "yes", "on"]) +def test_the_spellings_operators_actually_use(value): + """`TLS_ENABLED=true` silently meaning off is the same failure as the one + this module exists to fix.""" + assert tls.tls_enabled(env={"TLS_ENABLED": value}) is True + + +def test_a_missing_certificate_refuses_to_start(tmp_path): + """The one behaviour that must never exist: asking for TLS, not getting + it, and serving plain HTTP anyway.""" + with pytest.raises(tls.TLSConfigError) as excinfo: + tls.resolve(env={ + "TLS_ENABLED": "1", + "TLS_CERT": str(tmp_path / "nope.crt"), + "TLS_KEY": str(tmp_path / "nope.key"), + }) + assert "plain HTTP" in str(excinfo.value) + + +def test_half_a_configuration_is_refused(tmp_path): + """Setting only one of the pair looks deliberate. Filling the other half + from a generated certificate would serve TLS the operator did not + configure and would not know was in use.""" + with pytest.raises(tls.TLSConfigError) as excinfo: + tls.resolve(env={"TLS_ENABLED": "1", "TLS_CERT": str(tmp_path / "a.crt")}) + assert "TLS_KEY" in str(excinfo.value) + + +def test_a_mismatched_pair_fails_at_startup_not_per_connection(tmp_path): + """A key that does not match its certificate handshakes fine until the + first client arrives, and then fails once per connection in a log nobody + is reading.""" + from certs.generate_certs import ensure_certs + + a = ensure_certs(outdir=str(tmp_path / "a"), cn="a.local", days=30, log=lambda *_: None) + b = ensure_certs(outdir=str(tmp_path / "b"), cn="b.local", days=30, log=lambda *_: None) + + with pytest.raises(tls.TLSConfigError) as excinfo: + tls.resolve(env={"TLS_ENABLED": "1", "TLS_CERT": a["crt"], "TLS_KEY": b["key"]}) + assert "usable pair" in str(excinfo.value) + + +def test_a_real_pair_is_accepted(tmp_path): + from certs.generate_certs import ensure_certs + + paths = ensure_certs(outdir=str(tmp_path), cn="soc.local", days=30, log=lambda *_: None) + config = tls.resolve(env={ + "TLS_ENABLED": "1", "TLS_CERT": paths["crt"], "TLS_KEY": paths["key"], + }) + assert config == {"cert": paths["crt"], "key": paths["key"]} + + # The shape Sanic wants. Asserted because getting it wrong produces a + # server that starts and then serves nothing. + context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) + context.load_cert_chain(certfile=config["cert"], keyfile=config["key"]) + + +# -------------------------------------------------------------------------- +# The cookie and the transport have to agree +# -------------------------------------------------------------------------- + +def test_the_cookie_follows_tls_when_nobody_said_otherwise(): + """These were two independent settings that had to match, and nothing + made them. TLS on with the flag left alone gave an HTTPS console whose + session cookie was still allowed onto plain HTTP.""" + assert tls.cookie_should_be_secure(env={"TLS_ENABLED": "1"}) is True + assert tls.cookie_should_be_secure(env={}) is False + + +def test_an_explicit_setting_still_wins(): + """TLS terminating at a proxy is invisible from inside this process, and + that deployment serves HTTP here and still needs the flag.""" + assert tls.cookie_should_be_secure(env={"SESSION_COOKIE_SECURE": "1"}) is True + assert tls.cookie_should_be_secure( + env={"TLS_ENABLED": "1", "SESSION_COOKIE_SECURE": "0"}) is False + + +# -------------------------------------------------------------------------- +# The wiring, not just the helper +# -------------------------------------------------------------------------- + +def test_app_actually_passes_ssl_to_run(): + """`core/tls.py` being correct is worth nothing if `app.run()` never + receives what it returns - which is precisely the bug this replaces.""" + source = (ROOT / "app.py").read_text(encoding="utf-8") + run = source[source.index(" app.run("):] + # To the closing paren of the call, not the first paren in it - the first + # is inside `single_process=(num_workers == 1)`, and slicing there made + # this test read an empty argument list and fail on a correct file. + run = run[:run.index(chr(10) + " )")] + assert "ssl=ssl_config" in run, "app.run() does not receive the TLS config" + + +def test_the_documented_settings_are_the_implemented_ones(): + """The drift that caused this: `.env.example` and the production guide + named three variables, and the code read none of them. Whatever the docs + promise has to appear in the module that delivers it.""" + source = (ROOT / "core" / "tls.py").read_text(encoding="utf-8") + for name in ("TLS_ENABLED", "TLS_CERT", "TLS_KEY"): + assert f'"{name}"' in source or f"get(\"{name}\"" in source, name + + env_example = (ROOT / ".env.example").read_text(encoding="utf-8") + guide = (ROOT / "docs" / "production-deployment.md").read_text(encoding="utf-8") + for name in ("TLS_ENABLED", "TLS_CERT", "TLS_KEY"): + assert name in env_example, f"{name} is implemented but undocumented" + assert name in guide, f"{name} is implemented but not in the guide"