[PATCH] [ModemManager] Some issues with SIM PIN handling

[Eric Shienbrood <ers google com>]

TL;DR: PIN retry count needs to be fetched after every PIN operation.

There are some deficiencies in the current handling of PIN-related operations in modem-manager, such that it mis-reports the true state of the SIM:

• The EnablePin and ChangePin operations on an unlocked SIM will result in the PIN retry count being decremented when they are supplied with an incorrect PIN. But modem-manager doesn't fetch the retry count following those operations, so the UnlockRetries property remains unchanged.
• After trying EnablePin or ChangePin three times with an incorrect PIN, the SIM becomes blocked, i.e., it is locked until the PUK is entered. Again, modem-manager doesn't recognize this, so the UnlockRequired property doesn't change - its value continues to be the empty string.
• As implied by the above two points, modem-manager assumes that the PIN retry count is relevant only when the SIM is locked. Once it is successfully unlocked, modem-manager forces UnlockRetries to zero. This is incorrect, because even when the SIM is unlocked, you only have three chances to enter the right PIN for the other operations.

So after an EnablePin or ChangePin operation, the modem needs to be queried for the new retry count, and a check needs to be done to see whether a SIM_PUK error was returned. Finally, UnlockRetries should always reflect what the modem reports for the retry count, even when the SIM is unlocked. The attached patch does these things.

There's one other enhancement that I think is needed in this area. Right now, there's no way based on the UnlockRequired and UnlockRetries properties to tell the difference between a SIM for which PIN locking is disabled, and a SIM that has locking enabled, but is currently unlocked. In both cases, UnlockedRequired is the empty string, and UnlockRetries is zero. This presents a problem for a UI that wants to put up a dialog to allow the user to enable or disable PIN locking, and wants to show a checkbox that reflects the current enabled/disabled state of locking.

I suggest either introducing a new property to explicitly describe the enable/disable state, or reserving the {UnlockRequired = "", UnlockRetries=0} combination for the case in which locking is disabled. The first of these seems more robust to me, but perhaps others have suggestions for alternatives.

Eric

From 68de3f5fba0357061cbbbe6f4f5209f9444acba9 Mon Sep 17 00:00:00 2001
From: Eric Shienbrood <ers google com>
Date: Tue, 19 Jul 2011 13:29:31 -0400
Subject: [PATCH] Correctly track the number of SIM PIN retries left.

There are other operations besides SendPin and SendPuk that
can result in the retry count being decremented. An EnablePin
or ChangePin in which an incorrect PIN is supplied will also
decrement the retry count, and will put the SIM into blocked
mode (requiring a PUK) if the retry count drops to zero. To
correctly track this, the retry count needs to be fetched after
each of these operations, and also initially when AT+CPIN? returns
READY.
---
 src/mm-generic-gsm.c |   33 ++++++++++++++++++++++++++++++++-
 1 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c
index c5c5969..f49338f 100644
--- a/src/mm-generic-gsm.c
+++ b/src/mm-generic-gsm.c
@@ -300,7 +300,10 @@ pin_check_done (MMAtSerialPort *port,
         if (g_str_has_prefix (str, "READY")) {
             mm_modem_base_set_unlock_required (MM_MODEM_BASE (info->modem), NULL);
             if (MM_MODEM_GSM_CARD_GET_INTERFACE (info->modem)->get_unlock_retries)
-                mm_modem_base_set_unlock_retries (MM_MODEM_BASE (info->modem), 0);
+                mm_modem_gsm_card_get_unlock_retries (MM_MODEM_GSM_CARD (info->modem),
+                                                      "sim-pin",
+                                                      get_unlock_retries_cb,
+                                                      NULL);
             else
                 mm_modem_base_set_unlock_retries (MM_MODEM_BASE (info->modem),
                                                   MM_MODEM_GSM_CARD_UNLOCK_RETRIES_NOT_SUPPORTED);
@@ -2196,6 +2199,32 @@ pin_puk_recheck_done (MMModem *modem, GError *error, gpointer user_data)
     mm_callback_info_schedule (info);
 }
 
+/* Following an operation other than unlock that requires
+ * a pin, refetch the retry count, which may have changed
+ * if an incorrect PIN was supplied. Check also for a SIM_PUK
+ * error, which occurs if PIN retries has reached zero. */
+static void
+update_pin_puk_status(MMCallbackInfo *info, GError *error)
+{
+    const char *pin_type = "sim-pin";
+
+    if (error) {
+        if (error->domain != MM_MOBILE_ERROR)
+            return;
+        if (error->code == MM_MOBILE_ERROR_SIM_PUK) {
+            mm_modem_base_set_unlock_required (MM_MODEM_BASE (info->modem),
+                                               "sim-puk");
+            pin_type = "sim-puk";
+        } else if (error->code != MM_MOBILE_ERROR_WRONG_PASSWORD) {
+            return;
+        }
+    }
+    mm_modem_gsm_card_get_unlock_retries (MM_MODEM_GSM_CARD (info->modem),
+                                          pin_type,
+                                          get_unlock_retries_cb,
+                                          NULL);
+}
+
 static void
 send_puk_done (MMAtSerialPort *port,
                GString *response,
@@ -2346,6 +2375,7 @@ enable_pin_done (MMAtSerialPort *port,
     if (mm_callback_info_check_modem_removed (info))
         return;
 
+    update_pin_puk_status(info, error);
     if (error)
         info->error = g_error_copy (error);
     mm_callback_info_schedule (info);
@@ -2381,6 +2411,7 @@ change_pin_done (MMAtSerialPort *port,
     if (mm_callback_info_check_modem_removed (info))
         return;
 
+    update_pin_puk_status(info, error);
     if (error)
         info->error = g_error_copy (error);
     mm_callback_info_schedule (info);
-- 
1.7.3.1