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

[Eric Shienbrood <ers google com>]

On Thu, Jul 21, 2011 at 12:53 AM, Dan Williams <dcbw redhat com> wrote:

On Tue, 2011-07-19 at 13:34 -0400, Eric Shienbrood wrote:
> 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.

So we'll need to also update the API spec to indicate that when
'UnlockRequired' is empty, that UnlockRetries indicates the number of
SIM-PIN tries, if known?  The spec won't match reality if this patch is
applied without such an update.

 

You're right, I'll add that to the patch. 

 

Or, really, we could also add another property (a{sv} mapping lock name
to # retries like {SIM-PIN => 3, SIM-PUK => 10}) that lists # tries for
all known types.  I know of at least 3 or 4 different modems that can
provide this information so it makes sense to extend this.

> 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.

So perhaps a new SimLock property that's something like sa{sbu} where
the first 's' is the current "UnlockRequired" value, and the dict is a
mapping of { PIN name => (enable/disable, # tries) }.  Most of the
"facilities" given by AT+CLCK have analogs for PIN names used with AT
+CPIN so we could be OK here.  Thoughts?

So for the value of SimLock, you mean (sa{s(bu)}) ? What's the benefit of concatenating the current UnlockRequired value with the dictionary? They seem to be separate items to me, and I think I would keep them as separate properties, i.e, keep the current UnlockRequired, and add a SimLockInfo dictionary.

However, this may be overkill. The EnablePin and ChangePin methods in ModemManager are hardwired to operate only on sim-pin, so there's not much benefit in maintaining the enable/disable state for every possible facility. And doing so would be expensive, since we'd have to issue 15 or more AT+CLCK commands to query the status of every possible lock.

Furthermore, AFAICT there's no standard way to query the number of retries remaining. The Icera Y-3300, for example, uses a proprietary command, but it only reports # of retries for sim-pin, sim-pin2, sim-puk, and sim-puk2. Ericsson returns info for those four, plus ph-net-pin, ph-netsub-pin, ph-sp-pin, and ph-corp-pin (but apparently *not* the puk versions of those), using their proprietary command.

So how about if we add the dictionary, but only populate it with entries for sim-pin, sim-pin2, sim-puk, and sim-puk2? Or maybe we ask the plugin which locks it can return info for, and just populate the dictionary with those?

Eric