[PATCH] sim: check if lock is locked after code input attempt

[PATCH] sim: check if lock is locked after code input attempt

[Jussi Kangas]

[-- Attachment #1: Type: text/plain, Size: 3291 bytes --]

Hi again,

This is third attempt to fix the problem where PUK required information
is not shown correctly after user tries to change pin code too many
times with wrong passwords. Basically solution is pretty much as in
original made by Marit Henriksen except it does not do the sim interface
initialization anymore and it goes to PRE_SIM state if PUK is required
except in case of PIN2. Fix is also extended to pin locking and pin
unlocking.  

Br,
-Jussi

---
 src/sim.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index d627647..00f0463 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -622,6 +622,45 @@ static void sim_locked_cb(struct ofono_sim *sim,
gboolean locked)
 	sim_pin_retries_check(sim);
 }
 
+static void fail_reason_check_cb(const struct ofono_error *error,
+				enum ofono_sim_password_type pin_type,
+				void *data)
+{
+	struct ofono_sim *sim = data;
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(sim->atom);
+	struct ofono_modem *modem = __ofono_atom_get_modem(sim->atom);
+	const char *pin_name;
+
+	if (sim->pin_type != pin_type) {
+		sim->pin_type = pin_type;
+		pin_name = sim_passwd_name(pin_type);
+
+		if (pin_type != OFONO_SIM_PASSWORD_NONE &&
+				password_is_pin(pin_type) == FALSE)
+			pin_type = puk2pin(pin_type);
+
+		if (pin_type != OFONO_SIM_PASSWORD_INVALID)
+			sim->locked_pins[pin_type] = TRUE;
+
+		ofono_dbus_signal_property_changed(conn, path,
+						OFONO_SIM_MANAGER_INTERFACE,
+						"PinRequired", DBUS_TYPE_STRING,
+						&pin_name);
+
+		if ((sim->state != OFONO_SIM_STATE_READY) ||
+		   ((sim->locked_pins[pin_type] == TRUE) && (
+		     pin_type == OFONO_SIM_PASSWORD_SIM_PIN ||
+		     pin_type == OFONO_SIM_PASSWORD_PHSIM_PIN ||
+		     pin_type == OFONO_SIM_PASSWORD_PHFSIM_PIN ||
+		     pin_type == OFONO_SIM_PASSWORD_PHNET_PIN ||
+		     pin_type == OFONO_SIM_PASSWORD_PHNETSUB_PIN ||
+		     pin_type == OFONO_SIM_PASSWORD_PHSP_PIN ||
+		     pin_type == OFONO_SIM_PASSWORD_PHCORP_PIN)))
+			ofono_modem_reset(modem);
+	}
+}
+
 static void sim_unlock_cb(const struct ofono_error *error, void *data)
 {
 	struct ofono_sim *sim = data;
@@ -630,6 +669,9 @@ static void sim_unlock_cb(const struct ofono_error
*error, void *data)
 		DBusMessage *reply = __ofono_error_failed(sim->pending);
 
 		__ofono_dbus_pending_reply(&sim->pending, reply);
+
+		sim->driver->query_passwd_state(sim, fail_reason_check_cb, sim);
+
 		sim_pin_retries_check(sim);
 
 		return;
@@ -646,6 +688,9 @@ static void sim_lock_cb(const struct ofono_error
*error, void *data)
 		DBusMessage *reply = __ofono_error_failed(sim->pending);
 
 		__ofono_dbus_pending_reply(&sim->pending, reply);
+
+		sim->driver->query_passwd_state(sim, fail_reason_check_cb, sim);
+
 		sim_pin_retries_check(sim);
 
 		return;
@@ -717,6 +762,8 @@ static void sim_change_pin_cb(const struct
ofono_error *error, void *data)
 		__ofono_dbus_pending_reply(&sim->pending,
 				__ofono_error_failed(sim->pending));
 
+		sim->driver->query_passwd_state(sim, fail_reason_check_cb, sim);
+
 		sim_pin_retries_check(sim);
 
 		return;
-- 
1.7.1

Re: [PATCH] sim: check if lock is locked after code input attempt

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2888 bytes --]

