@cryptotaxi247 / kubo / commits / c48f456bd

remove msgio double wrap

There was doublewrapping with an unneeded msgio. given that we use a stream muxer now, msgio is only needed by secureConn -- to signal the boundaries of an encrypted / mac-ed ciphertext. Side note: i think including the varint length in the clear is actually a bad idea that can be exploited by an attacker. it should be encrypted, too. (TODO) License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>

Juan Batiz-Benet committed Apr 12, 2015 at 15:39 UTC c48f456bdf034dddfd5ea3b6684326b15f8f7fe0
6 files changed +39 -59
p2p/net/conn/conn.go
+3 -25
@@ -6,7 +6,6 @@ import (
6 "net"
7 "time"
8
9 - msgio "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-msgio"
9 mpool "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-msgio/mpool"
10 ma "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multiaddr"
11 manet "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multiaddr-net"
@@ -32,7 +31,6 @@ type singleConn struct {
31 local peer.ID
32 remote peer.ID
33 maconn manet.Conn
35 - msgrw msgio.ReadWriteCloser
34 event io.Closer
35 }
36
@@ -44,7 +42,6 @@ func newSingleConn(ctx context.Context, local, remote peer.ID, maconn manet.Conn
42 local: local,
43 remote: remote,
44 maconn: maconn,
47 - msgrw: msgio.NewReadWriter(maconn),
45 event: log.EventBegin(ctx, "connLifetime", ml),
46 }
47
@@ -62,7 +59,7 @@ func (c *singleConn) Close() error {
59 }()
60
61 // close underlying connection
65 - return c.msgrw.Close()
62 + return c.maconn.Close()
63 }
64
65 // ID is an identifier unique to this connection.
@@ -123,31 +120,12 @@ func (c *singleConn) RemotePeer() peer.ID {
120
121 // Read reads data, net.Conn style
122 func (c *singleConn) Read(buf []byte) (int, error) {
126 - return c.msgrw.Read(buf)
123 + return c.maconn.Read(buf)
124 }
125
126 // Write writes data, net.Conn style
127 func (c *singleConn) Write(buf []byte) (int, error) {
131 - return c.msgrw.Write(buf)
132 -}
133 -
134 -func (c *singleConn) NextMsgLen() (int, error) {
135 - return c.msgrw.NextMsgLen()
136 -}
137 -
138 -// ReadMsg reads data, net.Conn style
139 -func (c *singleConn) ReadMsg() ([]byte, error) {
140 - return c.msgrw.ReadMsg()
141 -}
142 -
143 -// WriteMsg writes data, net.Conn style
144 -func (c *singleConn) WriteMsg(buf []byte) error {
145 - return c.msgrw.WriteMsg(buf)
146 -}
147 -
148 -// ReleaseMsg releases a buffer
149 -func (c *singleConn) ReleaseMsg(m []byte) {
150 - c.msgrw.ReleaseMsg(m)
128 + return c.maconn.Write(buf)
129 }
130
131 // ID returns the ID of a given Conn.
p2p/net/conn/conn_test.go
+22 -8
@@ -8,17 +8,25 @@ import (
8 "testing"
9 "time"
10
11 + msgio "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-msgio"
12 context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context"
13 travis "github.com/ipfs/go-ipfs/util/testutil/ci/travis"
14 )
15
16 +func msgioWrap(c Conn) msgio.ReadWriter {
17 + return msgio.NewReadWriter(c)
18 +}
19 +
20 func testOneSendRecv(t *testing.T, c1, c2 Conn) {
21 + mc1 := msgioWrap(c1)
22 + mc2 := msgioWrap(c2)
23 +
24 log.Debugf("testOneSendRecv from %s to %s", c1.LocalPeer(), c2.LocalPeer())
25 m1 := []byte("hello")
18 - if err := c1.WriteMsg(m1); err != nil {
26 + if err := mc1.WriteMsg(m1); err != nil {
27 t.Fatal(err)
28 }
21 - m2, err := c2.ReadMsg()
29 + m2, err := mc2.ReadMsg()
30 if err != nil {
31 t.Fatal(err)
32 }
@@ -28,11 +36,14 @@ func testOneSendRecv(t *testing.T, c1, c2 Conn) {
36 }
37
38 func testNotOneSendRecv(t *testing.T, c1, c2 Conn) {
39 + mc1 := msgioWrap(c1)
40 + mc2 := msgioWrap(c2)
41 +
42 m1 := []byte("hello")
32 - if err := c1.WriteMsg(m1); err == nil {
43 + if err := mc1.WriteMsg(m1); err == nil {
44 t.Fatal("write should have failed", err)
45 }
35 - _, err := c2.ReadMsg()
46 + _, err := mc2.ReadMsg()
47 if err == nil {
48 t.Fatal("read should have failed", err)
49 }
@@ -72,10 +83,13 @@ func TestCloseLeak(t *testing.T) {
83 ctx, cancel := context.WithCancel(context.Background())
84 c1, c2, _, _ := setupSingleConn(t, ctx)
85
86 + mc1 := msgioWrap(c1)
87 + mc2 := msgioWrap(c2)
88 +
89 for i := 0; i < num; i++ {
90 b1 := []byte(fmt.Sprintf("beep%d", i))
77 - c1.WriteMsg(b1)
78 - b2, err := c2.ReadMsg()
91 + mc1.WriteMsg(b1)
92 + b2, err := mc2.ReadMsg()
93 if err != nil {
94 panic(err)
95 }
@@ -84,8 +98,8 @@ func TestCloseLeak(t *testing.T) {
98 }
99
100 b2 = []byte(fmt.Sprintf("boop%d", i))
87 - c2.WriteMsg(b2)
88 - b1, err = c1.ReadMsg()
101 + mc2.WriteMsg(b2)
102 + b1, err = mc1.ReadMsg()
103 if err != nil {
104 panic(err)
105 }
p2p/net/conn/dial_test.go
+5 -5
@@ -187,10 +187,10 @@ func testDialer(t *testing.T, secure bool) {
187 }
188
189 // fmt.Println("sending")
190 - c.WriteMsg([]byte("beep"))
191 - c.WriteMsg([]byte("boop"))
192 -
193 - out, err := c.ReadMsg()
190 + mc := msgioWrap(c)
191 + mc.WriteMsg([]byte("beep"))
192 + mc.WriteMsg([]byte("boop"))
193 + out, err := mc.ReadMsg()
194 if err != nil {
195 t.Fatal(err)
196 }
@@ -201,7 +201,7 @@ func testDialer(t *testing.T, secure bool) {
201 t.Error("unexpected conn output", data)
202 }
203
204 - out, err = c.ReadMsg()
204 + out, err = mc.ReadMsg()
205 if err != nil {
206 t.Fatal(err)
207 }
p2p/net/conn/interface.go
+2 -3
@@ -11,7 +11,6 @@ import (
11 transport "github.com/ipfs/go-ipfs/p2p/net/transport"
12 peer "github.com/ipfs/go-ipfs/p2p/peer"
13
14 - msgio "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-msgio"
14 ma "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multiaddr"
15 )
16
@@ -46,8 +45,8 @@ type Conn interface {
45 SetReadDeadline(t time.Time) error
46 SetWriteDeadline(t time.Time) error
47
49 - msgio.Reader
50 - msgio.Writer
48 + io.Reader
49 + io.Writer
50 }
51
52 // Dialer is an object that can open connections. We could have a "convenience"
p2p/net/conn/secure_conn.go
-14
@@ -119,20 +119,6 @@ func (c *secureConn) Write(buf []byte) (int, error) {
119 return c.secure.ReadWriter().Write(buf)
120 }
121
122 -func (c *secureConn) NextMsgLen() (int, error) {
123 - return c.secure.ReadWriter().NextMsgLen()
124 -}
125 -
126 -// ReadMsg reads data, net.Conn style
127 -func (c *secureConn) ReadMsg() ([]byte, error) {
128 - return c.secure.ReadWriter().ReadMsg()
129 -}
130 -
131 -// WriteMsg writes data, net.Conn style
132 -func (c *secureConn) WriteMsg(buf []byte) error {
133 - return c.secure.ReadWriter().WriteMsg(buf)
134 -}
135 -
122 // ReleaseMsg releases a buffer
123 func (c *secureConn) ReleaseMsg(m []byte) {
124 c.secure.ReadWriter().ReleaseMsg(m)
p2p/net/conn/secure_conn_test.go
+7 -4
@@ -145,13 +145,16 @@ func TestSecureCloseLeak(t *testing.T) {
145 }
146
147 runPair := func(c1, c2 Conn, num int) {
148 + mc1 := msgioWrap(c1)
149 + mc2 := msgioWrap(c2)
150 +
151 log.Debugf("runPair %d", num)
152
153 for i := 0; i < num; i++ {
154 log.Debugf("runPair iteration %d", i)
155 b1 := []byte("beep")
153 - c1.WriteMsg(b1)
154 - b2, err := c2.ReadMsg()
156 + mc1.WriteMsg(b1)
157 + b2, err := mc2.ReadMsg()
158 if err != nil {
159 panic(err)
160 }
@@ -160,8 +163,8 @@ func TestSecureCloseLeak(t *testing.T) {
163 }
164
165 b2 = []byte("beep")
163 - c2.WriteMsg(b2)
164 - b1, err = c1.ReadMsg()
166 + mc2.WriteMsg(b2)
167 + b1, err = mc1.ReadMsg()
168 if err != nil {
169 panic(err)
170 }