Make sure that blobs downloaded by the client are validated#4075
Make sure that blobs downloaded by the client are validated#4075bart-linera wants to merge 2 commits intolinera-io:mainfrom
Conversation
linera-core/src/client/mod.rs
Outdated
| let Some(blob) = blob_certificate | ||
| .value() | ||
| .block() | ||
| .created_blobs() |
There was a problem hiding this comment.
Since this function doesn't assume it's a chain description blob, we shouldn't assume it's in created_blobs. Could just be a required blob.
linera-core/src/client/mod.rs
Outdated
| .collect()) | ||
| } | ||
|
|
||
| async fn fetch_blob( |
There was a problem hiding this comment.
Isn't this doing the same as update_local_node_with_blobs_from, but serially instead of in parallel?
There was a problem hiding this comment.
Indeed! 🤦♂️ Well, it also returns the blob, but it can just be read afterwards if needed.
There was a problem hiding this comment.
Ah, no, wait, there is a difference: update_local_node_with_blobs_from works for multiple blobs and a single validator, while fetch_blob works for a single blob and multiple validators. I guess ideally we'd just have a function for multiple blobs and multiple validators, but that's what the TODO is about, after all 😬
There was a problem hiding this comment.
Yes, I'd prefer generalizing over duplicating. And we can make the other function return the blob, too?
This comment was marked as spam.
This comment was marked as spam.
|
Closing in favor of #4082. |
This is a completion of @bart-linera's #4075: ## Motivation #3787 left a potential security issue: when a client was downloading a missing `ChainDescription` blob, it would just download a blob, but wouldn't make sure that it was legitimately created on another chain. ## Proposal Whenever a `ChainDescription` is fetched, fetch the certificate for the block that created it and validate it against committees known from the admin chain. ## Test Plan The only occurrences of `RemoteNode::download_blob[s]` are now after `process_certificate` returned `BlobsNotFound`, i.e. after checking a certificate's signatures. CI should catch regressions. ## Release Plan - Nothing to do / These changes follow the usual release cycle. ## Links - Closes #2351 - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist) ## Notes The `TODO` in `attempted_changes.rs` has been removed after verifying that the safety issue is no longer present there: the committees are taken from a blob, but that blob can only exist on the validator if it has been legitimately created on another chain. --------- Co-authored-by: Bartłomiej Kamiński <bartlomiej.kaminski@linera.io>
3 participants
Motivation
#3787 left a potential security issue: when a client was downloading a missing
ChainDescriptionblob, it would just download a blob, but wouldn't make sure that it was legitimately created on another chain.Proposal
Whenever a
ChainDescriptionis fetched, fetch the certificate for the block that created it and validate it against committees known from the admin chain.Test Plan
CI should catch regressions.
Release Plan
Links
process_confirmed_block. #2351Notes
The
TODOinattempted_changes.rshas been removed after verifying that the safety issue is no longer present there: the committees are taken from a blob, but that blob can only exist on the validator if it has been legitimately created on another chain.