Hi Jussi,

On 01/17/2011 05:34 AM, Jussi Kangas wrote:
> Hi again,
> 
> This is third attempt to fix the problem where PUK required information
> is not shown correctly after user tries to change pin code too many
> times with wrong passwords. Basically solution is pretty much as in
> original made by Marit Henriksen except it does not do the sim interface
> initialization anymore and it goes to PRE_SIM state if PUK is required
> except in case of PIN2. Fix is also extended to pin locking and pin
> unlocking.  
> 
> Br,
> -Jussi
> 
> ---
>  src/sim.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 47 insertions(+), 0 deletions(-)
> 
> diff --git a/src/sim.c b/src/sim.c
> index d627647..00f0463 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -622,6 +622,45 @@ static void sim_locked_cb(struct ofono_sim *sim,
> gboolean locked)
>  	sim_pin_retries_check(sim);
>  }
>  
> +static void fail_reason_check_cb(const struct ofono_error *error,
> +				enum ofono_sim_password_type pin_type,
> +				void *data)
> +{
> +	struct ofono_sim *sim = data;
> +	DBusConnection *conn = ofono_dbus_get_connection();
> +	const char *path = __ofono_atom_get_path(sim->atom);
> +	struct ofono_modem *modem = __ofono_atom_get_modem(sim->atom);
> +	const char *pin_name;
> +
> +	if (sim->pin_type != pin_type) {
> +		sim->pin_type = pin_type;
> +		pin_name = sim_passwd_name(pin_type);
> +
> +		if (pin_type != OFONO_SIM_PASSWORD_NONE &&
> +				password_is_pin(pin_type) == FALSE)
> +			pin_type = puk2pin(pin_type);
> +
> +		if (pin_type != OFONO_SIM_PASSWORD_INVALID)
> +			sim->locked_pins[pin_type] = TRUE;
> +
> +		ofono_dbus_signal_property_changed(conn, path,
> +						OFONO_SIM_MANAGER_INTERFACE,
> +						"PinRequired", DBUS_TYPE_STRING,
> +						&pin_name);
> +
> +		if ((sim->state != OFONO_SIM_STATE_READY) ||
> +		   ((sim->locked_pins[pin_type] == TRUE) && (

Please don't mix tabs and spaces for indentation

> +		     pin_type == OFONO_SIM_PASSWORD_SIM_PIN ||
> +		     pin_type == OFONO_SIM_PASSWORD_PHSIM_PIN ||
> +		     pin_type == OFONO_SIM_PASSWORD_PHFSIM_PIN ||
> +		     pin_type == OFONO_SIM_PASSWORD_PHNET_PIN ||
> +		     pin_type == OFONO_SIM_PASSWORD_PHNETSUB_PIN ||
> +		     pin_type == OFONO_SIM_PASSWORD_PHSP_PIN ||
> +		     pin_type == OFONO_SIM_PASSWORD_PHCORP_PIN)))
> +			ofono_modem_reset(modem);

So I'm mostly happy with this approach except that ofono_modem_reset is
too big of a hammer to use here.  We need something that drops us back
to pre-sim state without having to re-power on the device (which is what
reset does).

Can you try using sim_inserted / sim_removed or maybe even be smarter
and do 90% of what sim removal does but without affecting information
which is already read.  So things like EFiccid, EFpl/EFlp, EFecc, etc.

Regards,
-Denis

Re: [PATCH] sim: check if lock is locked after code input attempt

[Jussi Kangas]

[-- Attachment #1: Type: text/plain, Size: 2037 bytes --]


Hi, Denis.

> So I'm mostly happy with this approach except that ofono_modem_reset is
> too big of a hammer to use here.  We need something that drops us back
> to pre-sim state without having to re-power on the device (which is what
> reset does).
> 
> Can you try using sim_inserted / sim_removed or maybe even be smarter
> and do 90% of what sim removal does but without affecting information
> which is already read.  So things like EFiccid, EFpl/EFlp, EFecc, etc.
> 
> Regards,
> -Denis

Dropping back to pre_sim is easy if I just call modem_change_state
directly and not through ofono_modem_reset. Problem with that approach
is how to come back to online state after PUK has been given without
effort from client. Calling of ofono_modem_set_powered from
ofono_modem_reset enables the situation where Ofono comes back online by
itself after unblocking the pin. 

Original solution made by Marit did pretty much what u sound to be
looking for now. In original solution ofono get's network "denied" state
but all the APIs stay up since modem does not go to PRE_SIM state. But
if client goes and tries to retrieve something from network or sim it
get's error. And when PUK was given system went back online nicely and
without any extra pushing from client side. 

So why PRE_SIM state? I suppose you are worried that user goes under
different cell or network during denied state and ofono does not read
all changed information? Or ofono client reads some cached information
totally unaware that it's not online anymore? I haven't gone walking
around with this thing to see how well it get's the new info when it
returns from denied state to online in different network or cell so
there might something.

Playing with sim inserted sim removed states is plain wrong to me: What
we have here has nothing to do with sim's presence. I have to
investigate the sim remove functionality more before I can say for sure
if there is something that could be reused. 

Br,
-Jussi



   

Re: [PATCH] sim: check if lock is locked after code input attempt

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2703 bytes --]

Hi Jussi,

On 01/25/2011 09:34 AM, Jussi Kangas wrote:
> 
> Hi, Denis.
> 
>> So I'm mostly happy with this approach except that ofono_modem_reset is
>> too big of a hammer to use here.  We need something that drops us back
>> to pre-sim state without having to re-power on the device (which is what
>> reset does).
>>
>> Can you try using sim_inserted / sim_removed or maybe even be smarter
>> and do 90% of what sim removal does but without affecting information
>> which is already read.  So things like EFiccid, EFpl/EFlp, EFecc, etc.
>>
>> Regards,
>> -Denis
> 
> Dropping back to pre_sim is easy if I just call modem_change_state
> directly and not through ofono_modem_reset. Problem with that approach
> is how to come back to online state after PUK has been given without
> effort from client. Calling of ofono_modem_set_powered from
> ofono_modem_reset enables the situation where Ofono comes back online by
> itself after unblocking the pin. 
> 
> Original solution made by Marit did pretty much what u sound to be
> looking for now. In original solution ofono get's network "denied" state
> but all the APIs stay up since modem does not go to PRE_SIM state. But
> if client goes and tries to retrieve something from network or sim it
> get's error. And when PUK was given system went back online nicely and
> without any extra pushing from client side. 
> 
> So why PRE_SIM state? I suppose you are worried that user goes under
> different cell or network during denied state and ofono does not read
> all changed information? Or ofono client reads some cached information
> totally unaware that it's not online anymore? I haven't gone walking
> around with this thing to see how well it get's the new info when it
> returns from denied state to online in different network or cell so
> there might something.

My main concern here is that we do not expose functionality that is
actually not available.  That confuses the applications.  It might also
be that your SIM PUK state is semi-permanent (e.g. you don't know the
PUK, and being on hold with your operator makes you shudder)

So we should simply remove all the atoms that are not relevant and
'forget' all the information that is not available if the SIM PIN is not
known.

> 
> Playing with sim inserted sim removed states is plain wrong to me: What
> we have here has nothing to do with sim's presence. I have to
> investigate the sim remove functionality more before I can say for sure
> if there is something that could be reused. 

I agree that sim_inserted is not quite the right approach.  Re-using
parts of it would definitely be the right direction.

Regards,
-Denis