[PATCH] sim: Sim PIN1 cache upon modem reset/crash

[PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Nandini Rebello]

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

Adding SIM PIN caching feature to oFono. oFono now caches the SIM PIN1 type
against the ICCID throughout its lifetime in a link list and enters
implicitly upon modem reset/crash.

Handles cases of incorrect pin and sim pin changed externally.

Adding to all modems by default.
---
 src/sim.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 114 insertions(+)

diff --git a/src/sim.c b/src/sim.c
index 129ff5d..28d9a00 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -141,6 +141,11 @@ struct ofono_sim {
 	bool wait_initialized : 1;
 };
 
+struct cached_pin {
+	char *id;
+	char *pin;
+};
+
 struct msisdn_set_request {
 	struct ofono_sim *sim;
 	int pending;
@@ -176,6 +181,8 @@ static void sim_own_numbers_update(struct ofono_sim *sim);
 
 static GSList *g_drivers = NULL;
 
+static GSList *cached_pins = NULL;
+
 static const char *sim_passwd_name(enum ofono_sim_password_type type)
 {
 	return passwd_name[type];
@@ -473,6 +480,75 @@ done:
 	return reply;
 }
 
+static struct cached_pin *pin_cache_lookup(const char *iccid)
+{
+	struct cached_pin *c;
+	GSList *l;
+
+	for (l = cached_pins; l; l = l->next) {
+		c = l->data;
+
+		if (g_strcmp0(iccid, c->id) == 0)
+			return c;
+	}
+
+	return NULL;
+}
+
+static gboolean pin_cache_update(const char *iccid, const char *pin)
+{
+	struct cached_pin *pin_cached = pin_cache_lookup(iccid);
+	struct cached_pin *cpins;
+
+	if (pin_cached != NULL) {
+		g_free(pin_cached->pin);
+		pin_cached->pin = g_strdup(pin);
+		return TRUE;
+	}
+
+	cpins = g_new0(struct cached_pin, 1);
+
+	if (cpins == NULL)
+		return FALSE;
+
+	cpins->id = g_strdup(iccid);
+	cpins->pin = g_strdup(pin);
+	cached_pins = g_slist_prepend(cached_pins, cpins);
+
+	return TRUE;
+}
+
+static void pin_cache_remove(const char *iccid)
+{
+	struct cached_pin *pin_cached = pin_cache_lookup(iccid);
+
+	if (pin_cached == NULL)
+		return;
+
+	cached_pins = g_slist_remove(cached_pins, pin_cached);
+}
+
+static void pin_cache_enter_cb(const struct ofono_error *error, void *data)
+{
+	struct ofono_sim *sim = data;
+
+	/* If PIN entry fails, then remove cached PIN*/
+	if (sim->initialized || error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		pin_cache_remove(sim->iccid);
+		goto recheck;
+	}
+
+	if (sim->pin_type == OFONO_SIM_PASSWORD_SIM_PIN ||
+			sim->pin_type == OFONO_SIM_PASSWORD_SIM_PUK) {
+		sim->wait_initialized = true;
+		DBG("Waiting for ofono_sim_initialized_notify");
+		return;
+	}
+
+recheck:
+	__ofono_sim_recheck_pin(sim);
+}
+
 static void sim_pin_retries_query_cb(const struct ofono_error *error,
 					int retries[OFONO_SIM_PASSWORD_INVALID],
 					void *data)
@@ -681,6 +757,13 @@ static void sim_locked_cb(struct ofono_sim *sim, gboolean locked)
 						OFONO_SIM_MANAGER_INTERFACE,
 						"LockedPins", DBUS_TYPE_STRING,
 						&locked_pins);
+
+	/*Cache pin only for SIM PIN type*/
+	if (g_strcmp0(typestr, "pin") == 0) {
+		if (!pin_cache_update(sim->iccid, pin))
+			ofono_error("Failed to cache PIN.");
+	}
+
 	g_strfreev(locked_pins);
 
 	sim_pin_retries_check(sim);
