[PATCH] [RFC] Do the PIN check in SIMManager

[PATCH] [RFC] Do the PIN check in SIMManager

[Andrzej Zaborowski]

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

This adds checking whether PIN is required during SIM initialisation and
delaying the sim ready notifications until after correct PIN is given.
According to 31.102 the IMSI should be retrieved after the PIN check
however on the SIMs I've tried it can be retrieved before authentication
and I think it would make sense to provide the imsi to the clients so
they know which card they're authenticating to if they're swapping cards.
---
 drivers/atmodem/sim.c |  171 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/sim.h         |   13 ++++
 include/types.h       |   20 ++++++
 src/sim.c             |  136 ++++++++++++++++++++++++++++++++++++++-
 src/simutil.c         |   24 +++++++
 src/simutil.h         |    2 +
 6 files changed, 365 insertions(+), 1 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 8333670..fdb972a 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -431,6 +431,174 @@ error:
 	}
 }
 
+static struct {
+	enum ofono_sim_passwd_type type;
+	const char *name;
+} const at_sim_name[] = {
+	{ OFONO_PASSWD_SIM_PIN, "SIM PIN" },
+	{ OFONO_PASSWD_SIM_PUK, "SIM PUK" },
+	{ OFONO_PASSWD_PHSIM_PIN, "PH-SIM PIN" },
+	{ OFONO_PASSWD_PHFSIM_PIN, "PH-FSIM PIN" },
+	{ OFONO_PASSWD_PHFSIM_PUK, "PH-FSIM PUK" },
+	{ OFONO_PASSWD_SIM_PIN2, "SIM PIN2" },
+	{ OFONO_PASSWD_SIM_PUK2, "SIM PUK2" },
+	{ OFONO_PASSWD_PHNET_PIN, "PH-NET PIN" },
+	{ OFONO_PASSWD_PHNET_PUK, "PH-NET PUK" },
+	{ OFONO_PASSWD_PHNETSUB_PIN, "PH-NETSUB PIN" },
+	{ OFONO_PASSWD_PHNETSUB_PUK, "PH-NETSUB PUK" },
+	{ OFONO_PASSWD_PHSP_PIN, "PH-SP PIN" },
+	{ OFONO_PASSWD_PHSP_PUK, "PH-SP PUK" },
+	{ OFONO_PASSWD_PHCORP_PIN, "PH-CORP PIN" },
+	{ OFONO_PASSWD_PHCORP_PUK, "PH-CORP PUK" },
+};
+
+static void at_cpin_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	GAtResultIter iter;
+	ofono_sim_passwd_cb_t cb = cbd->cb;
+	struct ofono_error error;
+	const char *pin_required;
+	int pin_type;
+	int i;
+	int len = sizeof(at_sim_name) / sizeof(*at_sim_name);
+
+	dump_response("at_cpin_cb", ok, result);
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok) {
+		cb(&error, -1, cbd->data);
+		return;
+	}
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CPIN:")) {
+		DECLARE_FAILURE(e);
+
+		cb(&e, -1, cbd->data);
+		return;
+	}
+
+	pin_required = g_at_result_iter_raw_line(&iter);
+
+	pin_type = -1;
+	if (!strcmp(pin_required, "READY"))
+		pin_type = OFONO_PASSWD_NONE;
+	else
+		for (i = 0; i < len; i++)
+			if (!g_str_has_suffix(pin_required,
+						at_sim_name[i].name))
+				pin_type = at_sim_name[i].type;
+
+	if (pin_type == -1) {
+		DECLARE_FAILURE(e);
+
+		cb(&e, -1, cbd->data);
+		return;
+	}
+
+	ofono_debug("crsm_pin_cb: %s", pin_required);
+
+	cb(&error, pin_type, cbd->data);
+}
+
+static void at_pin_query(struct ofono_sim *sim, ofono_sim_passwd_cb_t cb,
+			void *data)
+{
+	GAtChat *chat = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, data);
+
+	if (!cbd)
+		goto error;
+
+	if (g_at_chat_send(chat, "AT+CPIN?", NULL,
+				at_cpin_cb, cbd, g_free) > 0)
+		return;
+
+error:
+	if (cbd)
+		g_free(cbd);
+
+	{
+		DECLARE_FAILURE(error);
+		cb(&error, -1, data);
+	}
+}
+
+static void at_cpin_set_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_sim_passwd_send_cb_t cb = cbd->cb;
+	struct ofono_error error;
+
+	dump_response("at_cpin_set_cb", ok, result);
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	cb(&error, cbd->data);
+}
+
+static void at_pin_send(struct ofono_sim *sim, const char *passwd,
+			ofono_sim_passwd_send_cb_t cb, void *data)
+{
+	GAtChat *chat = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	char buf[64];
+	int ret;
+
+	if (!cbd)
+		goto error;
+
+	snprintf(buf, sizeof(buf), "AT+CPIN=\"%s\"", passwd);
+
+	ret = g_at_chat_send(chat, buf, NULL, at_cpin_set_cb, cbd, g_free);
+
+	memset(buf, 0, sizeof(buf));
+
+	if (ret > 0)
+		return;
+
+error:
+	if (cbd)
+		g_free(cbd);
+
+	{
+		DECLARE_FAILURE(error);
+		cb(&error, data);
+	}
+}
+
+static void at_pin_send_puk(struct ofono_sim *sim, const char *puk,
+				const char *passwd,
+				ofono_sim_passwd_send_cb_t cb, void *data)
+{
+	GAtChat *chat = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	char buf[64];
+	int ret;
+
+	if (!cbd)
+		goto error;
+
+	snprintf(buf, sizeof(buf), "AT+CPIN=\"%s\",\"%s\"", puk, passwd);
+
+	ret = g_at_chat_send(chat, buf, NULL, at_cpin_set_cb, cbd, g_free);
+
+	memset(buf, 0, sizeof(buf));
+
+	if (ret > 0)
+		return;
+
+error:
+	if (cbd)
+		g_free(cbd);
+
+	{
+		DECLARE_FAILURE(error);
+		cb(&error, data);
+	}
+}
+
 static gboolean at_sim_register(gpointer user)
 {
 	struct ofono_sim *sim = user;
@@ -467,6 +635,9 @@ static struct ofono_sim_driver driver = {
 	.write_file_linear	= at_sim_update_record,
 	.write_file_cyclic	= at_sim_update_cyclic,
 	.read_imsi		= at_read_imsi,
+	.query_passwd_state	= at_pin_query,
+	.send_passwd		= at_pin_send,
+	.send_new_passwd	= at_pin_send_puk,
 };
 
 void at_sim_init()
diff --git a/include/sim.h b/include/sim.h
index 1da486c..90bedae 100644
--- a/include/sim.h
+++ b/include/sim.h
@@ -64,6 +64,12 @@ typedef void (*ofono_sim_file_read_cb_t)(int ok,
 
 typedef void (*ofono_sim_file_write_cb_t)(int ok, void *userdata);
 
+typedef void (*ofono_sim_passwd_cb_t)(const struct ofono_error *error,
+					int passwd_type, void *data);
+
+typedef void (*ofono_sim_passwd_send_cb_t)(const struct ofono_error *error,
+					void *data);
+
 struct ofono_sim_driver {
 	const char *name;
 	int (*probe)(struct ofono_sim *sim, unsigned int vendor, void *data);
@@ -90,6 +96,13 @@ struct ofono_sim_driver {
 			ofono_sim_write_cb_t cb, void *data);
 	void (*read_imsi)(struct ofono_sim *sim,
 			ofono_sim_imsi_cb_t cb, void *data);
+	void (*query_passwd_state)(struct ofono_sim *sim,
+			ofono_sim_passwd_cb_t cb, void *data);
+	void (*send_passwd)(struct ofono_sim *sim, const char *passwd,
+			ofono_sim_passwd_send_cb_t cb, void *data);
+	void (*send_new_passwd)(struct ofono_sim *sim, const char *puk,
+			const char *passwd,
+			ofono_sim_passwd_send_cb_t cb, void *data);
 };
 
 int ofono_sim_driver_register(const struct ofono_sim_driver *d);
diff --git a/include/types.h b/include/types.h
index 6a9681d..99e2ca5 100644
--- a/include/types.h
+++ b/include/types.h
@@ -91,6 +91,26 @@ struct ofono_call {
 	int clip_validity;
 };
 
+enum ofono_sim_passwd_type {
+	OFONO_PASSWD_NONE = 0,
+
+	OFONO_PASSWD_SIM_PIN,
+	OFONO_PASSWD_SIM_PUK,
+	OFONO_PASSWD_PHSIM_PIN,
+	OFONO_PASSWD_PHFSIM_PIN,
+	OFONO_PASSWD_PHFSIM_PUK,
+	OFONO_PASSWD_SIM_PIN2,
+	OFONO_PASSWD_SIM_PUK2,
+	OFONO_PASSWD_PHNET_PIN,
+	OFONO_PASSWD_PHNET_PUK,
+	OFONO_PASSWD_PHNETSUB_PIN,
+	OFONO_PASSWD_PHNETSUB_PUK,
+	OFONO_PASSWD_PHSP_PIN,
+	OFONO_PASSWD_PHSP_PUK,
+	OFONO_PASSWD_PHCORP_PIN,
+	OFONO_PASSWD_PHCORP_PUK,
+};
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/sim.c b/src/sim.c
index a072c8c..44f0a3b 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -55,6 +55,7 @@ static GSList *g_drivers = NULL;
 static gboolean sim_op_next(gpointer user_data);
 static gboolean sim_op_retrieve_next(gpointer user);
 static void sim_own_numbers_update(struct ofono_sim *sim);
+static void sim_pin_check(struct ofono_sim *sim);
 
 struct sim_file_op {
 	int id;
@@ -77,6 +78,8 @@ struct ofono_sim {
 	GSList *service_numbers;
 	gboolean sdn_ready;
 	gboolean ready;
+	gboolean pin_ready;
+	int pin_type;
 	GQueue *simop_q;
 	gint simop_source;
 	unsigned char efmsisdn_length;
@@ -86,6 +89,7 @@ struct ofono_sim {
 	const struct ofono_sim_driver *driver;
 	void *driver_data;
 	struct ofono_atom *atom;
+	DBusMessage *pending;
 };
 
 struct msisdn_set_request {
@@ -95,6 +99,7 @@ struct msisdn_set_request {
 	DBusMessage *msg;
 };
 
+
 struct sim_ready_watch {
 	unsigned int id;
 	ofono_sim_ready_notify_cb_t notify;
@@ -171,6 +176,8 @@ static DBusMessage *sim_get_properties(DBusConnection *conn,
 	DBusMessageIter dict;
 	char **own_numbers;
 	char **service_numbers;
+	dbus_bool_t pin_needed;
+	const char *pin_name;
 
 	reply = dbus_message_new_method_return(msg);
 	if (!reply)
@@ -205,6 +212,18 @@ static DBusMessage *sim_get_properties(DBusConnection *conn,
 		g_strfreev(service_numbers);
 	}
 
+	pin_needed = !sim->pin_ready;
+	ofono_dbus_dict_append(&dict, "WaitingForPIN",
+				DBUS_TYPE_BOOLEAN, &pin_needed);
+
+	if (sim->pin_type != OFONO_PASSWD_NONE) {
+		pin_name = sim_passwd_name(sim->pin_type);
+
+		ofono_dbus_dict_append(&dict, "PINTypeRequired",
+					DBUS_TYPE_STRING,
+					(void *) &pin_name);
+	}
+
 	dbus_message_iter_close_container(&iter, &dict);
 
 	return reply;
@@ -353,10 +372,64 @@ error:
 	return __ofono_error_invalid_args(msg);
 }
 
+static void sim_send_pin_cb(const struct ofono_error *error, void *data)
+{
+	struct ofono_sim *sim = data;
+	DBusMessage *reply;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+		reply = __ofono_error_failed(sim->pending);
+	else
+		reply = dbus_message_new_method_return(sim->pending);
+
+
+	__ofono_dbus_pending_reply(&sim->pending, reply);
+
+	/* Re-check the authentication status and update DBus properties
+	 * as needed.  We need to do this regardless of whether the password
+	 * given was correct: if it was incorrect, the number of retries
+	 * may have expired and a different password is being requested now.
+	 * If it was correct, possibly proceed with SIM initialisation.
+	 */
+	sim_pin_check(sim);
+}
+
+static DBusMessage *sim_send_pin(DBusConnection *conn, DBusMessage *msg,
+					void *data)
+{
+	struct ofono_sim *sim = data;
+	DBusMessageIter iter;
+	const char *pin;
+
+	if (!sim->driver->send_passwd)
+		return __ofono_error_not_implemented(msg);
+
+	if (sim->pending)
+		return __ofono_error_busy(msg);
+
+	if (!dbus_message_iter_init(msg, &iter))
+		return __ofono_error_invalid_args(msg);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &pin);
+
+	if (!is_valid_pin(pin))
+		return __ofono_error_invalid_format(msg);
+
+	sim->pending = dbus_message_ref(msg);
+	sim->driver->send_passwd(sim, pin, sim_send_pin_cb, sim);
+
+	return NULL;
+}
+
 static GDBusMethodTable sim_methods[] = {
 	{ "GetProperties",	"",	"a{sv}",	sim_get_properties	},
 	{ "SetProperty",	"sv",	"",		sim_set_property,
 							G_DBUS_METHOD_FLAG_ASYNC },
+	{ "SendPIN",	"s",	"",			sim_send_pin,
+							G_DBUS_METHOD_FLAG_ASYNC },
 	{ }
 };
 
@@ -612,6 +685,67 @@ static void sim_retrieve_imsi(struct ofono_sim *sim)
 	sim->driver->read_imsi(sim, sim_imsi_cb, sim);
 }
 
+static void sim_pin_check_done(struct ofono_sim *sim)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(sim->atom);
+	dbus_bool_t value = FALSE;
+
+	if (sim->pin_ready == FALSE) {
+		sim->pin_ready = TRUE;
+
+		ofono_dbus_signal_property_changed(conn, path,
+							SIM_MANAGER_INTERFACE,
+							"WaitingForPIN",
+							DBUS_TYPE_BOOLEAN,
+							&value);
+	}
+
+	sim_retrieve_imsi(sim);
+}
+
+static void sim_pin_query_cb(const struct ofono_error *error, int pin_type,
+		void *data)
+{
+	struct ofono_sim *sim = data;
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(sim->atom);
+	const char *pin_name;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_error("Querrying PIN authentication state failed");
+
+		sim_pin_check_done(sim);
+		return;
+	}
+
+	if (sim->pin_type != pin_type) {
+		sim->pin_type = pin_type;
+		pin_name = sim_passwd_name(pin_type);
+
+		ofono_dbus_signal_property_changed(conn, path,
+							SIM_MANAGER_INTERFACE,
+							"PINTypeRequired",
+							DBUS_TYPE_STRING,
+							&pin_name);
+	}
+
+	if (pin_type == OFONO_PASSWD_NONE)
+		sim_pin_check_done(sim);
+}
+
+static void sim_pin_check(struct ofono_sim *sim)
+{
+	if (!sim->driver->query_passwd_state) {
+		sim_pin_check_done(sim);
+		return;
+	}
+
+	sim->pin_ready = FALSE;
+
+	sim->driver->query_passwd_state(sim, sim_pin_query_cb, sim);
+}
+
 static void sim_op_error(struct ofono_sim *sim)
 {
 	struct sim_file_op *op = g_queue_pop_head(sim->simop_q);
@@ -1312,7 +1446,7 @@ void ofono_sim_register(struct ofono_sim *sim)
 	 * arbitrary files to be written or read, assuming their presence
 	 * in the EFust
 	 */
-	sim_retrieve_imsi(sim);
+	sim_pin_check(sim);
 }
 
 void ofono_sim_remove(struct ofono_sim *sim)
diff --git a/src/simutil.c b/src/simutil.c
index b80e014..2b26700 100644
--- a/src/simutil.c
+++ b/src/simutil.c
@@ -488,3 +488,27 @@ void sim_adn_build(unsigned char *data, int length,
 	/* Ext1 unused */
 	*data++ = 0xff;
 }
+
+static const char *const passwd_name[] = {
+	[OFONO_PASSWD_NONE] = "None",
+	[OFONO_PASSWD_SIM_PIN] = "SIM PIN",
+	[OFONO_PASSWD_SIM_PUK] = "SIM PUK",
+	[OFONO_PASSWD_PHSIM_PIN] = "PH-SIM PIN",
+	[OFONO_PASSWD_PHFSIM_PIN] = "PH-FSIM PIN",
+	[OFONO_PASSWD_PHFSIM_PUK] = "PH-FSIM PUK",
+	[OFONO_PASSWD_SIM_PIN2] = "SIM PIN2",
+	[OFONO_PASSWD_SIM_PUK2] = "SIM PUK2",
+	[OFONO_PASSWD_PHNET_PIN] = "PH-NET PIN",
+	[OFONO_PASSWD_PHNET_PUK] = "PH-NET PUK",
+	[OFONO_PASSWD_PHNETSUB_PIN] = "PH-NETSUB PIN",
+	[OFONO_PASSWD_PHNETSUB_PUK] = "PH-NETSUB PUK",
+	[OFONO_PASSWD_PHSP_PIN] = "PH-SP PIN",
+	[OFONO_PASSWD_PHSP_PUK] = "PH-SP PUK",
+	[OFONO_PASSWD_PHCORP_PIN] = "PH-CORP PIN",
+	[OFONO_PASSWD_PHCORP_PUK] = "PH-CORP PUK",
+};
+
+const char *sim_passwd_name(enum ofono_sim_passwd_type type)
+{
+	return passwd_name[type];
+}
diff --git a/src/simutil.h b/src/simutil.h
index de3e55e..fc4f063 100644
--- a/src/simutil.h
+++ b/src/simutil.h
@@ -88,3 +88,5 @@ gboolean sim_adn_parse(const unsigned char *data, int length,
 void sim_adn_build(unsigned char *data, int length,
 			const struct ofono_phone_number *ph,
 			const char *identifier);
+
+const char *sim_passwd_name(enum ofono_sim_passwd_type type);
-- 
1.6.1

Re: [PATCH] [RFC] Do the PIN check in SIMManager

[Santtu Lakkala]

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrzej Zaborowski wrote:
> This adds checking whether PIN is required during SIM initialisation and
> delaying the sim ready notifications until after correct PIN is given.
> According to 31.102 the IMSI should be retrieved after the PIN check
> however on the SIMs I've tried it can be retrieved before authentication
> and I think it would make sense to provide the imsi to the clients so
> they know which card they're authenticating to if they're swapping cards.

> +		for (i = 0; i < len; i++)
> +			if (!g_str_has_suffix(pin_required,
> +						at_sim_name[i].name))
> +				pin_type = at_sim_name[i].type;

Should this be:
> +		for (i = 0; i < len; i++)
> +			if (g_str_has_suffix(pin_required,
> +						at_sim_name[i].name)) {
> +				pin_type = at_sim_name[i].type;
> +				break;
> +			}

With the original code, I always get PH-CORP PUK (or PH-CORP PIN, should
 the required code be PH-CORP PUK).

- --
Santtu Lakkala
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkqk2AwACgkQX9Rc0+po4p1YVQCgl//9VD6QQ+BmRBqtIn9/pRMH
ticAn0QT9CvLrtQp/6cVLT0dwg8xe5q4
=R/Q0
-----END PGP SIGNATURE-----

Re: [PATCH] [RFC] Do the PIN check in SIMManager

[Andrzej Zaborowski]

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

Hi Santtu,

2009/9/7 Santtu Lakkala <inz@inz.fi>:
> Andrzej Zaborowski wrote:
>> +             for (i = 0; i < len; i++)
>> +                     if (!g_str_has_suffix(pin_required,
>> +                                             at_sim_name[i].name))
>> +                             pin_type = at_sim_name[i].type;
>
> Should this be:
>> +             for (i = 0; i < len; i++)
>> +                     if (g_str_has_suffix(pin_required,
>> +                                             at_sim_name[i].name)) {

Ugh, yes, this was wrong but what I wanted was g_str_has_prefix() and
no break; so that the loop would effectively find the longest matching
prefix.  I now replaced it with full strcmp() and made use of Denis'
newly added g_at_result_iter_next_unquoted_string call.

Maybe we should pass the PIN type as string instead of enum and allow
any name beside the 27.007-defined ones.

Regards,
Andrew

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Do-the-PIN-check-in-SIMManager.patch --]
[-- Type: text/x-patch, Size: 14127 bytes --]

From 1a5bf29f7ef14fd010d637d94dadaea9d3f8f62c Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Tue, 8 Sep 2009 06:33:25 +0200
Subject: [PATCH] Do the PIN check in SIMManager

This adds checking whether PIN is required during SIM initialisation and
delaying the sim ready notifications until after correct PIN is given.
According to 31.102 the IMSI should be retrieved after the PIN check
however on the SIMs I've tried it can be retrieved before authentication
and I think it would make sense to provide the imsi to the clients so
they know which card they're authenticating to if they're swapping cards.
---
 drivers/atmodem/sim.c |  172 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/sim.h         |   13 ++++
 include/types.h       |   20 ++++++
 src/sim.c             |  136 ++++++++++++++++++++++++++++++++++++++-
 src/simutil.c         |   24 +++++++
 src/simutil.h         |    2 +
 6 files changed, 366 insertions(+), 1 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index fcd7895..8288c78 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -431,6 +431,175 @@ error:
 	}
 }
 
+static struct {
+	enum ofono_sim_passwd_type type;
+	const char *name;
+} const at_sim_name[] = {
+	{ OFONO_PASSWD_SIM_PIN, "SIM PIN" },
+	{ OFONO_PASSWD_SIM_PUK, "SIM PUK" },
+	{ OFONO_PASSWD_PHSIM_PIN, "PH-SIM PIN" },
+	{ OFONO_PASSWD_PHFSIM_PIN, "PH-FSIM PIN" },
+	{ OFONO_PASSWD_PHFSIM_PUK, "PH-FSIM PUK" },
+	{ OFONO_PASSWD_SIM_PIN2, "SIM PIN2" },
+	{ OFONO_PASSWD_SIM_PUK2, "SIM PUK2" },
+	{ OFONO_PASSWD_PHNET_PIN, "PH-NET PIN" },
+	{ OFONO_PASSWD_PHNET_PUK, "PH-NET PUK" },
+	{ OFONO_PASSWD_PHNETSUB_PIN, "PH-NETSUB PIN" },
+	{ OFONO_PASSWD_PHNETSUB_PUK, "PH-NETSUB PUK" },
+	{ OFONO_PASSWD_PHSP_PIN, "PH-SP PIN" },
+	{ OFONO_PASSWD_PHSP_PUK, "PH-SP PUK" },
+	{ OFONO_PASSWD_PHCORP_PIN, "PH-CORP PIN" },
+	{ OFONO_PASSWD_PHCORP_PUK, "PH-CORP PUK" },
+};
+
+static void at_cpin_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	GAtResultIter iter;
+	ofono_sim_passwd_cb_t cb = cbd->cb;
+	struct ofono_error error;
+	const char *pin_required;
+	int pin_type;
+	int i;
+	int len = sizeof(at_sim_name) / sizeof(*at_sim_name);
+
+	dump_response("at_cpin_cb", ok, result);
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok) {
+		cb(&error, -1, cbd->data);
+		return;
+	}
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CPIN:")) {
+		DECLARE_FAILURE(e);
+
+		cb(&e, -1, cbd->data);
+		return;
+	}
+
+	g_at_result_iter_next_unquoted_string(&iter, &pin_required);
+
+	pin_type = -1;
+	if (!strcmp(pin_required, "READY"))
+		pin_type = OFONO_PASSWD_NONE;
+	else
+		for (i = 0; i < len; i++)
+			if (!strcmp(pin_required, at_sim_name[i].name)) {
+				pin_type = at_sim_name[i].type;
+				break;
+			}
+
+	if (pin_type == -1) {
+		DECLARE_FAILURE(e);
+
+		cb(&e, -1, cbd->data);
+		return;
+	}
+
+	ofono_debug("crsm_pin_cb: %s", pin_required);
+
+	cb(&error, pin_type, cbd->data);
+}
+
+static void at_pin_query(struct ofono_sim *sim, ofono_sim_passwd_cb_t cb,
+			void *data)
+{
+	GAtChat *chat = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, data);
+
+	if (!cbd)
+		goto error;
+
+	if (g_at_chat_send(chat, "AT+CPIN?", NULL,
+				at_cpin_cb, cbd, g_free) > 0)
+		return;
+
+error:
+	if (cbd)
+		g_free(cbd);
+
+	{
+		DECLARE_FAILURE(error);
+		cb(&error, -1, data);
+	}
+}
+
+static void at_cpin_set_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_sim_passwd_send_cb_t cb = cbd->cb;
+	struct ofono_error error;
+
+	dump_response("at_cpin_set_cb", ok, result);
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	cb(&error, cbd->data);
+}
+
+static void at_pin_send(struct ofono_sim *sim, const char *passwd,
+			ofono_sim_passwd_send_cb_t cb, void *data)
+{
+	GAtChat *chat = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	char buf[64];
+	int ret;
+
+	if (!cbd)
+		goto error;
+
+	snprintf(buf, sizeof(buf), "AT+CPIN=\"%s\"", passwd);
+
+	ret = g_at_chat_send(chat, buf, NULL, at_cpin_set_cb, cbd, g_free);
+
+	memset(buf, 0, sizeof(buf));
+
+	if (ret > 0)
+		return;
+
+error:
+	if (cbd)
+		g_free(cbd);
+
+	{
+		DECLARE_FAILURE(error);
+		cb(&error, data);
+	}
+}
+
+static void at_pin_send_puk(struct ofono_sim *sim, const char *puk,
+				const char *passwd,
+				ofono_sim_passwd_send_cb_t cb, void *data)
+{
+	GAtChat *chat = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	char buf[64];
+	int ret;
+
+	if (!cbd)
+		goto error;
+
+	snprintf(buf, sizeof(buf), "AT+CPIN=\"%s\",\"%s\"", puk, passwd);
+
+	ret = g_at_chat_send(chat, buf, NULL, at_cpin_set_cb, cbd, g_free);
+
+	memset(buf, 0, sizeof(buf));
+
+	if (ret > 0)
+		return;
+
+error:
+	if (cbd)
+		g_free(cbd);
+
+	{
+		DECLARE_FAILURE(error);
+		cb(&error, data);
+	}
+}
+
 static gboolean at_sim_register(gpointer user)
 {
 	struct ofono_sim *sim = user;
@@ -467,6 +636,9 @@ static struct ofono_sim_driver driver = {
 	.write_file_linear	= at_sim_update_record,
 	.write_file_cyclic	= at_sim_update_cyclic,
 	.read_imsi		= at_read_imsi,
+	.query_passwd_state	= at_pin_query,
+	.send_passwd		= at_pin_send,
+	.send_new_passwd	= at_pin_send_puk,
 };
 
 void at_sim_init()
diff --git a/include/sim.h b/include/sim.h
index 1da486c..90bedae 100644
--- a/include/sim.h
+++ b/include/sim.h
@@ -64,6 +64,12 @@ typedef void (*ofono_sim_file_read_cb_t)(int ok,
 
 typedef void (*ofono_sim_file_write_cb_t)(int ok, void *userdata);
 
+typedef void (*ofono_sim_passwd_cb_t)(const struct ofono_error *error,
+					int passwd_type, void *data);
+
+typedef void (*ofono_sim_passwd_send_cb_t)(const struct ofono_error *error,
+					void *data);
+
 struct ofono_sim_driver {
 	const char *name;
 	int (*probe)(struct ofono_sim *sim, unsigned int vendor, void *data);
@@ -90,6 +96,13 @@ struct ofono_sim_driver {
 			ofono_sim_write_cb_t cb, void *data);
 	void (*read_imsi)(struct ofono_sim *sim,
 			ofono_sim_imsi_cb_t cb, void *data);
+	void (*query_passwd_state)(struct ofono_sim *sim,
+			ofono_sim_passwd_cb_t cb, void *data);
+	void (*send_passwd)(struct ofono_sim *sim, const char *passwd,
+			ofono_sim_passwd_send_cb_t cb, void *data);
+	void (*send_new_passwd)(struct ofono_sim *sim, const char *puk,
+			const char *passwd,
+			ofono_sim_passwd_send_cb_t cb, void *data);
 };
 
 int ofono_sim_driver_register(const struct ofono_sim_driver *d);
diff --git a/include/types.h b/include/types.h
index 6a9681d..99e2ca5 100644
--- a/include/types.h
+++ b/include/types.h
@@ -91,6 +91,26 @@ struct ofono_call {
 	int clip_validity;
 };
 
+enum ofono_sim_passwd_type {
+	OFONO_PASSWD_NONE = 0,
+
+	OFONO_PASSWD_SIM_PIN,
+	OFONO_PASSWD_SIM_PUK,
+	OFONO_PASSWD_PHSIM_PIN,
+	OFONO_PASSWD_PHFSIM_PIN,
+	OFONO_PASSWD_PHFSIM_PUK,
+	OFONO_PASSWD_SIM_PIN2,
+	OFONO_PASSWD_SIM_PUK2,
+	OFONO_PASSWD_PHNET_PIN,
+	OFONO_PASSWD_PHNET_PUK,
+	OFONO_PASSWD_PHNETSUB_PIN,
+	OFONO_PASSWD_PHNETSUB_PUK,
+	OFONO_PASSWD_PHSP_PIN,
+	OFONO_PASSWD_PHSP_PUK,
+	OFONO_PASSWD_PHCORP_PIN,
+	OFONO_PASSWD_PHCORP_PUK,
+};
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/sim.c b/src/sim.c
index a072c8c..44f0a3b 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -55,6 +55,7 @@ static GSList *g_drivers = NULL;
 static gboolean sim_op_next(gpointer user_data);
 static gboolean sim_op_retrieve_next(gpointer user);
 static void sim_own_numbers_update(struct ofono_sim *sim);
+static void sim_pin_check(struct ofono_sim *sim);
 
 struct sim_file_op {
 	int id;
@@ -77,6 +78,8 @@ struct ofono_sim {
 	GSList *service_numbers;
 	gboolean sdn_ready;
 	gboolean ready;
+	gboolean pin_ready;
+	int pin_type;
 	GQueue *simop_q;
 	gint simop_source;
 	unsigned char efmsisdn_length;
@@ -86,6 +89,7 @@ struct ofono_sim {
 	const struct ofono_sim_driver *driver;
 	void *driver_data;
 	struct ofono_atom *atom;
+	DBusMessage *pending;
 };
 
 struct msisdn_set_request {
@@ -95,6 +99,7 @@ struct msisdn_set_request {
 	DBusMessage *msg;
 };
 
+
 struct sim_ready_watch {
 	unsigned int id;
 	ofono_sim_ready_notify_cb_t notify;
@@ -171,6 +176,8 @@ static DBusMessage *sim_get_properties(DBusConnection *conn,
 	DBusMessageIter dict;
 	char **own_numbers;
 	char **service_numbers;
+	dbus_bool_t pin_needed;
+	const char *pin_name;
 
 	reply = dbus_message_new_method_return(msg);
 	if (!reply)
@@ -205,6 +212,18 @@ static DBusMessage *sim_get_properties(DBusConnection *conn,
 		g_strfreev(service_numbers);
 	}
 
+	pin_needed = !sim->pin_ready;
+	ofono_dbus_dict_append(&dict, "WaitingForPIN",
+				DBUS_TYPE_BOOLEAN, &pin_needed);
+
+	if (sim->pin_type != OFONO_PASSWD_NONE) {
+		pin_name = sim_passwd_name(sim->pin_type);
+
+		ofono_dbus_dict_append(&dict, "PINTypeRequired",
+					DBUS_TYPE_STRING,
+					(void *) &pin_name);
+	}
+
 	dbus_message_iter_close_container(&iter, &dict);
 
 	return reply;
@@ -353,10 +372,64 @@ error:
 	return __ofono_error_invalid_args(msg);
 }
 
+static void sim_send_pin_cb(const struct ofono_error *error, void *data)
+{
+	struct ofono_sim *sim = data;
+	DBusMessage *reply;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+		reply = __ofono_error_failed(sim->pending);
+	else
+		reply = dbus_message_new_method_return(sim->pending);
+
+
+	__ofono_dbus_pending_reply(&sim->pending, reply);
+
+	/* Re-check the authentication status and update DBus properties
+	 * as needed.  We need to do this regardless of whether the password
+	 * given was correct: if it was incorrect, the number of retries
+	 * may have expired and a different password is being requested now.
+	 * If it was correct, possibly proceed with SIM initialisation.
+	 */
+	sim_pin_check(sim);
+}
+
+static DBusMessage *sim_send_pin(DBusConnection *conn, DBusMessage *msg,
+					void *data)
+{
+	struct ofono_sim *sim = data;
+	DBusMessageIter iter;
+	const char *pin;
+
+	if (!sim->driver->send_passwd)
+		return __ofono_error_not_implemented(msg);
+
+	if (sim->pending)
+		return __ofono_error_busy(msg);
+
+	if (!dbus_message_iter_init(msg, &iter))
+		return __ofono_error_invalid_args(msg);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &pin);
+
+	if (!is_valid_pin(pin))
+		return __ofono_error_invalid_format(msg);
+
+	sim->pending = dbus_message_ref(msg);
+	sim->driver->send_passwd(sim, pin, sim_send_pin_cb, sim);
+
+	return NULL;
+}
+
 static GDBusMethodTable sim_methods[] = {
 	{ "GetProperties",	"",	"a{sv}",	sim_get_properties	},
 	{ "SetProperty",	"sv",	"",		sim_set_property,
 							G_DBUS_METHOD_FLAG_ASYNC },
+	{ "SendPIN",	"s",	"",			sim_send_pin,
+							G_DBUS_METHOD_FLAG_ASYNC },
 	{ }
 };
 
@@ -612,6 +685,67 @@ static void sim_retrieve_imsi(struct ofono_sim *sim)
 	sim->driver->read_imsi(sim, sim_imsi_cb, sim);
 }
 
+static void sim_pin_check_done(struct ofono_sim *sim)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(sim->atom);
+	dbus_bool_t value = FALSE;
+
+	if (sim->pin_ready == FALSE) {
+		sim->pin_ready = TRUE;
+
+		ofono_dbus_signal_property_changed(conn, path,
+							SIM_MANAGER_INTERFACE,
+							"WaitingForPIN",
+							DBUS_TYPE_BOOLEAN,
+							&value);
+	}
+
+	sim_retrieve_imsi(sim);
+}
+
+static void sim_pin_query_cb(const struct ofono_error *error, int pin_type,
+		void *data)
+{
+	struct ofono_sim *sim = data;
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(sim->atom);
+	const char *pin_name;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_error("Querrying PIN authentication state failed");
+
+		sim_pin_check_done(sim);
+		return;
+	}
+
+	if (sim->pin_type != pin_type) {
+		sim->pin_type = pin_type;
+		pin_name = sim_passwd_name(pin_type);
+
+		ofono_dbus_signal_property_changed(conn, path,
+							SIM_MANAGER_INTERFACE,
+							"PINTypeRequired",
+							DBUS_TYPE_STRING,
+							&pin_name);
+	}
+
+	if (pin_type == OFONO_PASSWD_NONE)
+		sim_pin_check_done(sim);
+}
+
+static void sim_pin_check(struct ofono_sim *sim)
+{
+	if (!sim->driver->query_passwd_state) {
+		sim_pin_check_done(sim);
+		return;
+	}
+
+	sim->pin_ready = FALSE;
+
+	sim->driver->query_passwd_state(sim, sim_pin_query_cb, sim);
+}
+
 static void sim_op_error(struct ofono_sim *sim)
 {
 	struct sim_file_op *op = g_queue_pop_head(sim->simop_q);
@@ -1312,7 +1446,7 @@ void ofono_sim_register(struct ofono_sim *sim)
 	 * arbitrary files to be written or read, assuming their presence
 	 * in the EFust
 	 */
-	sim_retrieve_imsi(sim);
+	sim_pin_check(sim);
 }
 
 void ofono_sim_remove(struct ofono_sim *sim)
diff --git a/src/simutil.c b/src/simutil.c
index b80e014..2b26700 100644
--- a/src/simutil.c
+++ b/src/simutil.c
@@ -488,3 +488,27 @@ void sim_adn_build(unsigned char *data, int length,
 	/* Ext1 unused */
 	*data++ = 0xff;
 }
+
+static const char *const passwd_name[] = {
+	[OFONO_PASSWD_NONE] = "None",
+	[OFONO_PASSWD_SIM_PIN] = "SIM PIN",
+	[OFONO_PASSWD_SIM_PUK] = "SIM PUK",
+	[OFONO_PASSWD_PHSIM_PIN] = "PH-SIM PIN",
+	[OFONO_PASSWD_PHFSIM_PIN] = "PH-FSIM PIN",
+	[OFONO_PASSWD_PHFSIM_PUK] = "PH-FSIM PUK",
+	[OFONO_PASSWD_SIM_PIN2] = "SIM PIN2",
+	[OFONO_PASSWD_SIM_PUK2] = "SIM PUK2",
+	[OFONO_PASSWD_PHNET_PIN] = "PH-NET PIN",
+	[OFONO_PASSWD_PHNET_PUK] = "PH-NET PUK",
+	[OFONO_PASSWD_PHNETSUB_PIN] = "PH-NETSUB PIN",
+	[OFONO_PASSWD_PHNETSUB_PUK] = "PH-NETSUB PUK",
+	[OFONO_PASSWD_PHSP_PIN] = "PH-SP PIN",
+	[OFONO_PASSWD_PHSP_PUK] = "PH-SP PUK",
+	[OFONO_PASSWD_PHCORP_PIN] = "PH-CORP PIN",
+	[OFONO_PASSWD_PHCORP_PUK] = "PH-CORP PUK",
+};
+
+const char *sim_passwd_name(enum ofono_sim_passwd_type type)
+{
+	return passwd_name[type];
+}
diff --git a/src/simutil.h b/src/simutil.h
index de3e55e..fc4f063 100644
--- a/src/simutil.h
+++ b/src/simutil.h
@@ -88,3 +88,5 @@ gboolean sim_adn_parse(const unsigned char *data, int length,
 void sim_adn_build(unsigned char *data, int length,
 			const struct ofono_phone_number *ph,
 			const char *identifier);
+
+const char *sim_passwd_name(enum ofono_sim_passwd_type type);
-- 
1.6.1

Re: [PATCH] [RFC] Do the PIN check in SIMManager

[Andrzej Zaborowski]

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

And here's a version that implements most of what's specified in
doc/sim-api.txt.  There are two caveats: when you re-enable SIM PIN
authentication, you have to give the old password from before it was
changed to "", i.e to disable and enable SIM PIN, do:
SimManager.ChangePin("pin", "1234", "")
SimManager.ChangePin("pin", "1234", "1234")

rather than the more logical

SimManager.ChangePin("pin", "1234", "")
SimManager.ChangePin("pin, "", "1234)

The other problem is that if changing the password succeeds but
enabling authentiation fails, ChangePin still returns failure.

Regards

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Do-PIN-authentication.patch --]
[-- Type: text/x-patch, Size: 20303 bytes --]

From 7209a4406224a0c4f8ad1c241ea355ee00e1fb5b Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Thu, 17 Sep 2009 22:43:22 +0200
Subject: [PATCH 2/2] Do PIN authentication

This adds checking whether PIN is required during SIM initialisation and
delaying the sim ready notifications until after correct PIN is given.
---
 drivers/atmodem/sim.c |  241 +++++++++++++++++++++++++++++++++++++++++
 include/sim.h         |   19 ++++
 include/types.h       |   20 ++++
 src/sim.c             |  286 ++++++++++++++++++++++++++++++++++++++++++++++++-
 src/simutil.c         |   36 ++++++
 src/simutil.h         |    3 +
 6 files changed, 604 insertions(+), 1 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index c629ec1..b6f34c2 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -398,6 +398,242 @@ error:
 	CALLBACK_WITH_FAILURE(cb, NULL, data);
 }
 
+static struct {
+	enum ofono_passwd_type type;
+	const char *name;
+} const at_sim_name[] = {
+	{ OFONO_PASSWD_SIM_PIN, "SIM PIN" },
+	{ OFONO_PASSWD_SIM_PUK, "SIM PUK" },
+	{ OFONO_PASSWD_PHSIM_PIN, "PH-SIM PIN" },
+	{ OFONO_PASSWD_PHFSIM_PIN, "PH-FSIM PIN" },
+	{ OFONO_PASSWD_PHFSIM_PUK, "PH-FSIM PUK" },
+	{ OFONO_PASSWD_SIM_PIN2, "SIM PIN2" },
+	{ OFONO_PASSWD_SIM_PUK2, "SIM PUK2" },
+	{ OFONO_PASSWD_PHNET_PIN, "PH-NET PIN" },
+	{ OFONO_PASSWD_PHNET_PUK, "PH-NET PUK" },
+	{ OFONO_PASSWD_PHNETSUB_PIN, "PH-NETSUB PIN" },
+	{ OFONO_PASSWD_PHNETSUB_PUK, "PH-NETSUB PUK" },
+	{ OFONO_PASSWD_PHSP_PIN, "PH-SP PIN" },
+	{ OFONO_PASSWD_PHSP_PUK, "PH-SP PUK" },
+	{ OFONO_PASSWD_PHCORP_PIN, "PH-CORP PIN" },
+	{ OFONO_PASSWD_PHCORP_PUK, "PH-CORP PUK" },
+};
+
+static void at_cpin_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	GAtResultIter iter;
+	ofono_sim_passwd_cb_t cb = cbd->cb;
+	struct ofono_error error;
+	const char *pin_required;
+	int pin_type;
+	int i;
+	int len = sizeof(at_sim_name) / sizeof(*at_sim_name);
+
+	dump_response("at_cpin_cb", ok, result);
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok) {
+		cb(&error, -1, cbd->data);
+		return;
+	}
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CPIN:")) {
+		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+		return;
+	}
+
+	g_at_result_iter_next_unquoted_string(&iter, &pin_required);
+
+	pin_type = -1;
+	if (!strcmp(pin_required, "READY"))
+		pin_type = OFONO_PASSWD_NONE;
+	else
+		for (i = 0; i < len; i++)
+			if (!strcmp(pin_required, at_sim_name[i].name)) {
+				pin_type = at_sim_name[i].type;
+				break;
+			}
+
+	if (pin_type == -1) {
+		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+		return;
+	}
+
+	ofono_debug("crsm_pin_cb: %s", pin_required);
+
+	cb(&error, pin_type, cbd->data);
+}
+
+static void at_pin_query(struct ofono_sim *sim, ofono_sim_passwd_cb_t cb,
+			void *data)
+{
+	GAtChat *chat = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, data);
+
+	if (!cbd)
+		goto error;
+
+	if (g_at_chat_send(chat, "AT+CPIN?", NULL,
+				at_cpin_cb, cbd, g_free) > 0)
+		return;
+
+error:
+	if (cbd)
+		g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, -1, data);
+}
+
+static void at_lock_unlock_cb(gboolean ok, GAtResult *result,
+				gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_sim_lock_unlock_cb_t cb = cbd->cb;
+	struct ofono_error error;
+
+	dump_response("at_lock_unlock_cb", ok, result);
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	cb(&error, cbd->data);
+}
+
+static void at_pin_send(struct ofono_sim *sim, const char *passwd,
+			ofono_sim_lock_unlock_cb_t cb, void *data)
+{
+	GAtChat *chat = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	char buf[64];
+	int ret;
+
+	if (!cbd)
+		goto error;
+
+	snprintf(buf, sizeof(buf), "AT+CPIN=\"%s\"", passwd);
+
+	ret = g_at_chat_send(chat, buf, NULL, at_lock_unlock_cb, cbd, g_free);
+
+	memset(buf, 0, sizeof(buf));
+
+	if (ret > 0)
+		return;
+
+error:
+	if (cbd)
+		g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
+static void at_pin_send_puk(struct ofono_sim *sim, const char *puk,
+				const char *passwd,
+				ofono_sim_lock_unlock_cb_t cb, void *data)
+{
+	GAtChat *chat = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	char buf[64];
+	int ret;
+
+	if (!cbd)
+		goto error;
+
+	snprintf(buf, sizeof(buf), "AT+CPIN=\"%s\",\"%s\"", puk, passwd);
+
+	ret = g_at_chat_send(chat, buf, NULL, at_lock_unlock_cb, cbd, g_free);
+
+	memset(buf, 0, sizeof(buf));
+
+	if (ret > 0)
+		return;
+
+error:
+	if (cbd)
+		g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
+static const char *const at_clck_cpwd_fac[] = {
+	[OFONO_PASSWD_SIM_PIN] = "SC",
+	[OFONO_PASSWD_SIM_PIN2] = "P2",
+	[OFONO_PASSWD_PHSIM_PIN] = "PS",
+	[OFONO_PASSWD_PHFSIM_PIN] = "PF",
+	[OFONO_PASSWD_PHNET_PIN] = "PN",
+	[OFONO_PASSWD_PHNETSUB_PIN] = "PU",
+	[OFONO_PASSWD_PHSP_PIN] = "PP",
+	[OFONO_PASSWD_PHCORP_PIN] = "PC",
+};
+
+static void at_pin_enable(struct ofono_sim *sim, int passwd_type, int enable,
+			const char *passwd,
+			ofono_sim_lock_unlock_cb_t cb, void *data)
+{
+	GAtChat *chat = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	char buf[64];
+	int ret;
+	int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
+
+	if (!cbd)
+		goto error;
+
+	if (passwd_type < 0 || passwd_type >= len ||
+			!at_clck_cpwd_fac[passwd_type])
+		goto error;
+
+	snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",%i,\"%s\"",
+			at_clck_cpwd_fac[passwd_type], enable ? 1 : 0, passwd);
+
+	ret = g_at_chat_send(chat, buf, NULL, at_lock_unlock_cb, cbd, g_free);
+
+	memset(buf, 0, sizeof(buf));
+
+	if (ret > 0)
+		return;
+
+error:
+	if (cbd)
+		g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
+static void at_change_passwd(struct ofono_sim *sim, int passwd_type,
+			const char *old, const char *new,
+			ofono_sim_lock_unlock_cb_t cb, void *data)
+{
+	GAtChat *chat = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	char buf[64];
+	int ret;
+	int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
+
+	if (!cbd)
+		goto error;
+
+	if (passwd_type < 0 || passwd_type >= len ||
+			!at_clck_cpwd_fac[passwd_type])
+		goto error;
+
+	snprintf(buf, sizeof(buf), "AT+CPWD=\"%s\",\"%s\",\"%s\"",
+			at_clck_cpwd_fac[passwd_type], old, new);
+
+	ret = g_at_chat_send(chat, buf, NULL, at_lock_unlock_cb, cbd, g_free);
+
+	memset(buf, 0, sizeof(buf));
+
+	if (ret > 0)
+		return;
+
+error:
+	if (cbd)
+		g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
 static gboolean at_sim_register(gpointer user)
 {
 	struct ofono_sim *sim = user;
@@ -434,6 +670,11 @@ static struct ofono_sim_driver driver = {
 	.write_file_linear	= at_sim_update_record,
 	.write_file_cyclic	= at_sim_update_cyclic,
 	.read_imsi		= at_read_imsi,
+	.query_passwd_state	= at_pin_query,
+	.send_passwd		= at_pin_send,
+	.reset_passwd		= at_pin_send_puk,
+	.lock			= at_pin_enable,
+	.change_passwd		= at_change_passwd,
 };
 
 void at_sim_init()
diff --git a/include/sim.h b/include/sim.h
index 1da486c..751ae10 100644
--- a/include/sim.h
+++ b/include/sim.h
@@ -64,6 +64,12 @@ typedef void (*ofono_sim_file_read_cb_t)(int ok,
 
 typedef void (*ofono_sim_file_write_cb_t)(int ok, void *userdata);
 
+typedef void (*ofono_sim_passwd_cb_t)(const struct ofono_error *error,
+					int passwd_type, void *data);
+
+typedef void (*ofono_sim_lock_unlock_cb_t)(const struct ofono_error *error,
+					void *data);
+
 struct ofono_sim_driver {
 	const char *name;
 	int (*probe)(struct ofono_sim *sim, unsigned int vendor, void *data);
@@ -90,6 +96,19 @@ struct ofono_sim_driver {
 			ofono_sim_write_cb_t cb, void *data);
 	void (*read_imsi)(struct ofono_sim *sim,
 			ofono_sim_imsi_cb_t cb, void *data);
+	void (*query_passwd_state)(struct ofono_sim *sim,
+			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,
+			ofono_sim_lock_unlock_cb_t cb, void *data);
+	void (*change_passwd)(struct ofono_sim *sim, int passwd_type,
+			const char *old, const char *new,
+			ofono_sim_lock_unlock_cb_t cb, void *data);
+	void (*lock)(struct ofono_sim *sim, int passwd_type, int enable,
+			const char *passwd,
+			ofono_sim_lock_unlock_cb_t cb, void *data);
 };
 
 int ofono_sim_driver_register(const struct ofono_sim_driver *d);
diff --git a/include/types.h b/include/types.h
index 6a9681d..82a3216 100644
--- a/include/types.h
+++ b/include/types.h
@@ -91,6 +91,26 @@ struct ofono_call {
 	int clip_validity;
 };
 
+enum ofono_passwd_type {
+	OFONO_PASSWD_NONE = 0,
+
+	OFONO_PASSWD_SIM_PIN,
+	OFONO_PASSWD_SIM_PUK,
+	OFONO_PASSWD_PHSIM_PIN,
+	OFONO_PASSWD_PHFSIM_PIN,
+	OFONO_PASSWD_PHFSIM_PUK,
+	OFONO_PASSWD_SIM_PIN2,
+	OFONO_PASSWD_SIM_PUK2,
+	OFONO_PASSWD_PHNET_PIN,
+	OFONO_PASSWD_PHNET_PUK,
+	OFONO_PASSWD_PHNETSUB_PIN,
+	OFONO_PASSWD_PHNETSUB_PUK,
+	OFONO_PASSWD_PHSP_PIN,
+	OFONO_PASSWD_PHSP_PUK,
+	OFONO_PASSWD_PHCORP_PIN,
+	OFONO_PASSWD_PHCORP_PUK,
+};
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/sim.c b/src/sim.c
index 3f26678..962f366 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -55,6 +55,7 @@ static GSList *g_drivers = NULL;
 static gboolean sim_op_next(gpointer user_data);
 static gboolean sim_op_retrieve_next(gpointer user);
 static void sim_own_numbers_update(struct ofono_sim *sim);
+static void sim_pin_check(struct ofono_sim *sim);
 
 struct sim_file_op {
 	int id;
@@ -77,6 +78,7 @@ struct ofono_sim {
 	GSList *service_numbers;
 	gboolean sdn_ready;
 	gboolean ready;
+	int pin_type;
 	char **language_prefs;
 	GQueue *simop_q;
 	gint simop_source;
@@ -89,6 +91,7 @@ struct ofono_sim {
 	const struct ofono_sim_driver *driver;
 	void *driver_data;
 	struct ofono_atom *atom;
+	DBusMessage *pending;
 };
 
 struct msisdn_set_request {
@@ -98,6 +101,12 @@ struct msisdn_set_request {
 	DBusMessage *msg;
 };
 
+struct pin_enable_request {
+	struct ofono_sim *sim;
+	int type;
+	char *passwd;
+};
+
 struct service_number {
 	char *id;
 	struct ofono_phone_number ph;
@@ -167,6 +176,7 @@ static DBusMessage *sim_get_properties(DBusConnection *conn,
 	DBusMessageIter dict;
 	char **own_numbers;
 	char **service_numbers;
+	const char *pin_name;
 
 	reply = dbus_message_new_method_return(msg);
 	if (!reply)
@@ -206,6 +216,11 @@ static DBusMessage *sim_get_properties(DBusConnection *conn,
 		                                DBUS_TYPE_STRING,
 		                                &sim->language_prefs);
 
+	pin_name = sim_passwd_name(sim->pin_type);
+	ofono_dbus_dict_append(&dict, "PinRequired",
+				DBUS_TYPE_STRING,
+				(void *) &pin_name);
+
 	dbus_message_iter_close_container(&iter, &dict);
 
 	return reply;
@@ -354,10 +369,234 @@ error:
 	return __ofono_error_invalid_args(msg);
 }
 
+static void sim_enable_pin_cb(const struct ofono_error *error, void *data)
+{
+	struct ofono_sim *sim = data;
+	DBusMessage *reply;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+		reply = __ofono_error_failed(sim->pending);
+	else
+		reply = dbus_message_new_method_return(sim->pending);
+
+	__ofono_dbus_pending_reply(&sim->pending, reply);
+}
+
+static void sim_change_pin_cb(const struct ofono_error *error, void *data)
+{
+	struct pin_enable_request *req = data;
+	struct ofono_sim *sim = req->sim;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		__ofono_dbus_pending_reply(&sim->pending,
+				__ofono_error_failed(sim->pending));
+		goto cleanup;
+	}
+
+	if (!sim->driver->lock) {
+		__ofono_dbus_pending_reply(&sim->pending,
+				dbus_message_new_method_return(sim->pending));
+		goto cleanup;
+	}
+
+	sim->driver->lock(sim, req->type, 1, req->passwd,
+				sim_enable_pin_cb, sim);
+
+cleanup:
+	memset(req->passwd, 0, strlen(req->passwd));
+	g_free(req->passwd);
+	g_free(req);
+}
+
+static DBusMessage *sim_change_pin(DBusConnection *conn, DBusMessage *msg,
+					void *data)
+{
+	struct ofono_sim *sim = data;
+	struct pin_enable_request *req;
+	DBusMessageIter iter;
+	enum ofono_passwd_type type;
+	const char *typestr;
+	const char *old;
+	const char *new;
+
+	if (sim->pending)
+		return __ofono_error_busy(msg);
+
+	if (!dbus_message_iter_init(msg, &iter))
+		return __ofono_error_invalid_args(msg);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &typestr);
+
+	type = sim_string_to_passwd(typestr);
+	if (type == OFONO_PASSWD_NONE)
+		return __ofono_error_invalid_format(msg);
+
+	dbus_message_iter_next(&iter);
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &old);
+
+	if (!is_valid_pin(old))
+		return __ofono_error_invalid_format(msg);
+
+	dbus_message_iter_next(&iter);
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &new);
+
+	if (new[0] != '\0' && strcmp(new, old)) {
+		if (!sim->driver->change_passwd)
+			return __ofono_error_not_implemented(msg);
+
+		if (!is_valid_pin(new))
+			return __ofono_error_invalid_format(msg);
+
+		req = g_new0(struct pin_enable_request, 1);
+
+		req->sim = sim;
+		req->type = type;
+		req->passwd = g_strdup(new);
+
+		sim->pending = dbus_message_ref(msg);
+
+		sim->driver->change_passwd(sim, type, old, new,
+						sim_change_pin_cb, req);
+	} else {
+		if (!sim->driver->lock)
+			return __ofono_error_not_implemented(msg);
+
+		sim->pending = dbus_message_ref(msg);
+		sim->driver->lock(sim, type, new[0] != '\0',
+					old, sim_enable_pin_cb, sim);
+	}
+
+	return NULL;
+}
+
+static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
+{
+	struct ofono_sim *sim = data;
+	DBusMessage *reply;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+		reply = __ofono_error_failed(sim->pending);
+	else
+		reply = dbus_message_new_method_return(sim->pending);
+
+	__ofono_dbus_pending_reply(&sim->pending, reply);
+
+	sim_pin_check(sim);
+}
+
+static DBusMessage *sim_enter_pin(DBusConnection *conn, DBusMessage *msg,
+					void *data)
+{
+	struct ofono_sim *sim = data;
+	DBusMessageIter iter;
+	const char *typestr;
+	int type;
+	const char *pin;
+
+	if (!sim->driver->send_passwd)
+		return __ofono_error_not_implemented(msg);
+
+	if (sim->pending)
+		return __ofono_error_busy(msg);
+
+	if (!dbus_message_iter_init(msg, &iter))
+		return __ofono_error_invalid_args(msg);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &typestr);
+
+	type = sim_string_to_passwd(typestr);
+	if (type == OFONO_PASSWD_NONE || type != sim->pin_type)
+		return __ofono_error_invalid_format(msg);
+
+	dbus_message_iter_next(&iter);
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &pin);
+
+	if (!is_valid_pin(pin))
+		return __ofono_error_invalid_format(msg);
+
+	sim->pending = dbus_message_ref(msg);
+	sim->driver->send_passwd(sim, pin, sim_enter_pin_cb, sim);
+
+	return NULL;
+}
+
+static DBusMessage *sim_reset_pin(DBusConnection *conn, DBusMessage *msg,
+					void *data)
+{
+	struct ofono_sim *sim = data;
+	DBusMessageIter iter;
+	const char *typestr;
+	int type;
+	const char *puk;
+	const char *pin;
+
+	if (!sim->driver->reset_passwd)
+		return __ofono_error_not_implemented(msg);
+
+	if (sim->pending)
+		return __ofono_error_busy(msg);
+
+	if (!dbus_message_iter_init(msg, &iter))
+		return __ofono_error_invalid_args(msg);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &typestr);
+
+	type = sim_string_to_passwd(typestr);
+	if (type == OFONO_PASSWD_NONE || type != sim->pin_type)
+		return __ofono_error_invalid_format(msg);
+
+	dbus_message_iter_next(&iter);
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &puk);
+
+	if (!is_valid_pin(puk))
+		return __ofono_error_invalid_format(msg);
+
+	dbus_message_iter_next(&iter);
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &pin);
+
+	if (!is_valid_pin(pin))
+		return __ofono_error_invalid_format(msg);
+
+	sim->pending = dbus_message_ref(msg);
+	sim->driver->reset_passwd(sim, puk, pin, sim_enter_pin_cb, sim);
+
+	return NULL;
+}
+
 static GDBusMethodTable sim_methods[] = {
 	{ "GetProperties",	"",	"a{sv}",	sim_get_properties	},
 	{ "SetProperty",	"sv",	"",		sim_set_property,
 							G_DBUS_METHOD_FLAG_ASYNC },
+	{ "ChangePin",		"sss",	"",		sim_change_pin,
+							G_DBUS_METHOD_FLAG_ASYNC },
+	{ "EnterPin",		"ss",	"",		sim_enter_pin,
+							G_DBUS_METHOD_FLAG_ASYNC },
+	{ "ResetPin",		"sss",	"",		sim_reset_pin,
+							G_DBUS_METHOD_FLAG_ASYNC },
 	{ }
 };
 
@@ -613,6 +852,51 @@ static void sim_retrieve_imsi(struct ofono_sim *sim)
 	sim->driver->read_imsi(sim, sim_imsi_cb, sim);
 }
 
+static void sim_pin_check_done(struct ofono_sim *sim)
+{
+	sim_retrieve_imsi(sim);
+}
+
+static void sim_pin_query_cb(const struct ofono_error *error, int pin_type,
+		void *data)
+{
+	struct ofono_sim *sim = data;
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(sim->atom);
+	const char *pin_name;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_error("Querying PIN authentication state failed");
+
+		goto checkdone;
+	}
+
+	if (sim->pin_type != pin_type) {
+		sim->pin_type = pin_type;
+		pin_name = sim_passwd_name(pin_type);
+
+		ofono_dbus_signal_property_changed(conn, path,
+							SIM_MANAGER_INTERFACE,
+							"PinRequired",
+							DBUS_TYPE_STRING,
+							&pin_name);
+	}
+
+checkdone:
+	if (pin_type == OFONO_PASSWD_NONE)
+		sim_pin_check_done(sim);
+}
+
+static void sim_pin_check(struct ofono_sim *sim)
+{
+	if (!sim->driver->query_passwd_state) {
+		sim_pin_check_done(sim);
+		return;
+	}
+
+	sim->driver->query_passwd_state(sim, sim_pin_query_cb, sim);
+}
+
 static void sim_efli_read_cb(int ok,
 		                enum ofono_sim_file_structure structure,
 		                int length, int record,
@@ -1469,7 +1753,7 @@ void ofono_sim_register(struct ofono_sim *sim)
 	 * in the EFust
 	 */
 	sim_retrieve_efli(sim);
-	sim_retrieve_imsi(sim);
+	sim_pin_check(sim);
 }
 
 void ofono_sim_remove(struct ofono_sim *sim)
