Updates https://github.com/tendermint/tendermint/issues/850
My security alarms falsely blarred when I skimmed and noticed
keys being compared with `==`, without the proper context
so I mistakenly filed an issue, yet the purpose of that
comparison was to check if the local ephemeral public key
was just the least, sorted lexicographically.
Anyways, let's use the proper bytes.Equal check, to save future labor.
Fixes https://github.com/tendermint/tendermint/issues/851
Go1.9 and below's net.Pipe did not implement the SetDeadline
method so after commit
e2dd8ca946
this problem was exposed since now we check for errors.
To counter this problem, implement a simple composition for
net.Conn that always returns nil on SetDeadline instead of
tripping out.
Added build tags so that anyone using go1.10 when it is released
will be able to automatically use net.Pipe's net.Conns
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
```
Just noticed while auditing the code in p2p/addrbook.go,
wg.Add(1) but no subsequent defer.
@jaekwon and I had a discussion offline and we agreed to
comment about why the code was that way and why
we shouldn't move the wg.Add(1) into .saveRoutine() because
if go a.saveRoutine() isn't started before anyone invokes
a.Wait(), then we'd have raced a.saveRoutine().
Emmanuel Odeke
Ethan Buchman
Zach Ramsay
Guanghua Guo
caffix
Anton Kaliaev