Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

noise: add an option to allow unknown peer ID in SecureOutbound #1823

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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