Prevent records from sharing a storage location - #57
Open
damilolaedwards wants to merge 1 commit into
Open
Conversation
A create request for a beacon state, block, bad block, bad blob, execution trace, or execution bad block could succeed even when its location was already tied to a completely different record (a different node, slot, or root). Since retention deletes blobs by location, an unrelated record could ride along on someone else's blob and later cause it to be deleted when the unrelated record aged out, even though the original record was still active. Each create handler now checks whether the location is already associated with a different record and rejects the request if so. Legitimate retries of the exact same record are unaffected.
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
1 participant
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.
Summary
A create request for a beacon state, block, bad block, bad blob, execution trace, or execution bad block could succeed even when its location was already tied to a completely different record (a different node, slot, or root). Since retention deletes blobs by location, an unrelated record could ride along on someone else's blob and later cause it to be deleted once the unrelated record aged out of retention, even though the original record was still active and untouched by the requester.
Each create handler now checks whether the location is already associated with a different record and rejects the request with
AlreadyExistsif so. Legitimate retries of the exact same record are unaffected.Two of the six handlers (execution block trace, execution bad block) had no location or identity check of any kind before this change, so this also closes that gap for them.
Changes
pkg/server/service/indexer/indexer.go: eachCreate*handler now calls a location-ownership check before insertingpkg/server/service/indexer/location_ownership.go: the six type-specific ownership checkspkg/server/service/indexer/location_ownership_test.go: tests covering the rejection case, the legitimate-retry case, and the previously-unprotected execution bad block handlerTest plan
go build ./...andgo vet ./...passlocation_ownership_test.gopass locallypkg/server/service/indexer(depends on a Docker-backed MinIO mock, not runnable in the sandbox this was developed in - should be verified in CI)