[BFT Testing] implements wintermute attack orchestrator #2291

yhassanzadeh13 · 2022-04-11T22:54:06Z

Implements Wintermute attack orchestrator. The orchestrator takes control of a subset of corrupted execution and verification nodes, and performs the following attack logic:

Orchestrator corrupts the result of the first incoming execution receipt from any of the corrupted execution node.
The orchestrator sends the corrupted execution result to all corrupted execution nodes.
If Orchestrator receives any chunk data pack request for a corrupted chunk from a corrupted verification node, it replies it with attestation for that chunk.
If Orchestrator receives any chunk data pack response from a corrupted execution node to an honest verification node, it drops the response if it is for one of the corrupted chunks.
Any other incoming messages to the orchestrator are passed through, i.e., are sent as they are in the original Flow network without any tampering.

…_CorruptSingleExecutionResult() so it passes but needs assertions

…te-attack

AlexHentschel · 2022-04-21T20:28:52Z

I am not sure, though I feel there might be a bit of an imprecision in the terminology here:

Here, you define that CorruptedId is the Identifier of a conduit:

flow-go/insecure/event.go

Line 20 in 8c57979

CorruptedId flow.Identifier // identifier of corrupted conduit
- Presumably, a single node generally has multiple conduits (one conduit per channel?). So for a single node ID, there should be multiple CorruptedId
However, I feel there is a conflicting definition in other places in the code:

flow-go/insecure/wintermute/attackOrchestrator.go

Line 32 in 8c57979

corruptedIds flow.IdentifierList // identifier of corrupted nodes.

Here, we specifically have the Identifiers of the corrupted nodes, and then ask, whether the ID of a particular conduit is part of this list:

flow-go/insecure/wintermute/attackOrchestrator.go

Line 103 in 8c57979

ok := o.corruptedIds.Contains(receiptEvent.CorruptedId)

In short, I feel the code (or documentation?) is conflating conduits and nodes Identifiers.

@gomisha response:
Each corrupted conduit is uniquely identified by 1) corrupted node ID and 2) channel. There is no such thing as the ID of a corrupted conduit.
I have updated the code and comments to clarify this by changing all reference to corruptedId to corruptedNodeId

a235c6a

AlexHentschel

Looks good. Mostly focused my efforts on reviewing the attack logic. Its very cool to see that we can encode the entire attack logic for Wintermute in the relatively compact wintermute/attackOrchestrator.go 👏

Most of my comments are w.r.t. documentation. While the flow is coarsely described, I feel we gloss over some important details thereby introducing imprecision and ambiguity that might mislead engineers trying to understand the code. I tried to give detailed explanations in my comments where I think the lack of precision might be misleading.
Overall, very nice work. Thanks.

insecure/wintermute/attackOrchestrator.go

Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>

…instead of bounce-back

…kResponseEvent()

…gh chunk data request unaltered

gomisha

Addressed all PR review comments

* BFT Testing - Wintermute - minor clean up * BFT Testing - Wintermute - more clean up, prep for VN tests * refactors test for corrupted en to span scenario * encapsulates corruption logic of execution receipts in a method * renames part of complete execution result fixture * implements corruption logic for wintermute attacker * implements test covering corruption logic for wintermute orchestrator * implements bounce-back logic * wip adds event sanity check * fixes single exeuction receipt test * refactors corrupted receipt with result * adds wintermute sanity check for execution results * fixes and adds documentation to orchestrator output sanity check * removes debug printing off orchestrator * implements distinct execution receipt fixture * re-organizes test with fixtures * adds comment * adds receipts with same result fixture * adds comment for distinct result test * adds test for the same result * adds attack state * refactors attack state * integrates attack state with orchestrator * adds sanity check to test fixtures * makes code more dry * adds multiple receipts with same result fixture * adds a comment * adds a comment * adds test for multiple receipts * adds count to receipts with distinct result * fixes broken tests * adds test multiple concurrent receipts with distinct results * fixes a method receivier * cleans up tests * implements pre-attack and post-attack scenarios * encapsulates corrupting execution receipts * refactors with post attack func * refactors with post attack func * adds chunk data request sanity check * implements chunk data pack request for receipts * adds pre-attack request response test * adds chunk index map * adds responding with attestation for corrupted chunk * adds corrupted chunk request test * adds more constraints to the test * fixes a bug * toughen up parameters of test * adds a comment * adds test for bouncing back * adds handle chunk data pack response * adds godoc * adds godoc * adds a fatal level log * adds chunkDataPackResponseForReceipts * adds TestBouncingBackChunkDataResponse_NoAttack * adds TestBouncingBackChunkDataResponse_WithAttack * refactors a test * refactors a test * adds TestWintermuteChunkResponseForCorruptedChunks * moves chunkDataPackRequestForReceipts * fixes a typo * fixes lint * adds protocol to string * simplifies tests * adds more cleanup * fixes the tests * refactors out chunk index map * refactors corrupted identifier list * adds more logging * refactors comments * renames to pass through * clarified explanation of corrupted chunk data pack request * clarified docs of HandleEventFromCorruptedNode() Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co> * updated docs to clarify wintermute behavior, using pass-through term instead of bounce-back * clarified comments about passing through event for handleChunkDataPackResponseEvent() * updated docs for handleChunkDataPackRequestEvent() when passing through chunk data request unaltered * renamed corruptedIds to corruptedNodeIds to remove ambiguity Co-authored-by: Misha <misha.rybalov@dapperlabs.com> Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>

gomisha and others added 30 commits March 10, 2022 10:56

BFT testing - Wintermute implemenation - initial commit - in progress

09b3b84

WIP

f847c10

BFT Testing - Wintermute - WIP - completed TestWintermuteOrchestrator…

91fc672

…_CorruptSingleExecutionResult() so it passes but needs assertions

wip

109e532

BFT Testing - Wintermute - WIP

0c86cfc

Merge branch 'master' into misha/bft-testing-wintermute-implementation

49d1ef0

BFT Testing - Wintermute - WIP - update test after merge from master

5798b29

fixes test

40e7245

BFT Testing - Wintermute - minor clean up

f1d5d33

BFT Testing - Wintermute - more clean up, prep for VN tests

28941aa

refactors test for corrupted en to span scenario

9d6cc38

encapsulates corruption logic of execution receipts in a method

81a8940

renames part of complete execution result fixture

33b1e3e

implementes corruption logic for wintermute attacker

dc5f5cd

implements test covering corruption logic for wintermute orchestrator

daf1099

implements bounce-back logic

5cb4d6f

wip adds event sanity check

5d2133a

fixes single exeuction receipt test

d66e9ca

refactors corrupted receipt with result

10fedf8

adds wintermute sanity check for execution results

229c860

fixes and adds documentation to orchestrator output sanity check

9889363

removes debug printing off orchestrator

1207206

Merge remote-tracking branch 'origin/master' into yahya/6211-wintermu…

5b1b434

…te-attack

implements distinct exectuion receipt fixture

8b28cf1

re-organizes test with fixtures

3777f81

adds comment

f6e2264

adds receipts with same result fixture

1569c60

adds comment for distinct result test

46304ff

adds test for the same result

5d13869

adds attack state

8fd0e0c

yhassanzadeh13 added 2 commits April 13, 2022 16:47

adds more logging

6d401bc

refactors comments

d29fe38

yhassanzadeh13 marked this pull request as ready for review April 14, 2022 00:04

yhassanzadeh13 requested a review from vishalchangrani as a code owner April 14, 2022 00:04

yhassanzadeh13 requested review from peterargue and removed request for vishalchangrani and ramtinms April 14, 2022 00:06

yhassanzadeh13 added 5 commits April 19, 2022 08:17

Merge branch 'master' into yahya/6211-wintermute-attack

c810f54

nit

f0555a9

adds a comment

e752e4f

adds a comment

4d7c99e

adds a comment

303245f

yhassanzadeh13 requested a review from gomisha April 19, 2022 15:24

renames to pass through

8c57979

gomisha requested a review from AlexHentschel April 19, 2022 21:14

AlexHentschel approved these changes Apr 21, 2022

View reviewed changes

gomisha and others added 8 commits April 22, 2022 06:47

Merge branch 'master' into yahya/6211-wintermute-attack

065cabb

clarified explanation of corrupted chunk data pack request

f63f2a7

clarified docs of HandleEventFromCorruptedNode()

7bdbb04

Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>

updated docs to clarify wintermute behavior, using pass-through term …

4691894

…instead of bounce-back

clarified comments about passing through event for handleChunkDataPac…

7b8117d

…kResponseEvent()

updated docs for handleChunkDataPackRequestEvent() when passing throu…

40594f7

…gh chunk data request unaltered

Merge branch 'master' into yahya/6211-wintermute-attack

db430c5

renamed corruptedIds to corruptedNodeIds to remove ambiguity

a235c6a

gomisha approved these changes Apr 26, 2022

View reviewed changes

gomisha merged commit fdcb6b3 into master Apr 26, 2022

gomisha deleted the yahya/6211-wintermute-attack branch April 26, 2022 17:11

fxamacker mentioned this pull request Jun 16, 2022

Prevent Linux Page Cache from hoarding 294GB - 394GB RAM on Execution Nodes #2336

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BFT Testing] implements wintermute attack orchestrator #2291

[BFT Testing] implements wintermute attack orchestrator #2291

yhassanzadeh13 commented Apr 11, 2022 •

edited

Loading

AlexHentschel commented Apr 21, 2022 •

edited by gomisha

Loading

AlexHentschel left a comment •

edited

Loading

gomisha left a comment

[BFT Testing] implements wintermute attack orchestrator #2291

[BFT Testing] implements wintermute attack orchestrator #2291

Conversation

yhassanzadeh13 commented Apr 11, 2022 • edited Loading

AlexHentschel commented Apr 21, 2022 • edited by gomisha Loading

AlexHentschel left a comment • edited Loading

Choose a reason for hiding this comment

gomisha left a comment

Choose a reason for hiding this comment

yhassanzadeh13 commented Apr 11, 2022 •

edited

Loading

AlexHentschel commented Apr 21, 2022 •

edited by gomisha

Loading

AlexHentschel left a comment •

edited

Loading