connman-api: added "none" auth_method

PATCH Applied Giacinto Cifelli 2 participants 11 messages Oct 6, 2018 – Oct 9, 2018

Share & tools

Giacinto Cifelli

author

2018-10-06 05:45 UTC #0 lore raw

PATCH 1/5 connman-api: added "none" auth_method

---
 doc/connman-api.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

doc/connman-api.txt +2 -1

index 9220d0de..578b9755 100644
@@ -192,7 +192,8 @@ Properties	boolean Active [readwrite]
 
 		string AuthenticationMethod [readwrite]
 			Holds the PPP authentication method to use.  Valid
-			values are "pap" and "chap".  Defaults to "chap".
+			values are "pap", "chap" and "none".
+			Defaults to "chap".
 
 		string Username [readwrite]

Signature · 1 line

2.17.1

Giacinto Cifelli

author

2018-10-06 05:45 UTC #1 lore raw

PATCH 2/5 gprs-context: added OFONO_GPRS_AUTH_METHOD_NONE

This method makes explicit the lack of authentication.

When selected, the username and password are ignored, but they are not changed in the user-defined properties for the context.
This treatment is necessary to allow setting independently auth_method, username and password.

This method is also selected implicitly when username is set to an empty string. Also this selection is done without changing the user-defined auth_method for the context, so that the behavior is consistent.

---
 include/gprs-context.h | 1 +
 1 file changed, 1 insertion(+)

include/gprs-context.h +1 -0

index 20ca9efc..8869c12e 100644
@@ -57,6 +57,7 @@ enum ofono_gprs_context_type {
 enum ofono_gprs_auth_method {
 	OFONO_GPRS_AUTH_METHOD_CHAP = 0,
 	OFONO_GPRS_AUTH_METHOD_PAP,
+	OFONO_GPRS_AUTH_METHOD_NONE,
 };
 
 struct ofono_gprs_primary_context {

Signature · 1 line

2.17.1

Giacinto Cifelli

author

2018-10-06 05:45 UTC #2 lore raw

PATCH 3/5 src/gprs: support for NONE auth

In reply to Giacinto Cifelli

---
 src/gprs.c | 7 +++++++
 1 file changed, 7 insertions(+)

src/gprs.c +7 -0

index 79fafdbc..235c8884 100644
@@ -261,6 +261,10 @@ static const char *gprs_auth_method_to_string(enum ofono_gprs_auth_method auth)
 		return "chap";
 	case OFONO_GPRS_AUTH_METHOD_PAP:
 		return "pap";
+	case OFONO_GPRS_AUTH_METHOD_NONE:
+		return "none";
+	default:
+		return NULL;
 	};
 
 	return NULL;

@@ -275,6 +279,9 @@ static gboolean gprs_auth_method_from_string(const char *str,
 	} else if (g_str_equal(str, "pap")) {
 		*auth = OFONO_GPRS_AUTH_METHOD_PAP;
 		return TRUE;
+	} else if (g_str_equal(str, "none")) {
+		*auth = OFONO_GPRS_AUTH_METHOD_NONE;
+		return TRUE;
 	}
 
 	return FALSE;

Signature · 1 line

2.17.1

Denis Kenzior

Applied

2018-10-09 17:37 UTC #3 lore raw

Re: [PATCH 3/5] src/gprs: support for NONE auth

Hi Giacinto,

Quoted 3 lines · Giacinto Cifelli

---
  src/gprs.c | 7 +++++++
  1 file changed, 7 insertions(+)

Applied, see below:

src/gprs.c static const char *gprs_auth_method_to_string(enum ofono_gprs_auth_method auth) · line 267

Quoted 13 lines

src/gprs.c +4 -0

index 79fafdbc..235c8884 100644
@@ -261,6 +261,10 @@ static const char *gprs_auth_method_to_string(enum ofono_gprs_auth_method auth)
  		return "chap";
  	case OFONO_GPRS_AUTH_METHOD_PAP:
  		return "pap";
+	case OFONO_GPRS_AUTH_METHOD_NONE:
+		return "none";
+	default:
+		return NULL;

I dropped this default statement for reasons outlined in our earlier discussions.

Quoted 5 lines

src/gprs.c +0 -0

  	};
 
  	return NULL;

