Skip to content

Commit

Permalink
secio: better error detection
Browse files Browse the repository at this point in the history
The same keys + nonces in secio were being observed. As described in
ipfs#1016 -- the handshake must
be talking to itself. This can happen in an outgoing TCP dial with
REUSEPORT on to the same address.
  • Loading branch information
jbenet committed Apr 14, 2015
1 parent 695a15e commit 127c032
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 14 deletions.
6 changes: 3 additions & 3 deletions p2p/crypto/secio/al.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ func newBlockCipher(cipherT string, key []byte) (cipher.Block, error) {
// Determines which algorithm to use. Note: f(a, b) = f(b, a)
func selectBest(order int, p1, p2 string) (string, error) {
var f, s []string
switch order {
case -1:
switch {
case order < 0:
f = strings.Split(p2, ",")
s = strings.Split(p1, ",")
case 1:
case order > 0:
f = strings.Split(p1, ",")
s = strings.Split(p2, ",")
default: // Exact same preferences.
Expand Down
24 changes: 13 additions & 11 deletions p2p/crypto/secio/protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ var ErrUnsupportedKeyType = errors.New("unsupported key type")
// ErrClosed signals the closing of a connection.
var ErrClosed = errors.New("connection closed")

// ErrEcho is returned when we're attempting to handshake with the same keys and nonces.
var ErrEcho = errors.New("same keys and nonces. one side talking to self.")

// nonceSize is the size of our nonces (in bytes)
const nonceSize = 16

Expand Down Expand Up @@ -145,6 +148,10 @@ func (s *secureSession) handshake(ctx context.Context, insecure io.ReadWriter) e
oh1 := u.Hash(append(proposeIn.GetPubkey(), nonceOut...))
oh2 := u.Hash(append(myPubKeyBytes, proposeIn.GetRand()...))
order := bytes.Compare(oh1, oh2)
if order == 0 {
return ErrEcho // talking to self (same socket. must be reuseport + dialing self)
}

s.local.curveT, err = selectBest(order, SupportedExchanges, proposeIn.GetExchanges())
if err != nil {
return err
Expand Down Expand Up @@ -242,19 +249,14 @@ func (s *secureSession) handshake(ctx context.Context, insecure io.ReadWriter) e
k1, k2 := ci.KeyStretcher(s.local.cipherT, s.local.hashT, s.sharedSecret)

// use random nonces to decide order.
switch order {
case 1:
case -1:
switch {
case order > 0:
// just break
case order < 0:
k1, k2 = k2, k1 // swap
default:
log.Error("WOAH: same keys (AND same nonce: 1/(2^128) chance!).")
log.Errorf("k1: %v, k2: %v, insecure: %v, insecureM %v", k1, k2, s.insecure, s.insecureM)

// this shouldn't happen. must determine order another way.
// use the same keys but, make sure to copy underlying data!
copy(k2.IV, k1.IV)
copy(k2.MacKey, k1.MacKey)
copy(k2.CipherKey, k1.CipherKey)
// we should've bailed before this. but if not, bail here.
return ErrEcho
}
s.local.keys = k1
s.remote.keys = k2
Expand Down

0 comments on commit 127c032

Please sign in to comment.