diff --git a/src/simutil.c b/src/simutil.c
index b80e014..e1b0f2a 100644
--- a/src/simutil.c
+++ b/src/simutil.c
@@ -488,3 +488,39 @@ void sim_adn_build(unsigned char *data, int length,
 	/* Ext1 unused */
 	*data++ = 0xff;
 }
+
+static const char *const passwd_name[] = {
+	[OFONO_PASSWD_NONE] = "none",
+	[OFONO_PASSWD_SIM_PIN] = "pin",
+	[OFONO_PASSWD_SIM_PUK] = "puk",
+	[OFONO_PASSWD_PHSIM_PIN] = "phone",
+	[OFONO_PASSWD_PHFSIM_PIN] = "firstphone",
+	[OFONO_PASSWD_PHFSIM_PUK] = "firstphonepuk",
+	[OFONO_PASSWD_SIM_PIN2] = "pin2",
+	[OFONO_PASSWD_SIM_PUK2] = "puk2",
+	[OFONO_PASSWD_PHNET_PIN] = "network",
+	[OFONO_PASSWD_PHNET_PUK] = "networkpuk",
+	[OFONO_PASSWD_PHNETSUB_PIN] = "netsub",
+	[OFONO_PASSWD_PHNETSUB_PUK] = "netsubpuk",
+	[OFONO_PASSWD_PHSP_PIN] = "service",
+	[OFONO_PASSWD_PHSP_PUK] = "servicepuk",
+	[OFONO_PASSWD_PHCORP_PIN] = "corp",
+	[OFONO_PASSWD_PHCORP_PUK] = "corppuk",
+};
+
+const char *sim_passwd_name(enum ofono_passwd_type type)
+{
+	return passwd_name[type];
+}
+
+enum ofono_passwd_type sim_string_to_passwd(const char *name)
+{
+	int len = sizeof(passwd_name) / sizeof(*passwd_name);
+	int i;
+
+	for (i = 0; i < len; i++)
+		if (!strcmp(passwd_name[i], name))
+			return i;
+
+	return OFONO_PASSWD_NONE;
+}
diff --git a/src/simutil.h b/src/simutil.h
index 2cd3b73..df84416 100644
--- a/src/simutil.h
+++ b/src/simutil.h
@@ -93,3 +93,6 @@ gboolean sim_adn_parse(const unsigned char *data, int length,
 void sim_adn_build(unsigned char *data, int length,
 			const struct ofono_phone_number *ph,
 			const char *identifier);
+
+const char *sim_passwd_name(enum ofono_passwd_type type);
+enum ofono_passwd_type sim_string_to_passwd(const char *name);
-- 
1.6.1

Re: [PATCH] [RFC] Do the PIN check in SIMManager

[Denis Kenzior]

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

Hi Andrew,

> And here's a version that implements most of what's specified in
> doc/sim-api.txt.  There are two caveats: when you re-enable SIM PIN

Patch has been applied, thanks.

> authentication, you have to give the old password from before it was
> changed to "", i.e to disable and enable SIM PIN, do:
> SimManager.ChangePin("pin", "1234", "")
> SimManager.ChangePin("pin", "1234", "1234")
>
> rather than the more logical
>
> SimManager.ChangePin("pin", "1234", "")
> SimManager.ChangePin("pin, "", "1234)
>
> The other problem is that if changing the password succeeds but
> enabling authentiation fails, ChangePin still returns failure

I added LockPin and UnlockPin methods to take care of these two cases.

Regards,
-Denis

Re: [PATCH] [RFC] Do the PIN check in SIMManager

[Denis Kenzior]

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

Hi Andrew,

> Should we have a read-only property so clients know what the current
> state is?  Unfortunately this would be some 6 properties and 6 more

Actually more like 7.  However, right now we only really care about pin lock.  
We can theoretically optimize the query somewhat by looking at whether the 
CPIN is being asked for at startup.  If it is, the PIN is locked, and unlocked 
otherwise.  Then the LockPin / UnlockPin callbacks can update the state 
appropriately.

So the thought right now is to add LockedPins property, which will be a list 
of pins that are currently locked.  If nothing is locked, an empty list is 
returned.

For now I'm planning to implement LockedPins support for pin only, but the 
others will have to be queried eventually as well.

> commands on startup, for little gain.  I attached a diff to add plugin
> interface for this without dbus interface.  The other two changes are
> just clean-up.

Patches have been applied, thanks.

Regards,
-Denis

Re: [PATCH] [RFC] Do the PIN check in SIMManager

[andrzej zaborowski]

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

Hi,

2009/9/22 Denis Kenzior <denkenz@gmail.com>:
>> authentication, you have to give the old password from before it was
>> changed to "", i.e to disable and enable SIM PIN, do:
>> SimManager.ChangePin("pin", "1234", "")
>> SimManager.ChangePin("pin", "1234", "1234")
>>
>> rather than the more logical
>>
>> SimManager.ChangePin("pin", "1234", "")
>> SimManager.ChangePin("pin, "", "1234)
>>
>> The other problem is that if changing the password succeeds but
>> enabling authentiation fails, ChangePin still returns failure
>
> I added LockPin and UnlockPin methods to take care of these two cases.

Should we have a read-only property so clients know what the current
state is?  Unfortunately this would be some 6 properties and 6 more
commands on startup, for little gain.  I attached a diff to add plugin
interface for this without dbus interface.  The other two changes are
just clean-up.

Regards

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Update-sim-api.txt-ChangePin-no-longer-disables-pin.patch --]
[-- Type: text/x-patch, Size: 751 bytes --]

