-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Changes from 8 commits
25fb9d2
dabddb9
1f7b15b
9a5a2d5
8e117a9
a2913e6
eeb8d4d
09d0d63
bbc2cb4
f1a924d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,22 +50,34 @@ 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) | ||
c, err := newSecureSession(i.t, ctx, insecure, p, i.prologue, i.initiatorEarlyDataHandler, i.responderEarlyDataHandler, false, p != "" || i.disablePeerIdCheck) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ckousik Why are you setting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is to keep behaviour consistent with setting an empty string as peer id. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a quick test and the correct conditional here was |
||
if err != nil { | ||
addr, maErr := manet.FromNetAddr(insecure.RemoteAddr()) | ||
if maErr == nil { | ||
|
@@ -77,5 +89,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) | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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 { | ||||||
|
@@ -67,15 +67,15 @@ 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) { | ||||||
st := &SessionTransport{t: t} | ||||||
func (t *Transport) WithSessionOptions(opts ...SessionOption) (*SessionTransport, error) { | ||||||
st := &SessionTransport{t: t, disablePeerIdCheck: false} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
for _, opt := range opts { | ||||||
if err := opt(st); err != nil { | ||||||
return nil, err | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.