Skip to content

Commit

Permalink
noise: add an option to allow unknown peer ID in SecureOutbound (#1823)
Browse files Browse the repository at this point in the history
  • Loading branch information
ckousik authored Nov 9, 2022
1 parent 0575c19 commit c334288
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 14 deletions.
2 changes: 1 addition & 1 deletion p2p/security/noise/crypto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func TestCryptoFailsIfHandshakeIncomplete(t *testing.T) {
init, resp := net.Pipe()
_ = resp.Close()

session, _ := newSecureSession(initTransport, context.TODO(), init, "remote-peer", nil, nil, nil, true)
session, _ := newSecureSession(initTransport, context.TODO(), init, "remote-peer", nil, nil, nil, true, true)
_, err := session.encrypt(nil, []byte("hi"))
if err == nil {
t.Error("expected encryption error when handshake incomplete")
Expand Down
7 changes: 2 additions & 5 deletions p2p/security/noise/handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,11 +272,8 @@ func (s *secureSession) handleRemoteHandshakePayload(payload []byte, remoteStati
return nil, err
}

// check the peer ID for:
// * all outbound connection
// * inbound connections, if we know which peer we want to connect to (SecureInbound called with a peer ID)
if (s.initiator && s.remoteID != id) || (!s.initiator && s.remoteID != "" && s.remoteID != id) {
// use Pretty() as it produces the full b58-encoded string, rather than abbreviated forms.
// check the peer ID if enabled
if s.checkPeerID && s.remoteID != id {
return nil, fmt.Errorf("peer id mismatch: expected %s, but remote key matches %s", s.remoteID.Pretty(), id.Pretty())
}

Expand Down
6 changes: 4 additions & 2 deletions p2p/security/noise/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import (
)

type secureSession struct {
initiator bool
initiator bool
checkPeerID bool

localID peer.ID
localKey crypto.PrivKey
Expand Down Expand Up @@ -47,7 +48,7 @@ type secureSession struct {

// newSecureSession creates a Noise session over the given insecureConn Conn, using
// the libp2p identity keypair from the given Transport.
func newSecureSession(tpt *Transport, ctx context.Context, insecure net.Conn, remote peer.ID, prologue []byte, initiatorEDH, responderEDH EarlyDataHandler, initiator bool) (*secureSession, error) {
func newSecureSession(tpt *Transport, ctx context.Context, insecure net.Conn, remote peer.ID, prologue []byte, initiatorEDH, responderEDH EarlyDataHandler, initiator, checkPeerID bool) (*secureSession, error) {
s := &secureSession{
insecureConn: insecure,
insecureReader: bufio.NewReader(insecure),
Expand All @@ -58,6 +59,7 @@ func newSecureSession(tpt *Transport, ctx context.Context, insecure net.Conn, re
prologue: prologue,
initiatorEarlyDataHandler: initiatorEDH,
responderEarlyDataHandler: responderEDH,
checkPeerID: checkPeerID,
}

// the go-routine we create to run the handshake will
Expand Down
19 changes: 16 additions & 3 deletions p2p/security/noise/session_transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,22 +50,35 @@ func EarlyData(initiator, responder EarlyDataHandler) SessionOption {
}
}

// DisablePeerIDCheck disables checking the remote peer ID for a noise connection.
// For outbound connections, this is the equivalent of calling `SecureInbound` with an empty
// peer ID. This is susceptible to MITM attacks since we do not verify the identity of the remote
// peer.
func DisablePeerIDCheck() SessionOption {
return func(s *SessionTransport) error {
s.disablePeerIDCheck = true
return nil
}
}

var _ sec.SecureTransport = &SessionTransport{}

// SessionTransport can be used
// to provide per-connection options
type SessionTransport struct {
t *Transport
// options
prologue []byte
prologue []byte
disablePeerIDCheck bool

initiatorEarlyDataHandler, responderEarlyDataHandler EarlyDataHandler
}

// SecureInbound runs the Noise handshake as the responder.
// If p is empty, connections from any peer are accepted.
func (i *SessionTransport) SecureInbound(ctx context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, error) {
c, err := newSecureSession(i.t, ctx, insecure, p, i.prologue, i.initiatorEarlyDataHandler, i.responderEarlyDataHandler, false)
checkPeerID := !i.disablePeerIDCheck && p != ""
c, err := newSecureSession(i.t, ctx, insecure, p, i.prologue, i.initiatorEarlyDataHandler, i.responderEarlyDataHandler, false, checkPeerID)
if err != nil {
addr, maErr := manet.FromNetAddr(insecure.RemoteAddr())
if maErr == nil {
Expand All @@ -77,5 +90,5 @@ func (i *SessionTransport) SecureInbound(ctx context.Context, insecure net.Conn,

// SecureOutbound runs the Noise handshake as the initiator.
func (i *SessionTransport) SecureOutbound(ctx context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, error) {
return newSecureSession(i.t, ctx, insecure, p, i.prologue, i.initiatorEarlyDataHandler, i.responderEarlyDataHandler, true)
return newSecureSession(i.t, ctx, insecure, p, i.prologue, i.initiatorEarlyDataHandler, i.responderEarlyDataHandler, true, !i.disablePeerIDCheck)
}
6 changes: 3 additions & 3 deletions p2p/security/noise/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func New(privkey crypto.PrivKey, muxers []protocol.ID) (*Transport, error) {
// If p is empty, connections from any peer are accepted.
func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, error) {
responderEDH := newTransportEDH(t)
c, err := newSecureSession(t, ctx, insecure, p, nil, nil, responderEDH, false)
c, err := newSecureSession(t, ctx, insecure, p, nil, nil, responderEDH, false, p != "")
if err != nil {
addr, maErr := manet.FromNetAddr(insecure.RemoteAddr())
if maErr == nil {
Expand All @@ -67,14 +67,14 @@ func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn, p peer
// SecureOutbound runs the Noise handshake as the initiator.
func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, error) {
initiatorEDH := newTransportEDH(t)
c, err := newSecureSession(t, ctx, insecure, p, nil, initiatorEDH, nil, true)
c, err := newSecureSession(t, ctx, insecure, p, nil, initiatorEDH, nil, true, true)
if err != nil {
return c, err
}
return SessionWithConnState(c, initiatorEDH.MatchMuxers(true)), err
}

func (t *Transport) WithSessionOptions(opts ...SessionOption) (sec.SecureTransport, error) {
func (t *Transport) WithSessionOptions(opts ...SessionOption) (*SessionTransport, error) {
st := &SessionTransport{t: t}
for _, opt := range opts {
if err := opt(st); err != nil {
Expand Down
38 changes: 38 additions & 0 deletions p2p/security/noise/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,44 @@ func TestPeerIDMismatchInboundFailsHandshake(t *testing.T) {
<-done
}

func TestPeerIDInboundCheckDisabled(t *testing.T) {
initTransport := newTestTransport(t, crypto.Ed25519, 2048)
respTransport := newTestTransport(t, crypto.Ed25519, 2048)
init, resp := newConnPair(t)

initSessionTransport, err := initTransport.WithSessionOptions(DisablePeerIDCheck())
require.NoError(t, err)
errChan := make(chan error)
go func() {
_, err := initSessionTransport.SecureInbound(context.Background(), init, "test")
errChan <- err
}()
_, err = respTransport.SecureOutbound(context.Background(), resp, initTransport.localID)
require.NoError(t, err)
initErr := <-errChan
require.NoError(t, initErr)
}

func TestPeerIDOutboundNoCheck(t *testing.T) {
initTransport := newTestTransport(t, crypto.Ed25519, 2048)
respTransport := newTestTransport(t, crypto.Ed25519, 2048)
init, resp := newConnPair(t)

initSessionTransport, err := initTransport.WithSessionOptions(DisablePeerIDCheck())
require.NoError(t, err)

errChan := make(chan error)
go func() {
_, err := initSessionTransport.SecureOutbound(context.Background(), init, "test")
errChan <- err
}()

_, err = respTransport.SecureInbound(context.Background(), resp, "")
require.NoError(t, err)
initErr := <-errChan
require.NoError(t, initErr)
}

func makeLargePlaintext(size int) []byte {
buf := make([]byte, size)
rand.Read(buf)
Expand Down

0 comments on commit c334288

Please sign in to comment.