Produce exclusively low-R signatures from wallet keys

core/src/main/java/bisq/core/account/sign/SignedWitnessService.java

@@ -18,6 +18,7 @@
 package bisq.core.account.sign;
 
 import bisq.core.account.witness.AccountAgeWitness;
+import bisq.core.crypto.LowRSigningKey;
 import bisq.core.filter.FilterManager;
 import bisq.core.support.dispute.arbitration.arbitrator.ArbitratorManager;
 import bisq.core.user.User;
@@ -280,7 +281,7 @@ private String signAndPublishAccountAgeWitness(Coin tradeAmount,
         }
 
         String accountAgeWitnessHashAsHex = Utilities.encodeToHex(accountAgeWitness.getHash());
-        String signatureBase64 = key.signMessage(accountAgeWitnessHashAsHex);
+        String signatureBase64 = LowRSigningKey.from(key).signMessage(accountAgeWitnessHashAsHex);
         SignedWitness signedWitness = new SignedWitness(SignedWitness.VerificationMethod.ARBITRATOR,
                 accountAgeWitness.getHash(),
                 signatureBase64.getBytes(Charsets.UTF_8),
@@ -289,7 +290,7 @@ private String signAndPublishAccountAgeWitness(Coin tradeAmount,
                 time,
                 tradeAmount.value);
         publishSignedWitness(signedWitness);
-        log.info("Arbitrator signed witness {}", signedWitness.toString());
+        log.info("Arbitrator signed witness {}", signedWitness);
         return "";
     }
 
@@ -322,7 +323,7 @@ public Optional<SignedWitness> signAndPublishAccountAgeWitness(Coin tradeAmount,
                 new Date().getTime(),
                 tradeAmount.value);
         publishSignedWitness(signedWitness);
-        log.info("Trader signed witness {}", signedWitness.toString());
+        log.info("Trader signed witness {}", signedWitness);
         return Optional.of(signedWitness);
     }
 
