Skip to content

Add cache validation - #625

Open
inteon wants to merge 1 commit into
mainfrom
add_cache_validation
Open

Add cache validation#625
inteon wants to merge 1 commit into
mainfrom
add_cache_validation

Conversation

@inteon

@inteon inteon commented May 29, 2026

Copy link
Copy Markdown
Member

Add verify-cache command that validates the cache, preventing cache poisoning.

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
@cert-manager-prow cert-manager-prow Bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 29, 2026
@cert-manager-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: inteon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cert-manager-prow cert-manager-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 29, 2026
@cert-manager cert-manager deleted a comment from cert-manager-prow Bot May 29, 2026
Comment thread modules/tools/00_mod.mk
## restore in CI environments where the cache write-side is not fully trusted
## (e.g. node-local hostPath shared with low-trust presubmit jobs).
## @category [shared] Tools
verify-cache:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it mean we need to run make verify-cache in every single Prow job? I imagine that's not a big downside, but I wish it was a bit more automatic

@inteon inteon May 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'll update the prowjob definitions to first call make verify-cache.

@maelvls

maelvls commented May 29, 2026

Copy link
Copy Markdown
Member

at first, I didn't believe it as I thought that the inode modification timestamp mismatch would prevent the attack. And I thought that the report was wrong as it mentioned poisonning _bin/downloaded/kind which doesn't actually exists...

I realized that I was wrong.

$ echo $'#!/bin/bash\necho I GOT YOU>&2'> ~/.cache/makefile-modules/downloaded/tools/kind@v0.31.0_darwin_arm64 && chmod +x ~/.cache/makefile-modules/downloaded/tools/kind@v0.31.0_darwin_arm64
$ make _bin/tools/kind
$ _bin/tools/kind
I GOT YOU

I'm surprised that we aren't verifying the hash when creating the symlink. Right now, we verify after downloading and then never verify again.

And since some artifacts are large tarballs, we would need check their tarballs' hash and untar them just before creating the symlink.

My recommendation would be exactly this: instead of uncompressing + checking hash on download, I'd uncompress + check hash upon creating the symlink. This would 100% mitigate the issue IMO.

Quick proof of concept for verifying hashes at runtime instead of at the time of download: cert-manager/cert-manager#8833

@wallrj

wallrj commented Aug 21, 2026

Copy link
Copy Markdown
Member

Thanks @inteon — this has grown into the right shape: hash-check every restored file against the reviewed SHA-256 in 00_mod.mk, purge anything unknown or mismatched, and go mod verify the module cache. The reviewed hash is the trust anchor, so a poisoned file simply fails to match and is removed — no signing or fork-scoping needed.

Two thoughts:

  1. It's a step you have to remember. verify-cache only protects a job that calls it, after restore and before any tool runs. Nothing enforces that ordering, so a job that forgets is silently unprotected. @maelvls' make: check the hash at runtime instead of after downloading cert-manager#8833 verifies at symlink-creation time, which has the opposite trade-off: automatic, but only fires for $(NEEDS_*) targets and is bypassed by anything that runs _bin/tools/<x> off PATH directly. The strongest version is content-addressing: key the cache directory by the reviewed hash, so a poisoned file isn't found under its expected key and verification collapses into a lookup — no separate step, no PATH bypass. Short of that, at least wire verify-cache in as a documented, mandatory first step after every cache restore.

  2. go mod verify is belt-and-braces. GOMODCACHE is already verified against go.sum/GOSUMDB on every use, so a poisoned module cache is caught at build time regardless. No harm keeping it, but it's not where the exposure is — the tool cache, verified only at download until this PR, is.

Net: I'd like the tool-cache half of this to land here in makefile-modules, folding in cert-manager/cert-manager#8833's "verify at point of use, not just at download" insight rather than running it as a separate command. Happy to help push it over the line.

with claude fable-5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants