diff --git a/client/lib/rendezvous.go b/client/lib/rendezvous.go index 68a81e6..bf1a48c 100644 --- a/client/lib/rendezvous.go +++ b/client/lib/rendezvous.go @@ -124,16 +124,13 @@ func newBrokerChannelFromConfig(config ClientConfig) (*BrokerChannel, error) { // Negotiate uses a RendezvousMethod to send the client's WebRTC SDP offer // and receive a snowflake proxy WebRTC SDP answer in return. -func (bc *BrokerChannel) Negotiate(offer *webrtc.SessionDescription) ( +func (bc *BrokerChannel) Negotiate( + offer *webrtc.SessionDescription, + natTypeToSend string, +) ( *webrtc.SessionDescription, error, ) { - - bc.lock.Lock() - natType := bc.natType - bridgeFingerprint := bc.BridgeFingerprint - bc.lock.Unlock() - - encReq, err := preparePollRequest(offer, natType, bridgeFingerprint) + encReq, err := preparePollRequest(offer, natTypeToSend, bc.BridgeFingerprint) if err != nil { return nil, err } @@ -183,9 +180,72 @@ func (bc *BrokerChannel) SetNATType(NATType string) { log.Printf("NAT Type: %s", NATType) } +func (bc *BrokerChannel) GetNATType() string { + bc.lock.Lock() + defer bc.lock.Unlock() + return bc.natType +} + +type NATPolicy struct { + assumedUnrestrictedNATAndFailedToConnect bool +} + +// When our NAT type is unknown, we want to try to connect to a +// restricted / unknown proxy initially +// to offload the unrestricted ones. +// So, instead of always sending the actual NAT type, +// we should use this function to determine the NAT type to send. +// +// This is useful when our STUN servers are blocked or don't support +// the NAT discovery feature, or if they're just slow. +func (p *NATPolicy) NATTypeToSend(actualNatType string) string { + if !p.assumedUnrestrictedNATAndFailedToConnect && actualNatType == nat.NATUnknown { + // If our NAT type is unknown, and we haven't failed to connect + // with a spoofed NAT type yet, then spoof a NATUnrestricted + // type. + return nat.NATUnrestricted + } else { + // In all other cases, do not spoof, and just return our actual + // NAT type (even if it is NATUnknown). + return actualNatType + } +} + +// This function must be called whenever a connection with a proxy succeeds, +// because the connection outcome tells us about NAT compatibility +// between the proxy and us. +func (p *NATPolicy) Success(actualNATType, sentNATType string) { + // Yes, right now this does nothing but log. + if actualNATType != sentNATType { + log.Printf( + "Connected to a proxy by using a spoofed NAT type \"%v\"! "+ + "Our actual NAT type was \"%v\"", + sentNATType, + actualNATType, + ) + } +} + +// This function must be called whenever a connection with a proxy fails, +// because the connection outcome tells us about NAT compatibility +// between the proxy and us. +func (p *NATPolicy) Failure(actualNATType, sentNATType string) { + if actualNATType == nat.NATUnknown && sentNATType == nat.NATUnrestricted { + log.Printf( + "Tried to connect to a restricted proxy while our NAT type "+ + "is \"%v\", and failed. Let's not do that again.", + actualNATType, + ) + p.assumedUnrestrictedNATAndFailedToConnect = true + } +} + // WebRTCDialer implements the |Tongue| interface to catch snowflakes, using BrokerChannel. type WebRTCDialer struct { *BrokerChannel + // Can be `nil`, in which case we won't apply special logic, + // and simply always send the current NAT type instead. + natPolicy *NATPolicy webrtcConfig *webrtc.Configuration max int @@ -193,19 +253,42 @@ type WebRTCDialer struct { proxy *url.URL } -// Deprecated: Use NewWebRTCDialerWithEventsAndProxy instead +// Deprecated: Use NewWebRTCDialerWithNatPolicyAndEventsAndProxy instead func NewWebRTCDialer(broker *BrokerChannel, iceServers []webrtc.ICEServer, max int) *WebRTCDialer { - return NewWebRTCDialerWithEventsAndProxy(broker, iceServers, max, nil, nil) + return NewWebRTCDialerWithNatPolicyAndEventsAndProxy( + broker, nil, iceServers, max, nil, nil, + ) } -// Deprecated: Use NewWebRTCDialerWithEventsAndProxy instead +// Deprecated: Use NewWebRTCDialerWithNatPolicyAndEventsAndProxy instead func NewWebRTCDialerWithEvents(broker *BrokerChannel, iceServers []webrtc.ICEServer, max int, eventLogger event.SnowflakeEventReceiver) *WebRTCDialer { - return NewWebRTCDialerWithEventsAndProxy(broker, iceServers, max, eventLogger, nil) + return NewWebRTCDialerWithNatPolicyAndEventsAndProxy( + broker, nil, iceServers, max, eventLogger, nil, + ) } -// NewWebRTCDialerWithEventsAndProxy constructs a new WebRTCDialer. +// Deprecated: Use NewWebRTCDialerWithNatPolicyAndEventsAndProxy instead func NewWebRTCDialerWithEventsAndProxy(broker *BrokerChannel, iceServers []webrtc.ICEServer, max int, eventLogger event.SnowflakeEventReceiver, proxy *url.URL, +) *WebRTCDialer { + return NewWebRTCDialerWithNatPolicyAndEventsAndProxy( + broker, + nil, + iceServers, + max, + eventLogger, + proxy, + ) +} + +// NewWebRTCDialerWithNatPolicyAndEventsAndProxy constructs a new WebRTCDialer. +func NewWebRTCDialerWithNatPolicyAndEventsAndProxy( + broker *BrokerChannel, + natPolicy *NATPolicy, + iceServers []webrtc.ICEServer, + max int, + eventLogger event.SnowflakeEventReceiver, + proxy *url.URL, ) *WebRTCDialer { config := webrtc.Configuration{ ICEServers: iceServers, @@ -213,6 +296,7 @@ func NewWebRTCDialerWithEventsAndProxy(broker *BrokerChannel, iceServers []webrt return &WebRTCDialer{ BrokerChannel: broker, + natPolicy: natPolicy, webrtcConfig: &config, max: max, @@ -225,7 +309,9 @@ func NewWebRTCDialerWithEventsAndProxy(broker *BrokerChannel, iceServers []webrt func (w WebRTCDialer) Catch() (*WebRTCPeer, error) { // TODO: [#25591] Fetch ICE server information from Broker. // TODO: [#25596] Consider TURN servers here too. - return NewWebRTCPeerWithEventsAndProxy(w.webrtcConfig, w.BrokerChannel, w.eventLogger, w.proxy) + return NewWebRTCPeerWithNatPolicyAndEventsAndProxy( + w.webrtcConfig, w.BrokerChannel, w.natPolicy, w.eventLogger, w.proxy, + ) } // GetMax returns the maximum number of snowflakes to collect. diff --git a/client/lib/rendezvous_test.go b/client/lib/rendezvous_test.go index 17f7cd2..16a0cb3 100644 --- a/client/lib/rendezvous_test.go +++ b/client/lib/rendezvous_test.go @@ -423,7 +423,10 @@ func TestBrokerChannel(t *testing.T) { So(err, ShouldBeNil) brokerChannel.SetNATType(nat.NATRestricted) - answerSdpReturned, err := brokerChannel.Negotiate(offerSdp) + answerSdpReturned, err := brokerChannel.Negotiate( + offerSdp, + brokerChannel.GetNATType(), + ) So(err, ShouldBeNil) So(answerSdpReturned, ShouldEqual, answerSdp) diff --git a/client/lib/snowflake.go b/client/lib/snowflake.go index 29251ba..815d751 100644 --- a/client/lib/snowflake.go +++ b/client/lib/snowflake.go @@ -156,12 +156,14 @@ func NewSnowflakeClient(config ClientConfig) (*Transport, error) { } go updateNATType(iceServers, broker, config.CommunicationProxy) + natPolicy := &NATPolicy{} + max := 1 if config.Max > max { max = config.Max } eventsLogger := event.NewSnowflakeEventDispatcher() - transport := &Transport{dialer: NewWebRTCDialerWithEventsAndProxy(broker, iceServers, max, eventsLogger, config.CommunicationProxy), eventDispatcher: eventsLogger} + transport := &Transport{dialer: NewWebRTCDialerWithNatPolicyAndEventsAndProxy(broker, natPolicy, iceServers, max, eventsLogger, config.CommunicationProxy), eventDispatcher: eventsLogger} return transport, nil } diff --git a/client/lib/webrtc.go b/client/lib/webrtc.go index b648f59..9d803a2 100644 --- a/client/lib/webrtc.go +++ b/client/lib/webrtc.go @@ -45,28 +45,43 @@ type WebRTCPeer struct { proxy *url.URL } -// Deprecated: Use NewWebRTCPeerWithEventsAndProxy Instead. +// Deprecated: Use NewWebRTCPeerWithNatPolicyAndEventsAndProxy Instead. func NewWebRTCPeer( config *webrtc.Configuration, broker *BrokerChannel, ) (*WebRTCPeer, error) { - return NewWebRTCPeerWithEventsAndProxy(config, broker, nil, nil) + return NewWebRTCPeerWithNatPolicyAndEventsAndProxy( + config, broker, nil, nil, nil, + ) } -// Deprecated: Use NewWebRTCPeerWithEventsAndProxy Instead. +// Deprecated: Use NewWebRTCPeerWithNatPolicyAndEventsAndProxy Instead. func NewWebRTCPeerWithEvents( config *webrtc.Configuration, broker *BrokerChannel, eventsLogger event.SnowflakeEventReceiver, ) (*WebRTCPeer, error) { - return NewWebRTCPeerWithEventsAndProxy(config, broker, eventsLogger, nil) + return NewWebRTCPeerWithNatPolicyAndEventsAndProxy( + config, broker, nil, eventsLogger, nil, + ) } -// NewWebRTCPeerWithEventsAndProxy constructs a WebRTC PeerConnection to a snowflake proxy. +// Deprecated: Use NewWebRTCPeerWithNatPolicyAndEventsAndProxy Instead. +func NewWebRTCPeerWithEventsAndProxy( + config *webrtc.Configuration, broker *BrokerChannel, + eventsLogger event.SnowflakeEventReceiver, proxy *url.URL, +) (*WebRTCPeer, error) { + return NewWebRTCPeerWithNatPolicyAndEventsAndProxy( + config, broker, nil, eventsLogger, proxy, + ) +} + +// NewWebRTCPeerWithNatPolicyAndEventsAndProxy constructs +// a WebRTC PeerConnection to a snowflake proxy. // // The creation of the peer handles the signaling to the Snowflake broker, including // the exchange of SDP information, the creation of a PeerConnection, and the establishment // of a DataChannel to the Snowflake proxy. -func NewWebRTCPeerWithEventsAndProxy( - config *webrtc.Configuration, broker *BrokerChannel, +func NewWebRTCPeerWithNatPolicyAndEventsAndProxy( + config *webrtc.Configuration, broker *BrokerChannel, natPolicy *NATPolicy, eventsLogger event.SnowflakeEventReceiver, proxy *url.URL, ) (*WebRTCPeer, error) { if eventsLogger == nil { @@ -92,7 +107,7 @@ func NewWebRTCPeerWithEventsAndProxy( connection.eventsLogger = eventsLogger connection.proxy = proxy - err := connection.connect(config, broker) + err := connection.connect(config, broker, natPolicy) if err != nil { connection.Close() return nil, err @@ -165,8 +180,15 @@ func (c *WebRTCPeer) checkForStaleness(timeout time.Duration) { } // connect does the bulk of the work: gather ICE candidates, send the SDP offer to broker, -// receive an answer from broker, and wait for data channel to open -func (c *WebRTCPeer) connect(config *webrtc.Configuration, broker *BrokerChannel) error { +// receive an answer from broker, and wait for data channel to open. +// +// `natPolicy` can be nil, in which case we'll always send our actual +// NAT type to the broker. +func (c *WebRTCPeer) connect( + config *webrtc.Configuration, + broker *BrokerChannel, + natPolicy *NATPolicy, +) error { log.Println(c.id, " connecting...") err := c.preparePeerConnection(config, broker.keepLocalAddresses) @@ -179,7 +201,24 @@ func (c *WebRTCPeer) connect(config *webrtc.Configuration, broker *BrokerChannel return err } - answer, err := broker.Negotiate(localDescription) + actualNatType := broker.GetNATType() + var natTypeToSend string + if natPolicy != nil { + natTypeToSend = natPolicy.NATTypeToSend(actualNatType) + } else { + natTypeToSend = actualNatType + } + if natTypeToSend != actualNatType { + log.Printf( + "Our NAT type is \"%v\", but let's tell the broker it's \"%v\".", + actualNatType, + natTypeToSend, + ) + } else { + log.Printf("natTypeToSend: \"%v\" (same as actualNatType)", natTypeToSend) + } + + answer, err := broker.Negotiate(localDescription, natTypeToSend) c.eventsLogger.OnNewSnowflakeEvent(event.EventOnBrokerRendezvous{ WebRTCRemoteDescription: answer, Error: err, @@ -197,9 +236,15 @@ func (c *WebRTCPeer) connect(config *webrtc.Configuration, broker *BrokerChannel // Wait for the datachannel to open or time out select { case <-c.open: + if natPolicy != nil { + natPolicy.Success(actualNatType, natTypeToSend) + } case <-time.After(DataChannelTimeout): c.transport.Close() err = errors.New("timeout waiting for DataChannel.OnOpen") + if natPolicy != nil { + natPolicy.Failure(actualNatType, natTypeToSend) + } c.eventsLogger.OnNewSnowflakeEvent(event.EventOnSnowflakeConnectionFailed{Error: err}) return err }