[sim-reset-pin-v2 0/3] pass reset password type to driver

[sim-reset-pin-v2 0/3] pass reset password type to driver

[Pekka.Pessi]

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

Hi all,

Here is a less controversial version of the SIM API change.

The reset password type is passed down to the driver.

The generic atmodem sim driver caches the pin type from last CPIN?, and if
it is not PUK, returns an error.

A quirk is added for mbm so it will try to trigger the +CPIN: SIM PUK/PUK2
response.

--Pekka

[sim-reset-pin-v2 1/3] sim: pass reset password type to driver

[Pekka.Pessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

---
 include/sim.h |    5 +++--
 src/sim.c     |    9 +++++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/sim.h b/include/sim.h
index 36a99b9..d3e564c 100644
--- a/include/sim.h
+++ b/include/sim.h
@@ -143,8 +143,9 @@ struct ofono_sim_driver {
 			ofono_sim_passwd_cb_t cb, void *data);
 	void (*send_passwd)(struct ofono_sim *sim, const char *passwd,
 			ofono_sim_lock_unlock_cb_t cb, void *data);
-	void (*reset_passwd)(struct ofono_sim *sim, const char *puk,
-			const char *passwd,
+	void (*reset_passwd)(struct ofono_sim *sim,
+			enum ofono_sim_password_type type,
+			const char *puk, const char *passwd,
 			ofono_sim_lock_unlock_cb_t cb, void *data);
 	void (*change_passwd)(struct ofono_sim *sim,
 			enum ofono_sim_password_type type,
diff --git a/src/sim.c b/src/sim.c
index d0b5988..5e918f2 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -747,8 +747,13 @@ static DBusMessage *sim_reset_pin(DBusConnection *conn, DBusMessage *msg,
 
 	type = sim_string_to_passwd(typestr);
 
-	if (type == OFONO_SIM_PASSWORD_NONE || type != sim->pin_type)
+	switch (type) {
+	case OFONO_SIM_PASSWORD_SIM_PIN:
+	case OFONO_SIM_PASSWORD_SIM_PIN2:
+		break;
+	default:
 		return __ofono_error_invalid_format(msg);
+	}
 
 	if (!is_valid_pin(puk, PIN_TYPE_PUK))
 		return __ofono_error_invalid_format(msg);
@@ -757,7 +762,7 @@ static DBusMessage *sim_reset_pin(DBusConnection *conn, DBusMessage *msg,
 		return __ofono_error_invalid_format(msg);
 
 	sim->pending = dbus_message_ref(msg);
-	sim->driver->reset_passwd(sim, puk, pin, sim_enter_pin_cb, sim);
+	sim->driver->reset_passwd(sim, type, puk, pin, sim_enter_pin_cb, sim);
 
 	return NULL;
 }
-- 
1.7.0.4

[sim-reset-pin-v2 2/3] atmodem/sim: add pin type to PIN reset method

[Pekka.Pessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

The PIN reset requires that modem waits for corresponding PUK code.
---
 drivers/atmodem/sim.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 21bc933..a80891c 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -44,6 +44,7 @@
 struct sim_data {
 	GAtChat *chat;
 	unsigned int vendor;
+	enum ofono_sim_password_type cpin_type;
 	guint epev_id;
 	guint epev_source;
 };
@@ -459,6 +460,8 @@ static void at_cpin_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	int len = sizeof(at_sim_name) / sizeof(*at_sim_name);
 	const char *final = g_at_result_final_response(result);
 
+	sd->cpin_type = OFONO_SIM_PASSWORD_NONE;
+
 	if (sd->vendor == OFONO_VENDOR_WAVECOM && ok && strlen(final) > 7)
 		decode_at_error(&error, "OK");
 	else
@@ -498,6 +501,8 @@ static void at_cpin_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	DBG("crsm_pin_cb: %s", pin_required);
 
+	sd->cpin_type = pin_type;
+
 	cb(&error, pin_type, cbd->data);
 }
 
@@ -616,8 +621,9 @@ error:
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
-static void at_pin_send_puk(struct ofono_sim *sim, const char *puk,
-				const char *passwd,
+static void at_pin_send_puk(struct ofono_sim *sim,
+				enum ofono_sim_password_type type,
+				const char *puk, const char *passwd,
 				ofono_sim_lock_unlock_cb_t cb, void *data)
 {
 	struct sim_data *sd = ofono_sim_get_data(sim);
@@ -628,6 +634,10 @@ static void at_pin_send_puk(struct ofono_sim *sim, const char *puk,
 	if (!cbd)
 		goto error;
 
+	if (type != OFONO_SIM_PASSWORD_SIM_PIN ||
+			sd->cpin_type != OFONO_SIM_PASSWORD_SIM_PUK)
+		goto error;
+
 	snprintf(buf, sizeof(buf), "AT+CPIN=\"%s\",\"%s\"", puk, passwd);
 
 	ret = g_at_chat_send(sd->chat, buf, none_prefix,
-- 
1.7.0.4

[sim-reset-pin-v2 3/3] atmodem/sim: add mbm quirk for PIN/PIN2 reset

[Pekka.Pessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

Trigger prompt for the correct PUK on mbm.
---
 drivers/atmodem/sim.c |  169 +++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 136 insertions(+), 33 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index a80891c..1ef4342 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -621,39 +621,6 @@ error:
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
-static void at_pin_send_puk(struct ofono_sim *sim,
-				enum ofono_sim_password_type type,
-				const char *puk, const char *passwd,
-				ofono_sim_lock_unlock_cb_t cb, void *data)
-{
-	struct sim_data *sd = ofono_sim_get_data(sim);
-	struct cb_data *cbd = cb_data_new(cb, data);
-	char buf[64];
-	int ret;
-
-	if (!cbd)
-		goto error;
-
-	if (type != OFONO_SIM_PASSWORD_SIM_PIN ||
-			sd->cpin_type != OFONO_SIM_PASSWORD_SIM_PUK)
-		goto error;
-
-	snprintf(buf, sizeof(buf), "AT+CPIN=\"%s\",\"%s\"", puk, passwd);
-
-	ret = g_at_chat_send(sd->chat, buf, none_prefix,
-				at_lock_unlock_cb, cbd, g_free);
-
-	memset(buf, 0, sizeof(buf));
-
-	if (ret > 0)
-		return;
-
-error:
-	g_free(cbd);
-
-	CALLBACK_WITH_FAILURE(cb, data);
-}
-
 static const char *const at_clck_cpwd_fac[] = {
 	[OFONO_SIM_PASSWORD_SIM_PIN] = "SC",
 	[OFONO_SIM_PASSWORD_SIM_PIN2] = "P2",
@@ -699,6 +666,142 @@ error:
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
+struct at_pin_reset_data {
+	struct cb_data base;
+	enum ofono_sim_password_type type;
+	char puk[9];
+	char pin[9];
+};
+
+static void at_pin_query_result(const struct ofono_error *error,
+				enum ofono_sim_password_type pin_type,
+				void *data)
+{
+	struct at_pin_reset_data *cbd = data;
+	struct sim_data *sd = ofono_sim_get_data(cbd->base.user);
+	ofono_sim_lock_unlock_cb_t cb = cbd->base.cb;
+	void *user_data = cbd->base.data;
+	char buf[64];
+	int ret;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+		goto failed;
+
+	switch (pin_type) {
+	case OFONO_SIM_PASSWORD_SIM_PUK:
+		if (cbd->type != OFONO_SIM_PASSWORD_SIM_PIN)
+			goto failed;
+		break;
+	case OFONO_SIM_PASSWORD_SIM_PUK2:
+		if (cbd->type != OFONO_SIM_PASSWORD_SIM_PIN2)
+			goto failed;
+		break;
+	default:
+		goto failed;
+	}
+
+	snprintf(buf, sizeof(buf), "AT+CPIN=\"%s\",\"%s\"", cbd->puk, cbd->pin);
+
+	memset(cbd->pin, 0, sizeof(cbd->pin));
+	memset(cbd->puk, 0, sizeof(cbd->puk));
+
+	ret = g_at_chat_send(sd->chat, buf, none_prefix,
+				at_lock_unlock_cb, cbd, g_free);
+
+	memset(buf, 0, sizeof(buf));
+
+	if (ret > 0)
+		return;
+
+failed:
+	g_free(cbd);
+	CALLBACK_WITH_FAILURE(cb, user_data);
+}
+
+static void at_trigger_puk(struct ofono_sim *sim,
+				enum ofono_sim_password_type type,
+				const char *puk, const char *pin,
+				ofono_sim_lock_unlock_cb_t cb, void *data)
+{
+	struct sim_data *sd = ofono_sim_get_data(sim);
+	struct at_pin_reset_data *cbd;
+	char buf[64];
+
+	if (type != OFONO_SIM_PASSWORD_SIM_PIN &&
+			type == OFONO_SIM_PASSWORD_SIM_PIN2)
+		goto error;
+
+	cbd = g_new0(struct at_pin_reset_data, 1);
+	if (!cbd)
+		goto error;
+
+	cbd->base.cb = cb;
+	cbd->base.data = data;
+	cbd->base.user = sim;
+	cbd->type = type;
+	strcpy(cbd->puk, puk);
+	strcpy(cbd->pin, pin);
+
+	/* We need to trigger AT+CPIN? to return SIM PUK or SIM PUK2 */
+
+	snprintf(buf, sizeof(buf), "AT+CPWD=\"%s\",\"%s\",\"%s\"",
+			at_clck_cpwd_fac[type], pin, pin);
+
+	g_at_chat_send(sd->chat, buf, none_prefix, NULL, NULL, NULL);
+
+	memset(buf, 0, sizeof(buf));
+
+	at_pin_query(sim, at_pin_query_result, cbd);
+
+	return;
+
+error:
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
+static void at_pin_send_puk(struct ofono_sim *sim,
+				enum ofono_sim_password_type type,
+				const char *puk, const char *passwd,
+				ofono_sim_lock_unlock_cb_t cb, void *data)
+{
+	struct sim_data *sd = ofono_sim_get_data(sim);
+	struct cb_data *cbd = NULL;
+	char buf[64];
+	int ret;
+
+	if (type != OFONO_SIM_PASSWORD_SIM_PIN ||
+			sd->cpin_type != OFONO_SIM_PASSWORD_SIM_PUK) {
+		switch (sd->vendor) {
+		case OFONO_VENDOR_MBM:
+			at_trigger_puk(sim, type, puk, passwd, cb, data);
+			return;
+		default:
+			break;
+		}
+
+		goto error;
+	}
+
+	cbd = cb_data_new(cb, data);
+	if (!cbd)
+		goto error;
+
+	snprintf(buf, sizeof(buf), "AT+CPIN=\"%s\",\"%s\"", puk, passwd);
+
+	ret = g_at_chat_send(sd->chat, buf, none_prefix,
+				at_lock_unlock_cb, cbd, g_free);
+
+	memset(buf, 0, sizeof(buf));
+
+	if (ret > 0)
+		return;
+
+error:
+	g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
 static void at_change_passwd(struct ofono_sim *sim,
 			enum ofono_sim_password_type passwd_type,
 			const char *old, const char *new,
-- 
1.7.0.4

Re: [sim-reset-pin-v2 1/3] sim: pass reset password type to driver

[Denis Kenzior]

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

Hi Pekka,

On 08/30/2010 12:53 PM, Pekka.Pessi(a)nokia.com wrote:
> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> ---
>  include/sim.h |    5 +++--
>  src/sim.c     |    9 +++++++--
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/include/sim.h b/include/sim.h
> index 36a99b9..d3e564c 100644
> --- a/include/sim.h
> +++ b/include/sim.h
> @@ -143,8 +143,9 @@ struct ofono_sim_driver {
>  			ofono_sim_passwd_cb_t cb, void *data);
>  	void (*send_passwd)(struct ofono_sim *sim, const char *passwd,
>  			ofono_sim_lock_unlock_cb_t cb, void *data);
> -	void (*reset_passwd)(struct ofono_sim *sim, const char *puk,
> -			const char *passwd,
> +	void (*reset_passwd)(struct ofono_sim *sim,
> +			enum ofono_sim_password_type type,
> +			const char *puk, const char *passwd,

Honestly I'm still lost, why are we doing this again?  reset_passwd
corresponds to CPIN from 27.007.  This one does not accept a type.  Some
modems (like the calypso) have the +CPIN2 command which can be used to
enter PIN/PUK2.  Are you trying to solve this case?

If so, we either need to add the type to both reset_passwd and
send_passwd or leave this up to the driver to figure out.

>  			ofono_sim_lock_unlock_cb_t cb, void *data);
>  	void (*change_passwd)(struct ofono_sim *sim,
>  			enum ofono_sim_password_type type,
> diff --git a/src/sim.c b/src/sim.c
> index d0b5988..5e918f2 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -747,8 +747,13 @@ static DBusMessage *sim_reset_pin(DBusConnection *conn, DBusMessage *msg,
>  
>  	type = sim_string_to_passwd(typestr);
>  
> -	if (type == OFONO_SIM_PASSWORD_NONE || type != sim->pin_type)
> +	switch (type) {
> +	case OFONO_SIM_PASSWORD_SIM_PIN:
> +	case OFONO_SIM_PASSWORD_SIM_PIN2:
> +		break;
> +	default:

And now you're changing the API semantics completely.  ResetPin is
supposed to be called like this:

ResetPin("puk"/"puk2"/"corppuk"/"networkpuk"/etc, puk, newpin)

Now you're changing this to be called like:

ResetPin("pin"/"pin2", ...)

I'm not necessarily saying this is a bad idea, but this does not cover
all the other puks described in 27.007.  It also arbitrarily changes the
API with no reasoning given.

If you're making such subtle changes, at least describe the reasoning in
the commit description.  Otherwise everyone has to stare at this code
and figure out what you're doing and why.

>  		return __ofono_error_invalid_format(msg);
> +	}
>  
>  	if (!is_valid_pin(puk, PIN_TYPE_PUK))
>  		return __ofono_error_invalid_format(msg);
> @@ -757,7 +762,7 @@ static DBusMessage *sim_reset_pin(DBusConnection *conn, DBusMessage *msg,
>  		return __ofono_error_invalid_format(msg);
>  
>  	sim->pending = dbus_message_ref(msg);
> -	sim->driver->reset_passwd(sim, puk, pin, sim_enter_pin_cb, sim);
> +	sim->driver->reset_passwd(sim, type, puk, pin, sim_enter_pin_cb, sim);
>  
>  	return NULL;
>  }

Regards,
-Denis

Re: [sim-reset-pin-v2 1/3] sim: pass reset password type to driver

[Pekka Pessi]

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

Hi Denis,

2010/9/1 Denis Kenzior <denkenz@gmail.com>:
>>       void (*send_passwd)(struct ofono_sim *sim, const char *passwd,
>>                       ofono_sim_lock_unlock_cb_t cb, void *data);
>> -     void (*reset_passwd)(struct ofono_sim *sim, const char *puk,
>> -                     const char *passwd,
>> +     void (*reset_passwd)(struct ofono_sim *sim,
>> +                     enum ofono_sim_password_type type,
>> +                     const char *puk, const char *passwd,
>
> Honestly I'm still lost, why are we doing this again?  reset_passwd
> corresponds to CPIN from 27.007.  This one does not accept a type.  Some
> modems (like the calypso) have the +CPIN2 command which can be used to
> enter PIN/PUK2.  Are you trying to solve this case?

Yes, I'm trying to reset PIN2. The +CPIN="puk","pin" uses the type
from the last +CPIN?. Unfortunately it may be transient (and I still
don't understand how the PIN2 is supposed to be entered using the AT
command interface).

> If so, we either need to add the type to both reset_passwd and
> send_passwd or leave this up to the driver to figure out.

OK, I'll do it that way.

>>                       ofono_sim_lock_unlock_cb_t cb, void *data);
>>       void (*change_passwd)(struct ofono_sim *sim,
>>                       enum ofono_sim_password_type type,
>> diff --git a/src/sim.c b/src/sim.c
>> index d0b5988..5e918f2 100644
>> --- a/src/sim.c
>> +++ b/src/sim.c
>> @@ -747,8 +747,13 @@ static DBusMessage *sim_reset_pin(DBusConnection *conn, DBusMessage *msg,
>>
>>       type = sim_string_to_passwd(typestr);
>>
>> -     if (type == OFONO_SIM_PASSWORD_NONE || type != sim->pin_type)
>> +     switch (type) {
>> +     case OFONO_SIM_PASSWORD_SIM_PIN:
>> +     case OFONO_SIM_PASSWORD_SIM_PIN2:
>> +             break;
>> +     default:
>
> And now you're changing the API semantics completely.  ResetPin is
> supposed to be called like this:
>
> ResetPin("puk"/"puk2"/"corppuk"/"networkpuk"/etc, puk, newpin)
>
> Now you're changing this to be called like:
>
> ResetPin("pin"/"pin2", ...)
>
> I'm not necessarily saying this is a bad idea, but this does not cover
> all the other puks described in 27.007.

Oops.  I'll remove the check, it should be in the driver.

-- 
Pekka.Pessi mail at nokia.com