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

Thu Aug 10 09:28:19 UTC 2017

Hey

>>>>> It really is unfortunate that the MBIM_CID_PIN query only reports the
>>>>> information of the "currently applicable" PIN type, although the truth
>>>>> is that that's the only useful one.
>>>>
>>>> Yes, indeed. It would have been much better if MBIM_CID_PIN query
>>>> could take a PIN type as argument. I'd expect PIN1 (PUK1) is likely
>>>> the one being used and thus the most relevant / useful one in typical
>>>> scenarios.
>>>>
>>>> Here's the issue: If PIN1 is currently disabled, some modem reports
>>>> the info about another PIN, say PIN2, to a MBIM_CID_PIN query. When we
>>>> try to enable PIN1, we won't know the retries count until after
>>>> entering a PIN once. Suppose the retries count doesn't go to 0 after a
>>>> wrong attempt (i.e. not in sim-puk state), we still can't query MM for
>>>> the retries count of PIN1.
>>>>
>>>
>>> Aha, that was what I was missing, the PIN disabled->enabled transition
>>> is the problematic one, because with PIN disabled it would report you
>>> PIN2 attempts. But, if we do the disabled->enabled transition,
>>> couldn't we reload unlock attempts in that moment (after enabling
>>> SIM-PIN)? I'm assuming that the "current" PIN would change to PIN1
>>> right away and we would get reported the number of attempts of PIN1;
>>> without even trying to send a PIN once.
>>>
>>
>> Don't we need to enter the correct PIN to re-enable PIN1? so the
>> disabled->enabled transition is essentially a MBIM_CID_PIN set
>> operation. Regardless of whether the operation succeeds or fails, the
>> response should report the latest remaining attempts for PIN1, which
>> we want to propagate that to UnlockRetries. That's why patch #4 always
>> calls mm_iface_modem_update_unlock_retries() if we can parse the
>> response, regardless of whether the operation succeeds or not.
>>
>
> That makes total sense, indeed. Your logic makes the best compromise
> given the APIs we have in MBIM.
>

Hum... looks like the patched logic is actually way more confusing
than the original one... See attached logs (the -patched file is after
your changes).

Several issues in your patches:
  * When we get SIM-PUK locked after 3 SIM-PIN attempts, we're leaving
the old "SIM-PIN 1 attempt". We could try to detect that and, as soon
as we're asked SIM-PUK, set an explicit "SIM-PIN 0 attempts"
ourselves.
  * If we then insert SIM-PUK correctly after some bad attempts, we're
leaving the old "SIM-PUK X attempts". See the "Insert PUK ok and
recover PIN..." step in the -patched file; it's a bit of a mess, as we
have there the old "SIM-PIN 1 attempt" and the old "SIM-PUK 9
attempts", and actually we don't have *any* lock at that moment, they
both should be at their max number of attempts.

So, we have issues when going from SIM-PIN required -> SIM-PUK
required and also when successfully entering either SIM-PIN or SIM-PUK
after some previous failed attempts.

Compared this, to the original log, where the only issue is that if
the modem has SIM-PIN disabled and we want to enable PIN we don't
explicitly know how many attempts we have (although we always have the
MAX attempts in that case anyway)...

Don't know what to say, I think the current logic without your patches
make more sense right now...

-- 
Aleksander
https://aleksander.es
-------------- next part --------------

=====================
Insert wrong PIN 3 times

$ mmcli -m 0 | grep lock
  Status   |           lock: 'sim-pin'
           | unlock retries: 'sim-pin (3)'
           |          state: 'locked'

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

$ mmcli -m 0 | grep lock
  Status   |           lock: 'sim-pin'
           | unlock retries: 'sim-pin (2)'
           |          state: 'locked'

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

$ mmcli -m 0 | grep lock
  Status   |           lock: 'sim-pin'
           | unlock retries: 'sim-pin (1)'
           |          state: 'locked'

$ mmcli -i 0 --pin=1234
error: couldn't send PIN code to the SIM: 'GDBus.Error:org.freedesktop.ModemManager1.Error.MobileEquipment.SimPuk: SIM PUK required'

$ mmcli -m 0 | grep lock
  Status   |           lock: 'sim-puk'
           | unlock retries: 'sim-puk (10)'
           |          state: 'locked'

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

$ mmcli -i 0 --pin=1234 --puk=12345678
error: couldn't send PUK code to the SIM: 'GDBus.Error:org.freedesktop.libmbim.Error.Status.Failure: Failure'

$ mmcli -m 0 | grep lock
  Status   |           lock: 'sim-puk'
           | unlock retries: 'sim-puk (9)'
           |          state: 'locked'

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

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

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

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

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

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

=====================
Try to enable PIN request with bad PIN

$ mmcli -i 0 --enable-pin --pin=1111
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-pin2 (3)'
           |  enabled locks: 'fixed-dialing'

$ mmcli -i 0 --enable-pin --pin=1111
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-pin2 (3)'
           |  enabled locks: 'fixed-dialing'

$ mmcli -i 0 --enable-pin --pin=1111
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 ok and recover PIN...

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

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

-------------- next part --------------

=====================
Insert wrong PIN 3 times

$ mmcli -m 0 | grep lock
  Status   |           lock: 'sim-pin'
           | unlock retries: 'sim-pin (3)'
           |          state: 'locked'

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

$ mmcli -m 0 | grep lock
  Status   |           lock: 'sim-pin'
           | unlock retries: 'sim-pin (2)'
           |          state: 'locked'

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

$ mmcli -m 0 | grep lock
  Status   |           lock: 'sim-pin'
           | unlock retries: 'sim-pin (1)'
           |          state: 'locked'

$ mmcli -i 0 --pin=1111
error: couldn't send PIN code to the SIM: 'GDBus.Error:org.freedesktop.ModemManager1.Error.MobileEquipment.SimPuk: SIM PUK required'

$ mmcli -m 0 | grep lock
  Status   |           lock: 'sim-puk'
           | unlock retries: 'sim-pin (1), sim-puk (10)'
           |          state: 'locked'

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

$ mmcli -i 0 --pin=1111 --puk=12345678
error: couldn't send PUK code to the SIM: 'GDBus.Error:org.freedesktop.libmbim.Error.Status.Failure: Failure'

$ mmcli -m 0 | grep lock
  Status   |           lock: 'sim-puk'
           | unlock retries: 'sim-pin (1), sim-puk (9)'
           |          state: 'locked'

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

$ mmcli -i 0 --pin=1111 --puk=55555555
successfully sent PUK code to the SIM

$ mmcli -m 0 | grep lock
  Status   |           lock: 'none'
           | unlock retries: 'sim-pin (1), sim-pin2 (3), sim-puk (9)'
           |  enabled locks: 'sim, 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 (1), sim-pin2 (3), sim-puk (9)'
           |  enabled locks: 'fixed-dialing'

=====================
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), sim-puk (9)'
           |  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), sim-puk (9)'
           |  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-pin (1), sim-pin2 (3), sim-puk (10)'
           |  enabled locks: 'fixed-dialing'

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

$ mmcli -i 0 --pin=1111 --puk=55555555
successfully sent PUK code to the SIM

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