Re: [PATCH 4/4] plugins/provisioning and mbpi: support for auth NONE

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Giacinto,

On 10/04/2018 12:20 AM, Giacinto Cifelli wrote:
> support of 'none' in file-provisioning and in mbpi.
> The default authentication method is set to 'none'.
> ---
>   plugins/file-provision.c | 7 +++++--
>   plugins/mbpi.c           | 5 ++++-
>   2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/plugins/file-provision.c b/plugins/file-provision.c
> index d4846a65..3a1a5a68 100644
> --- a/plugins/file-provision.c
> +++ b/plugins/file-provision.c
> @@ -93,18 +93,21 @@ static int config_file_provision_get_settings(const char *mcc,
>   	if (value != NULL)
>   		(*settings)[0].password = value;
>   
> -	(*settings)[0].auth_method = OFONO_GPRS_AUTH_METHOD_CHAP;
> +	/* select default authentication method */
> +	(*settings)[0].auth_method = OFONO_GPRS_AUTH_METHOD_NONE;
> +
>   	value = g_key_file_get_string(key_file, setting_group,
>   					"internet.AuthenticationMethod", NULL);
>   
>   	if (value != NULL) {
> +

Not sure why this is here

>   		if (g_strcmp0(value, "chap") == 0)
>   			(*settings)[0].auth_method =
>   						OFONO_GPRS_AUTH_METHOD_CHAP;
>   		else if (g_strcmp0(value, "pap") == 0)
>   			(*settings)[0].auth_method =
>   						OFONO_GPRS_AUTH_METHOD_PAP;
> -		else
> +		else if (g_strcmp0(value, "none") != 0)
>   			DBG("Unknown auth method: %s", value);
>   
>   		g_free(value);
> diff --git a/plugins/mbpi.c b/plugins/mbpi.c
> index ae92c762..d101085f 100644
> --- a/plugins/mbpi.c
> +++ b/plugins/mbpi.c
> @@ -148,11 +148,14 @@ static void authentication_start(GMarkupParseContext *context,
>   		return;
>   	}
>   
> +	/* select default authentication method */
> +	*auth_method = OFONO_GPRS_AUTH_METHOD_NONE;
> +
>   	if (strcmp(text, "chap") == 0)
>   		*auth_method = OFONO_GPRS_AUTH_METHOD_CHAP;
>   	else if (strcmp(text, "pap") == 0)
>   		*auth_method = OFONO_GPRS_AUTH_METHOD_PAP;
> -	else
> +	else if (strcmp(text, "none") != 0)

Please study the serviceproviders.2.dtd which is part of the 
mobile-broadband-provider-info project.  'none' is not a valid value.

It looks like mbpi assumes that authentication tag can either be 
omitted, but if present it has to specify chap or pap.  So if empty we 
can probably assume 'none'.  Of course there's nothing preventing the 
entry from having a username/password and no authentication tag.  In 
which case we should assume chap.

Unfortunately authentication_start method might not have all the 
information available to make the above decisions, so you might need 
extra fixup code elsewhere to actually sanitize auth_method based on 
username/password presence.

>   		mbpi_g_set_error(context, error, G_MARKUP_ERROR,
>   					G_MARKUP_ERROR_UNKNOWN_ATTRIBUTE,
>   					"Unknown authentication method: %s",
> 

Regards,
-Denis