Browse Source

p2p: peer should respect errors from SetDeadline

Noticed while auditing the code that we aren't respecting
(*net.Conn) SetDeadline errors which return after
a connection has been killed and is simultaneously
being used.

For example given program, without SetDeadline error checks
```go
package main

import (
  "log"
  "net"
  "time"
)

func main() {
  conn, err := net.Dial("tcp", "tendermint.com:443")
  if err != nil {
    log.Fatal(err)
  }
  go func() {
    <-time.After(400 * time.Millisecond)
    conn.Close()
  }()
  for i := 0; i < 5; i++ {
    if err := conn.SetDeadline(time.Now().Add(time.Duration(10 * time.Second))); err != nil {
      log.Fatalf("set deadline #%d, err: %v", i, err)
    }
    log.Printf("Successfully set deadline #%d", i)
    <-time.After(150 * time.Millisecond)
  }
}
```

erraneously gives
```shell
2017/11/14 17:46:28 Successfully set deadline #0
2017/11/14 17:46:29 Successfully set deadline #1
2017/11/14 17:46:29 Successfully set deadline #2
2017/11/14 17:46:29 Successfully set deadline #3
2017/11/14 17:46:29 Successfully set deadline #4
```

However, if we properly fix it to respect that error with
```diff
--- wild.go 2017-11-14 17:44:38.000000000 -0700
+++ main.go 2017-11-14 17:45:40.000000000 -0700
@@ -16,7 +16,9 @@
    conn.Close()
  }()
  for i := 0; i < 5; i++ {
-   conn.SetDeadline(time.Now().Add(time.Duration(10 * time.Second)))
+   if err := conn.SetDeadline(time.Now().Add(time.Duration(10 *
time.Second))); err != nil {
+     log.Fatalf("set deadline #%d, err: %v", i, err)
+   }
    log.Printf("Successfully set deadline #%d", i)
    <-time.After(150 * time.Millisecond)
  }
```

properly catches any problems and gives
```shell
$ go run main.go
2017/11/14 17:43:44 Successfully set deadline #0
2017/11/14 17:43:45 Successfully set deadline #1
2017/11/14 17:43:45 Successfully set deadline #2
2017/11/14 17:43:45 set deadline #3, err: set tcp 10.182.253.51:57395:
use of closed network connection
exit status 1
```
pull/848/head
Emmanuel Odeke 7 years ago
parent
commit
7b0fa6c889
No known key found for this signature in database GPG Key ID: 1CA47A292F89DD40
1 changed files with 9 additions and 3 deletions
  1. +9
    -3
      p2p/peer.go

+ 9
- 3
p2p/peer.go View File

@ -113,7 +113,9 @@ func newPeerFromConnAndConfig(rawConn net.Conn, outbound bool, reactorsByCh map[
// Encrypt connection
if config.AuthEnc {
conn.SetDeadline(time.Now().Add(config.HandshakeTimeout * time.Second))
if err := conn.SetDeadline(time.Now().Add(config.HandshakeTimeout * time.Second)); err != nil {
return nil, errors.Wrap(err, "Error setting deadline while encrypting connection")
}
var err error
conn, err = MakeSecretConnection(conn, ourNodePrivKey)
@ -165,7 +167,9 @@ func (p *peer) IsPersistent() bool {
// NOTE: blocking
func (p *peer) HandshakeTimeout(ourNodeInfo *NodeInfo, timeout time.Duration) error {
// Set deadline for handshake so we don't block forever on conn.ReadFull
p.conn.SetDeadline(time.Now().Add(timeout))
if err := p.conn.SetDeadline(time.Now().Add(timeout)); err != nil {
return errors.Wrap(err, "Error setting deadline")
}
var peerNodeInfo = new(NodeInfo)
var err1 error
@ -196,7 +200,9 @@ func (p *peer) HandshakeTimeout(ourNodeInfo *NodeInfo, timeout time.Duration) er
}
// Remove deadline
p.conn.SetDeadline(time.Time{})
if err := p.conn.SetDeadline(time.Time{}); err != nil {
return errors.Wrap(err, "Error removing deadline")
}
peerNodeInfo.RemoteAddr = p.Addr().String()


Loading…
Cancel
Save