Hi Giacinto,
On 10/02/2018 01:26 AM, Giacinto Cifelli wrote:
Added the explicit support for auth NONE.
It is added in all drivers/*/gprs-context.c atoms that support
authentication.
The support for no-authentication case is already present in all
drivers. This patch allows to use it explicitly.
Note that the swmodem driver does not have any authentication
concept: no changes.
Additionally, the behavior is left unchanged in case of inconsistent
parameters: if username is empty, then fallback to auth NONE.
---
drivers/atmodem/gprs-context.c | 20 ++++++++++++++++----
drivers/ifxmodem/gprs-context.c | 6 ++++++
drivers/isimodem/gprs-context.c | 5 +++++
drivers/mbimmodem/gprs-context.c | 16 +++++++++++++---
drivers/mbmmodem/gprs-context.c | 11 ++++++-----
drivers/qmimodem/gprs-context.c | 13 +++++++++----
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, 81 insertions(+), 29 deletions(-)
diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c
index 79ac4c8e..d53fd1d8 100644
--- a/drivers/atmodem/gprs-context.c
+++ b/drivers/atmodem/gprs-context.c
@@ -158,7 +158,12 @@ 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);
+ else
+ g_at_ppp_set_credentials(gcd->ppp, NULL, NULL);
This else part seems unneeded
/* set connect and disconnect callbacks */
g_at_ppp_set_connect_function(gcd->ppp, ppp_connect, gc);
@@ -247,7 +252,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 +260,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;
+ gcd->username[0] = 0;
+ gcd->password[0] = 0;
+ break;
}
gcd->state = STATE_ENABLING;
@@ -304,6 +312,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:
diff --git a/drivers/ifxmodem/gprs-context.c b/drivers/ifxmodem/gprs-context.c
index 885e41bb..81b056cc 100644
--- a/drivers/ifxmodem/gprs-context.c
+++ b/drivers/ifxmodem/gprs-context.c
@@ -466,9 +466,15 @@ 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) {
+ gcd->username[0] = 0;
+ gcd->password[0] = 0;
+ }
+
Ugh, I'd really prefer something like:
if (method == NONE) {
memset(gcd->username, 0, sizeof(gcd->username));
...
} else {
memcpy(...);
}
gcd->state = STATE_ENABLING;
gcd->proto = ctx->proto;
diff --git a/drivers/isimodem/gprs-context.c b/drivers/isimodem/gprs-context.c
index ce53d022..b1c819c2 100644
--- a/drivers/isimodem/gprs-context.c
+++ b/drivers/isimodem/gprs-context.c
@@ -544,6 +544,11 @@ static void isi_gprs_activate_primary(struct
ofono_gprs_context *gc,
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) {
+ cd->username[0] = 0;
+ cd->password[0] = 0;
+ }
+
cd->pep = g_isi_pep_create(cd->idx, NULL, NULL);
if (cd->pep == NULL)
goto error;
diff --git a/drivers/mbimmodem/gprs-context.c b/drivers/mbimmodem/gprs-context.c
index 79793c92..be256e43 100644
--- a/drivers/mbimmodem/gprs-context.c
+++ b/drivers/mbimmodem/gprs-context.c
@@ -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),
diff --git a/drivers/mbmmodem/gprs-context.c b/drivers/mbmmodem/gprs-context.c
index e961afa1..fa8b44b6 100644
--- a/drivers/mbmmodem/gprs-context.c
+++ b/drivers/mbmmodem/gprs-context.c
@@ -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;
diff --git a/drivers/qmimodem/gprs-context.c b/drivers/qmimodem/gprs-context.c
index 9a22b89f..7f31dd0e 100644
--- a/drivers/qmimodem/gprs-context.c
+++ b/drivers/qmimodem/gprs-context.c
@@ -238,7 +238,12 @@ static void qmi_activate_primary(struct ofono_gprs_context
*gc,
struct cb_data *cbd = cb_data_new(cb, user_data);
struct qmi_param *param;
uint8_t ip_family;
- uint8_t auth;
+ /*
+ * set default authentication to CHAP, even if unneeded,
+ * otherwise the compiler complains that:
+ * ‘auth’ may be used uninitialized in this function
+ */
+ uint8_t auth = QMI_WDS_AUTHENTICATION_CHAP;
Ugh. I really hate GCC people sometimes. While the warning is strictly
correct, it is useless for 99% of the code and forces us to jump through
hoops. Anyway, I would propose we use the following pattern:
int auth_method_to_qmi(enum ofono_auth_method method, uint8_t *out_auth)
{
switch (method)
case CHAP:
*out_auth = ...
return 0;
case:
...
}
return -ERANGE;
}
DBG("cid %u", ctx->cid);
@@ -273,7 +278,7 @@ static void qmi_activate_primary(struct ofono_gprs_context *gc,
case OFONO_GPRS_AUTH_METHOD_PAP:
auth = QMI_WDS_AUTHENTICATION_PAP;
break;
- default:
+ case OFONO_GPRS_AUTH_METHOD_NONE:
auth = QMI_WDS_AUTHENTICATION_NONE;
break;
}
@@ -281,11 +286,11 @@ static void qmi_activate_primary(struct
ofono_gprs_context *gc,
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);
diff --git a/drivers/rilmodem/gprs-context.c b/drivers/rilmodem/gprs-context.c
index 1f476e23..ef62cba9 100644
--- a/drivers/rilmodem/gprs-context.c
+++ b/drivers/rilmodem/gprs-context.c
@@ -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
diff --git a/drivers/stemodem/gprs-context.c b/drivers/stemodem/gprs-context.c
index 18b2bfa4..32facd8c 100644
--- a/drivers/stemodem/gprs-context.c
+++ b/drivers/stemodem/gprs-context.c
@@ -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;
diff --git a/drivers/telitmodem/gprs-context-ncm.c b/drivers/telitmodem/gprs-context-ncm.c
index 7139740c..9c9b9500 100644
--- a/drivers/telitmodem/gprs-context-ncm.c
+++ b/drivers/telitmodem/gprs-context-ncm.c
@@ -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;
}
diff --git a/drivers/ubloxmodem/gprs-context.c
b/drivers/ubloxmodem/gprs-context.c
index 6fe2719f..7eb18f09 100644
--- a/drivers/ubloxmodem/gprs-context.c
+++ b/drivers/ubloxmodem/gprs-context.c
@@ -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\"",
Anyway, these look fine to me otherwise. But I still wouldn't mind some
explanation of how we're addressing the questions I raised in my E-mail
on Sep 27.
Also, what are we doing with plugins/mbpi.c? That database only
supports pap/chap but potentially with missing username/password
attributes. Should we be converting these to type 'none'? Or leaving
things as is and having the drivers deal with converting type chap,
empty username/password to type none. I thought the whole idea of
introducing type 'None' was that you hated that drivers were doing this?
I believe you referred to this as a 'hack' or similar terminology? :)
Regards,
-Denis
_______________________________________________
ofono mailing list
[email protected]
https://lists.ofono.org/mailman/listinfo/ofono
