| Message ID | 5842EA9CC042B141995329508713AD672F8A60C1@ILMAIL1.corp.local |
|---|---|
| State | Changes Requested |
| Headers | show |
On Tue, Jun 14, 2016 at 12:49:44PM +0000, Cedric Izoard wrote: > Compute the actual language code length and don't assume > it is 3 characters long Would you happen to have an example where this is needed and the current implementation not handling a two character language code? > diff --git a/hs20/client/osu_client.c b/hs20/client/osu_client.c > @@ -2794,18 +2794,20 @@ static int osu_cert_cb(void *_ctx, struct http_cert *cert) > + int lang_len = os_strlen(ctx->friendly_name[j].lang); > + > for (i = 0; i < cert->num_othername; i++) { > if (os_strcmp(cert->othername[i].oid, > "1.3.6.1.4.1.40808.1.1.1") != 0) > continue; > - if (cert->othername[i].len < 3) > + if (cert->othername[i].len < lang_len) > continue; This does not look correct. id-wfa-hotspot-friendlyName is defined in a way that it shall start with a three octet field containing the country code. If this is a two octet country code, there would still need to be three octets with the last one being 0x00. > if (os_strncasecmp((char *) cert->othername[i].data, > - ctx->friendly_name[j].lang, 3) != 0) > + ctx->friendly_name[j].lang, lang_len) != 0) This would not catch a case where ctx->friendly_name[j].lang is a two octet value and cert->othername[i].data has a three octet country code. Such a case should not allow to be continued.. os_strncasecmp with fixed length 3 does check for that as well. > - if (os_strncmp((char *) cert->othername[i].data + 3, > + if (os_strncmp((char *) cert->othername[i].data + lang_len, > ctx->friendly_name[j].text, > - cert->othername[i].len - 3) == 0) { > + cert->othername[i].len - lang_len) == 0) { This is not correct either since cert->othername[i].data (i.e., id-wfa-hotspot-friendlyName) starts with a fixed length three octet country code field.
> > Compute the actual language code length and don't assume it is 3 > > characters long > > Would you happen to have an example where this is needed and the current > implementation not handling a two character language code? > > > diff --git a/hs20/client/osu_client.c b/hs20/client/osu_client.c @@ > > -2794,18 +2794,20 @@ static int osu_cert_cb(void *_ctx, struct > > http_cert *cert) > > + int lang_len = os_strlen(ctx->friendly_name[j].lang); > > + > > for (i = 0; i < cert->num_othername; i++) { > > if (os_strcmp(cert->othername[i].oid, > > "1.3.6.1.4.1.40808.1.1.1") != 0) > > continue; > > - if (cert->othername[i].len < 3) > > + if (cert->othername[i].len < lang_len) > > continue; > > This does not look correct. id-wfa-hotspot-friendlyName is defined in a way > that it shall start with a three octet field containing the country code. If > this is a two octet country code, there would still need to be three octets > with the last one being 0x00. You're right you can forget this patch I tested using a certificate that wasn't correct and didn't take time to check the spec in details. Sorry for the noise. cedric
diff --git a/hs20/client/osu_client.c b/hs20/client/osu_client.c index c05c57d..deb1301 100644 --- a/hs20/client/osu_client.c +++ b/hs20/client/osu_client.c @@ -2794,18 +2794,20 @@ static int osu_cert_cb(void *_ctx, struct http_cert *cert) for (j = 0; !ctx->no_osu_cert_validation && j < ctx->friendly_name_count; j++) { int found = 0; + int lang_len = os_strlen(ctx->friendly_name[j].lang); + for (i = 0; i < cert->num_othername; i++) { if (os_strcmp(cert->othername[i].oid, "1.3.6.1.4.1.40808.1.1.1") != 0) continue; - if (cert->othername[i].len < 3) + if (cert->othername[i].len < lang_len) continue; if (os_strncasecmp((char *) cert->othername[i].data, - ctx->friendly_name[j].lang, 3) != 0) + ctx->friendly_name[j].lang, lang_len) != 0) continue; - if (os_strncmp((char *) cert->othername[i].data + 3, + if (os_strncmp((char *) cert->othername[i].data + lang_len, ctx->friendly_name[j].text, - cert->othername[i].len - 3) == 0) { + cert->othername[i].len - lang_len) == 0) { found = 1; break; }
Compute the actual language code length and don't assume it is 3 characters long Signed-off-by: Cedric Izoard <cedric.izoard@ceva-dsp.com> --- hs20/client/osu_client.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)