[PATCH 0/3] Driver specific dial string validation

[PATCH 0/3] Driver specific dial string validation

[Kuba Pawlak]

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

Dial() method is generic for all modems but hfpmodem may not support some dial
strings. HFP 1.6 SPEC for ATD says: "Only voice calls are covered in this
specification."
Problem is with dialling USSD codes. Request is accepted and a call object
is state dialling is created. USSD code is completed, result displays on
the phone, but there are no events that would allow oFono to remove this fake 
call object. 
We may end up in such situation (Samsung S3 4.3):
ofonod[292]: > ATD*132#;\r
ofonod[292]: < \r\nOK\r\n
ofonod[292]: src/voicecall.c:dial_handle_result() Registering new call: 1
ofonod[292]: > AT+CHUP\r
ofonod[292]: < \r\nOK\r\n
ofonod[292]: > AT+CHUP\r
ofonod[292]: < \r\nOK\r\n
ofonod[292]: > AT+CHUP\r
ofonod[292]: < \r\nOK\r\n

This call cannot be ended and no more calls can be done as there can only be
one call in state 'Dialling'.
Proposed workaround is to have a validation string also in the driver, where
a USSD request can be stopped.

Kuba Pawlak (3):
  voicecall.h: add dial string validation to driver
  voicecall.c: call driver's dial string validation
  hfp: define driver specific dial string validation

 drivers/hfpmodem/voicecall.c | 37 ++++++++++++++++++++++++++++++++++++-
 include/voicecall.h          |  6 ++++++
 src/voicecall.c              |  7 ++++++-
 3 files changed, 48 insertions(+), 2 deletions(-)

-- 
1.7.11.7

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Prof. Dr. Hermann Eul
Chairperson of the Supervisory Board: Tiffany Doon Silva
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 0/3] Driver specific dial string validation

[Kuba Pawlak]

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

Dial() method is generic for all modems but hfpmodem may not support some dial
strings. HFP 1.6 SPEC for ATD says: "Only voice calls are covered in this
specification."
Problem is with dialling USSD codes. Request is accepted and a call object
is state dialling is created. USSD code is completed, result displays on
the phone, but there are no events that would allow oFono to remove this fake 
call object. 
We may end up in such situation (Samsung S3 4.3):
ofonod[292]: > ATD*132#;\r
ofonod[292]: < \r\nOK\r\n
ofonod[292]: src/voicecall.c:dial_handle_result() Registering new call: 1
ofonod[292]: > AT+CHUP\r
ofonod[292]: < \r\nOK\r\n
ofonod[292]: > AT+CHUP\r
ofonod[292]: < \r\nOK\r\n
ofonod[292]: > AT+CHUP\r
ofonod[292]: < \r\nOK\r\n

This call cannot be ended and no more calls can be done as there can only be
one call in state 'Dialling'.
Proposed workaround is to have a validation string also in the driver, where
a USSD request can be stopped.

Kuba Pawlak (3):
  voicecall.h: add dial string validation to driver
  voicecall.c: call driver's dial string validation
  hfp: define driver specific dial string validation

 drivers/hfpmodem/voicecall.c | 37 ++++++++++++++++++++++++++++++++++++-
 include/voicecall.h          |  6 ++++++
 src/voicecall.c              |  7 ++++++-
 3 files changed, 48 insertions(+), 2 deletions(-)

-- 
1.7.11.7

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Prof. Dr. Hermann Eul
Chairperson of the Supervisory Board: Tiffany Doon Silva
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 1/3] voicecall.h: add dial string validation to driver

[Kuba Pawlak]

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

---
 include/voicecall.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/voicecall.h b/include/voicecall.h
index 4d0b3886b81f78839530f157cf717b633c2f45c3..a8af30090ccc1d97fe429a1cf6a9f0beea96bf27 100644
--- a/include/voicecall.h
+++ b/include/voicecall.h
@@ -133,6 +133,12 @@ struct ofono_voicecall_driver {
 			ofono_voicecall_cb_t cb, void *data);
 	void (*send_tones)(struct ofono_voicecall *vc, const char *tones,
 			ofono_voicecall_cb_t cb, void *data);
+
+	/*
+	 * Check dial string using driver specific validation
+	 */
+	ofono_bool_t (*validate_dial)(const char *number);
+
 };
 
 void ofono_voicecall_en_list_notify(struct ofono_voicecall *vc,
-- 
1.7.11.7

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Prof. Dr. Hermann Eul
Chairperson of the Supervisory Board: Tiffany Doon Silva
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 2/3] voicecall.c: call driver's dial string validation

[Kuba Pawlak]

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

---
 src/voicecall.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index 550b79b404f605b9570e771c7754b51c52cfbabb..e030027c6d5b94ebbc7e92ff96c193d5a000e471 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -1513,7 +1513,12 @@ static int voicecall_dial(struct ofono_voicecall *vc, const char *number,
 	if (g_slist_length(vc->call_list) >= MAX_VOICE_CALLS)
 		return -EPERM;
 
-	if (!valid_long_phone_number_format(number))
+	if (vc->driver->validate_dial) {
+		if (!vc->driver->validate_dial(number)) {
+			return -EINVAL;
+		}
+	}
+	else if (!valid_long_phone_number_format(number))
 		return -EINVAL;
 
 	if (ofono_modem_get_online(modem) == FALSE)
-- 
1.7.11.7

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Prof. Dr. Hermann Eul
Chairperson of the Supervisory Board: Tiffany Doon Silva
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 3/3] hfp: define driver specific dial string validation

[Kuba Pawlak]

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