@@ -776,6 +859,14 @@ static DBusMessage *sim_unlock_pin(DBusConnection *conn, DBusMessage *msg,
 static void sim_change_pin_cb(const struct ofono_error *error, void *data)
 {
 	struct ofono_sim *sim = data;
+	const char *typestr;
+	const char *old;
+	const char *new;
+
+	dbus_message_get_args(sim->pending, NULL, DBUS_TYPE_STRING, &typestr,
+					     DBUS_TYPE_STRING, &old,
+					     DBUS_TYPE_STRING, &new,
+				             DBUS_TYPE_INVALID);
 
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
 		__ofono_dbus_pending_reply(&sim->pending,
@@ -786,6 +877,12 @@ static void sim_change_pin_cb(const struct ofono_error *error, void *data)
 		return;
 	}
 
+	/*Cache pin only for SIM PIN type*/
+	if (g_strcmp0(typestr, "pin") == 0) {
+		if (!pin_cache_update(sim->iccid, new))
+			ofono_error("Failed to cache PIN.");
+	}
+
 	__ofono_dbus_pending_reply(&sim->pending,
 				dbus_message_new_method_return(sim->pending));
 
@@ -837,8 +934,14 @@ static DBusMessage *sim_change_pin(DBusConnection *conn, DBusMessage *msg,
 static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
 {
 	struct ofono_sim *sim = data;
+	const char *typestr;
+	const char *pin;
 	DBusMessage *reply;
 
+	dbus_message_get_args(sim->pending, NULL, DBUS_TYPE_STRING, &typestr,
+					     DBUS_TYPE_STRING, &pin,
+					     DBUS_TYPE_INVALID);
+
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
 		reply = __ofono_error_failed(sim->pending);
 	else
@@ -850,6 +953,12 @@ static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
 	if (sim->initialized || error->type != OFONO_ERROR_TYPE_NO_ERROR)
 		goto recheck;
 
+	/*Cache pin only for SIM PIN type*/
+	if (g_strcmp0(typestr, "pin") == 0) {
+		if (!pin_cache_update(sim->iccid, pin))
+			ofono_error("Failed to cache PIN.");
+	}
+
 	if (sim->pin_type == OFONO_SIM_PASSWORD_SIM_PIN ||
 			sim->pin_type == OFONO_SIM_PASSWORD_SIM_PUK) {
 		sim->wait_initialized = true;
@@ -3023,6 +3132,7 @@ static void sim_pin_query_cb(const struct ofono_error *error,
 	struct ofono_sim *sim = data;
 	DBusConnection *conn = ofono_dbus_get_connection();
 	const char *path = __ofono_atom_get_path(sim->atom);
+	struct cached_pin *cpins = pin_cache_lookup(sim->iccid);
 	const char *pin_name;
 	char **locked_pins;
 	gboolean lock_changed;
@@ -3067,6 +3177,10 @@ static void sim_pin_query_cb(const struct ofono_error *error,
 						&pin_name);
 	}
 
+	if (g_strcmp0(pin_name, "pin") == 0 && cpins != NULL)
+		sim->driver->send_passwd(sim, cpins->pin,
+					       pin_cache_enter_cb, sim);
+
 	switch (pin_type) {
 	case OFONO_SIM_PASSWORD_NONE:
 	case OFONO_SIM_PASSWORD_SIM_PIN2:
-- 
2.7.4

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Giacinto Cifelli]

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

No, absolutely not!
It is forbidden to do this by the 3GPP!

On Wed, Dec 12, 2018 at 7:46 AM Nandini Rebello
<nandini.rebello@intel.com> wrote:
>
> Adding SIM PIN caching feature to oFono. oFono now caches the SIM PIN1 type
> against the ICCID throughout its lifetime in a link list and enters
> implicitly upon modem reset/crash.
>
> Handles cases of incorrect pin and sim pin changed externally.
>
> Adding to all modems by default.
> ---
>  src/sim.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 114 insertions(+)
>
> diff --git a/src/sim.c b/src/sim.c
> index 129ff5d..28d9a00 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -141,6 +141,11 @@ struct ofono_sim {
>         bool wait_initialized : 1;
>  };
>
> +struct cached_pin {
> +       char *id;
> +       char *pin;
> +};
> +
>  struct msisdn_set_request {
>         struct ofono_sim *sim;
>         int pending;
> @@ -176,6 +181,8 @@ static void sim_own_numbers_update(struct ofono_sim *sim);
>
>  static GSList *g_drivers = NULL;
>
> +static GSList *cached_pins = NULL;
> +
>  static const char *sim_passwd_name(enum ofono_sim_password_type type)
>  {
>         return passwd_name[type];
> @@ -473,6 +480,75 @@ done:
>         return reply;
>  }
>
> +static struct cached_pin *pin_cache_lookup(const char *iccid)
> +{
> +       struct cached_pin *c;
> +       GSList *l;
> +
> +       for (l = cached_pins; l; l = l->next) {
> +               c = l->data;
> +
> +               if (g_strcmp0(iccid, c->id) == 0)
> +                       return c;
> +       }
> +
> +       return NULL;
> +}
> +
> +static gboolean pin_cache_update(const char *iccid, const char *pin)
> +{
> +       struct cached_pin *pin_cached = pin_cache_lookup(iccid);
> +       struct cached_pin *cpins;
> +
> +       if (pin_cached != NULL) {
> +               g_free(pin_cached->pin);
> +               pin_cached->pin = g_strdup(pin);
> +               return TRUE;
> +       }
> +
> +       cpins = g_new0(struct cached_pin, 1);
> +
> +       if (cpins == NULL)
> +               return FALSE;
> +
> +       cpins->id = g_strdup(iccid);
> +       cpins->pin = g_strdup(pin);
> +       cached_pins = g_slist_prepend(cached_pins, cpins);
> +
> +       return TRUE;
> +}
> +
> +static void pin_cache_remove(const char *iccid)
> +{
> +       struct cached_pin *pin_cached = pin_cache_lookup(iccid);
> +
> +       if (pin_cached == NULL)
> +               return;
> +
> +       cached_pins = g_slist_remove(cached_pins, pin_cached);
> +}
> +
> +static void pin_cache_enter_cb(const struct ofono_error *error, void *data)
> +{
> +       struct ofono_sim *sim = data;
> +
> +       /* If PIN entry fails, then remove cached PIN*/
> +       if (sim->initialized || error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> +               pin_cache_remove(sim->iccid);
> +               goto recheck;
> +       }
> +
> +       if (sim->pin_type == OFONO_SIM_PASSWORD_SIM_PIN ||
> +                       sim->pin_type == OFONO_SIM_PASSWORD_SIM_PUK) {
> +               sim->wait_initialized = true;
> +               DBG("Waiting for ofono_sim_initialized_notify");
> +               return;
> +       }
> +
> +recheck:
> +       __ofono_sim_recheck_pin(sim);
> +}
> +
>  static void sim_pin_retries_query_cb(const struct ofono_error *error,
>                                         int retries[OFONO_SIM_PASSWORD_INVALID],
>                                         void *data)
> @@ -681,6 +757,13 @@ static void sim_locked_cb(struct ofono_sim *sim, gboolean locked)
>                                                 OFONO_SIM_MANAGER_INTERFACE,
>                                                 "LockedPins", DBUS_TYPE_STRING,
>                                                 &locked_pins);
> +
> +       /*Cache pin only for SIM PIN type*/
> +       if (g_strcmp0(typestr, "pin") == 0) {
> +               if (!pin_cache_update(sim->iccid, pin))
> +                       ofono_error("Failed to cache PIN.");
> +       }
> +
>         g_strfreev(locked_pins);
>
>         sim_pin_retries_check(sim);
> @@ -776,6 +859,14 @@ static DBusMessage *sim_unlock_pin(DBusConnection *conn, DBusMessage *msg,
>  static void sim_change_pin_cb(const struct ofono_error *error, void *data)
>  {
>         struct ofono_sim *sim = data;
> +       const char *typestr;
> +       const char *old;
> +       const char *new;
> +
> +       dbus_message_get_args(sim->pending, NULL, DBUS_TYPE_STRING, &typestr,
> +                                            DBUS_TYPE_STRING, &old,
> +                                            DBUS_TYPE_STRING, &new,
> +                                            DBUS_TYPE_INVALID);
>
>         if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
>                 __ofono_dbus_pending_reply(&sim->pending,
> @@ -786,6 +877,12 @@ static void sim_change_pin_cb(const struct ofono_error *error, void *data)
>                 return;
>         }
>
> +       /*Cache pin only for SIM PIN type*/
> +       if (g_strcmp0(typestr, "pin") == 0) {
> +               if (!pin_cache_update(sim->iccid, new))
> +                       ofono_error("Failed to cache PIN.");
> +       }
> +
>         __ofono_dbus_pending_reply(&sim->pending,
>                                 dbus_message_new_method_return(sim->pending));
>
> @@ -837,8 +934,14 @@ static DBusMessage *sim_change_pin(DBusConnection *conn, DBusMessage *msg,
>  static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
>  {
>         struct ofono_sim *sim = data;
> +       const char *typestr;
> +       const char *pin;
>         DBusMessage *reply;
>
> +       dbus_message_get_args(sim->pending, NULL, DBUS_TYPE_STRING, &typestr,
> +                                            DBUS_TYPE_STRING, &pin,
> +                                            DBUS_TYPE_INVALID);
> +
>         if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
>                 reply = __ofono_error_failed(sim->pending);
>         else
> @@ -850,6 +953,12 @@ static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
>         if (sim->initialized || error->type != OFONO_ERROR_TYPE_NO_ERROR)
>                 goto recheck;
>
> +       /*Cache pin only for SIM PIN type*/
> +       if (g_strcmp0(typestr, "pin") == 0) {
> +               if (!pin_cache_update(sim->iccid, pin))
> +                       ofono_error("Failed to cache PIN.");
> +       }
> +
>         if (sim->pin_type == OFONO_SIM_PASSWORD_SIM_PIN ||
>                         sim->pin_type == OFONO_SIM_PASSWORD_SIM_PUK) {
>                 sim->wait_initialized = true;
> @@ -3023,6 +3132,7 @@ static void sim_pin_query_cb(const struct ofono_error *error,
>         struct ofono_sim *sim = data;
>         DBusConnection *conn = ofono_dbus_get_connection();
>         const char *path = __ofono_atom_get_path(sim->atom);
> +       struct cached_pin *cpins = pin_cache_lookup(sim->iccid);
>         const char *pin_name;
>         char **locked_pins;
>         gboolean lock_changed;
> @@ -3067,6 +3177,10 @@ static void sim_pin_query_cb(const struct ofono_error *error,
>                                                 &pin_name);
>         }
>
> +       if (g_strcmp0(pin_name, "pin") == 0 && cpins != NULL)
> +               sim->driver->send_passwd(sim, cpins->pin,
> +                                              pin_cache_enter_cb, sim);
> +
>         switch (pin_type) {
>         case OFONO_SIM_PASSWORD_NONE:
>         case OFONO_SIM_PASSWORD_SIM_PIN2:
> --
> 2.7.4
>
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Pavel Machek]

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

On Wed 2018-12-12 08:59:34, Giacinto Cifelli wrote:
> No, absolutely not!
> It is forbidden to do this by the 3GPP!

Checking if 3GPP is allowed to make laws in this country.... no.

:-)

I'm not sure if it makes sense by default, but... I don't think 3GPP
has or should have power over ofono project.
								Pavel
								

> On Wed, Dec 12, 2018 at 7:46 AM Nandini Rebello
> <nandini.rebello@intel.com> wrote:
> >
> > Adding SIM PIN caching feature to oFono. oFono now caches the SIM PIN1 type
> > against the ICCID throughout its lifetime in a link list and enters
> > implicitly upon modem reset/crash.
> >
> > Handles cases of incorrect pin and sim pin changed externally.
> >
> > Adding to all modems by default.
> > ---
> >  src/sim.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 114 insertions(+)
> >
> > diff --git a/src/sim.c b/src/sim.c
> > index 129ff5d..28d9a00 100644
> > --- a/src/sim.c
> > +++ b/src/sim.c
> > @@ -141,6 +141,11 @@ struct ofono_sim {
> >         bool wait_initialized : 1;
> >  };
> >
> > +struct cached_pin {
> > +       char *id;
> > +       char *pin;
> > +};
> > +
> >  struct msisdn_set_request {
> >         struct ofono_sim *sim;
> >         int pending;
> > @@ -176,6 +181,8 @@ static void sim_own_numbers_update(struct ofono_sim *sim);
> >
> >  static GSList *g_drivers = NULL;
> >
> > +static GSList *cached_pins = NULL;
> > +
> >  static const char *sim_passwd_name(enum ofono_sim_password_type type)
> >  {
> >         return passwd_name[type];
> > @@ -473,6 +480,75 @@ done:
> >         return reply;
> >  }
> >
> > +static struct cached_pin *pin_cache_lookup(const char *iccid)
> > +{
> > +       struct cached_pin *c;
> > +       GSList *l;
> > +
> > +       for (l = cached_pins; l; l = l->next) {
> > +               c = l->data;
> > +
> > +               if (g_strcmp0(iccid, c->id) == 0)
> > +                       return c;
> > +       }
> > +
> > +       return NULL;
> > +}
> > +
> > +static gboolean pin_cache_update(const char *iccid, const char *pin)
> > +{
> > +       struct cached_pin *pin_cached = pin_cache_lookup(iccid);
> > +       struct cached_pin *cpins;
> > +
> > +       if (pin_cached != NULL) {
> > +               g_free(pin_cached->pin);
> > +               pin_cached->pin = g_strdup(pin);
> > +               return TRUE;
> > +       }
> > +
> > +       cpins = g_new0(struct cached_pin, 1);
> > +
> > +       if (cpins == NULL)
> > +               return FALSE;
> > +
> > +       cpins->id = g_strdup(iccid);
> > +       cpins->pin = g_strdup(pin);
> > +       cached_pins = g_slist_prepend(cached_pins, cpins);
> > +
> > +       return TRUE;
> > +}
> > +
> > +static void pin_cache_remove(const char *iccid)
> > +{
> > +       struct cached_pin *pin_cached = pin_cache_lookup(iccid);
> > +
> > +       if (pin_cached == NULL)
> > +               return;
> > +
> > +       cached_pins = g_slist_remove(cached_pins, pin_cached);
> > +}
> > +
> > +static void pin_cache_enter_cb(const struct ofono_error *error, void *data)
> > +{
> > +       struct ofono_sim *sim = data;
> > +
> > +       /* If PIN entry fails, then remove cached PIN*/
> > +       if (sim->initialized || error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> > +               pin_cache_remove(sim->iccid);
> > +               goto recheck;
> > +       }
> > +
> > +       if (sim->pin_type == OFONO_SIM_PASSWORD_SIM_PIN ||
> > +                       sim->pin_type == OFONO_SIM_PASSWORD_SIM_PUK) {
> > +               sim->wait_initialized = true;
> > +               DBG("Waiting for ofono_sim_initialized_notify");
> > +               return;
> > +       }
> > +
> > +recheck:
> > +       __ofono_sim_recheck_pin(sim);
> > +}
> > +
> >  static void sim_pin_retries_query_cb(const struct ofono_error *error,
> >                                         int retries[OFONO_SIM_PASSWORD_INVALID],
> >                                         void *data)
> > @@ -681,6 +757,13 @@ static void sim_locked_cb(struct ofono_sim *sim, gboolean locked)
> >                                                 OFONO_SIM_MANAGER_INTERFACE,
> >                                                 "LockedPins", DBUS_TYPE_STRING,
> >                                                 &locked_pins);
> > +
> > +       /*Cache pin only for SIM PIN type*/
> > +       if (g_strcmp0(typestr, "pin") == 0) {
> > +               if (!pin_cache_update(sim->iccid, pin))
> > +                       ofono_error("Failed to cache PIN.");
> > +       }
> > +
> >         g_strfreev(locked_pins);
> >
> >         sim_pin_retries_check(sim);
> > @@ -776,6 +859,14 @@ static DBusMessage *sim_unlock_pin(DBusConnection *conn, DBusMessage *msg,
> >  static void sim_change_pin_cb(const struct ofono_error *error, void *data)
> >  {
> >         struct ofono_sim *sim = data;
> > +       const char *typestr;
> > +       const char *old;
> > +       const char *new;
> > +
> > +       dbus_message_get_args(sim->pending, NULL, DBUS_TYPE_STRING, &typestr,
> > +                                            DBUS_TYPE_STRING, &old,
> > +                                            DBUS_TYPE_STRING, &new,
> > +                                            DBUS_TYPE_INVALID);
> >
> >         if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> >                 __ofono_dbus_pending_reply(&sim->pending,
> > @@ -786,6 +877,12 @@ static void sim_change_pin_cb(const struct ofono_error *error, void *data)
> >                 return;
> >         }
> >
> > +       /*Cache pin only for SIM PIN type*/
> > +       if (g_strcmp0(typestr, "pin") == 0) {
> > +               if (!pin_cache_update(sim->iccid, new))
> > +                       ofono_error("Failed to cache PIN.");
> > +       }
> > +
> >         __ofono_dbus_pending_reply(&sim->pending,
> >                                 dbus_message_new_method_return(sim->pending));
> >
> > @@ -837,8 +934,14 @@ static DBusMessage *sim_change_pin(DBusConnection *conn, DBusMessage *msg,
> >  static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
> >  {
> >         struct ofono_sim *sim = data;
> > +       const char *typestr;
> > +       const char *pin;
> >         DBusMessage *reply;
> >
> > +       dbus_message_get_args(sim->pending, NULL, DBUS_TYPE_STRING, &typestr,
> > +                                            DBUS_TYPE_STRING, &pin,
> > +                                            DBUS_TYPE_INVALID);
> > +
> >         if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
> >                 reply = __ofono_error_failed(sim->pending);
> >         else
> > @@ -850,6 +953,12 @@ static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
> >         if (sim->initialized || error->type != OFONO_ERROR_TYPE_NO_ERROR)
> >                 goto recheck;
> >
> > +       /*Cache pin only for SIM PIN type*/
> > +       if (g_strcmp0(typestr, "pin") == 0) {
> > +               if (!pin_cache_update(sim->iccid, pin))
> > +                       ofono_error("Failed to cache PIN.");
> > +       }
> > +
> >         if (sim->pin_type == OFONO_SIM_PASSWORD_SIM_PIN ||
> >                         sim->pin_type == OFONO_SIM_PASSWORD_SIM_PUK) {
> >                 sim->wait_initialized = true;
> > @@ -3023,6 +3132,7 @@ static void sim_pin_query_cb(const struct ofono_error *error,
> >         struct ofono_sim *sim = data;
> >         DBusConnection *conn = ofono_dbus_get_connection();
> >         const char *path = __ofono_atom_get_path(sim->atom);
> > +       struct cached_pin *cpins = pin_cache_lookup(sim->iccid);
> >         const char *pin_name;
> >         char **locked_pins;
> >         gboolean lock_changed;
> > @@ -3067,6 +3177,10 @@ static void sim_pin_query_cb(const struct ofono_error *error,
> >                                                 &pin_name);
> >         }
> >
> > +       if (g_strcmp0(pin_name, "pin") == 0 && cpins != NULL)
> > +               sim->driver->send_passwd(sim, cpins->pin,
> > +                                              pin_cache_enter_cb, sim);
> > +
> >         switch (pin_type) {
> >         case OFONO_SIM_PASSWORD_NONE:
> >         case OFONO_SIM_PASSWORD_SIM_PIN2:
> > --
> > 2.7.4
> >
> > _______________________________________________
> > ofono mailing list
> > ofono(a)ofono.org
> > https://lists.ofono.org/mailman/listinfo/ofono
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> https://lists.ofono.org/mailman/listinfo/ofono

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Giacinto Cifelli]

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

On Wed, Dec 12, 2018 at 11:04 AM Pavel Machek <pavel@ucw.cz> wrote:
>
> On Wed 2018-12-12 08:59:34, Giacinto Cifelli wrote:
> > No, absolutely not!
> > It is forbidden to do this by the 3GPP!
>
> Checking if 3GPP is allowed to make laws in this country.... no.
>
> :-)
>
> I'm not sure if it makes sense by default, but... I don't think 3GPP
> has or should have power over ofono project.

the user owns the pin, not ofono, and can decide to disable it -
that's the mechanism.
Besides, patching in ofono modem bugs is not the way: fix the bugs instead.

3GPP is constituted by operators and countries, who have also devised
lots of certifications.
If ofono goes against these, then it cannot be used in any
3GPP-compliant network.

>                                                                 Pavel

Rgds,
Giacinto

>
>
> > On Wed, Dec 12, 2018 at 7:46 AM Nandini Rebello
> > <nandini.rebello@intel.com> wrote:
> > >
> > > Adding SIM PIN caching feature to oFono. oFono now caches the SIM PIN1 type
> > > against the ICCID throughout its lifetime in a link list and enters
> > > implicitly upon modem reset/crash.
> > >
> > > Handles cases of incorrect pin and sim pin changed externally.
> > >
> > > Adding to all modems by default.

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Denis Kenzior]

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

Hi Giacinto,

No top-posting on this mailing list please.

On 12/12/2018 01:59 AM, Giacinto Cifelli wrote:
> No, absolutely not!
> It is forbidden to do this by the 3GPP!
> 

Can you cite the 3GPP specification number and section that this would 
be violating?

Regards,
-Denis

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Giacinto Cifelli]

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

Hi Denis,

On Wed, Dec 12, 2018 at 4:35 PM Denis Kenzior <denkenz@gmail.com> wrote:
>
> Hi Giacinto,
>
> No top-posting on this mailing list please.
>
> On 12/12/2018 01:59 AM, Giacinto Cifelli wrote:
> > No, absolutely not!
> > It is forbidden to do this by the 3GPP!
> >
>
> Can you cite the 3GPP specification number and section that this would
> be violating?

I will check this, but I remember that it is forbidden for at least two reasons:
- the PIN can be changed (in another phone/application), and then it
would be blocked
  this would make the pin cache feature also quite complicated,
because it must be deleted when the presentation is wrong, and if the
SIM is blocked, then also the application is more complicated because
it must consider that it needs to unblock the PIN under some
conditions. Also, if the PIN is changed by ofono, it must also be
tracked by the caching feature.
- the PIN presentation is prerogative and privilege of the user. It
cannot be delegated to the ME. Otherwise there would be no need for a
PIN at all. I have to say, I have seen several SIM with the PIN
disabled (especially in the US), but the user can always enable it if
he likes, and then wouldn't welcome that the device can be used
without his consent.
  from an operator point of view, it attributes the responsibility to
network access and use to the user, just like with the credit cards
chip&pin.

>
> Regards,
> -Denis

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Denis Kenzior]

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

Hi Giacinto,

>> Can you cite the 3GPP specification number and section that this would
>> be violating?
> 
> I will check this, but I remember that it is forbidden for at least two reasons:

FYI, I have seen modems from Nokia & Intel do this.  They would store 
the PIN in the NVRAM and if the firmware crashed, the PIN would be 
entered automagically.  That is why we have the ofono_modem_reset API. 
So you really need to cite a spec when you use such strong wording as 
'forbidden'.

> - the PIN can be changed (in another phone/application), and then it
> would be blocked

Yes, and I've already made Nandini aware of this concern.  However, the 
cache is not kept across ofono daemon restarts and is only meant for 
modem crashes.

Perhaps we need to clear the cache in a few additional scenarios to 
mitigate the above concern.  E.g. if the Modem was powered off via 
D-Bus, then any inserted SIM ICCID/PIN combo can be cleared.  Or if the 
modem driver issues an ofono_sim_inserted_notify(false) (because the 
user decided to physically remove the SIM), then any cache entries for 
the current ICCID/PIN can also be cleared.

>    this would make the pin cache feature also quite complicated,
> because it must be deleted when the presentation is wrong, and if the

This already happens...

> SIM is blocked, then also the application is more complicated because
> it must consider that it needs to unblock the PIN under some
> conditions. Also, if the PIN is changed by ofono, it must also be
> tracked by the caching feature.

And this already happens as well.

> - the PIN presentation is prerogative and privilege of the user. It
> cannot be delegated to the ME. Otherwise there would be no need for a
> PIN at all. I have to say, I have seen several SIM with the PIN
> disabled (especially in the US), but the user can always enable it if
> he likes, and then wouldn't welcome that the device can be used
> without his consent.
>    from an operator point of view, it attributes the responsibility to
> network access and use to the user, just like with the credit cards
> chip&pin.

As I mentioned above, the PIN cache is not persisted.  So the user would 
still need to enter the PIN on a reboot / insertion.

The cache is really only meant for the case where a modem goes down 
unexpectedly.  One could in theory argue that it is possible to trigger 
this with USB sticks and maybe we need an additional hint from the 
driver to enable this behavior.  E.g. make it opt-in rather than a 
default for everyone.

Do note that projects like NM/MM persist the PIN across reboots, though 
they do it for different reasons...

Regards,
-Denis

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Giacinto Cifelli]

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

Hi Denis,

On Wed, Dec 12, 2018 at 5:44 PM Denis Kenzior <denkenz@gmail.com> wrote:
>
> Hi Giacinto,
>
> >> Can you cite the 3GPP specification number and section that this would
> >> be violating?

21.111 (USIM and IC card requirements), or predecessors/successors.
I have taken the rel.7, whose paragraph 5.3 (User Data Stored in ME) mentions:

"User related security codes such as PIN and Unblock PIN may only be
stored by the ME during the procedures involving such a code and shall
be discarded by the ME immediately after completion of the procedure."

Is this forbidding enough?

> >
> > I will check this, but I remember that it is forbidden for at least two reasons:
>
> FYI, I have seen modems from Nokia & Intel do this.  They would store

I know that Nokia used to do what they wanted. It is the problem of a
de facto monopoly.

> the PIN in the NVRAM and if the firmware crashed, the PIN would be
> entered automagically.  That is why we have the ofono_modem_reset API.
> So you really need to cite a spec when you use such strong wording as
> 'forbidden'.
>
> > - the PIN can be changed (in another phone/application), and then it
> > would be blocked
>
> Yes, and I've already made Nandini aware of this concern.  However, the
> cache is not kept across ofono daemon restarts and is only meant for
> modem crashes.
>
> Perhaps we need to clear the cache in a few additional scenarios to
> mitigate the above concern.  E.g. if the Modem was powered off via
> D-Bus, then any inserted SIM ICCID/PIN combo can be cleared.  Or if the
> modem driver issues an ofono_sim_inserted_notify(false) (because the
> user decided to physically remove the SIM), then any cache entries for
> the current ICCID/PIN can also be cleared.
>
> >    this would make the pin cache feature also quite complicated,
> > because it must be deleted when the presentation is wrong, and if the
>
> This already happens...
>
> > SIM is blocked, then also the application is more complicated because
> > it must consider that it needs to unblock the PIN under some
> > conditions. Also, if the PIN is changed by ofono, it must also be
> > tracked by the caching feature.
>
> And this already happens as well.
>
> > - the PIN presentation is prerogative and privilege of the user. It
> > cannot be delegated to the ME. Otherwise there would be no need for a
> > PIN at all. I have to say, I have seen several SIM with the PIN
> > disabled (especially in the US), but the user can always enable it if
> > he likes, and then wouldn't welcome that the device can be used
> > without his consent.
> >    from an operator point of view, it attributes the responsibility to
> > network access and use to the user, just like with the credit cards
> > chip&pin.
>
> As I mentioned above, the PIN cache is not persisted.  So the user would
> still need to enter the PIN on a reboot / insertion.
>
> The cache is really only meant for the case where a modem goes down
> unexpectedly.  One could in theory argue that it is possible to trigger
> this with USB sticks and maybe we need an additional hint from the
> driver to enable this behavior.  E.g. make it opt-in rather than a
> default for everyone.
>
> Do note that projects like NM/MM persist the PIN across reboots, though
> they do it for different reasons...
>
> Regards,
> -Denis

Regards,
Giacinto

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Denis Kenzior]

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

Hi Giacinto,

>>>> Can you cite the 3GPP specification number and section that this would >>>> be violating?> > 21.111 (USIM and IC card requirements), or 
predecessors/successors.> I have taken the rel.7, whose paragraph 5.3 
(User Data Stored in ME) mentions:> > "User related security codes such 
as PIN and Unblock PIN may only be> stored by the ME during the 
procedures involving such a code and shall> be discarded by the ME 
immediately after completion of the procedure."
Thanks for digging that up, this section should definitely be mentioned 
in a comment in the patch or the commit description.

The question is really what to do and I can see both sides of the 
argument.  So let me play devil's advocate:

If a firmware crashes on a device with a PIN lock and the user was 
browsing the internet at that time, it would be quite intrusive to 
interrupt the user and prompt them for a PIN (after all, they already 
entered the PIN).  Additionally, if the PIN was stored for just this 
case and the firmware reboots fast enough, a crash might not even be 
noticed by the user at all.  Now one can argue that the firmware 
shouldn't crash, and I agree, but realistically the chances of that 
never happening are NIL.

So if we do proceed with this feature, it should try pretty hard to 
comply with the spirit of the cited section, even if it isn't complying 
with it in a literal sense.

Regards,
-Denis

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Giacinto Cifelli]

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

Hi Denis,

On Thu, Dec 13, 2018 at 5:28 PM Denis Kenzior <denkenz@gmail.com> wrote:
>
> Hi Giacinto,
>
> >>>> Can you cite the 3GPP specification number and section that this would
>>>> be violating?
>>
>> 21.111 (USIM and IC card requirements), or
>> predecessors/successors.> I have taken the rel.7, whose paragraph 5.3
>> (User Data Stored in ME) mentions:
>>
>> "User related security codes such
>> as PIN and Unblock PIN may only be
>> stored by the ME during the
>> procedures involving such a code and shall
>> be discarded by the ME
>> immediately after completion of the procedure."

> Thanks for digging that up, this section should definitely be mentioned
> in a comment in the patch or the commit description.
>
> The question is really what to do and I can see both sides of the
> argument.  So let me play devil's advocate:
>
> If a firmware crashes on a device with a PIN lock and the user was
> browsing the internet at that time, it would be quite intrusive to
> interrupt the user and prompt them for a PIN (after all, they already
> entered the PIN).  Additionally, if the PIN was stored for just this
> case and the firmware reboots fast enough, a crash might not even be
> noticed by the user at all.  Now one can argue that the firmware
> shouldn't crash, and I agree, but realistically the chances of that
> never happening are NIL.

on the other hand, the sessions will be lost anyway, downloads and
streaming interrupted, due to the change of mobile IP, even if
the reboot would be fast.

>
> So if we do proceed with this feature, it should try pretty hard to
> comply with the spirit of the cited section, even if it isn't complying
> with it in a literal sense.

I don't agree with this.
In the original intents, the PIN is shared through DBus,
with quite some risk to disclose it to still other applications.

And then, if you want to restore the status as before the reboot,
maybe there were other PINs or keys presented.
Should they be stored and managed too?

And in case of USB devices, they disappear and re-enum later.
Should we think about storing the PIN non-volatile
for supporting the feature for these devices???

And should we have a timeout for the reboot?

>
> Regards,
> -Denis

Regards,
Giacinto

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Denis Kenzior]

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

Hi Giacinto,

> on the other hand, the sessions will be lost anyway, downloads and
> streaming interrupted, due to the change of mobile IP, even if
> the reboot would be fast.

Hey, I'm not fully convinced this is a good idea either ;)

But then to play devil's advocate:
- TCP can already deal with network interruptions
- Sane networks should issue the same IP
- This has already been done before with good success

Whether this is a good idea to do on the application processor is 
another question.

> 
>>
>> So if we do proceed with this feature, it should try pretty hard to
>> comply with the spirit of the cited section, even if it isn't complying
>> with it in a literal sense.
> 
> I don't agree with this.
> In the original intents, the PIN is shared through DBus,
> with quite some risk to disclose it to still other applications.

Please stop going off on tangents.  There is no more risk of exposing 
the PIN through D-Bus with a properly setup D-Bus security policy than 
any other IPC mechanism you can think of.  In fact it is probably safer 
than any IPC you can roll on your own.  If you want to discuss D-Bus 
security issues, take it to the D-Bus mailing list.

> 
> And then, if you want to restore the status as before the reboot,
> maybe there were other PINs or keys presented.
> Should they be stored and managed too?

Maybe? That is a question for the modem guys.

> 
> And in case of USB devices, they disappear and re-enum later.
> Should we think about storing the PIN non-volatile
> for supporting the feature for these devices???
> 
> And should we have a timeout for the reboot?
> 

timeouts are generally a bad idea since you can't control how long 
something takes.  But yes, maybe the cache should also have an 
expiration time set when the device is removed as well as the IMEI of 
the device such that only one IMEI/ICCID/PIN combination can exist.

Regards,
-Denis

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Giacinto Cifelli]

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

Hi Denis,

On Thu, Dec 13, 2018 at 6:31 PM Denis Kenzior <denkenz@gmail.com> wrote:
>
> Hi Giacinto,
>
> > on the other hand, the sessions will be lost anyway, downloads and
> > streaming interrupted, due to the change of mobile IP, even if
> > the reboot would be fast.
>
> Hey, I'm not fully convinced this is a good idea either ;)
>
> But then to play devil's advocate:
> - TCP can already deal with network interruptions

not if the IP changes.

> - Sane networks should issue the same IP

not at all. They are handled in DHCP, and with million users, most
likely it will be different the next time.
There are recommendations not to reuse the same IP on some networks,
and to reassign the same on others.
Most of the networks do not guarantee it, unless there is a special
contract for fix IP (for servers over mobile network).

> - This has already been done before with good success

on CS networks mainly. On PS the situation evolves every day, and old
solutions might not work even today.

>
> Whether this is a good idea to do on the application processor is
> another question.
>
> >
> >>
> >> So if we do proceed with this feature, it should try pretty hard to
> >> comply with the spirit of the cited section, even if it isn't complying
> >> with it in a literal sense.
> >
> > I don't agree with this.
> > In the original intents, the PIN is shared through DBus,
> > with quite some risk to disclose it to still other applications.
>
> Please stop going off on tangents.  There is no more risk of exposing
> the PIN through D-Bus with a properly setup D-Bus security policy than
> any other IPC mechanism you can think of.  In fact it is probably safer
> than any IPC you can roll on your own.  If you want to discuss D-Bus
> security issues, take it to the D-Bus mailing list.
>
> >
> > And then, if you want to restore the status as before the reboot,
> > maybe there were other PINs or keys presented.
> > Should they be stored and managed too?
>
> Maybe? That is a question for the modem guys.

The SIM, apart from the authentication and USIM files,
deals with user sensitive contents that are completely transparent
to the modem and to ofono, and only tunneled end-to-end.

>
> >
> > And in case of USB devices, they disappear and re-enum later.
> > Should we think about storing the PIN non-volatile
> > for supporting the feature for these devices???
> >
> > And should we have a timeout for the reboot?
> >
>
> timeouts are generally a bad idea since you can't control how long
> something takes.  But yes, maybe the cache should also have an
> expiration time set when the device is removed as well as the IMEI of
> the device such that only one IMEI/ICCID/PIN combination can exist.
>
> Regards,
> -Denis

Regards,
Giacinto

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Pavel Machek]

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

Hi!

> If a firmware crashes on a device with a PIN lock and the user was browsing
> the internet at that time, it would be quite intrusive to interrupt the user
> and prompt them for a PIN (after all, they already entered the PIN).
> Additionally, if the PIN was stored for just this case and the firmware
> reboots fast enough, a crash might not even be noticed by the user at all.
> Now one can argue that the firmware shouldn't crash, and I agree, but
> realistically the chances of that never happening are NIL.

Can I have another scenario?

User is waiting for important call, phone in his pocket, expecting it
to ring. Firmware crashes, reboots... and asks for a PIN.

When the important call comes, phone is not available... as it is
waiting for PIN...

I believe even pretty old Nokia's did cache the PIN so .. exactly this
did not happen.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Pavel Machek]

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

On Thu 2018-12-13 23:01:45, Pavel Machek wrote:
> Hi!
> 
> > If a firmware crashes on a device with a PIN lock and the user was browsing
> > the internet at that time, it would be quite intrusive to interrupt the user
> > and prompt them for a PIN (after all, they already entered the PIN).
> > Additionally, if the PIN was stored for just this case and the firmware
> > reboots fast enough, a crash might not even be noticed by the user at all.
> > Now one can argue that the firmware shouldn't crash, and I agree, but
> > realistically the chances of that never happening are NIL.
> 
> Can I have another scenario?
> 
> User is waiting for important call, phone in his pocket, expecting it
> to ring. Firmware crashes, reboots... and asks for a PIN.
> 
> When the important call comes, phone is not available... as it is
> waiting for PIN...

And when user realizes what happened, he disables the PIN on his SIM
card. When his phone is stolen, thief is able to do many expensive
calls on his account...

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Christophe Ronco]

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

Hi,

On 12/13/18 11:06 PM, Pavel Machek wrote:
> On Thu 2018-12-13 23:01:45, Pavel Machek wrote:
>> Hi!
>>
>>> If a firmware crashes on a device with a PIN lock and the user was browsing
>>> the internet at that time, it would be quite intrusive to interrupt the user
>>> and prompt them for a PIN (after all, they already entered the PIN).
>>> Additionally, if the PIN was stored for just this case and the firmware
>>> reboots fast enough, a crash might not even be noticed by the user at all.
>>> Now one can argue that the firmware shouldn't crash, and I agree, but
>>> realistically the chances of that never happening are NIL.
>> Can I have another scenario?
>>
>> User is waiting for important call, phone in his pocket, expecting it
>> to ring. Firmware crashes, reboots... and asks for a PIN.
>>
>> When the important call comes, phone is not available... as it is
>> waiting for PIN...
> And when user realizes what happened, he disables the PIN on his SIM
> card. When his phone is stolen, thief is able to do many expensive
> calls on his account...
>
> 									Pavel

Just my 2 cents on this subject.

I used to develop phone firmwares 15 years ago back when we did not have
a kernel inside a phone. This means that any application (mail, WAP,
your favorite game, L1, ...) crash ends up in a phone reboot. Of course
PIN was not asked in this case. Feature name was "silent reboot" if I
remember well.

So I do confirm this used to be implemented. Maybe it was not compliant
with 3GPP and a security hole, maybe it was a bad idea.

Christophe Ronco

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Giacinto Cifelli]

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

Hi Christophe,
On Fri, Dec 14, 2018 at 10:19 AM Christophe Ronco <c.ronco@kerlink.fr> wrote:
>
> Hi,
>
> On 12/13/18 11:06 PM, Pavel Machek wrote:
> > On Thu 2018-12-13 23:01:45, Pavel Machek wrote:
> >> Hi!
> >>
> >>> If a firmware crashes on a device with a PIN lock and the user was browsing
> >>> the internet at that time, it would be quite intrusive to interrupt the user
> >>> and prompt them for a PIN (after all, they already entered the PIN).
> >>> Additionally, if the PIN was stored for just this case and the firmware
> >>> reboots fast enough, a crash might not even be noticed by the user at all.
> >>> Now one can argue that the firmware shouldn't crash, and I agree, but
> >>> realistically the chances of that never happening are NIL.
> >> Can I have another scenario?
> >>
> >> User is waiting for important call, phone in his pocket, expecting it
> >> to ring. Firmware crashes, reboots... and asks for a PIN.
> >>
> >> When the important call comes, phone is not available... as it is
> >> waiting for PIN...
> > And when user realizes what happened, he disables the PIN on his SIM
> > card. When his phone is stolen, thief is able to do many expensive
> > calls on his account...
> >
> >                                                                       Pavel
>
> Just my 2 cents on this subject.
>
> I used to develop phone firmwares 15 years ago back when we did not have
> a kernel inside a phone. This means that any application (mail, WAP,
> your favorite game, L1, ...) crash ends up in a phone reboot. Of course
> PIN was not asked in this case. Feature name was "silent reboot" if I
> remember well.

In ofono there is an additional difficulty. It does not restore the
situation all by itself.
Ofono provides mechanisms, not policies, so its client shall send the
powered, online, roaming, pdp params and activation.
I don't see why not also the PIN.

>
> So I do confirm this used to be implemented. Maybe it was not compliant
> with 3GPP and a security hole, maybe it was a bad idea.
>
> Christophe Ronco
>
>
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Nandini Rebello]

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

