Skip to content

Record who executed what, over which data, when - #77

Open
jorgeMFS wants to merge 1 commit into
live-logsfrom
audit-log
Open

jorgeMFS wants to merge 1 commit into
live-logsfrom
audit-log

Conversation

@jorgeMFS

Copy link
Copy Markdown
Contributor

Closes F7 (#26), the first item of Milestone 3 — unblocked now that E1
(#75) exists. Based on the chain tip live-logs (#72), which carries both of
its dependencies: B1 (async runs, #64) and E5 (output provenance, #71).

Why the design is mostly about coverage

An audit trail is only ever read after the fact. The failure mode is not a
wrong entry — it is a missing one that nobody notices until the question is
asked, and by then it is too late to add the line.

So the record is written at the single point where a run’s state changes,
not at each call site. A hook per call site is one that a new transition forgets
to add, and a gap in an audit trail is invisible from inside the thing that
should have written it. A test asserts there is exactly one recording point,
so that property cannot quietly stop being true.

What it deliberately does not claim

Append-only is not immutable. Written through a single os.write to a file
opened O_APPEND, so concurrent workers interleave whole lines rather than
fragments and nothing rewrites what it already said. But anything that can write
the file can truncate it. Immutability is the filesystem’s job — ship it
somewhere append-only, or set the attribute. An assurance nobody can keep is
worse than an absent one, and the README says this plainly.

caller is only as honest as the provider. For bearer it is None — a
shared secret is not an identity and every holder is the same caller. The event
records which provider authorised the call, so "we do not know who" reads as a
fact about the deployment rather than a hole in the log. F3 (Passports) will
light the field up without the format changing.

There is no endpoint serving it. An audit trail reachable over the same API
it audits is a thing an attacker reads to find out what you noticed.

One prerequisite this had to fix

The auth middleware was discarding the identity it was given:

self.provider.authenticate(Request(scope))   # return value dropped

authenticate() is documented to return an identity, and nothing held the
value, so there was nowhere for an audit trail to read "who" from. It now lands
in the ASGI scope alongside the provider name.

A bug worth naming

The first version recorded str(state), which for a str-Enum yields
"RunState.RUNNING" rather than the WES term "RUNNING". #64 adopted that
vocabulary precisely so states would be legible to something that is not this
service
— and an exported trail is exactly that reader. Caught by the test
asserting the recorded values, not by review.

Behaviour

A write failure raises rather than being swallowed: a deployment that
believes it is auditing and is not is in a worse position than one that knows it
is not. Unset BIOCHEF_AUDIT_LOG writes nothing, so this changes nothing for
existing deployments
until it is turned on.

285 passed, 4 skipped.

F7. A TRE requirement, and the one piece of evidence that is only ever read
after the fact -- which is why the failure mode is not a wrong entry but a
missing one that nobody notices until the question is asked.

The record is written where a run's state changes, which is one place, rather
than at each call site. A hook per call site is one a new transition forgets to
add, and a gap in an audit trail is invisible from inside the thing that should
have written it. A test asserts there is exactly one recording point, so that
property does not quietly stop being true.

Append only, one JSON object per line, through a single os.write to a file
opened O_APPEND -- so concurrent workers interleave whole lines rather than
fragments, and no entry can be rewritten by a later one. That is NOT
immutability and the documentation says so: anything that can write the file can
truncate it, and immutability is the filesystem's job. An assurance nobody can
keep is worse than an absent one.

JSON Lines because the format has to be readable by something that is not this
service, years after this service stopped running. For the same reason the state
is recorded as its WES term and not as str() of the enum, which yields
"RunState.RUNNING" -- #64 adopted that vocabulary precisely so the states would
be legible to other tools, and the first version of this threw that away.

There is no endpoint serving the trail, deliberately. An audit trail reachable
over the same API it audits is a thing an attacker reads to find out what you
noticed.

The middleware now keeps the identity the provider returns. It was discarding
it: authenticate() is documented to return an identity and nothing held the
value, so there was nowhere for this to read "who" from. For the bearer provider
that identity is None, and it is recorded as None rather than guessed at -- a
shared secret is not an identity and every holder is the same caller. The event
also records which provider authorised the call, so "we do not know who" reads
as a fact about the deployment rather than a hole in the log, and a provider
that does carry identity lights the field up without the format changing.

A write failure raises rather than being swallowed. A deployment that believes
it is auditing and is not is in a worse position than one that knows it is not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant