From c33d8a83531062b479f5a59345901c0dd5376001 Mon Sep 17 00:00:00 2001 From: Raghav Gulati Date: Tue, 7 May 2019 09:56:09 -0700 Subject: [PATCH 01/13] Add test for improperly signed message Test the case where an adversarial peer signs a message with a key they didn't originally register with. First, we test that an adversarial peer will allow the message to pass through validation as they turn off strict verification (putting themselves at risk), but an honest peer with strict verification on will never see the message! --- floodsub_test.go | 127 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 121 insertions(+), 6 deletions(-) diff --git a/floodsub_test.go b/floodsub_test.go index d9fb732..40fe28b 100644 --- a/floodsub_test.go +++ b/floodsub_test.go @@ -12,10 +12,11 @@ import ( host "github.com/libp2p/go-libp2p-host" peer "github.com/libp2p/go-libp2p-peer" + protocol "github.com/libp2p/go-libp2p-protocol" swarmt "github.com/libp2p/go-libp2p-swarm/testing" + //bhost "github.com/libp2p/go-libp2p/p2p/host/basic" bhost "github.com/libp2p/go-libp2p-blankhost" - "github.com/libp2p/go-libp2p-protocol" ) func checkMessageRouting(t *testing.T, topic string, pubs []*PubSub, subs []*Subscription) { @@ -90,14 +91,18 @@ func connectAll(t *testing.T, hosts []host.Host) { } } +func getPubsub(ctx context.Context, h host.Host, opts ...Option) *PubSub { + ps, err := NewFloodSub(ctx, h, opts...) + if err != nil { + panic(err) + } + return ps +} + func getPubsubs(ctx context.Context, hs []host.Host, opts ...Option) []*PubSub { var psubs []*PubSub for _, h := range hs { - ps, err := NewFloodSub(ctx, h, opts...) - if err != nil { - panic(err) - } - psubs = append(psubs, ps) + psubs = append(psubs, getPubsub(ctx, h, opts...)) } return psubs } @@ -943,3 +948,113 @@ func TestWithSigning(t *testing.T) { t.Fatalf("unexpected data: %s", string(msg.Data)) } } + +func TestImproperlySignedMessageNotRelayed(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + hosts := getNetHosts(t, ctx, 2) + adversarialPeer := hosts[0] + honestPeer := hosts[1] + + // The adversary enables signing, but disables verification to let through + // an incorrectly signed message. + adversaryPubSub := getPubsub( + ctx, + adversarialPeer, + WithMessageSigning(true), + WithStrictSignatureVerification(false), + ) + honestPubSub := getPubsub( + ctx, + honestPeer, + WithStrictSignatureVerification(true), + ) + + connect(t, hosts[0], hosts[1]) + + var ( + topic = "foobar" + correctMessage = []byte("this is a correct message") + incorrectMessage = []byte("this is the incorrect message") + ) + + _, err := adversaryPubSub.Subscribe(topic) + if err != nil { + t.Fatal(err) + } + + adversarialPeerSubscription, err := adversaryPubSub.Subscribe(topic) + if err != nil { + t.Fatal(err) + } + honestPeerSubscription, err := honestPubSub.Subscribe(topic) + if err != nil { + t.Fatal(err) + } + time.Sleep(time.Millisecond * 10) + + // First the adversary sends the correct message. + err = adversaryPubSub.Publish(topic, correctMessage) + if err != nil { + t.Fatal(err) + } + + // Change the sign key for the adversarial peer, and send the second message. + adversaryPubSub.signID = honestPubSub.signID + adversaryPubSub.signKey = honestPubSub.host.Peerstore().PrivKey(honestPubSub.signID) + err = adversaryPubSub.Publish(topic, incorrectMessage) + if err != nil { + t.Fatal(err) + } + + var adversaryMessages []*Message + adversaryContext, adversaryCancel := context.WithCancel(ctx) + go func(ctx context.Context) { + for { + select { + case <-ctx.Done(): + return + default: + msg, err := adversarialPeerSubscription.Next(ctx) + if err != nil { + return + } + adversaryMessages = append(adversaryMessages, msg) + } + } + }(adversaryContext) + + <-time.After(2 * time.Second) + adversaryCancel() + + if len(adversaryMessages) != 2 { + t.Fatalf("got %d messages, expected 2", len(adversaryMessages)) + } + + // the honest peer's validation process will drop the message; + // next will never furnish the incorrect message. + var honestPeerMessages []*Message + honestPeerContext, honestPeerCancel := context.WithCancel(ctx) + go func(ctx context.Context) { + for { + select { + case <-ctx.Done(): + return + default: + msg, err := honestPeerSubscription.Next(ctx) + if err != nil { + return + } + honestPeerMessages = append(honestPeerMessages, msg) + } + } + }(honestPeerContext) + + <-time.After(2 * time.Second) + honestPeerCancel() + + if len(honestPeerMessages) != 1 { + t.Fatalf("got %d messages, expected 1", len(honestPeerMessages)) + } +} From ed746477f95267d02cc7b65faf0ac825e71f9e8f Mon Sep 17 00:00:00 2001 From: Raghav Gulati Date: Tue, 7 May 2019 10:09:40 -0700 Subject: [PATCH 02/13] Reduce time to wait 2 seconds per wait seemed a bit aggressive. Halved the total time of the test. --- floodsub_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/floodsub_test.go b/floodsub_test.go index 40fe28b..38f5ff0 100644 --- a/floodsub_test.go +++ b/floodsub_test.go @@ -1025,7 +1025,7 @@ func TestImproperlySignedMessageNotRelayed(t *testing.T) { } }(adversaryContext) - <-time.After(2 * time.Second) + <-time.After(1 * time.Second) adversaryCancel() if len(adversaryMessages) != 2 { @@ -1051,7 +1051,7 @@ func TestImproperlySignedMessageNotRelayed(t *testing.T) { } }(honestPeerContext) - <-time.After(2 * time.Second) + <-time.After(1 * time.Second) honestPeerCancel() if len(honestPeerMessages) != 1 { From b44471d7ce60d06cf797f34c7204cd0a059ac982 Mon Sep 17 00:00:00 2001 From: Raghav Gulati Date: Tue, 7 May 2019 10:10:05 -0700 Subject: [PATCH 03/13] Ensure that the received message is the correct one --- floodsub_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/floodsub_test.go b/floodsub_test.go index 38f5ff0..52ba129 100644 --- a/floodsub_test.go +++ b/floodsub_test.go @@ -1057,4 +1057,11 @@ func TestImproperlySignedMessageNotRelayed(t *testing.T) { if len(honestPeerMessages) != 1 { t.Fatalf("got %d messages, expected 1", len(honestPeerMessages)) } + if string(honestPeerMessages[0].GetData()) != string(correctMessage) { + t.Fatalf( + "got %s, expected message %s", + honestPeerMessages[0].GetData(), + correctMessage, + ) + } } From d740f535311f46dbc223fd8460bdfb98f3be48a1 Mon Sep 17 00:00:00 2001 From: Raghav Gulati Date: Tue, 7 May 2019 10:14:37 -0700 Subject: [PATCH 04/13] adversarial -> adversary --- floodsub_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/floodsub_test.go b/floodsub_test.go index 52ba129..f54a17e 100644 --- a/floodsub_test.go +++ b/floodsub_test.go @@ -954,14 +954,14 @@ func TestImproperlySignedMessageNotRelayed(t *testing.T) { defer cancel() hosts := getNetHosts(t, ctx, 2) - adversarialPeer := hosts[0] + adversary := hosts[0] honestPeer := hosts[1] // The adversary enables signing, but disables verification to let through // an incorrectly signed message. adversaryPubSub := getPubsub( ctx, - adversarialPeer, + adversary, WithMessageSigning(true), WithStrictSignatureVerification(false), ) @@ -984,7 +984,7 @@ func TestImproperlySignedMessageNotRelayed(t *testing.T) { t.Fatal(err) } - adversarialPeerSubscription, err := adversaryPubSub.Subscribe(topic) + adversaryPeerSubscription, err := adversaryPubSub.Subscribe(topic) if err != nil { t.Fatal(err) } @@ -1016,7 +1016,7 @@ func TestImproperlySignedMessageNotRelayed(t *testing.T) { case <-ctx.Done(): return default: - msg, err := adversarialPeerSubscription.Next(ctx) + msg, err := adversaryPeerSubscription.Next(ctx) if err != nil { return } From 6b2635b6ac105f0e77a26d727190efdbee542cbb Mon Sep 17 00:00:00 2001 From: Raghav Gulati Date: Tue, 7 May 2019 10:17:55 -0700 Subject: [PATCH 05/13] adversaryPeer -> adversary --- floodsub_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/floodsub_test.go b/floodsub_test.go index f54a17e..13652d0 100644 --- a/floodsub_test.go +++ b/floodsub_test.go @@ -984,7 +984,7 @@ func TestImproperlySignedMessageNotRelayed(t *testing.T) { t.Fatal(err) } - adversaryPeerSubscription, err := adversaryPubSub.Subscribe(topic) + adversarySubscription, err := adversaryPubSub.Subscribe(topic) if err != nil { t.Fatal(err) } @@ -1016,7 +1016,7 @@ func TestImproperlySignedMessageNotRelayed(t *testing.T) { case <-ctx.Done(): return default: - msg, err := adversaryPeerSubscription.Next(ctx) + msg, err := adversarySubscription.Next(ctx) if err != nil { return } From 516c32e854232921a6a39cf818c12d6dbd81ed74 Mon Sep 17 00:00:00 2001 From: Raghav Gulati Date: Tue, 7 May 2019 15:05:00 -0700 Subject: [PATCH 06/13] Remove commented out code bhost import was commented out. --- floodsub_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/floodsub_test.go b/floodsub_test.go index 13652d0..ca7905f 100644 --- a/floodsub_test.go +++ b/floodsub_test.go @@ -15,7 +15,6 @@ import ( protocol "github.com/libp2p/go-libp2p-protocol" swarmt "github.com/libp2p/go-libp2p-swarm/testing" - //bhost "github.com/libp2p/go-libp2p/p2p/host/basic" bhost "github.com/libp2p/go-libp2p-blankhost" ) From 8272eb583e0fa81222cc736387039fd01bc05bed Mon Sep 17 00:00:00 2001 From: Raghav Gulati Date: Wed, 8 May 2019 08:59:12 -0700 Subject: [PATCH 07/13] Change test name to reflect no relaying Improperly named test. In this test, we're only testing that we reject malicious messages. --- floodsub_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/floodsub_test.go b/floodsub_test.go index ca7905f..b123dab 100644 --- a/floodsub_test.go +++ b/floodsub_test.go @@ -948,7 +948,7 @@ func TestWithSigning(t *testing.T) { } } -func TestImproperlySignedMessageNotRelayed(t *testing.T) { +func TestImproperlySignedMessageRejected(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() From b136dae80c115c5c3136da5cee554170142c074a Mon Sep 17 00:00:00 2001 From: Raghav Gulati Date: Wed, 8 May 2019 08:59:47 -0700 Subject: [PATCH 08/13] Remove erroneous subscribe --- floodsub_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/floodsub_test.go b/floodsub_test.go index b123dab..eb353fa 100644 --- a/floodsub_test.go +++ b/floodsub_test.go @@ -978,11 +978,6 @@ func TestImproperlySignedMessageRejected(t *testing.T) { incorrectMessage = []byte("this is the incorrect message") ) - _, err := adversaryPubSub.Subscribe(topic) - if err != nil { - t.Fatal(err) - } - adversarySubscription, err := adversaryPubSub.Subscribe(topic) if err != nil { t.Fatal(err) From 84bb35ebf4aa76addc360f6c0157152c949a9e72 Mon Sep 17 00:00:00 2001 From: Raghav Gulati Date: Wed, 8 May 2019 09:00:00 -0700 Subject: [PATCH 09/13] Bump wait from 10 -> 50ms This matches the waits in the rest of the package. --- floodsub_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/floodsub_test.go b/floodsub_test.go index eb353fa..8ba03d3 100644 --- a/floodsub_test.go +++ b/floodsub_test.go @@ -986,7 +986,7 @@ func TestImproperlySignedMessageRejected(t *testing.T) { if err != nil { t.Fatal(err) } - time.Sleep(time.Millisecond * 10) + time.Sleep(time.Millisecond * 50) // First the adversary sends the correct message. err = adversaryPubSub.Publish(topic, correctMessage) From 594e6a4ab98ff4cdb5fced9f9612c510266d2d6f Mon Sep 17 00:00:00 2001 From: Raghav Gulati Date: Wed, 8 May 2019 09:00:25 -0700 Subject: [PATCH 10/13] Add a note why we're expecting two messages As the adversary has turned of signature verification, we should expect to see two messages (the correctly signed and the incorrectly signed one) when we publish. Those that have verification on, should only see one. --- floodsub_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/floodsub_test.go b/floodsub_test.go index 8ba03d3..ca8d923 100644 --- a/floodsub_test.go +++ b/floodsub_test.go @@ -1022,6 +1022,9 @@ func TestImproperlySignedMessageRejected(t *testing.T) { <-time.After(1 * time.Second) adversaryCancel() + // Ensure the adversary successfully publishes the incorrectly signed + // message. If the adversary "sees" this, we successfully got through + // their local validation. if len(adversaryMessages) != 2 { t.Fatalf("got %d messages, expected 2", len(adversaryMessages)) } From a726a896dbc3d933324e64dca5e6482f65d448ce Mon Sep 17 00:00:00 2001 From: Raghav Gulati Date: Wed, 8 May 2019 09:27:26 -0700 Subject: [PATCH 11/13] Use names rather than index We've aliased array elements, use the explicit names instead. --- floodsub_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/floodsub_test.go b/floodsub_test.go index ca8d923..3dfe701 100644 --- a/floodsub_test.go +++ b/floodsub_test.go @@ -970,7 +970,7 @@ func TestImproperlySignedMessageRejected(t *testing.T) { WithStrictSignatureVerification(true), ) - connect(t, hosts[0], hosts[1]) + connect(t, adversary, honestPeer) var ( topic = "foobar" From 08c4a28ee3cb7c85f6efdd735ef523f0c9becd34 Mon Sep 17 00:00:00 2001 From: Raghav Gulati Date: Wed, 8 May 2019 09:27:57 -0700 Subject: [PATCH 12/13] Better comment for signing with a different key Note that the second message is the incorrectly signed message. --- floodsub_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/floodsub_test.go b/floodsub_test.go index 3dfe701..585e166 100644 --- a/floodsub_test.go +++ b/floodsub_test.go @@ -994,7 +994,8 @@ func TestImproperlySignedMessageRejected(t *testing.T) { t.Fatal(err) } - // Change the sign key for the adversarial peer, and send the second message. + // Change the sign key for the adversarial peer, and send the second, + // incorrectly signed, message. adversaryPubSub.signID = honestPubSub.signID adversaryPubSub.signKey = honestPubSub.host.Peerstore().PrivKey(honestPubSub.signID) err = adversaryPubSub.Publish(topic, incorrectMessage) From 3c78c237ce2e621b839bf70e3587f0e10c2e46b2 Mon Sep 17 00:00:00 2001 From: Raghav Gulati Date: Wed, 8 May 2019 10:09:30 -0700 Subject: [PATCH 13/13] Remove empty line in imports --- floodsub_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/floodsub_test.go b/floodsub_test.go index 585e166..be673ea 100644 --- a/floodsub_test.go +++ b/floodsub_test.go @@ -10,12 +10,11 @@ import ( "testing" "time" + bhost "github.com/libp2p/go-libp2p-blankhost" host "github.com/libp2p/go-libp2p-host" peer "github.com/libp2p/go-libp2p-peer" protocol "github.com/libp2p/go-libp2p-protocol" swarmt "github.com/libp2p/go-libp2p-swarm/testing" - - bhost "github.com/libp2p/go-libp2p-blankhost" ) func checkMessageRouting(t *testing.T, topic string, pubs []*PubSub, subs []*Subscription) {