Adding SIM PIN caching feature to oFono. oFono now caches the SIM PIN1 type
against the ICCID throughout its lifetime in a link list and enters
implicitly upon modem reset/crash.

Violates 3GPP spec 21.111, section 5.3 - User Data stored in ME. Helps
in user experience by not barring out cellular services unless pin is entered
manually.

Handles cases of incorrect pin and sim pin changed externally.
Clears all cached PINs incase modem disabled manually and selectively when
sim is removed.

Adding to all modems by default.
---
 src/sim.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 128 insertions(+)

diff --git a/src/sim.c b/src/sim.c
index 886f291..dd28484 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -143,6 +143,11 @@ struct ofono_sim {
 	bool wait_initialized : 1;
 };
 
+struct cached_pin {
+	char *id;
+	char *pin;
+};
+
 struct msisdn_set_request {
 	struct ofono_sim *sim;
 	int pending;
@@ -178,6 +183,8 @@ static void sim_own_numbers_update(struct ofono_sim *sim);
 
 static GSList *g_drivers = NULL;
 
+static GSList *cached_pins = NULL;
+
 static const char *sim_passwd_name(enum ofono_sim_password_type type)
 {
 	return passwd_name[type];
@@ -475,6 +482,78 @@ done:
 	return reply;
 }
 
+static struct cached_pin *pin_cache_lookup(const char *iccid)
+{
+	struct cached_pin *c;
+	GSList *l;
+
+	if (cached_pins == NULL)
+		return NULL;
+
+	for (l = cached_pins; l; l = l->next) {
+		c = l->data;
+
+		if (g_strcmp0(iccid, c->id) == 0)
+			return c;
+	}
+
+	return NULL;
+}
+
+static gboolean pin_cache_update(const char *iccid, const char *pin)
+{
+	struct cached_pin *pin_cached = pin_cache_lookup(iccid);
+	struct cached_pin *cpins;
+
+	if (pin_cached != NULL) {
+		g_free(pin_cached->pin);
+		pin_cached->pin = g_strdup(pin);
+		return TRUE;
+	}
+
+	cpins = g_new0(struct cached_pin, 1);
+
+	if (cpins == NULL)
+		return FALSE;
+
+	cpins->id = g_strdup(iccid);
+	cpins->pin = g_strdup(pin);
+	cached_pins = g_slist_prepend(cached_pins, cpins);
+
+	return TRUE;
+}
+
+static void pin_cache_remove(const char *iccid)
+{
+	struct cached_pin *pin_cached = pin_cache_lookup(iccid);
+
+	if (pin_cached == NULL)
+		return;
+
+	cached_pins = g_slist_remove(cached_pins, pin_cached);
+}
+
+static void pin_cache_enter_cb(const struct ofono_error *error, void *data)
+{
+	struct ofono_sim *sim = data;
+
+	/* If PIN entry fails, then remove cached PIN*/
+	if (sim->initialized || error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		pin_cache_remove(sim->iccid);
+		goto recheck;
+	}
+
+	if (sim->pin_type == OFONO_SIM_PASSWORD_SIM_PIN ||
+			sim->pin_type == OFONO_SIM_PASSWORD_SIM_PUK) {
+		sim->wait_initialized = true;
+		DBG("Waiting for ofono_sim_initialized_notify");
+		return;
+	}
+
+recheck:
+	__ofono_sim_recheck_pin(sim);
+}
+
 static void sim_pin_retries_query_cb(const struct ofono_error *error,
 					int retries[OFONO_SIM_PASSWORD_INVALID],
 					void *data)
@@ -683,6 +762,13 @@ static void sim_locked_cb(struct ofono_sim *sim, gboolean locked)
 						OFONO_SIM_MANAGER_INTERFACE,
 						"LockedPins", DBUS_TYPE_STRING,
 						&locked_pins);
