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;
Date:   Wed, 15 Jan 2020 20:09:04 +0100
From:   "michal.lowas-rzechonek@silvair.com" 
        <michal.lowas-rzechonek@silvair.com>
To:     "Gix, Brian" <brian.gix@intel.com>
Cc:     "jakub.witowski@silvair.com" <jakub.witowski@silvair.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        "istotlan@ingas-xps13.amr.corp.intel.com" 
        <istotlan@ingas-xps13.amr.corp.intel.com>
Subject: Re: [PATCH 0/1] Sequence number out of range fix
Message-ID: <20200115190904.qcusabsj3ti7sid4@kynes>
Mail-Followup-To: "Gix, Brian" <brian.gix@intel.com>,
        "jakub.witowski@silvair.com" <jakub.witowski@silvair.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        "istotlan@ingas-xps13.amr.corp.intel.com" <istotlan@ingas-xps13.amr.corp.intel.com>
References: <20200115170830.7479-1-jakub.witowski@silvair.com>
 <4da92cbf6134822629bd2204fb7161c04008780e.camel@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <4da92cbf6134822629bd2204fb7161c04008780e.camel@intel.com>
User-Agent: NeoMutt/20180716
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Brian, Jakub,

On 01/15, Gix, Brian wrote:
> This is a very dangerous course of action.  The suggested patch might
> potentially cause a node to re-use a sequence number more than once,
> which would cause a "dirty nonce" condition, and allow unauthorized
> entities to derive the encrypted data without the keys.

Good point. Note that his is also possible in the current
implementation: since seq_num is applied to nonce with a 24bit mask,
it's going to wrap around.

> While this is technically possible, especially with a daemon that
> might be looping back messages internally without ever using the OTA
> interface, it is not really possible in an actual BT driven real life
> system.

There is another option to exhaust the 24 bit range: we have the
overcommit mechanism in the storage. Let's say the daemon starts, sends
a few messages (and overcommits the sequence by a certain value), then
either the daemon, or the system crashes.

Do that a few times, and you end up with storage containing sequence
number exceeding 24 bits.

> Beacuse we store sequence numbers internally with a u32 sized data
> type, we should *let* the value go over the max legal sequence
> nunmbver of 0xFFFFFF (perhaps capping it at 0x01000000 to prevent
> "super overflows").  Then we *reject* all send requests with a
> sequence number > 0xFFFFFF.

Sounds good.

-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND