[review] https://github.com/cbchan/ModemManager/tree/sim-mbim-unlock-retries

[Ben Chan]

On Thu, Aug 10, 2017 at 3:34 PM, Aleksander Morgado
<aleksander at aleksander.es> wrote:
>>>> However, we may be able to improve the situation a bit when MM is
>>>> running by making mm_iface_modem_update_lock_info preserve what we
>>>> have learned from a prior MBIM_CID_PIN response.  That's what I'm
>>>> trying to address with this patch set.
>>>>
>>>
>>> Yes, I agree, keeping the information returned in a response is
>>> useful, but why not fully overwrite the list of unlock attempts
>>> instead of merging as you were trying?
>>
>> Let me make sure I understand your proposal. Are you suggesting the following?
>>
>> (1) When MMSimMbim receives a response to a MBIM_CID_PIN set
>> operation, it always overwrite IfaceModem's MMUnlockRetries as
>> MBIM_CID_PIN provides the most relevant info at that point, which I
>> think makes sense (e.g. we really don't care about PIN2 in MM, at
>> least for now. And we don't care about PIN1 when PUK1 is active)
>>
>> (2) When MMBroadbandModemMbim later reload unlock retries via a
>> MBIM_CID_PIN query, we try to preserve PIN1 in IfaceModem's
>> MMUnlockRetries if MBIM_CID_PIN query reports PIN2, for example.
>>
>
> I was actually suggesting we leave this as it is now (i.e. query pin
> lock and retries, and just use that, without merging)...
>
> From a user point of view, we have the main features we need:
>   * If locked, we know the lock enabled and the amount of unlock retries left.
>   * We know if a lock unlock operation succeeds or fails, and if it
> fails we know the amount of unlock retries left.
>   * We know if a lock enable/disable operation succeeds or fails.
>
> What we don't know is the amount of retries left if the lock
> enable/disable operation fails. We could try to hack something in MM
> to keep some of that logic, but then, if the device/system is rebooted
> between e.g. failed lock enable attempts, we're back without knowing
> anything until we let the user retry again. There's no perfect
> solution either way, not sure.

I totally agree that we don't have a perfect solution here. The
workaround I propose here is a bit of a hack. The part that I'm not
very comfortable with the current situation is that an UI can't
indicate the remaining attempts for PIN1 after a user tries to enable
the SIM lock with the wrong PIN (even though we've got the infor about
PIN1, which is overwritten by info about PIN2).

What makes the situation worse from a user perspective is how
counter-intuitive the SIM lock mechanism is. Some users may actually
think that they should enter a new PIN when enabling the SIM lock.
They may not know that a SIM may come with a default PIN. Or they may
believe that when they previously disabled the SIM lock, the PIN was
clear.  So knowing the remaining attempts for PIN1 when they try to
enable the SIM lock would be quite helpful as they may stop entering a
new PIN after the first failed attempt.

The SIM lock mechanism is dated and it seems overlooked that MBIM
provides no way to explicitly query the remaining attempts for PIN1.
Unfortunately, we will live with these issues until the SIM locking
mechanism is deprecated.

Anyway, I'd try one more time :)  I've modified the patch to address
the issues with PIN-PUK transition and tried it on a SIM (finally I
got the PUK code :p).  Here's the mmcli output.  Thanks again for
reviewing. Let me know how you think.

>
> --
> Aleksander
> https://aleksander.es
-------------- next part --------------
=====================
Try to enable PIN request with bad PIN

$ mmcli -i 0 --enable-pin --pin=1234
error: couldn't enable PIN code request in the SIM: 'GDBus.Error:org.freedesktop.libmbim.Error.Status.Failure: Failure'

$ mmcli -m 0 | grep lock
  Status   |           lock: 'none'
           | unlock retries: 'sim-pin (2), sim-pin2 (3)'
           |  enabled locks: 'fixed-dialing'

$ mmcli -i 0 --enable-pin --pin=1234
error: couldn't enable PIN code request in the SIM: 'GDBus.Error:org.freedesktop.libmbim.Error.Status.Failure: Failure'

$ mmcli -m 0 | grep lock
  Status   |           lock: 'none'
           | unlock retries: 'sim-pin (1), sim-pin2 (3)'
           |  enabled locks: 'fixed-dialing'

$ mmcli -i 0 --enable-pin --pin=1234
error: couldn't enable PIN code request in the SIM: 'GDBus.Error:org.freedesktop.libmbim.Error.Status.Failure: Failure'

$ mmcli -m 0 | grep lock
  Status   |           lock: 'sim-puk'
           | unlock retries: 'sim-puk (10)'
           |  enabled locks: 'fixed-dialing'

=====================
Insert PUK wrong...

$ mmcli -i 0 --pin=1234 --puk=12345678
error: couldn't send PUK code to the SIM: 'GDBus.Error:org.freedesktop.ModemManager1.Error.MobileEquipment.IncorrectPassword: Incorrect password'

$ mmcli -m 0 | grep lock
  Status   |           lock: 'sim-puk'
           | unlock retries: 'sim-puk (9)'
           |  enabled locks: 'fixed-dialing'

=====================
Insert PUK ok and recover PIN...

$ mmcli -i 0 --pin=1234 --puk=11111111
successfully sent PUK code to the SIM

$ mmcli -m 0 | grep lock
  Status   |           lock: 'none'
           | unlock retries: 'sim-pin (3), sim-pin2 (3)'
           |  enabled locks: 'fixed-dialing'

=====================
Change PIN request

$ mmcli -i 0 --change-pin=1111 --pin=1234
successfully changed PIN code in the SIM

$ mmcli -m 0 | grep lock
  Status   |           lock: 'none'
           | unlock retries: 'sim-pin (3), sim-pin2 (3)'
           |  enabled locks: 'fixed-dialing'

=====================
Disable PIN request

$ mmcli -i 0 --disable-pin --pin=1111
successfully disabled PIN code request in the SIM

$ mmcli -m 0 | grep lock
  Status   |           lock: 'none'
           | unlock retries: 'sim-pin (3), sim-pin2 (3)'
           |  enabled locks: 'fixed-dialing'