Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200407085610.231013-1-apusaka@google.com> <CABBYNZ+3SUbZbO+BJ7BX3uj5dds=KzWpe316SghncrOE8ikLBw@mail.gmail.com>
In-Reply-To: <CABBYNZ+3SUbZbO+BJ7BX3uj5dds=KzWpe316SghncrOE8ikLBw@mail.gmail.com>
From:   Archie Pusaka <apusaka@google.com>
Date:   Wed, 8 Apr 2020 11:41:45 +0800
Message-ID: <CAJQfnxHTiJ6ft5TF=8YC2KxWkBL3r57AoR8YTh96-T2uNt4mHg@mail.gmail.com>
Subject: Re: [Bluez PATCH v4 0/4] Check the signature of att packets
To:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc:     linux-bluetooth <linux-bluetooth@vger.kernel.org>,
        Archie Pusaka <apusaka@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

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