Return-Path: Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 1D2BC1AEF for ; Thu, 8 Oct 2015 00:29:46 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-ig0-f172.google.com (mail-ig0-f172.google.com [209.85.213.172]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 6811879 for ; Thu, 8 Oct 2015 00:29:45 +0000 (UTC) Received: by igcpe7 with SMTP id pe7so1952465igc.0 for ; Wed, 07 Oct 2015 17:29:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type; bh=YYLxkGgHh6j0msYdb0ucHGIksofq0VExMUFxsr+cUp4=; b=u2aLLUZjdG9qEnP4kjkpM8kbZC1R8Gxxmk1ILegs2HYzesxON8vvz5HZ9Dbgqhr9Yp XEtunV3u9+JphJ+cl/tb6JkVJWOF4Ne9rAxkT1QweAykmKaxSJANA5i44D7r1hPS3Fm1 nKXId2pay0pSfliE3xGDFjB+7gofN0uOBcxVH9K2Z/0AeyDmItk45r0BcxTTgHfTnjOt lJyXWLCKp2sxguZrcqbUcRKSx0NLVTjmjUvCcq+VRJhaGo+s09IaKOBmuxOz1IdQ/Xaf twetYXhrdDbMzJG7sFxRHlYU+zqzdf9+DaVafZFsQROHPIZKAdo3M3zR4c4Oyi/zD09Y Vyyg== X-Received: by 10.50.28.113 with SMTP id a17mr495622igh.67.1444264184896; Wed, 07 Oct 2015 17:29:44 -0700 (PDT) MIME-Version: 1.0 Received: by 10.64.195.131 with HTTP; Wed, 7 Oct 2015 17:29:25 -0700 (PDT) In-Reply-To: References: <56155572.5040501@domob.eu> From: "James O'Beirne" Date: Wed, 7 Oct 2015 17:29:25 -0700 Message-ID: To: Daniel Kraft Content-Type: multipart/alternative; boundary=089e01538ac8b3e2d205218cf5fa X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,HTML_MESSAGE,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: bitcoin-dev@lists.linuxfoundation.org Subject: Re: [bitcoin-dev] The new obfuscation patch & GetStats X-BeenThere: bitcoin-dev@lists.linuxfoundation.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Bitcoin Development Discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Oct 2015 00:29:46 -0000 --089e01538ac8b3e2d205218cf5fa Content-Type: text/plain; charset=UTF-8 This has been confirmed as a bug. Thanks again for reporting. I've filed a fix here (https://github.com/bitcoin/bitcoin/pull/6777), and will be writing tests to prevent regressions. On Wed, Oct 7, 2015 at 4:32 PM, James O'Beirne wrote: > Hey, Daniel. > > Patch author here. Thanks for the diligence; I think this indeed may be an > oversight, though I'm going to need to look into a bit more thoroughly at > home. Curious that it didn't fail any of the automated tests. > > Correct me if I'm wrong, but the only actual invocation of that method is > here > > (and even then, proxied through a few layers of CCoinView-machinery). In > fact, this line > makes > me suspect that the implementation of GetStats you reference may be dead > code. > > In any case, you raise a good point: if users of CLevelDBWrapper go > directly for the iterator, they run the risk of dealing with obfuscated > data. This should be remedied somehow. > > I'll give it more look this evening. > > Thanks again for the find, > James > > On Wed, Oct 7, 2015 at 10:25 AM, Daniel Kraft via bitcoin-dev < > bitcoin-dev@lists.linuxfoundation.org> wrote: > >> Hi! >> >> I hope this is not a stupid question, but I thought I'd ask here first >> instead of opening a Github ticket (in case I'm wrong). >> >> With the recently merged "obfuscation" patch, content of the >> "chainstate" LevelDB is obfuscated by XOR'ing against a random "key". >> This is handled by CLevelDBWrapper's Read/Write methods, which probably >> cover most of the usecases. >> >> *However*, shouldn't it also be handled when iterating over the >> database? In particular, I would expect that the obfuscation key is >> applied before line 119 in txdb.cpp (i. e., while iterating over the >> coin database in CCoinsViewDB::GetStats). >> >> Is there a reason why this need not be done there, or is this an actual >> oversight? >> >> Yours, >> Daniel >> >> -- >> http://www.domob.eu/ >> OpenPGP: 1142 850E 6DFF 65BA 63D6 88A8 B249 2AC4 A733 0737 >> Namecoin: id/domob -> https://nameid.org/?name=domob >> -- >> Done: Arc-Bar-Cav-Hea-Kni-Ran-Rog-Sam-Tou-Val-Wiz >> To go: Mon-Pri >> >> >> _______________________________________________ >> bitcoin-dev mailing list >> bitcoin-dev@lists.linuxfoundation.org >> https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev >> >> > --089e01538ac8b3e2d205218cf5fa Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
This has been confirmed as a bug. Thanks again for reporti= ng. I've filed a fix here (https://github.com/bitcoin/bitcoin/pull/6777), and will be= writing tests to prevent regressions.

=
On Wed, Oct 7, 2015 at 4:32 PM, James O'Beir= ne <james.obeirne@gmail.com> wrote:
Hey, Daniel.

Patch aut= hor here. Thanks for the diligence; I think this indeed may be an oversight= , though I'm going to need to look into a bit more thoroughly at home. = Curious that it didn't fail any of the automated tests.

<= /div>
Correct me if I'm wrong, but the only actual invocation of th= at method is here (and even then, proxied = through a few layers of CCoinView-machinery). In fact, = this line makes me suspect that the implementation of GetStats you refe= rence may be dead code.

In any case, you raise a g= ood point: if users of CLevelDBWrapper go directly for the iterator, they r= un the risk of dealing with obfuscated data. This should be remedied someho= w.

I'll give it more look this evening.
<= div>
Thanks again for the find,
James

On Wed, Oct 7, 2015 at 10:25 AM, Daniel Kraft via bitcoin-dev <bitcoin-dev@lists.linuxfoundation.org> wrote:
Hi!

I hope this is not a stupid question, but I thought I'd ask here first<= br> instead of opening a Github ticket (in case I'm wrong).

With the recently merged "obfuscation" patch, content of the
"chainstate" LevelDB is obfuscated by XOR'ing against a rando= m "key".
This is handled by CLevelDBWrapper's Read/Write methods, which probably=
cover most of the usecases.

*However*, shouldn't it also be handled when iterating over the
database?=C2=A0 In particular, I would expect that the obfuscation key is applied before line 119 in txdb.cpp (i. e., while iterating over the
coin database in CCoinsViewDB::GetStats).

Is there a reason why this need not be done there, or is this an actual
oversight?

Yours,
Daniel

--
http:= //www.domob.eu/
OpenPGP: 1142 850E 6DFF 65BA 63D6=C2=A0 88A8 B249 2AC4 A733 0737
Namecoin: id/domob -> https://nameid.org/?name=3Ddomob
--
Done:=C2=A0 Arc-Bar-Cav-Hea-Kni-Ran-Rog-Sam-Tou-Val-Wiz
To go: Mon-Pri


_____________________________________________= __
bitcoin-dev mailing list
= bitcoin-dev@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mail= man/listinfo/bitcoin-dev



--089e01538ac8b3e2d205218cf5fa--