886765 - make sure that no other application will send traffic when we send/receive a MMS and data traffic is off

Status: VERIFIED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Beatriz Rodríguez [:brg]]

When data is off but user send/receive a MMS, we must be sure that we will only send/receive the MMS and no other application(email, twitter...) will use the data connection to send data.

Comment 1

[José Antonio Olivera Ortega [:jaoo]]

After having a look at this I've noticed that once the network interface brings up the network manager adds mmsproxy and/or mmsc host routes. Moreover it adds the default route on the same interface which could cause non-related MMS traffic being routed through the network interface for MMS traffic.

I/Gecko   (  118): RIL Worker[0]: Handling parcel as REQUEST_SETUP_DATA_CALL
I/Gecko   (  118): -*- RadioInterfaceLayer: Received message from worker: {"status":0,"suggestedRetryTime":-1,"cid":"0","active":2,"type":"IP","ifname":"rmnet0","ipaddr":"10.28.19.3/29","dns":["80.58.61.250","80.58.61.254"],"gw":"10.28.19.1","ip":"10.28.19.3","netmask":"255.255.255.248","broadcast":"10.28.19.7","state":1,"radioTech":3,"apn":"telefonica.es","user":"telefonica","passwd":"telefonica","chappap":3,"pdptype":"IP","rilMessageType":"datacallstatechange"}
I/Gecko   (  118): -*- RadioInterfaceLayer: Data call ID: 0, interface name: rmnet0, APN name: telefonica.es
I/Gecko   (  118): RIL Worker[0]: Next parcel size unknown, going to sleep.
I/Gecko   (  118): -*- NetworkManager: Going to add host route on rmnet0
I/Gecko   (  118): Network Worker: received message: {"cmd":"addHostRoute","ifname":"rmnet0","gateway":"10.28.19.1","hostnames":["80.58.61.250","80.58.61.254",""]}
I/Gecko   (  118): -*- NetworkManager: Remove default route for rmnet0
I/Gecko   (  118): Network Worker: received message: {"cmd":"removeDefaultRoute","ifname":"rmnet0"}
I/Gecko   (  118): -*- NetworkManager: Evaluating whether active network needs to be changed.
I/Gecko   (  118): -*- NetworkManager: Network 'rmnet0' registered, adding mmsproxy and/or mmsc route
I/Gecko   (  118): -*- NetworkManager: Going to add host route after dns resolution on rmnet0
I/Gecko   (  118): -*- NetworkManager: Network 'rmnet0' registered.
I/Gecko   (  118): Network Worker: received message: {"cmd":"addHostRoute","ifname":"rmnet0","gateway":"10.28.19.1","hostnames":["10.138.255.5"]}
I/Gecko   (  118): -@- MmsService: Got the MMS network connected! Resend the buffered MMS requests: number: 1
I/Gecko   (  118): -@- MmsService: sendRequest: register proxy filter to http://mms.movistar.com
I/Gecko   (  118): -@- MmsService: applyFilter: MMSC is matched: {"url":"http://mms.movistar.com","proxyInfo":{"host":"10.138.255.5","port":8080,"type":"http","flags":1,"resolveFlags":0,"failoverTimeout":1800,"failoverProxy":null,"TRANSPARENT_PROXY_RESOLVES_HOST":1}}
I/Gecko   (  118): -*- NetworkManager: Network rmnet0 changed state to 1
I/Gecko   (  118): -*- NetworkManager: Going to add host route on rmnet0
I/Gecko   (  118): -*- NetworkManager: Remove default route for rmnet0
I/Gecko   (  118): -*- NetworkManager: Evaluating whether active network needs to be changed.
I/Gecko   (  118): Network Worker: received message: {"cmd":"addHostRoute","ifname":"rmnet0","gateway":"10.28.19.1","hostnames":["80.58.61.250","80.58.61.254",""]}
I/Gecko   (  118): -*- NetworkManager: Going to change route and DNS to rmnet0
I/Gecko   (  118): Network Worker: received message: {"cmd":"removeDefaultRoute","ifname":"rmnet0"}
I/Gecko   (  118): -*- NetworkManager: No proxy support for rmnet0 network interface.
I/Gecko   (  118): Network Worker: received message: {"cmd":"setDefaultRouteAndDNS","ifname":"rmnet0","oldIfname":null,"gateway_str":"10.28.19.1","dns1_str":"80.58.61.250","dns2_str":"80.58.61.254"}


[eniac ~]$ adb shell cat /proc/net/route
Iface	Destination	Gateway 	Flags	RefCnt	Use	Metric	Mask		MTU	Window	IRTT                                                       
rmnet0	00000000	01131C0A	0003	0	0	0	00000000	0	0	0                                                                             
rmnet0	00131C0A	00000000	0001	0	0	0	F8FFFFFF	0	0	0                                                                             
rmnet0	05FF8A0A	01131C0A	0007	0	0	0	FFFFFFFF	0	0	0                                                                             
rmnet0	FA3D3A50	01131C0A	0007	0	0	0	FFFFFFFF	0	0	0                                                                             
rmnet0	FE3D3A50	01131C0A	0007	0	0	0	FFFFFFFF	0	0	0

