| Message ID | 20200407085610.231013-1-apusaka@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Check the signature of att packets | expand |
Hi Archie, On Tue, Apr 7, 2020 at 1:56 AM Archie Pusaka <apusaka@google.com> wrote: > > From: Archie Pusaka <apusaka@chromium.org> > > According to bluetooth spec Ver 5.1, Vol 3, Part C (GAP), 10.4.2 > A device receiving signed data shall authenticate it by performing > the Signing Algorithm. The signed data shall be authenticated by > performing the Signing Algorithm where m is the Data PDU to be > authenticated, k is the stored CSRK and the SignCounter is the > received counter value. If the MAC computed by the Signing > Algorithm does not match the received MAC, the verification fails > and the Host shall ignore the received Data PDU. > > Currently bluez ignore the signature of received signed att > packets, as the function bt_crypto_sign_att() only generates the > signature, and not actually make any check about the genuineness > of the signature itself. > > This patch also fix a wrong boolean condition which prevents > handle_signed() to be called. > > Tested to pass these BT certification test > SM/MAS/SIGN/BV-03-C > SM/MAS/SIGN/BI-01-C > > Changes in v4: > - Fix wrong variable assignment > - Fixing test-gatt.c > > Changes in v3: > - Add check for the case where pdu_len < ATT_SIGN_LEN > - Add unit test > - Separate into three patches > > Changes in v2: > - Move the signature verification part to crypto.c > - Attempt not to copy the whole pdu while verifying the signature > by not separating the opcode from the rest of pdu too early, so > we don't have to rejoin them later. > > Archie Pusaka (4): > shared/crypto: Add bt_crypto_verify_att_sign > unit/test-crypto: test for bt_crypto_verify_att_sign > shared/att: Check the signature of att packets > unit/test-gatt: Fix unknown request with signed bit > > src/shared/att.c | 25 +++++++++---------- > src/shared/crypto.c | 28 +++++++++++++++++++-- > src/shared/crypto.h | 2 ++ > unit/test-crypto.c | 59 +++++++++++++++++++++++++++++++++++++++++++++ > unit/test-gatt.c | 32 ++++++++++++++++++++---- > 5 files changed, 126 insertions(+), 20 deletions(-) > > -- > 2.26.0.292.g33ef6b2f38-goog Ive applied 1-3 and reworked a little bit the third patch so we actually attempt to find a handler before we attempt to check the signature.
Hi Luiz, On Wed, 8 Apr 2020 at 04:07, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Archie, > > On Tue, Apr 7, 2020 at 1:56 AM Archie Pusaka <apusaka@google.com> wrote: > > > > From: Archie Pusaka <apusaka@chromium.org> > > > > According to bluetooth spec Ver 5.1, Vol 3, Part C (GAP), 10.4.2 > > A device receiving signed data shall authenticate it by performing > > the Signing Algorithm. The signed data shall be authenticated by > > performing the Signing Algorithm where m is the Data PDU to be > > authenticated, k is the stored CSRK and the SignCounter is the > > received counter value. If the MAC computed by the Signing > > Algorithm does not match the received MAC, the verification fails > > and the Host shall ignore the received Data PDU. > > > > Currently bluez ignore the signature of received signed att > > packets, as the function bt_crypto_sign_att() only generates the > > signature, and not actually make any check about the genuineness > > of the signature itself. > > > > This patch also fix a wrong boolean condition which prevents > > handle_signed() to be called. > > > > Tested to pass these BT certification test > > SM/MAS/SIGN/BV-03-C > > SM/MAS/SIGN/BI-01-C > > > > Changes in v4: > > - Fix wrong variable assignment > > - Fixing test-gatt.c > > > > Changes in v3: > > - Add check for the case where pdu_len < ATT_SIGN_LEN > > - Add unit test > > - Separate into three patches > > > > Changes in v2: > > - Move the signature verification part to crypto.c > > - Attempt not to copy the whole pdu while verifying the signature > > by not separating the opcode from the rest of pdu too early, so > > we don't have to rejoin them later. > > > > Archie Pusaka (4): > > shared/crypto: Add bt_crypto_verify_att_sign > > unit/test-crypto: test for bt_crypto_verify_att_sign > > shared/att: Check the signature of att packets > > unit/test-gatt: Fix unknown request with signed bit > > > > src/shared/att.c | 25 +++++++++---------- > > src/shared/crypto.c | 28 +++++++++++++++++++-- > > src/shared/crypto.h | 2 ++ > > unit/test-crypto.c | 59 +++++++++++++++++++++++++++++++++++++++++++++ > > unit/test-gatt.c | 32 ++++++++++++++++++++---- > > 5 files changed, 126 insertions(+), 20 deletions(-) > > > > -- > > 2.26.0.292.g33ef6b2f38-goog > > Ive applied 1-3 and reworked a little bit the third patch so we > actually attempt to find a handler before we attempt to check the > signature. Ack, thanks a bunch! Regards, Archie
From: Archie Pusaka <apusaka@chromium.org> According to bluetooth spec Ver 5.1, Vol 3, Part C (GAP), 10.4.2 A device receiving signed data shall authenticate it by performing the Signing Algorithm. The signed data shall be authenticated by performing the Signing Algorithm where m is the Data PDU to be authenticated, k is the stored CSRK and the SignCounter is the received counter value. If the MAC computed by the Signing Algorithm does not match the received MAC, the verification fails and the Host shall ignore the received Data PDU. Currently bluez ignore the signature of received signed att packets, as the function bt_crypto_sign_att() only generates the signature, and not actually make any check about the genuineness of the signature itself. This patch also fix a wrong boolean condition which prevents handle_signed() to be called. Tested to pass these BT certification test SM/MAS/SIGN/BV-03-C SM/MAS/SIGN/BI-01-C Changes in v4: - Fix wrong variable assignment - Fixing test-gatt.c Changes in v3: - Add check for the case where pdu_len < ATT_SIGN_LEN - Add unit test - Separate into three patches Changes in v2: - Move the signature verification part to crypto.c - Attempt not to copy the whole pdu while verifying the signature by not separating the opcode from the rest of pdu too early, so we don't have to rejoin them later. Archie Pusaka (4): shared/crypto: Add bt_crypto_verify_att_sign unit/test-crypto: test for bt_crypto_verify_att_sign shared/att: Check the signature of att packets unit/test-gatt: Fix unknown request with signed bit src/shared/att.c | 25 +++++++++---------- src/shared/crypto.c | 28 +++++++++++++++++++-- src/shared/crypto.h | 2 ++ unit/test-crypto.c | 59 +++++++++++++++++++++++++++++++++++++++++++++ unit/test-gatt.c | 32 ++++++++++++++++++++---- 5 files changed, 126 insertions(+), 20 deletions(-)