Repository navigation
Add security keys, give security_audit a collector, and finish the console - #48
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, move the last pages onto the shared kit with real loading and failure states, and correct the agent module reference.
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.
Testing
2710 passed, 5 skipped(Python) ·42 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.