Comment 2

[Dietrich Ayala (:dietrich)]

Conversed at triage, closing this. B will reopen once the code lands and if this is an issue at that time.

Comment 3

[José Antonio Olivera Ortega [:jaoo]]

IMHO we shouldn't resolve this bug as invalid but reopen it. After applying bug 837488 on birch and making some test the issue is still here and other apps could route traffic through the interface we bring up for sending the MMS message.

This is how the routing table is configured by the time of sending the message:

[eniac ~]$ adb shell cat /proc/net/route
Iface	Destination	Gateway 	Flags	Mask	
rmnet0	00000000	BDC9030A	0003	00000000
rmnet0	BCC9030A	00000000	0001	FCFFFFFF
rmnet0	05FF8A0A	BDC9030A	0007	FFFFFFFF
rmnet0	FA3D3A50	BDC9030A	0007	FFFFFFFF
rmnet0	FE3D3A50	BDC9030A	0007	FFFFFFFF

The network manager adds routes to DNS servers ["80.58.61.250","80.58.61.254"] and mmscproxy host ["10.138.255.5"] and also adds the default route (see [1]) on the `rmnet0` interface. After doing that the network manager also changes the `Services.io.offline` property (see [2]) which AFAIK it notifies gecko when the network comes back online.

You guys could think that the network gateway drops the traffic but: i) we cannot make sure of that since carriers might not do it and ii) we could be using a shared APN for data and MMS which makes the network gateway route the traffic. 

[1] http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/NetworkManager.js#501
[2] http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/NetworkManager.js#516

Ken, AFAIK you've been discussing with Beatriz about this issue. Could you shed some light here please? Thanks.

Comment 4

[Antonio Manuel Amaya Calvo (:amac)]

Er why was this closed as invalid?

If the APN is shared (as it happens to be on Movistar Spain) and we allow data to be sent when we bring the interface up to send a MMS, then unless the user has a flat rate data contract (which if he had data off he probably don't) he'll be charged for use. Charge that can run pretty high pretty fast. Seems like a serious enough problem to merit fixing it before launch.

Comment 5

[Ken Chang[:kenkai]]

José, you are right. We should not set defualt route for MMS and supl network interface.

Comment 6

[Beatriz Rodríguez [:brg]]

Thanks Ken and José Antonio for investigating this issue.
Following previous comments, bug mark as REOPEN.

Comment 7

[Ken Chang[:kenkai]]

Attachment: WIP V1.1

Comment 8

[Ken Chang[:kenkai]]

Attachment: WIP V1.1

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 769623 [details] [diff] [review]
WIP V1.1

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

Will have a discuss with Vicent Chang tomorrow.  Two things in concern:

1) Is allowing MMS/SUPL connections being "active interface" really meaningful? Right now we want to skip setting default route for MMS/SUPL connections, which means general apps can not access internet basically.  Such flexibility seems a accidental result of multiple APN support.
2) Host routing management is only effective in NetworkManager for RIL network interfaces.  There is no such thing for WiFi.  How about moving host routing management into RILNetworkInterface as well?

Comment 10

[Vincent Chang[:vchang][changyihsin]]

There are two cases, 
1. If one of MMS/SUPL shares the same network interface name with 3G data connection, for instance both of MMS and 3G data have interface name "rmnet0", we should set the connection to "active interface". However, if 
2. MMS/SUPL and 3G data connections have different network interface name, certainly, we should not set "active interface" when MMS/SUPL is connected. 

Not quite sure if case 1 is possible. 

Wifi also needs to support httpproxy in Bug 870704

Comment 11

[Ken Chang[:kenkai]]

Comment on attachment 769623 [details] [diff] [review]
WIP V1.1

After discussing with Vicamo, we don't have appropriate method to handle this problem. So we can review the attachment 769623 [details] [diff] [review] for this bug now. Request review again.

Comment 12

[Beatriz Rodríguez [:brg]]

(In reply to Ken Chang from comment #11)
> Comment on attachment 769623 [details] [diff] [review]
> WIP V1.1
> 
> After discussing with Vicamo, we don't have appropriate method to handle
> this problem. So we can review the attachment 769623 [details] [diff] [review]
> [review] for this bug now. Request review again.

Can you provide more detail information about how we can fix this issue? This is very important for end customers.

(In reply to Vincent Chang[:vchang] from comment #10)
> There are two cases, 
> 1. If one of MMS/SUPL shares the same network interface name with 3G data
> connection, for instance both of MMS and 3G data have interface name
> "rmnet0", we should set the connection to "active interface". However, if 
> 2. MMS/SUPL and 3G data connections have different network interface name,
> certainly, we should not set "active interface" when MMS/SUPL is connected. 
> 
> Not quite sure if case 1 is possible. 
> 
> Wifi also needs to support httpproxy in Bug 870704

Case 1 is the case of Spain, where MMS/SUPL/3G had the same apn name/user/password

Comment 13

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 769623 [details] [diff] [review]
WIP V1.1

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

::: dom/system/gonk/NetworkManager.js
@@ +226,5 @@
>              // to set default route only on preferred network
>              this.removeDefaultRoute(network.name);
>              this.setAndConfigureActive();
> +            // Add extra host route. For example, mms proxy or mmsc.
> +            this.setExtraHostRoute(subject);

Please move these two lines upward before |// Remove pre-created ...|, and pass |network| to |setExtraHostRoute()| instead.

@@ +245,5 @@
>                  network.type == Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_SUPL) {
>                this.removeHostRoute(network);
>              }
> +            // Remove extra host route. For example, mms proxy or mmsc.
> +            this.removeExtraHostRoute(subject);

Please pass |network| to |setExtraHostRoute()| instead.

@@ +262,5 @@
>          }
>          break;
>        case TOPIC_INTERFACE_REGISTERED:
> +        // Add extra host route. For example, mms proxy or mmsc.
> +        this.setExtraHostRoute(subject);

ditto

@@ +267,4 @@
>          break;
>        case TOPIC_INTERFACE_UNREGISTERED:
> +        // Remove extra host route. For example, mms proxy or mmsc.
> +        this.removeExtraHostRoute(subject);

ditto

@@ +474,5 @@
>              this._overriddenActive.name);
>        // The override was just set, so reconfigure the network.
>        if (this.active != this._overriddenActive) {
>          this.active = this._overriddenActive;
> +        // Don't set default route and DNS on secondary APN

We shall forbid setting overridden active to MMS/SUPL network as well.  Even though that doesn't stop a MMS/SUPL network from becoming an active one, we can still skip changes here.

@@ -487,5 @@
>          continue;
>        }
> -      if (network.type == Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE) {
> -        defaultDataNetwork = network;
> -      }

Please keep it.

@@ +527,1 @@
>        }

The original code logic here is to prevent a MMS/SUPL network be selected while there is a MOBILE data network that is suitable for global use.  That is, it's only effective when MMS/SUPL doesn't share the same APN with default data connection.  Instead, you should have:

  // Give higher ....
  ....
  if (defaultDataNetwork &&
    ....
  }
+ // Don't set default route and DNS on secondary APN
+ if (this.active.type ....
+   ...
+ }

Comment 14

[Ken Chang[:kenkai]]

Attachment: WIP V1.2

Have done what you suggested.

Comment 15

[Ken Chang[:kenkai]]

Attachment: WIP V1.3

Comment 16

[Ken Chang[:kenkai]]

Attachment: WIP V1.3

Comment 17

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 775576 [details] [diff] [review]
WIP V1.3

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

Thank you.  r=me with the last thing addressed :)

::: dom/system/gonk/NetworkManager.js
@@ +511,5 @@
> +        this.active.type == Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_SUPL) {
> +        this.setDNS(this.active);
> +      } else {
> +        this.setDefaultRouteAndDNS(oldActive);
> +      }

My fault, should have commented on this.  Please also remove MMS/SUPL from |set preferredNetworkType()| and leave the code here what it originally was.

Comment 18

[Ken Chang[:kenkai]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #17)
Thanks for your review. I will do what you suggested.

Comment 19

[Ken Chang[:kenkai]]

Attachment: Bug-886765-fix

Comment 20

[Ken Chang[:kenkai]]

Attachment: [b2g18] bug-886765-fix

Comment 21

[Ken Chang[:kenkai]]

Attachment: [b2g18] bug-886765-fix, r=vicamo

Comment 22

[Ken Chang[:kenkai]]

Attachment: bug-886765-fix, r=vicamo

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/projects/birch/rev/e7938a3a8acc

Comment 24

[Ryan VanderMeulen [:RyanVM]]

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

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/21265aae3e60

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/21265aae3e60

Comment 27

[isabelrios]

Verified unagi v1-train build:
Gecko-e537445
Gaia-bdc8fb7

The default route does not appear in the routing table.
rmnet0    545F6C0A    00000000    0001    0    0    0    FCFFFFFF0    0    0                                                                             
rmnet0    05FF8A0A    555F6C0A    0007    0    0    0    FFFFFFFF0    0    0                                                                             
 rmnet0    85FF8A0A    555F6C0A    0007    0    0    0    FFFFFFFF0    0    0                                                                             
rmnet0    FA3D3A50    555F6C0A    0007    0    0    0    FFFFFFFF0    0    0                                                                             
 rmnet0    FE3D3A50    555F6C0A    0007    0    0    0    FFFFFFFF0    0    0