How Perfect Offline Wallets Can Still Leak Bitcoin Private Keys

[hhanh00]

If someone would like to review my code, I'd be glad - https://github.com/hhanh00/offlinesig - but it's not easy to get someone to do it.
I didn't rewrite a crypto library. I used bouncycastle and provided a generator for k following the RFCxxxx. The only values exchanged are (r, s) and I don't see any other sidechannel since it works offline.

[1715129568]

1715129568

[1715129568]

1715129568

[bcearl]

If someone would like to review my code, I'd be glad - https://github.com/hhanh00/offlinesig - but it's not easy to get someone to do it.
I didn't rewrite a crypto library. I used bouncycastle and provided a generator for k following the RFCxxxx. The only values exchanged are (r, s) and I don't see any other sidechannel since it works offline.

I think he meant code revision in general, not just for this single potential bug. You can't just review a Bitcoin wallet by reading it once or twice, it's way too complicated. So better stick with known code that has some user base actually testing it.

[gmaxwell]

I tried to give it a quick glance at least, but I'm unfamiliar with the language... and when I couldn't even tell how the private key was being passed to the nonce generation function I gave up.

Though it is the case that whatever glance I would have given it wouldn't really be enough to de-risk it compared to widely reviewed code... I figured a glance would be the least I could do. Well, seems not, a failed glance was the least I could do.

[hhanh00]

Thanks for trying, I appreciate it.

ECDSASigner is a bouncycastle class that accepts an optional provider for the nonce https://github.com/bcgit/bc-java/blob/master/core/src/main/java/org/bouncycastle/crypto/signers/DSAKCalculator.java.

The implementation of ECDSASigner will pass the secret to the k calculator.
It's done in

Code:

    val kGen = new RFC6979KCalculator
    val signer = new ECDSASigner(kGen)

    val params = new ECPrivateKeyParameters(secret.bigInteger, Bitcoin.ecDomain)

Then it comes to DSAKCalculator through

Code:

  override def init(q: BigInteger, x: BigInteger, m: Array[Byte]): Unit

which is Scala for

Code:

    /**
     * Deterministic initialiser.
     *
     * @param n the order of the DSA group.
     * @param d the DSA private value.
     * @param message the message being signed.
     */
    void init(BigInteger n, BigInteger d, byte[] message);

The rest is RFC6979.

I think he meant code revision in general, not just for this single potential bug. You can't just review a Bitcoin wallet by reading it once or twice, it's way too complicated. So better stick with known code that has some user base actually testing it.

I wrote this wallet because there is no implementation that fits my need and that I feel confident enough with. Basically for the same reasons that you mention. The most popular wallets don't have the features I want and the others are not reviewed enough to qualify either. I tried to reduce risk as much as I could by using standard libraries where possible. It leaves BIP32, RFC6979 and tx serialization to write. It's hard but I wouldn't call this impossible to do. I welcome as much review as possible. Maybe you could take a look?

Thanks,
--h