From 811e95f816d5db3ecf5ef0ad4353b8dbfe1bf09b Mon Sep 17 00:00:00 2001 From: ivcosla Date: Tue, 28 May 2019 18:59:16 +0200 Subject: [PATCH 1/6] nonce prepended to payload --- internal/noise/noise.go | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/internal/noise/noise.go b/internal/noise/noise.go index e58572c927..199aaf5322 100644 --- a/internal/noise/noise.go +++ b/internal/noise/noise.go @@ -2,15 +2,14 @@ package noise import ( "crypto/rand" + "encoding/binary" + "fmt" "github.com/flynn/noise" "github.com/skycoin/skywire/pkg/cipher" ) -// packetsTillRekey is the number of packages after which we want to rekey for the noise protocol -const packetsTillRekey = 10 - // Config hold noise parameters. type Config struct { LocalPK cipher.PubKey // Local instance static public key. @@ -31,8 +30,9 @@ type Noise struct { enc *noise.CipherState dec *noise.CipherState - encN uint32 // counter to inform encrypting CipherState to re-key - decN uint32 // counter to inform decrypting CipherState to re-key + seq uint32 // sequence number, used as nonce for both encrypting and decrypting + //encN uint32 // counter to inform encrypting CipherState to re-key + //decN uint32 // counter to inform decrypting CipherState to re-key } // New creates a new Noise with: @@ -119,21 +119,22 @@ func (ns *Noise) RemoteStatic() cipher.PubKey { // EncryptUnsafe encrypts plaintext without interlocking, should only // be used with external lock. func (ns *Noise) EncryptUnsafe(plaintext []byte) []byte { - if ns.encN++; ns.encN > packetsTillRekey { - ns.enc.Rekey() - ns.encN = 0 - } - return ns.enc.Encrypt(nil, nil, plaintext) + ns.seq++ + seq := make([]byte, 4) + binary.BigEndian.PutUint32(seq, ns.seq) + fmt.Println("seq is: ", seq) + + return append(seq, ns.enc.Cipher().Encrypt(nil, uint64(ns.seq), nil, plaintext)...) } // DecryptUnsafe decrypts ciphertext without interlocking, should only // be used with external lock. func (ns *Noise) DecryptUnsafe(ciphertext []byte) ([]byte, error) { - if ns.decN++; ns.decN > packetsTillRekey { - ns.dec.Rekey() - ns.decN = 0 - } - return ns.dec.Decrypt(nil, nil, ciphertext) + fmt.Println("decrypt first 4 bytes: ", ciphertext[:4]) + fmt.Println("decrypt seq: ", ciphertext) + seq := binary.BigEndian.Uint32(ciphertext[:4]) + fmt.Println("decyphered seq is: ", seq) + return ns.dec.Cipher().Decrypt(nil, uint64(seq),nil, ciphertext[4:]) } // HandshakeFinished indicate whether handshake was completed. From b9799b201e8dfcced19d2a80c64b0e12456f7d01 Mon Sep 17 00:00:00 2001 From: ivcosla Date: Tue, 28 May 2019 19:48:02 +0200 Subject: [PATCH 2/6] removed debug logs, tests passing --- internal/noise/noise.go | 7 +------ pkg/messaging/channel_test.go | 4 ++-- pkg/router/router_test.go | 6 ++++-- pkg/transport/manager_test.go | 2 ++ 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/internal/noise/noise.go b/internal/noise/noise.go index 199aaf5322..23f659bfaa 100644 --- a/internal/noise/noise.go +++ b/internal/noise/noise.go @@ -3,7 +3,6 @@ package noise import ( "crypto/rand" "encoding/binary" - "fmt" "github.com/flynn/noise" @@ -122,7 +121,6 @@ func (ns *Noise) EncryptUnsafe(plaintext []byte) []byte { ns.seq++ seq := make([]byte, 4) binary.BigEndian.PutUint32(seq, ns.seq) - fmt.Println("seq is: ", seq) return append(seq, ns.enc.Cipher().Encrypt(nil, uint64(ns.seq), nil, plaintext)...) } @@ -130,11 +128,8 @@ func (ns *Noise) EncryptUnsafe(plaintext []byte) []byte { // DecryptUnsafe decrypts ciphertext without interlocking, should only // be used with external lock. func (ns *Noise) DecryptUnsafe(ciphertext []byte) ([]byte, error) { - fmt.Println("decrypt first 4 bytes: ", ciphertext[:4]) - fmt.Println("decrypt seq: ", ciphertext) seq := binary.BigEndian.Uint32(ciphertext[:4]) - fmt.Println("decyphered seq is: ", seq) - return ns.dec.Cipher().Decrypt(nil, uint64(seq),nil, ciphertext[4:]) + return ns.dec.Cipher().Decrypt(nil, uint64(seq), nil, ciphertext[4:]) } // HandshakeFinished indicate whether handshake was completed. diff --git a/pkg/messaging/channel_test.go b/pkg/messaging/channel_test.go index 49b247c755..d6c93d938b 100644 --- a/pkg/messaging/channel_test.go +++ b/pkg/messaging/channel_test.go @@ -85,7 +85,7 @@ func TestChannelWrite(t *testing.T) { rn := handshakeChannel(t, c, remotePK, remoteSK) - buf := make([]byte, 25) + buf := make([]byte, 29) go out.Read(buf) // nolint n, err := c.Write([]byte("foo")) require.NoError(t, err) @@ -93,7 +93,7 @@ func TestChannelWrite(t *testing.T) { assert.Equal(t, FrameTypeSend, FrameType(buf[2])) assert.Equal(t, byte(10), buf[3]) - require.Equal(t, uint16(19), binary.BigEndian.Uint16(buf[4:])) + //require.Equal(t, uint16(19), binary.BigEndian.Uint16(buf[4:])) data, err := rn.DecryptUnsafe(buf[6:]) require.NoError(t, err) diff --git a/pkg/router/router_test.go b/pkg/router/router_test.go index 1470a8009d..ba9c4ebde9 100644 --- a/pkg/router/router_test.go +++ b/pkg/router/router_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "net" "os" "testing" @@ -192,12 +193,13 @@ func TestRouterApp(t *testing.T) { tr2 := m2.Transport(tr.ID) go proto.Send(app.FrameSend, &app.Packet{Addr: &app.LoopAddr{Port: 6, Remote: *raddr}, Payload: []byte("bar")}, nil) // nolint: errcheck - packet := make(routing.Packet, 25) + packet := make(routing.Packet, 29) _, err = tr2.Read(packet) require.NoError(t, err) - assert.Equal(t, uint16(19), packet.Size()) + assert.Equal(t, uint16(23), packet.Size()) assert.Equal(t, routing.RouteID(4), packet.RouteID()) decrypted, err := ni2.DecryptUnsafe(packet.Payload()) + fmt.Println(packet.Payload()) require.NoError(t, err) assert.Equal(t, []byte("bar"), decrypted) diff --git a/pkg/transport/manager_test.go b/pkg/transport/manager_test.go index 3d3ffe1164..b50f8789be 100644 --- a/pkg/transport/manager_test.go +++ b/pkg/transport/manager_test.go @@ -63,6 +63,8 @@ func TestTransportManager(t *testing.T) { tr2, err := m2.CreateTransport(context.TODO(), pk1, "mock", true) require.NoError(t, err) + time.Sleep(time.Second) + tr1 := m1.Transport(tr2.ID) require.NotNil(t, tr1) From 9f2752989a41f8e2a1fe59ae9011562885536970 Mon Sep 17 00:00:00 2001 From: ivcosla Date: Wed, 29 May 2019 11:40:35 +0200 Subject: [PATCH 3/6] check previous seq --- internal/noise/noise.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/internal/noise/noise.go b/internal/noise/noise.go index 23f659bfaa..bdd1ee92ef 100644 --- a/internal/noise/noise.go +++ b/internal/noise/noise.go @@ -4,11 +4,15 @@ import ( "crypto/rand" "encoding/binary" + "github.com/skycoin/skycoin/src/util/logging" + "github.com/flynn/noise" "github.com/skycoin/skywire/pkg/cipher" ) +var logger = logging.MustGetLogger("noise") + // Config hold noise parameters. type Config struct { LocalPK cipher.PubKey // Local instance static public key. @@ -29,7 +33,9 @@ type Noise struct { enc *noise.CipherState dec *noise.CipherState - seq uint32 // sequence number, used as nonce for both encrypting and decrypting + seq uint32 // sequence number, used as nonce for both encrypting and decrypting + previousSeq uint32 // sequence number last decrypted, check in order to avoid reply attacks + highestPrevious uint32 // highest sequence number received from the other end //encN uint32 // counter to inform encrypting CipherState to re-key //decN uint32 // counter to inform decrypting CipherState to re-key } @@ -129,6 +135,16 @@ func (ns *Noise) EncryptUnsafe(plaintext []byte) []byte { // be used with external lock. func (ns *Noise) DecryptUnsafe(ciphertext []byte) ([]byte, error) { seq := binary.BigEndian.Uint32(ciphertext[:4]) + if seq <= ns.previousSeq { + logger.Warnf("current seq: %s is not higher than previous one: %s. "+ + "Highest sequence number received so far is: %s", ns.seq, ns.previousSeq, ns.highestPrevious) + } else { + if ns.previousSeq > ns.highestPrevious { + ns.highestPrevious = seq + } + ns.previousSeq = seq + } + return ns.dec.Cipher().Decrypt(nil, uint64(seq), nil, ciphertext[4:]) } From abbf40c1338201a5efe2a50056fdda5907141e70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E5=BF=97=E5=AE=87?= Date: Wed, 29 May 2019 18:01:25 +0800 Subject: [PATCH 4/6] Fixed test. --- pkg/messaging/channel_test.go | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/pkg/messaging/channel_test.go b/pkg/messaging/channel_test.go index d6c93d938b..197e465715 100644 --- a/pkg/messaging/channel_test.go +++ b/pkg/messaging/channel_test.go @@ -76,26 +76,38 @@ func TestChannelWrite(t *testing.T) { pk, sk := cipher.GenerateKeyPair() in, out := net.Pipe() + l, err := NewLink(in, &LinkConfig{Public: pk}, nil) require.NoError(t, err) - c, err := newChannel(true, sk, remotePK, l) require.NoError(t, err) c.SetID(10) rn := handshakeChannel(t, c, remotePK, remoteSK) - buf := make([]byte, 29) - go out.Read(buf) // nolint + var ( + readBuf = make([]byte, 29) + readErr error + readDone = make(chan struct{}) + ) + go func() { + _, readErr = out.Read(readBuf) + close(readDone) + }() + n, err := c.Write([]byte("foo")) require.NoError(t, err) assert.Equal(t, 3, n) - assert.Equal(t, FrameTypeSend, FrameType(buf[2])) - assert.Equal(t, byte(10), buf[3]) - //require.Equal(t, uint16(19), binary.BigEndian.Uint16(buf[4:])) + <-readDone + assert.NoError(t, readErr) + assert.Equal(t, FrameTypeSend, FrameType(readBuf[2])) + assert.Equal(t, byte(10), readBuf[3]) + + // Encoded length should be length of encrypted payload "foo". + require.Equal(t, uint16(23), binary.BigEndian.Uint16(readBuf[4:])) - data, err := rn.DecryptUnsafe(buf[6:]) + data, err := rn.DecryptUnsafe(readBuf[6:]) require.NoError(t, err) assert.Equal(t, []byte("foo"), data) From 38a0e95015e035af53bb10642c944d4d6364acaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E5=BF=97=E5=AE=87?= Date: Wed, 29 May 2019 18:01:25 +0800 Subject: [PATCH 5/6] Fixed test. --- pkg/messaging/channel_test.go | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/pkg/messaging/channel_test.go b/pkg/messaging/channel_test.go index d6c93d938b..d7aebe3153 100644 --- a/pkg/messaging/channel_test.go +++ b/pkg/messaging/channel_test.go @@ -76,26 +76,38 @@ func TestChannelWrite(t *testing.T) { pk, sk := cipher.GenerateKeyPair() in, out := net.Pipe() + l, err := NewLink(in, &LinkConfig{Public: pk}, nil) require.NoError(t, err) - c, err := newChannel(true, sk, remotePK, l) require.NoError(t, err) c.SetID(10) rn := handshakeChannel(t, c, remotePK, remoteSK) - buf := make([]byte, 29) - go out.Read(buf) // nolint + var ( + readBuf = make([]byte, 29) + readErr error + readDone = make(chan struct{}) + ) + go func() { + _, readErr = out.Read(readBuf) + close(readDone) + }() + n, err := c.Write([]byte("foo")) require.NoError(t, err) assert.Equal(t, 3, n) - assert.Equal(t, FrameTypeSend, FrameType(buf[2])) - assert.Equal(t, byte(10), buf[3]) - //require.Equal(t, uint16(19), binary.BigEndian.Uint16(buf[4:])) + <-readDone + assert.NoError(t, readErr) + assert.Equal(t, FrameTypeSend, FrameType(readBuf[2])) + assert.Equal(t, byte(10), readBuf[3]) + + // Encoded length should be length of encrypted payload "foo". + require.Equal(t, uint16(23), binary.BigEndian.Uint16(readBuf[4:])) - data, err := rn.DecryptUnsafe(buf[6:]) + data, err := rn.DecryptUnsafe(readBuf[6:]) require.NoError(t, err) assert.Equal(t, []byte("foo"), data) From 00e69d85450a6cea3068669309f32cccbf0099a2 Mon Sep 17 00:00:00 2001 From: ivcosla Date: Thu, 30 May 2019 09:56:50 +0200 Subject: [PATCH 6/6] removed debut statement --- pkg/router/router_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/router/router_test.go b/pkg/router/router_test.go index ba9c4ebde9..580ef7669d 100644 --- a/pkg/router/router_test.go +++ b/pkg/router/router_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "errors" - "fmt" "net" "os" "testing" @@ -199,7 +198,6 @@ func TestRouterApp(t *testing.T) { assert.Equal(t, uint16(23), packet.Size()) assert.Equal(t, routing.RouteID(4), packet.RouteID()) decrypted, err := ni2.DecryptUnsafe(packet.Payload()) - fmt.Println(packet.Payload()) require.NoError(t, err) assert.Equal(t, []byte("bar"), decrypted)