diff --git a/.env.example b/.env.example index 0cc1716..cbeb694 100644 --- a/.env.example +++ b/.env.example @@ -83,10 +83,17 @@ AGENT_SHARED_SECRET= # 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. +# certificate is generated on first boot, one per deployment, into TLS_DIR. +# Browsers will warn - nothing trusts that CA - and agents need its +# rootCA.crt, which the installer fetches from /api/agent/ca. Use a real +# certificate for anything reachable beyond your own network. +# +# TLS_DIR is where generated material lives. It defaults to certs/ beside the +# checkout, and compose overrides it to /app/data/certs: certs/ ships inside +# the image owned by root while the server runs unprivileged, so generation +# there fails with EACCES - and, correctly refusing to serve plain HTTP +# instead, takes the container into a restart loop. +# TLS_DIR=/app/data/certs # TLS_CERT=certs/server.crt # TLS_KEY=certs/server.key # TLS_CN=soc.example.com diff --git a/Sentora/main.py b/Sentora/main.py index faa3b11..0116315 100644 --- a/Sentora/main.py +++ b/Sentora/main.py @@ -9,6 +9,7 @@ import json import subprocess import os +import pathlib import signal import requests import sys @@ -17,7 +18,8 @@ from datetime import datetime, timedelta import ipaddress -from modules.db import insert_record, fetch_unsent, mark_sent, fetch_one +from modules.db import (insert_record, fetch_unsent, mark_sent, fetch_one, + reoffer_recent, prune_sent) import modules.enc_db as enc_db import modules.screen_capture as screen_capture import modules.agent_paths as agent_paths @@ -338,6 +340,105 @@ def run(self): SERVER_HOSTNAME = None #: An unbounded blocking read is how a collector thread disappears for good. INGEST_TIMEOUT_S = 30 + +#: Whether this server has ever acknowledged a batch. +#: +#: The receipt frame is optional, because an agent can be pointed at a server +#: older than it - and for that case "no reply" has to mean "mark them sent", +#: or the agent retries the same rows for ever. Once a server has replied even +#: once, that reasoning no longer applies and silence is a fault. +_SERVER_ACKNOWLEDGES = False + +#: How many rows per table to offer again when the server's database turns out +#: to have been recreated. +#: +#: Bounded on purpose. `hardware_inventory` on one host holds 530,000 rows, and +#: re-offering all of them at fifty a batch is a replay measured in days during +#: which nothing current gets through. The tables this rescues - port scans, +#: process events, SOAR actions, permission changes - hold tens or hundreds, +#: so they are covered completely. +_REOFFER_LIMIT = 1000 + +#: The server database identity last seen. Empty until a receipt carries one; +#: a server older than the epoch sends none, and that has to keep working. +_SERVER_EPOCH = "" + + +def _epoch_file() -> str: + """Where the last-seen server identity is remembered. + + Beside config.json, because it belongs to this enrolment: re-point the + agent at a different server and it should notice that too. + """ + return os.path.join(agent_paths.persistent_config_dir(), "..", + ".server_epoch") + + +def _remember_epoch(epoch: str) -> None: + try: + with open(_epoch_file(), "w", encoding="utf-8") as fh: + fh.write(epoch) + except OSError as e: + # Not fatal, but it means the reset will be detected again on the next + # start - the resend is deduplicated server-side, so the cost is a + # repeated offer rather than duplicated data. + print(f"[!] could not record the server epoch: {e}", flush=True) + + +def _load_epoch() -> str: + try: + with open(_epoch_file(), "r", encoding="utf-8") as fh: + return fh.read().strip() + except OSError: + return "" + + +def _handle_server_epoch(epoch: str) -> None: + """Notice that the server has forgotten, and offer the rows again. + + `sent` is this agent's only record of what the server holds, and a + server-side database reset is invisible from here. Every table that does + not produce new rows constantly then stays empty on the server for ever, + which is exactly what happened: 93 port-scan rows held locally, none on + the server, and nothing wrong at either end. + """ + global _SERVER_EPOCH + if not epoch or epoch == _SERVER_EPOCH: + return + + previous, _SERVER_EPOCH = _SERVER_EPOCH, epoch + _remember_epoch(epoch) + + if previous: + why = (f"The server's database for this agent was recreated " + f"({previous[:8]} -> {epoch[:8]}). Everything it held is gone") + else: + # First sighting, and it re-offers too. + # + # The tempting rule is "nothing to compare against, so do nothing" - + # but the agents that most need this are precisely the ones that have + # been running since before the epoch existed, holding rows marked + # sent against a server that may never have received them. That is the + # state this was written for. + # + # It costs nothing on a fresh install: an agent with an empty local + # database has no rows to offer, so the sweep finds none. And the + # resend is deduplicated server-side, so the worst case on an agent + # that was already in sync is a batch of rows the server recognises + # and skips. + why = ("This agent has no record of which server database it has been " + "talking to, so rows already marked sent may never have " + "arrived") + + print(f"[!] {why}. Re-offering the most recent {_REOFFER_LIMIT} row(s) " + f"per table; the server skips anything it already holds.", flush=True) + for table in TABLES: + try: + count = reoffer_recent(table, _REOFFER_LIMIT) + if count: + print(f" {table}: {count} row(s) queued again", flush=True) + except Exception as e: + print(f" {table}: could not re-offer ({e})", flush=True) AUTOMATIONS_API_URL = None AUTOMATIONS_MODE = "auto" AGENT_SHARED_SECRET = None @@ -377,8 +478,16 @@ def run(self): 'network_connections', 'process_events', 'hardware_inventory', - 'security_audit', 'docker_containers', + # `security_audit` was here and is not any more. Nothing in this agent + # ever wrote a row to it - no collector, not a broken one - so it was + # shipped empty every cycle and the console showed it permanently as + # NOT COLLECTED, which reads as a sensor that failed rather than one that + # was never built. + # + # The server keeps the table and its ALLOWED_TABLES entry, so a collector + # can be added later without touching the server. What is not kept is the + # claim that this agent reports it. ] MAX_WORKERS = 6 @@ -643,6 +752,35 @@ def send_table(table: str): f"({receipt['error']}); rows kept for retry") return + global _SERVER_ACKNOWLEDGES + if receipt is not None: + _SERVER_ACKNOWLEDGES = True + # After the batch is decided, before it is marked sent: a reset + # noticed here re-offers rows, and marking this batch sent + # immediately afterwards is correct - the server has just + # confirmed it stored them. + _handle_server_epoch(receipt.get("epoch") or "") + elif _SERVER_ACKNOWLEDGES: + # Silence from a server that has acknowledged before is a failure, + # not an old build. + # + # "No receipt means an older server, so mark the rows sent" is + # right exactly once - before this server has ever replied. After + # that it becomes a way to lose data quietly, and there is a real + # configuration that produces it: `INGEST_TLS_REQUIRED=1` closes + # the plaintext listener, but Docker keeps publishing the port, so + # a connection is accepted by the proxy, the batch is written into + # a socket nobody reads, and the close looks exactly like an old + # server that stored everything. + telemetry_health.record_send_failure( + table, "the server accepted the connection and did not " + "acknowledge the batch; it has acknowledged before, so " + "this is a broken path rather than an older server") + if debug: + print(f"[!] {table}: no acknowledgement from a server that has " + f"acknowledged before - rows kept for retry") + return + mark_sent(table, [r['id'] for r in rows]) telemetry_health.record_send(table, len(rows), receipt=receipt) if debug: @@ -680,6 +818,46 @@ def db_sender_loop(): time.sleep(10) +#: How many rows to keep per table locally once the server has them. +#: +#: Above `_REOFFER_LIMIT` on purpose: pruning must not eat the material a +#: server-side reset needs to offer again. Below that and recovery would be +#: quietly capped by whatever the cleaner happened to leave behind. +_RETENTION_ROWS = 5000 + + +def retention_loop(): + """Keep the agent's local database from growing for ever. + + Nothing pruned it, and the agent is append-only, so it grew for the life + of the install: 530,428 rows of `hardware_inventory` on one host after a + few weeks, every one already shipped and never read again. Disk on a + monitored endpoint is the one place a monitoring tool should not be + quietly consuming. + + Hourly rather than per cycle. Deleting is cheap here but not free, and + nothing about this is urgent - a table that overshoots by an hour's + collection is not a problem, whereas a delete competing with the send loop + for the same rows is. + """ + while True: + # An hour before the first pass. On a fresh install there is nothing + # to prune, and on an upgrade the first thing this process should be + # doing is shipping the backlog, not deleting it. + time.sleep(3600) + total = 0 + for table in TABLES: + try: + total += prune_sent(table, _RETENTION_ROWS) + except Exception as e: + # One table failing must not stop the rest - the same rule the + # collectors follow, and for the same reason. + print(f"[!] retention failed on {table}: {e}", flush=True) + if total: + print(f"[*] Retention: removed {total} already-shipped row(s); " + f"keeping the newest {_RETENTION_ROWS} per table", flush=True) + + #: How many consecutive failures between repeat reports, once a collector has #: announced itself broken. Loud on the first, then rare - a collector that #: has been failing for a day should not be writing a line every two minutes. @@ -1521,6 +1699,11 @@ def start_threads(): daemon=True ).start() + threading.Thread( + target=retention_loop, + daemon=True + ).start() + start_server_channel() @@ -1742,9 +1925,16 @@ def start_server_channel() -> None: "the server will reach this agent over HTTP.", flush=True) return + # The same CA the telemetry socket verifies with. One `server_ca` for the + # host, because the two connections go to the same server - giving it to + # only one of them produced an agent that shipped telemetry over TLS and + # could not open its channel at all, which since the endpoint stopped + # listening on a port of its own means an agent that reports and cannot + # be reached. client = link.AgentLinkClient( server_url, AGENT_SHARED_SECRET, dispatch_channel_request, - agent_name=AGENT_NAME, open_stream=open_channel_stream) + agent_name=AGENT_NAME, open_stream=open_channel_stream, + ca_cert=INGEST_CA) threading.Thread(target=client.run_forever, daemon=True).start() print(f"[link] channel opening to {client.channel_url()}", flush=True) @@ -1828,6 +2018,65 @@ def _parse_args(): return parser.parse_args() +def _install_trust_bundle(ca_path): + """Make one CA visible to every outbound client in this process. + + `server_ca` is a statement about the host, not about one socket, and it + was being wired in per connection. That cost three separate fixes for the + same defect: the telemetry socket had it, the channel did not, and the + `requests` calls - bootstrap, automations polling, enrolment - did not + either. Each failure looked different and each needed the same answer. + + So it is installed once, here, before anything connects. Anything added + later that speaks TLS inherits it without having to be told. + + The bundle is the system trust store *plus* this CA rather than this CA + alone. Replacing the store would make the agent distrust every other HTTPS + endpoint on the machine in order to solve a problem with one of them, and + that failure would arrive somewhere far from this line. + """ + if not ca_path or not os.path.exists(ca_path): + return None + + try: + private = pathlib.Path(ca_path).read_bytes() + except OSError as e: + print(f"[!] cannot read server_ca at {ca_path}: {e}", flush=True) + return None + + system = b"" + try: + import certifi + system = pathlib.Path(certifi.where()).read_bytes() + except Exception as e: + # Not fatal, but worth saying: without the system roots the agent + # trusts this CA and nothing else. + print(f"[!] system trust store unavailable ({e}); trusting only " + f"{ca_path}", flush=True) + + payload = (system + b"\n" + private) if system else private + bundle = pathlib.Path(ca_path).with_name("sentora-trust-bundle.pem") + try: + bundle.write_bytes(payload) + except OSError: + # A read-only install directory is a real deployment, and a temp file + # is fine here: the bundle is derived, not state. + import tempfile + bundle = pathlib.Path(tempfile.gettempdir()) / "sentora-trust-bundle.pem" + try: + bundle.write_bytes(payload) + except OSError as e: + print(f"[!] could not write a trust bundle: {e}", flush=True) + return None + + # `requests` reads the first two; `ssl.create_default_context()` with no + # cafile reads the third. Between them that is every client in this + # process, including any added after this was written. + for name in ("REQUESTS_CA_BUNDLE", "CURL_CA_BUNDLE", "SSL_CERT_FILE"): + os.environ[name] = str(bundle) + return str(bundle) + + def _resolve_ingest_transport(cfg: dict, args) -> tuple[bool, int, str | None, str | None]: """Whether telemetry goes over TLS, and to which port. @@ -2204,6 +2453,18 @@ def main(): INGEST_TLS, SERVER_PORT, INGEST_CA, SERVER_HOSTNAME = _resolve_ingest_transport( cfg, args) + # Before anything connects. Bootstrap runs a few lines below and is the + # first thing in this process to speak TLS. + _trust_bundle = _install_trust_bundle(INGEST_CA) + if _trust_bundle: + print(f"[*] Trusting {INGEST_CA} for every outbound connection") + + # What the server's database identity was last time this agent ran. Loaded + # rather than assumed empty, so a reset that happened while the agent was + # stopped is still noticed on the first receipt after it starts. + global _SERVER_EPOCH + _SERVER_EPOCH = _load_epoch() + if not AGENT_NAME: AGENT_NAME = "agent" if not AGENT_SHARED_SECRET or not cfg.get("server_url"): diff --git a/Sentora/modules/db.py b/Sentora/modules/db.py index 81d92d3..0d55039 100644 --- a/Sentora/modules/db.py +++ b/Sentora/modules/db.py @@ -201,6 +201,76 @@ def mark_sent(table: str, ids: list): cur.execute(query, (ids,)) conn.commit() +def reoffer_recent(table: str, limit: int) -> int: + """Mark the newest `limit` rows unsent again, and say how many. + + For one situation: the server's database for this agent has been recreated + and no longer holds what `sent` claims it does. `sent` is the agent's + private record of the server's contents, so a server-side reset is + invisible from here - and a row marked sent is never offered again. Tables + that produce rows constantly refill on their own, so the damage lands + entirely on the quiet ones: a host sat holding 93 port-scan rows against a + server holding none, with nothing wrong at either end. + + Bounded, and that bound is the point. `hardware_inventory` on one host + holds 530,000 rows; re-offering all of them at fifty per batch is a replay + measured in days, during which nothing current gets through. The newest + rows are the ones worth having back, and the tables this actually rescues + are small enough to be covered completely. + + The resend is safe because the server deduplicates: rows it already holds + are recognised and skipped rather than stored twice. + """ + query = (f"UPDATE {table} SET sent = FALSE WHERE id IN " + f"(SELECT id FROM {table} ORDER BY id DESC LIMIT %s)") + with get_conn() as conn: + with conn.cursor() as cur: + cur.execute(query, (limit,)) + changed = cur.rowcount + conn.commit() + return changed if changed and changed > 0 else 0 + + +def prune_sent(table: str, keep: int) -> int: + """Drop rows the server already has, beyond the newest `keep`. + + Nothing pruned this table before, and the agent is append-only, so the + local database grew for the life of the install. On one host after a few + weeks: 530,428 rows of `hardware_inventory`, 225,145 of + `network_connections`, 88,201 of `fim_data` - every one of them marked + sent, none of them ever read again. That is disk on a monitored endpoint, + which is the one place a monitoring tool has no business consuming. + + It also constrains recovery. `reoffer_recent` has to be bounded because + replaying half a million rows at fifty a batch takes days, so the backlog + was actively limiting how much could be rescued after a server reset. + + Two rules, and both matter: + + Only `sent` rows. An unsent row has not reached the server, and deleting + it is the data loss this whole module has spent the day preventing. + + Always keep the newest `keep`, which is set comfortably above + `_REOFFER_LIMIT` so a server-side reset still has rows to offer again. + + The delete is bounded by a threshold id rather than `NOT IN (...)`: one + index lookup and a range scan instead of a comparison against a five + thousand row list, on a table that may hold half a million. + """ + query = ( + f"DELETE FROM {table} WHERE sent = TRUE AND id < " + f"(SELECT id FROM {table} ORDER BY id DESC LIMIT 1 OFFSET %s)" + ) + with get_conn() as conn: + with conn.cursor() as cur: + # Fewer rows than `keep` means the subquery is NULL, `id < NULL` + # is NULL, and nothing is deleted - which is the right answer. + cur.execute(query, (keep,)) + removed = cur.rowcount + conn.commit() + return removed if removed and removed > 0 else 0 + + def fetch_one(table: str, where: str = "1=1", params: tuple = (), order_by: str = None): query = f"SELECT * FROM {table} WHERE {where}" if order_by: diff --git a/Sentora/modules/link.py b/Sentora/modules/link.py index 6103658..1cabbe6 100644 --- a/Sentora/modules/link.py +++ b/Sentora/modules/link.py @@ -115,9 +115,20 @@ class AgentLinkClient: """ def __init__(self, server_url: str, agent_key: str, dispatch, - *, agent_name: str = "", open_stream=None): + *, agent_name: str = "", open_stream=None, ca_cert=None): self.server_url = (server_url or "").rstrip("/") self.agent_key = agent_key + # PEM bundle for a self-signed server, or None to verify against the + # system trust store. + # + # This was missing, and the shape of the failure is why it matters + # more than it looks. `server_ca` reached the telemetry socket and + # stopped there, so against a self-signed server the agent shipped + # telemetry over TLS quite happily and could not open its channel at + # all - and since the endpoint no longer listens on a port of its own, + # that is an agent which reports and cannot be reached. Config, + # console, SOAR and the screen stream all travel here. + self.ca_cert = ca_cert or None self.dispatch = dispatch # `open_stream(kind, args) -> object with read/write/close`, supplied # by the caller so this module stays ignorant of what a console or a @@ -138,6 +149,10 @@ def __init__(self, server_url: str, agent_key: str, dispatch, #: continuous capture from pushing a reply past the server's deadline. self._control_waiting = 0 self._control_lock = threading.Lock() + #: One diagnostic, not one per reconnect. The condition it reports + #: does not change between attempts, and a reconnect loop that prints + #: the same paragraph every two seconds buries the thing it explains. + self._explained_tls = False self.connected = False self.last_error = "" @@ -178,6 +193,7 @@ def run_forever(self) -> None: except Exception as e: self.last_error = str(e) print(f"[link] channel closed: {e}", flush=True) + self._explain_once_if_the_server_speaks_tls() if self._stop.is_set(): return @@ -188,6 +204,52 @@ def run_forever(self) -> None: self._stop.wait(wait * (0.5 + random.random())) delay = min(delay * 2, RECONNECT_MAX_S) + def _explain_once_if_the_server_speaks_tls(self) -> None: + """Name the cause when a plain `ws://` agent meets an HTTPS server. + + Turning on TLS breaks the channel of every agent already installed: + `config.json` still says `http://`, so this dials `ws://`, and the + server now answers only TLS on that port. What the agent reports is + + [link] channel closed: Connection to remote host was lost. + + which is true and useless. Telemetry keeps flowing on its own port, so + the host looks healthy while being uncommandable - and with no + listener on the endpoint any more, uncommandable is unreachable. + + So: probe once, and only after a failure. If a TLS handshake succeeds + against the same address this just failed to reach in the clear, the + answer is not ambiguous and the agent should say it rather than leave + an operator reading a reconnect loop. + """ + import socket + import ssl as _ssl + + if self._explained_tls or not self.channel_url().startswith("ws://"): + return + self._explained_tls = True + + target = self.channel_url()[len("ws://"):].split("/", 1)[0] + host, _, port = target.partition(":") + try: + with socket.create_connection((host, int(port or 80)), timeout=5) as raw: + # Verification deliberately off: the question is "does anything + # here speak TLS", not "do we trust it", and a self-signed + # certificate is the likeliest thing on the other end. + context = _ssl.SSLContext(_ssl.PROTOCOL_TLS_CLIENT) + context.check_hostname = False + context.verify_mode = _ssl.CERT_NONE + with context.wrap_socket(raw, server_hostname=host): + pass + except Exception: + return # not a TLS server; the failure is something else + + print(f"[link] {host}:{port or 80} is serving TLS, and this agent is " + f"configured for http:// - so telemetry still flows and no " + f"command can reach this host. Set server_url to https://{target} " + f"in config.json and give it the server's CA as server_ca " + f"(GET /api/agent/ca), then restart the agent.", flush=True) + # -- one session ------------------------------------------------------- def _serve_once(self) -> None: @@ -205,11 +267,18 @@ def _serve_once(self) -> None: # would leave a window where the agent is connected and unidentified - # short, but it is the window a reconnect storm lives in, and the # version is most wanted exactly when something is going wrong. + # Only when we have one: `websocket-client` verifies against the + # system trust store by default, which is right for a real + # certificate, and passing `ca_certs=None` explicitly is not the same + # thing on every version of the library. + sslopt = {"ca_certs": self.ca_cert} if self.ca_cert else None + ws = websocket.create_connection( url, header=[f"X-Agent-Key: {self.agent_key}", f"X-Agent-Version: {AGENT_VERSION}"], - timeout=SOCKET_TIMEOUT_S) + timeout=SOCKET_TIMEOUT_S, + **({"sslopt": sslopt} if sslopt else {})) self._ws = ws self.connected = True self.last_error = "" diff --git a/app.py b/app.py index 28ff854..0f6f794 100644 --- a/app.py +++ b/app.py @@ -386,8 +386,12 @@ async def download_ca(request): 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(): + # Resolved through `core.tls`, not from a path written out again here. The + # directory moves with TLS_DIR, and a second copy of the path is how the + # route that hands out the CA comes to look somewhere the generator never + # wrote to. + ca = product_tls.ca_certificate() + if ca is None: return json({ "error": "no_local_ca", "message": "This server has no self-signed CA. That is expected " @@ -737,14 +741,59 @@ async def enroll_agent(request): proto = "https" if request.scheme == "https" else "http" host = request.host base = f"{proto}://{host}" + + # A command that cannot work is worse than no command. + # + # These were emitted unconditionally, and on a self-signed deployment + # neither one can fetch the script it is supposed to run: + # + # iwr : Temel alinan baglanti kapatildi: SSL/TLS guvenli kanali icin + # guven iliskisi kurulamadi. + # + # Nothing on the endpoint trusts this CA yet - the installer is what puts + # it there - so enrolment cannot begin. The console handed out a broken + # one-liner and said nothing about why. + # + # So when this server signs with its own CA, the snippet fetches the + # script with certificate validation suspended for that one request. That + # is the same trust on first use the enrolment token in this URL already + # depends on; the script it fetches installs the CA properly, and + # everything after that verifies. + # + # `curl.exe` on Windows, not Invoke-WebRequest with a validation callback. + # The obvious form, + # + # [Net.ServicePointManager]::ServerCertificateValidationCallback = {$true} + # + # is a PowerShell script block, and .NET invokes it on an I/O thread with + # no runspace. The callback itself throws, and what reaches the operator + # is "an unexpected error occurred on a send" - which reads as a TLS + # failure and sends you looking at protocol versions and cipher suites. + # It cost an hour of exactly that. `curl.exe` ships with Windows 10 1803 + # and later, runs out of process, and leaves no static state to restore. + self_signed = product_tls.ca_certificate() is not None + linux = f"curl -fsSL '{base}/api/agent/deploy/linux?token={token}' | sudo bash" + windows = f"iwr -useb '{base}/api/agent/deploy/windows?token={token}' | iex" + if self_signed and proto == "https": + linux = (f"curl -fsSLk '{base}/api/agent/deploy/linux?token={token}' " + f"| sudo bash") + windows = ( + f"iex (curl.exe -fsSk " + f"'{base}/api/agent/deploy/windows?token={token}' | Out-String)" + ) + return sanic_json({ "status": "success", "token": token, "expires_at": expires_at.strftime("%Y-%m-%d %H:%M:%S"), "server_url": base, + # Whether the snippets carry a first-contact exception, so the UI can + # say why rather than leaving somebody to wonder what the extra line + # is doing. + "self_signed": self_signed and proto == "https", "install": { - "linux": f"curl -fsSL '{base}/api/agent/deploy/linux?token={token}' | sudo bash", - "windows": f"iwr -useb '{base}/api/agent/deploy/windows?token={token}' | iex", + "linux": linux, + "windows": windows, } }) @@ -7118,7 +7167,58 @@ def _server_table_counts(agent: str, tables) -> dict: return counts -def _classify_link(agent_side: dict, server_rows) -> tuple[str, str]: +def _agent_db_created_at(agent: str): + """When this agent's database on the server was created, or None. + + The missing half of the "lost in transit" verdict. A table the agent has + shipped and the server does not hold reads as "accepted and discarded", + and that is one of two very different situations: + + the server took the rows and threw them away - a real fault + the server's database was recreated after they were sent + + The second is ordinary - a re-enrolment, a deleted agent, a fresh volume - + and the rows are simply gone rather than being dropped on the floor now. + Reporting it as the first sent somebody hunting a bug that is not there, + twice in one day, so the timestamp is worth one query. + """ + db = _agent_name_forms(agent)[1] + "_db" + try: + with sync_mysql_conn() as conn: + cursor = conn.cursor() + try: + cursor.execute( + f"SELECT created_at FROM {_quote_identifier(db)}" + f".ingest_epoch WHERE id = 1") + row = cursor.fetchone() + return row[0] if row else None + finally: + cursor.close() + except Exception: + # No epoch table means a database older than the stamp. Unknown is a + # fine answer; it just costs the sharper wording below. + return None + + +def _shipped_before(last_sent_at, db_created_at) -> bool: + """Whether this table was last shipped before the server's database existed. + + `last_sent_at` is a unix timestamp from the agent; `db_created_at` is a + MySQL DATETIME. Both are naive UTC on every deployment this runs on, and a + comparison that cannot be made confidently returns False - the sharper + wording is a bonus, and claiming a reset that did not happen would be a + worse error than the vague verdict it replaces. + """ + if last_sent_at is None or db_created_at is None: + return False + try: + created = db_created_at.timestamp() + except AttributeError: + return False + return float(last_sent_at) < created + + +def _classify_link(agent_side: dict, server_rows, db_created_at=None) -> tuple[str, str]: """Where the chain is broken for one table, if it is. The whole point of this file. Each of these states looked identical from @@ -7142,6 +7242,14 @@ def _classify_link(agent_side: dict, server_rows) -> tuple[str, str]: if shipped == 0 and unsent > 0: return ("queued", f"{unsent} row(s) collected and none shipped yet") if shipped > 0 and server_rows == 0: + # Two very different situations look identical here, and calling both + # of them "discarded" sent somebody hunting a bug that was not there. + if _shipped_before(agent_side.get("last_sent_at"), db_created_at): + return ("server reset", + f"the agent last shipped this before this server's " + f"database for it was created - the {shipped} row(s) went " + f"to a database that no longer exists, and rows already " + f"marked sent are re-offered once, bounded") # The state that was invisible. The agent's log says "sent (50 rows)" # every cycle and the server holds none of them. return ("lost in transit", @@ -7188,12 +7296,15 @@ async def get_telemetry_health(request, agent): agent_tables = (body or {}).get("tables") or {} server = await asyncio.to_thread( _server_table_counts, agent, sorted(agent_tables)) + # One query, so "shipped to a database that no longer exists" can be told + # apart from "shipped and discarded". + db_created_at = await asyncio.to_thread(_agent_db_created_at, agent) report = [] for table in sorted(agent_tables): side = agent_tables[table] or {} rows = server.get(table, {}).get("rows") - state, detail = _classify_link(side, rows) + state, detail = _classify_link(side, rows, db_created_at) report.append({ "table": table, "state": state, @@ -7206,6 +7317,9 @@ async def get_telemetry_health(request, agent): "server_rows": rows, }) + # `server reset` is deliberately not counted as broken. The rows are gone + # and nothing is wrong now - counting it would keep the banner red over a + # condition that resolves itself on the next re-offer. broken = [r for r in report if r["state"] in ("lost in transit", "send failing", "not collected")] return sanic_json({ diff --git a/core/installers.py b/core/installers.py index 84044e7..912ca03 100644 --- a/core/installers.py +++ b/core/installers.py @@ -91,8 +91,34 @@ def _render_linux_install(server_url: str, server_ip: str, token: str) -> str: echo "[!] Missing enrollment token"; exit 1 fi +# Trust before anything else touches the network. +# +# This used to be fetched at the end, after registration and after the binary +# download - so against a self-signed server every one of those calls failed +# first, and the CA that would have fixed them arrived two steps too late. +# +# `-k` for this one request only: it is the same trust-on-first-use the +# enrolment token and the agent binary already travel on in this script. Every +# call after it passes --cacert and verifies properly. +SERVER_CA="" +CURL_CA="" +mkdir -p "$INSTALL_DIR" +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" + CURL_CA="--cacert $SERVER_CA" + echo "[+] Stored the server CA; the rest of this install verifies against it." + else + rm -f "$INSTALL_DIR/rootCA.crt" + echo "[i] No local CA at $SERVER_URL/api/agent/ca - the server is using a certificate this host already trusts." + fi + ;; +esac + echo "[*] Registering with server..." -REG_RESP="$(curl -fsSL -X POST "$SERVER_URL/api/agents/register" \\ +REG_RESP="$(curl -fsSL $CURL_CA -X POST "$SERVER_URL/api/agents/register" \\ -H 'Content-Type: application/json' \\ -d "{{\\"token\\":\\"$TOKEN\\",\\"hostname\\":\\"$HOSTNAME_VAL\\",\\"os_type\\":\\"$OS_TYPE\\"}}")" @@ -110,30 +136,10 @@ def _render_linux_install(server_url: str, server_ip: str, token: str) -> str: cd "$INSTALL_DIR" echo "[*] Downloading agent binary..." -curl -fsSL -H "X-Agent-Key: $AGENT_KEY" -o agent.zip "$SERVER_URL/api/agent/download/linux" +curl -fsSL $CURL_CA -H "X-Agent-Key: $AGENT_KEY" -o agent.zip "$SERVER_URL/api/agent/download/linux" 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 @@ -305,6 +311,71 @@ def _render_windows_install(server_url: str, server_ip: str, token: str) -> str: return }} + # Trust before anything else touches the network. + # + # This used to happen near the end, after registration and after the + # binary download, which meant a self-signed server broke enrolment + # at the first call with + # + # Temel alinan baglanti kapatildi: SSL/TLS guvenli kanali icin + # guven iliskisi kurulamadi. + # + # and the CA that would have fixed it was fetched two steps later. + # Even the one-liner that downloads this script could not run. + # + # So: fetch the CA with validation suspended for that single request, + # then put it in the machine store. Everything after this line - here, + # in the agent, and in a browser on this host - verifies normally. + # + # Suspending validation for the fetch is the same trust-on-first-use + # the enrolment token and the agent binary already travel on in this + # very script. + # + # `curl.exe` rather than Invoke-WebRequest with a validation callback. + # The obvious form, + # + # [Net.ServicePointManager]::ServerCertificateValidationCallback = {{ $true }} + # + # does not work here and fails in a way that hides its own cause: the + # callback is a PowerShell script block, .NET invokes it on an I/O + # thread that has no runspace, and the resulting + # PSInvalidOperationException surfaces as + # + # Temel alinan baglanti kapatildi: Gonderme isleminde beklenmeyen + # hata olustu. + # + # which reads as a TLS failure and sends you looking at protocol + # versions and cipher suites. `curl.exe` ships with Windows 10 1803 + # and later, runs out of process, and confines the exception to that + # one request with no static state to restore. + $ServerCa = "" + if ($ServerUrl -like "https://*") {{ + $CaPath = Join-Path $InstallDir "rootCA.crt" + if (!(Test-Path $InstallDir)) {{ New-Item -ItemType Directory -Path $InstallDir | Out-Null }} + if (Get-Command curl.exe -ErrorAction SilentlyContinue) {{ + curl.exe -fsSk -o "$CaPath" "$ServerUrl/api/agent/ca" 2>$null + }} else {{ + Write-Host "[!] curl.exe is not available, so the server CA cannot be fetched on first contact." -ForegroundColor Yellow + Write-Host " Copy rootCA.crt from the server to $CaPath and re-run." -ForegroundColor Yellow + }} + if (-not (Test-Path $CaPath)) {{ + Write-Host "[i] No local CA at $ServerUrl/api/agent/ca - the server is using a certificate the system already trusts." -ForegroundColor DarkGray + }} + + if ((Test-Path $CaPath) -and ((Get-Item $CaPath).Length -gt 0)) {{ + $ServerCa = $CaPath + try {{ + Import-Certificate -FilePath $CaPath -CertStoreLocation "Cert:\\LocalMachine\\Root" | Out-Null + Write-Host "[+] Server CA installed in the machine trust store." -ForegroundColor Green + }} catch {{ + # Not fatal. The agent is given the CA by path in + # config.json and verifies with it either way; what is + # lost is browsers on this host trusting the console. + Write-Host "[i] Could not add the CA to the machine store ($($_.Exception.Message)); the agent will still use it by path." -ForegroundColor DarkGray + }} + }} + }} + # An upgrade is not an enrolment. This used to call /register # unconditionally, and the server allocates a fresh name whenever the # requested one is taken - so re-running this one-liner on a machine @@ -435,23 +506,6 @@ def _render_windows_install(server_url: str, server_ip: str, token: str) -> 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 diff --git a/core/tls.py b/core/tls.py index db8076c..851090b 100644 --- a/core/tls.py +++ b/core/tls.py @@ -47,6 +47,34 @@ def tls_enabled(env=None) -> bool: return str(env.get("TLS_ENABLED", "")).strip().lower() in TRUTHY +def material_dir(env=None) -> str: + """Where a generated certificate and its CA live. + + Defaults to `certs/` beside this checkout, which is right for someone + running the server directly and is where `certs/generate_certs.py` writes + when run by hand. + + `TLS_DIR` moves it, and the container sets it to the data volume. That is + not a preference: `certs/` ships inside the image, owned by root, and the + server runs as an unprivileged user - so the first boot with TLS on failed + with + + could not generate a certificate: + [Errno 13] Permission denied: '/app/certs/rootCA.key' + + and, correctly refusing to fall back to plain HTTP, took the container + into a restart loop. A generated key is state, like `data/fernet.key` + beside it: it belongs in the volume that survives a rebuild, not in the + source directory that is replaced by one. + """ + env = os.environ if env is None else env + configured = (env.get("TLS_DIR") or "").strip() + if configured: + return configured + return os.path.join( + os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "certs") + + def resolve(env=None, generate=True, log=print) -> dict | None: """The `ssl` argument for `app.run()`, or None to serve plain HTTP. @@ -124,8 +152,18 @@ def _generate(env, log) -> tuple[str, str]: ) from e cn = (env.get("TLS_CN") or "").strip() or "localhost" + outdir = material_dir(env) try: - paths = ensure_certs(cn=cn, log=log) + os.makedirs(outdir, exist_ok=True) + paths = ensure_certs(outdir=outdir, cn=cn, log=log) + except OSError as e: + # Named, because the bare errno gives no clue which of the two + # plausible causes it is - and they need opposite fixes. + raise TLSConfigError( + f"cannot write TLS material to {outdir!r}: {e}. Either point " + f"TLS_DIR at a directory this process owns, or set " + f"TLS_CERT/TLS_KEY at a certificate somebody else manages." + ) from e except Exception as e: raise TLSConfigError(f"could not generate a certificate: {e}") from e @@ -133,8 +171,8 @@ def _generate(env, log) -> tuple[str, str]: "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.") + log(f"[tls] Agents need {paths['root_crt']} to verify this server; the " + f"installer fetches it from /api/agent/ca, or set server_ca by hand.") return paths["crt"], paths["key"] @@ -155,15 +193,29 @@ def existing(env=None) -> dict | 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") + # The same directory `_generate` writes to, resolved the same way - + # two copies of this path is how the reader and the writer come to + # disagree about where the certificate is. + outdir = material_dir(env) + cert = os.path.join(outdir, "server.crt") + key = os.path.join(outdir, "server.key") if not (os.path.exists(cert) and os.path.exists(key)): return None return {"cert": cert, "key": key} +def ca_certificate(env=None) -> str | None: + """The self-signed CA an agent needs in order to verify this server. + + None when there is none, which is the ordinary answer for a deployment + using a certificate from a real CA - the endpoint's trust store already + has what it needs. + """ + path = os.path.join(material_dir(env), "rootCA.crt") + return path if os.path.exists(path) else None + + def describe(env=None) -> str: """One line for the boot banner, so the transport is visible at a glance. diff --git a/db/init.sql b/db/init.sql index df9091f..9a6d2d1 100644 --- a/db/init.sql +++ b/db/init.sql @@ -461,3 +461,24 @@ CREATE TABLE IF NOT EXISTS security_audit ( KEY idx_audit_sev (severity), KEY idx_audit_dup (dup_fp) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- ================== ingest_epoch ================== +-- +-- Which incarnation of this agent's database the server is on. +-- +-- `sent` lives on the agent and is its only record of what the server holds. +-- When this database is dropped and recreated - a re-enrolment, a deleted +-- agent, a fresh volume - the agent has no way to learn that, and by design +-- it never re-offers a row it has marked sent. Tables that produce new rows +-- constantly refill on their own; tables that rarely change stay empty for +-- ever. That is how a host sat with 93 port-scan rows held locally and none +-- on the server, with nothing wrong at either end. +-- +-- One row, written once, carrying a value that is new every time this table +-- is created. It rides back on every ingest receipt, and an agent that sees +-- it change knows the server has forgotten and offers its rows again. +CREATE TABLE IF NOT EXISTS ingest_epoch ( + id TINYINT UNSIGNED NOT NULL PRIMARY KEY, + epoch CHAR(36) NOT NULL, + created_at TIMESTAMP NULL DEFAULT CURRENT_TIMESTAMP +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; diff --git a/docker-compose.yaml b/docker-compose.yaml index 8b40f32..12ffc36 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -114,6 +114,18 @@ services: OLLAMA_BASE_URL: http://ollama:11434/api OLLAMA_MODEL: llama3.2:3b WORKERS: 1 + # Generated TLS material goes in the data volume, beside data/fernet.key + # and for the same reasons: it is state, it has to survive a rebuild, + # and it is the one directory this image prepares for the unprivileged + # user the container runs as. + # + # `/app/certs` is where it went first, and that failed on the first boot + # with TLS on: the directory ships inside the image owned by root, the + # server runs as `sentora`, and generation died with + # "Permission denied: '/app/certs/rootCA.key'". Refusing to fall back to + # plain HTTP is correct, so the container restart-looped instead - the + # right behaviour making a packaging mistake impossible to miss. + TLS_DIR: /app/data/certs # core/mq.py falls back to guest:guest when this is unset, which would # silently break publishing the moment the broker credentials change. RABBITMQ_URL: amqp://${RABBITMQ_USER:-sentora}:${RABBITMQ_PASSWORD:?RABBITMQ_PASSWORD must be set in .env}@rabbitmq/ @@ -130,18 +142,6 @@ 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 @@ -168,6 +168,11 @@ services: environment: OLLAMA_BASE_URL: http://ollama:11434/api OLLAMA_MODEL: llama3.2:3b + # The same directory `app` writes the certificate to. A container path, + # not a deployment preference, which is why it is here rather than in + # .env - and it has to be on both services or this one looks in + # /app/certs and reports a certificate that was never missing. + TLS_DIR: /app/data/certs RABBITMQ_URL: amqp://${RABBITMQ_USER:-sentora}:${RABBITMQ_PASSWORD:?RABBITMQ_PASSWORD must be set in .env}@rabbitmq/ depends_on: db: @@ -195,10 +200,12 @@ services: # `app` owns the volume and creates the key on first boot. Mounting it # read-only here means a container that starts before that cannot # quietly generate a different one and decrypt nothing. + # Read-only, which is also what makes the TLS arrangement work: `app` + # generates the certificate into this volume and this process only reads + # it. Two processes generating independently would each hold a + # certificate the other does not, and an agent that verified the console + # would then fail against ingest for no visible reason. - 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 @@ -365,8 +372,8 @@ services: volumes: mysql_data: opensearch_data: + # Holds data/fernet.key and data/certs/. Losing the key makes previously + # encrypted telemetry unreadable; losing the certificates is survivable - a + # new pair is generated - but every agent holding the old CA stops verifying + # the server and stops sending until it is re-pointed. 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 40cc063..7d60ef2 100644 --- a/docs/production-deployment.md +++ b/docs/production-deployment.md @@ -185,117 +185,357 @@ 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. 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 - TLS_ENABLED=1 - TLS_CERT=/app/certs/server.crt - TLS_KEY=/app/certs/server.key - ``` - 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 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: +> **If you are upgrading from a build before this section changed, assume the +> console was never encrypted.** `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. +> Rotate any credential that was typed into it. + +Everything below was verified by turning it on against a running stack, and +then against a real agent. Six things were wrong that no test caught, because +each needed a real container, a real certificate and a real handshake to +appear; they are called out in place, because each is a way the same mistake +gets made again. + +Three of the six were the same defect in different clothes: the agent's CA +reaching one connection and not the next. If you take one thing from this +section, take that -- trust is a property of the host, and wiring it per +connection guarantees you will miss one. + +### 3.1 What TLS covers + +Three connections, and **all three have to work or the deployment is worse +than it was**: + +| Connection | Plain | TLS | Carries | +| :--- | :--- | :--- | :--- | +| Operator → console | `http://host:8000` | `https://host:8000` | The session cookie and every credential | +| Agent → ingest | `host:5001` | `host:5011` | All telemetry: logs, paths, process names, hostnames | +| Agent → channel | `ws://host:8000/agent-link` | `wss://host:8000/agent-link` | Config push, SOAR execute, console, screen stream | + +The third is the one that bites. The agent opens the channel; the server never +dials an endpoint, and since the agent stopped listening on a port of its own +there is no second route to a host. **An agent whose channel is down but whose +telemetry still flows looks healthy and cannot be commanded** — the console +shows it reporting, and nothing reaches it. + +### 3.2 Turning it on + +```ini +# .env +TLS_ENABLED=1 +TLS_CN=soc.example.com # the name agents and browsers will use +``` + +That is enough. On first boot `app` generates a CA and a server certificate +for this deployment, `ingest` reads the same pair and opens `5011`, and the +channel URL becomes `wss://` because the agent derives it from `server_url`. + +`INGEST_TLS_PORT` moves the encrypted telemetry port off `5011`. Both sides +have to agree, and only one of them reads this variable — the agent derives +`5011` from the `https://` scheme and has no way to learn you changed it — so +an agent pointed at a moved port needs `--ingest-port` to match. Leave it +alone unless something else already owns `5011`. + +For a certificate you already have: + +```ini +TLS_ENABLED=1 +TLS_CERT=/app/data/certs/fullchain.pem +TLS_KEY=/app/data/certs/privkey.pem +``` + +Both or neither. Setting one alone is refused at startup — half a +configuration looks deliberate, and filling the other half from a generated +certificate would serve TLS the operator did not configure and will not know +is in use. + +**A certificate that cannot be loaded stops the server.** Missing, unreadable +or mismatched, it exits with a message naming the file rather than falling +back to plain HTTP. Falling back is how the original problem comes back +wearing an error nobody reads in a container log. + +#### Where the generated material lives + +`TLS_DIR`, which compose sets to `/app/data/certs` — inside the `sentora_data` +volume, beside `data/fernet.key` and for the same reasons: it is state, it has +to survive a rebuild, and it is the one directory the image prepares for the +unprivileged user the container runs as. + +> **This is the first thing that was wrong.** It went to `/app/certs`, which +> ships inside the image owned by root while the server runs as `sentora`. +> The first boot with TLS on died with +> `Permission denied: '/app/certs/rootCA.key'`, and because refusing to +> downgrade is correct, the container restart-looped instead of serving +> something weaker. The right behaviour turned a packaging mistake into an +> outage rather than a silent downgrade — which is the trade this whole +> section is built on. + +A regenerated certificate is a **new identity for the deployment**. Every +agent holding the old CA stops verifying and stops reporting. Keep `TLS_DIR` +on a volume, and back that volume up. + +`ingest` mounts the volume read-only and only ever reads. Two processes +generating independently would each hold a certificate the other does not, and +an agent that verified the console would then fail against ingest for no +visible reason. On a cold start `ingest` may lose the race and log + +``` +[!] ingest TLS was requested and cannot be served: no certificate at + certs/server.crt yet. `app` generates it on first boot... +``` + +`restart: always` brings it back once `app` has written the pair. One or two +of those lines on a first boot is expected; a stream of them means the two +services are not sharing a volume. + +### 3.3 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, +which against an attacker on the path is what not encrypting it would have +achieved. So the agent needs 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" + "server_ca": "C:\\Program Files\\Sentora-Agent\\rootCA.crt" } ``` -`SENTORA_CA_CERT` and `--ca` do the same thing. Copy `certs/rootCA.crt` from -the server — the certificate, never `rootCA.key`. +`SENTORA_CA_CERT` and `--ca` do the same thing. The installer fetches it +automatically from `/api/agent/ca` when `server_url` is `https://`. + +For an agent you are **re-pointing** rather than reinstalling, take the CA out +of the server instead of downloading it: -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 +```powershell +cd C:\path\to\Sentora +docker compose cp app:/app/data/certs/rootCA.crt "$env:USERPROFILE\Desktop\rootCA.crt" + +# then, from an *elevated* shell - the step above cannot do this one: +Copy-Item "$env:USERPROFILE\Desktop\rootCA.crt" "C:\Program Files\Sentora-Agent\rootCA.crt" -Force +``` + +Two commands, because `docker compose cp` writes the file as *you*, not as the +Docker daemon, and that fails against `C:\Program Files` even from an elevated +shell — `open ...: Access is denied`. Land it somewhere you own, then move it +with a shell that can write there. + +That is out of band — it never crosses the network — so it sidesteps the +trust-on-first-use caveat above entirely, and it avoids a trap on Windows: +`Invoke-WebRequest -SkipCertificateCheck` is PowerShell 7+ only. On Windows +PowerShell 5.1 that parameter does not exist, the download fails, and if you +have already rewritten `config.json` the agent is now pointed at `https://` +with a `server_ca` that is not there. Put the file in place **before** +restarting the agent. + +That endpoint serves the CA **certificate** and never `rootCA.key`. The +certificate is the half of the pair whose 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 it is the same trust the enrolment token and the agent +binary already travel on in the same installer run. Ship `rootCA.crt` out of +band if that is not acceptable. + +> **Second, third and fourth things wrong, all found here.** +> `/api/agent/ca` read `certs/rootCA.crt` directly while the generator wrote +> to `TLS_DIR`, so the endpoint an installer depends on for trust answered +> 404 on every containerised deployment. +> +> Then `server_ca` turned out to be reaching one connection at a time. The +> telemetry socket had it; the channel did not, so the agent shipped +> telemetry over TLS quite happily and could not open its channel at all -- +> reporting, and unreachable. Fixing that left the `requests` calls, which +> failed on the first thing the agent does: +> +> ``` +> [!] Agent bootstrap attempt 1 failed: SSLError(SSLCertVerificationError( +> certificate verify failed: unable to get local issuer certificate)) +> ``` +> +> Three fixes for one defect. `server_ca` is a statement about the *host*, +> so the agent now installs it once at startup -- as a bundle of the system +> trust store **plus** this CA, written beside the CA and pointed at by +> `REQUESTS_CA_BUNDLE`, `CURL_CA_BUNDLE` and `SSL_CERT_FILE`. Every client +> in the process picks it up, including any added later. Trusting the +> private CA *instead of* the system roots would have been the easier +> change, and would have made the agent distrust every other HTTPS +> endpoint on the machine. + +The port and the scheme are derived from `server_url`, not configured +separately: `https://` means telemetry goes to `5011` inside TLS and the +channel goes to `wss://`. Two settings that have 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. +With a certificate from a public CA, leave `server_ca` unset — the endpoint's +trust store already has what it needs. -### 3.2 Session cookies require this +### 3.4 Migrating a fleet, in this order -Whichever pattern you choose, **set `SESSION_COOKIE_SECURE=1` once TLS is -terminating anywhere in front of the UI.** The session cookie is what -authenticates the operator; without `Secure` it can travel over plain HTTP. +**Turning on TLS breaks the channel of every agent already installed.** Their +`config.json` still says `http://`, so they dial `ws://` against a port that +now answers only TLS. Telemetry keeps flowing on `5001`, so nothing looks +wrong — and no command reaches any host. -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. +The agent says so, once, rather than leaving a reconnect loop to be read: + +``` +[link] channel closed: Connection to remote host was lost. +[link] soc.example.com:8000 is serving TLS, and this agent is configured for + http:// - so telemetry still flows and no command can reach this host. + Set server_url to https://... and give it the server's CA as server_ca + (GET /api/agent/ca), then restart the agent. +``` -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. +Do it in this order: + +1. **Enable TLS on the server.** `TLS_ENABLED=1`, `TLS_CN=`, then `docker compose up -d --build app ingest`. Leave `5001` + published; both ingest listeners run. +2. **Confirm the server is serving what it says** — §3.7. +3. **Re-point the agents.** Set `server_url` to `https://…` and `server_ca` to + the CA, then restart each agent. Reinstalling does both for you. +4. **Watch the plaintext listener go quiet.** `ingest` names each agent still + arriving in the clear, once each: + ``` + [!] web-01 is sending telemetry in the clear on port 5001. Its logs, + hostnames and process names cross the network unencrypted... + ``` +5. **Close the plaintext path** when nothing is named any more: set + `INGEST_TLS_REQUIRED=1` **and remove the `"5001:5001"` line from + `docker-compose.yaml`.** + +> **Both halves of step 5, or you get the fourth thing that was wrong.** +> `INGEST_TLS_REQUIRED=1` closes the listener *inside the container*; Docker +> goes on publishing the host port. A connection is then accepted by the +> proxy, the batch is written into a socket nobody reads, and the close looks +> exactly like a server that stored everything. Verified on a live stack: the +> bytes were accepted, nothing was ingested, and the batch reported as sent. +> +> The agent now refuses to be fooled by this — once a server has acknowledged +> a batch, silence from it is treated as a failure and the rows are kept — but +> a refused connection is still the honest answer, and that means removing the +> port mapping. + +Setting `INGEST_TLS_REQUIRED=1` without `TLS_ENABLED=1` is a startup failure: +it would close the plaintext port and open nothing, leaving an ingest service +listening on nothing at all, which from the fleet's side is indistinguishable +from a quiet week. + +### 3.5 Session cookies + +**Unset, `SESSION_COOKIE_SECURE` now follows `TLS_ENABLED`.** It used to +default to `0` regardless, so turning on TLS and leaving it alone gave an +HTTPS console whose session cookie was still allowed onto plain HTTP. + +Set it explicitly to `1` when a **reverse proxy** terminates TLS — that is +invisible from inside this process, so nothing can infer it for you. + +The inverse is a real outage: `1` while serving plain HTTP means the browser +silently drops the cookie and nobody can log in. `SESSION_COOKIE_SAMESITE` defaults to `Lax`, which blocks the cross-site POST/XHR that CSRF needs. Only a split-origin deployment needs `None`, and that requires `SESSION_COOKIE_SECURE=1`. -### 3.3 Rotation +### 3.6 Terminating at a reverse proxy instead + +Leave `TLS_ENABLED` unset, serve plain HTTP at `:8000` and plain TCP at +`:5001`, and put nginx in front (§2.3). Rotation is a proxy reload, and agents +verify the proxy's certificate against the system trust store with no +`server_ca`. + +Two things this deployment still has to do by hand: + +- `SESSION_COOKIE_SECURE=1`, per §3.5. +- Terminate TLS for **ingest** too, or leave it in the clear knowingly. It is + binary TCP, not HTTP — nginx `stream`, not a `location` block. + +### 3.7 Rotating a certificate + +Replacing the **server certificate** is server-side only: + +```bash +docker compose cp new.crt app:/app/data/certs/server.crt +docker compose cp new.key app:/app/data/certs/server.key +docker compose restart app ingest +``` + +Restart both. `ingest` reads the same pair read-only, and a console on the new +certificate with telemetry still presenting the old one is the kind of +half-migration that shows up as a few hosts going quiet. -Reverse proxy: +Replacing the **CA** is not server-side only, and this is the trap. +`python certs/generate_certs.py --force` issues a new CA as well as a new +certificate, so every agent holding the old `rootCA.crt` refuses the new +server and stops sending. It is a new identity for the deployment, not a +renewal. Replace `rootCA.crt` on the endpoints first — or re-enrol them, which +does it for you. + +Agents pin nothing: they verify against the system trust store plus +`server_ca` if one is set. With a certificate from a public CA, renewal needs +nothing on the endpoints at all. + +### 3.8 Verifying it, rather than assuming it + +Run these after enabling TLS. Every one of them found something the first time. + +**The console is actually HTTPS, and only HTTPS:** ```bash -nginx -t && nginx -s reload # no downtime +curl -sk -o /dev/null -w "https=%{http_code}\n" https://localhost:8000/health +curl -s -o /dev/null -w "http=%{http_code}\n" --max-time 5 http://localhost:8000/health +# expect: https=200, http=000 ``` -App-terminated: +**The CA the server hands out verifies the certificate it presents.** This is +the test that matters, because it is what every agent does: ```bash -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 +curl -sk https://localhost:8000/api/agent/ca -o /tmp/rootCA.crt +python - <<'EOF' +import socket, ssl +ctx = ssl.create_default_context(cafile="/tmp/rootCA.crt") +for port in (8000, 5011): + with socket.create_connection(("localhost", port), timeout=8) as raw: + with ctx.wrap_socket(raw, server_hostname="localhost") as tls: + print(port, tls.version(), + dict(x[0] for x in tls.getpeercert()["subject"])["commonName"]) +EOF +# expect TLSv1.3 and the CN from TLS_CN on both ports ``` -`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. +Use Python rather than `curl --cacert` on Windows: curl's schannel backend +cannot check revocation for a private CA and reports +`CERT_TRUST_REVOCATION_STATUS_UNKNOWN` against a chain that is perfectly +valid. Python's `ssl` is what the agent uses, so it is also the honest test. + +**The plaintext listener still answers during the migration:** + +```bash +docker compose logs ingest | grep "ingest listening" +# expect both: TLS ingest on 5011, and TCP ingest on 5001 (in the clear) +``` + +**Every agent has actually moved** — this is what tells you step 5 is safe: + +```bash +docker compose logs ingest | grep "in the clear on port" +# expect nothing once the fleet has migrated +``` -**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. +**Channels are up, not just telemetry.** The dashboard's agent list carries +`channel_connected` per host; an agent that is reporting with no channel is +the failure this section keeps warning about. Check it there rather than +inferring health from row counts. -### 3.4 Why not mTLS, and what it would take +### 3.9 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: @@ -336,8 +576,7 @@ and mTLS is the thing that would help. | Path | Contains | Priority | Restore | | :--- | :--- | :--- | :--- | | 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`) | +| Volume `sentora_data` | `data/fernet.key` — the **agent** telemetry key, and `data/certs/` — this deployment's TLS certificate and CA | **Critical** | Encrypted vault (it holds two private keys) | | 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 | @@ -351,11 +590,14 @@ 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. +Losing `data/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. It lives +in `sentora_data` beside the Fernet key rather than in a volume of its own, +because `certs/` inside the image is owned by root and the server runs +unprivileged — see §3.2. The backup script tars every volume +`docker-compose.yaml` declares, so both are picked up without anybody having +to remember them. ### 4.2 Backup script diff --git a/frontend/src/pages/TelemetryHealth.tsx b/frontend/src/pages/TelemetryHealth.tsx index 994124a..a1f04c2 100644 --- a/frontend/src/pages/TelemetryHealth.tsx +++ b/frontend/src/pages/TelemetryHealth.tsx @@ -42,6 +42,10 @@ const STATE_TONE: Record = { 'send failing': 'critical', 'not collected': 'high', 'no table': 'medium', + // Not critical: the rows went to a database that has since been recreated, + // which is history rather than an active fault. It used to be reported as + // `lost in transit` and sent people hunting a bug that was not there. + 'server reset': 'low', queued: 'low', empty: 'neutral', flowing: 'ok', @@ -52,9 +56,10 @@ const STATE_RANK: Record = { 'send failing': 1, 'not collected': 2, 'no table': 3, - queued: 4, - empty: 5, - flowing: 6, + 'server reset': 4, + queued: 5, + empty: 6, + flowing: 7, }; const rank = (s: string) => STATE_RANK[s] ?? 99; diff --git a/server.py b/server.py index a34ff17..6402cbb 100644 --- a/server.py +++ b/server.py @@ -614,6 +614,16 @@ def create_tables_if_not_exist(db_name): except mysql.connector.Error as e: print(f"[!] encrypted-width migration skipped: {e}") + try: + # Stamped once per incarnation of this database. INSERT IGNORE, so + # a database that already has an identity keeps it and only a + # freshly created one gets a new value - which is exactly the + # event the agent needs to hear about. + cursor.execute( + "INSERT IGNORE INTO ingest_epoch (id, epoch) VALUES (1, UUID())") + except mysql.connector.Error as e: + print(f"[!] could not stamp the ingest epoch: {e}") + try: # Every column update_agent_info's INSERT names is declared here. # @@ -752,6 +762,29 @@ def _parse_os_info_tail(os_info: str | None): return base, hostname, mac +def _ingest_epoch(cursor) -> str: + """Which incarnation of this agent's database the server is on. + + Returned with every receipt so the agent can notice a reset. `sent` lives + on the agent and is its only record of what the server holds; when this + database is recreated the agent has no way to learn that, and by design it + never re-offers a row it marked sent. Tables that produce rows constantly + refill on their own, so the damage lands exactly on the quiet ones - a + host held 93 port-scan rows against a server holding none, with nothing + wrong at either end. + + Empty on failure rather than raising. An unknown epoch has to read as "no + information", because the alternative - the agent treating a hiccup as a + reset - would re-ship its whole history on a bad query. + """ + try: + cursor.execute("SELECT epoch FROM ingest_epoch WHERE id = 1") + row = cursor.fetchone() + return row[0] if row else "" + except Exception: + return "" + + async def insert_data(agent: str, table: str, data: list, public_ip: str = None, os_info: str = None, reported_ip: str = None) -> dict: """Store a batch and report what actually happened to it. @@ -923,7 +956,8 @@ async def insert_data(agent: str, table: str, data: list, public_ip: str = None, print(f"[!] {agent}/{table}: stored {stored}, rejected {rejected} " f"({first_error})", flush=True) return {"stored": stored, "duplicates": duplicates, - "rejected": rejected, "error": None} + "rejected": rejected, "error": None, + "epoch": _ingest_epoch(cursor)} except Exception as e: # Printed unconditionally, not behind `debug`. A batch that fails to # store is the single most important thing this process can say, and diff --git a/tests/test_agent_retention.py b/tests/test_agent_retention.py new file mode 100644 index 0000000..03cc9a3 --- /dev/null +++ b/tests/test_agent_retention.py @@ -0,0 +1,123 @@ +"""The agent's local database has to stop growing. + +Nothing pruned it, and the agent only ever appends, so it grew for the life of +the install. On one host after a few weeks: + + hardware_inventory 530,428 rows + network_connections 225,145 + fim_data 88,201 + +every one of them already shipped and never read again. Disk on a monitored +endpoint is the one place a monitoring tool has no business quietly consuming. + +It also constrained recovery. `reoffer_recent` has to be bounded, because +replaying half a million rows at fifty a batch takes days - so the backlog was +actively limiting how much could be rescued after a server-side reset. +""" +from __future__ import annotations + +import ast +import pathlib + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +MAIN = ROOT / "Sentora" / "main.py" +AGENT_DB = ROOT / "Sentora" / "modules" / "db.py" + + +def _function(path: pathlib.Path, name: str): + return next(n for n in ast.walk(ast.parse(path.read_text(encoding="utf-8"))) + if isinstance(n, ast.FunctionDef) and n.name == name) + + +def _const(path: pathlib.Path, name: str): + node = next(n for n in ast.parse(path.read_text(encoding="utf-8")).body + if isinstance(n, ast.Assign) + and any(getattr(t, "id", "") == name for t in n.targets)) + return ast.literal_eval(node.value) + + +def test_only_shipped_rows_are_deleted(): + """An unsent row has not reached the server. Deleting it is exactly the + data loss the rest of this work spent the day preventing.""" + body = ast.unparse(_function(AGENT_DB, "prune_sent")) + assert "sent = TRUE" in body, "retention would delete rows never shipped" + + +def test_a_floor_is_always_kept(): + """`DELETE FROM table WHERE sent` with no floor empties the table between + a reset and the re-offer that would have rescued it.""" + body = ast.unparse(_function(AGENT_DB, "prune_sent")) + assert "ORDER BY id DESC" in body and "OFFSET" in body + + +def test_the_floor_is_above_what_recovery_needs(): + """Pruning must not eat the material a server-side reset offers again. Set + it below `_REOFFER_LIMIT` and recovery is quietly capped by whatever the + cleaner happened to leave behind.""" + keep = _const(MAIN, "_RETENTION_ROWS") + reoffer = _const(MAIN, "_REOFFER_LIMIT") + assert keep > reoffer, ( + f"retention keeps {keep} rows and recovery wants {reoffer}" + ) + + +def test_it_runs_on_its_own(): + """A cleaner nobody calls is a cleaner that does not exist.""" + source = MAIN.read_text(encoding="utf-8") + start = ast.unparse(_function(MAIN, "start_threads")) + assert "retention_loop" in start, "the retention loop is never started" + assert "prune_sent" in source + + +def test_one_table_failing_does_not_stop_the_rest(): + """The rule every collector in this agent already follows.""" + fn = _function(MAIN, "retention_loop") + handlers = [h for h in ast.walk(fn) if isinstance(h, ast.ExceptHandler)] + assert handlers, "a single failing table aborts the whole sweep" + loop = next(n for n in ast.walk(fn) if isinstance(n, ast.For)) + assert any(isinstance(h, ast.ExceptHandler) for h in ast.walk(loop)), \ + "the handler is outside the per-table loop" + + +def test_it_does_not_run_immediately_on_startup(): + """On an upgrade the first thing this process should do is ship its + backlog, not delete it.""" + body = ast.unparse(_function(MAIN, "retention_loop")) + first_sleep = body.index("time.sleep") + first_prune = body.index("prune_sent") + assert first_sleep < first_prune, "retention runs before the first send" + + +# -------------------------------------------------------------------------- +# Nothing is shipped that nothing writes +# -------------------------------------------------------------------------- + +def _shipped_tables() -> list: + return _const(MAIN, "TABLES") + + +@pytest.mark.parametrize("table", _shipped_tables()) +def test_every_shipped_table_has_something_that_writes_it(table): + """`security_audit` sat in this list with no collector anywhere - not a + broken one, none at all. It was shipped empty every cycle and the console + showed it permanently as NOT COLLECTED, which reads as a sensor that + failed rather than one that was never built. + + A table here is a promise that this agent reports it. + """ + import re + + sources = [p for p in (ROOT / "Sentora").rglob("*.py")] + writers = [ + p.name for p in sources + if re.search(rf"insert_record(_enc)?\(\s*['\"]{table}['\"]", + p.read_text(encoding="utf-8", errors="ignore")) + or re.search(rf"TABLE\s*=\s*['\"]{table}['\"]", + p.read_text(encoding="utf-8", errors="ignore")) + ] + assert writers, ( + f"the agent ships '{table}' and nothing writes a row to it, so the " + f"console will show it as a collector that failed" + ) diff --git a/tests/test_agent_trust.py b/tests/test_agent_trust.py new file mode 100644 index 0000000..2a459e0 --- /dev/null +++ b/tests/test_agent_trust.py @@ -0,0 +1,167 @@ +"""One CA, trusted by everything the agent connects with. + +`server_ca` is a statement about the host, and it was being wired in per +connection. That cost three fixes for one defect, each with a different +symptom and the same cause: + + the telemetry socket got it - telemetry flowed over TLS + the channel did not - reporting, and uncommandable + the `requests` calls did not - bootstrap failed, retrying for ever + +The third only appeared when a real agent was pointed at a real TLS server: + + [!] Agent bootstrap attempt 1 failed: SSLError(SSLCertVerificationError( + certificate verify failed: unable to get local issuer certificate)) + +Three per-connection fixes would have left a fourth client to find later, so +trust is installed once, before anything connects, where every client - and +anything added after this was written - picks it up. + +The other half of the design is what the bundle contains. Pointing +`SSL_CERT_FILE` at the private CA *alone* would make the agent distrust every +other HTTPS endpoint on the machine to solve a problem with one of them, and +that failure would surface a long way from the line that caused it. So the +bundle is the system trust store plus this CA. +""" +from __future__ import annotations + +import ast +import os +import pathlib + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +MAIN = ROOT / "Sentora" / "main.py" + + +def _install(): + """`_install_trust_bundle`, compiled on its own.""" + fn = next(n for n in ast.parse(MAIN.read_text(encoding="utf-8")).body + if isinstance(n, ast.FunctionDef) + and n.name == "_install_trust_bundle") + namespace = {"os": os, "pathlib": pathlib, "print": lambda *a, **k: None} + exec(compile(ast.Module(body=[fn], type_ignores=[]), str(MAIN), "exec"), + namespace) + return namespace["_install_trust_bundle"] + + +@pytest.fixture +def clean_env(monkeypatch): + for name in ("REQUESTS_CA_BUNDLE", "CURL_CA_BUNDLE", "SSL_CERT_FILE"): + monkeypatch.delenv(name, raising=False) + return monkeypatch + + +@pytest.fixture +def ca(tmp_path): + from certs.generate_certs import ensure_certs + + paths = ensure_certs(outdir=str(tmp_path), cn="soc.test", days=30, + log=lambda *_: None) + return paths["root_crt"] + + +# -------------------------------------------------------------------------- +# Every client, not one +# -------------------------------------------------------------------------- + +def test_all_three_variables_are_set(clean_env, ca): + """`requests` reads the first two; `ssl.create_default_context()` with no + cafile reads the third. Between them that is every client in the + process.""" + bundle = _install()(ca) + assert bundle + for name in ("REQUESTS_CA_BUNDLE", "CURL_CA_BUNDLE", "SSL_CERT_FILE"): + assert os.environ.get(name) == bundle, f"{name} was not set" + + +def test_the_bundle_actually_verifies_that_ca(clean_env, ca, tmp_path): + """The point of the file, checked by loading it the way a client does.""" + import ssl + + bundle = _install()(ca) + context = ssl.create_default_context(cafile=bundle) + assert context.get_ca_certs(), "the bundle contains no certificates" + + subjects = str(context.get_ca_certs()) + assert "Sentora-Local-RootCA" in subjects + + +def test_the_system_roots_survive(clean_env, ca): + """The trap this avoids: trusting the private CA *instead of* everything + else. The agent would then fail against any other HTTPS endpoint, a long + way from the line that caused it.""" + certifi = pytest.importorskip("certifi") + + bundle = _install()(ca) + written = pathlib.Path(bundle).read_bytes() + system = pathlib.Path(certifi.where()).read_bytes() + + assert len(written) > len(system), \ + "the bundle is not larger than the system store, so it replaced it" + assert system in written, "the system roots are not in the bundle" + assert pathlib.Path(ca).read_bytes() in written, "the private CA is missing" + + +def test_no_ca_configured_changes_nothing(clean_env): + """The overwhelmingly common deployment: a real certificate, and nothing + to add. Writing a bundle anyway would be a file to go stale.""" + assert _install()(None) is None + assert _install()("") is None + assert os.environ.get("SSL_CERT_FILE") is None + + +def test_a_missing_ca_file_is_not_treated_as_one(clean_env, tmp_path): + """`server_ca` pointing at a file that is not there is a real state - it + happened during this migration - and it must not produce a bundle + containing nothing.""" + assert _install()(str(tmp_path / "absent.crt")) is None + assert os.environ.get("SSL_CERT_FILE") is None + + +def test_an_unwritable_location_falls_back(clean_env, ca, tmp_path, monkeypatch): + """A read-only install directory is a real deployment. The bundle is + derived rather than state, so a temp file is a correct answer.""" + import tempfile + + real_write = pathlib.Path.write_bytes + install_dir = pathlib.Path(ca).parent + + def refuse(self, data): + if self.parent == install_dir: + raise OSError("read-only install directory") + return real_write(self, data) + + monkeypatch.setattr(pathlib.Path, "write_bytes", refuse) + bundle = _install()(ca) + assert bundle, "a read-only install directory disabled trust entirely" + assert bundle.startswith(tempfile.gettempdir()) + + +# -------------------------------------------------------------------------- +# It has to run before anything connects +# -------------------------------------------------------------------------- + +def test_trust_is_installed_before_the_first_connection(): + """Bootstrap is the first thing in the process to speak TLS, and it is + what failed. Installing trust after it would fix nothing.""" + source = MAIN.read_text(encoding="utf-8") + fn = next(n for n in ast.walk(ast.parse(source)) + if isinstance(n, ast.FunctionDef) and n.name == "main") + body = ast.unparse(fn) + + install_at = body.index("_install_trust_bundle") + bootstrap_at = body.index("_init_agent_bootstrap") + assert install_at < bootstrap_at, \ + "the agent connects before it knows what to trust" + + +def test_the_explicit_paths_are_kept_as_well(): + """Belt and braces on the two connections that matter most. The env + variables cover every client; passing the CA directly means the telemetry + socket and the channel do not depend on an environment variable surviving + whatever else the process does to it.""" + source = MAIN.read_text(encoding="utf-8") + assert "ca_cert=INGEST_CA" in source, "the channel lost its explicit CA" + assert "cafile=INGEST_CA" in source, "the ingest socket lost its explicit CA" diff --git a/tests/test_ingest_acknowledgement.py b/tests/test_ingest_acknowledgement.py index 358ea59..78479d4 100644 --- a/tests/test_ingest_acknowledgement.py +++ b/tests/test_ingest_acknowledgement.py @@ -307,3 +307,47 @@ def test_rejected_rows_are_not_retried_for_ever(): 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" + + +# -------------------------------------------------------------------------- +# Silence means different things at different times +# -------------------------------------------------------------------------- + +def test_silence_from_a_server_that_has_replied_before_is_a_failure(): + """"No receipt means an older server, so mark them sent" is right exactly + once - before this server has ever replied. + + After that it is a way to lose data quietly, and there is a real + configuration that produces it. `INGEST_TLS_REQUIRED=1` closes the + plaintext listener inside the container, but Docker keeps publishing the + port: a connection is accepted by the proxy, the batch is written into a + socket nobody reads, and the close is indistinguishable from an old server + that stored everything. Verified on a live stack - the bytes were + accepted, nothing was ingested, and the batch reported as sent. + """ + body = _send_table_body() + assert "_SERVER_ACKNOWLEDGES" in body, ( + "the agent cannot tell an old server from a broken path, so a " + "silently discarded batch is reported as sent" + ) + + # The rows have to be kept, which means returning before mark_sent. + guard = body[:body.index("mark_sent")] + assert guard.count("return") >= 2, \ + "the unacknowledged-but-should-have-been path does not keep the rows" + + +def test_the_first_silence_is_still_treated_as_an_older_server(): + """The case the flag must not break: an agent pointed at a server built + before the reply frame. Treating that as failure makes it retry the same + rows for ever.""" + main = MAIN.read_text(encoding="utf-8") + tree = ast.parse(main) + assign = next(n for n in tree.body + if isinstance(n, ast.Assign) + and any(getattr(t, "id", "") == "_SERVER_ACKNOWLEDGES" + for t in n.targets)) + assert ast.literal_eval(assign.value) is False, ( + "the agent starts out assuming the server acknowledges, so a " + "genuinely old server would never have its batches marked sent" + ) diff --git a/tests/test_installer_transport.py b/tests/test_installer_transport.py index a487ae2..bdb4c31 100644 --- a/tests/test_installer_transport.py +++ b/tests/test_installer_transport.py @@ -177,3 +177,155 @@ def test_the_written_config_is_valid_json(): # Shell variables stand in for values; substitute something JSON-safe. literal = re.sub(r"\$[A-Z_]+", "x", body).strip() json.loads(literal) + + +# -------------------------------------------------------------------------- +# Enrolment has to be possible in the first place +# -------------------------------------------------------------------------- + +def test_trust_is_established_before_the_first_network_call(): + """The CA was fetched at the *end* of both installers, after registration + and after the binary download. + + Against a self-signed server every one of those calls failed first, and + the CA that would have fixed them arrived two steps too late: + + iwr : Temel alinan baglanti kapatildi: SSL/TLS guvenli kanali icin + guven iliskisi kurulamadi. + + So enrolment - the one path a machine with no Sentora on it has to walk - + was impossible the moment TLS was turned on. + """ + for name, script in (("linux", _linux("https://soc.example.com")), + ("windows", _windows("https://soc.example.com"))): + ca_at = script.index("/api/agent/ca") + register_at = script.index("/api/agents/register") + download_at = script.index("/api/agent/download/") + assert ca_at < register_at, f"{name} registers before it trusts anything" + assert ca_at < download_at, f"{name} downloads before it trusts anything" + + +def test_the_rest_of_the_install_verifies(): + """The exception is for one request. Every call after it has the CA + available and must use it - otherwise the bypass has quietly become the + transport.""" + linux = _linux("https://soc.example.com") + for call in ("/api/agents/register", "/api/agent/download/linux"): + line = next(l for l in linux.splitlines() if call in l) + assert "$CURL_CA" in line, f"{call} does not verify the server" + + windows = _windows("https://soc.example.com") + assert r"Cert:\LocalMachine\Root" in windows, ( + "the Windows installer never installs the CA, so every later call - " + "and any browser on the host - still cannot verify the console" + ) + + +def test_the_bypass_leaves_nothing_behind(): + """The first version restored a static `ServerCertificateValidationCallback` + in a `finally`, which was the right instinct about the wrong mechanism - + that callback cannot work on PS 5.1 at all (see below). + + `curl.exe` needs no restoring: the exception lives and dies with a child + process, so nothing in the PowerShell session is left unverified. This + pins that property rather than the old ceremony - if the fetch ever moves + back in-process, it has to reintroduce the restore too. + """ + code = _executable(_windows("https://soc.example.com")) + fetch = code[code.index("$CaPath = Join-Path"):] + fetch = fetch[:fetch.index("Import-Certificate")] + + assert "curl.exe" in fetch, "the first-contact fetch is in-process again" + assert "ServicePointManager" not in fetch, ( + "process-wide TLS state is being changed; if that is deliberate it " + "has to be restored, and a script-block callback still will not work" + ) + + +def test_no_bypass_when_there_is_nothing_to_bypass(): + """An http:// server has no certificate to distrust, and adding `-k` + there would be a habit that outlives the reason for it.""" + linux = _linux("http://10.0.0.5:8000") + assert "curl -fsSk" not in linux.split("case \"$SERVER_URL\"")[0] + + +@pytest.mark.parametrize("scheme,self_signed,expect_bypass", [ + ("https", True, True), # our own CA: nothing trusts it yet + ("https", False, False), # a real certificate: the snippet must stay clean + ("http", True, False), # no TLS at all +]) +def test_the_console_hands_out_a_command_that_can_run(scheme, self_signed, expect_bypass): + """The one-liner is the first thing a new host runs, and it was emitted + without regard to whether anything could verify the server.""" + app_py = (ROOT / "app.py").read_text(encoding="utf-8") + block = app_py[app_py.index('"install": {') - 3000:app_py.index('"install": {')] + assert "ca_certificate()" in block, ( + "the enrolment snippet is built without checking whether this server " + "signs with its own CA" + ) + assert 'proto == "https"' in block, \ + "the snippet would carry a TLS exception on a plain-http server" + + +# -------------------------------------------------------------------------- +# The bypass has to actually run +# -------------------------------------------------------------------------- + +def _executable(script: str) -> str: + """The script with its comment lines stripped. + + The comments here warn about the exact construct they name, so matching + the raw text finds the warning and reads it as the code. That has now + caught seven tests in this suite. + """ + return "\n".join(line for line in script.splitlines() + if not line.strip().startswith("#")) + + +def test_no_script_block_is_used_as_a_certificate_callback(): + """`ServerCertificateValidationCallback = {$true}` is the form everyone + reaches for, and on PowerShell 5.1 it does not work. + + .NET invokes the callback on an I/O thread that has no runspace, so the + script block itself throws: + + PSInvalidOperationException: There is no Runspace available to run + scripts in this thread. The script block you attempted to invoke + was: $true + + What the operator sees is "an unexpected error occurred on a send", which + reads as a TLS failure. An hour went into protocol versions, cipher + suites, ALPN and IPv6 before the inner exception was unwrapped. Any fix + here has to be a compiled type or an out-of-process client - never a + script block. + """ + code = _executable(_windows("https://soc.example.com")) + assert "ServerCertificateValidationCallback" not in code, ( + "the installer sets a certificate callback again; on PS 5.1 a script " + "block there fails on a thread with no runspace" + ) + + +def test_the_first_contact_fetch_runs_out_of_process(): + """`curl.exe` ships with Windows 10 1803 and later, has no runspace to + lose, and confines the exception to one request with no static state left + behind for the rest of the session.""" + code = _executable(_windows("https://soc.example.com")) + assert "curl.exe -fsSk" in code + + missing = _executable(_windows("https://soc.example.com")) + assert "Get-Command curl.exe" in missing, ( + "nothing says what to do on a Windows old enough to lack curl.exe" + ) + + +def test_the_console_snippet_avoids_the_same_trap(): + """The one-liner is the first thing anybody runs, and it had the same + broken callback in it.""" + app_py = (ROOT / "app.py").read_text(encoding="utf-8") + block = app_py[app_py.index("self_signed = product_tls.ca_certificate()"):] + block = block[:block.index('"install": {')] + executable = "\n".join(l for l in block.splitlines() + if not l.strip().startswith("#")) + assert "ServerCertificateValidationCallback" not in executable + assert "curl.exe -fsSk" in executable diff --git a/tests/test_server_reset_recovery.py b/tests/test_server_reset_recovery.py new file mode 100644 index 0000000..d4a2217 --- /dev/null +++ b/tests/test_server_reset_recovery.py @@ -0,0 +1,265 @@ +"""When the server forgets, the agent has to be told. + +`sent` lives on the agent and is its only record of what the server holds. +Nothing tells it when the server's database for that agent is dropped and +recreated - a re-enrolment, a deleted agent, a fresh volume - and by design a +row marked sent is never offered again. + +The damage lands entirely on the quiet tables. `fim_data` and +`network_connections` produce rows constantly, so the server refills within +minutes and nobody notices anything happened. `portscan_result`, +`process_events` and `soar_actions` do not, so they stay empty for ever. On a +live host that read as: + + portscan_result LOST IN TRANSIT held 93 unsent 0 on server 0 + +with the server holding no fingerprints for it either - not discarded, never +offered. Every layer was behaving correctly and the data was unreachable. + +The fix is one value. The server stamps each incarnation of an agent database +with an identity, returns it on every ingest receipt, and an agent that sees +it change knows the server has forgotten and offers its rows again. +""" +from __future__ import annotations + +import ast +import pathlib + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +SERVER = ROOT / "server.py" +SCHEMA = ROOT / "db" / "init.sql" +MAIN = ROOT / "Sentora" / "main.py" +AGENT_DB = ROOT / "Sentora" / "modules" / "db.py" + + +def _function(path: pathlib.Path, name: str): + return next(n for n in ast.walk(ast.parse(path.read_text(encoding="utf-8"))) + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)) + and n.name == name) + + +# -------------------------------------------------------------------------- +# The server has an identity to report +# -------------------------------------------------------------------------- + +def test_the_schema_carries_an_epoch(): + schema = SCHEMA.read_text(encoding="utf-8") + assert "CREATE TABLE IF NOT EXISTS ingest_epoch" in schema + + +def test_it_is_stamped_once_per_database(): + """`INSERT IGNORE` on a fixed primary key: a database that already has an + identity keeps it, and only a freshly created one gets a new value. A + plain INSERT would change the epoch on every batch and make the agent + resend its history for ever.""" + source = SERVER.read_text(encoding="utf-8") + stamp = next(line for line in source.splitlines() + if "INSERT IGNORE INTO ingest_epoch" in line) + assert "id, epoch" in stamp and "UUID()" in stamp + + +def test_every_receipt_carries_it(): + """The agent has no other way to hear about a reset. A receipt without it + is a receipt that cannot report the one thing this exists for.""" + body = ast.unparse(_function(SERVER, "insert_data")) + success = body[body.index("conn.commit()"):] + success = success[:success.index("except Exception")] + assert "'epoch'" in success or '"epoch"' in success + + +def test_an_unreadable_epoch_reads_as_no_information(): + """Not as a reset. The agent treating a failed query as "the server + forgot" would make it re-ship its whole history on a bad connection.""" + fn = _function(SERVER, "_ingest_epoch") + handler = next(h for h in ast.walk(fn) if isinstance(h, ast.ExceptHandler)) + body = ast.unparse(handler) + assert "return ''" in body or 'return ""' in body + + +# -------------------------------------------------------------------------- +# The agent acts on it +# -------------------------------------------------------------------------- + +def test_a_changed_epoch_re_offers_rows(): + body = ast.unparse(_function(MAIN, "_handle_server_epoch")) + assert "reoffer_recent" in body + assert "TABLES" in body, "only some tables would be recovered" + + +def test_the_first_sighting_also_re_offers(): + """The obvious rule - "nothing to compare against, so do nothing" - is + wrong, and this test exists because it was written that way first. + + The agents that most need recovery are exactly the ones that have been + running since before the epoch existed, holding rows marked sent against a + server that may never have received them. Skipping the first sighting + fixes the next reset and leaves the current damage in place, which on the + host this was written for meant 93 port-scan rows that were never coming + back. + + It is safe because it costs nothing where it does not apply: a fresh + install has an empty local database and the sweep finds no rows, and an + agent already in sync offers rows the server recognises and skips. + """ + body = ast.unparse(_function(MAIN, "_handle_server_epoch")) + guard = body[:body.index("reoffer_recent")] + assert "return" not in guard.split("if not epoch")[-1].split("previous")[0] \ + or "if previous" in guard, \ + "a first sighting still returns early, so stranded rows stay stranded" + assert "if previous" in body, \ + "the two cases are no longer distinguished in the message" + + +def test_an_absent_epoch_changes_nothing(): + """A server older than this sends no epoch, and that has to keep + working.""" + body = ast.unparse(_function(MAIN, "_handle_server_epoch")) + assert "if not epoch" in body + + +def test_it_survives_a_restart(): + """A reset that happens while the agent is stopped is the likeliest case - + that is when somebody deletes an agent and reinstalls it. Holding the + epoch only in memory would miss exactly that.""" + source = MAIN.read_text(encoding="utf-8") + assert "_load_epoch()" in source + main_body = ast.unparse(_function(MAIN, "main")) + assert "_load_epoch()" in main_body, \ + "the remembered epoch is never loaded at startup" + + +def test_the_resend_is_bounded(): + """`hardware_inventory` on one host held 530,000 rows. Re-offering all of + them at fifty a batch is a replay measured in days, during which nothing + current gets through - a recovery that costs more than the loss.""" + source = MAIN.read_text(encoding="utf-8") + assign = next(n for n in ast.parse(source).body + if isinstance(n, ast.Assign) + and any(getattr(t, "id", "") == "_REOFFER_LIMIT" + for t in n.targets)) + limit = ast.literal_eval(assign.value) + assert 0 < limit <= 5000, f"_REOFFER_LIMIT is {limit}" + + body = ast.unparse(_function(AGENT_DB, "reoffer_recent")) + assert "LIMIT" in body, "the re-offer is unbounded" + assert "ORDER BY id DESC" in body, \ + "an unordered LIMIT re-offers arbitrary rows rather than the newest" + + +def test_resending_cannot_duplicate(): + """The resend is only safe because the server deduplicates. If a table + were ever re-offered into a server that stores blindly, recovery would + become corruption.""" + dedup = next(n for n in ast.parse(SERVER.read_text(encoding="utf-8")).body + if isinstance(n, ast.Assign) + and any(getattr(t, "id", "") == "DEDUP_TABLES" for t in n.targets)) + deduplicated = set(ast.literal_eval(dedup.value)) + snapshot = next(n for n in ast.parse(SERVER.read_text(encoding="utf-8")).body + if isinstance(n, ast.Assign) + and any(getattr(t, "id", "") == "SNAPSHOT_TABLES" for t in n.targets)) + snapshotted = set(ast.literal_eval(snapshot.value)) + + # Every table the agent ships is one or the other, so a re-offered row is + # either skipped as a duplicate or replaces the previous picture. + allowed = next(n for n in ast.parse(SERVER.read_text(encoding="utf-8")).body + if isinstance(n, ast.Assign) + and any(getattr(t, "id", "") == "ALLOWED_TABLES" for t in n.targets)) + unprotected = set(ast.literal_eval(allowed.value)) - deduplicated - snapshotted + assert not unprotected, ( + f"{sorted(unprotected)} would accumulate duplicates on a re-offer" + ) + + +# -------------------------------------------------------------------------- +# The bounded update itself +# -------------------------------------------------------------------------- + +def test_reoffer_reports_what_it_changed(): + """Silence here is how a recovery that did nothing looks like one that + worked.""" + fn = _function(AGENT_DB, "reoffer_recent") + returns = [n for n in ast.walk(fn) if isinstance(n, ast.Return)] + assert returns and all(n.value is not None for n in returns) + assert "rowcount" in ast.unparse(fn) + + +def test_the_operator_is_told_it_happened(): + """Rows quietly reappearing is indistinguishable from a bug. The log has + to name the event, because it explains a burst of traffic somebody will + otherwise investigate.""" + body = ast.unparse(_function(MAIN, "_handle_server_epoch")) + assert "recreated" in body + + +# -------------------------------------------------------------------------- +# The console has to name the right cause +# -------------------------------------------------------------------------- + +def test_a_reset_is_not_reported_as_discarded(): + """"the agent has shipped 93 row(s) and this server holds none - accepted + and discarded" is one of two situations, and only one of them is a fault: + + the server took the rows and threw them away + the server's database was recreated after they were sent + + Reporting the second as the first sent somebody hunting a bug that was not + there, twice in one day. The timestamp that separates them costs one + query. + """ + app_py = (ROOT / "app.py").read_text(encoding="utf-8") + fn = next(n for n in ast.walk(ast.parse(app_py)) + if isinstance(n, ast.FunctionDef) and n.name == "_classify_link") + body = ast.unparse(fn) + assert "server reset" in body + assert "_shipped_before" in body + + +def test_the_comparison_refuses_to_guess(): + """A missing timestamp on either side has to mean "cannot tell", not + "reset". Claiming a reset that did not happen is a worse error than the + vague verdict it replaces.""" + app_py = (ROOT / "app.py").read_text(encoding="utf-8") + fn = next(n for n in ast.walk(ast.parse(app_py)) + if isinstance(n, ast.FunctionDef) and n.name == "_shipped_before") + namespace: dict = {} + exec(compile(ast.Module(body=[fn], type_ignores=[]), "app.py", "exec"), + namespace) + shipped_before = namespace["_shipped_before"] + + import datetime as dt + assert shipped_before(None, dt.datetime(2026, 1, 1)) is False + assert shipped_before(1000.0, None) is False + assert shipped_before(1000.0, "not a datetime") is False + + created = dt.datetime(2026, 9, 4, 12, 0, 0) + assert shipped_before(created.timestamp() - 60, created) is True + assert shipped_before(created.timestamp() + 60, created) is False + + +def test_a_reset_does_not_keep_the_banner_red(): + """The rows are gone and nothing is wrong now. Counting it as broken would + hold the console at a warning over a condition that resolves itself on the + next re-offer.""" + app_py = (ROOT / "app.py").read_text(encoding="utf-8") + fn = next(n for n in ast.walk(ast.parse(app_py)) + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)) + and n.name == "get_telemetry_health") + broken = ast.unparse(fn) + broken = broken[broken.index("broken ="):] + assert "server reset" not in broken.split("]")[0] + + +def test_the_console_knows_the_new_state(): + """A state the API can return and the UI has never heard of renders as a + neutral chip with no meaning - which is how it looked before any of this + had names.""" + page = (ROOT / "frontend" / "src" / "pages" / "TelemetryHealth.tsx").read_text( + encoding="utf-8") + assert "'server reset'" in page + tone = page[page.index("const STATE_TONE"):page.index("const STATE_RANK")] + assert "'server reset'" in tone + rank = page[page.index("const STATE_RANK"):] + rank = rank[:rank.index("};")] + assert "'server reset'" in rank, "the new state sorts to the bottom by default" diff --git a/tests/test_telemetry_health.py b/tests/test_telemetry_health.py index 6925ff7..a39a943 100644 --- a/tests/test_telemetry_health.py +++ b/tests/test_telemetry_health.py @@ -36,12 +36,21 @@ def _classifier(): - """`_classify_link`, compiled without importing app.py.""" + """`_classify_link`, compiled without importing app.py. + + With the helpers it calls. Compiling the function alone raised + `NameError: _shipped_before` the moment it grew a dependency, which reads + as a bug in app.py rather than in this harness - so the helpers are lifted + with it by name. + """ tree = ast.parse(APP.read_text(encoding="utf-8")) - fn = next(n for n in ast.walk(tree) - if isinstance(n, ast.FunctionDef) and n.name == "_classify_link") + wanted = ("_classify_link", "_shipped_before") + fns = [n for n in ast.walk(tree) + if isinstance(n, ast.FunctionDef) and n.name in wanted] + assert len(fns) == len(wanted), f"missing one of {wanted} in app.py" + namespace: dict = {} - exec(compile(ast.Module(body=[fn], type_ignores=[]), "", "exec"), + exec(compile(ast.Module(body=fns, type_ignores=[]), "", "exec"), namespace) return namespace["_classify_link"] diff --git a/tests/test_tls_documentation.py b/tests/test_tls_documentation.py new file mode 100644 index 0000000..b601781 --- /dev/null +++ b/tests/test_tls_documentation.py @@ -0,0 +1,166 @@ +"""The TLS guide has to describe the code, not a plan for it. + +This suite exists because the previous version of that guide described three +settings the code did not read. It was not careless writing - it was written +first, the implementation slipped, and nothing tied the two together. An +operator who set `TLS_ENABLED=1` got a console on plain HTTP and no reason to +doubt it. + +So these tests pin the load-bearing claims in `docs/production-deployment.md` +to the code that has to make them true. They are deliberately about +*existence*, not prose: a doc test that checks wording becomes a chore that +gets deleted, while one that checks a documented setting is still read catches +the failure that actually happened. +""" +from __future__ import annotations + +import ast +import pathlib +import re + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +GUIDE = ROOT / "docs" / "production-deployment.md" +ENV_EXAMPLE = ROOT / ".env.example" +TLS = ROOT / "core" / "tls.py" +SERVER = ROOT / "server.py" +COMPOSE = ROOT / "docker-compose.yaml" + + +def _guide() -> str: + return GUIDE.read_text(encoding="utf-8") + + +def _tls_section() -> str: + text = _guide() + start = text.index("## 3. TLS / Certificate Handling") + return text[start:text.index("\n## 4.", start)] + + +# -------------------------------------------------------------------------- +# Every setting the guide names is a setting something reads +# -------------------------------------------------------------------------- + +DOCUMENTED_SETTINGS = [ + ("TLS_ENABLED", TLS), ("TLS_CERT", TLS), ("TLS_KEY", TLS), + ("TLS_CN", TLS), ("TLS_DIR", TLS), + ("INGEST_TLS_REQUIRED", SERVER), ("INGEST_TLS_PORT", SERVER), +] + + +@pytest.mark.parametrize("name,module", DOCUMENTED_SETTINGS, + ids=[n for n, _ in DOCUMENTED_SETTINGS]) +def test_a_documented_setting_is_read_somewhere(name, module): + """The exact failure this file exists for: three variables in the guide, + in `.env.example`, and in the generator's closing output - and read by no + line of code.""" + assert name in _tls_section(), f"{name} is implemented but undocumented" + assert name in module.read_text(encoding="utf-8"), ( + f"the guide documents {name} and {module.name} never reads it" + ) + + +@pytest.mark.parametrize("name,_module", DOCUMENTED_SETTINGS, + ids=[n for n, _ in DOCUMENTED_SETTINGS]) +def test_the_example_env_agrees_with_the_guide(name, _module): + """Two places an operator looks for the same answer.""" + assert name in ENV_EXAMPLE.read_text(encoding="utf-8"), \ + f"{name} is in the guide and not in .env.example" + + +# -------------------------------------------------------------------------- +# Claims that would send somebody to the wrong place +# -------------------------------------------------------------------------- + +def test_the_guide_does_not_send_anyone_to_the_old_certificate_path(): + """`/app/certs` is the path that failed with EACCES and restart-looped the + container. It survives in the guide only inside the warning about it.""" + section = _tls_section() + outside_the_warning = [ + line for line in section.splitlines() + if "/app/certs" in line and not line.lstrip().startswith(">") + ] + assert not outside_the_warning, ( + "the guide still points at /app/certs outside the warning: " + + "; ".join(outside_the_warning) + ) + + +def test_no_stale_volume_is_referenced(): + """A `sentora_certs` volume existed for about an hour. A guide naming a + volume compose does not declare sends the reader to look for something + that was never created.""" + import yaml + + declared = set(yaml.safe_load(COMPOSE.read_text(encoding="utf-8"))["volumes"]) + for named in re.findall(r"`(sentora_\w+)`", _guide()): + assert named in declared, f"the guide names {named}, compose does not declare it" + + +def test_the_documented_tls_dir_is_the_one_compose_sets(): + """A path written out in prose and again in compose is a path that drifts.""" + import yaml + + compose = yaml.safe_load(COMPOSE.read_text(encoding="utf-8")) + configured = compose["services"]["app"]["environment"]["TLS_DIR"] + assert configured in _tls_section(), ( + f"compose sets TLS_DIR={configured} and the guide documents a " + f"different path" + ) + + +def test_the_ports_table_covers_the_tls_listener(): + """5011 exists; a ports table without it is a firewall rule nobody opens.""" + ports = _guide()[_guide().index("### 2.2 Ports"):] + ports = ports[:ports.index("###", 10)] + assert "5011" in ports + assert "5001" in ports + + +# -------------------------------------------------------------------------- +# The two-part step, because half of it silently loses data +# -------------------------------------------------------------------------- + +def test_closing_the_plaintext_path_documents_both_halves(): + """`INGEST_TLS_REQUIRED=1` closes the listener inside the container while + Docker goes on publishing the port. A connection is then accepted by the + proxy and the batch written into a socket nobody reads - which is why the + port mapping has to go too, and why the guide has to say so.""" + section = _tls_section() + assert "INGEST_TLS_REQUIRED=1" in section + assert '"5001:5001"' in section, ( + "the guide tells the operator to require TLS without telling them to " + "stop publishing the plaintext port" + ) + + +def test_the_agent_side_of_that_trap_is_implemented(): + """The guide claims the agent will not be fooled by an accepted-and- + discarded batch. That claim depends on it remembering that this server has + acknowledged before.""" + main = (ROOT / "Sentora" / "main.py").read_text(encoding="utf-8") + assert "_SERVER_ACKNOWLEDGES" in main + + +# -------------------------------------------------------------------------- +# The verification steps have to be runnable +# -------------------------------------------------------------------------- + +def test_the_verification_section_exists_and_is_concrete(): + """"Verify TLS is working" without commands is a step everybody skips.""" + section = _tls_section() + verify = section[section.index("Verifying it, rather than assuming it"):] + assert "/api/agent/ca" in verify + assert "create_default_context" in verify, \ + "the check that matters - does the CA verify the served cert - is missing" + assert "in the clear on port" in verify, \ + "nothing tells the operator how to know the fleet has finished migrating" + + +def test_the_windows_curl_caveat_is_recorded(): + """`curl --cacert` on Windows reports CERT_TRUST_REVOCATION_STATUS_UNKNOWN + against a perfectly valid private chain. Without this note the first + verification step looks like a failure and somebody 'fixes' a working + deployment.""" + assert "schannel" in _tls_section() diff --git a/tests/test_tls_end_to_end.py b/tests/test_tls_end_to_end.py new file mode 100644 index 0000000..fa240bf --- /dev/null +++ b/tests/test_tls_end_to_end.py @@ -0,0 +1,246 @@ +"""What only turning TLS on could have told us. + +`tests/test_tls_serving.py` and `tests/test_ingest_tls.py` both passed against +a build where enabling TLS took the whole stack down. They were right about +every question they asked; they asked about the wrong layer. Three defects +survived them, and each needed a real container, a real certificate and a real +handshake to appear: + +**The certificate could not be written.** `certs/` ships inside the image +owned by root, the server runs as `sentora`, and generation died with +`Permission denied: '/app/certs/rootCA.key'`. Refusing to fall back to plain +HTTP is correct, so the container restart-looped rather than serving something +weaker - the right behaviour turning a packaging mistake into an outage +instead of a silent downgrade. Generated material now lives in the data +volume, beside `data/fernet.key` and for the same reasons. + +**The CA route read a path of its own.** `/api/agent/ca` looked in `certs/` +while the generator wrote to `TLS_DIR`, so the endpoint an installer depends +on for trust would have answered 404 on every containerised deployment. + +**The control channel never received the CA.** `server_ca` reached the +telemetry socket and stopped there. Against a self-signed server the agent +shipped telemetry over TLS quite happily and could not open its channel at +all - and since the endpoint no longer listens on a port of its own, that is +an agent which reports and cannot be reached. Config, console, SOAR and the +screen stream all travel on that channel. + +The pattern in all three: TLS is not one switch. It is a certificate that has +to be writable, discoverable by every process that needs it, and trusted by +both of the agent's connections. Any one of those missing produces a +deployment that looks encrypted from wherever you happen to be looking. +""" +from __future__ import annotations + +import ast +import pathlib + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +COMPOSE = ROOT / "docker-compose.yaml" +LINK = ROOT / "Sentora" / "modules" / "link.py" +MAIN = ROOT / "Sentora" / "main.py" + +from core import tls # noqa: E402 + + +# -------------------------------------------------------------------------- +# Where the material lives +# -------------------------------------------------------------------------- + +def test_the_material_directory_is_configurable(): + """It was hard-coded to `certs/`, which the container cannot write to.""" + assert tls.material_dir(env={"TLS_DIR": "/somewhere"}) == "/somewhere" + + +def test_it_defaults_to_the_checkout(): + """Unchanged for anyone running the server directly, which is also where + `certs/generate_certs.py` writes when run by hand.""" + assert tls.material_dir(env={}).replace("\\", "/").endswith("/certs") + + +def test_the_reader_and_the_writer_agree(): + """`existing()` is what `ingest` uses to find what `app` generated. Two + copies of the path is how the two come to disagree about where the + certificate is - and the symptom is ingest reporting a missing + certificate that is sitting right there.""" + env = {"TLS_ENABLED": "1", "TLS_DIR": "/somewhere"} + source = (ROOT / "core" / "tls.py").read_text(encoding="utf-8") + + fn = next(n for n in ast.parse(source).body + if isinstance(n, ast.FunctionDef) and n.name == "existing") + assert "material_dir" in ast.unparse(fn) + + gen = next(n for n in ast.parse(source).body + if isinstance(n, ast.FunctionDef) and n.name == "_generate") + assert "material_dir" in ast.unparse(gen) + + # And nothing is found where nothing was written. + assert tls.existing(env=env) is None + + +def test_generation_reports_an_unwritable_directory_by_name(tmp_path): + """The bare errno gives no clue which of the two plausible causes it is, + and they need opposite fixes: point TLS_DIR somewhere writable, or hand + over a certificate somebody else manages. + + The unwritable path is a *file* with a directory path underneath it, which + fails the same way on both platforms. The first version of this test used + `/proc/nonexistent/certs`, which is unwritable on Linux and an ordinary + relative path on Windows - so it created `C:\\proc\\nonexistent\\certs` and + generated a real key inside it. A POSIX assumption in a test that exists to + catch a filesystem-permission bug. + """ + blocker = tmp_path / "not-a-directory" + blocker.write_text("", encoding="utf-8") + + with pytest.raises(tls.TLSConfigError) as excinfo: + tls.resolve(env={"TLS_ENABLED": "1", + "TLS_DIR": str(blocker / "certs")}, + log=lambda *_: None) + assert "TLS_DIR" in str(excinfo.value) + + +def test_both_services_are_told_where_it_is(): + """A container path, so it belongs in compose - and on *both* services. + On one only, ingest looks in /app/certs and reports a certificate that + was never missing.""" + import yaml + + compose = yaml.safe_load(COMPOSE.read_text(encoding="utf-8")) + for service in ("app", "ingest"): + env = compose["services"][service]["environment"] + assert env.get("TLS_DIR"), f"{service} is not told where TLS material lives" + assert (compose["services"]["app"]["environment"]["TLS_DIR"] + == compose["services"]["ingest"]["environment"]["TLS_DIR"]) + + +def test_the_material_lives_on_a_volume_the_server_owns(): + """`/app/certs` ships in the image owned by root and the server runs + unprivileged. `/app/data` is the directory the Dockerfile chowns, which is + why the Fernet key already lives there.""" + import yaml + + compose = yaml.safe_load(COMPOSE.read_text(encoding="utf-8")) + tls_dir = compose["services"]["app"]["environment"]["TLS_DIR"] + mounts = compose["services"]["app"]["volumes"] + assert any(m.split(":")[1] and tls_dir.startswith(m.split(":")[1]) + for m in mounts if ":" in m), ( + f"TLS_DIR={tls_dir} is not inside a mounted volume, so the " + f"certificate is regenerated on every rebuild - which changes the " + f"deployment's identity and stops every agent that trusted the old CA" + ) + + dockerfile = (ROOT / "Dockerfile").read_text(encoding="utf-8") + assert "chown -R sentora:sentora /app/data" in dockerfile, \ + "the directory TLS_DIR sits in is no longer prepared for the app user" + + +# -------------------------------------------------------------------------- +# The CA has to be findable by the route that hands it out +# -------------------------------------------------------------------------- + +def test_the_ca_route_follows_the_same_directory(): + """It read `certs/rootCA.crt` directly while the generator wrote to + TLS_DIR, so the endpoint an installer depends on for trust answered 404 + on every containerised deployment.""" + app_py = (ROOT / "app.py").read_text(encoding="utf-8") + fn = next(n for n in ast.walk(ast.parse(app_py)) + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)) + and n.name == "download_ca") + body = ast.unparse(fn) + assert "ca_certificate" in body + assert '"certs"' not in body and "'certs'" not in body + + +def test_the_ca_helper_never_reaches_for_the_key(): + """The distinction the design rests on: one half of the pair is meant to + be distributed, the other signs everything this deployment vouches for.""" + source = (ROOT / "core" / "tls.py").read_text(encoding="utf-8") + fn = next(n for n in ast.parse(source).body + if isinstance(n, ast.FunctionDef) and n.name == "ca_certificate") + body = list(fn.body) + if body and isinstance(body[0], ast.Expr) and isinstance(body[0].value, ast.Constant): + body = body[1:] + code = "\n".join(ast.unparse(n) for n in body) + assert "rootCA.crt" in code + assert "rootCA.key" not in code + + +# -------------------------------------------------------------------------- +# Both of the agent's connections have to trust the same server +# -------------------------------------------------------------------------- + +def test_the_channel_accepts_a_ca(): + """Telemetry and control go to the same server over two connections. The + CA reached one of them.""" + source = LINK.read_text(encoding="utf-8") + init = source[source.index(" def __init__(self, server_url"):] + init = init[:init.index(" # -- addressing")] + assert "ca_cert" in init, "AgentLinkClient cannot be given a CA" + + serve = source[source.index(" def _serve_once"):] + serve = serve[:serve.index(" def _send(")] + assert "sslopt" in serve, "the CA is accepted and then not used" + assert "ca_certs" in serve + + +def test_the_agent_hands_the_same_ca_to_both(): + """One `server_ca` for the host. Giving it to only the telemetry socket + produced an agent that reported over TLS and was uncommandable - and with + no listener on the endpoint any more, uncommandable is unreachable.""" + source = MAIN.read_text(encoding="utf-8") + call = source[source.index("client = link.AgentLinkClient("):] + call = call[:call.index(")\n")] + assert "ca_cert=INGEST_CA" in call, ( + "the channel is constructed without the CA the telemetry socket uses" + ) + + +#: The functions that establish a connection the agent then trusts. Scoped to +#: these rather than the whole file, because one function deliberately does +#: not verify and must not be lumped in with them - see the test below. +CONNECTING_FUNCTIONS = [ + (MAIN, "_ingest_socket"), + (LINK, "_serve_once"), +] + + +@pytest.mark.parametrize("path,name", CONNECTING_FUNCTIONS, + ids=[n for _, n in CONNECTING_FUNCTIONS]) +def test_verification_is_never_switched_off_to_make_it_work(path, name): + """The tempting fix when a handshake fails. An unverified TLS connection + is encrypted to whoever answered, which against an attacker on the path is + what not encrypting it would have achieved.""" + fn = next(n for n in ast.walk(ast.parse(path.read_text(encoding="utf-8"))) + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)) + and n.name == name) + body = ast.unparse(fn) + for escape in ("CERT_NONE", "check_hostname = False", + "_create_unverified_context", "'cert_reqs': 0"): + assert escape not in body, f"{name} disables verification ({escape})" + + +def test_the_one_unverified_handshake_carries_nothing(): + """`_explain_once_if_the_server_speaks_tls` deliberately does not verify, + and that is correct: it asks "does anything here speak TLS", not "do I + trust it", and the likeliest answer is a self-signed certificate it could + not verify by definition. + + What makes it safe is that it sends nothing, reads nothing, and returns + nothing to the caller - it exists to turn an unexplained reconnect loop + into one line naming the cause. This pins that: the moment it starts + carrying data it becomes an unverified connection like any other. + """ + fn = next(n for n in ast.walk(ast.parse(LINK.read_text(encoding="utf-8"))) + if isinstance(n, ast.FunctionDef) + and n.name == "_explain_once_if_the_server_speaks_tls") + body = ast.unparse(fn) + + assert "CERT_NONE" in body, "the probe now verifies, so it cannot answer" + for carries_data in ("send", "recv", "return raw", "self._ws"): + assert carries_data not in body, ( + f"the diagnostic probe does {carries_data!r} on an unverified " + f"connection; it may only observe that a handshake succeeds" + )