Re: [PATCH] sim: validate IMS private identity
Hi Sergey, This field may not be defined for ISIM in use. In this case the field still can be read from ISIM, though it will not contain a valid UTF8 string. For instance, it may contain 255 0xFF bytes. Ugh, seems like the SIM vendor can't follow RFC's either? 31.103 Section 4.2.2 says: "For contents and syntax of NAI TLV data object values see IETF RFC 2486 [24]. The NAI shall be encoded to an octet string according to UTF-8 encoding rules as specified in IETF RFC 3629 [27]. The tag value of the NAI TLV data object shall be '80'. " This crash occured during my experiments with eSIM. As I mentioned, the content of that TLV data object was 0xff bytes. IIUC it looks like vendor could just skip initialization of that particular TLV data object during bootstrap. Though I am not yet familiar with eSIM init procedure... I figured. The likely reason for this is that SIM strings are generally encoded in another format. If you're interested, see src/util.c sim_string_to_utf8(). 0xff is used as padding in such cases and thus a field with only 0xffs is treated as empty. However, the above would seem not to apply to EFimpi and a few others that 3GPP wants encoded as a utf-8 string. So, likely, a bug on the SIM, but we should have sanitized the contents better. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] sim: validate IMS private identity
Hi Denis, > > Make sure that IMPI is a valid UTF8 string before attempting > > to report it via DBus. Otherwise ofono may crash on dbus assert. > > This field may not be defined for ISIM in use. In this case the > > field still can be read from ISIM, though it will not contain > > a valid UTF8 string. For instance, it may contain 255 0xFF bytes. > > Ugh, seems like the SIM vendor can't follow RFC's either? 31.103 Section > 4.2.2 says: > > "For contents and syntax of NAI TLV data object values see IETF RFC 2486 > [24]. The NAI shall be encoded to an octet string according to UTF-8 > encoding rules as specified in IETF RFC 3629 [27]. The tag value of the NAI > TLV data object shall be '80'. " This crash occured during my experiments with eSIM. As I mentioned, the content of that TLV data object was 0xff bytes. IIUC it looks like vendor could just skip initialization of that particular TLV data object during bootstrap. Though I am not yet familiar with eSIM init procedure... > > --- > > src/sim.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/sim.c b/src/sim.c > > index 33e1245f..f60f5d1b 100644 > > --- a/src/sim.c > > +++ b/src/sim.c > > @@ -423,7 +423,7 @@ static DBusMessage *sim_get_properties(DBusConnection > > *conn, > > ofono_dbus_dict_append(&dict, "ServiceProviderName", > > DBUS_TYPE_STRING, &sim->spn); > > - if (sim->impi) > > + if (sim->impi && g_utf8_validate(sim->impi, 255, NULL)) > > Hmm, this would imply that we're setting sim->impi incorrectly.. Also, > since we have __ofono_sim_get_impi() API, the better fix would be to make > sure sim->impi is set correctly in impi_read_cb() Ok. I will set sim->impi in impi_read_cb only if it is a valid UTF8 string. Regards, Sergey ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] sim: validate IMS private identity
Hi Sergey, On 1/15/21 10:38 AM, Sergey Matyukevich wrote: Make sure that IMPI is a valid UTF8 string before attempting to report it via DBus. Otherwise ofono may crash on dbus assert. This field may not be defined for ISIM in use. In this case the field still can be read from ISIM, though it will not contain a valid UTF8 string. For instance, it may contain 255 0xFF bytes. Ugh, seems like the SIM vendor can't follow RFC's either? 31.103 Section 4.2.2 says: "For contents and syntax of NAI TLV data object values see IETF RFC 2486 [24]. The NAI shall be encoded to an octet string according to UTF-8 encoding rules as specified in IETF RFC 3629 [27]. The tag value of the NAI TLV data object shall be '80'. " --- src/sim.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sim.c b/src/sim.c index 33e1245f..f60f5d1b 100644 --- a/src/sim.c +++ b/src/sim.c @@ -423,7 +423,7 @@ static DBusMessage *sim_get_properties(DBusConnection *conn, ofono_dbus_dict_append(&dict, "ServiceProviderName", DBUS_TYPE_STRING, &sim->spn); - if (sim->impi) + if (sim->impi && g_utf8_validate(sim->impi, 255, NULL)) Hmm, this would imply that we're setting sim->impi incorrectly.. Also, since we have __ofono_sim_get_impi() API, the better fix would be to make sure sim->impi is set correctly in impi_read_cb() ofono_dbus_dict_append(&dict, "ImsPrivateIdentity", DBUS_TYPE_STRING, &sim->impi); ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
[PATCH] sim: validate IMS private identity
Make sure that IMPI is a valid UTF8 string before attempting to report it via DBus. Otherwise ofono may crash on dbus assert. This field may not be defined for ISIM in use. In this case the field still can be read from ISIM, though it will not contain a valid UTF8 string. For instance, it may contain 255 0xFF bytes. --- src/sim.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sim.c b/src/sim.c index 33e1245f..f60f5d1b 100644 --- a/src/sim.c +++ b/src/sim.c @@ -423,7 +423,7 @@ static DBusMessage *sim_get_properties(DBusConnection *conn, ofono_dbus_dict_append(&dict, "ServiceProviderName", DBUS_TYPE_STRING, &sim->spn); - if (sim->impi) + if (sim->impi && g_utf8_validate(sim->impi, 255, NULL)) ofono_dbus_dict_append(&dict, "ImsPrivateIdentity", DBUS_TYPE_STRING, &sim->impi); ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