@@ -537,7 +538,7 @@ public void addToMap(SignedWitness signedWitness) {
 
     private void publishSignedWitness(SignedWitness signedWitness) {
         if (!signedWitnessMap.containsKey(signedWitness.getHashAsByteArray())) {
-            log.info("broadcast signed witness {}", signedWitness.toString());
+            log.info("broadcast signed witness {}", signedWitness);
             // We set reBroadcast to true to achieve better resilience.
             p2PService.addPersistableNetworkPayload(signedWitness, true);
             addToMap(signedWitness);

core/src/main/java/bisq/core/alert/AlertManager.java

@@ -17,6 +17,7 @@
 
 package bisq.core.alert;
 
+import bisq.core.crypto.LowRSigningKey;
 import bisq.core.user.User;
 
 import bisq.network.p2p.P2PService;
@@ -160,7 +161,7 @@ private boolean isKeyValid(String privKeyString) {
 
     private void signAndAddSignatureToAlertMessage(Alert alert) {
         String alertMessageAsHex = Utils.HEX.encode(alert.getMessage().getBytes(Charsets.UTF_8));
-        String signatureAsBase64 = alertSigningKey.signMessage(alertMessageAsHex);
+        String signatureAsBase64 = LowRSigningKey.from(alertSigningKey).signMessage(alertMessageAsHex);
         alert.setSigAndPubKey(signatureAsBase64, keyRing.getSignatureKeyPair().getPublic());
     }
 

core/src/main/java/bisq/core/alert/PrivateNotificationManager.java

@@ -17,6 +17,8 @@
 
 package bisq.core.alert;
 
+import bisq.core.crypto.LowRSigningKey;
+
 import bisq.network.p2p.DecryptedMessageWithPubKey;
 import bisq.network.p2p.NodeAddress;
 import bisq.network.p2p.P2PService;
@@ -175,7 +177,7 @@ private boolean isKeyValid(String privKeyString) {
 
     private void signAndAddSignatureToPrivateNotificationMessage(PrivateNotificationPayload privateNotification) {
         String privateNotificationMessageAsHex = Utils.HEX.encode(privateNotification.getMessage().getBytes(Charsets.UTF_8));
-        String signatureAsBase64 = privateNotificationSigningKey.signMessage(privateNotificationMessageAsHex);
+        String signatureAsBase64 = LowRSigningKey.from(privateNotificationSigningKey).signMessage(privateNotificationMessageAsHex);
         privateNotification.setSigAndPubKey(signatureAsBase64, keyRing.getSignatureKeyPair().getPublic());
     }
 

core/src/main/java/bisq/core/btc/setup/WalletConfig.java

@@ -20,6 +20,7 @@
 import bisq.core.btc.nodes.LocalBitcoinNode;
 import bisq.core.btc.nodes.ProxySocketFactory;
 import bisq.core.btc.wallet.BisqRiskAnalysis;
+import bisq.core.btc.wallet.BisqWallet;
 
 import bisq.common.config.Config;
 import bisq.common.file.FileUtil;
@@ -405,7 +406,7 @@ private Wallet loadWallet(boolean shouldReplayWallet, File walletFile, boolean i
             WalletExtension[] extArray = new WalletExtension[]{};
             Protos.Wallet proto = WalletProtobufSerializer.parseToProto(walletStream);
             final WalletProtobufSerializer serializer;
-            serializer = new WalletProtobufSerializer();
+            serializer = new WalletProtobufSerializer(BisqWallet::new);
             // Hack to convert bitcoinj 0.14 wallets to bitcoinj 0.15 format
             serializer.setKeyChainFactory(new BisqKeyChainFactory(isBsqWallet));
             wallet = serializer.readWallet(params, extArray, proto);
@@ -432,7 +433,7 @@ protected Wallet createWallet(boolean isBsqWallet) {
                 kcgBuilder.fromSeed(vBtcWallet.getKeyChainSeed(), preferredOutputScriptType);
             }
         }
-        return new Wallet(params, kcgBuilder.build());
+        return new BisqWallet(params, kcgBuilder.build());
     }
 
     private void maybeMoveOldWalletOutOfTheWay(File walletFile) {

core/src/main/java/bisq/core/btc/wallet/BisqWallet.java

@@ -0,0 +1,105 @@
+/*
+ * This file is part of Bisq.
+ *
+ * Bisq is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or (at
+ * your option) any later version.
+ *
+ * Bisq is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with Bisq. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package bisq.core.btc.wallet;
+
+import org.bitcoinj.core.NetworkParameters;
+import org.bitcoinj.core.Transaction;
+import org.bitcoinj.core.TransactionInput;
+import org.bitcoinj.core.TransactionOutput;
+import org.bitcoinj.script.Script;
+import org.bitcoinj.script.ScriptException;
+import org.bitcoinj.signers.MissingSigResolutionSigner;
+import org.bitcoinj.signers.TransactionSigner;
+import org.bitcoinj.wallet.DecryptingKeyBag;
+import org.bitcoinj.wallet.KeyBag;
+import org.bitcoinj.wallet.KeyChainGroup;
+import org.bitcoinj.wallet.RedeemData;
+import org.bitcoinj.wallet.SendRequest;
+import org.bitcoinj.wallet.Wallet;
+
+import java.util.List;
+
+import lombok.extern.slf4j.Slf4j;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+
+@Slf4j
+public class BisqWallet extends Wallet {
+    public BisqWallet(NetworkParameters params, KeyChainGroup keyChainGroup) {
+        super(params, keyChainGroup);
+    }
+
+    // Largely copied from super, but modified to supply instances of LowRSigningKey to the tx signers,
+    // instead of deterministic keys, so that low-R signatures always result when using the SendRequest
+    // API. This likely breaks the keychain marrying functionality of bitcoinj, used for multisig inputs,
+    // but we don't use that anywhere in Bisq.
+    @Override
+    public void signTransaction(SendRequest req) {
+        lock.lock();
+        try {
+            Transaction tx = req.tx;
+            List<TransactionInput> inputs = tx.getInputs();
+            List<TransactionOutput> outputs = tx.getOutputs();
+            checkState(inputs.size() > 0);
+            checkState(outputs.size() > 0);
+
+            KeyBag maybeDecryptingKeyBag = new LowRSigningKeyBag(new DecryptingKeyBag(this, req.aesKey));
+
+            int numInputs = tx.getInputs().size();
+            for (int i = 0; i < numInputs; i++) {
+                TransactionInput txIn = tx.getInput(i);
+                TransactionOutput connectedOutput = txIn.getConnectedOutput();
+                if (connectedOutput == null) {
+                    // Missing connected output, assuming already signed.
+                    continue;
+                }
+                Script scriptPubKey = connectedOutput.getScriptPubKey();
+
+                try {
+                    // We assume if its already signed, its hopefully got a SIGHASH type that will not invalidate when
+                    // we sign missing pieces (to check this would require either assuming any signatures are signing
+                    // standard output types or a way to get processed signatures out of script execution)
+                    txIn.getScriptSig().correctlySpends(tx, i, txIn.getWitness(), connectedOutput.getValue(),
+                            connectedOutput.getScriptPubKey(), Script.ALL_VERIFY_FLAGS);
+                    log.warn("Input {} already correctly spends output, assuming SIGHASH type used will be safe and skipping signing.", i);
+                    continue;
+                } catch (ScriptException e) {
+                    log.debug("Input contained an incorrect signature", e);
+                    // Expected.
+                }
+
+                RedeemData redeemData = txIn.getConnectedRedeemData(maybeDecryptingKeyBag);
+                checkNotNull(redeemData, "Transaction exists in wallet that we cannot redeem: %s", txIn.getOutpoint().getHash());
+                txIn.setScriptSig(scriptPubKey.createEmptyInputScript(redeemData.keys.get(0), redeemData.redeemScript));
+                txIn.setWitness(scriptPubKey.createEmptyWitness(redeemData.keys.get(0)));
+            }
+
+            TransactionSigner.ProposedTransaction proposal = new TransactionSigner.ProposedTransaction(tx);
+            for (TransactionSigner signer : getTransactionSigners()) {
+                if (!signer.signInputs(proposal, maybeDecryptingKeyBag))
+                    log.info("{} returned false for the tx", signer.getClass().getName());
+            }
+
+            // resolve missing sigs if any
+            new MissingSigResolutionSigner(req.missingSigsMode).signInputs(proposal, maybeDecryptingKeyBag);
+        } finally {
+            lock.unlock();
+        }
+    }
+}

core/src/main/java/bisq/core/btc/wallet/LowRSigningKeyBag.java

@@ -0,0 +1,61 @@
+/*
+ * This file is part of Bisq.
+ *
+ * Bisq is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or (at
+ * your option) any later version.
+ *
+ * Bisq is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with Bisq. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package bisq.core.btc.wallet;
+
+import bisq.core.crypto.LowRSigningKey;
+
+import org.bitcoinj.core.ECKey;
+import org.bitcoinj.script.Script;
+import org.bitcoinj.wallet.KeyBag;
+import org.bitcoinj.wallet.RedeemData;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+import javax.annotation.Nullable;
+
+public class LowRSigningKeyBag implements KeyBag {
+    private final KeyBag target;
+
+    public LowRSigningKeyBag(KeyBag target) {
+        this.target = target;
+    }
+
+    @Nullable
+    @Override
+    public ECKey findKeyFromPubKeyHash(byte[] pubKeyHash, @Nullable Script.ScriptType scriptType) {
+        return LowRSigningKey.from(target.findKeyFromPubKeyHash(pubKeyHash, scriptType));
+    }
+
+    @Nullable
+    @Override
+    public ECKey findKeyFromPubKey(byte[] pubKey) {
+        return LowRSigningKey.from(target.findKeyFromPubKey(pubKey));
+    }
+
+    @Nullable
+    @Override
+    public RedeemData findRedeemDataFromScriptHash(byte[] scriptHash) {
+        RedeemData redeemData = target.findRedeemDataFromScriptHash(scriptHash);
+        if (redeemData == null) {
+            return null;
+        }
+        List<ECKey> lowRKeys = redeemData.keys.stream().map(LowRSigningKey::from).collect(Collectors.toList());
+        return RedeemData.of(lowRKeys, redeemData.redeemScript);
+    }
+}

core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java

@@ -26,6 +26,7 @@
 import bisq.core.btc.model.RawTransactionInput;
 import bisq.core.btc.setup.WalletConfig;
 import bisq.core.btc.setup.WalletsSetup;
+import bisq.core.crypto.LowRSigningKey;
 import bisq.core.locale.Res;
 import bisq.core.user.Preferences;
 
@@ -744,7 +745,7 @@ public byte[] signDelayedPayoutTx(Transaction delayedPayoutTx,
             checkNotNull(aesKey);
         }
 
-        ECKey.ECDSASignature mySignature = myMultiSigKeyPair.sign(sigHash, aesKey).toCanonicalised();
+        ECKey.ECDSASignature mySignature = LowRSigningKey.from(myMultiSigKeyPair).sign(sigHash, aesKey);
         WalletService.printTx("delayedPayoutTx for sig creation", delayedPayoutTx);
         WalletService.verifyTransaction(delayedPayoutTx);
         return mySignature.encodeToDER();
@@ -841,7 +842,7 @@ public byte[] buyerSignsPayoutTx(Transaction depositTx,
         if (multiSigKeyPair.isEncrypted()) {
             checkNotNull(aesKey);
         }
-        ECKey.ECDSASignature buyerSignature = multiSigKeyPair.sign(sigHash, aesKey).toCanonicalised();
+        ECKey.ECDSASignature buyerSignature = LowRSigningKey.from(multiSigKeyPair).sign(sigHash, aesKey);
         WalletService.printTx("prepared payoutTx", preparedPayoutTx);
         WalletService.verifyTransaction(preparedPayoutTx);
         return buyerSignature.encodeToDER();
@@ -893,7 +894,7 @@ public Transaction sellerSignsAndFinalizesPayoutTx(Transaction depositTx,
         if (multiSigKeyPair.isEncrypted()) {
             checkNotNull(aesKey);
         }
-        ECKey.ECDSASignature sellerSignature = multiSigKeyPair.sign(sigHash, aesKey).toCanonicalised();
+        ECKey.ECDSASignature sellerSignature = LowRSigningKey.from(multiSigKeyPair).sign(sigHash, aesKey);
         TransactionSignature buyerTxSig = new TransactionSignature(ECKey.ECDSASignature.decodeFromDER(buyerSignature),
                 Transaction.SigHash.ALL, false);
         TransactionSignature sellerTxSig = new TransactionSignature(sellerSignature, Transaction.SigHash.ALL, false);
@@ -948,7 +949,7 @@ public byte[] signMediatedPayoutTx(Transaction depositTx,
         if (myMultiSigKeyPair.isEncrypted()) {
             checkNotNull(aesKey);
         }
-        ECKey.ECDSASignature mySignature = myMultiSigKeyPair.sign(sigHash, aesKey).toCanonicalised();
+        ECKey.ECDSASignature mySignature = LowRSigningKey.from(myMultiSigKeyPair).sign(sigHash, aesKey);
         WalletService.printTx("prepared mediated payoutTx for sig creation", preparedPayoutTx);
         WalletService.verifyTransaction(preparedPayoutTx);
         return mySignature.encodeToDER();
@@ -1058,7 +1059,7 @@ public Transaction traderSignAndFinalizeDisputedPayoutTx(byte[] depositTxSeriali
         if (tradersMultiSigKeyPair.isEncrypted()) {
             checkNotNull(aesKey);
         }
-        ECKey.ECDSASignature tradersSignature = tradersMultiSigKeyPair.sign(sigHash, aesKey).toCanonicalised();
+        ECKey.ECDSASignature tradersSignature = LowRSigningKey.from(tradersMultiSigKeyPair).sign(sigHash, aesKey);
         TransactionSignature tradersTxSig = new TransactionSignature(tradersSignature, Transaction.SigHash.ALL, false);
         TransactionSignature arbitratorTxSig = new TransactionSignature(ECKey.ECDSASignature.decodeFromDER(arbitratorSignature),
                 Transaction.SigHash.ALL, false);
@@ -1137,7 +1138,7 @@ public String emergencyGenerateSignature(String rawTxHex,
 
         ECKey myPrivateKey = ECKey.fromPrivate(Utils.HEX.decode(myPrivKeyAsHex));
         checkNotNull(myPrivateKey, "key must not be null");
-        ECKey.ECDSASignature myECDSASignature = myPrivateKey.sign(sigHash, aesKey).toCanonicalised();
+        ECKey.ECDSASignature myECDSASignature = LowRSigningKey.from(myPrivateKey).sign(sigHash, aesKey);
         TransactionSignature myTxSig = new TransactionSignature(myECDSASignature, Transaction.SigHash.ALL, false);
         return Utils.HEX.encode(myTxSig.encodeToBitcoin());
     }
@@ -1409,9 +1410,8 @@ private Transaction createPayoutTx(Transaction depositTx,
     private void signInput(Transaction transaction, TransactionInput input, int inputIndex) throws SigningException {
         checkNotNull(input.getConnectedOutput(), "input.getConnectedOutput() must not be null");
         Script scriptPubKey = input.getConnectedOutput().getScriptPubKey();
-        ECKey sigKey = input.getOutpoint().getConnectedKey(wallet);
-        checkNotNull(sigKey, "signInput: sigKey must not be null. input.getOutpoint()=" +
-                input.getOutpoint().toString());
+        ECKey sigKey = LowRSigningKey.from(input.getOutpoint().getConnectedKey(wallet));
+        checkNotNull(sigKey, "signInput: sigKey must not be null. input.getOutpoint()=%s", input.getOutpoint());
         if (sigKey.isEncrypted()) {
             checkNotNull(aesKey);
         }

core/src/main/java/bisq/core/btc/wallet/WalletService.java

@@ -24,6 +24,7 @@
 import bisq.core.btc.listeners.TxConfidenceListener;
 import bisq.core.btc.setup.WalletsSetup;
 import bisq.core.btc.wallet.http.MemPoolSpaceTxBroadcaster;
+import bisq.core.crypto.LowRSigningKey;
 import bisq.core.provider.fee.FeeService;
 import bisq.core.user.Preferences;
 
@@ -359,8 +360,8 @@ public static void signTransactionInput(Wallet wallet,
                 if (pubKey instanceof DeterministicKey)
                     propTx.keyPaths.put(scriptPubKey, (((DeterministicKey) pubKey).getPath()));
 
-                ECKey key;
-                if ((key = redeemData.getFullKey()) == null) {
+                ECKey key = LowRSigningKey.from(redeemData.getFullKey());
+                if (key == null) {
                     log.warn("No local key found for input {}", index);
                     return;
                 }