+
+	/*Cache pin only for SIM PIN type*/
+	if (g_strcmp0(typestr, "pin") == 0) {
+		if (!pin_cache_update(sim->iccid, pin))
+			ofono_error("Failed to cache PIN.");
+	}
+
 	g_strfreev(locked_pins);
 
 	sim_pin_retries_check(sim);
@@ -778,6 +864,14 @@ static DBusMessage *sim_unlock_pin(DBusConnection *conn, DBusMessage *msg,
 static void sim_change_pin_cb(const struct ofono_error *error, void *data)
 {
 	struct ofono_sim *sim = data;
+	const char *typestr;
+	const char *old;
+	const char *new;
+
+	dbus_message_get_args(sim->pending, NULL, DBUS_TYPE_STRING, &typestr,
+					     DBUS_TYPE_STRING, &old,
+					     DBUS_TYPE_STRING, &new,
+				             DBUS_TYPE_INVALID);
 
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
 		__ofono_dbus_pending_reply(&sim->pending,
@@ -788,6 +882,12 @@ static void sim_change_pin_cb(const struct ofono_error *error, void *data)
 		return;
 	}
 
+	/*Cache pin only for SIM PIN type*/
+	if (g_strcmp0(typestr, "pin") == 0) {
+		if (!pin_cache_update(sim->iccid, new))
+			ofono_error("Failed to cache PIN.");
+	}
+
 	__ofono_dbus_pending_reply(&sim->pending,
 				dbus_message_new_method_return(sim->pending));
 
@@ -839,8 +939,14 @@ static DBusMessage *sim_change_pin(DBusConnection *conn, DBusMessage *msg,
 static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
 {
 	struct ofono_sim *sim = data;
+	const char *typestr;
+	const char *pin;
 	DBusMessage *reply;
 
+	dbus_message_get_args(sim->pending, NULL, DBUS_TYPE_STRING, &typestr,
+					     DBUS_TYPE_STRING, &pin,
+					     DBUS_TYPE_INVALID);
+
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
 		reply = __ofono_error_failed(sim->pending);
 	else
@@ -852,6 +958,12 @@ static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
 	if (sim->initialized || error->type != OFONO_ERROR_TYPE_NO_ERROR)
 		goto recheck;
 
+	/*Cache pin only for SIM PIN type*/
+	if (g_strcmp0(typestr, "pin") == 0) {
+		if (!pin_cache_update(sim->iccid, pin))
+			ofono_error("Failed to cache PIN.");
+	}
+
 	if (sim->pin_type == OFONO_SIM_PASSWORD_SIM_PIN ||
 			sim->pin_type == OFONO_SIM_PASSWORD_SIM_PUK) {
 		sim->wait_initialized = true;
@@ -2751,6 +2863,8 @@ void ofono_sim_inserted_notify(struct ofono_sim *sim, ofono_bool_t inserted)
 		sim->pin_retries[OFONO_SIM_PASSWORD_SIM_PIN2] = -1;
 		sim->pin_retries[OFONO_SIM_PASSWORD_SIM_PUK2] = -1;
 
+		pin_cache_remove(sim->iccid);
+
 		sim_free_state(sim);
 	}
 }
@@ -3024,6 +3138,7 @@ static void sim_pin_query_cb(const struct ofono_error *error,
 	struct ofono_sim *sim = data;
 	DBusConnection *conn = ofono_dbus_get_connection();
 	const char *path = __ofono_atom_get_path(sim->atom);
+	struct cached_pin *cpins = pin_cache_lookup(sim->iccid);
 	const char *pin_name;
 	char **locked_pins;
 	gboolean lock_changed;
@@ -3068,6 +3183,10 @@ static void sim_pin_query_cb(const struct ofono_error *error,
 						&pin_name);
 	}
 
+	if (g_strcmp0(pin_name, "pin") == 0 && cpins != NULL)
+		sim->driver->send_passwd(sim, cpins->pin,
+					       pin_cache_enter_cb, sim);
+
 	switch (pin_type) {
 	case OFONO_SIM_PASSWORD_NONE:
 	case OFONO_SIM_PASSWORD_SIM_PIN2:
@@ -3301,6 +3420,15 @@ void ofono_sim_register(struct ofono_sim *sim)
 	__ofono_atom_register(sim->atom, sim_unregister);
 }
 
+void ofono_sim_clear_cached_pins(void)
+{
+	if (cached_pins == NULL)
+		return;
+
+	g_slist_free_full(cached_pins, g_free);
+	cached_pins = NULL;
+}
+
 void ofono_sim_remove(struct ofono_sim *sim)
 {
 	__ofono_atom_free(sim->atom);
-- 
2.7.4

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Giacinto Cifelli]

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

Hi Nandini,

On Thu, Jan 3, 2019 at 12:01 PM Nandini Rebello
<nandini.rebello@intel.com> wrote:
>
> Adding SIM PIN caching feature to oFono. oFono now caches the SIM PIN1 type
> against the ICCID throughout its lifetime in a link list and enters
> implicitly upon modem reset/crash.
>
> Violates 3GPP spec 21.111, section 5.3 - User Data stored in ME. Helps
> in user experience by not barring out cellular services unless pin is entered
> manually.
>
> Handles cases of incorrect pin and sim pin changed externally.
> Clears all cached PINs incase modem disabled manually and selectively when
> sim is removed.
>
> Adding to all modems by default.

Since it can cause losing certification, it would be better to have a
property CachePIN1, disabled by default, and the application can
select whether to go against the standard or not.

Regards,
Giacinto

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Denis Kenzior]

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

Hi Nandini,

On 01/03/2019 05:01 AM, Nandini Rebello wrote:
> Adding SIM PIN caching feature to oFono. oFono now caches the SIM PIN1 type
> against the ICCID throughout its lifetime in a link list and enters
> implicitly upon modem reset/crash.
> 
> Violates 3GPP spec 21.111, section 5.3 - User Data stored in ME. Helps
> in user experience by not barring out cellular services unless pin is entered
> manually.
> 
> Handles cases of incorrect pin and sim pin changed externally.
> Clears all cached PINs incase modem disabled manually and selectively when
> sim is removed.
> 
> Adding to all modems by default.
> ---
>   src/sim.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 128 insertions(+)
> 

<snip>

> +static void pin_cache_enter_cb(const struct ofono_error *error, void *data)
> +{
> +	struct ofono_sim *sim = data;
> +
> +	/* If PIN entry fails, then remove cached PIN*/
> +	if (sim->initialized || error->type != OFONO_ERROR_TYPE_NO_ERROR) {

Hm, I just noticed that this if condition is incorrect.  You shouldn't 
be removing the pin if sim->initialized is true.  This would break 
drivers that call ofono_sim_initialized right after ofono_sim_inserted. 
See commit 54d56d763e40bc44c99a9b24aa0477bd373ea085 for details.

> +		pin_cache_remove(sim->iccid);
> +		goto recheck;
> +	}
> +
> +	if (sim->pin_type == OFONO_SIM_PASSWORD_SIM_PIN ||
> +			sim->pin_type == OFONO_SIM_PASSWORD_SIM_PUK) {
> +		sim->wait_initialized = true;
> +		DBG("Waiting for ofono_sim_initialized_notify");
> +		return;
> +	}
> +
> +recheck:
> +	__ofono_sim_recheck_pin(sim);

Actually this whole function might be better written as:

if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
	pin_cache_remove(sim->iccid);

sim_enter_pin_cb(...);

> +}
> +
>   static void sim_pin_retries_query_cb(const struct ofono_error *error,
>   					int retries[OFONO_SIM_PASSWORD_INVALID],
>   					void *data)
> @@ -683,6 +762,13 @@ static void sim_locked_cb(struct ofono_sim *sim, gboolean locked)
>   						OFONO_SIM_MANAGER_INTERFACE,
>   						"LockedPins", DBUS_TYPE_STRING,
>   						&locked_pins);
> +
> +	/*Cache pin only for SIM PIN type*/
> +	if (g_strcmp0(typestr, "pin") == 0) {
> +		if (!pin_cache_update(sim->iccid, pin))
> +			ofono_error("Failed to cache PIN.");

pin_cache_update really cannot fail since g_new0 will never fail...

> +	}
> +
>   	g_strfreev(locked_pins);
>   
>   	sim_pin_retries_check(sim);
> @@ -778,6 +864,14 @@ static DBusMessage *sim_unlock_pin(DBusConnection *conn, DBusMessage *msg,
>   static void sim_change_pin_cb(const struct ofono_error *error, void *data)
>   {
>   	struct ofono_sim *sim = data;
> +	const char *typestr;
> +	const char *old;
> +	const char *new;
> +
> +	dbus_message_get_args(sim->pending, NULL, DBUS_TYPE_STRING, &typestr,
> +					     DBUS_TYPE_STRING, &old,
> +					     DBUS_TYPE_STRING, &new,
> +				             DBUS_TYPE_INVALID);
>   
>   	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
>   		__ofono_dbus_pending_reply(&sim->pending,
> @@ -788,6 +882,12 @@ static void sim_change_pin_cb(const struct ofono_error *error, void *data)
>   		return;
>   	}
>   
> +	/*Cache pin only for SIM PIN type*/
> +	if (g_strcmp0(typestr, "pin") == 0) {
> +		if (!pin_cache_update(sim->iccid, new))
> +			ofono_error("Failed to cache PIN.");

As above

> +	}
> +
>   	__ofono_dbus_pending_reply(&sim->pending,
>   				dbus_message_new_method_return(sim->pending));
>   
> @@ -839,8 +939,14 @@ static DBusMessage *sim_change_pin(DBusConnection *conn, DBusMessage *msg,
>   static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
>   {
>   	struct ofono_sim *sim = data;
> +	const char *typestr;
> +	const char *pin;
>   	DBusMessage *reply;
>   
> +	dbus_message_get_args(sim->pending, NULL, DBUS_TYPE_STRING, &typestr,
> +					     DBUS_TYPE_STRING, &pin,
> +					     DBUS_TYPE_INVALID);
> +
>   	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
>   		reply = __ofono_error_failed(sim->pending);
>   	else
> @@ -852,6 +958,12 @@ static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
>   	if (sim->initialized || error->type != OFONO_ERROR_TYPE_NO_ERROR)
>   		goto recheck;
>   
> +	/*Cache pin only for SIM PIN type*/
> +	if (g_strcmp0(typestr, "pin") == 0) {
> +		if (!pin_cache_update(sim->iccid, pin))
> +			ofono_error("Failed to cache PIN.");

And here

> +	}
> +
>   	if (sim->pin_type == OFONO_SIM_PASSWORD_SIM_PIN ||
>   			sim->pin_type == OFONO_SIM_PASSWORD_SIM_PUK) {
>   		sim->wait_initialized = true;

<snip>

> @@ -3301,6 +3420,15 @@ void ofono_sim_register(struct ofono_sim *sim)
>   	__ofono_atom_register(sim->atom, sim_unregister);
>   }
>   
> +void ofono_sim_clear_cached_pins(void)
> +{
> +	if (cached_pins == NULL)
> +		return;
> +
> +	g_slist_free_full(cached_pins, g_free);
> +	cached_pins = NULL;

Why would you clear the entire cache?  Don't you want to clear only the 
currently inserted iccid?

> +}
> +
>   void ofono_sim_remove(struct ofono_sim *sim)
>   {
>   	__ofono_atom_free(sim->atom);
> 

Regards,
-Denis

[PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Nandini Rebello]

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

Adding SIM PIN caching feature to oFono. oFono now caches the SIM PIN1 type
against the ICCID throughout its lifetime in a link list and enters
implicitly upon modem reset/crash.

Violates 3GPP spec 21.111, section 5.3 - User Data stored in ME. Helps
in user experience by not barring out cellular services unless pin is entered
manually.

Handles cases of incorrect pin and sim pin changed externally.
Clear cached PIN incase modem disabled manually and selectively when
sim is removed.

Seperate 'pin_cache_enter_cb' added without dbus calls to handle implict entering of
cached pin.

Adding to all modems by default.
---
 src/sim.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 111 insertions(+)

diff --git a/src/sim.c b/src/sim.c
index 886f291..d5e6d40 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -143,6 +143,11 @@ struct ofono_sim {
 	bool wait_initialized : 1;
 };
 
+struct cached_pin {
+	char *id;
+	char *pin;
+};
+
 struct msisdn_set_request {
 	struct ofono_sim *sim;
 	int pending;
@@ -178,6 +183,8 @@ static void sim_own_numbers_update(struct ofono_sim *sim);
 
 static GSList *g_drivers = NULL;
 
+static GSList *cached_pins = NULL;
+
 static const char *sim_passwd_name(enum ofono_sim_password_type type)
 {
 	return passwd_name[type];
@@ -475,6 +482,68 @@ done:
 	return reply;
 }
 
+static struct cached_pin *pin_cache_lookup(const char *iccid)
+{
+	struct cached_pin *c;
+	GSList *l;
+
+	if (cached_pins == NULL)
+		return NULL;
+
+	for (l = cached_pins; l; l = l->next) {
+		c = l->data;
+
+		if (g_strcmp0(iccid, c->id) == 0)
+			return c;
+	}
+
+	return NULL;
+}
+
+static void pin_cache_update(const char *iccid, const char *pin)
+{
+	struct cached_pin *pin_cached = pin_cache_lookup(iccid);
+	struct cached_pin *cpins;
+
+	if (pin_cached != NULL) {
+		g_free(pin_cached->pin);
+		pin_cached->pin = g_strdup(pin);
+		return;
+	}
+
+	cpins = g_new0(struct cached_pin, 1);
+
+	cpins->id = g_strdup(iccid);
+	cpins->pin = g_strdup(pin);
+	cached_pins = g_slist_prepend(cached_pins, cpins);
+}
+
+static void pin_cache_remove(const char *iccid)
+{
+	struct cached_pin *pin_cached = pin_cache_lookup(iccid);
+
+	if (pin_cached == NULL)
+		return;
+
+	cached_pins = g_slist_remove(cached_pins, pin_cached);
+}
+
+static void pin_cache_enter_cb(const struct ofono_error *error, void *data)
+{
+	struct ofono_sim *sim = data;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		pin_cache_remove(sim->iccid);
+
+		__ofono_sim_recheck_pin(sim);
+
+		return;
+	}
+
+	sim->wait_initialized = true;
+	DBG("Waiting for ofono_sim_initialized_notify");
+}
+
 static void sim_pin_retries_query_cb(const struct ofono_error *error,
 					int retries[OFONO_SIM_PASSWORD_INVALID],
 					void *data)
@@ -683,6 +752,11 @@ static void sim_locked_cb(struct ofono_sim *sim, gboolean locked)
 						OFONO_SIM_MANAGER_INTERFACE,
 						"LockedPins", DBUS_TYPE_STRING,
 						&locked_pins);
+
+	/*Cache pin only for SIM PIN type*/
+	if (g_strcmp0(typestr, "pin") == 0)
+		pin_cache_update(sim->iccid, pin);
+
 	g_strfreev(locked_pins);
 
 	sim_pin_retries_check(sim);
@@ -778,6 +852,14 @@ static DBusMessage *sim_unlock_pin(DBusConnection *conn, DBusMessage *msg,
 static void sim_change_pin_cb(const struct ofono_error *error, void *data)
 {
 	struct ofono_sim *sim = data;
+	const char *typestr;
+	const char *old;
+	const char *new;
+
+	dbus_message_get_args(sim->pending, NULL, DBUS_TYPE_STRING, &typestr,
+					     DBUS_TYPE_STRING, &old,
+					     DBUS_TYPE_STRING, &new,
+				             DBUS_TYPE_INVALID);
 
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
 		__ofono_dbus_pending_reply(&sim->pending,
@@ -788,6 +870,10 @@ static void sim_change_pin_cb(const struct ofono_error *error, void *data)
 		return;
 	}
 
+	/*Cache pin only for SIM PIN type*/
+	if (g_strcmp0(typestr, "pin") == 0)
+		pin_cache_update(sim->iccid, new);
+
 	__ofono_dbus_pending_reply(&sim->pending,
 				dbus_message_new_method_return(sim->pending));
 
@@ -839,8 +925,14 @@ static DBusMessage *sim_change_pin(DBusConnection *conn, DBusMessage *msg,
 static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
 {
 	struct ofono_sim *sim = data;
+	const char *typestr;
+	const char *pin;
 	DBusMessage *reply;
 
+	dbus_message_get_args(sim->pending, NULL, DBUS_TYPE_STRING, &typestr,
+					     DBUS_TYPE_STRING, &pin,
+					     DBUS_TYPE_INVALID);
+
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
 		reply = __ofono_error_failed(sim->pending);
 	else
@@ -852,6 +944,10 @@ static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
 	if (sim->initialized || error->type != OFONO_ERROR_TYPE_NO_ERROR)
 		goto recheck;
 
+	/*Cache pin only for SIM PIN type*/
+	if (g_strcmp0(typestr, "pin") == 0)
+		pin_cache_update(sim->iccid, pin);
+
 	if (sim->pin_type == OFONO_SIM_PASSWORD_SIM_PIN ||
 			sim->pin_type == OFONO_SIM_PASSWORD_SIM_PUK) {
 		sim->wait_initialized = true;
@@ -2751,6 +2847,8 @@ void ofono_sim_inserted_notify(struct ofono_sim *sim, ofono_bool_t inserted)
 		sim->pin_retries[OFONO_SIM_PASSWORD_SIM_PIN2] = -1;
 		sim->pin_retries[OFONO_SIM_PASSWORD_SIM_PUK2] = -1;
 
+		pin_cache_remove(sim->iccid);
+
 		sim_free_state(sim);
 	}
 }
@@ -3024,6 +3122,7 @@ static void sim_pin_query_cb(const struct ofono_error *error,
 	struct ofono_sim *sim = data;
 	DBusConnection *conn = ofono_dbus_get_connection();
 	const char *path = __ofono_atom_get_path(sim->atom);
+	struct cached_pin *cpins = pin_cache_lookup(sim->iccid);
 	const char *pin_name;
 	char **locked_pins;
 	gboolean lock_changed;
@@ -3068,6 +3167,10 @@ static void sim_pin_query_cb(const struct ofono_error *error,
 						&pin_name);
 	}
 
+	if (g_strcmp0(pin_name, "pin") == 0 && cpins != NULL)
+		sim->driver->send_passwd(sim, cpins->pin,
+					       pin_cache_enter_cb, sim);
+
 	switch (pin_type) {
 	case OFONO_SIM_PASSWORD_NONE:
 	case OFONO_SIM_PASSWORD_SIM_PIN2:
@@ -3301,6 +3404,14 @@ void ofono_sim_register(struct ofono_sim *sim)
 	__ofono_atom_register(sim->atom, sim_unregister);
 }
 
+void ofono_sim_clear_cached_pins(struct ofono_sim *sim)
+{
+	if (cached_pins == NULL)
+		return;
+
+	pin_cache_remove(sim->iccid);
+}
+
 void ofono_sim_remove(struct ofono_sim *sim)
 {
 	__ofono_atom_free(sim->atom);
-- 
2.7.4

Re: [PATCH] sim: Sim PIN1 cache upon modem reset/crash

[Denis Kenzior]

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

On 01/16/2019 12:37 AM, Nandini Rebello wrote:
> Adding SIM PIN caching feature to oFono. oFono now caches the SIM PIN1 type
> against the ICCID throughout its lifetime in a link list and enters
> implicitly upon modem reset/crash.
> 
> Violates 3GPP spec 21.111, section 5.3 - User Data stored in ME. Helps
> in user experience by not barring out cellular services unless pin is entered
> manually.
> 
> Handles cases of incorrect pin and sim pin changed externally.
> Clear cached PIN incase modem disabled manually and selectively when
> sim is removed.
> 
> Seperate 'pin_cache_enter_cb' added without dbus calls to handle implict entering of
> cached pin.
> 
> Adding to all modems by default.
> ---
>   src/sim.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 111 insertions(+)
> 

I went ahead and applied this patch after minor style cleanups.  I also 
renamed void ofono_sim_clear_cached_pins(struct ofono_sim *sim) to
__ofono_sim... and put it in ofono.h.

This means that your other patch was unnecessary.  In the future, please 
send all these as a series.

Additionally, I pushed out commit 
0c1685a205570f407fb8dcdb5ccb75de21a96c7b.  Please make sure it is 
correct and there are no other memory leaks lurking.

Regards,
-Denis