Add security keys, give security_audit a collector, and finish the console - #49
Merged
Merged
Conversation
…nsole Derive the WebAuthn relying party from the request so there is nothing to configure, write the posture collector the schema had carried since the beginning with no writer, add TLS_SAN and report a certificate that predates the configured names, authenticate the automations poll so the missed-push fallback exists, stop reporting a Windows vulnerability scan that could never match as clean, make the search query builder faithful to the filter that was clicked, keep the MAC address and the rotated database password out of logs, move the last pages onto the shared kit with real loading and failure states, and correct the agent module reference.
| # Once, here, rather than on every table send. This is the banner that | ||
| # used to be repeated thousands of times a day in agent.log. | ||
| print(f"[*] Host: {OS_INFO}") | ||
| print(f"[*] Host: {_os_info_for_log()}") |
| f"({'server' if (AUTOMATIONS_MODE == 'server' or (AUTOMATIONS_MODE == 'auto' and AUTOMATIONS_API_URL)) else 'db'})") | ||
| print(f"[*] Public IP (auto-detected): {get_public_ip()}") | ||
| print(f"[*] OS Info: {OS_INFO}") | ||
| print(f"[*] OS Info: {_os_info_for_log()}") |
| # alone can read gives the same recovery with none of that. | ||
| rescue = ENV_PATH.with_name(".db_password_rescue") | ||
| try: | ||
| rescue.write_text(f"DB_PASSWORD={new_password}\n", encoding="utf-8") |
| f"and the rescue file could not be written ({write_error}).") | ||
| print(f" {e}") | ||
| print("\n This is the only copy. Save it now:") | ||
| print(f" DB_PASSWORD={new_password}") # nosec - see above |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security keys, a table that never had a collector, and the console around them
Verified against the running deployment rather than only in tests — which
turned out to matter, because four of the bugs below were found by clicking the
button, after the tests for them had passed.
1. Security keys (WebAuthn)
A one-time code typed into a convincing copy of this login page works on the
real one, and the operator has no way to tell. A key signs over the origin it
is actually talking to.
There is no configuration, and that was the design question. The
relying-party ID is derived from the request, so a laptop at
https://localhost:8000and an estate athttps://soc.example.comboth workwithout being told anything, and no setting can disagree with the URL people
type.
The cost is that some origins cannot host a key, and the console says which
rather than showing a button that fails:
https://soc.example.comhttp://localhost:8000https://10.0.0.5:8000http://soc.example.comTLS_ENABLED=1.webauthnpackage absentOne-time codes are unaffected in every row, and the message says so — an
operator who reads "unavailable" as "two-factor is off here" turns the whole
thing off.
Three decisions worth reviewing:
localhostis onethe browser will not offer at a hostname. Without the column the console
lists a factor that cannot work and the prompt times out silently.
_has_security_keyis deliberately not scoped to the RP ID. Whether alogin needs a second factor is a property of the account; whether this
browser can offer a key is a property of the origin. Conflating them would
let somebody with a key registered at a hostname skip the second factor
entirely by opening the console at an address — turning the origin binding,
the whole point of WebAuthn, into a bypass.
last one seen is refused and raises
WEBAUTHN_COUNTER_REGRESSED(CRITICAL).Zero means "not implemented" — true of most platform passkeys — and is
accepted, because refusing it would switch the control off for the hardware
people actually have.
Verified end to end on a real authenticator:
base64url is not base64
frontend/src/lib/webauthn.tshas its own tests because this is where WebAuthnfails intermittently.
-and_stand in for+and/, padding is dropped,and
atobaccepts neither. The alphabets agree until a byte lands on one ofthe four differing characters — so an implementation that forgets to translate
passes a demo, passes review, then throws
InvalidCharacterErrorfor oneoperator in three.
2. Certificate names (
TLS_SAN)The shipped certificate carried
DNS:localhost, DNS:localhost— the SAN listwas
[cn, "localhost"]with no dedup and no way to add a name. A console athttps://10.0.0.5:8000failed hostname verification in every browser, on topof the untrusted-CA warning: two errors that look like one.
TLS_SANtakes hostnames and addresses; anything that parses as an addressbecomes an IP SAN, because a bare address in the DNS list matches nothing.
Generation is idempotent, so editing
.envon a deployment that alreadyhas a certificate does nothing — correctly, since a new identity per restart
would break every agent that pinned the CA. It now says so at startup with the
command to fix it. That check silently vanished on its first draft
(
except: returnmade "unreadable" and "names match" produce identicaloutput); it has its own test.
docs/production-deployment.md§3.8 is the six-step version, including that--forceissues a new CA, and what happens if you skip all of it: nothingbreaks, you just do not get a security key.
3.
security_audit: the table nothing ever wrote toIt existed in both schemas, in
ALLOWED_TABLES, inDEDUP_TABLESand in twoencrypted-field maps since the beginning. Three modules were documented as
producing it. None of them ever did. So it shipped empty every cycle and
the console showed it permanently as NOT COLLECTED — a sensor that reads as
broken rather than as never built.
Sentora/modules/security_audit.pyis the collector: posture findings, readonly. Windows (registry, services, local accounts, firewall) and Linux
(
sshd_config, sudoers,/etc/passwd, file modes).Running it on a real host immediately produced three bugs worth keeping:
WinDefend,WdNisSvcandMDCoreSvccame out as CRITICAL persistence, because\programdata\was in the writable-directory list. That root is writable,which is what makes it the textbook example — but the vendor subdirectories
under it are not, and Defender lives there. The worst false positive
available, and one is enough for an operator to stop reading the category.
subprocess.run(text=True)decodeswith the console code page — cp1254 here — and raised
UnicodeDecodeErrorinside subprocess's own reader thread, where the caller cannot catch it.
PasswordRequired=False, which means the account is permitted to have ablank password, not that it has one. Downgraded and reworded; a finding that
overstates is one an operator learns to dismiss, along with the next.
The group-membership check queries by SID (
S-1-5-32-544), not by name:Administratorsin English,Yöneticilerhere, and matching the name is acheck that silently finds nothing on a localised install.
Deduplication is on
(category, finding)and deliberately not ondetails— details drift ("2 members: …" → "3 members: …") and a fingerprintover them re-inserts the same finding whenever one changes.
send_alertlearned the general version of this: a state-reporting detector without
deduplication produced the identical alert 288 times a day.
And then the chain still ended in a wall. The rows arrived, stored
correctly, decryptable — and there was no route and no UI. Added both:
GET /<agent>/security_auditand a Posture tab. Its empty state is wordeddifferently from the telemetry tabs on purpose: empty here is a real answer.
Verified live: 6 findings on this host, all independently confirmed, read back
through the real decrypting path.
4. Agent fixes
automations/pendinghad been 401ing for the life of every install.AutomationsClientsent no headers at all. The 401 is correct — unauthenticatedthe route let anyone read the response actions queued for a host and POST
{"task_id": N, "status": "SUCCESS"}against every id, so the actions never ranwhile the console showed them green.
What was missed is that this is not dead code.
call_agent_soarqueues apending row precisely "so an agent that missed the push can still poll for it".
Broken, the fallback did not exist. It authenticates now, and a rejection is
said once rather than four times a second.
The vulnerability scan reported a clean result it could not have reached.
scanners/vuln.pymapped Windows to OSV'sNuGetecosystem, but the packagelist is registry
Uninstalldisplay names — "Google Chrome" — and NuGet isNewtonsoft.Json. The two name spaces do not overlap and OSV has no ecosystemfor programs installed on Windows, so the query was structurally incapable of
matching while reporting
new_findings=0every cycle. It is a named skip now,surfaced through the
skipped_reasonthe console already renders.5. The console (C2 + responsive)
All eight remaining pages moved onto the shared kit: 460 inline style objects
→ 171, and eight hand-rolled modal overlays → the kit's
Modal(Escapecloses, backdrop closes, portals to
document.bodybecauseposition: fixedresolves against the nearest containing block — AgentDetail had hit exactly
that).
The styling was the smaller half. Every one of the eight swallowed its load
errors. An empty Identity & Access reads as "nobody has access". Blank
AdminConfig forms read as "nothing is configured" and those forms save.
Playbooks reported a failed run to the browser console only — on a page whose
own editor labels steps "cannot be undone". AIAnalysis had a hardcoded green
"RabbitMQ Worker Active" pill with no request behind it. Deployment lost
enrolment tokens silently, because
navigator.clipboarddoes not exist outsidea secure context and the button still said "Copied".
Then responsiveness, which the kit's own docstring had warned about: six pages
wrote
gridTemplateColumns: '300px 1fr'inline, where a breakpoint cannot go.Added
.split-grid. Three places decided layout fromwindow.innerWidthreadduring render — which never follows a resize, and looks responsive to anyone
testing by reloading at each size.
Sidebarwas right only by accident; ittakes
isMobileas a prop now.6. Documentation that described a different product
Sentora/docs/MODULES.mdwas unchecked and substantially wrong:installed_software,resource_log,disk_info.edr_enforcer's entry was wrong end to end — documented as ahash-baseline scan writing
security_audit; it is the agent's main periodiccollector and writes five other tables, four of them undocumented.
network_inventoryandpackagesappeared nowhere.tests/test_module_reference.pyholds both directions now. While correctingthe paragraph about
scrub_nuls, a real NUL byte got into the file from anescape that collapsed one layer too far — enough for
grepto call it binaryand go quiet. Fixed, with a test.
7.
tsc --noEmit -p tsconfig.jsonchecks nothingfrontend/tsconfig.jsonis a solution file:"files": []and projectreferences. Pointing
tsc -pat it type-checks the empty set and exits 0, forany code.
vite builddoes not close the gap either — rolldown strips typeswithout checking them.
Two unused imports left by a refactor passed both, and were caught by the
Docker image build (which runs
npm run build→tsc -b) — after thecontainer had spent a day serving an old agent binary, because the failing
build never produced a new image and the failure was masked by a
docker ... | tailpipeline returningtail's exit code.tests/test_frontend_typecheck.pymakes the real gate structural.8. Four bugs the tests could not have caught
Every one was found by using the feature, and in each case the tests written
for it passed:
security_audithad no route or UIregister/beginreturned 500jsonin app.py is Sanic's response helper, not the module. Valid Python, fails at request time onlylogin/2fa/webauthn/*returned 401_PUBLIC_EXACT_PATHSis a path-shaped mirror, consulted only as a backstop;authenticatematches handler namesNew invariants for each: no module attribute may be read off a shadowed name
in app.py; any route that authenticates with a pending token must be in
_PUBLIC_HANDLERS; and the mirror must not drift from it. Each detector wasrun against the shipped state to confirm it fires.
9. CodeQL alerts
Eight open on
main. Five were real, two were true-but-misread, and one is thedesign.
The search query builder was not faithful (
js/incomplete-sanitization).The label misleads here: the query box beside the builder is editable by
design, so an operator can type raw Lucene whenever they like and there is no
boundary being crossed. What was broken is that clicking a filter produced a
query meaning something else. It escaped the quote and not the backslash, so
every Windows path ran off the end of its own phrase; and it only quoted when
the value held whitespace or a quote, so
a)OR(bwent in bare and changed thestructure of the query rather than the term. Moved to
frontend/src/lib/luceneQuery.tswith tests — backslash escaped first, becausedoing it second escapes the backslash that pass just added.
Two TLS floors stated rather than inherited.
create_default_contexthasset TLS 1.2 as the minimum since Python 3.10, so
Sentora/main.pychangesnothing today — which is why it is worth writing down, since the guarantee
otherwise depends on which Python the agent was frozen against, and the
connection it protects is every byte of telemetry the endpoint produces. The
probe in
link.pyis different: nothing is sent over it and verification isalready off, so the floor is not protecting the handshake. It bounds what the
message is allowed to claim — a server that only speaks TLS 1.0 should not
produce "this is serving TLS, switch to https://", because switching would be
to something worse than what the agent has now.
TRUSTED_PROXIESis not a secret, but that branch runs precisely whensomebody has put something in it that is not an address, and the commonest way
that happens is a value pasted into the wrong variable. It logs the entry's
position now instead of its content, which is what an operator needs to find it
anyway.
The agent banner carried the MAC address.
OS_INFOisplatform.platform()plus|HOST=and|MAC=; the server needs all of it,because that is how a reinstalled agent is recognised as the same machine. The
log does not. A hostname is unremarkable in a file that lives on that host; a
MAC is a stable hardware identifier that follows the machine across networks
and reinstalls, sitting in a plain-text file that support bundles and
screenshots routinely include.
The rotation script printed the new database password on its failure path.
The recovery has to exist — the account has already been altered at that point,
so a password nobody has is a database nobody can reach. It does not have to
put the secret in terminal scrollback, a CI job log, or the screenshot somebody
takes of the failure to ask about it. It writes
.db_password_rescuebeside.env, owner-readable only, and prints the path. Printing survives for the onecase left: the rescue file cannot be written.
One is dismissed rather than fixed.
.envholdsDB_PASSWORDin cleartext because that is how every service in the stack is configured, and there is
no version of this product where it does not. The addressable half is done —
.envis restricted to its owner after writing. The rest is a true finding,accepted.
Testing
2711 passed, 5 skipped(Python) ·53 pass, 0 fail(frontend) ·npm run buildanddocker compose buildboth exit 0, unmasked.Verified against the running deployment: the security-key ceremony completed on
real hardware,
security_auditfindings reached the server and read backdecrypted,
automations/pendinganswers 200, and the agent log carries no NULerrors.