From 8f63f7860271f634a5106f68d10ff7331343dea5 Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Wed, 23 Sep 2009 23:17:06 +0200
Subject: [PATCH] Update sim-api.txt: ChangePin no longer disables pin.

---
 doc/sim-api.txt |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/doc/sim-api.txt b/doc/sim-api.txt
index caf2dc4..f87dbeb 100644
--- a/doc/sim-api.txt
+++ b/doc/sim-api.txt
@@ -14,8 +14,7 @@ Methods		dict GetProperties()
 
 		ChangePin(string type, string oldpin, string newpin)
 
-			Changes the pin given by string type.  If newpin is
-			empty, this has the effect of disabling the pin.
+			Changes the pin given by string type.
 
 		EnterPin(string type, string pin)
 
-- 
1.6.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0003-Add-a-plugin-interface-for-getting-PIN-lock-state.patch --]
[-- Type: text/x-patch, Size: 3494 bytes --]

From ab0131e797fe09049b31beb2a1bf622515eab78f Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Thu, 24 Sep 2009 00:33:28 +0200
Subject: [PATCH] Add plugin interface for getting PIN lock state.

It may be useful to have the information of whether card is currently
locked and emit events when this changes but if we want to have it as a
property, we would need properties for all types of locks and it wouldn't
be all that useful.
---
 drivers/atmodem/sim.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/sim.h         |    6 +++++
 2 files changed, 67 insertions(+), 0 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index b7d8368..d04b736 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -634,6 +634,66 @@ error:
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
+static void at_lock_status_cb(gboolean ok, GAtResult *result,
+		gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	GAtResultIter iter;
+	ofono_sim_locked_cb_t cb = cbd->cb;
+	struct ofono_error error;
+	int locked;
+
+	dump_response("at_lock_status_cb", ok, result);
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok) {
+		cb(&error, -1, cbd->data);
+		return;
+	}
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CLCK:")) {
+		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+		return;
+	}
+
+	g_at_result_iter_next_number(&iter, &locked);
+
+	ofono_debug("lock_status_cb: %i", locked);
+
+	cb(&error, locked, cbd->data);
+}
+
+static void at_pin_query_enabled(struct ofono_sim *sim,
+				enum ofono_sim_password_type passwd_type,
+				ofono_sim_locked_cb_t cb, void *data)
+{
+	GAtChat *chat = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	char buf[64];
+	unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
+
+	if (!cbd)
+		goto error;
+
+	if (passwd_type >= len || !at_clck_cpwd_fac[passwd_type])
+		goto error;
+
+	snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",2",
+			at_clck_cpwd_fac[passwd_type]);
+
+	if (g_at_chat_send(chat, buf, NULL,
+				at_lock_status_cb, cbd, g_free) > 0)
+		return;
+
+error:
+	if (cbd)
+		g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, -1, data);
+}
+
 static gboolean at_sim_register(gpointer user)
 {
 	struct ofono_sim *sim = user;
@@ -675,6 +735,7 @@ static struct ofono_sim_driver driver = {
 	.reset_passwd		= at_pin_send_puk,
 	.lock			= at_pin_enable,
 	.change_passwd		= at_change_passwd,
+	.query_locked		= at_pin_query_enabled,
 };
 
 void at_sim_init()
diff --git a/include/sim.h b/include/sim.h
index af2fd1e..fa5276b 100644
--- a/include/sim.h
+++ b/include/sim.h
@@ -91,6 +91,9 @@ typedef void (*ofono_sim_passwd_cb_t)(const struct ofono_error *error,
 typedef void (*ofono_sim_lock_unlock_cb_t)(const struct ofono_error *error,
 					void *data);
 
+typedef void (*ofono_sim_locked_cb_t)(const struct ofono_error *error,
+					int locked, void *data);
+
 struct ofono_sim_driver {
 	const char *name;
 	int (*probe)(struct ofono_sim *sim, unsigned int vendor, void *data);
@@ -131,6 +134,9 @@ struct ofono_sim_driver {
 	void (*lock)(struct ofono_sim *sim, enum ofono_sim_password_type type,
 			int enable, const char *passwd,
 			ofono_sim_lock_unlock_cb_t cb, void *data);
+	void (*query_locked)(struct ofono_sim *sim,
+			enum ofono_sim_password_type type,
+			ofono_sim_locked_cb_t cb, void *data);
 };
 
 int ofono_sim_driver_register(const struct ofono_sim_driver *d);
-- 
1.6.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0004-Remove-unused-struct-field.patch --]
[-- Type: text/x-patch, Size: 655 bytes --]

From 35546966ef1107115003481928aa12d6b472ce7d Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Thu, 24 Sep 2009 00:35:15 +0200
Subject: [PATCH] Remove unused struct field.

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

diff --git a/src/sim.c b/src/sim.c
index ec003ee..746c7a8 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -86,7 +86,6 @@ struct ofono_sim {
 	unsigned char efmsisdn_records;
 	unsigned char *efli;
 	unsigned char efli_length;
-	unsigned int next_ready_watch_id;
 	struct ofono_watchlist *ready_watches;
 	const struct ofono_sim_driver *driver;
 	void *driver_data;
-- 
1.6.1