OnchainKeyManager: add new validation checks

Check that
- non-segwit uxto has been provided
- inputs are signed with SIGHASH_ALL

eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala

@@ -171,7 +171,7 @@ trait Eclair {
 
   def getOnchainMasterPubKey(account: Long): String
 
-  def getOnchainMasterMasterFingerprintHex: String
+  def getOnchainMasterFingerprintHex: String
 
   def getDescriptors(fingerprint: Int, chain_opt: Option[String], account: Long): (List[String], List[String])
 
@@ -656,5 +656,5 @@ class EclairImpl(appKit: Kit) extends Eclair with Logging {
     Future.successful(())
   }
 
-  override def getOnchainMasterMasterFingerprintHex: String = this.appKit.nodeParams.onchainKeyManager.getOnchainMasterMasterFingerprintHex
+  override def getOnchainMasterFingerprintHex: String = this.appKit.nodeParams.onchainKeyManager.getOnchainMasterMasterFingerprintHex
 }

eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/rpc/BitcoinCoreClient.scala

@@ -458,7 +458,7 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val onchainKeyManag
    * If the current wallet uses Eclair to sign transaction, then we'll use our onchain key manager to sign the transaction,
    * with the following assumptions:
    * - all inputs belong to us
-   * - all outputs except for the one that sends to `pubkeyScript` belong to us`
+   * - all outputs except for the one that sends to `pubkeyScript` belong to us
    *
    *
    * @param pubkeyScript public key script to sent funds to

eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxFunder.scala

@@ -284,9 +284,9 @@ private class ReplaceableTxFunder(nodeParams: NodeParams,
   private def sign(fundedTx: ReplaceableTxWithWitnessData, txFeerate: FeeratePerKw, amountIn: Satoshi): Behavior[Command] = {
     val channelKeyPath = keyManager.keyPath(cmd.commitment.localParams, cmd.commitment.params.channelConfig)
     fundedTx match {
-      case claimTx: ClaimLocalAnchorWithWitnessData =>
-        val localSig = keyManager.sign(claimTx.txInfo, keyManager.fundingPublicKey(cmd.commitment.localParams.fundingKeyPath), TxOwner.Local, cmd.commitment.params.commitmentFormat)
-        val signedTx = claimTx.copy(txInfo = addSigs(claimTx.txInfo, localSig))
+      case claimAnchorTx: ClaimLocalAnchorWithWitnessData =>
+        val localSig = keyManager.sign(claimAnchorTx.txInfo, keyManager.fundingPublicKey(cmd.commitment.localParams.fundingKeyPath), TxOwner.Local, cmd.commitment.params.commitmentFormat)
+        val signedTx = claimAnchorTx.copy(txInfo = addSigs(claimAnchorTx.txInfo, localSig))
         signWalletInputs(signedTx, txFeerate, amountIn)
       case htlcTx: HtlcWithWitnessData =>
         val localPerCommitmentPoint = keyManager.commitmentPoint(channelKeyPath, cmd.commitment.localCommit.index)
@@ -322,6 +322,7 @@ private class ReplaceableTxFunder(nodeParams: NodeParams,
   private def signWalletInputs(locallySignedTx: ReplaceableTxWithWalletInputs, txFeerate: FeeratePerKw, amountIn: Satoshi): Behavior[Command] = {
     import fr.acinq.bitcoin.scalacompat.KotlinUtils._
 
+    // we finalize (sign) the input that we control, and will then ask our bitcoin client to sign wallet inputs
     val psbt = new Psbt(locallySignedTx.txInfo.tx)
     val updated = psbt.updateWitnessInput(locallySignedTx.txInfo.input.outPoint, locallySignedTx.txInfo.input.txOut, null, fr.acinq.bitcoin.Script.parse(locallySignedTx.txInfo.input.redeemScript), null, java.util.Map.of())
     val finalized = EitherKt.flatMap(updated, (psbt: Psbt) => psbt.finalizeWitnessInput(0, locallySignedTx.txInfo.tx.txIn.head.witness))
@@ -400,7 +401,7 @@ private class ReplaceableTxFunder(nodeParams: NodeParams,
             val packageWeight = commitTx.weight() + dummySignedTx.tx.weight()
 
             // above, we asked bitcoin core to use the package weight to estimate fees when it built and funded this transaction, so we
-            // use the same package weight here to compute thet actual fee rate that we get
+            // use the same package weight here to compute the actual fee rate that we get
             val actualFeerate = FeeratePerKw((fundTxResponse.fee * 1000) / packageWeight)
             require(actualFeerate < targetFeerate * 2, s"actual fee rate $actualFeerate is more than twice the requested fee rate $targetFeerate")
 

eclair-core/src/main/scala/fr/acinq/eclair/crypto/keymanager/LocalOnchainKeyManager.scala

@@ -3,7 +3,7 @@ package fr.acinq.eclair.crypto.keymanager
 import fr.acinq.bitcoin.ScriptWitness
 import fr.acinq.bitcoin.psbt.{Psbt, SignPsbtResult}
 import fr.acinq.bitcoin.scalacompat.DeterministicWallet._
-import fr.acinq.bitcoin.scalacompat.{Block, ByteVector32, DeterministicWallet, MnemonicCode, Satoshi, Script}
+import fr.acinq.bitcoin.scalacompat.{Block, ByteVector32, DeterministicWallet, MnemonicCode}
 import fr.acinq.bitcoin.utils.EitherKt
 import grizzled.slf4j.Logging
 import scodec.bits.ByteVector
@@ -18,7 +18,11 @@ class LocalOnchainKeyManager(entropy: ByteVector, chainHash: ByteVector32, passp
 
   import LocalOnchainKeyManager._
 
-  // master key used for HWI wallets
+  // master key. we will use it to generate a BIP84 wallet that can be used:
+  // - to generate a watch-only wallet with any BIP84-compatible bitcoin wallet
+  // - to generate descriptors that can be used by Bitcoin Core through HWI to create a wallet with the `external signer`
+  // option set, the external signer being this onchain key manager accessed through Eclair's API
+
   // m / purpose' / coin_type' / account' / change / address_index
   private val mnemonics = MnemonicCode.toMnemonics(entropy)
   private val seed = MnemonicCode.toSeed(mnemonics, passphrase)
@@ -105,9 +109,14 @@ class LocalOnchainKeyManager(entropy: ByteVector, chainHash: ByteVector32, passp
     // - the actual transaction that was spent, in the PSBT's non-witness utxo field
     // we check that this fields are consistent and match the outpoint that is spent in the PSBT
     // this prevents attacks where bitcoin core would lie about the amount being spent and make us pay very high fees
+    require(input.getNonWitnessUtxo != null, "non-witness utxo is missing")
     require(input.getNonWitnessUtxo.txid == psbt.getGlobal.getTx.txIn.get(pos).outPoint.txid, "utxo txid mismatch")
     require(input.getNonWitnessUtxo.txOut.get(psbt.getGlobal.getTx.txIn.get(pos).outPoint.index.toInt) == input.getWitnessUtxo, "utxo mismatch")
 
+    // not using SIGHASH_ALL would make us vulnerable to "signature reuse" attacks
+    // here null means unspecified means SIGHASH_ALL
+    require(Option(input.getSighashType).forall(_ == SigHash.SIGHASH_ALL), "input sighashtype must be SIGHASH_ALL")
+
     // check that we're signing a p2wpkh input and that the keypath is provided and correct
     require(Script.isPay2wpkh(input.getWitnessUtxo.publicKeyScript.toByteArray), "spent input is not p2wpkh")
     require(input.getDerivationPaths.size() == 1, "invalid bip32 path")
@@ -117,11 +126,15 @@ class LocalOnchainKeyManager(entropy: ByteVector, chainHash: ByteVector32, passp
     val priv = fr.acinq.bitcoin.DeterministicWallet.derivePrivateKey(master.priv, keypath.getKeyPath).getPrivateKey
     require(priv.publicKey() == pub, "cannot compute private key")
 
-    // update the input which the right script for a p2wpkh input, which is a * p2pkh * script
-    // then sign and finalized the psbt input
+    // update the input with the right script for a p2wpkh input, which is a * p2pkh * script
+    // then sign and finalize the psbt input
     val updated = psbt.updateWitnessInput(psbt.getGlobal.getTx.txIn.get(pos).outPoint, input.getWitnessUtxo, null, Script.pay2pkh(pub), SigHash.SIGHASH_ALL, input.getDerivationPaths)
     val signed = EitherKt.flatMap(updated, (p: Psbt) => p.sign(priv, pos))
-    val finalized = EitherKt.flatMap(signed, (s: SignPsbtResult) => s.getPsbt.finalizeWitnessInput(pos, new ScriptWitness().push(s.getSig).push(pub.value)))
+    val finalized = EitherKt.flatMap(signed, (s: SignPsbtResult) => {
+      val sig = s.getSig
+      require(sig.get(sig.size() - 1).toInt == SigHash.SIGHASH_ALL, "signature must end with SIGHASH_ALL")
+      s.getPsbt.finalizeWitnessInput(pos, new ScriptWitness().push(sig).push(pub.value))
+    })
     require(finalized.isRight, s"cannot sign psbt input, error = ${finalized.getLeft}")
     finalized.getRight
   }

eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala

@@ -31,7 +31,7 @@ import fr.acinq.eclair.blockchain.bitcoind.BitcoindService.BitcoinReq
 import fr.acinq.eclair.blockchain.bitcoind.rpc.BitcoinCoreClient._
 import fr.acinq.eclair.blockchain.bitcoind.rpc.BitcoinJsonRPCAuthMethod.UserPassword
 import fr.acinq.eclair.blockchain.bitcoind.rpc.{BasicBitcoinJsonRPCClient, BitcoinCoreClient, JsonRPCError}
-import fr.acinq.eclair.blockchain.fee.{BitcoinCoreFeeProvider, FeeratePerByte, FeeratePerKB, FeeratePerKw}
+import fr.acinq.eclair.blockchain.fee.{FeeratePerByte, FeeratePerKw}
 import fr.acinq.eclair.crypto.keymanager.LocalOnchainKeyManager
 import fr.acinq.eclair.transactions.{Scripts, Transactions}
 import fr.acinq.eclair.{BlockHeight, TestConstants, TestKitBaseClass, addressFromPublicKeyScript, addressToPublicKeyScript, randomBytes32, randomKey}
@@ -64,7 +64,7 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A
     assume(!useExternalSigner)
 
     val sender = TestProbe()
-    val bitcoinClient = new BitcoinCoreClient(bitcoinrpcclient)
+    val bitcoinClient = makeBitcoinCoreClient
     val walletPassword = Random.alphanumeric.take(8).mkString
     sender.send(bitcoincli, BitcoinReq("encryptwallet", walletPassword))
     sender.expectMsgType[JString](60 seconds)

eclair-core/src/test/scala/fr/acinq/eclair/crypto/keymanager/LocalOnchainKeyManagerSpec.scala

@@ -1,8 +1,8 @@
 package fr.acinq.eclair.crypto.keymanager
 
-import fr.acinq.bitcoin.ScriptFlags
 import fr.acinq.bitcoin.psbt.{KeyPathWithMaster, Psbt}
 import fr.acinq.bitcoin.scalacompat.{Block, DeterministicWallet, OutPoint, Satoshi, Script, Transaction, TxIn, TxOut}
+import fr.acinq.bitcoin.{ScriptFlags, SigHash}
 import org.scalatest.funsuite.AnyFunSuite
 import scodec.bits.ByteVector
 
@@ -99,6 +99,30 @@ class LocalOnchainKeyManagerSpec extends AnyFunSuite {
       }
       assert(error.getMessage.contains("utxo mismatch"))
     }
+    {
+      // do not provide non-witness utxo for utxo #2
+      val psbt: Psbt = {
+        val p0 = new Psbt(tx).updateWitnessInput(OutPoint(utxos(0), 0), utxos(0).txOut(0), null, Script.pay2pkh(getPublicKey(0)).map(scala2kmp).asJava, null, java.util.Map.of(getPublicKey(0), bip32paths(0)))
+        val p1 = EitherKt.flatMap(p0, (psbt: Psbt) => psbt.updateWitnessInput(OutPoint(utxos(1), 0), utxos(1).txOut(0), null, Script.pay2pkh(getPublicKey(1)).map(scala2kmp).asJava, null, java.util.Map.of(getPublicKey(1), bip32paths(1))))
+        val p2 = EitherKt.flatMap(p1, (psbt: Psbt) => psbt.updateWitnessInput(OutPoint(utxos(2), 0), utxos(2).txOut(0), null, Script.pay2pkh(getPublicKey(2)).map(scala2kmp).asJava, null, java.util.Map.of(getPublicKey(2), bip32paths(2))))
+        val p3 = EitherKt.flatMap(p2, (psbt: Psbt) => psbt.updateNonWitnessInput(utxos(0), 0, null, null, java.util.Map.of()))
+        val p4 = EitherKt.flatMap(p3, (psbt: Psbt) => psbt.updateNonWitnessInput(utxos(1), 0, null, null, java.util.Map.of()))
+        val p5 = EitherKt.flatMap(p4, (psbt: Psbt) => psbt.updateWitnessOutput(0, null, null, java.util.Map.of(getPublicKey(0), bip32paths(0))))
+        p5.getRight
+      }
+      val error = intercept[IllegalArgumentException] {
+        onchainKeyManager.signPsbt(psbt, Seq(0, 1, 2), Seq(0))
+      }
+      assert(error.getMessage.contains("non-witness utxo is missing"))
+    }
+    {
+      // use sighash type != SIGHASH_ALL
+      val updated = psbt.updateWitnessInput(OutPoint(utxos(0), 0), utxos(0).txOut(0), null, Script.pay2pkh(getPublicKey(0)).map(scala2kmp).asJava, SigHash.SIGHASH_SINGLE, java.util.Map.of(getPublicKey(0), bip32paths(0))).getRight
+      val error = intercept[IllegalArgumentException] {
+        onchainKeyManager.signPsbt(updated, Seq(0, 1, 2), Seq(0))
+      }
+      assert(error.getMessage.contains("input sighashtype must be SIGHASH_ALL"))
+    }
   }
 
   test("compute descriptor checksums") {

eclair-node/src/main/scala/fr/acinq/eclair/api/handlers/OnChain.scala

@@ -63,7 +63,7 @@ trait OnChain {
       "model" -> JString("eclair"),
       "label" -> JString(""),
       "path" -> JString(""),
-      "fingerprint" -> JString(this.eclairApi.getOnchainMasterMasterFingerprintHex),
+      "fingerprint" -> JString(this.eclairApi.getOnchainMasterFingerprintHex),
       "needs_pin_sent" -> JBool(false),
       "needs_passphrase_sent" -> JBool(false)
     ))