840962 - PeerConnection can't be created if computer running Nightly is connected to VPN

Status: VERIFIED FIXED

(Core :: WebRTC: Networking, defect, P2)

Description

[Andrey Kovalenko]

User Agent: Opera/9.80 (Windows NT 6.1) Presto/2.12.388 Version/12.14

Steps to reproduce:

PC running Windows 7 is connected to PPTP VPN. I open Nightly and try to create WebRTC peer connection using following JS code

pc1 = new mozRTCPeerConnection();


Actual results:

[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [IPeerConnection.initialize]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/components/PeerConnection.js :: <TOP_LEVEL> :: line 305"  data: no]


Expected results:

Peer connection should have been created, as it does with the same code if I disconnect from VPN

Comment 1

[Jason Smith [:jsmith]]

Thanks for filing. Looks like a legit bug, maybe one of the guys cc-ed here can provide some input.

Looks like a blocker too.

Comment 2

[Eric Rescorla (:ekr)]

Can you re-run with the environment variables R_LOG_LEVEL=9 and R_LOG_DESTINATION=stderr and send us the output

Comment 3

[Andrey Kovalenko]

I believe this is the significant part of output:

(ice/DEBUG) ICE(PC:): Initializing candidates
(ice/DEBUG) ICE(PC:): initializing component with id 1
(registry/DEBUG) Couldn't fetch node 'stun.address' ('registry')
(generic/DEBUG) Got AdaptersInfo
(generic/DEBUG) Adapter Name (GUID) = {AFB1558F-C44B-4EC7-A0A8-3495E06033D1}
(generic/DEBUG) Adapter Description = VPN Connection
(generic/ERR) Got error 2 opening adapter reg key

(generic/ERR) Error 3 getting friendly name for adapter with GUID = {AFB1558F-C44B-4EC7-A0A8-3495E06033D1}
(ice/ERR) ICE(PC:): unable to find local addresses

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Hi, I tested this on my Windows 7 with an Open VPN connection and wasn't able to reproduce the problem.

Could you please give me more information about the specific PPTP VPN software you are using so I can replicate the problem?

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Or are you using the built-in Windows 7 feature? I'll test that next.

Comment 6

[Andrey Kovalenko]

Yes, I'm using build-in Windows 7 VPN client (in Network and Sharing Center)

Comment 7

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Don't fail ICE on VPN adapters which store friendly names differently

Verified fixed with a Windows 7<->Windows 7 PPTP VPN connection.

Here's a try to make sure I didn't break anything https://tbpl.mozilla.org/?tree=Try&rev=ebe8037ed1ad

Comment 8

[Eric Rescorla (:ekr)]

Comment on attachment 729668 [details] [diff] [review]
Don't fail ICE on VPN adapters which store friendly names differently

Review of attachment 729668 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/mtransport/third_party/nICEr/src/stun/addrs.c
@@ +390,5 @@
>  
>        r_log(NR_LOG_STUN, LOG_DEBUG, "Adapter Name (GUID) = %s", pAdapter->AdapterName);
>        r_log(NR_LOG_STUN, LOG_DEBUG, "Adapter Description = %s", pAdapter->Description);
>  
> +      if (!nr_win32_get_adapter_friendly_name(pAdapter->AdapterName,

Please don't test for ! when this is really success. It's confusing.

Is there a case where friendly_name is going to be zero-length?

@@ +399,5 @@
> +        friendly_name = 0;
> +      } else {
> +        // Not all adapters follow the friendly name convention. Windows' PPTP
> +        // VPN adapter puts "VPN Connection 2" in the Description field instead.
> +        r_log(NR_LOG_STUN, LOG_INFO, "Found adapter with description: %s", pAdapter->Description);

Do we know that descriptions are unique? These values need to be.

Comment 9

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Don't fail ICE on VPN adapters which store friendly names differently (2)

> Do we know that descriptions are unique? These values need to be.

From IRC: The pattern I saw from the list of adapters in the debugger was that, of the series of (unique) names seen in Start/Control Panel/Network and Internet/Network Connections (a.k.a. the adapters), things like "Local Area Connection", "Local Area Connection 2" and "Wireless Network Connection", were stored as friendly name and description held specifics of the device like "Broadcom 123etc". The exception was "VPN Connection" and "VPN Connection 2", which were stored in Description with no findable friendly name.

Windows detected me trying to rename "VPN Connection 2" to "Local Area Connection", saying:

  Error Renaming Connection:

  Cannot rename this connection. A connection with the name you
  specified already exists. Specify a different name."

even though these two names are stored differently in the registry, so it looks like Windows keeps them unique for us.

> Is there a case where friendly_name is going to be zero-length?

Now treats this as absent.
Fixed nit.

Comment 10

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Don't fail ICE on VPN adapters which store friendly names differently (3)

Fixes style nit.

Comment 12

[Eric Rescorla (:ekr)]

Comment on attachment 729799 [details] [diff] [review]
Don't fail ICE on VPN adapters which store friendly names differently (3)

Review of attachment 729799 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f1a9428e1719

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f1a9428e1719

Comment 15

[Andrey Kovalenko]

Yes, now PeerConnection is created successfully

Comment 16

[Jason Smith [:jsmith]]

(In reply to akvakh from comment #15)
> Yes, now PeerConnection is created successfully

Thanks! I'll mark as verified then.