diff --git a/src/main/java/com/sparrowwallet/drongo/psbt/PSBTInput.java b/src/main/java/com/sparrowwallet/drongo/psbt/PSBTInput.java index 9d0bce36..dacd4780 100644 --- a/src/main/java/com/sparrowwallet/drongo/psbt/PSBTInput.java +++ b/src/main/java/com/sparrowwallet/drongo/psbt/PSBTInput.java @@ -10,6 +10,8 @@ import java.io.ByteArrayOutputStream; import java.nio.charset.StandardCharsets; +import org.bouncycastle.math.ec.ECPoint; + import java.util.*; import java.util.stream.Collectors; @@ -38,6 +40,7 @@ public class PSBTInput { public static final byte PSBT_IN_REQUIRED_TIME_LOCKTIME = 0x11; public static final byte PSBT_IN_REQUIRED_HEIGHT_LOCKTIME = 0x12; public static final byte PSBT_IN_TAP_KEY_SIG = 0x13; + public static final byte PSBT_IN_TAP_SCRIPT_SIG = 0x14; public static final byte PSBT_IN_TAP_BIP32_DERIVATION = 0x16; public static final byte PSBT_IN_TAP_INTERNAL_KEY = 0x17; public static final byte PSBT_IN_SP_ECDH_SHARE = 0x1d; @@ -62,6 +65,14 @@ public class PSBTInput { private byte[] hash160Preimage; private byte[] hash256Preimage; private final Map proprietary = new LinkedHashMap<>(); + /** + * Taproot script path signatures, keyed by the x only public key and leaf hash they were made for. + * + * Not verifiable here, since PSBT_IN_TAP_LEAF_SCRIPT is not parsed: what they give a caller is that the input + * has been signed, which was otherwise unreadable. Held as hex like proprietary entries so a combiner hands back + * what it was given: re-encoded, a 65 byte signature whose hash type byte is zero comes back 64 bytes long. + */ + private final Map tapScriptSignatures = new LinkedHashMap<>(); private TransactionSignature tapKeyPathSignature; private Map>> tapDerivedPublicKeys = new LinkedHashMap<>(); private ECKey tapInternalKey; @@ -367,6 +378,18 @@ public class PSBTInput { this.tapKeyPathSignature = TransactionSignature.decodeFromBitcoin(SCHNORR, entry.getData(), true); log.debug("Found input taproot key path signature " + Utils.bytesToHex(entry.getData())); break; + case PSBT_IN_TAP_SCRIPT_SIG: + //The key data, not the key length: a key type may use a longer compact size integer than it needs, and this + //parser dispatches on the low byte, so the padding lands in the key length and hides short key data + if(entry.getKeyData() == null || entry.getKeyData().length != 64) { + throw new PSBTParseException("PSBT key type must be one byte plus x only pub key plus leaf hash"); + } + if(entry.getData().length != 64 && entry.getData().length != 65) { + throw new PSBTParseException("PSBT taproot script path signature must be 64 or 65 bytes"); + } + this.tapScriptSignatures.put(Utils.bytesToHex(entry.getKeyData()), Utils.bytesToHex(entry.getData())); + log.debug("Found input taproot script path signature " + Utils.bytesToHex(entry.getData())); + break; case PSBT_IN_TAP_BIP32_DERIVATION: entry.checkOneBytePlusXOnlyPubKey(); ECKey tapPublicKey = ECKey.fromPublicOnly(entry.getKeyData()); @@ -549,6 +572,10 @@ public List getInputEntries(int psbtVersion) { entries.add(populateEntry(PSBT_IN_TAP_KEY_SIG, null, tapKeyPathSignature.encodeToBitcoin())); } + for(Map.Entry entry : tapScriptSignatures.entrySet()) { + entries.add(populateEntry(PSBT_IN_TAP_SCRIPT_SIG, Utils.hexToBytes(entry.getKey()), Utils.hexToBytes(entry.getValue()))); + } + for(Map.Entry>> entry : tapDerivedPublicKeys.entrySet()) { if(!entry.getValue().isEmpty()) { entries.add(populateEntry(PSBT_IN_TAP_BIP32_DERIVATION, entry.getKey().getPubKeyXCoord(), serializeTaprootKeyDerivation(Collections.emptyList(), entry.getValue().keySet().iterator().next()))); @@ -677,6 +704,8 @@ void combine(PSBTInput psbtInput) { tapKeyPathSignature = psbtInput.tapKeyPathSignature; } + tapScriptSignatures.putAll(psbtInput.tapScriptSignatures); + tapDerivedPublicKeys.putAll(psbtInput.tapDerivedPublicKeys); if(psbtInput.tapInternalKey != null) { @@ -684,6 +713,10 @@ void combine(PSBTInput psbtInput) { } } + public Map getTapScriptSignatures() { + return Collections.unmodifiableMap(tapScriptSignatures); + } + public Transaction getNonWitnessUtxo() { return nonWitnessUtxo; } @@ -951,6 +984,110 @@ public Collection getSignatures() { } } + /** + * The point a key stands for, or null where those bytes are not one. ECKey.fromPublicOnly defers the decode, so a + * key that is not on the curve is only found out here. + */ + private static ECPoint pointOf(ECKey key) { + try { + return key.getPubKeyPoint().normalize(); + } catch(RuntimeException e) { + return null; + } + } + + /** + * Whether this input's signatures still name the keys that made them, which is every input before it is finalised. + * + * A partial signature names its key, so there is no product to do: verify it against that key alone, once it is + * one the caller vouches for. The guarantee is unchanged, because the membership test is what the caller trusts + * and the file cannot forge it. An 11 of 15 input costs 11 checks instead of 165, which is the difference between + * checking a large multisig consolidation and giving up on it. A finalised input carries pushes with no names. + */ + public boolean namesItsKeys() { + return getFinalScriptWitness() == null && getFinalScriptSig() == null && getTapKeyPathSignature() == null; + } + + /** + * As getVerifiedSignatures, keeping the key each signature verified under. + * + * A signature does not carry the key that made it, and TransactionSignature compares by hash type and by r and s, + * so a caller pairing the two by value pairs a signature with any key that files a copy of it. Only the pair is + * the fact, and a caller choosing which signature goes in which slot needs the pair rather than the signature. + */ + public Map getVerifiedPartialSignatures(Collection trustedKeys) { + if(trustedKeys == null || trustedKeys.isEmpty() || getUtxo() == null || !namesItsKeys()) { + return Collections.emptyMap(); + } + + Script signingScript; + try { + signingScript = getSigningScript(); + } catch(RuntimeException e) { + return Collections.emptyMap(); + } + + return signingScript == null ? Collections.emptyMap() : verifiedPartialSignatures(signingScript, trustedKeys); + } + + /** As above, for an input whose signatures still name the keys that made them. */ + private Map verifiedPartialSignatures(Script signingScript, Collection trustedKeys) { + Map partialSignatures = getPartialSignatures(); + if(partialSignatures.size() > MAX_SIGNATURE_CHECKS) { + return Collections.emptyMap(); + } + + //By the point, not by the key. ECKey.equals compares the private part too, so a key the caller vouches for + //publicly never matches the same key carrying a private one, and a swept key stopped being counted. The point + //is also what makes the two encodings of one key the same key. + Set trusted = new HashSet<>(); + for(ECKey trustedKey : trustedKeys) { + ECPoint point = pointOf(trustedKey); + if(point != null) { + trusted.add(point); + } + } + + Map verified = new LinkedHashMap<>(); + Map sigHashes = new HashMap<>(); + + for(Map.Entry entry : partialSignatures.entrySet()) { + //The key is the PSBT's, so reading its point is reading attacker supplied bytes: a 33 byte value that is + //not on the curve parses without complaint and only fails here. One of those must cost this entry and not + //the whole input, and never the label. + ECPoint named = pointOf(entry.getKey()); + if(named == null || !trusted.contains(named)) { + continue; + } + + TransactionSignature signature = entry.getValue(); + if(!sigHashes.containsKey(signature.sighashFlags)) { + Sha256Hash computed = null; + try { + computed = getHashForSignature(signingScript, signature.sighashFlags); + } catch(RuntimeException e) { + //As above: a message that cannot be built is one that cannot be checked + } + sigHashes.put(signature.sighashFlags, computed); + } + + Sha256Hash hash = sigHashes.get(signature.sighashFlags); + if(hash == null) { + continue; + } + + try { + if(entry.getKey().verify(hash, signature)) { + verified.put(entry.getKey(), signature); + } + } catch(IllegalArgumentException e) { + //A key of the wrong kind for this signature verifies nothing, and says nothing about the others + } + } + + return verified; + } + /** * The most signature checks worth making for one input, comfortably past the twenty keys consensus will check in a * multisig and far short of what a hostile input can ask for. @@ -972,6 +1109,11 @@ public Collection getSignatures() { * arrange for any hash type they like. Checking against keys the caller already trusts, its own wallet's, answers * whether one of those keys signed, which is the question a claim about this transaction rests on. * + * An input still carrying partial signatures is answered from those, and there each signature is checked against + * the key naming it rather than against every key given: the name has to be one of them and the signature still + * has to verify under it. So one filed under a key that did not make it is not found, where reading a finalised + * input's pushes has no name to go on and checks them all. getVerifiedPartialSignatures keeps those pairs. + * * It answers with less than is present, never more, and it does not throw. An input with no spent output has no * message to build, a hash type that names no message cannot be checked, a script that cannot be read leaves * nothing to check against, a tapscript path names a key this cannot recover, and an input asking for more checks @@ -979,6 +1121,10 @@ public Collection getSignatures() { * reporting a protection has to treat what is missing as absent. */ public List getVerifiedSignatures(Collection trustedKeys) { + if(namesItsKeys()) { + return new ArrayList<>(getVerifiedPartialSignatures(trustedKeys).values()); + } + //The spent output is what the message is built over, and getSigningScript reads it, so its absence is answered //here rather than thrown from there if(trustedKeys == null || trustedKeys.isEmpty() || getUtxo() == null) { @@ -1201,6 +1347,21 @@ private void requireRequestedType(SigHash declared, byte sigHashType) throws PSB } } + /** + * Whether this key verifies this signature, taking a key that is not a point on the curve as a failure. + * + * Both keys here come out of the PSBT and are not checked when it is parsed, so decoding one throws + * IllegalArgumentException from a method that declares PSBTSignatureException. Callers catch what is declared, so + * it left the open flow uncaught on a file anyone could write. + */ + private static boolean verifies(ECKey key, Sha256Hash hash, TransactionSignature signature) { + try { + return key.verify(hash, signature); + } catch(IllegalArgumentException e) { + return false; + } + } + boolean verifySignatures() throws PSBTSignatureException { if(getNonWitnessUtxo() != null || getWitnessUtxo() != null) { Script signingScript = getSigningScript(); @@ -1218,7 +1379,7 @@ boolean verifySignatures() throws PSBTSignatureException { requireRequestedType(declared, tapKeyPathSignature.sighashFlags); Sha256Hash hash = sigHashes.computeIfAbsent(tapKeyPathSignature.sighashFlags, sigHashType -> getHashForSignature(signingScript, sigHashType)); requireDigest(hash, tapKeyPathSignature.sighashFlags); - if(!outputKey.verify(hash, tapKeyPathSignature)) { + if(!verifies(outputKey, hash, tapKeyPathSignature)) { throw new PSBTSignatureException("Tweaked internal key does not verify against provided taproot keypath signature"); } } else { @@ -1227,7 +1388,7 @@ boolean verifySignatures() throws PSBTSignatureException { requireRequestedType(declared, signature.sighashFlags); Sha256Hash hash = sigHashes.computeIfAbsent(signature.sighashFlags, sigHashType -> getHashForSignature(signingScript, sigHashType)); requireDigest(hash, signature.sighashFlags); - if(!sigPublicKey.verify(hash, signature)) { + if(!verifies(sigPublicKey, hash, signature)) { throw new PSBTSignatureException("Partial signature does not verify against provided public key"); } } @@ -1362,6 +1523,7 @@ public void clearNonFinalFields() { proprietary.clear(); tapDerivedPublicKeys.clear(); tapKeyPathSignature = null; + tapScriptSignatures.clear(); silentPaymentsEcdhShares.clear(); silentPaymentsDLEQProofs.clear(); silentPaymentsSpendDerivations.clear(); diff --git a/src/test/java/com/sparrowwallet/drongo/psbt/TapScriptSignatureTest.java b/src/test/java/com/sparrowwallet/drongo/psbt/TapScriptSignatureTest.java new file mode 100644 index 00000000..590e9d6e --- /dev/null +++ b/src/test/java/com/sparrowwallet/drongo/psbt/TapScriptSignatureTest.java @@ -0,0 +1,171 @@ +package com.sparrowwallet.drongo.psbt; + +import com.sparrowwallet.drongo.Utils; +import com.sparrowwallet.drongo.protocol.Script; +import com.sparrowwallet.drongo.protocol.Sha256Hash; +import com.sparrowwallet.drongo.protocol.SigHash; +import com.sparrowwallet.drongo.protocol.Transaction; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +/** + * A taproot script path signature is kept rather than dropped at the door. + * + * It was falling through to the unrecognized branch, which logs and discards. An input carrying only these then arrived + * with no partial signatures, no key path signature and nothing finalised, which is indistinguishable from an input + * nothing has signed. A wallet asking "has anything signed this" got no, and a label that reports what a transaction + * will be republished the file's own declaration as though it had been checked. + * + * They are not verified here. The leaf script that would check them travels in PSBT_IN_TAP_LEAF_SCRIPT, which this does + * not parse, so what they establish is presence and nothing more, which is exactly what the caller needs to stop + * reading an unsigned answer off a signed transaction. + */ +public class TapScriptSignatureTest { + private static final String X_ONLY_KEY = "aa".repeat(32); + private static final String LEAF_HASH = "bb".repeat(32); + + private byte[] signature(byte sigHashType) { + byte[] signature = new byte[65]; + Arrays.fill(signature, (byte)0x33); + signature[64] = sigHashType; + return signature; + } + + private PSBTEntry tapScriptEntry(byte[] signature) { + byte[] keyData = Utils.hexToBytes(X_ONLY_KEY + LEAF_HASH); + byte[] key = new byte[1 + keyData.length]; + key[0] = PSBTInput.PSBT_IN_TAP_SCRIPT_SIG; + System.arraycopy(keyData, 0, key, 1, keyData.length); + + return new PSBTEntry(key, PSBTInput.PSBT_IN_TAP_SCRIPT_SIG, keyData, signature); + } + + private PSBTInput inputFrom(List entries) throws Exception { + Transaction transaction = new Transaction(); + transaction.setVersion(2); + transaction.addInput(Sha256Hash.ZERO_HASH, 0, new Script(new byte[0])); + + return new PSBTInput(new PSBT(transaction), entries, 0); + } + + @Test + public void a_script_path_signature_is_read_and_kept() throws Exception { + byte unifiedAll = (byte)(SigHash.UNIFIED_FLAG | SigHash.ALL.byteValue()); + PSBTInput psbtInput = inputFrom(List.of(tapScriptEntry(signature(unifiedAll)))); + + Assertions.assertEquals(1, psbtInput.getTapScriptSignatures().size(), + "an input carrying only these read as one nothing had signed"); + Assertions.assertEquals(Utils.bytesToHex(signature(unifiedAll)), + psbtInput.getTapScriptSignatures().get(X_ONLY_KEY + LEAF_HASH), + "kept exactly as given, since nothing here interprets it and a combiner hands back what it got"); + } + + /** And survives being written back out, so keeping it does not come at the cost of dropping it on the way out. */ + @Test + public void it_survives_a_round_trip() throws Exception { + byte[] signature = signature(SigHash.ALL.byteValue()); + PSBTInput psbtInput = inputFrom(List.of(tapScriptEntry(signature))); + + List written = new ArrayList<>(); + for(PSBTEntry entry : psbtInput.getInputEntries(0)) { + if(entry.getKeyType() == PSBTInput.PSBT_IN_TAP_SCRIPT_SIG) { + written.add(entry); + } + } + Assertions.assertEquals(1, written.size(), "it was parsed and then not written back"); + Assertions.assertArrayEquals(signature, written.get(0).getData()); + Assertions.assertEquals(X_ONLY_KEY + LEAF_HASH, Utils.bytesToHex(written.get(0).getKeyData())); + + Assertions.assertEquals(1, inputFrom(written).getTapScriptSignatures().size(), + "and reads back the same on the other side"); + } + + /** Key data that is not a public key and a leaf hash is refused rather than stored under a nonsense key. */ + @Test + public void a_malformed_key_is_refused() { + byte[] keyData = new byte[7]; + byte[] key = new byte[1 + keyData.length]; + key[0] = PSBTInput.PSBT_IN_TAP_SCRIPT_SIG; + + Assertions.assertThrows(PSBTParseException.class, + () -> inputFrom(List.of(new PSBTEntry(key, PSBTInput.PSBT_IN_TAP_SCRIPT_SIG, keyData, signature(SigHash.ALL.byteValue()))))); + } + + /** Cleared with the rest of the pre-finalisation state, since a finalised input carries its witness instead. */ + @Test + public void it_is_cleared_when_the_input_is_finalised() throws Exception { + PSBTInput psbtInput = inputFrom(List.of(tapScriptEntry(signature(SigHash.ALL.byteValue())))); + psbtInput.clearNonFinalFields(); + + Assertions.assertTrue(psbtInput.getTapScriptSignatures().isEmpty()); + } + + /** + * A key type written as a longer compact size integer than it needs is still read as this type, because the entry + * parser does not require the canonical form and the switch that dispatches on it takes the low byte. The padding + * lands in the key length, so a check on the whole key admits key data that is short by exactly the padding. + * + * Refused on the key data itself, which is the thing being asserted. Left on the key length it parsed, and was then + * written back out with the one byte form, and the PSBT this library produced could no longer be read by it. + */ + @Test + public void a_key_type_padded_to_a_longer_compact_size_is_refused() { + byte[] keyData = new byte[62]; + Arrays.fill(keyData, (byte)0x77); + + ByteBuffer entry = ByteBuffer.allocate(1 + 65 + 1 + 65); + entry.put((byte)65); + entry.put(new byte[] {(byte)0xfd, 0x14, 0x01}); + entry.put(keyData); + entry.put((byte)65); + entry.put(signature(SigHash.ALL.byteValue())); + entry.flip(); + + PSBTEntry parsed = Assertions.assertDoesNotThrow(() -> new PSBTEntry(entry)); + Assertions.assertEquals(276, parsed.getKeyType(), "the padded form has to reach the type this test is about"); + Assertions.assertEquals(65, parsed.getKey().length, "and has to pass a check made on the key length"); + + Assertions.assertThrows(PSBTParseException.class, () -> inputFrom(List.of(parsed)), + "62 bytes is not an x only public key and a leaf hash, whatever the key length says"); + } + + /** + * A signature whose last byte is zero keeps that byte. Decoded and re-encoded it would not: that byte is read as + * the hash type, zero is the default type, and the encoder omits the byte for it, so 65 bytes in came 64 bytes out + * and a combiner rewrote a signature another signer had made. + */ + @Test + public void a_signature_ending_in_a_zero_byte_is_handed_back_unchanged() throws Exception { + byte[] signature = signature((byte)0x00); + PSBTInput psbtInput = inputFrom(List.of(tapScriptEntry(signature))); + + Assertions.assertEquals(Utils.bytesToHex(signature), + psbtInput.getTapScriptSignatures().get(X_ONLY_KEY + LEAF_HASH)); + for(PSBTEntry entry : psbtInput.getInputEntries(0)) { + if(entry.getKeyType() == PSBTInput.PSBT_IN_TAP_SCRIPT_SIG) { + Assertions.assertArrayEquals(signature, entry.getData(), "the trailing byte was dropped on the way out"); + } + } + } + + /** + * A value that is not a signature length at all is refused rather than stored, and refused as a PSBT parse + * failure. Handed to the Schnorr decoder it threw IllegalArgumentException instead, unchecked, out of a + * constructor that declares PSBTParseException, which is the escape the neighbouring cases are written to avoid. + */ + @Test + public void a_signature_of_the_wrong_length_is_refused() { + for(int length : new int[] {0, 63, 66}) { + byte[] wrong = new byte[length]; + Arrays.fill(wrong, (byte)0x33); + + Assertions.assertThrows(PSBTParseException.class, () -> inputFrom(List.of(tapScriptEntry(wrong))), + length + " bytes is not a signature"); + } + } +} diff --git a/src/test/java/com/sparrowwallet/drongo/psbt/VerifiedSignaturesTest.java b/src/test/java/com/sparrowwallet/drongo/psbt/VerifiedSignaturesTest.java index 573de303..562a975a 100644 --- a/src/test/java/com/sparrowwallet/drongo/psbt/VerifiedSignaturesTest.java +++ b/src/test/java/com/sparrowwallet/drongo/psbt/VerifiedSignaturesTest.java @@ -351,4 +351,112 @@ public void nothing_verifies_where_there_is_no_message_to_verify_against() { Assertions.assertTrue(psbtInput.getVerifiedSignatures(trusted()).isEmpty()); } + + /** + * A partial signature names the key that made it, and that name is written by whoever wrote the input. Verifying + * against the named key rather than against every key the caller holds is only sound while the name is checked for + * membership and the signature still has to verify: naming a key the caller vouches for buys nothing on its own. + */ + @Test + public void a_forged_signature_under_a_trusted_key_name_is_not_counted() { + PSBTInput psbtInput = signedInput(SigHash.ALL.byteValue()); + ECKey outputKey = ScriptType.P2WPKH.getOutputKey(PolicyType.SINGLE_HD, key()); + + TransactionSignature real = psbtInput.getPartialSignature(ECKey.fromPublicOnly(outputKey)); + byte[] forged = real.encodeToBitcoin(); + forged[10] ^= 0x01; + psbtInput.getPartialSignatures().put(ECKey.fromPublicOnly(outputKey), + TransactionSignature.decodeFromBitcoin(TransactionSignature.Type.ECDSA, forged, false)); + + Assertions.assertTrue(psbtInput.getVerifiedSignatures(trusted()).isEmpty(), + "the name is not the guarantee, the signature is"); + } + + /** + * And a signature this key really did make, over another transaction, moved into this one. The message is rebuilt + * from this transaction, so a signature made over a different one cannot verify against it whoever made it. + */ + @Test + public void a_signature_this_key_made_over_another_transaction_is_not_counted() { + PSBTInput other = signedInput(SigHash.ALL.byteValue()); + ECKey outputKey = ScriptType.P2WPKH.getOutputKey(PolicyType.SINGLE_HD, key()); + TransactionSignature elsewhere = other.getPartialSignature(ECKey.fromPublicOnly(outputKey)); + + //A different message: the amount this input spends is committed to, so changing it is enough + PSBTInput psbtInput = signedInput(SigHash.ALL.byteValue()); + Script spk = ScriptType.P2WPKH.getOutputScript(PolicyType.SINGLE_HD, key()); + psbtInput.setWitnessUtxo(new TransactionOutput(null, VALUE - 20_000, spk.getProgram())); + psbtInput.getPartialSignatures().clear(); + psbtInput.getPartialSignatures().put(ECKey.fromPublicOnly(outputKey), elsewhere); + + Assertions.assertTrue(psbtInput.getVerifiedSignatures(trusted()).isEmpty(), + "a signature over another transaction says nothing about this one"); + } + + /** + * A public key in a PSBT is attacker supplied and is not checked when it is parsed: ECKey.fromPublicOnly wraps a + * lazy point, so a 33 byte value that is not on the curve is accepted and only throws when the point is first + * read. Verifying a partial signature against the key that names it reads that point, which is the one thing this + * method promises not to do: the caller is drawing a label, and an exception leaves whatever it said before. + * + * The old loop never touched a key from the PSBT at all, so this became reachable only by the change that made it + * check the named key. + */ + @Test + public void a_public_key_that_is_not_on_the_curve_answers_nothing_rather_than_throwing() { + PSBTInput psbtInput = signedInput(SigHash.ALL.byteValue()); + ECKey outputKey = ScriptType.P2WPKH.getOutputKey(PolicyType.SINGLE_HD, key()); + TransactionSignature real = psbtInput.getPartialSignature(ECKey.fromPublicOnly(outputKey)); + + ECKey offCurve = ECKey.fromPublicOnly(Utils.hexToBytes("02" + "ff".repeat(32))); + psbtInput.getPartialSignatures().put(offCurve, real); + + List verified = Assertions.assertDoesNotThrow( + () -> psbtInput.getVerifiedSignatures(trusted()), + "a key that is not a point on the curve must not take the label with it"); + Assertions.assertEquals(1, verified.size(), + "the real signature beside it still has to be found, not lost to the bad entry"); + } + + /** + * The caller's key and the key the PSBT names are the same key when they are the same point, whatever each carries + * around it. ECKey.equals compares the private field, so a caller vouching with a key it can sign with never + * matched the public one named in the input, and a swept key stopped being counted. The two encodings of one + * public key are the same point too. + */ + @Test + public void a_key_is_matched_by_its_point_and_not_by_what_it_is_wrapped_in() { + PSBTInput psbtInput = signedInput(SigHash.ALL.byteValue()); + ECKey outputKey = ScriptType.P2WPKH.getOutputKey(PolicyType.SINGLE_HD, key()); + + Assertions.assertEquals(1, psbtInput.getVerifiedSignatures(List.of(outputKey)).size(), + "a key carrying its private part is the same key as the public one the input names"); + + ECKey uncompressed = ECKey.fromPublicOnly(outputKey.getPubKeyPoint().getEncoded(false)); + Assertions.assertEquals(65, uncompressed.getPubKey().length, "the fixture has to be the other encoding"); + Assertions.assertEquals(1, psbtInput.getVerifiedSignatures(List.of(uncompressed)).size(), + "the two encodings of one public key are one key"); + + ECKey stranger = ECKey.fromPublicOnly(ECKey.fromPrivate(Utils.hexToBytes("44".repeat(32))).getPubKey()); + Assertions.assertTrue(psbtInput.getVerifiedSignatures(List.of(stranger)).isEmpty(), + "and a different key is still a different key"); + } + + /** + * And the same key on the path a PSBT takes when it is opened. verifySignatures declares PSBTSignatureException + * and the caller catches that alone, so an IllegalArgumentException from decoding a key the PSBT names left the + * open flow uncaught. Pre-existing, and reachable from any file, paste or scan. + */ + @Test + public void opening_a_psbt_naming_a_key_that_is_not_on_the_curve_is_refused_not_crashed() { + PSBTInput psbtInput = signedInput(SigHash.ALL.byteValue()); + ECKey outputKey = ScriptType.P2WPKH.getOutputKey(PolicyType.SINGLE_HD, key()); + TransactionSignature real = psbtInput.getPartialSignature(ECKey.fromPublicOnly(outputKey)); + + psbtInput.getPartialSignatures().clear(); + psbtInput.getPartialSignatures().put(ECKey.fromPublicOnly(Utils.hexToBytes("02" + "ff".repeat(32))), real); + + Assertions.assertThrows(PSBTSignatureException.class, psbtInput::verifySignatures, + "a key that is not a point on the curve is an invalid PSBT, not an escaping runtime exception"); + } }