HFP spec only covers ATD behaviour for vocecalls.
If a string starts with '8' or '#' this is a USSD code
Reject a request like that.
---
 drivers/hfpmodem/voicecall.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/hfpmodem/voicecall.c b/drivers/hfpmodem/voicecall.c
index f1e9096940328f05415cf5a6f3bed9ceaf447fe3..1b34f76646fc78b835960959b047e4a2d3a739fd 100644
--- a/drivers/hfpmodem/voicecall.c
+++ b/drivers/hfpmodem/voicecall.c
@@ -695,6 +695,40 @@ error:
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
+static ofono_bool_t hfp_valid_number_format(const char *number)
+{
+	int len = strlen(number);
+	int begin = 0;
+	int i;
+
+	if (!len)
+		return FALSE;
+
+	if (number[0] == '+')
+		begin = 1;
+
+	if (number[0] == '*' || number[0] == '#')
+		return FALSE;
+
+	if (begin == len)
+		return FALSE;
+
+	if ((len - begin) > OFONO_MAX_PHONE_NUMBER_LENGTH)
+		return FALSE;
+
+	for (i = begin; i < len; i++) {
+		if (number[i] >= '0' && number[i] <= '9')
+			continue;
+
+		if (number[i] == '*' || number[i] == '#')
+			continue;
+
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
 static void no_carrier_notify(GAtResult *result, gpointer user_data)
 {
 	DBG("");
@@ -1252,7 +1286,8 @@ static struct ofono_voicecall_driver driver = {
 	.transfer		= hfp_transfer,
 	.deflect		= NULL,
 	.swap_without_accept	= NULL,
-	.send_tones		= hfp_send_dtmf
+	.send_tones		= hfp_send_dtmf,
+	.validate_dial = hfp_valid_number_format
 };
 
 void hfp_voicecall_init(void)
-- 
1.7.11.7

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Prof. Dr. Hermann Eul
Chairperson of the Supervisory Board: Tiffany Doon Silva
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 0/3] Driver specific dial string validation

[Denis Kenzior]

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

Hi Kuba,

On 08/06/2015 11:56 AM, Kuba Pawlak wrote:
> Dial() method is generic for all modems but hfpmodem may not support some dial
> strings. HFP 1.6 SPEC for ATD says: "Only voice calls are covered in this
> specification."
> Problem is with dialling USSD codes. Request is accepted and a call object
> is state dialling is created. USSD code is completed, result displays on
> the phone, but there are no events that would allow oFono to remove this fake
> call object.
> We may end up in such situation (Samsung S3 4.3):
> ofonod[292]: > ATD*132#;\r
> ofonod[292]: < \r\nOK\r\n

Funny that the HFP AG implementation accepts this, but okay.

On a phone, the Dialer was instructed to call 
SupplementaryServices.Initiate() first, before calling 
VoiceCallManager.Dial().  See doc/mmi-codes.txt.

Since we have no SupplementaryServices / ussd atom, I guess it would 
make sense to have the voicecall atom double check that the passed in 
string is not a USSD string.

Try the attached patch.

Regards,
-Denis

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-voicecall-Don-t-accept-USSD-strings-in-Dial.patch --]
[-- Type: text/x-patch, Size: 726 bytes --]

>From 9ec8d03c7c579a0d661fb7885b2627c0300f14c1 Mon Sep 17 00:00:00 2001
From: Denis Kenzior <denkenz@gmail.com>
Date: Thu, 6 Aug 2015 17:16:03 -0500
Subject: [PATCH] voicecall: Don't accept USSD strings in Dial()

---
 src/voicecall.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/voicecall.c b/src/voicecall.c
index 550b79b..b088239 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -1513,6 +1513,9 @@ static int voicecall_dial(struct ofono_voicecall *vc, const char *number,
 	if (g_slist_length(vc->call_list) >= MAX_VOICE_CALLS)
 		return -EPERM;
 
+	if (valid_ussd_string(number, vc->call_list != NULL))
+		return -EINVAL;
+
 	if (!valid_long_phone_number_format(number))
 		return -EINVAL;
 
-- 
2.3.6

RE: [PATCH 0/3] Driver specific dial string validation

[Pawlak, KubaX T]

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

Hi Denis,

> On 08/06/2015 11:56 AM, Kuba Pawlak wrote:
> > Dial() method is generic for all modems but hfpmodem may not support
> some dial
> > strings. HFP 1.6 SPEC for ATD says: "Only voice calls are covered in this
> > specification."
> > Problem is with dialling USSD codes. Request is accepted and a call object
> > is state dialling is created. USSD code is completed, result displays on
> > the phone, but there are no events that would allow oFono to remove this
> fake
> > call object.
> > We may end up in such situation (Samsung S3 4.3):
> > ofonod[292]: > ATD*132#;\r
> > ofonod[292]: < \r\nOK\r\n
> 
> Funny that the HFP AG implementation accepts this, but okay.

This is the only phone I have that does that. S3 mini, some HTC are returning CME ERROR.

> On a phone, the Dialer was instructed to call
> SupplementaryServices.Initiate() first, before calling
> VoiceCallManager.Dial().  See doc/mmi-codes.txt.
> 
> Since we have no SupplementaryServices / ussd atom, I guess it would
> make sense to have the voicecall atom double check that the passed in
> string is not a USSD string.
> 
> Try the attached patch.
 
Looks good. Stops USSD commands from reaching the phone. And is shorter.

Regards,
Kuba
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Prof. Dr. Hermann Eul
Chairperson of the Supervisory Board: Tiffany Doon Silva
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928