* Re: Cannot build BlueZ Release 4.70...
From: Johan Hedberg @ 2010-08-28 8:22 UTC (permalink / raw)
To: Ho, Albert O; +Cc: linux-bluetooth@vger.kernel.org, marcel@holtmann.org
In-Reply-To: <CF9C39F99A89134C9CF9C4CCB68B8DDF25AB2D2864@orsmsx501.amr.corp.intel.com>
Hi Albert,
On Sat, Aug 28, 2010, Ho, Albert O wrote:
> Hi, I attempted to build Release 4.70 but it's not buildable. Using
> sources either from bluez-4.70.tar.gz (www.bluez.org) or from git
> results to compile errors. The offending lines are all in
> attrib-server.c & all are 'undefined references to xxxxx'. Same setup
> can build 4.69. Pls advise.
It compiles fine for me. You'll need to copy-paste the exact errors you
get as well as the configure script options that you used.
Johan
^ permalink raw reply
* Cannot build BlueZ Release 4.70...
From: Ho, Albert O @ 2010-08-28 7:12 UTC (permalink / raw)
To: linux-bluetooth@vger.kernel.org; +Cc: marcel@holtmann.org
Hi, I attempted to build Release 4.70 but it's not buildable. Using source=
s either from bluez-4.70.tar.gz (www.bluez.org) or from git results to comp=
ile errors. The offending lines are all in attrib-server.c & all are 'unde=
fined references to xxxxx'. Same setup can build 4.69. Pls advise. =20
Regards
Albert
^ permalink raw reply
* Re: [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
From: Johan Hedberg @ 2010-08-27 20:11 UTC (permalink / raw)
To: Waldemar.Rymarkiewicz
Cc: hadess, linux-bluetooth, par-gunnar.p.hjalmdahl, joakim.xj.ceder,
arunkr.singh
In-Reply-To: <99B09243E1A5DA4898CDD8B7001114480976E15A78@EXMB04.eu.tieto.com>
Hi Waldek,
On Fri, Aug 27, 2010, Waldemar.Rymarkiewicz@tieto.com wrote:
> However, I would appreciate any comments to the changes as I'm not
> sure what was originally assumed for the high security level.
Your assumptions were more or less correct. Additionally there should be
the check for encryption key size (to fulfill SAP requirements), though
that'd require at least a 3.0 capable controller if for the standardized
HCI command.
> What's more, I'am not happy that the link key request and the pin code
> request are handled in bluez. It makes that bluez need to pass it to
> the kernel as it's needed there. I used ioctl for that. Some comments
> on this?
I'm not happy with it either. In fact, the arbitrary split of security
logic between kernel and userspace has caused us lots of grief,
especially with SSP. Because of this, the plan for BlueZ 5.x is to move
most security related logic to the kernel side. The only thing remaining
in userspace is security policy requiring user interaction (e.g.
answering user confirmation requests) and persistent link key storage
(runtime storage will be added to the kernel side). The promiscuous HCI
socket will go away from bluetoothd so the daemon doesn't always wake up
when there's some HCI traffic.
To make all this possible we're designing a new protocol between kernel
and userspace. Hopefully we'll get the first bits, at least a
preliminary specification, upstream within the next month or so. Because
of this I'm wondering if the new ioctl that you're adding is really
worth it. Maybe we should just wait for BlueZ 5.x with this. That'd also
give us the possibility of adding a new parameter to
Agent.RequestPinCode since we need to break the agent API anyway (e.g.
add an incoming just-works pairing callback).
Johan
^ permalink raw reply
* Re: [PATCH] Bluetooth: BT_SECURITY_HIGH requires 16 digit pin code
From: Gustavo F. Padovan @ 2010-08-27 19:55 UTC (permalink / raw)
To: Waldemar Rymarkiewicz
Cc: linux-bluetooth, par-gunnar.p.hjalmdahl, joakim.xj.ceder,
johan.hedberg
In-Reply-To: <1282909460-19337-1-git-send-email-waldemar.rymarkiewicz@tieto.com>
Hi Waldemar,
* Waldemar Rymarkiewicz <waldemar.rymarkiewicz@tieto.com> [2010-08-27 13:44:20 +0200]:
> The security level BT_SECURITY_HIGH expects secure connection
> and a minimum 16 digit pin code used for bonding. It's requitred by the
> Sim Access Profile.
>
> Patch on behalf of ST-Ericsson SA.
>
> Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@tieto.com>
> ---
> include/net/bluetooth/hci.h | 17 +++++++
> include/net/bluetooth/hci_core.h | 4 ++
> net/bluetooth/hci_conn.c | 97 +++++++++++++++++++++++++++++++-------
> net/bluetooth/hci_event.c | 4 ++
> net/bluetooth/hci_sock.c | 3 +
> net/bluetooth/rfcomm/core.c | 10 ++++-
> 6 files changed, 116 insertions(+), 19 deletions(-)
> mode change 100644 => 100755 include/net/bluetooth/hci.h
> mode change 100644 => 100755 include/net/bluetooth/hci_core.h
> mode change 100644 => 100755 net/bluetooth/hci_conn.c
> mode change 100644 => 100755 net/bluetooth/hci_event.c
> mode change 100644 => 100755 net/bluetooth/rfcomm/core.c
You are changing the files modes. Don't do that.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH] Bluetooth: BT_SECURITY_HIGH requires 16 digit pin code
From: Antonio Ospite @ 2010-08-27 19:45 UTC (permalink / raw)
To: Waldemar Rymarkiewicz
Cc: linux-bluetooth, par-gunnar.p.hjalmdahl, joakim.xj.ceder,
johan.hedberg
In-Reply-To: <1282909460-19337-1-git-send-email-waldemar.rymarkiewicz@tieto.com>
[-- Attachment #1: Type: text/plain, Size: 662 bytes --]
On Fri, 27 Aug 2010 13:44:20 +0200
Waldemar Rymarkiewicz <waldemar.rymarkiewicz@tieto.com> wrote:
> The security level BT_SECURITY_HIGH expects secure connection
> and a minimum 16 digit pin code used for bonding. It's requitred by the
typo:
s/requitred/required/
> Sim Access Profile.
>
> Patch on behalf of ST-Ericsson SA.
>
> Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@tieto.com>
[...]
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] Bluetooth: Add socket option definitions for AMP
From: Mat Martineau @ 2010-08-27 18:06 UTC (permalink / raw)
To: David Vrabel; +Cc: linux-bluetooth, marcel, rshaffer, linux-arm-msm
In-Reply-To: <4C6BE410.1020505@csr.com>
David -
On Wed, 18 Aug 2010, David Vrabel wrote:
> Mat Martineau wrote:
>> This adds a new BT_AMP socket option to control the use of AMP channels.
>> It is for use with the SOL_BLUETOOTH option level on L2CAP sockets.
>>
>> Available option values are defined as:
>>
>> [lots of nice documentation]
>
> Why is none of this documentation in the code?
>
> I believe we also agreed that this policy could be changed at any point
> in the life time of the channel so this should be documented as well.
>
>> +#define BT_AMP 8
>
> Suggest BT_AMP_POLICY, BT_AMP_POLICY_REQUIRE_BR_EDR etc. Just in case
> there are other AMP related socket options in the future.
>
>> +#define BT_AMP_REQUIRE_BR_EDR 0
>> +#define BT_AMP_PREFER_AMP 1
>> +#define BT_AMP_PREFER_BR_EDR 2
Thanks for the helpful feedback - I'll add the documentation, adjust
the names, and repost.
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* RE: [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
From: Waldemar.Rymarkiewicz @ 2010-08-27 13:32 UTC (permalink / raw)
To: johan.hedberg
Cc: hadess, linux-bluetooth, par-gunnar.p.hjalmdahl, joakim.xj.ceder,
arunkr.singh
In-Reply-To: <20100827124524.GA9484@jh-x301>
Johan,
>-----Original Message-----
>From: Johan Hedberg [mailto:johan.hedberg@gmail.com]
>Sent: Friday, August 27, 2010 2:45 PM
>Hi Waldemar,
>
>On Fri, Aug 27, 2010, Waldemar.Rymarkiewicz@tieto.com wrote:
>> I assume that user will know that the 16 digit pin is requred, so
>> should be enough to let the user type 16 digit in an agent I guess.
>> Usually a service that requires high security will generate
>right pin
>> code.
>
>I don't think "the user should know" is enough here, so we may
>need to think about ways that we could relay this info to the
>agent (maybe a special adapter property or an extra parameter
>in the PIN request agent callback).
I agree and I guess adding a new property is more convenient then braking existing agen api.
>> Originaly the high security level was planned to require max
>pin code
>> lenght as I know.
>
>That's correct, however the UI side hasn't really been
>considered so far.
>
>In general about the patches, I suspect it'll take a bit
>longer than usual to get them in since the code you're
>touching is one of the most sensitive ones with respect to
>breakage. We've finetuned it multiple times over the last few
>years to get rid of IOP issues, security vulnerabilities and
>to make sure all qualification tests pass. Another reason for
>an expected delay is that you're introducing changes to the
>kernel interface and that always requires some extra reviewing.
I'm aware of that and I actually expected delay. Sending those patches I have been rather more interesting in starting some disscution on this topic, then get it up streamed now.
>So how well have you tested the patches? I.e. how confident
>are you that you're not introducing any regressions? Scenarios
>that would need to be tested before an upstream merge are (and
>I'm probably forgetting several of them):
>
>- legacy pairing acceptor & initiator
>- security mode 3 acceptor & initiator
>- ssp acceptor & initiator
>- renewed link key handling for both debug and normal keys
>- security level upgrading (i.e. connect first to a low security socket
> and then over the same ACL to a higher security socket)
>- complete and partial failure scenarios for all of the above
>
>Additionally all these test should be done against several
>different controllers due to differences in HCI interface
>behavior (event ordering, error codes, etc). In that list I'd
>include at least one CSR, and one Broadcom adapter and any
>other adapters from other manufacturers that you can get hold of.
>
>So how many of these tests do you already have covered? I'm
>not very comfortable with pushing the patches upstream before
>most of the above scenarios have been tested and verified not
>to introduce any regressions.
>
>Johan
>
To be honest, I did not check all mentioned use cases. I did tests against lagacy and ssp parring and security level upgrading.
So, I will put more effort into testing to be sure.
However, I would appreciate any comments to the changes as I'm not sure what was originally assumed for the high security level. What's more, I'am not happy that the link key request and the pin code request are handled in bluez. It makes that bluez need to pass it to the kernel as it's needed there. I used ioctl for that. Some comments on this?
Thanks,
/Waldek
^ permalink raw reply
* Re: [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
From: Johan Hedberg @ 2010-08-27 12:45 UTC (permalink / raw)
To: Waldemar.Rymarkiewicz
Cc: hadess, linux-bluetooth, par-gunnar.p.hjalmdahl, joakim.xj.ceder,
arunkr.singh
In-Reply-To: <99B09243E1A5DA4898CDD8B7001114480976E15A2C@EXMB04.eu.tieto.com>
Hi Waldemar,
On Fri, Aug 27, 2010, Waldemar.Rymarkiewicz@tieto.com wrote:
> I assume that user will know that the 16 digit pin is requred, so
> should be enough to let the user type 16 digit in an agent I guess.
> Usually a service that requires high security will generate right pin
> code.
I don't think "the user should know" is enough here, so we may need to
think about ways that we could relay this info to the agent (maybe a
special adapter property or an extra parameter in the PIN request agent
callback).
> Originaly the high security level was planned to require max pin code
> lenght as I know.
That's correct, however the UI side hasn't really been considered so
far.
In general about the patches, I suspect it'll take a bit longer than
usual to get them in since the code you're touching is one of the most
sensitive ones with respect to breakage. We've finetuned it multiple
times over the last few years to get rid of IOP issues, security
vulnerabilities and to make sure all qualification tests pass. Another
reason for an expected delay is that you're introducing changes to the
kernel interface and that always requires some extra reviewing.
So how well have you tested the patches? I.e. how confident are you that
you're not introducing any regressions? Scenarios that would need to be
tested before an upstream merge are (and I'm probably forgetting several
of them):
- legacy pairing acceptor & initiator
- security mode 3 acceptor & initiator
- ssp acceptor & initiator
- renewed link key handling for both debug and normal keys
- security level upgrading (i.e. connect first to a low security socket
and then over the same ACL to a higher security socket)
- complete and partial failure scenarios for all of the above
Additionally all these test should be done against several different
controllers due to differences in HCI interface behavior (event
ordering, error codes, etc). In that list I'd include at least one CSR,
and one Broadcom adapter and any other adapters from other manufacturers
that you can get hold of.
So how many of these tests do you already have covered? I'm not very
comfortable with pushing the patches upstream before most of the above
scenarios have been tested and verified not to introduce any
regressions.
Johan
^ permalink raw reply
* RE: [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
From: Bastien Nocera @ 2010-08-27 12:32 UTC (permalink / raw)
To: Waldemar.Rymarkiewicz
Cc: linux-bluetooth, par-gunnar.p.hjalmdahl, joakim.xj.ceder,
johan.hedberg, arunkr.singh
In-Reply-To: <99B09243E1A5DA4898CDD8B7001114480976E15A2C@EXMB04.eu.tieto.com>
On Fri, 2010-08-27 at 15:26 +0300, Waldemar.Rymarkiewicz@tieto.com
wrote:
> Hi,
>
> >-----Original Message-----
> >From: Bastien Nocera [mailto:hadess@hadess.net]
> >Sent: Friday, August 27, 2010 2:12 PM
> >To: Rymarkiewicz Waldemar
> >Cc: linux-bluetooth@vger.kernel.org;
> >par-gunnar.p.hjalmdahl@stericsson.com;
> >joakim.xj.ceder@stericsson.com; johan.hedberg@gmail.com
> >Subject: Re: [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
> >
> >On Fri, 2010-08-27 at 13:45 +0200, Waldemar Rymarkiewicz wrote:
> >> The security level BT_SECURITY_HIGH expects secure connection and a
> >> minimum 16 digit pin code used for bonding. It's requitred
> >by the Sim
> >> Access Profile.
> >
> >How is user-space (meaning the pairing agent) supposed to handle that?
> >I'd need to make changes to gnome-bluetooth to use longer PIN
> >codes for the maximum security.
> >
> >Cheers
> >
>
> I assume that user will know that the 16 digit pin is requred, so
> should be enough to let the user type 16 digit in an agent I guess.
> Usually a service that requires high security will generate right pin
> code.
How would they know? Pairing the device isn't connecting to the
service... Furthermore, gnome-bluetooth's wizard takes a lot of care
generating pin codes for the user by default, so we'd need to know that
a 16 digit pin code is required.
Supporting 16 digits pin code would probably require interface changes.
And I was under the impression that the PIN code's length didn't come
into account for the creation of the encryption, just for the initial
challenge-response needed to verify the device is who it says it is.
> Originaly the high security level was planned to require max pin code
> lenght as I know.
^ permalink raw reply
* RE: [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
From: Waldemar.Rymarkiewicz @ 2010-08-27 12:26 UTC (permalink / raw)
To: hadess
Cc: linux-bluetooth, par-gunnar.p.hjalmdahl, joakim.xj.ceder,
johan.hedberg, arunkr.singh
In-Reply-To: <1282911133.835.30.camel@localhost.localdomain>
Hi,
>-----Original Message-----
>From: Bastien Nocera [mailto:hadess@hadess.net]
>Sent: Friday, August 27, 2010 2:12 PM
>To: Rymarkiewicz Waldemar
>Cc: linux-bluetooth@vger.kernel.org;
>par-gunnar.p.hjalmdahl@stericsson.com;
>joakim.xj.ceder@stericsson.com; johan.hedberg@gmail.com
>Subject: Re: [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
>
>On Fri, 2010-08-27 at 13:45 +0200, Waldemar Rymarkiewicz wrote:
>> The security level BT_SECURITY_HIGH expects secure connection and a
>> minimum 16 digit pin code used for bonding. It's requitred
>by the Sim
>> Access Profile.
>
>How is user-space (meaning the pairing agent) supposed to handle that?
>I'd need to make changes to gnome-bluetooth to use longer PIN
>codes for the maximum security.
>
>Cheers
>
I assume that user will know that the 16 digit pin is requred, so should be enough to let the user type 16 digit in an agent I guess.
Usually a service that requires high security will generate right pin code.
Originaly the high security level was planned to require max pin code lenght as I know.
Thanks,
/Waldek
^ permalink raw reply
* Re: [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
From: Bastien Nocera @ 2010-08-27 12:12 UTC (permalink / raw)
To: Waldemar Rymarkiewicz
Cc: linux-bluetooth, par-gunnar.p.hjalmdahl, joakim.xj.ceder,
johan.hedberg
In-Reply-To: <1282909526-19374-1-git-send-email-waldemar.rymarkiewicz@tieto.com>
On Fri, 2010-08-27 at 13:45 +0200, Waldemar Rymarkiewicz wrote:
> The security level BT_SECURITY_HIGH expects secure connection
> and a minimum 16 digit pin code used for bonding. It's requitred by the
> Sim Access Profile.
How is user-space (meaning the pairing agent) supposed to handle that?
I'd need to make changes to gnome-bluetooth to use longer PIN codes for
the maximum security.
Cheers
^ permalink raw reply
* [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
From: Waldemar Rymarkiewicz @ 2010-08-27 11:45 UTC (permalink / raw)
To: linux-bluetooth
Cc: par-gunnar.p.hjalmdahl, joakim.xj.ceder, johan.hedberg,
Waldemar Rymarkiewicz
The security level BT_SECURITY_HIGH expects secure connection
and a minimum 16 digit pin code used for bonding. It's requitred by the
Sim Access Profile.
Patch on behalf of ST-Ericsson SA.
Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@tieto.com>
---
lib/hci.h | 8 +++++++-
src/dbus-hci.c | 33 ++++++++++++++++++++++++++++-----
src/security.c | 38 ++++++++++++++++++++++++++++++++++----
3 files changed, 69 insertions(+), 10 deletions(-)
mode change 100644 => 100755 lib/bluetooth.h
mode change 100644 => 100755 lib/hci.h
mode change 100644 => 100755 lib/l2cap.h
mode change 100644 => 100755 lib/rfcomm.h
mode change 100644 => 100755 src/btio.c
mode change 100644 => 100755 src/btio.h
mode change 100644 => 100755 src/dbus-hci.c
mode change 100644 => 100755 src/security.c
diff --git a/lib/bluetooth.h b/lib/bluetooth.h
old mode 100644
new mode 100755
diff --git a/lib/hci.h b/lib/hci.h
old mode 100644
new mode 100755
index 512dab9..a313929
--- a/lib/hci.h
+++ b/lib/hci.h
@@ -96,7 +96,7 @@ enum {
#define HCISETLINKMODE _IOW('H', 226, int)
#define HCISETACLMTU _IOW('H', 227, int)
#define HCISETSCOMTU _IOW('H', 228, int)
-
+#define HCISETCONNINFO _IOW('H', 229, int)
#define HCIBLOCKADDR _IOW('H', 230, int)
#define HCIUNBLOCKADDR _IOW('H', 231, int)
@@ -2326,9 +2326,15 @@ struct hci_conn_info_req {
struct hci_conn_info conn_info[0];
};
+struct hci_set_conn_info_req {
+ bdaddr_t bdaddr;
+ uint8_t pin_len;
+ uint8_t key_type;
+};
struct hci_auth_info_req {
bdaddr_t bdaddr;
uint8_t type;
+ uint8_t level;
};
struct hci_inquiry_req {
diff --git a/lib/l2cap.h b/lib/l2cap.h
old mode 100644
new mode 100755
diff --git a/lib/rfcomm.h b/lib/rfcomm.h
old mode 100644
new mode 100755
diff --git a/src/btio.c b/src/btio.c
old mode 100644
new mode 100755
diff --git a/src/btio.h b/src/btio.h
old mode 100644
new mode 100755
diff --git a/src/dbus-hci.c b/src/dbus-hci.c
old mode 100644
new mode 100755
index 9055ffe..177f536
--- a/src/dbus-hci.c
+++ b/src/dbus-hci.c
@@ -165,6 +165,8 @@ static void pincode_cb(struct agent *agent, DBusError *err,
{
struct btd_adapter *adapter = device_get_adapter(device);
pin_code_reply_cp pr;
+ struct hci_auth_info_req ar;
+ struct hci_set_conn_info_req cr;
bdaddr_t sba, dba;
size_t len;
int dev;
@@ -180,13 +182,30 @@ static void pincode_cb(struct agent *agent, DBusError *err,
adapter_get_address(adapter, &sba);
device_get_address(device, &dba);
- if (err) {
- hci_send_cmd(dev, OGF_LINK_CTL,
- OCF_PIN_CODE_NEG_REPLY, 6, &dba);
- goto done;
- }
+ if (err)
+ goto reject;
len = strlen(pincode);
+ memset(&ar, 0, sizeof(ar));
+ bacpy(&ar.bdaddr, &dba);
+ if (ioctl(dev, HCIGETAUTHINFO, (unsigned long) &ar) < 0) {
+ error("Can't get auth info: %s (%d)", strerror(errno), errno);
+ goto reject;
+ }
+
+ if (ar.level == BT_SECURITY_HIGH && len < 16) {
+ error("Not enough secure PIN (expected 16 digit PIN, but got \
+ %d digit).", len);
+ goto reject;
+ }
+
+ bacpy(&cr.bdaddr, &dba);
+ cr.pin_len = len;
+ cr.key_type = 0xff;
+ if (ioctl(dev, HCISETCONNINFO, (unsigned long) &cr) < 0) {
+ error("Can't set conn info: %s (%d)", strerror(errno), errno);
+ goto reject;
+ }
set_pin_length(&sba, len);
@@ -196,7 +215,11 @@ static void pincode_cb(struct agent *agent, DBusError *err,
pr.pin_len = len;
hci_send_cmd(dev, OGF_LINK_CTL, OCF_PIN_CODE_REPLY,
PIN_CODE_REPLY_CP_SIZE, &pr);
+ goto done;
+reject:
+ hci_send_cmd(dev, OGF_LINK_CTL,
+ OCF_PIN_CODE_NEG_REPLY, 6, &dba);
done:
hci_close_dev(dev);
}
diff --git a/src/security.c b/src/security.c
old mode 100644
new mode 100755
index 667f1f1..b25d1e4
--- a/src/security.c
+++ b/src/security.c
@@ -309,6 +309,7 @@ static void link_key_request(int dev, bdaddr_t *sba, bdaddr_t *dba)
char sa[18], da[18];
uint8_t type;
int err;
+ int pinlen;
if (!get_adapter_and_device(sba, dba, &adapter, &device, FALSE))
device = NULL;
@@ -325,9 +326,11 @@ static void link_key_request(int dev, bdaddr_t *sba, bdaddr_t *dba)
DBG("HCIGETAUTHINFO failed %s (%d)",
strerror(errno), errno);
req.type = 0x00;
+ req.level = BT_SECURITY_LOW;
}
- DBG("kernel auth requirements = 0x%02x", req.type);
+ DBG("kernel auth requirements = 0x%02x and security level = 0x%02x", \
+ req.type, req.level);
if (main_opts.debug_keys && device && device_get_debug_key(device, key))
type = 0x03;
@@ -341,18 +344,34 @@ static void link_key_request(int dev, bdaddr_t *sba, bdaddr_t *dba)
DBG("link key type = 0x%02x", type);
+ pinlen = read_pin_length(sba, dba);
+ DBG("stored link key type = 0x%02x pin_len = %d", type, pinlen);
+
/* Don't use unauthenticated combination keys if MITM is
- * required */
- if (type == 0x04 && req.type != 0xff && (req.type & 0x01))
+ * required and also don't use combination link keys authenticated with
+ * an PIN code len < 16 if security level BT_SECURITY_HIGH is required*/
+ if ((type == 0x04 && req.type != 0xff && (req.type & 0x01)) ||
+ (type == 0x00 && req.level == BT_SECURITY_HIGH && pinlen < 16))
hci_send_cmd(dev, OGF_LINK_CTL, OCF_LINK_KEY_NEG_REPLY,
6, dba);
else {
link_key_reply_cp lr;
+ struct hci_set_conn_info_req cr;
memcpy(lr.link_key, key, 16);
bacpy(&lr.bdaddr, dba);
- hci_send_cmd(dev, OGF_LINK_CTL, OCF_LINK_KEY_REPLY,
+ bacpy(&cr.bdaddr, dba);
+ cr.pin_len = pinlen;
+ cr.key_type = type;
+
+ if (ioctl(dev, HCISETCONNINFO, (unsigned long) &cr) < 0) {
+ error("Can't set conn info: %s (%d)", strerror(errno),
+ errno);
+ hci_send_cmd(dev, OGF_LINK_CTL, OCF_LINK_KEY_NEG_REPLY,
+ 6, dba);
+ } else
+ hci_send_cmd(dev, OGF_LINK_CTL, OCF_LINK_KEY_REPLY,
LINK_KEY_REPLY_CP_SIZE, &lr);
}
}
@@ -523,6 +542,7 @@ static void pin_code_request(int dev, bdaddr_t *sba, bdaddr_t *dba)
pin_code_reply_cp pr;
struct hci_conn_info_req *cr;
struct hci_conn_info *ci;
+ struct hci_auth_info_req ar;
char sa[18], da[18], pin[17];
int pinlen;
@@ -542,10 +562,20 @@ static void pin_code_request(int dev, bdaddr_t *sba, bdaddr_t *dba)
}
ci = cr->conn_info;
+ memset(&ar, 0, sizeof(ar));
+ bacpy(&ar.bdaddr, dba);
+ if (ioctl(dev, HCIGETAUTHINFO, (unsigned long) &ar) < 0) {
+ error("Can't get auth info: %s (%d)", strerror(errno), errno);
+ goto reject;
+ }
memset(pin, 0, sizeof(pin));
pinlen = read_pin_code(sba, dba, pin);
if (pinlen > 0) {
+ if (ar.level == BT_SECURITY_HIGH && pinlen < 16) {
+ error("Not 16 digit pin code.");
+ goto reject;
+ }
set_pin_length(sba, pinlen);
memcpy(pr.pin_code, pin, pinlen);
pr.pin_len = pinlen;
--
1.7.0.4
^ permalink raw reply related
* [PATCH] Bluetooth: BT_SECURITY_HIGH requires 16 digit pin code
From: Waldemar Rymarkiewicz @ 2010-08-27 11:44 UTC (permalink / raw)
To: linux-bluetooth
Cc: par-gunnar.p.hjalmdahl, joakim.xj.ceder, johan.hedberg,
Waldemar Rymarkiewicz
The security level BT_SECURITY_HIGH expects secure connection
and a minimum 16 digit pin code used for bonding. It's requitred by the
Sim Access Profile.
Patch on behalf of ST-Ericsson SA.
Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@tieto.com>
---
include/net/bluetooth/hci.h | 17 +++++++
include/net/bluetooth/hci_core.h | 4 ++
net/bluetooth/hci_conn.c | 97 +++++++++++++++++++++++++++++++-------
net/bluetooth/hci_event.c | 4 ++
net/bluetooth/hci_sock.c | 3 +
net/bluetooth/rfcomm/core.c | 10 ++++-
6 files changed, 116 insertions(+), 19 deletions(-)
mode change 100644 => 100755 include/net/bluetooth/hci.h
mode change 100644 => 100755 include/net/bluetooth/hci_core.h
mode change 100644 => 100755 net/bluetooth/hci_conn.c
mode change 100644 => 100755 net/bluetooth/hci_event.c
mode change 100644 => 100755 net/bluetooth/rfcomm/core.c
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
old mode 100644
new mode 100755
index bcbdd6d..34a6fbf
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -99,6 +99,7 @@ enum {
#define HCISETLINKMODE _IOW('H', 226, int)
#define HCISETACLMTU _IOW('H', 227, int)
#define HCISETSCOMTU _IOW('H', 228, int)
+#define HCISETCONNINFO _IOW('H', 229, int)
#define HCIBLOCKADDR _IOW('H', 230, int)
#define HCIUNBLOCKADDR _IOW('H', 231, int)
@@ -224,6 +225,15 @@ enum {
#define HCI_AT_GENERAL_BONDING 0x04
#define HCI_AT_GENERAL_BONDING_MITM 0x05
+/* Link Key types */
+#define HCI_LK_COMBINATION 0x00
+#define HCI_LK_LOCAL_UNIT 0x01
+#define HCI_LK_REMOTE_UNIT 0x02
+#define HCI_LK_DEBUG_COMBINATION 0x03
+#define HCI_LK_UNAUTHENTICATED_COMBINATION 0x04
+#define HCI_LK_AUTHENTICATED_COMBINATION 0x05
+#define HCI_LK_CHANGEED_COMBINATION_KEY 0x06
+
/* ----- HCI Commands ---- */
#define HCI_OP_INQUIRY 0x0401
struct hci_cp_inquiry {
@@ -1022,9 +1032,16 @@ struct hci_conn_info_req {
struct hci_conn_info conn_info[0];
};
+struct hci_set_conn_info_req {
+ bdaddr_t bdaddr;
+ __u8 pin_len;
+ __u8 key_type;
+};
+
struct hci_auth_info_req {
bdaddr_t bdaddr;
__u8 type;
+ __u8 level;
};
struct hci_inquiry_req {
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
old mode 100644
new mode 100755
index 4568b93..9eb2da3
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -183,6 +183,8 @@ struct hci_conn {
__u32 link_mode;
__u8 auth_type;
__u8 sec_level;
+ __u8 key_type;
+ __u8 pin_len;
__u8 power_save;
__u16 disc_timeout;
unsigned long pend;
@@ -430,6 +432,8 @@ int hci_get_dev_info(void __user *arg);
int hci_get_conn_list(void __user *arg);
int hci_get_conn_info(struct hci_dev *hdev, void __user *arg);
int hci_get_auth_info(struct hci_dev *hdev, void __user *arg);
+int hci_set_conn_info(struct hci_dev *hdev, void __user *arg);
+
int hci_inquiry(void __user *arg);
struct bdaddr_list *hci_blacklist_lookup(struct hci_dev *hdev, bdaddr_t *bdaddr);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
old mode 100644
new mode 100755
index 0b1e460..ed7a007
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -233,6 +233,8 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
conn->mode = HCI_CM_ACTIVE;
conn->state = BT_OPEN;
conn->auth_type = HCI_AT_GENERAL_BONDING;
+ conn->key_type = 0xff;
+ conn->pin_len = 0;
conn->power_save = 1;
conn->disc_timeout = HCI_DISCONN_TIMEOUT;
@@ -433,15 +435,11 @@ int hci_conn_check_link_mode(struct hci_conn *conn)
EXPORT_SYMBOL(hci_conn_check_link_mode);
/* Authenticate remote device */
-static int hci_conn_auth(struct hci_conn *conn, __u8 sec_level, __u8 auth_type)
+static void hci_conn_auth(struct hci_conn *conn, __u8 sec_level, __u8 auth_type)
{
BT_DBG("conn %p", conn);
- if (sec_level > conn->sec_level)
- conn->sec_level = sec_level;
- else if (conn->link_mode & HCI_LM_AUTH)
- return 1;
-
+ conn->sec_level = sec_level;
conn->auth_type = auth_type;
if (!test_and_set_bit(HCI_CONN_AUTH_PEND, &conn->pend)) {
@@ -450,8 +448,20 @@ static int hci_conn_auth(struct hci_conn *conn, __u8 sec_level, __u8 auth_type)
hci_send_cmd(conn->hdev, HCI_OP_AUTH_REQUESTED,
sizeof(cp), &cp);
}
+}
- return 0;
+/* Encrypt the the link */
+static void hci_conn_encrypt(struct hci_conn *conn)
+{
+ BT_DBG("conn %p", conn);
+
+ if (!test_and_set_bit(HCI_CONN_ENCRYPT_PEND, &conn->pend)) {
+ struct hci_cp_set_conn_encrypt cp;
+ cp.handle = cpu_to_le16(conn->handle);
+ cp.encrypt = 1;
+ hci_send_cmd(conn->hdev, HCI_OP_SET_CONN_ENCRYPT,
+ sizeof(cp), &cp);
+ }
}
/* Enable security */
@@ -459,28 +469,54 @@ int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type)
{
BT_DBG("conn %p", conn);
+ /* For sdp we do not need the link key. */
if (sec_level == BT_SECURITY_SDP)
return 1;
+ /* For non 2.1 devices and low security level we do not need the
+ link key. */
if (sec_level == BT_SECURITY_LOW &&
(!conn->ssp_mode || !conn->hdev->ssp_mode))
return 1;
- if (conn->link_mode & HCI_LM_ENCRYPT)
- return hci_conn_auth(conn, sec_level, auth_type);
-
+ /* For other security levels we need link key. */
+ if (!(conn->link_mode & HCI_LM_AUTH))
+ goto do_auth;
+
+ /* An authenticated combination key has sufficient security for any
+ security level. */
+ if (conn->key_type == HCI_LK_AUTHENTICATED_COMBINATION)
+ goto do_encrypt;
+
+ /* An unauthenticated combination key has sufficient security for
+ security level 1 and 2. */
+ if (conn->key_type == HCI_LK_UNAUTHENTICATED_COMBINATION
+ && (sec_level == BT_SECURITY_MEDIUM
+ || sec_level == BT_SECURITY_LOW))
+ goto do_encrypt;
+
+ /* A combination key has always sufficient security for the security
+ levels 1 or 2. High security level requires that the combination key
+ was generated using the maximum PIN code length (16).
+ For pre 2.1 units. */
+ if ((conn->key_type == HCI_LK_COMBINATION))
+ if ((sec_level != BT_SECURITY_HIGH) || (conn->pin_len >= 16))
+ goto do_encrypt;
+
+do_auth:
if (test_and_set_bit(HCI_CONN_ENCRYPT_PEND, &conn->pend))
return 0;
- if (hci_conn_auth(conn, sec_level, auth_type)) {
- struct hci_cp_set_conn_encrypt cp;
- cp.handle = cpu_to_le16(conn->handle);
- cp.encrypt = 1;
- hci_send_cmd(conn->hdev, HCI_OP_SET_CONN_ENCRYPT,
- sizeof(cp), &cp);
- }
-
+ hci_conn_auth(conn, sec_level, auth_type);
return 0;
+
+do_encrypt:
+ if (conn->link_mode & HCI_LM_ENCRYPT)
+ return 1; /* sufficient link key */
+ else{
+ hci_conn_encrypt(conn);
+ return 0; /* auth pending */
+ }
}
EXPORT_SYMBOL(hci_conn_security);
@@ -713,6 +749,30 @@ int hci_get_conn_info(struct hci_dev *hdev, void __user *arg)
return copy_to_user(ptr, &ci, sizeof(ci)) ? -EFAULT : 0;
}
+int hci_set_conn_info(struct hci_dev *hdev, void __user *arg)
+{
+ struct hci_set_conn_info_req req;
+ struct hci_conn *conn;
+
+ if (copy_from_user(&req, arg, sizeof(req))) {
+ BT_DBG("copy from user failed");
+ return -EFAULT;
+ }
+
+ hci_dev_lock_bh(hdev);
+ conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &req.bdaddr);
+ if (conn) {
+ conn->pin_len = req.pin_len;
+ conn->key_type = req.key_type;
+ }
+ hci_dev_unlock_bh(hdev);
+
+ if (!conn)
+ return -ENOENT;
+
+ return 0;
+}
+
int hci_get_auth_info(struct hci_dev *hdev, void __user *arg)
{
struct hci_auth_info_req req;
@@ -725,6 +785,7 @@ int hci_get_auth_info(struct hci_dev *hdev, void __user *arg)
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &req.bdaddr);
if (conn)
req.type = conn->auth_type;
+ req.level = conn->sec_level;
hci_dev_unlock_bh(hdev);
if (!conn)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
old mode 100644
new mode 100755
index bfef5ba..6d6b04c
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1521,6 +1521,10 @@ static inline void hci_link_key_notify_evt(struct hci_dev *hdev, struct sk_buff
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
if (conn) {
hci_conn_hold(conn);
+ /* For Changed Combination Link Key the only link key has
+ * been changed, not link key type. */
+ if (conn->key_type != HCI_LK_CHANGEED_COMBINATION_KEY)
+ conn->key_type = ev->key_type;
conn->disc_timeout = HCI_DISCONN_TIMEOUT;
hci_conn_put(conn);
}
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 83acd16..502a7b0 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -269,6 +269,9 @@ static inline int hci_sock_bound_ioctl(struct sock *sk, unsigned int cmd, unsign
case HCIGETCONNINFO:
return hci_get_conn_info(hdev, (void __user *) arg);
+ case HCISETCONNINFO:
+ return hci_set_conn_info(hdev, (void __user *) arg);
+
case HCIGETAUTHINFO:
return hci_get_auth_info(hdev, (void __user *) arg);
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
old mode 100644
new mode 100755
index 7dca91b..2e248d5
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -2086,7 +2086,15 @@ static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt)
continue;
if (!status)
- set_bit(RFCOMM_AUTH_ACCEPT, &d->flags);
+ if (d->sec_level != BT_SECURITY_HIGH)
+ set_bit(RFCOMM_AUTH_ACCEPT, &d->flags);
+ else
+ if ((conn->key_type == HCI_LK_AUTHENTICATED_COMBINATION)
+ || (conn->key_type == HCI_LK_COMBINATION
+ && conn->pin_len >= 16))
+ set_bit(RFCOMM_AUTH_ACCEPT, &d->flags);
+ else
+ set_bit(RFCOMM_AUTH_REJECT, &d->flags);
else
set_bit(RFCOMM_AUTH_REJECT, &d->flags);
}
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] Fix handling empty fields in VCARDs
From: Johan Hedberg @ 2010-08-26 11:28 UTC (permalink / raw)
To: Lukasz Pawlik; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimN_hk1n7rDSBMpDP4-7ECV-4fm0NqHHwz0VBjo@mail.gmail.com>
Hi Lukasz,
On Thu, Aug 26, 2010, Lukasz Pawlik wrote:
> From c9a89b18acf88ac0546ba72d0a169d9c2f18656d Mon Sep 17 00:00:00 2001
> From: Lukasz Pawlik <lucas.pawlik@gmail.com>
> Date: Thu, 26 Aug 2010 11:33:33 +0200
> Subject: [PATCH] Fix handling empty fields in VCARDs
>
> Previously even mandatory TEL field was not printed in VCARD if it was
> empty. This patch fix this. Now tag TEL will be printed if phone number
> is not set in phonebook. This patch also fix handling category string
> for VCARDs in version 2.1 for url field.
> ---
> plugins/vcard.c | 54 ++++++++++++++++++++++++++++++++++++------------------
> 1 files changed, 36 insertions(+), 18 deletions(-)
Thanks. The patch is now upstream. Btw, it's written vCard and not VCARD
(I fixed this for you manually).
Johan
^ permalink raw reply
* Re: [PATCH 13/22] Add three new options for l2cap_options which used in setsockopt & getsockopt
From: David Vrabel @ 2010-08-26 10:41 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=8xVks--xcLXEQ_=sHfP3fY+Wvyctu5K_kMDG-@mail.gmail.com>
haijun liu wrote:
> From 897b281d14ba4cf9a5fbbf5ba65b84c85e688737 Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Mon, 23 Aug 2010 00:00:26 +0800
> Subject: [PATCH 13/22] Add three new options for l2cap_options which
> used in setsockopt & getsockopt.
>
> ---
> include/net/bluetooth/l2cap.h | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index d0ae9f5..4f87aec 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -65,6 +65,9 @@ struct l2cap_options {
> __u8 fcs;
> __u8 max_tx;
> __u16 txwin_size;
> + __u8 hschan_req;
This isn't the API that was agreed. See Mat Martineau's "Bluetooth:
Add socket option definitions for AMP" patch.
> + __u8 guaranteed;
Selecting guaranteed without being able to specify flow specs doesn't
seem useful.
I would like to see support for best-effort links merged first before
guaranteed links are considered.
> + __u8 reconfig;
I'd suggest removing this unless there's a profile/application that will
make use of it. I'd have thought that channel reconfiguration was
something the L2CAP stack does automatically when moving channels
between BR/EDR and AMP radios.
> };
David
--
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park, Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ http://www.csr.com/
Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
^ permalink raw reply
* [PATCH] Fix handling empty fields in VCARDs
From: Lukasz Pawlik @ 2010-08-26 9:48 UTC (permalink / raw)
To: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: 0001-Fix-handling-empty-fields-in-VCARDs.patch --]
[-- Type: text/x-patch, Size: 5044 bytes --]
From c9a89b18acf88ac0546ba72d0a169d9c2f18656d Mon Sep 17 00:00:00 2001
From: Lukasz Pawlik <lucas.pawlik@gmail.com>
Date: Thu, 26 Aug 2010 11:33:33 +0200
Subject: [PATCH] Fix handling empty fields in VCARDs
Previously even mandatory TEL field was not printed in VCARD if it was
empty. This patch fix this. Now tag TEL will be printed if phone number
is not set in phonebook. This patch also fix handling category string
for VCARDs in version 2.1 for url field.
---
plugins/vcard.c | 54 ++++++++++++++++++++++++++++++++++++------------------
1 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/plugins/vcard.c b/plugins/vcard.c
index a20ab4d..c4c4c5a 100644
--- a/plugins/vcard.c
+++ b/plugins/vcard.c
@@ -256,7 +256,7 @@ static void vcard_printf_number(GString *vcards, uint8_t format,
vcard_printf(vcards, buf, number);
}
-static void vcard_printf_tag(GString *vcards, const char *tag,
+static void vcard_printf_tag(GString *vcards, uint8_t format, const char *tag,
const char *category, const char *fld)
{
char *separator = "", *type = "";
@@ -265,12 +265,15 @@ static void vcard_printf_tag(GString *vcards, const char *tag,
if (tag == NULL || strlen(tag) == 0)
return;
- if (fld == NULL || strlen(fld) == 0)
+ if (fld == NULL || strlen(fld) == 0){
+ vcard_printf(vcards, "%s:", tag);
return;
+ }
if (category && strlen(category)) {
separator = ";";
- type = "TYPE=";
+ if (format == FORMAT_VCARD30)
+ type = "TYPE=";
} else {
category = "";
}
@@ -280,8 +283,9 @@ static void vcard_printf_tag(GString *vcards, const char *tag,
vcard_printf(vcards, "%s:%s", buf, fld);
}
-static void vcard_printf_slash_tag(GString *vcards, const char *tag,
- const char *category, const char *fld)
+static void vcard_printf_slash_tag(GString *vcards, uint8_t format,
+ const char *tag, const char *category,
+ const char *fld)
{
int len;
char *separator = "", *type = "";
@@ -290,12 +294,15 @@ static void vcard_printf_slash_tag(GString *vcards, const char *tag,
if (tag == NULL || strlen(tag) == 0)
return;
- if (fld == NULL || (len = strlen(fld)) == 0)
+ if (fld == NULL || (len = strlen(fld)) == 0){
+ vcard_printf(vcards, "%s:", tag);
return;
+ }
if (category && strlen(category)) {
separator = ";";
- type = "TYPE=";
+ if (format == FORMAT_VCARD30)
+ type = "TYPE=";
} else {
category = "";
}
@@ -314,9 +321,10 @@ static void vcard_printf_email(GString *vcards, uint8_t format,
char field[LEN_MAX];
int len = 0;
- if (!address || !(len = strlen(address)))
+ if (!address || !(len = strlen(address))){
+ vcard_printf(vcards, "EMAIL:");
return;
-
+ }
switch (category){
case EMAIL_TYPE_HOME:
if (format == FORMAT_VCARD21)
@@ -358,8 +366,10 @@ static gboolean org_fields_present(struct phonebook_contact *contact)
static void vcard_printf_org(GString *vcards,
struct phonebook_contact *contact)
{
- if (org_fields_present(contact) == FALSE)
+ if (org_fields_present(contact) == FALSE){
+ vcard_printf(vcards, "ORG:");
return;
+ }
vcard_printf(vcards, "ORG:%s;%s;%s", contact->company,
contact->department, contact->title);
@@ -428,9 +438,13 @@ void phonebook_add_contact(GString *vcards, struct phonebook_contact *contact,
vcard_printf_fullname(vcards, contact->fullname);
if (filter & FILTER_TEL) {
- GSList *l;
+ GSList *l = contact->numbers;
- for (l = contact->numbers; l; l = l->next) {
+ if (g_slist_length(l) == 0)
+ vcard_printf_number(vcards, format, NULL, 1,
+ TEL_TYPE_OTHER);
+
+ for (; l; l = l->next) {
struct phonebook_number *number = l->data;
vcard_printf_number(vcards, format, number->tel, 1,
@@ -439,9 +453,13 @@ void phonebook_add_contact(GString *vcards, struct phonebook_contact *contact,
}
if (filter & FILTER_EMAIL) {
- GSList *l;
+ GSList *l = contact->emails;
+
+ if (g_slist_length(l) == 0)
+ vcard_printf_email(vcards, format, NULL,
+ EMAIL_TYPE_OTHER);
- for (l = contact->emails; l; l = l->next){
+ for (; l; l = l->next){
struct phonebook_email *email = l->data;
vcard_printf_email(vcards, format, email->address, email->type);
@@ -452,18 +470,18 @@ void phonebook_add_contact(GString *vcards, struct phonebook_contact *contact,
vcard_printf_adr(vcards, contact);
if (filter & FILTER_BDAY)
- vcard_printf_tag(vcards, "BDAY", NULL, contact->birthday);
+ vcard_printf_tag(vcards, format, "BDAY", NULL, contact->birthday);
if (filter & FILTER_NICKNAME)
- vcard_printf_slash_tag(vcards, "NICKNAME", NULL,
+ vcard_printf_slash_tag(vcards, format, "NICKNAME", NULL,
contact->nickname);
if (filter & FILTER_URL)
- vcard_printf_slash_tag(vcards, "URL", "INTERNET",
+ vcard_printf_slash_tag(vcards, format, "URL", "INTERNET",
contact->website);
if (filter & FILTER_PHOTO)
- vcard_printf_tag(vcards, "PHOTO", NULL, contact->photo);
+ vcard_printf_tag(vcards, format, "PHOTO", NULL, contact->photo);
if (filter & FILTER_ORG)
vcard_printf_org(vcards, contact);
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] Fix handling empty fields in VCARDs
From: Johan Hedberg @ 2010-08-26 8:15 UTC (permalink / raw)
To: Lukasz Pawlik; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimB9Tvxt6zZKzfHwhGc_j8MnGTo1DbeH-pYoQpe@mail.gmail.com>
Hi Lukasz,
> From 4bb29cdaf4a9923ab62973c38914ebb67f718d64 Mon Sep 17 00:00:00 2001
> From: Lukasz Pawlik <lucas.pawlik@gmail.com>
> Date: Thu, 26 Aug 2010 09:10:26 +0200
> Subject: [PATCH] Fix handling empty fields in VCARDs
>
> Previously even mandatory TEL field was not printed in VCARD if it was
> empty. This patch fix this. Now tag TEL will be printed if phone number
> is not set in phonebook. This patch also fix handling category string
> for VCARDs in version 2.1 for url field.
> ---
> plugins/vcard.c | 51 ++++++++++++++++++++++++++++++++-------------------
> 1 files changed, 32 insertions(+), 19 deletions(-)
In general the patch seems fine, but you need to fix your editor
settings when dealing with BlueZ code. It seems like you're using spaces
for indentation when the coding style is tabs-only:
> - for (l = contact->numbers; l; l = l->next) {
> + if (g_slist_length(l) == 0)
> + vcard_printf_number(vcards, format, NULL, 1,
> + TEL_TYPE_OTHER);
All of the above three added lines are indented with spaces.
> + for (; l; l = l->next) {
This added line is indented with tabs. How did you manage to get your
editor to produce such mixed results?
> + if (g_slist_length(l) == 0)
> + vcard_printf_email(vcards, format, NULL,
> + EMAIL_TYPE_OTHER);
The first line is tabs but the two others use spaces!?
> - for (l = contact->emails; l; l = l->next){
> + for (; l; l = l->next){
Again spaces here.
Please enable whitespace visualization and use proper editor settings to
avoid these issues in the future. Thanks.
Johan
^ permalink raw reply
* [PATCH] Fix handling empty fields in VCARDs
From: Lukasz Pawlik @ 2010-08-26 7:45 UTC (permalink / raw)
To: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: 0001-Fix-handling-empty-fields-in-VCARDs.patch --]
[-- Type: text/x-patch, Size: 5038 bytes --]
From 4bb29cdaf4a9923ab62973c38914ebb67f718d64 Mon Sep 17 00:00:00 2001
From: Lukasz Pawlik <lucas.pawlik@gmail.com>
Date: Thu, 26 Aug 2010 09:10:26 +0200
Subject: [PATCH] Fix handling empty fields in VCARDs
Previously even mandatory TEL field was not printed in VCARD if it was
empty. This patch fix this. Now tag TEL will be printed if phone number
is not set in phonebook. This patch also fix handling category string
for VCARDs in version 2.1 for url field.
---
plugins/vcard.c | 51 ++++++++++++++++++++++++++++++++-------------------
1 files changed, 32 insertions(+), 19 deletions(-)
diff --git a/plugins/vcard.c b/plugins/vcard.c
index a20ab4d..a163d1b 100644
--- a/plugins/vcard.c
+++ b/plugins/vcard.c
@@ -256,7 +256,7 @@ static void vcard_printf_number(GString *vcards, uint8_t format,
vcard_printf(vcards, buf, number);
}
-static void vcard_printf_tag(GString *vcards, const char *tag,
+static void vcard_printf_tag(GString *vcards, uint8_t format, const char *tag,
const char *category, const char *fld)
{
char *separator = "", *type = "";
@@ -265,9 +265,10 @@ static void vcard_printf_tag(GString *vcards, const char *tag,
if (tag == NULL || strlen(tag) == 0)
return;
- if (fld == NULL || strlen(fld) == 0)
+ if (fld == NULL || strlen(fld) == 0){
+ vcard_printf(vcards, "%s:", tag);
return;
-
+ }
if (category && strlen(category)) {
separator = ";";
type = "TYPE=";
@@ -280,8 +281,9 @@ static void vcard_printf_tag(GString *vcards, const char *tag,
vcard_printf(vcards, "%s:%s", buf, fld);
}
-static void vcard_printf_slash_tag(GString *vcards, const char *tag,
- const char *category, const char *fld)
+static void vcard_printf_slash_tag(GString *vcards, uint8_t format,
+ const char *tag, const char *category,
+ const char *fld)
{
int len;
char *separator = "", *type = "";
@@ -290,9 +292,10 @@ static void vcard_printf_slash_tag(GString *vcards, const char *tag,
if (tag == NULL || strlen(tag) == 0)
return;
- if (fld == NULL || (len = strlen(fld)) == 0)
+ if (fld == NULL || (len = strlen(fld)) == 0){
+ vcard_printf(vcards, "%s:", tag);
return;
-
+ }
if (category && strlen(category)) {
separator = ";";
type = "TYPE=";
@@ -314,9 +317,10 @@ static void vcard_printf_email(GString *vcards, uint8_t format,
char field[LEN_MAX];
int len = 0;
- if (!address || !(len = strlen(address)))
+ if (!address || !(len = strlen(address))){
+ vcard_printf(vcards, "EMAIL:");
return;
-
+ }
switch (category){
case EMAIL_TYPE_HOME:
if (format == FORMAT_VCARD21)
@@ -358,9 +362,10 @@ static gboolean org_fields_present(struct phonebook_contact *contact)
static void vcard_printf_org(GString *vcards,
struct phonebook_contact *contact)
{
- if (org_fields_present(contact) == FALSE)
+ if (org_fields_present(contact) == FALSE){
+ vcard_printf(vcards, "ORG:");
return;
-
+ }
vcard_printf(vcards, "ORG:%s;%s;%s", contact->company,
contact->department, contact->title);
}
@@ -428,9 +433,13 @@ void phonebook_add_contact(GString *vcards, struct phonebook_contact *contact,
vcard_printf_fullname(vcards, contact->fullname);
if (filter & FILTER_TEL) {
- GSList *l;
+ GSList *l = contact->numbers;
- for (l = contact->numbers; l; l = l->next) {
+ if (g_slist_length(l) == 0)
+ vcard_printf_number(vcards, format, NULL, 1,
+ TEL_TYPE_OTHER);
+
+ for (; l; l = l->next) {
struct phonebook_number *number = l->data;
vcard_printf_number(vcards, format, number->tel, 1,
@@ -439,9 +448,13 @@ void phonebook_add_contact(GString *vcards, struct phonebook_contact *contact,
}
if (filter & FILTER_EMAIL) {
- GSList *l;
+ GSList *l = contact->emails;
+
+ if (g_slist_length(l) == 0)
+ vcard_printf_email(vcards, format, NULL,
+ EMAIL_TYPE_OTHER);
- for (l = contact->emails; l; l = l->next){
+ for (; l; l = l->next){
struct phonebook_email *email = l->data;
vcard_printf_email(vcards, format, email->address, email->type);
@@ -452,18 +465,18 @@ void phonebook_add_contact(GString *vcards, struct phonebook_contact *contact,
vcard_printf_adr(vcards, contact);
if (filter & FILTER_BDAY)
- vcard_printf_tag(vcards, "BDAY", NULL, contact->birthday);
+ vcard_printf_tag(vcards, format, "BDAY", NULL, contact->birthday);
if (filter & FILTER_NICKNAME)
- vcard_printf_slash_tag(vcards, "NICKNAME", NULL,
+ vcard_printf_slash_tag(vcards, format, "NICKNAME", NULL,
contact->nickname);
if (filter & FILTER_URL)
- vcard_printf_slash_tag(vcards, "URL", "INTERNET",
+ vcard_printf_slash_tag(vcards, format, "URL", "INTERNET",
contact->website);
if (filter & FILTER_PHOTO)
- vcard_printf_tag(vcards, "PHOTO", NULL, contact->photo);
+ vcard_printf_tag(vcards, format, "PHOTO", NULL, contact->photo);
if (filter & FILTER_ORG)
vcard_printf_org(vcards, contact);
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 02/22] Add eL2cap new features macro definition
From: haijun liu @ 2010-08-26 6:59 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <20100826010116.GC16062@vigoh>
Hi Gustavo,
On Thu, Aug 26, 2010 at 9:01 AM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
>
> We don't need the L2CAP_FEAT_RESERVED here.
>
Thank you, we'll remove it.
^ permalink raw reply
* New subscription request to list Bluez-devel from tang705@yahoo.com.cn
From: bluez-devel-owner @ 2010-08-26 2:57 UTC (permalink / raw)
To: bluez-devel-owner
Your authorization is required for a mailing list subscription request
approval:
For: tang705@yahoo.com.cn
List: bluez-devel@lists.sourceforge.net
At your convenience, visit:
https://lists.sourceforge.net/lists/admindb/bluez-devel
to process the request.
^ permalink raw reply
* Re: [PATCH 17/22] Add parameter enable_highspeed to control highspeed code in running phase
From: Gustavo F. Padovan @ 2010-08-26 1:28 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTinaFobBK5OiFaV+fGsEe3JRTy7_dT-FFLjSt-Rq@mail.gmail.com>
Hi Haijun,
* haijun liu <liuhaijun.er@gmail.com> [2010-08-26 06:38:27 +0800]:
> From c3fba9b5d7546576d9eb1d30eb45c453c8ee1dff Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Mon, 23 Aug 2010 00:17:24 +0800
> Subject: [PATCH 17/22] Add parameter enable_highspeed to control
> highspeed code in running phase.
>
> ---
> net/bluetooth/l2cap.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 449cbdd..e7f7067 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -4851,6 +4851,9 @@ module_exit(l2cap_exit);
> module_param(enable_ertm, bool, 0644);
> MODULE_PARM_DESC(enable_ertm, "Enable enhanced retransmission mode");
>
> +module_param(enable_highspeed, bool, 0644);
> +MODULE_PARM_DESC(enable_highspeed, "Enable Bluetooth V3.0+HS
> highspeed feature");
I did not run this code, but I'm sure that this patch doesn't compile. ;)
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 22/22] Parse EFS & EWS parameters for in l2cap_parse_conf_rsp
From: Gustavo F. Padovan @ 2010-08-26 1:26 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimQsNmOc-Mv30xA0x_NEpxvs4kXjGfuOfqHxEDP@mail.gmail.com>
Hi Haijun,
* haijun liu <liuhaijun.er@gmail.com> [2010-08-26 06:41:02 +0800]:
> From 72afc40895eb272269156b98ab3024ac4f130b0d Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Wed, 25 Aug 2010 22:08:47 +0800
> Subject: [PATCH 22/22] Parse EFS & EWS parameters for in l2cap_parse_conf_rsp.
>
> ---
> net/bluetooth/l2cap.c | 35 ++++++++++++++++++++++++++++++-----
> 1 files changed, 30 insertions(+), 5 deletions(-)
For 20, 21 and 22. Create a separeted patches for EFS and EWS, they are
different stuff and do not overstep the column 80 when coding.
Fix that and the we can do another review ;)
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 16/22] Store new configuration values in l2cap_pinfo
From: Gustavo F. Padovan @ 2010-08-26 1:21 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimnW0jk19cDG+r64Q+-HxdH2qdTBirTMB7ikr1W@mail.gmail.com>
Hi Haijun,
* haijun liu <liuhaijun.er@gmail.com> [2010-08-26 06:37:30 +0800]:
> From d093975dde6d85c824a5aaac943d676100810010 Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Mon, 23 Aug 2010 00:09:56 +0800
> Subject: [PATCH 16/22] Store new configuration values in l2cap_pinfo.
>
> ---
> include/net/bluetooth/l2cap.h | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 2d864d4..f2dd65d 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -406,6 +406,15 @@ struct l2cap_pinfo {
> __u16 remote_mps;
> __u16 mps;
>
> + __u8 ext_flowspec_enable;
> + struct ext_flow_spec loc_efs;
> + struct ext_flow_spec rem_efs;
> +
> + __u8 extwin_enable;
> + __u16 extwin_size;
> + __u8 rem_extwin_enable;
> + __u16 rem_extwin_size;
The same comment in 14/22 goes here and others patches in the patch set.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 14/22] Add three new options for l2cap_pinfo
From: Gustavo F. Padovan @ 2010-08-26 1:13 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTikf=UeHUaT8wLnwUDnxgg0ouz1f6QkgYZvbr8t8@mail.gmail.com>
Hi Haijun,
* haijun liu <liuhaijun.er@gmail.com> [2010-08-26 06:36:35 +0800]:
> From 06124ba36c1f6c9cf5bef861751ce749712cecb0 Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Mon, 23 Aug 2010 00:03:39 +0800
> Subject: [PATCH 14/22] Add three new options for l2cap_pinfo.
>
> ---
> include/net/bluetooth/l2cap.h | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 4f87aec..7a9bec4 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -367,6 +367,10 @@ struct l2cap_pinfo {
> __u8 num_conf_req;
> __u8 num_conf_rsp;
>
> + __u8 hschan_req;
> + __u8 guaranteed;
> + __u8 reconfig;
> +
The same here, you have to explain why you to increase struct l2cap_pinfo
size, and maybe only add them when you have a piece of code that uses
that.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 13/22] Add three new options for l2cap_options which used in setsockopt & getsockopt
From: Gustavo F. Padovan @ 2010-08-26 1:10 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=8xVks--xcLXEQ_=sHfP3fY+Wvyctu5K_kMDG-@mail.gmail.com>
Hi Haijun,
* haijun liu <liuhaijun.er@gmail.com> [2010-08-26 06:36:05 +0800]:
> From 897b281d14ba4cf9a5fbbf5ba65b84c85e688737 Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Mon, 23 Aug 2010 00:00:26 +0800
> Subject: [PATCH 13/22] Add three new options for l2cap_options which
> used in setsockopt & getsockopt.
>
> ---
> include/net/bluetooth/l2cap.h | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index d0ae9f5..4f87aec 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -65,6 +65,9 @@ struct l2cap_options {
> __u8 fcs;
> __u8 max_tx;
> __u16 txwin_size;
> + __u8 hschan_req;
> + __u8 guaranteed;
> + __u8 reconfig;
> };
Here you are extending the API with the userspace, so we need a patch
for each option explaining why you need it and why the userspace should
care about that. It's a good idea here add this option to l2cap_options
in the same patch you implement the handling for it apply such patch hen
it is actually needed. ;)
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox