Repository navigation
Conversation
fromTransientStore merged the private write sets of every transient store entry for a txID but kept the collection config of only one of them, and the puller checks the requesting peer against that config. Entries pushed over gossip carry a config supplied by the sending peer, so a channel member outside the collection could push an entry whose config names its own org and then pull the real private data for the same txID before the block is committed. Only return the write sets stored under the selected config. Signed-off-by: nussaiba shaikh <nussaibah@bugqore.com>
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.
Type of change
Description
When a peer serves a private data pull for a block it has not committed yet,
dataRetriever.fromTransientStoreingossip/privdata/dataretriever.goappends the write sets of every transient store entry for the requested txID, but keeps the collection config of only one entry (highestEndorsedAt, ties go to whichever entry is iterated last).puller.filterNotEligiblethen checks the requesting peer against that one config.Any channel member peer can add transient store entries through a gossip private data push (
GossipStateProviderImpl.privateDataMessage), and the collection config in that message is stored as sent, withEndorsedAt0. On a peer that received the real private data by dissemination (alsoEndorsedAt0), a channel member outside the collection can push a few entries for the same txID whose config lists its own org as a member, then send a pull request for that digest with aBlockSeqabove the victim's ledger height to force the transient store path. When one of its entries is iterated last, eligibility is checked against the forged config and the response carries the real private write set as well.The config selection is unchanged. The function now only returns write sets from entries whose collection config is equal to the selected one, so data stored under one policy is never released under another. When all entries share the same config, which is the normal case, the result is the same as before.
Additional details
Reproduced with a scratch test that used the real transient store,
dataRetrieverandpuller.filterNotEligible: with one legitimate entry and 10 pushed entries, the non-member received the private data in 18 of 20 runs before the change and 0 of 20 after. AddedTestNewDataRetriever_TransientStoreEntriesWithDifferentConfigs, which fails on the current code because both write sets are returned.go test ./gossip/privdata/... ./gossip/state/...andgolangci-lint run ./gossip/privdata/...pass locally.The endorsing peer itself is not affected, since its own entry has a non-zero
EndorsedAtand always wins the selection. The ledger path (fromLedger) uses config history and is not affected either.Related issues
None