src/gprs.c static gboolean gprs_auth_method_from_string(const char *str, · line 287

Quoted 9 lines

src/gprs.c +3 -0

@@ -275,6 +279,9 @@ static gboolean gprs_auth_method_from_string(const char *str,
  	} else if (g_str_equal(str, "pap")) {
  		*auth = OFONO_GPRS_AUTH_METHOD_PAP;
  		return TRUE;
+	} else if (g_str_equal(str, "none")) {
+		*auth = OFONO_GPRS_AUTH_METHOD_NONE;
+		return TRUE;
  	}
 
  	return FALSE;

Regards,
-Denis

Giacinto Cifelli

author

2018-10-09 18:49 UTC #4 lore raw

Re: [PATCH 3/5] src/gprs: support for NONE auth

Hi Denis,

Quoted 11 lines · Denis Kenzior

Hi Giacinto,
Quoted 3 lines · Giacinto Cifelli
---
  src/gprs.c | 7 +++++++
  1 file changed, 7 insertions(+)
Applied, see below:
Quoted 1 line

Quoted 15 lines

src/gprs.c static const char *gprs_auth_method_to_string(enum ofono_gprs_auth_method auth) · line 267

Quoted 12 lines

src/gprs.c +4 -0

index 79fafdbc..235c8884 100644
@@ -261,6 +261,10 @@ static const char *gprs_auth_method_to_string(enum ofono_gprs_auth_method auth)
              return "chap";
      case OFONO_GPRS_AUTH_METHOD_PAP:
              return "pap";
+     case OFONO_GPRS_AUTH_METHOD_NONE:
+             return "none";
+     default:
+             return NULL;

I dropped this default statement for reasons outlined in our earlier discussions.

oh yes, thanks. I forgot to change this too.

Quoted 13 lines

src/gprs.c +3 -0

      };
 
      return NULL;

@@ -275,6 +279,9 @@ static gboolean gprs_auth_method_from_string(const char *str,
      } else if (g_str_equal(str, "pap")) {
              *auth = OFONO_GPRS_AUTH_METHOD_PAP;
              return TRUE;
+     } else if (g_str_equal(str, "none")) {
+             *auth = OFONO_GPRS_AUTH_METHOD_NONE;
+             return TRUE;
      }

Quoted 5 lines

src/gprs.c static gboolean gprs_auth_method_from_string(const char *str, · line 287
Quoted 2 lines
src/gprs.c +0 -0
      return FALSE;
Regards,
-Denis

Regards,
Giacinto

Giacinto Cifelli

author

2018-10-06 05:45 UTC #5 lore raw

PATCH 4/5 plugins/provisioning and mbpi: support for auth NONE

In reply to Giacinto Cifelli

support of 'none' in file-provisioning:
the default authentication method is set to NONE

support of 'none' in mbpi:
the default method remains CHAP, but it is overridden by NONE after parsing the entire key for the apn and detecting no username/password

---
 plugins/file-provision.c | 7 +++++--
 plugins/mbpi.c           | 6 ++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

plugins/file-provision.c +5 -2

index d4846a65..3a1a5a68 100644
@@ -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) {
+
 		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);

plugins/mbpi.c +6 -0

index ae92c762..433f1b55 100644
@@ -325,6 +325,8 @@ static void apn_handler(GMarkupParseContext *context, struct gsm_data *gsm,
 	ap->apn = g_strdup(apn);
 	ap->type = OFONO_GPRS_CONTEXT_TYPE_INTERNET;
 	ap->proto = OFONO_GPRS_PROTO_IP;
+
+	/* pre-select default authentication method */
 	ap->auth_method = OFONO_GPRS_AUTH_METHOD_CHAP;
 
 	g_markup_parse_context_push(context, &apn_parser, ap);

@@ -395,6 +397,10 @@ static void gsm_end(GMarkupParseContext *context, const gchar *element_name,
 	if (ap == NULL)
 		return;
 
+	/* select authentication method NONE if fit */
+	if (!ap->username || !ap->password)
+		ap->auth_method = OFONO_GPRS_AUTH_METHOD_NONE;
+
 	if (gsm->allow_duplicates == FALSE) {
 		GSList *l;

Signature · 1 line

2.17.1

Denis Kenzior

2018-10-09 17:38 UTC #6 lore raw

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

Hi Giacinto,

Quoted 11 lines · Giacinto Cifelli

support of 'none' in file-provisioning:
the default authentication method is set to NONE
support of 'none' in mbpi:
the default method remains CHAP, but it is overridden by NONE after parsing the entire key for the apn and detecting no username/password
---
  plugins/file-provision.c | 7 +++++--
  plugins/mbpi.c           | 6 ++++++
  2 files changed, 11 insertions(+), 2 deletions(-)

I broke this up into two separate commits, also:

Quoted 7 lines

plugins/file-provision.c +0 -0

index d4846a65..3a1a5a68 100644
@@ -93,18 +93,21 @@ static int config_file_provision_get_settings(const char *mcc,
  	if (value != NULL)
  		(*settings)[0].password = value;

plugins/file-provision.c static int config_file_provision_get_settings(const char *mcc, · line 103

Quoted 10 lines

plugins/file-provision.c +4 -1

 
-	(*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) {
+

This part was dropped

Quoted 26 lines

plugins/file-provision.c +1 -1

  		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);

plugins/mbpi.c +2 -0

index ae92c762..433f1b55 100644
@@ -325,6 +325,8 @@ static void apn_handler(GMarkupParseContext *context, struct gsm_data *gsm,
  	ap->apn = g_strdup(apn);
  	ap->type = OFONO_GPRS_CONTEXT_TYPE_INTERNET;
  	ap->proto = OFONO_GPRS_PROTO_IP;
+
+	/* pre-select default authentication method */
  	ap->auth_method = OFONO_GPRS_AUTH_METHOD_CHAP;
 
  	g_markup_parse_context_push(context, &apn_parser, ap);

plugins/mbpi.c static void gsm_end(GMarkupParseContext *context, const gchar *element_name, · line 406

Quoted 10 lines

plugins/mbpi.c +4 -0

@@ -395,6 +397,10 @@ static void gsm_end(GMarkupParseContext *context, const gchar *element_name,
  	if (ap == NULL)
  		return;
 
+	/* select authentication method NONE if fit */
+	if (!ap->username || !ap->password)
+		ap->auth_method = OFONO_GPRS_AUTH_METHOD_NONE;
+
  	if (gsm->allow_duplicates == FALSE) {
  		GSList *l;

Regards,
-Denis

Giacinto Cifelli

author

2018-10-06 05:45 UTC #7 lore raw

PATCH 5/5 drivers: support for auth NONE

In reply to Giacinto Cifelli

Added the explicit support for auth NONE.
It needs to be added in all drivers/*/gprs-context.c atoms.

This method is already supported by all atoms that support authentication (ie, all but Sierra' swmodem driver).

The behavior is left unchanged in case of inconsistent parameters:
if username is empty, then fallback to auth NONE.

---
 drivers/atmodem/gprs-context.c        | 18 ++++++++++++----
 drivers/ifxmodem/gprs-context.c       | 10 +++++++--
 drivers/isimodem/gprs-context.c       | 14 ++++++++-----
 drivers/mbimmodem/gprs-context.c      | 16 +++++++++++---
 drivers/mbmmodem/gprs-context.c       | 11 +++++-----
 drivers/qmimodem/gprs-context.c       | 30 +++++++++++++++------------
 drivers/rilmodem/gprs-context.c       | 13 ++++++++----
 drivers/stemodem/gprs-context.c       |  9 ++++----
 drivers/telitmodem/gprs-context-ncm.c | 10 +++++++--
 drivers/ubloxmodem/gprs-context.c     |  7 ++++---
 10 files changed, 93 insertions(+), 45 deletions(-)

drivers/atmodem/gprs-context.c +14 -4

index 79ac4c8e..2212af69 100644
@@ -158,7 +158,10 @@ static gboolean setup_ppp(struct ofono_gprs_context *gc)
 		g_at_ppp_set_debug(gcd->ppp, ppp_debug, "PPP");
 
 	g_at_ppp_set_auth_method(gcd->ppp, gcd->auth_method);
-	g_at_ppp_set_credentials(gcd->ppp, gcd->username, gcd->password);
+
+	if (gcd->auth_method != G_AT_PPP_AUTH_METHOD_NONE)
+		g_at_ppp_set_credentials(gcd->ppp, gcd->username,
+								gcd->password);
 
 	/* set connect and disconnect callbacks */
 	g_at_ppp_set_connect_function(gcd->ppp, ppp_connect, gc);

@@ -247,7 +250,7 @@ static void at_gprs_activate_primary(struct ofono_gprs_context *gc,
 	memcpy(gcd->username, ctx->username, sizeof(ctx->username));
 	memcpy(gcd->password, ctx->password, sizeof(ctx->password));
 
-	/* We only support CHAP and PAP */
+	/* We support CHAP, PAP and NONE */
 	switch (ctx->auth_method) {
 	case OFONO_GPRS_AUTH_METHOD_CHAP:
 		gcd->auth_method = G_AT_PPP_AUTH_METHOD_CHAP;

@@ -255,8 +258,11 @@ static void at_gprs_activate_primary(struct ofono_gprs_context *gc,
 	case OFONO_GPRS_AUTH_METHOD_PAP:
 		gcd->auth_method = G_AT_PPP_AUTH_METHOD_PAP;
 		break;
-	default:
-		goto error;
+	case OFONO_GPRS_AUTH_METHOD_NONE:
+		gcd->auth_method = G_AT_PPP_AUTH_METHOD_NONE;
+		memset(gcd->username, 0, sizeof(gcd->username));
+		memset(gcd->password, 0, sizeof(gcd->password));
+		break;
 	}
 
 	gcd->state = STATE_ENABLING;

@@ -304,6 +310,10 @@ static void at_gprs_activate_primary(struct ofono_gprs_context *gc,
 				snprintf(buf + len, sizeof(buf) - len - 3,
 						",\"PAP:%s\"", ctx->apn);
 				break;
+			case OFONO_GPRS_AUTH_METHOD_NONE:
+				snprintf(buf + len, sizeof(buf) - len - 3,
+						",\"%s\"", ctx->apn);
+				break;
 			}
 			break;
 		default:

drivers/ifxmodem/gprs-context.c +8 -2

index 885e41bb..289b4341 100644
@@ -466,8 +466,14 @@ static void ifx_gprs_activate_primary(struct ofono_gprs_context *gc,
 	gcd->active_context = ctx->cid;
 	gcd->cb = cb;
 	gcd->cb_data = data;
-	memcpy(gcd->username, ctx->username, sizeof(ctx->username));
-	memcpy(gcd->password, ctx->password, sizeof(ctx->password));
+
+	if (ctx->auth_method == OFONO_GPRS_AUTH_METHOD_NONE) {
+		memset(gcd->username, 0, sizeof(gcd->username));
+		memset(gcd->password, 0, sizeof(gcd->password));
+	} else {
+		memcpy(gcd->username, ctx->username, sizeof(ctx->username));
+		memcpy(gcd->password, ctx->password, sizeof(ctx->password));
+	}
 
 	gcd->state = STATE_ENABLING;
 	gcd->proto = ctx->proto;

drivers/isimodem/gprs-context.c +9 -5

index ce53d022..4d2e7a12 100644
@@ -538,11 +538,15 @@ static void isi_gprs_activate_primary(struct ofono_gprs_context *gc,
 	strncpy(cd->apn, ctx->apn, GPDS_MAX_APN_STRING_LENGTH);
 	cd->apn[GPDS_MAX_APN_STRING_LENGTH] = '\0';
 
-	strncpy(cd->username, ctx->username, GPDS_MAX_USERNAME_LENGTH);
-	cd->username[GPDS_MAX_USERNAME_LENGTH] = '\0';
-
-	strncpy(cd->password, ctx->password, GPDS_MAX_PASSWORD_LENGTH);
-	cd->username[GPDS_MAX_PASSWORD_LENGTH] = '\0';
+	if (ctx->auth_method == OFONO_GPRS_AUTH_METHOD_NONE) {
+		memset(cd->username, 0, sizeof(cd->username));
+		memset(cd->password, 0, sizeof(cd->password));
+	} else {
+		strncpy(cd->username, ctx->username, GPDS_MAX_USERNAME_LENGTH);
+		cd->username[GPDS_MAX_USERNAME_LENGTH] = '\0';
+		strncpy(cd->password, ctx->password, GPDS_MAX_PASSWORD_LENGTH);
+		cd->username[GPDS_MAX_PASSWORD_LENGTH] = '\0';
+	}
 
 	cd->pep = g_isi_pep_create(cd->idx, NULL, NULL);
 	if (cd->pep == NULL)

drivers/mbimmodem/gprs-context.c +13 -3

index 79793c92..be256e43 100644
@@ -75,9 +75,11 @@ static uint32_t auth_method_to_auth_protocol(enum ofono_gprs_auth_method method)
 		return 2; /* MBIMAuthProtocolChap */
 	case OFONO_GPRS_AUTH_METHOD_PAP:
 		return 1; /* MBIMAuthProtocolPap */
+	case OFONO_GPRS_AUTH_METHOD_NONE:
+		return 0; /* MBIMAUthProtocolNone */
 	}
 
-	return 0;
+	return 0; /* MBIMAUthProtocolNone */
 }
 
 static void mbim_deactivate_cb(struct mbim_message *message, void *user)

@@ -345,6 +347,8 @@ static void mbim_gprs_activate_primary(struct ofono_gprs_context *gc,
 {
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	struct mbim_message *message;
+	const char username = NULL;
+	const char password = NULL;
 
 	DBG("cid %u", ctx->cid);

@@ -354,6 +358,12 @@ static void mbim_gprs_activate_primary(struct ofono_gprs_context *gc,
 	gcd->active_context = ctx->cid;
 	gcd->proto = ctx->proto;
 
+	if (ctx->auth_method != OFONO_GPRS_AUTH_METHOD_NONE && ctx->username[0])
+		username = ctx->username;
+
+	if (ctx->auth_method != OFONO_GPRS_AUTH_METHOD_NONE && ctx->password[0])
+		password = ctx->password;
+
 	message = mbim_message_new(mbim_uuid_basic_connect,
 					MBIM_CID_CONNECT,
 					MBIM_COMMAND_TYPE_SET);

@@ -361,8 +371,8 @@ static void mbim_gprs_activate_primary(struct ofono_gprs_context *gc,
 				ctx->cid,
 				1, /* MBIMActivationCommandActivate */
 				ctx->apn,
-				ctx->username[0] ? ctx->username : NULL,
-				ctx->password[0] ? ctx->password : NULL,
+				username,
+				password,
 				0, /*MBIMCompressionNone */
 				auth_method_to_auth_protocol(ctx->auth_method),
 				proto_to_context_ip_type(ctx->proto),

drivers/mbmmodem/gprs-context.c +6 -5

index e961afa1..fa8b44b6 100644
@@ -394,11 +394,12 @@ static void mbm_gprs_activate_primary(struct ofono_gprs_context *gc,
 	 * Set username and password, this should be done after CGDCONT
 	 * or an error can occur.  We don't bother with error checking
 	 * here
-	 * */
-	snprintf(buf, sizeof(buf), "AT*EIAAUW=%d,1,\"%s\",\"%s\"",
-			ctx->cid, ctx->username, ctx->password);
-
-	g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
+	 */
+	if (ctx->auth_method != OFONO_GPRS_AUTH_METHOD_NONE) {
+		snprintf(buf, sizeof(buf), "AT*EIAAUW=%d,1,\"%s\",\"%s\"",
+				ctx->cid, ctx->username, ctx->password);
+		g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
+	}
 
 	return;

drivers/qmimodem/gprs-context.c +17 ~~-13~~

index 9a22b89f..985bdd64 100644
@@ -230,6 +230,20 @@ static void qmi_gprs_read_settings(struct ofono_gprs_context* gc,
 	g_free(cbd);
 }
 
+static uint8_t auth_method_to_qmi_auth(enum ofono_gprs_auth_method method)
+{
+	switch (method) {
+	case OFONO_GPRS_AUTH_METHOD_CHAP:
+		return QMI_WDS_AUTHENTICATION_CHAP;
+	case OFONO_GPRS_AUTH_METHOD_PAP:
+		return QMI_WDS_AUTHENTICATION_PAP;
+	case OFONO_GPRS_AUTH_METHOD_NONE:
+		return QMI_WDS_AUTHENTICATION_NONE;
+	}
+
+	return QMI_WDS_AUTHENTICATION_NONE;
+}
+
 static void qmi_activate_primary(struct ofono_gprs_context *gc,
 				const struct ofono_gprs_primary_context *ctx,
 				ofono_gprs_context_cb_t cb, void *user_data)

@@ -266,26 +280,16 @@ static void qmi_activate_primary(struct ofono_gprs_context *gc,
 
 	qmi_param_append_uint8(param, QMI_WDS_PARAM_IP_FAMILY, ip_family);
 
-	switch (ctx->auth_method) {
-	case OFONO_GPRS_AUTH_METHOD_CHAP:
-		auth = QMI_WDS_AUTHENTICATION_CHAP;
-		break;
-	case OFONO_GPRS_AUTH_METHOD_PAP:
-		auth = QMI_WDS_AUTHENTICATION_PAP;
-		break;
-	default:
-		auth = QMI_WDS_AUTHENTICATION_NONE;
-		break;
-	}
+	auth = auth_method_to_qmi_auth(ctx->auth_method);
 
 	qmi_param_append_uint8(param, QMI_WDS_PARAM_AUTHENTICATION_PREFERENCE,
 					auth);
 
-	if (ctx->username[0] != '\0')
+	if (auth != QMI_WDS_AUTHENTICATION_NONE && ctx->username[0] != '\0')
 		qmi_param_append(param, QMI_WDS_PARAM_USERNAME,
 					strlen(ctx->username), ctx->username);
 
-	if (ctx->password[0] != '\0')
+	if (auth != QMI_WDS_AUTHENTICATION_NONE &&  ctx->password[0] != '\0')
 		qmi_param_append(param, QMI_WDS_PARAM_PASSWORD,
 					strlen(ctx->password), ctx->password);

drivers/rilmodem/gprs-context.c +9 -4

index 1f476e23..ef62cba9 100644
@@ -598,9 +598,12 @@ static void ril_gprs_context_activate_primary(struct ofono_gprs_context *gc,
 	 * We do the same as in $AOSP/frameworks/opt/telephony/src/java/com/
 	 * android/internal/telephony/dataconnection/DataConnection.java,
 	 * onConnect(), and use authentication or not depending on whether
-	 * the user field is empty or not.
+	 * the user field is empty or not,
+	 * on top of the verification for the authentication method.
 	 */
-	if (ctx->username[0] != '\0')
+
+	if (ctx->auth_method != OFONO_GPRS_AUTH_METHOD_NONE &&
+						ctx->username[0] != '\0')
 		auth_type = RIL_AUTH_BOTH;
 	else
 		auth_type = RIL_AUTH_NONE;

@@ -615,8 +618,10 @@ static void ril_gprs_context_activate_primary(struct ofono_gprs_context *gc,
 		parcel_w_string(&rilp, buf);
 
 		g_ril_append_print_buf(gcd->ril, "(%d,%s,%s,%s,%s,%d,%s,%u)",
-				tech, profile, ctx->apn, ctx->username,
-				ctx->password, auth_type,
+				tech, profile, ctx->apn,
+				auth_type == RIL_AUTH_NONE ? "" : ctx->username,
+				auth_type == RIL_AUTH_NONE ? "" : ctx->password,
+				auth_type,
 				ril_util_gprs_proto_to_ril_string(ctx->proto),
 				ctx->cid);
 	} else

drivers/stemodem/gprs-context.c +5 -4

index 18b2bfa4..32facd8c 100644
@@ -307,10 +307,11 @@ static void ste_gprs_activate_primary(struct ofono_gprs_context *gc,
 	 * or an error can occur.  We don't bother with error checking
 	 * here
 	 */
-	snprintf(buf, sizeof(buf), "AT*EIAAUW=%d,1,\"%s\",\"%s\"",
-			ctx->cid, ctx->username, ctx->password);
-
-	g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
+	if (ctx->auth_method != OFONO_GPRS_AUTH_METHOD_NONE) {
+		snprintf(buf, sizeof(buf), "AT*EIAAUW=%d,1,\"%s\",\"%s\"",
+				ctx->cid, ctx->username, ctx->password);
+		g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
+	}
 
 	return;

drivers/telitmodem/gprs-context-ncm.c +8 -2

index 7139740c..9c9b9500 100644
@@ -277,7 +277,8 @@ static void setup_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		return;
 	}
 
-	if (gcd->username[0] && gcd->password[0])
+	if (gcd->auth_method != AUTH_METHOD_NONE &&
+					gcd->username[0] && gcd->password[0])
 		sprintf(buf, "AT#PDPAUTH=%u,%u,\"%s\",\"%s\"",
 			gcd->active_context, gcd->auth_method,
 			gcd->username, gcd->password);

@@ -320,7 +321,7 @@ static void telitncm_gprs_activate_primary(struct ofono_gprs_context *gc,
 	gcd->state = STATE_ENABLING;
 	gcd->proto = ctx->proto;
 
-	/* We only support CHAP and PAP */
+	/* We support CHAP, PAP and NONE */
 	switch (ctx->auth_method) {
 	case OFONO_GPRS_AUTH_METHOD_CHAP:
 		gcd->auth_method = AUTH_METHOD_CHAP;

@@ -328,6 +329,11 @@ static void telitncm_gprs_activate_primary(struct ofono_gprs_context *gc,
 	case OFONO_GPRS_AUTH_METHOD_PAP:
 		gcd->auth_method = AUTH_METHOD_PAP;
 		break;
+	case OFONO_GPRS_AUTH_METHOD_NONE:
+		gcd->auth_method = AUTH_METHOD_NONE;
+		gcd->username[0] = 0;
+		gcd->password[0] = 0;
+		break;
 	default:
 		goto error;
 	}

drivers/ubloxmodem/gprs-context.c +4 -3

index 6fe2719f..7eb18f09 100644
@@ -315,9 +315,10 @@ static void ublox_send_uauthreq(struct ofono_gprs_context *gc,
 	case OFONO_GPRS_AUTH_METHOD_CHAP:
 		auth = 2;
 		break;
-	default:
-		ofono_error("Unsupported auth type %u", auth_method);
-		return;
+	case OFONO_GPRS_AUTH_METHOD_NONE:
+		auth = 0;
+		username = password = "";
+		break;
 	}
 
 	snprintf(buf, sizeof(buf), "AT+UAUTHREQ=%u,%u,\"%s\",\"%s\"",

Signature · 1 line

2.17.1

Denis Kenzior

2018-10-09 17:41 UTC #8 lore raw

Re: [PATCH 5/5] drivers: support for auth NONE

Hi Giacinto,

I applied a modified version of this patch, see below:

Quoted 18 lines

drivers/mbimmodem/gprs-context.c +3 -1

index 79793c92..be256e43 100644
@@ -75,9 +75,11 @@ static uint32_t auth_method_to_auth_protocol(enum ofono_gprs_auth_method method)
  		return 2; /* MBIMAuthProtocolChap */
  	case OFONO_GPRS_AUTH_METHOD_PAP:
  		return 1; /* MBIMAuthProtocolPap */
+	case OFONO_GPRS_AUTH_METHOD_NONE:
+		return 0; /* MBIMAUthProtocolNone */
  	}
 
-	return 0;
+	return 0; /* MBIMAUthProtocolNone */
  }
 
  static void mbim_deactivate_cb(struct mbim_message *message, void *user)

drivers/mbimmodem/gprs-context.c static void mbim_gprs_activate_primary(struct ofono_gprs_context *gc, · line 351

Quoted 5 lines

drivers/mbimmodem/gprs-context.c +2 -0

@@ -345,6 +347,8 @@ static void mbim_gprs_activate_primary(struct ofono_gprs_context *gc,
  {
  	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
  	struct mbim_message *message;
+	const char username = NULL;
+	const char password = NULL;

These were changed to const char *

Quoted 39 lines

drivers/mbimmodem/gprs-context.c +8 -2

  	DBG("cid %u", ctx->cid);

@@ -354,6 +358,12 @@ static void mbim_gprs_activate_primary(struct ofono_gprs_context *gc,
  	gcd->active_context = ctx->cid;
  	gcd->proto = ctx->proto;
 
+	if (ctx->auth_method != OFONO_GPRS_AUTH_METHOD_NONE && ctx->username[0])
+		username = ctx->username;
+
+	if (ctx->auth_method != OFONO_GPRS_AUTH_METHOD_NONE && ctx->password[0])
+		password = ctx->password;
+
  	message = mbim_message_new(mbim_uuid_basic_connect,
  					MBIM_CID_CONNECT,
  					MBIM_COMMAND_TYPE_SET);

@@ -361,8 +371,8 @@ static void mbim_gprs_activate_primary(struct ofono_gprs_context *gc,
  				ctx->cid,
  				1, /* MBIMActivationCommandActivate */
  				ctx->apn,
-				ctx->username[0] ? ctx->username : NULL,
-				ctx->password[0] ? ctx->password : NULL,
+				username,
+				password,
  				0, /*MBIMCompressionNone */
  				auth_method_to_auth_protocol(ctx->auth_method),
  				proto_to_context_ip_type(ctx->proto),

drivers/mbmmodem/gprs-context.c +0 -4

index e961afa1..fa8b44b6 100644
@@ -394,11 +394,12 @@ static void mbm_gprs_activate_primary(struct ofono_gprs_context *gc,
  	 * Set username and password, this should be done after CGDCONT
  	 * or an error can occur.  We don't bother with error checking
  	 * here
-	 * */
-	snprintf(buf, sizeof(buf), "AT*EIAAUW=%d,1,\"%s\",\"%s\"",
-			ctx->cid, ctx->username, ctx->password);
-

drivers/mbmmodem/gprs-context.c static void mbm_gprs_activate_primary(struct ofono_gprs_context *gc, · line 405

Quoted 10 lines

drivers/mbmmodem/gprs-context.c +6 -1

-	g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
+	 */
+	if (ctx->auth_method != OFONO_GPRS_AUTH_METHOD_NONE) {
+		snprintf(buf, sizeof(buf), "AT*EIAAUW=%d,1,\"%s\",\"%s\"",
+				ctx->cid, ctx->username, ctx->password);
+		g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
+	}
 
  	return;

I dropped this part entirely. The reason is that we can have profiles with username/password & without operating on the same CID. Since the setting is permanent on the modem, we need to set it/clear it even when AUTH_METHOD_NONE is used.

If you disagree, please send a follow up series.

Quoted 10 lines

drivers/stemodem/gprs-context.c +0 -2

index 18b2bfa4..32facd8c 100644
@@ -307,10 +307,11 @@ static void ste_gprs_activate_primary(struct ofono_gprs_context *gc,
  	 * or an error can occur.  We don't bother with error checking
  	 * here
  	 */
-	snprintf(buf, sizeof(buf), "AT*EIAAUW=%d,1,\"%s\",\"%s\"",
-			ctx->cid, ctx->username, ctx->password);

drivers/stemodem/gprs-context.c static void ste_gprs_activate_primary(struct ofono_gprs_context *gc, · line 317

Quoted 10 lines

drivers/stemodem/gprs-context.c +5 -2

-
-	g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
+	if (ctx->auth_method != OFONO_GPRS_AUTH_METHOD_NONE) {
+		snprintf(buf, sizeof(buf), "AT*EIAAUW=%d,1,\"%s\",\"%s\"",
+				ctx->cid, ctx->username, ctx->password);
+		g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
+	}
 
  	return;

Dropped for the same reason as above

Regards,
-Denis

Giacinto Cifelli

author

2018-10-09 18:51 UTC #9 lore raw

Re: [PATCH 5/5] drivers: support for auth NONE

Hi Denis,

Quoted 21 lines · Denis Kenzior

Hi Giacinto,

I applied a modified version of this patch, see below:

Quoted 16 lines

drivers/mbimmodem/gprs-context.c +3 -1

index 79793c92..be256e43 100644
@@ -75,9 +75,11 @@ static uint32_t auth_method_to_auth_protocol(enum ofono_gprs_auth_method method)
              return 2; /* MBIMAuthProtocolChap */
      case OFONO_GPRS_AUTH_METHOD_PAP:
              return 1; /* MBIMAuthProtocolPap */
+     case OFONO_GPRS_AUTH_METHOD_NONE:
+             return 0; /* MBIMAUthProtocolNone */
      }
 
-     return 0;
+     return 0; /* MBIMAUthProtocolNone */
  }

Quoted 9 lines

drivers/mbimmodem/gprs-context.c static void mbim_gprs_activate_primary(struct ofono_gprs_context *gc, · line 351

Quoted 7 lines

drivers/mbimmodem/gprs-context.c +2 -0

  static void mbim_deactivate_cb(struct mbim_message *message, void *user)

@@ -345,6 +347,8 @@ static void mbim_gprs_activate_primary(struct ofono_gprs_context *gc,
  {
      struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
      struct mbim_message *message;
+     const char username = NULL;
+     const char password = NULL;

These were changed to const char *

shame on me. I should have seen this...

Quoted 48 lines

drivers/mbimmodem/gprs-context.c +8 -2

      DBG("cid %u", ctx->cid);

@@ -354,6 +358,12 @@ static void mbim_gprs_activate_primary(struct ofono_gprs_context *gc,
      gcd->active_context = ctx->cid;
      gcd->proto = ctx->proto;
 
+     if (ctx->auth_method != OFONO_GPRS_AUTH_METHOD_NONE && ctx->username[0])
+             username = ctx->username;
+
+     if (ctx->auth_method != OFONO_GPRS_AUTH_METHOD_NONE && ctx->password[0])
+             password = ctx->password;
+
      message = mbim_message_new(mbim_uuid_basic_connect,
                                      MBIM_CID_CONNECT,
                                      MBIM_COMMAND_TYPE_SET);

@@ -361,8 +371,8 @@ static void mbim_gprs_activate_primary(struct ofono_gprs_context *gc,
                              ctx->cid,
                              1, /* MBIMActivationCommandActivate */
                              ctx->apn,
-                             ctx->username[0] ? ctx->username : NULL,
-                             ctx->password[0] ? ctx->password : NULL,
+                             username,
+                             password,
                              0, /*MBIMCompressionNone */
                              auth_method_to_auth_protocol(ctx->auth_method),
                              proto_to_context_ip_type(ctx->proto),

drivers/mbmmodem/gprs-context.c +6 -5

index e961afa1..fa8b44b6 100644
@@ -394,11 +394,12 @@ static void mbm_gprs_activate_primary(struct ofono_gprs_context *gc,
       * Set username and password, this should be done after CGDCONT
       * or an error can occur.  We don't bother with error checking
       * here
-      * */
-     snprintf(buf, sizeof(buf), "AT*EIAAUW=%d,1,\"%s\",\"%s\"",
-                     ctx->cid, ctx->username, ctx->password);
-
-     g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
+      */
+     if (ctx->auth_method != OFONO_GPRS_AUTH_METHOD_NONE) {
+             snprintf(buf, sizeof(buf), "AT*EIAAUW=%d,1,\"%s\",\"%s\"",
+                             ctx->cid, ctx->username, ctx->password);
+             g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
+     }

Quoted 7 lines

drivers/mbmmodem/gprs-context.c static void mbm_gprs_activate_primary(struct ofono_gprs_context *gc, · line 405
Quoted 2 lines
drivers/mbmmodem/gprs-context.c +0 -0
      return;
 
I dropped this part entirely. The reason is that we can have profiles with username/password & without operating on the same CID. Since the setting is permanent on the modem, we need to set it/clear it even when AUTH_METHOD_NONE is used.

I don't know these modules, so I believe you and it is perfectly fine with me.

Quoted 21 lines

If you disagree, please send a follow up series.

Quoted 18 lines

drivers/stemodem/gprs-context.c +5 -4

index 18b2bfa4..32facd8c 100644
@@ -307,10 +307,11 @@ static void ste_gprs_activate_primary(struct ofono_gprs_context *gc,
       * or an error can occur.  We don't bother with error checking
       * here
       */
-     snprintf(buf, sizeof(buf), "AT*EIAAUW=%d,1,\"%s\",\"%s\"",
-                     ctx->cid, ctx->username, ctx->password);
-
-     g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
+     if (ctx->auth_method != OFONO_GPRS_AUTH_METHOD_NONE) {
+             snprintf(buf, sizeof(buf), "AT*EIAAUW=%d,1,\"%s\",\"%s\"",
+                             ctx->cid, ctx->username, ctx->password);
+             g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
+     }

Quoted 7 lines

drivers/stemodem/gprs-context.c static void ste_gprs_activate_primary(struct ofono_gprs_context *gc, · line 317
Quoted 2 lines
drivers/stemodem/gprs-context.c +0 -0
      return;
 
Dropped for the same reason as above
Regards,
-Denis

Regards,
Giacinto

Denis Kenzior

2018-10-09 17:37 UTC #10 lore raw

In reply to Giacinto Cifelli

Hi Giacinto,

Quoted 4 lines · Giacinto Cifelli

---
  doc/connman-api.txt | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

Patch 1 & 2 applied, thanks.

Regards,
-Denis