Merge lp:~phablet-team/network-manager/lp1435776 into lp:~phablet-team/network-manager/vivid-phone-overlay

Proposed by Tony Espy
Status: Merged
Approved by: Alfonso Sanchez-Beato
Approved revision: no longer in the source branch.
Merge reported by: Tony Espy
Merged at revision: not available
Proposed branch: lp:~phablet-team/network-manager/lp1435776
Merge into: lp:~phablet-team/network-manager/vivid-phone-overlay
Diff against target: 69 lines (+49/-0)
3 files modified
debian/changelog (+9/-0)
debian/patches/lp1435776_rm_ofono_secret_settings.patch (+39/-0)
debian/patches/series (+1/-0)
To merge this branch: bzr merge lp:~phablet-team/network-manager/lp1435776
Reviewer Review Type Date Requested Status
Alfonso Sanchez-Beato Approve
Review via email: mp+258633@code.launchpad.net

Commit message

 * debian/patches/lp1435776_rm_ofono_secret_settings.patch: remove code
    that added USERNAME and PASSWORD to NM_SETTING_GSM object. NM doesn't
    actually need access to these settings, and they can cause issues with
    NM's secrets needed logic.

Description of the change

This change affects the ofono settings plugin and the way that it configures system connections based on ofono gprs_contexts ( aka APNs ).

The change removes the addition of USERNAME and PASSWORD NM_SETTING_GSM settings to the connection, as they're not needed by network-manager, and can cause secret-related activation errors.

To post a comment you must log in.
Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

I have tried the patch in arale vivid image #35, but the bug did not go away.

To reproduce the issue, I use a SIM that does not need user or password. I change the "gprs" file introducing a user, but without password. I check that the operator still activates the context using the activate-context script. Then I checked that current NM is not able to activate the context. Finally, using the patched NM I still see the issue. The only difference I have noted is that apparently NM is trying only once to activate the context. The trace I see are (the old NM produced similar ones, but twice):

May 11 13:08:31 ubuntu-phablet NetworkManager[1206]: <info> Auto-activating connection '/214050030479893/context1'.
May 11 13:08:31 ubuntu-phablet NetworkManager[1206]: <info> Activation (ril_0) starting connection '/214050030479893/context1'
May 11 13:08:31 ubuntu-phablet NetworkManager[1206]: <info> Activation (ril_0) Stage 1 of 5 (Device Prepare) scheduled...
May 11 13:08:31 ubuntu-phablet NetworkManager[1206]: <info> Activation (ril_0) Stage 1 of 5 (Device Prepare) started...
May 11 13:08:31 ubuntu-phablet NetworkManager[1206]: <info> (ril_0): device state change: disconnected -> prepare (reason 'none') [30 40 0]
May 11 13:08:31 ubuntu-phablet NetworkManager[1206]: <info> (ril_0): device state change: prepare -> failed (reason 'no-secrets') [40 120 7]
May 11 13:08:31 ubuntu-phablet NetworkManager[1206]: <warn> Activation (ril_0) failed for connection '/214050030479893/context1'
May 11 13:08:31 ubuntu-phablet NetworkManager[1206]: <info> Activation (ril_0) Stage 1 of 5 (Device Prepare) complete.
May 11 13:08:31 ubuntu-phablet NetworkManager[1206]: <info> (ril_0): device state change: failed -> disconnected (reason 'none') [120 30 0]
May 11 13:08:31 ubuntu-phablet NetworkManager[1206]: <info> (ril_0): deactivating device (reason 'none') [0]

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

Please ignore the previous comment, I did not copy around the right library. The fix does solve the issue, and the code seems right to me.

review: Approve
962. By Tony Espy

debian/patches/lp1435776_rm_ofono_secret_settings.patch: remove code
that added USERNAME and PASSWORD to NM_SETTING_GSM object. NM doesn't
actually need access to these settings, and they can cause issues with
NM's secrets needed logic ( LP: #1435776 ).

Revision history for this message
Tony Espy (awe) wrote :

Tested krillin, rc-proposed #171, SIM1: AT&T SIM2: empty

 - phone boots, mobile data (edge-only) connected

 - tested flight-mode ( 5 cycles via the indicator )

 - tested a context/APN w/username, but no passwrd ( this fails w/network-manager 4ubuntu15.1 )
   - manually edited /var/lib/ofono/<IMSI>/gprs after stopping NM & ofono
   - Note: NM won't even attempt to activate this without this fix

