Adds the bounded cache and its partition ids - #23
Merged
Conversation
The engine's local cache has no capacity limit, no sweeper, and no way for outside code to measure it, and the caching CMM calls it by module name rather than through the cache behaviour, so a bounded cache cannot be substituted for it either. Encryptor.Vault.CacheRecycler bounds it the only way that leaves: it stops the cache child on the configured :recycle_after interval and starts it again with a fresh empty table. Dropping the whole table is always safe, because every entry is derived material that can be re-fetched and the worst outcome is a cold miss. The recycler drives the restart itself rather than killing the cache and letting the supervisor react. A supervisor's restart intensity is a defence against a child that keeps failing, and spending it on scheduled maintenance would mean a short interval takes the whole vault down. Encryptor.Vault.Partition derives the fixed-width partition id a tenant's materials are cached under. The width is 16 bytes and that is load-bearing: the engine concatenates the partition id into its cache-id pre-image with no length prefix, so a variable width would let two partitions collide on one cache id, which is two tenants sharing a data key. The id is a cache-key input only, never key material and never in a message. Records: ADR-0001 decisions 6 and 7. Refs: enc-kur
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.
Why
The engine's materials cache is the one stateful thing in the whole library,
and it is unbounded.
LocalCachehas no capacity limit, no sweeper, and noway for outside code to measure it: it deletes an entry when a read finds it
expired, or when someone deletes it by its 48-byte cache id, and nothing
else. The caching CMM also calls
LocalCacheby module name rather thanthrough the cache behaviour, so a bounded implementation cannot be
substituted for it either (verified in
deps/aws_encryption_sdk, and openupstream as riddler/aws-encryption-sdk-elixir#95).
That matters because of how the vault partitions the cache. One entry per
tenant per encryption context accumulates forever - including for tenants
that were offboarded months ago, whose entries nothing will ever read again,
so expiry never notices them.
What
Encryptor.Vault.CacheRecycler- a per-vault child that stops the cachechild on the configured
:recycle_afterinterval and starts it again with afresh empty table. Dropping the whole table is always safe: every entry is
derived material that can be re-fetched, and the worst outcome is a cold
miss. It is supervised only when there is a cache, so a
cache: falsevaultruns neither.
:recycle_afteris in seconds (defaulting to20 * max_age,already resolved and validated by
Encryptor.Vault.Config); the timer is inmilliseconds, and the child spec is the only place those units meet.
Encryptor.Vault.Partition-id/2derives the 16-byte partition idfrom
sha256(vault, 0, encoded_selector). The width is load-bearing: theengine concatenates the partition id into its cache-id pre-image with no
length prefix, so a variable width would make the pre-image ambiguous and let
two partitions collide on one cache id, which is two tenants sharing a data
key. Sixteen bytes is the width of the UUID the engine generates when no
partition id is given.
Encryptor.Vault.recycler_name/1- the derived registered name, in thesame family as
cache_name/1andsupervisor_name/1.Records: ADR-0001 decisions 6 and 7. Cache bounds are consumed from
Vault.Config, not re-validated here.Notes
Three things worth a reviewer's attention:
vault_namespacein ADR-0001 decision 7 is read as the vault module.The record writes the pre-image as
sha256(vault_namespace, 0, encoded_selector)without definingvault_namespace, and the package has no configuration key by that name.The vault module is the reading everything else in the vault already uses
as its namespace -
{otp_app, vault_module}is the whole configurationkey, and the cache, supervisor and lifecycle names are all derived from
it. Flagging rather than deciding silently: if the record meant something
else, this is the place to say so, and the change is one line plus a
golden vector.
encoded_selectoris tagged, which the record does not specify.:defaultencodes as<<0>>and a string selector as<<1, selector>>.Without a tag, a
:tenantvault holding a tenant literally named"default"would derive the same partition as a:singlevault. Thecontext profile makes that unreachable today (a vault admits one selector
shape, never both), so the tag is defence in depth rather than a fix. It
affects no message and no interop - a partition id is an in-memory
cache-key input whose lifetime is one cache process.
The recycler drives the restart itself (
terminate_childthenrestart_child) rather than killing the cache and letting the supervisorreact. A supervisor's restart intensity is a defence against a child that
keeps failing; spending it on scheduled maintenance means a short
:recycle_aftertakes the whole vault down. There is a microseconds-widewindow between the two calls in which the cache's registered name resolves
to nothing and a concurrent call would exit
:noproc. That is inherent tobounding the cache this way and is documented in the module; the durable
fix is upstream.
Deliberately deferred: nothing here calls
Partition.id/2yet. The call siteis the encrypt path's, and lands with it (
enc-50m).Gate: full
mix qualitygreen on the rebased tree - format, compile withwarnings-as-errors, credo strict, dialyzer, deps audit, 239/239 tests, 98.8%
coverage against the 90% floor. Doctor, Gettext and Sobelow are not installed
in this project and never ran. Both new modules are at 100% line coverage.
This branch is also the first full gate run over the composed tree of the two
branches that merged without cross-gating (the vault macro/supervisor and the
provider behaviour). It is green, so there is no composition defect to
report.
All 13 new tests were sabotage-verified: every mutation named in the note
above a test was applied, the test confirmed red, and the mutation reverted.
The first harness run produced a false result - a reverted source file kept
its old
_build/testbeam because the rewrite landed inside the samemtime-second - so the whole set was re-run with forced recompilation and the
gate re-run afterward.
Refs: enc-kur