New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 751792 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

CrOS: Fail to change SIM PIN via settings UI

Project Member Reported by benchan@chromium.org, Aug 2 2017

Issue description

Forked from b/63803092:

What steps will reproduce the problem?
1. Open Network Settings UI and turn on Enable SIM locking. Enter the existing PIN to enable SIM locking.
2. Once PIN is enabled, click on Change PIN in UI. Enter the existing PIN followed by New PIN.

What is the expected output?
- PIN is changed successfully.

What do you see instead?
- UI throws an Incorrect PIN error and shows remaining retries as 0. It doesn't allow the user to change PIN.

How frequently does this problem reproduce? 
- Always

What is the impact to the user, and is there a workaround? If so, what is
it?
- User cannot change pin from Settings UI.

The issue seems to be on the Chrome side:

    src/extensions/browser/api/networking_private/networking_private_chromeos.cc:

    // Only set a new pin if require_pin is true.
    std::string set_new_pin = require_pin ? new_pin : "";
    NetworkHandler::Get()->network_device_handler()->RequirePin(
        device_state->path(), require_pin, current_pin,
        base::Bind(&RequirePinSuccess, device_state->path(), current_pin,
                   set_new_pin, success_callback, failure_callback),
        base::Bind(&NetworkHandlerFailureCallback, failure_callback));

The "Change Pin" action invokes
NetworkingPrivateChromeOS::SetCellularSimState() as follows

    (chrome) NetworkingPrivateChromeOS::SetCellularSimState
      -> (chrome) NetworkDeviceHandlerImpl::RequirePin
        -> (chrome) ShillDeviceClient::RequirePin
          -> (shill) Cellular::RequirePIN
           -> (shill) CelluarCapabilityUniversal::RequirePIN
            -> (ModemManager) org.freedesktop.ModemManager1.Sim.EnablePin

      // upon success
      -> (chrome) NetworkingPrivateChromeOS::{anonymous}::RequirePinSuccess
        -> (chrome) NetworkDeviceHandlerImpl::ChangePin
          -> (chrome) ShillDeviceClient::ChangePin
            -> (shill) Cellular::ChangePIN
              -> (shill) CelluarCapabiltyUniversal::ChangePIN
              -> (ModemManager) org.freedesktop.ModemManager1.Sim.ChangePin

The code seems to use EnablePin to handle the case when PIN lock hasn't been enabled, but EnablePin may fail if PIN lock is already enabled, which prevents ChangePin from being invoked.  ModemManager doesn't provide a way to query if a PIN lock is enabled (after the SIM has been unlocked), so we may want to change Chrome to first invoke ChangePin. If ChangePin fails, we could either present an error or invoke EnablePin.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/00f838c039e4507d8ead47c84c4a3d357c96249b

commit 00f838c039e4507d8ead47c84c4a3d357c96249b
Author: Ben Chan <benchan@chromium.org>
Date: Thu Aug 03 21:02:14 2017

Chrome OS: Fix SIM PIN change logic.

On Chrome OS, the settings UI supports enabling and disabling SIM
locking on a cellular modem, and changing the SIM PIN. In the current
implementation, all these actions eventually triggers
NetworkingPrivateChromeOS::SetCellularSimState, which then calls the
org.chromium.flimflam.Device.RequirePIN and
org.chromium.flimflam.Device.ChangePIN DBus API exposed by shill.

The "Change Pin" action is available after SIM locking is enabled.
RequirePIN is called once when SIM locking is enabled. After that,
ChangePIN should be used to change the PIN. However,
NetworkingPrivateChromeOS::SetCellularSimState currently handles the
"Change Pin" action by first calling RequirePIN first and then ChangePIN
upon the success completion of RequirePIN. That results in two
successive RequirePIN calls, and the second RequirePIN isn't necessary
and often fails as the modem complains about SIM locking is already
enabled. This CL changes NetworkingPrivateChromeOS::SetCellularSimState
to simply call ChangePIN for the "Change Pin" action.

BUG=b:63803092
BUG= chromium:751792 
TEST=Manually tested the following with a few cellular modems:

Under the expanded view of "Settings > Network > Mobile data":

1. Enable and disable SIM locking:
   a. Turn on "Enable SIM locking". A SIM PIN dialog is shown and asks
      for a previously set PIN.
   b. Enter a wrong PIN. The action fails.
   c. Enter the correct PIN. The action succeeds.
   d. Turn off "Enable SIM locking".
   e. Enter a wrong PIN. The action fails.
   f. Enter the correct PIN. The action succeeds.

2. Enable SIM locking, change the PIN, and disable SIM locking:
   a. Turn on "Enable SIM locking". A SIM PIN dialog is shown and asks
      for a previously set PIN.
   b. Enter the correct PIN. The action succeeds.
   c. Click "Change Pin". A SIM PIN dialog is shown and asks for the
      current PIN, the new PIN and the confirmation of the new PIN.
   d. Enter a wrong current PIN. The action fails.
   e. Enter the correct current PIN, but the confirmation of the new PIN
      not matching the new PIN. The action fails.
   f. Enter the correct current PIN, the new PIN and the confirmation of
      the new PIN that matches the new PIN. The action succeeds.
   g. Turn off "Enable SIM locking".
   h. Enter the old PIN. The action fails.
   i. Enter the new PIN. The action succeeds.

3. Enable SIM locking, reboot the system, and enter PIN to unlock the
   SIM:
   a. Turn on "Enable SIM locking". A SIM PIN dialog is shown and asks
      for a previously set PIN.
   b. Enter the correct PIN. The action succeeds.
   c. Reboot the system.
   d. A "SIM card is locked" message is shown under "Mobile data".
   e. Click "Unlock". A SIM PIN dialog is shown and asks for a PIN.
   f. Enter a wrong PIN. The action fails.
   g. Enter the correct PIN. The action succeeds.
   h. The cellular connection can be established after the SIM is unlocked.

Change-Id: Ia095c4243ffd777fcfc822a1dcf2b32003257c78
Reviewed-on: https://chromium-review.googlesource.com/595327
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Ben Chan <benchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491842}
[modify] https://crrev.com/00f838c039e4507d8ead47c84c4a3d357c96249b/extensions/browser/api/networking_private/networking_private_api.cc
[modify] https://crrev.com/00f838c039e4507d8ead47c84c4a3d357c96249b/extensions/browser/api/networking_private/networking_private_api.h
[modify] https://crrev.com/00f838c039e4507d8ead47c84c4a3d357c96249b/extensions/browser/api/networking_private/networking_private_chromeos.cc

Status: Fixed (was: Assigned)

Comment 3 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment