* Ofono has too tight limits for SIM lock code lengths
@ 2011-01-19 9:04 Jussi Kangas
2011-01-20 17:35 ` Denis Kenzior
0 siblings, 1 reply; 6+ messages in thread
From: Jussi Kangas @ 2011-01-19 9:04 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 868 bytes --]
Hi,
I've been testing SIM locks with Ofono and it looks to me that method
is_valid_pin in src/common.c has somewhat too tight rules for PIN
lenghts. SIM or subsidy lock codes can be longer than 8 digits (as
dictated in 22.022 chapter 14)
There are several ways to fix this. My current favourite is changing the
is_valid_pin so that it takes the actual return value of
sim_string_to_passwd instead of pin_type enumaration as parameter and
checks with 16-digit limit the SIM locks.
Other option that comes to my mind would be separate a valid check
method for SIM locks.
As third option I suggest adding fifth value to pin_type enumeration and
give that as a parameter for is_valid_pin to make it use different
limits. It would require more specific code type recognition in calling
function though.
Opinions? Other ideas?
Br,
-Jussi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Ofono has too tight limits for SIM lock code lengths
2011-01-19 9:04 Ofono has too tight limits for SIM lock code lengths Jussi Kangas
@ 2011-01-20 17:35 ` Denis Kenzior
0 siblings, 0 replies; 6+ messages in thread
From: Denis Kenzior @ 2011-01-20 17:35 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1217 bytes --]
Hi Jussi,
On 01/19/2011 03:04 AM, Jussi Kangas wrote:
> Hi,
>
> I've been testing SIM locks with Ofono and it looks to me that method
> is_valid_pin in src/common.c has somewhat too tight rules for PIN
> lenghts. SIM or subsidy lock codes can be longer than 8 digits (as
> dictated in 22.022 chapter 14)
>
You might very well be correct. The current assumptions are based on
11.11, but I'm not sure if USIMs or newer have relaxed these
restrictions. Maybe others can chime in here.
> There are several ways to fix this. My current favourite is changing the
> is_valid_pin so that it takes the actual return value of
> sim_string_to_passwd instead of pin_type enumaration as parameter and
> checks with 16-digit limit the SIM locks.
>
> Other option that comes to my mind would be separate a valid check
> method for SIM locks.
>
> As third option I suggest adding fifth value to pin_type enumeration and
> give that as a parameter for is_valid_pin to make it use different
> limits. It would require more specific code type recognition in calling
> function though.
The first option sounds reasonable to me as a first attempt. Care to
submit a proposal?
Regards,
-Denis
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Ofono has too tight limits for SIM lock code lengths
@ 2011-01-27 12:52 Jussi Kangas
2011-01-27 13:09 ` Marcel Holtmann
0 siblings, 1 reply; 6+ messages in thread
From: Jussi Kangas @ 2011-01-27 12:52 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 9058 bytes --]
Hi Denis,
On Thu, 2011-01-20 at 19:35 +0200, Denis Kenzior wrote:
> The first option sounds reasonable to me as a first attempt. Care to
> submit a proposal?
>
This seems to do the trick
---
include/sim.h | 1 +
src/call-barring.c | 14 ++++++++------
src/call-meter.c | 4 ++--
src/common.c | 29 ++++++++++++++++++++++++-----
src/common.h | 11 +++--------
src/ofono.h | 3 +++
src/sim.c | 13 +++++++------
7 files changed, 48 insertions(+), 27 deletions(-)
diff --git a/include/sim.h b/include/sim.h
index 5e3ba5b..ee63d74 100644
--- a/include/sim.h
+++ b/include/sim.h
@@ -54,6 +54,7 @@ enum ofono_sim_password_type {
OFONO_SIM_PASSWORD_PHNETSUB_PUK,
OFONO_SIM_PASSWORD_PHSP_PUK,
OFONO_SIM_PASSWORD_PHCORP_PUK,
+ OFONO_PASSWORD_PIN_TYPE_NET,
OFONO_SIM_PASSWORD_INVALID,
};
diff --git a/src/call-barring.c b/src/call-barring.c
index bb15530..6768833 100644
--- a/src/call-barring.c
+++ b/src/call-barring.c
@@ -402,7 +402,8 @@ static gboolean cb_ss_control(int type, const char *sc,
if (strlen(dn) > 0)
goto bad_format;
- if (type != SS_CONTROL_TYPE_QUERY && !is_valid_pin(sia, PIN_TYPE_NET))
+ if (type != SS_CONTROL_TYPE_QUERY &&
+ !is_valid_pin(sia, OFONO_PASSWORD_PIN_TYPE_NET))
goto bad_format;
switch (type) {
@@ -524,7 +525,8 @@ static gboolean cb_ss_passwd(const char *sc,
if (fac == NULL)
return FALSE;
- if (!is_valid_pin(old, PIN_TYPE_NET) || !is_valid_pin(new, PIN_TYPE_NET))
+ if (!is_valid_pin(old, OFONO_PASSWORD_PIN_TYPE_NET) ||
+ !is_valid_pin(new, OFONO_PASSWORD_PIN_TYPE_NET))
goto bad_format;
cb->pending = dbus_message_ref(msg);
@@ -862,7 +864,7 @@ static DBusMessage *cb_set_property(DBusConnection *conn, DBusMessage *msg,
return __ofono_error_invalid_args(msg);
dbus_message_iter_get_basic(&iter, &passwd);
- if (!is_valid_pin(passwd, PIN_TYPE_NET))
+ if (!is_valid_pin(passwd, OFONO_PASSWORD_PIN_TYPE_NET))
return __ofono_error_invalid_format(msg);
}
@@ -909,7 +911,7 @@ static DBusMessage *cb_disable_all(DBusConnection *conn, DBusMessage *msg,
DBUS_TYPE_INVALID) == FALSE)
return __ofono_error_invalid_args(msg);
- if (!is_valid_pin(passwd, PIN_TYPE_NET))
+ if (!is_valid_pin(passwd, OFONO_PASSWORD_PIN_TYPE_NET))
return __ofono_error_invalid_format(msg);
cb_set_query_bounds(cb, fac, FALSE);
@@ -957,10 +959,10 @@ static DBusMessage *cb_set_passwd(DBusConnection *conn, DBusMessage *msg,
DBUS_TYPE_INVALID) == FALSE)
return __ofono_error_invalid_args(msg);
- if (!is_valid_pin(old_passwd, PIN_TYPE_NET))
+ if (!is_valid_pin(old_passwd, OFONO_PASSWORD_PIN_TYPE_NET))
return __ofono_error_invalid_format(msg);
- if (!is_valid_pin(new_passwd, PIN_TYPE_NET))
+ if (!is_valid_pin(new_passwd, OFONO_PASSWORD_PIN_TYPE_NET))
return __ofono_error_invalid_format(msg);
cb->pending = dbus_message_ref(msg);
diff --git a/src/call-meter.c b/src/call-meter.c
index ac3ae6b..dd04459 100644
--- a/src/call-meter.c
+++ b/src/call-meter.c
@@ -548,7 +548,7 @@ static DBusMessage *cm_set_property(DBusConnection *conn, DBusMessage *msg,
dbus_message_iter_get_basic(&iter, &passwd);
- if (!is_valid_pin(passwd, PIN_TYPE_PIN))
+ if (!is_valid_pin(passwd, OFONO_SIM_PASSWORD_SIM_PIN2))
return __ofono_error_invalid_format(msg);
for (property = cm_properties; property->name; property++) {
@@ -620,7 +620,7 @@ static DBusMessage *cm_acm_reset(DBusConnection *conn, DBusMessage *msg,
DBUS_TYPE_INVALID) == FALSE)
return __ofono_error_invalid_args(msg);
- if (!is_valid_pin(pin2, PIN_TYPE_PIN))
+ if (!is_valid_pin(pin2, OFONO_SIM_PASSWORD_SIM_PIN2))
return __ofono_error_invalid_format(msg);
cm->pending = dbus_message_ref(msg);
diff --git a/src/common.c b/src/common.c
index 8bf9dbb..19459ac 100644
--- a/src/common.c
+++ b/src/common.c
@@ -649,7 +649,7 @@ const char *bearer_class_to_string(enum bearer_class cls)
return NULL;
}
-gboolean is_valid_pin(const char *pin, enum pin_type type)
+gboolean is_valid_pin(const char *pin, enum ofono_sim_password_type type)
{
unsigned int i;
@@ -662,25 +662,44 @@ gboolean is_valid_pin(const char *pin, enum pin_type type)
return FALSE;
switch (type) {
- case PIN_TYPE_PIN:
+ case OFONO_SIM_PASSWORD_SIM_PIN:
+ case OFONO_SIM_PASSWORD_SIM_PIN2:
/* 11.11 Section 9.3 ("CHV"): 4..8 IA-5 digits */
if (4 <= i && i <= 8)
return TRUE;
break;
- case PIN_TYPE_PUK:
+ case OFONO_SIM_PASSWORD_PHSIM_PIN:
+ case OFONO_SIM_PASSWORD_PHFSIM_PIN:
+ case OFONO_SIM_PASSWORD_PHNET_PIN:
+ case OFONO_SIM_PASSWORD_PHNETSUB_PIN:
+ case OFONO_SIM_PASSWORD_PHSP_PIN:
+ case OFONO_SIM_PASSWORD_PHCORP_PIN:
+ /* 22.022 Section 14 4..16 IA-5 digits */
+ if (4 <= i && i <= 16)
+ return TRUE;
+ break;
+ case OFONO_SIM_PASSWORD_SIM_PUK:
+ case OFONO_SIM_PASSWORD_SIM_PUK2:
+ case OFONO_SIM_PASSWORD_PHFSIM_PUK:
+ case OFONO_SIM_PASSWORD_PHNET_PUK:
+ case OFONO_SIM_PASSWORD_PHNETSUB_PUK:
+ case OFONO_SIM_PASSWORD_PHSP_PUK:
+ case OFONO_SIM_PASSWORD_PHCORP_PUK:
/* 11.11 Section 9.3 ("UNBLOCK CHV"), 8 IA-5 digits */
if (i == 8)
return TRUE;
break;
- case PIN_TYPE_NET:
+ case OFONO_PASSWORD_PIN_TYPE_NET:
/* 22.004 Section 5.2, 4 IA-5 digits */
if (i == 4)
return TRUE;
break;
- case PIN_TYPE_NONE:
+ case OFONO_SIM_PASSWORD_NONE:
if (i < 8)
return TRUE;
break;
+ case OFONO_SIM_PASSWORD_INVALID:
+ break;
}
return FALSE;
diff --git a/src/common.h b/src/common.h
index 09f2deb..acdbce5 100644
--- a/src/common.h
+++ b/src/common.h
@@ -19,6 +19,8 @@
*
*/
+#include "ofono.h"
+
/* 27.007 Section 7.3 <AcT> */
enum access_technology {
ACCESS_TECHNOLOGY_GSM = 0,
@@ -122,13 +124,6 @@ enum ss_cssu {
SS_MT_CALL_DEFLECTED = 9,
};
-enum pin_type {
- PIN_TYPE_NONE,
- PIN_TYPE_PIN,
- PIN_TYPE_PUK,
- PIN_TYPE_NET,
-};
-
/* 27.007 Section 10.1.10 */
enum context_status {
CONTEXT_STATUS_DEACTIVATED = 0,
@@ -162,7 +157,7 @@ const char *ss_control_type_to_string(enum ss_control_type type);
const char *bearer_class_to_string(enum bearer_class cls);
-gboolean is_valid_pin(const char *pin, enum pin_type type);
+gboolean is_valid_pin(const char *pin, enum ofono_sim_password_type type);
const char *registration_status_to_string(int status);
const char *registration_tech_to_string(int tech);
diff --git a/src/ofono.h b/src/ofono.h
index df20103..40602af 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -18,6 +18,8 @@
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*
*/
+#ifndef OFONO_H
+#define OFONO_H
#include <glib.h>
@@ -429,3 +431,4 @@ ofono_bool_t __ofono_gprs_provision_get_settings(const char *mcc,
void __ofono_gprs_provision_free_settings(
struct ofono_gprs_provision_data *settings,
int count);
+#endif
diff --git a/src/sim.c b/src/sim.c
index 3c5db90..a186cae 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -174,6 +174,7 @@ static gboolean password_is_pin(enum ofono_sim_password_type type)
case OFONO_SIM_PASSWORD_PHCORP_PUK:
case OFONO_SIM_PASSWORD_INVALID:
case OFONO_SIM_PASSWORD_NONE:
+ case OFONO_PASSWORD_PIN_TYPE_NET:
return FALSE;
}
@@ -675,7 +676,7 @@ static DBusMessage *sim_lock_or_unlock(struct ofono_sim *sim, int lock,
type == OFONO_SIM_PASSWORD_SIM_PIN2)
return __ofono_error_invalid_format(msg);
- if (!is_valid_pin(pin, PIN_TYPE_PIN))
+ if (!is_valid_pin(pin, type))
return __ofono_error_invalid_format(msg);
sim->pending = dbus_message_ref(msg);
@@ -747,10 +748,10 @@ static DBusMessage *sim_change_pin(DBusConnection *conn, DBusMessage *msg,
if (password_is_pin(type) == FALSE)
return __ofono_error_invalid_format(msg);
- if (!is_valid_pin(old, PIN_TYPE_PIN))
+ if (!is_valid_pin(old, type))
return __ofono_error_invalid_format(msg);
- if (!is_valid_pin(new, PIN_TYPE_PIN))
+ if (!is_valid_pin(new, type))
return __ofono_error_invalid_format(msg);
if (!strcmp(new, old))
@@ -802,7 +803,7 @@ static DBusMessage *sim_enter_pin(DBusConnection *conn, DBusMessage *msg,
if (type == OFONO_SIM_PASSWORD_NONE || type != sim->pin_type)
return __ofono_error_invalid_format(msg);
- if (!is_valid_pin(pin, PIN_TYPE_PIN))
+ if (!is_valid_pin(pin, type))
return __ofono_error_invalid_format(msg);
sim->pending = dbus_message_ref(msg);
@@ -1012,10 +1013,10 @@ static DBusMessage *sim_reset_pin(DBusConnection *conn, DBusMessage *msg,
if (type == OFONO_SIM_PASSWORD_NONE || type != sim->pin_type)
return __ofono_error_invalid_format(msg);
- if (!is_valid_pin(puk, PIN_TYPE_PUK))
+ if (!is_valid_pin(puk, type))
return __ofono_error_invalid_format(msg);
- if (!is_valid_pin(pin, PIN_TYPE_PIN))
+ if (!is_valid_pin(pin, type))
return __ofono_error_invalid_format(msg);
sim->pending = dbus_message_ref(msg);
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: Ofono has too tight limits for SIM lock code lengths
2011-01-27 12:52 Jussi Kangas
@ 2011-01-27 13:09 ` Marcel Holtmann
2011-01-27 14:31 ` Jussi Kangas
0 siblings, 1 reply; 6+ messages in thread
From: Marcel Holtmann @ 2011-01-27 13:09 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 895 bytes --]
Hi Jussi,
> ---
> include/sim.h | 1 +
> src/call-barring.c | 14 ++++++++------
> src/call-meter.c | 4 ++--
> src/common.c | 29 ++++++++++++++++++++++++-----
> src/common.h | 11 +++--------
> src/ofono.h | 3 +++
> src/sim.c | 13 +++++++------
> 7 files changed, 48 insertions(+), 27 deletions(-)
>
> diff --git a/include/sim.h b/include/sim.h
> index 5e3ba5b..ee63d74 100644
> --- a/include/sim.h
> +++ b/include/sim.h
> @@ -54,6 +54,7 @@ enum ofono_sim_password_type {
> OFONO_SIM_PASSWORD_PHNETSUB_PUK,
> OFONO_SIM_PASSWORD_PHSP_PUK,
> OFONO_SIM_PASSWORD_PHCORP_PUK,
> + OFONO_PASSWORD_PIN_TYPE_NET,
> OFONO_SIM_PASSWORD_INVALID,
> };
so every single enum has kept some sort of proper namespacing. And I
would really prefer to not break this for ofono_sim_password_type.
Regards
Marcel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Ofono has too tight limits for SIM lock code lengths
2011-01-27 13:09 ` Marcel Holtmann
@ 2011-01-27 14:31 ` Jussi Kangas
2011-01-28 15:20 ` Pekka Pessi
0 siblings, 1 reply; 6+ messages in thread
From: Jussi Kangas @ 2011-01-27 14:31 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 959 bytes --]
Hi Marcel,
On Thu, 2011-01-27 at 15:09 +0200, Marcel Holtmann wrote:
> so every single enum has kept some sort of proper namespacing. And I
> would really prefer to not break this for ofono_sim_password_type.
I suppose u are talking about parameter name here? and
OFONO_PASSWORD_PIN_TYPE_NET is somehow inproper? Well, how should it be
then? Code type is named differently because it is not a SIM password. I
see three possible ways to change this.
1) Rename enum. I would name it to ofono_password_type and all
parameters to OFONO_PASSWORD_xxx. Means lot's of more changing in
different directories and since this version control system is as it is
I would not like to do that.
2) Ignore the fact that network password used for call_barring is not
SIM password and rename it as OFONO_SIM_PASSWORD_TYPE_NET. Little bit
misguiding but fine with me.
3) Create new separate definition. I'm fine with this too.
Br,
-jussi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Ofono has too tight limits for SIM lock code lengths
2011-01-27 14:31 ` Jussi Kangas
@ 2011-01-28 15:20 ` Pekka Pessi
0 siblings, 0 replies; 6+ messages in thread
From: Pekka Pessi @ 2011-01-28 15:20 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]
Moro Jussi,
2011/1/27 Jussi Kangas <jussi.kangas@tieto.com>:
>> so every single enum has kept some sort of proper namespacing. And I
>> would really prefer to not break this for ofono_sim_password_type.
>
> I suppose u are talking about parameter name here? and
> OFONO_PASSWORD_PIN_TYPE_NET is somehow inproper? Well, how should it be
> then? Code type is named differently because it is not a SIM password. I
> see three possible ways to change this.
>
> 1) Rename enum. I would name it to ofono_password_type and all
> parameters to OFONO_PASSWORD_xxx. Means lot's of more changing in
> different directories and since this version control system is as it is
> I would not like to do that.
>
> 2) Ignore the fact that network password used for call_barring is not
> SIM password and rename it as OFONO_SIM_PASSWORD_TYPE_NET. Little bit
> misguiding but fine with me.
>
> 3) Create new separate definition. I'm fine with this too.
What about PIN_TYPE_PH_PIN and
static enum pin_type sim_get_pin_type(enum ofono_sim_password_type pwtype)
returning PIN_TYPE_PIN, PIN_TYPE_PUK or PIN_TYPE_PH_PIN?
so instead of
if (!is_valid_pin(pin, PIN_TYPE_PIN))
you would use
if (!is_valid_pin(pin, sim_get_pin_type(type)))
--
Pekka.Pessi mail at nokia.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-01-28 15:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-19 9:04 Ofono has too tight limits for SIM lock code lengths Jussi Kangas
2011-01-20 17:35 ` Denis Kenzior
-- strict thread matches above, loose matches on Subject: below --
2011-01-27 12:52 Jussi Kangas
2011-01-27 13:09 ` Marcel Holtmann
2011-01-27 14:31 ` Jussi Kangas
2011-01-28 15:20 ` Pekka Pessi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.