Re: nm_setting_gsm_get_pin() retuns null
- From: Anders Feder <anders feder dk>
- To: Dan Williams <dcbw redhat com>
- Cc: networkmanager-list gnome org
- Subject: Re: nm_setting_gsm_get_pin() retuns null
- Date: Wed, 12 Oct 2011 05:24:20 +0200
Yes, that did the trick.
I've added the keyring functionality and now have the attached diff.
I've tested it and it works (yay).
How can I get it included in the main source tree?
Thanks,
Anders Feder
On 10-10-2011 23:30, Dan Williams wrote:
On Sun, 2011-10-09 at 17:28 +0200, Anders Feder wrote:
I now have the attached diff. Any chance you can help me understand why
it segfaults when I try running it? Thanks,
Missing trailing G_TYPE_INVALID in gsm_device_added() on the GetAll call
will make dbus-glib overrun the end of the argument list. That's a
marker for the varargs list that tells dbus-glib it can stop processing
arguments. Not sure if that's it but should be fixed.
Any idea where it's crashing? can you get a backtrace when running the
applet in gdb?
Dan
Anders Feder
On 05-10-2011 23:35, Dan Williams wrote:
On Wed, 2011-10-05 at 23:21 +0200, Anders Feder wrote:
Alternatively, can I simply request "SimIdentifier" from unlock_reply()
instead of gsm_device_added()? That way sim_id_reply() won't be invoked
unless unlock_reply() has already completed.
You could chain them together that way, yes, if you moved the stuff from
gsm_device_added() up into unlock_reply() where you get a successful
reply for DeviceIdentifier.
Dan
Anders Feder
On 05-10-2011 22:45, Dan Williams wrote:
On Wed, 2011-10-05 at 22:12 +0200, Anders Feder wrote:
Thanks for that thorough explanation. However, I wonder if there isn't a
race condition in that implementation: if we request all three
properties in gsm_device_added(), can we be certain that we have all of
them once we are in sim_id_reply()? Isn't there a risk that
sim_id_reply() might be called back before unlock_reply()?
Yes, a small risk. This can be alleviated by doing some logic in both
functions and tracking in the 'info' struct whether we've gotten replies
for both the initial modem properties and the initial card properties,
and then only doing the unlock dialog when both of those are true AND
the other stuff I wrote was true, and doing that check from both places.
It just means more variables.
Dan
Anders Feder
On 03-10-2011 23:18, Dan Williams wrote:
On Mon, 2011-10-03 at 14:20 +0200, Anders Feder wrote:
Where would you propose the new code be added?
In the first iteration, I imagine it would work something like this:
If (UnlockRequired)
Get SimIdentifier
If (SimIdentifier is found)
Get PIN for SimIdentifier from keyring
If (PIN is found for SimIdentifier)
Try unlock using saved PIN
While (unlock failed)
Prompt user for new PIN
Try unlock using new PIN
If (unlock succeeded)
Save new PIN for SimIdentifier to keyring
If this works, a similar procedure could be applied for
DeviceIdentifier, if SimIdentifier is not found.
Is this the best approach?
Yeah, that's basically it. So UnlockRequired and DeviceIdentifier are
both properties of the org.freedesktop.ModemManager.Modem interface, and
thus you can retrieve them both in the same D-Bus properties call using
GetAll(). For that, in src/applet-device-gsm.c's gsm_device_added()
function, where it calls the Get() method with UnlockRequired, what you
want to do is call "GetAll" instead and kill the "UnlockRequired"
argument. Then in the unlock_reply() for the dbus_g_proxy_end_call()
you'll do something like:
GHashTable *props = NULL;
if (dbus_g_proxy_end_call (proxy, call,&error,
DBUS_TYPE_G_MAP_OF_VARIANT,&props,
G_TYPE_INVALID)) {
GHashTableIter iter;
const char *prop_name;
GValue *value;
g_hash_table_iter_init (&iter, props);
while (g_hash_table_iter_next (&iter, (gpointer)&prop_name, (gpointer)&value)) {
if ((strcmp (prop_name, "UnlockRequired") == 0)&& G_VALUE_HOLDS_STRING (value)) {
g_free (info->unlock_required);
info->unlock_required = parse_unlock_required (value);
}
if ((strcmp (prop_name, "DeviceIdentifier") == 0)&& G_VALUE_HOLDS_STRING (value)) {
g_free (info->devid);
info->devid = g_value_dup_string (value);
}
}
}
That takes care of the UnlockRequired and DeviceIdentifier properties.
Now you need to get the SimIdentifier property, which is a GSM-specific
property (unlike UnlockRequired and DeviceIdentifier which are provided
for CDMA devices too). For that you'll want to do something like this
in gsm_device_added():
dbus_g_proxy_begin_call (info->props_proxy, "Get",
sim_id_reply, info, NULL,
G_TYPE_STRING, MM_DBUS_INTERFACE_MODEM_GSM_CARD,
G_TYPE_STRING, "SimIdentifier",
G_TYPE_INVALID);
and then basically copy& paste the current unlock_reply() function as
sim_id_reply(), fix up the property stuff there (obviously you're
handling the SimIdentifier property instead of UnlockRequired) and then
at the end of that function, you want something like this:
if (info->unlock_required&& (info->simid || info->devid))
unlock_dialog_new (info->device, info);
which will actually do the unlock. You dont' want to call
unlock_dialog_new() in unlock_reply() because you don't have the
SimIdentifier yet.
Then unlock_dialog_new() is where you'd query the keyring for existing
PINs using SimIdentifier as one of the attributes the PIN would be
stored with (using gnome_keyring_find_itemsv_sync()) and if there wasn't
any result try again with DeviceIdentifier. Check out utils/utils.c and
src/applet-agent.c for more examples of how to write and read items from
the keyring.
Dan
Anders Feder
On 03-10-2011 06:00, Dan Williams wrote:
On Sat, 2011-10-01 at 02:41 +0200, Anders Feder wrote:
Thanks. I've left you a question in the bug report. I may be
interested in taking a stab at this if it isn't overwhelmingly
difficult.
Responded. It shouldn't be too difficult; there's already code there to
track the modem object from ModemManager and do the right thing. So in
addition to the UnlockRequired property, you'd also want to grab and
watch the SimIdentifier and DeviceIdentifier properties, and use those
to look in the keyring for the right PIN.
Dan
Anders Feder
On 30-09-2011 07:31, Dan Williams wrote:
On Wed, 2011-09-21 at 08:34 +0200, Anders Feder wrote:
Hi,
I'm trying to write a fix for this bug. I've been experimenting with
testing whether a given connection is configured to autoconnect, using
this code (inside applet-device-gsm.c):
NMSettingConnection *setting =
nm_connection_get_setting_connection (connection);
NMSettingGsm *setting_gsm = nm_connection_get_setting_gsm
(connection);
if ((autoconnects = autoconnects ||
(nm_setting_connection_get_autoconnect (setting)&&
nm_setting_gsm_get_pin (setting_gsm))))
break;
However, nm_setting_gsm_get_pin (setting_gsm) seems to always return
null, even when a PIN is set for the connection. Can someone pelase
tell me why this might be? Under what circumstances may this function
return null even when a PIN is set?
PIN codes in the connection data are deprecated because PINs are
specific to the SIM card, not to the connection. If you lose your SIM
and get another from the same provider, the PIN will be different, and
the PIN in the connection data will be wrong. I've written up some
notes on what I think should be done in the GNOME bug report for this
issue:
https://bugzilla.gnome.org/show_bug.cgi?id=618532
Happy to help if there are more questions.
Dan
diff --git a/src/applet-device-gsm.c b/src/applet-device-gsm.c
index d2740ef..8765546 100644
--- a/src/applet-device-gsm.c
+++ b/src/applet-device-gsm.c
@@ -73,6 +73,8 @@ typedef struct {
gboolean quality_valid;
guint32 quality;
char *unlock_required;
+ char *devid;
+ char *simid;
gboolean modem_enabled;
MMModemGsmAccessTech act;
@@ -760,6 +762,81 @@ gsm_get_secrets (SecretsRequest *req, GError **error)
/********************************************************************/
+static char *
+get_pin_from_keyring (GsmDeviceInfo *info, int *item_id)
+{
+ GnomeKeyringResult ret;
+ GList * found_list = NULL;
+ GnomeKeyringFound * found;
+ char * pin;
+
+ g_return_val_if_fail (info != NULL, NULL);
+
+ ret = gnome_keyring_find_itemsv_sync (GNOME_KEYRING_ITEM_GENERIC_SECRET,
+ &found_list,
+ "simid",
+ GNOME_KEYRING_ATTRIBUTE_TYPE_STRING,
+ info->simid,
+ NULL);
+ if (ret != GNOME_KEYRING_RESULT_OK) {
+ ret = gnome_keyring_find_itemsv_sync (GNOME_KEYRING_ITEM_GENERIC_SECRET,
+ &found_list,
+ "devid",
+ GNOME_KEYRING_ATTRIBUTE_TYPE_STRING,
+ info->devid,
+ NULL);
+ if (ret != GNOME_KEYRING_RESULT_OK)
+ return NULL;
+ }
+
+ found = (GnomeKeyringFound *) found_list->data;
+ pin = g_strdup (found->secret);
+ if (item_id)
+ *item_id = found->item_id;
+ gnome_keyring_found_list_free (found_list);
+
+ return pin;
+}
+
+static void
+set_pin_in_keyring (const char *devid,
+ const char *simid,
+ const char *pin)
+{
+ GnomeKeyringAttributeList * attributes;
+ GnomeKeyringAttribute attr;
+ GnomeKeyringResult ret;
+ const char * name;
+ guint32 item_id;
+
+ name = g_strdup_printf (_("PIN code for SIM card '%s' on '%s'"), simid, devid);
+
+ attributes = gnome_keyring_attribute_list_new ();
+ attr.name = g_strdup ("devid");
+ attr.type = GNOME_KEYRING_ATTRIBUTE_TYPE_STRING;
+ attr.value.string = g_strdup (devid);
+ g_array_append_val (attributes, attr);
+ attr.name = g_strdup ("simid");
+ attr.type = GNOME_KEYRING_ATTRIBUTE_TYPE_STRING;
+ attr.value.string = g_strdup (simid);
+ g_array_append_val (attributes, attr);
+
+ ret = gnome_keyring_item_create_sync (NULL,
+ GNOME_KEYRING_ITEM_GENERIC_SECRET,
+ name,
+ attributes,
+ pin,
+ TRUE,
+ &item_id);
+
+ if (ret != GNOME_KEYRING_RESULT_OK)
+ {
+ /* FIXME: Warn that saving PIN code failed */
+ }
+
+ gnome_keyring_attribute_list_free (attributes);
+}
+
static void
unlock_dialog_destroy (GsmDeviceInfo *info)
{
@@ -775,7 +852,12 @@ unlock_pin_reply (DBusGProxy *proxy, DBusGProxyCall *call, gpointer user_data)
const char *dbus_error, *msg = NULL;
if (dbus_g_proxy_end_call (proxy, call, &error, G_TYPE_INVALID)) {
- unlock_dialog_destroy (info);
+ if (info->dialog) {
+ const char *code1;
+ code1 = applet_mobile_pin_dialog_get_entry1 (info->dialog);
+ set_pin_in_keyring (info->devid, info->simid, code1);
+ unlock_dialog_destroy (info);
+ }
return;
}
@@ -785,7 +867,8 @@ unlock_pin_reply (DBusGProxy *proxy, DBusGProxyCall *call, gpointer user_data)
else
msg = error ? error->message : NULL;
- applet_mobile_pin_dialog_stop_spinner (info->dialog, msg);
+ if (info->dialog)
+ applet_mobile_pin_dialog_stop_spinner (info->dialog, msg);
g_warning ("%s: error unlocking with PIN: %s", __func__, error->message);
g_clear_error (&error);
}
@@ -946,6 +1029,27 @@ unlock_dialog_new (NMDevice *device, GsmDeviceInfo *info)
applet_mobile_pin_dialog_present (info->dialog, FALSE);
}
+static void
+try_unlock (NMDevice *device, GsmDeviceInfo *info)
+{
+ char * pin;
+
+ if (!strcmp (info->unlock_required, "sim-pin")) {
+ pin = get_pin_from_keyring(info, NULL);
+ if (pin != NULL)
+ /* Send the PIN code to ModemManager */
+ if (dbus_g_proxy_begin_call_with_timeout (info->card_proxy, "SendPin",
+ unlock_pin_reply, info, NULL,
+ 15000, /* 15 seconds */
+ G_TYPE_STRING, pin,
+ G_TYPE_INVALID))
+ return;
+ }
+
+ /* Applying PIN code from keyring failed, so prompt user for one instead */
+ unlock_dialog_new (info->device, info);
+}
+
/********************************************************************/
static void
@@ -1193,7 +1297,7 @@ parse_unlock_required (GValue *value)
}
static void
-unlock_reply (DBusGProxy *proxy, DBusGProxyCall *call, gpointer user_data)
+simid_reply (DBusGProxy *proxy, DBusGProxyCall *call, gpointer user_data)
{
GsmDeviceInfo *info = user_data;
GError *error = NULL;
@@ -1203,12 +1307,12 @@ unlock_reply (DBusGProxy *proxy, DBusGProxyCall *call, gpointer user_data)
G_TYPE_VALUE, &value,
G_TYPE_INVALID)) {
if (G_VALUE_HOLDS_STRING (&value)) {
- g_free (info->unlock_required);
- info->unlock_required = parse_unlock_required (&value);
+ g_free (info->simid);
+ info->simid = g_value_dup_string (&value);
- /* Show the unlock dialog if an unlock is now required */
- if (info->unlock_required)
- unlock_dialog_new (info->device, info);
+ /* Procure unlock code and apply it if an unlock is now required. */
+ if (info->unlock_required && (info->simid || info->devid))
+ try_unlock (info->device, info);
}
g_value_unset (&value);
}
@@ -1217,6 +1321,50 @@ unlock_reply (DBusGProxy *proxy, DBusGProxyCall *call, gpointer user_data)
check_start_polling (info);
}
+#define MM_DBUS_INTERFACE_MODEM_GSM_CARD "org.freedesktop.ModemManager.Modem.Gsm.Card"
+
+static void
+unlock_reply (DBusGProxy *proxy, DBusGProxyCall *call, gpointer user_data)
+{
+ GsmDeviceInfo *info = user_data;
+ GError *error = NULL;
+ GHashTable *props = NULL;
+
+ if (dbus_g_proxy_end_call (proxy, call, &error,
+ DBUS_TYPE_G_MAP_OF_VARIANT, &props,
+ G_TYPE_INVALID)) {
+ GHashTableIter iter;
+ const char *prop_name;
+ GValue *value;
+
+ g_hash_table_iter_init (&iter, props);
+ while (g_hash_table_iter_next (&iter, (gpointer) &prop_name, (gpointer) &value)) {
+ if ((strcmp (prop_name, "UnlockRequired") == 0) && G_VALUE_HOLDS_STRING (value)) {
+ g_free (info->unlock_required);
+ info->unlock_required = parse_unlock_required (value);
+ }
+
+ if ((strcmp (prop_name, "DeviceIdentifier") == 0) && G_VALUE_HOLDS_STRING (value)) {
+ g_free (info->devid);
+ info->devid = g_value_dup_string (value);
+ }
+ }
+ g_value_unset (value);
+
+ if (info->devid) {
+ /* Get SIM card identifier */
+ dbus_g_proxy_begin_call (info->props_proxy, "Get",
+ simid_reply, info, NULL,
+ G_TYPE_STRING, MM_DBUS_INTERFACE_MODEM_GSM_CARD,
+ G_TYPE_STRING, "SimIdentifier",
+ G_TYPE_INVALID);
+ }
+ }
+
+ g_clear_error (&error);
+ check_start_polling (info);
+}
+
static void
access_tech_reply (DBusGProxy *proxy, DBusGProxyCall *call, gpointer user_data)
{
@@ -1465,10 +1613,9 @@ gsm_device_added (NMDevice *device, NMApplet *applet)
info, NULL);
/* Ask whether the device needs to be unlocked */
- dbus_g_proxy_begin_call (info->props_proxy, "Get",
+ dbus_g_proxy_begin_call (info->props_proxy, "GetAll",
unlock_reply, info, NULL,
G_TYPE_STRING, MM_DBUS_INTERFACE_MODEM,
- G_TYPE_STRING, "UnlockRequired",
G_TYPE_INVALID);
/* Ask whether the device is enabled */
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