Everything worked as expected.

Revision history for this message
Tony Espy (awe) wrote :

Tested arale, tangxi-vivid-proposed #31, SIM: AT&T

 - phone boots, mobile data (hspa) connected

 - flight-mode still broken due to lp: #1445080

 - bad APN ( username w/no pw ) - OK

Revision history for this message
Tony Espy (awe) wrote :

I should've noted, my testing was performed with network-manager-0.9.10.0-4ubuntu15.1.1 from silo-028.

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

Tested on

current build number: 13
device name: arale
channel: ubuntu-touch/rc-proposed/meizu

and checked that both #1435776 are fixed #1450790.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2015-04-28 19:49:51 +0000
3+++ debian/changelog 2015-05-16 03:09:09 +0000
4@@ -1,3 +1,12 @@
5+network-manager (0.9.10.0-4ubuntu15.1.1) vivid; urgency=medium
6+
7+ * debian/patches/lp1435776_rm_ofono_secret_settings.patch: remove code
8+ that added USERNAME and PASSWORD to NM_SETTING_GSM object. NM doesn't
9+ actually need access to these settings, and they can cause issues with
10+ NM's secrets needed logic.
11+
12+ -- Tony Espy <espy@canonical.com> Fri, 15 May 2015 18:39:32 -0400
13+
14 network-manager (0.9.10.0-4ubuntu15.1) vivid-security; urgency=medium
15
16 * SECURITY UPDATE: directory traversal issue resulting in connection
17
18=== added file 'debian/patches/lp1435776_rm_ofono_secret_settings.patch'
19--- debian/patches/lp1435776_rm_ofono_secret_settings.patch 1970-01-01 00:00:00 +0000
20+++ debian/patches/lp1435776_rm_ofono_secret_settings.patch 2015-05-16 03:09:09 +0000
21@@ -0,0 +1,39 @@
22+Index: network-manager-0.9.10.0/src/settings/plugins/ofono/parser.c
23+===================================================================
24+--- network-manager-0.9.10.0.orig/src/settings/plugins/ofono/parser.c
25++++ network-manager-0.9.10.0/src/settings/plugins/ofono/parser.c
26+@@ -53,8 +53,6 @@ ofono_update_connection_from_context (NM
27+ char *idstr = NULL;
28+ char *uuid_base = NULL;
29+ char *uuid = NULL;
30+- const char *username = NULL;
31+- const char *password = NULL;
32+
33+ s_con = nm_connection_get_setting_connection (connection);
34+ if(!s_con) {
35+@@ -91,25 +89,6 @@ ofono_update_connection_from_context (NM
36+ */
37+ g_object_set (s_gsm, NM_SETTING_GSM_NUMBER, "*99#", NULL);
38+
39+- /* Set the APN we got from oFono config. */
40+- g_object_set (s_gsm,
41+- NM_SETTING_GSM_APN, g_hash_table_lookup (context, "Apn"),
42+- NULL);
43+-
44+- username = g_hash_table_lookup (context, "Username");
45+- if (username && g_strcmp0 (username, "") != 0) {
46+- g_object_set (s_gsm,
47+- NM_SETTING_GSM_USERNAME, username,
48+- NULL);
49+- }
50+-
51+- password = g_hash_table_lookup (context, "Password");
52+- if (password && g_strcmp0 (password, "") != 0) {
53+- g_object_set (s_gsm,
54+- NM_SETTING_GSM_PASSWORD, password,
55+- NULL);
56+- }
57+-
58+ nm_log_info (LOGD_SETTINGS, "SCPlugin-Ofono: "
59+ "update_connection_setting_from_context: name:%s, path:%s, type:%s, id:%s, uuid: %s",
60+ (char *) g_hash_table_lookup (context, "Name"),
61
62=== modified file 'debian/patches/series'
63--- debian/patches/series 2015-04-28 19:49:51 +0000
64+++ debian/patches/series 2015-05-16 03:09:09 +0000
65@@ -68,3 +68,4 @@
66 # killswitch
67 ignore_rfkill_if_urfkill_is_present.patch
68 CVE-2015-1322.patch
69+lp1435776_rm_ofono_secret_settings.patch

Subscribers

People subscribed via source and target branches

to all changes: