diff --git a/src/main/scala/org/bitcoins/core/crypto/DERSignatureUtil.scala b/src/main/scala/org/bitcoins/core/crypto/DERSignatureUtil.scala index f6cec0a2cb..6d9af3ad27 100644 --- a/src/main/scala/org/bitcoins/core/crypto/DERSignatureUtil.scala +++ b/src/main/scala/org/bitcoins/core/crypto/DERSignatureUtil.scala @@ -13,6 +13,7 @@ trait DERSignatureUtil extends BitcoinSLogger { /** * Checks if this signature is encoded to DER correctly * https://crypto.stackexchange.com/questions/1795/how-can-i-convert-a-der-ecdsa-signature-to-asn-1 + * NOTE: This will fail if this signature contains the hash type appended to the end of it * @return boolean representing if the signature is a valid */ def isDEREncoded(signature : ECDigitalSignature) : Boolean = isDEREncoded(signature.bytes) @@ -20,22 +21,22 @@ trait DERSignatureUtil extends BitcoinSLogger { /** * Checks if the bytes are encoded to DER correctly * https://crypto.stackexchange.com/questions/1795/how-can-i-convert-a-der-ecdsa-signature-to-asn-1 + * This will fail if this signature contains the hash type appended to the end of it * @return boolean representing if the signature is a valid */ def isDEREncoded(bytes : Seq[Byte]) : Boolean = { //signature is trivially valid if the signature is empty - - if (!bytes.isEmpty && bytes.size < 9) false - else if (!bytes.isEmpty) { + if (bytes.nonEmpty && bytes.size < 9) false + else if (bytes.nonEmpty) { //first byte must be 0x30 val firstByteIs0x30 = bytes.head == 0x30 logger.debug("firstByteIs0x30: " + firstByteIs0x30) //second byte must indicate the length of the remaining byte array val signatureSize = bytes(1).toLong logger.debug("Encoded Sisgnature Size: " + signatureSize) - logger.debug("Actual Signature Size: " + bytes.slice(3,bytes.size).size) + logger.debug("Actual Signature Size: " + bytes.slice(2,bytes.size).size) //checks to see if the signature length is the same as the signatureSize val - val signatureLengthIsCorrect = signatureSize == bytes.slice(3,bytes.size).size + val signatureLengthIsCorrect = signatureSize == bytes.slice(2,bytes.size).size logger.debug("Signature length is correct: " + signatureLengthIsCorrect) //third byte must be 0x02 val thirdByteIs0x02 = bytes(2) == 0x02 @@ -114,14 +115,15 @@ trait DERSignatureUtil extends BitcoinSLogger { /** * This functions implements the strict der encoding rules that were created in BIP66 - * https://github.com/bitcoin/bips/blob/master/bip-0066.mediawiki + * [[https://github.com/bitcoin/bips/blob/master/bip-0066.mediawiki]] + * [[https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L98]] * @param signature the signature to check if they are strictly der encoded * @return boolean indicating whether the signature was der encoded or not */ - def isStrictDEREncoding(signature : ECDigitalSignature) : Boolean = { + def isValidSignatureEncoding(signature : ECDigitalSignature) : Boolean = { signature match { case EmptyDigitalSignature => true - case signature : ECDigitalSignature => isStrictDEREncoding(signature.bytes) + case signature : ECDigitalSignature => isValidSignatureEncoding(signature.bytes) } } @@ -129,10 +131,11 @@ trait DERSignatureUtil extends BitcoinSLogger { /** * This functions implements the strict der encoding rules that were created in BIP66 * https://github.com/bitcoin/bips/blob/master/bip-0066.mediawiki + * [[https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L98]] * @param bytes the bytes to check if they are strictly der encoded * @return boolean indicating whether the bytes were der encoded or not */ - def isStrictDEREncoding(bytes : Seq[Byte]) : Boolean = { + def isValidSignatureEncoding(bytes : Seq[Byte]) : Boolean = { // Format: 0x30 [total-length] 0x02 [R-length] [R] 0x02 [S-length] [S] [sighash] // * total-length: 1-byte length descriptor of everything that follows, // excluding the sighash byte. @@ -150,9 +153,9 @@ trait DERSignatureUtil extends BitcoinSLogger { if (bytes.size == 0) return true //check if the bytes are ATLEAST der encoded - val isDerEncoded = isDEREncoded(bytes) +/* val isDerEncoded = isDEREncoded(bytes) if (!isDerEncoded) return false - logger.debug("Signature is at minimum DER encoded") + logger.debug("Signature is at minimum DER encoded")*/ if (bytes.size < 9) return false logger.debug("signature is the minimum size for strict der encoding") @@ -222,7 +225,6 @@ trait DERSignatureUtil extends BitcoinSLogger { /** * Requires the S value in signatures to be the low version of the S value * https://github.com/bitcoin/bips/blob/master/bip-0062.mediawiki#low-s-values-in-signatures - * * @param signature * @return if the S value is the low version */ @@ -231,12 +233,11 @@ trait DERSignatureUtil extends BitcoinSLogger { /** * Requires the S value in signatures to be the low version of the S value * https://github.com/bitcoin/bips/blob/master/bip-0062.mediawiki#low-s-values-in-signatures - * * @param signature * @return if the S value is the low version */ def isLowDerSignature(signature : Seq[Byte]) : Boolean = { - if (!isDEREncoded(signature)) false + if (!isValidSignatureEncoding(signature)) false else { val (r,s) = decodeSignature(signature) val upperBound = BigInt("7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0",16) diff --git a/src/main/scala/org/bitcoins/core/crypto/ECDigitalSignature.scala b/src/main/scala/org/bitcoins/core/crypto/ECDigitalSignature.scala index 6ff0d0d823..f8e8eb2295 100644 --- a/src/main/scala/org/bitcoins/core/crypto/ECDigitalSignature.scala +++ b/src/main/scala/org/bitcoins/core/crypto/ECDigitalSignature.scala @@ -17,16 +17,18 @@ sealed trait ECDigitalSignature extends BitcoinSLogger { /** * Checks if this signature is encoded to DER correctly * https://crypto.stackexchange.com/questions/1795/how-can-i-convert-a-der-ecdsa-signature-to-asn-1 - * * @return boolean representing if the signature is a valid */ def isDEREncoded : Boolean = DERSignatureUtil.isDEREncoded(this) + /** Checks if the signature is strictly der encoded as per BIP66 + * [[https://github.com/bitcoin/bips/blob/master/bip-0066.mediawiki]] + * */ + def isStrictEncoded: Boolean = DERSignatureUtil.isValidSignatureEncoding(this) /** * Decodes the digital signature into it's r and s points * throws an exception if the given sequence of bytes is not a DER encoded signature - * * @return the (r,s) values for the elliptic curve digital signature */ def decodeSignature : (BigInt,BigInt) = DERSignatureUtil.decodeSignature(this) @@ -40,7 +42,6 @@ sealed trait ECDigitalSignature extends BitcoinSLogger { /** * Represents the s value found in a elliptic curve digital signature - * * @return */ def s = decodeSignature._2 diff --git a/src/main/scala/org/bitcoins/core/crypto/TransactionSignatureChecker.scala b/src/main/scala/org/bitcoins/core/crypto/TransactionSignatureChecker.scala index e24bb968ef..fa910d2ffe 100644 --- a/src/main/scala/org/bitcoins/core/crypto/TransactionSignatureChecker.scala +++ b/src/main/scala/org/bitcoins/core/crypto/TransactionSignatureChecker.scala @@ -36,7 +36,7 @@ trait TransactionSignatureChecker extends BitcoinSLogger { pubKey: ECPublicKey, signature : ECDigitalSignature, flags : Seq[ScriptFlag]) : TransactionSignatureCheckerResult = { logger.info("Signature: " + signature) val pubKeyEncodedCorrectly = BitcoinScriptUtil.checkPubKeyEncoding(pubKey,flags) - if (ScriptFlagUtil.requiresStrictDerEncoding(flags) && !DERSignatureUtil.isStrictDEREncoding(signature)) { + if (ScriptFlagUtil.requiresStrictDerEncoding(flags) && !DERSignatureUtil.isValidSignatureEncoding(signature)) { logger.error("Signature was not stricly encoded der: " + signature.hex) SignatureValidationFailureNotStrictDerEncoding } else if (ScriptFlagUtil.requireLowSValue(flags) && !DERSignatureUtil.isLowDerSignature(signature)) { diff --git a/src/main/scala/org/bitcoins/core/crypto/TransactionSignatureCreator.scala b/src/main/scala/org/bitcoins/core/crypto/TransactionSignatureCreator.scala index 3dfd139c97..9c941b2453 100644 --- a/src/main/scala/org/bitcoins/core/crypto/TransactionSignatureCreator.scala +++ b/src/main/scala/org/bitcoins/core/crypto/TransactionSignatureCreator.scala @@ -9,7 +9,6 @@ trait TransactionSignatureCreator { /** * Creates a signature from a tx signature component - * * @param txSignatureComponent contains the tx, inputIndex which specify which input we are creating a sig for * @param privateKey the private key which we are signing the hash with * @param hashType the procedure to use for hashing to transaction @@ -19,7 +18,8 @@ trait TransactionSignatureCreator { val hash = TransactionSignatureSerializer.hashForSignature(txSignatureComponent, hashType) val signature = privateKey.sign(hash) //append 1 byte hash type onto the end - ECDigitalSignature(signature.bytes ++ Seq(hashType.byte)) + val sig = ECDigitalSignature(signature.bytes ++ Seq(hashType.byte)) + sig } } diff --git a/src/main/scala/org/bitcoins/core/script/crypto/CryptoInterpreter.scala b/src/main/scala/org/bitcoins/core/script/crypto/CryptoInterpreter.scala index 334c61df84..9938dcb15c 100644 --- a/src/main/scala/org/bitcoins/core/script/crypto/CryptoInterpreter.scala +++ b/src/main/scala/org/bitcoins/core/script/crypto/CryptoInterpreter.scala @@ -93,7 +93,7 @@ trait CryptoInterpreter extends ControlOperationsInterpreter with BitcoinSLogger val signature = ECDigitalSignature(executionInProgressScriptProgram.stack.tail.head.bytes) if (ScriptFlagUtil.requiresStrictDerEncoding(executionInProgressScriptProgram.flags) && - !DERSignatureUtil.isStrictDEREncoding(signature)) { + !DERSignatureUtil.isValidSignatureEncoding(signature)) { //this means all of the signatures must encoded according to BIP66 strict dersig //https://github.com/bitcoin/bips/blob/master/bip-0066.mediawiki //script verification fails since the sig is not strictly der encoded diff --git a/src/test/scala/org/bitcoins/core/crypto/DERSignatureUtilTest.scala b/src/test/scala/org/bitcoins/core/crypto/DERSignatureUtilTest.scala index fc99aa8620..478eee2aa3 100644 --- a/src/test/scala/org/bitcoins/core/crypto/DERSignatureUtilTest.scala +++ b/src/test/scala/org/bitcoins/core/crypto/DERSignatureUtilTest.scala @@ -11,17 +11,20 @@ class DERSignatureUtilTest extends FlatSpec with MustMatchers { val p2shSignature = ECDigitalSignature("304402205b7d2c2f177ae76cfbbf14d589c113b0b35db753d305d5562dd0b61cbf366cfb02202e56f93c4f08a27f986cd424ffc48a462c3202c4902104d4d0ff98ed28f4bf8001") val p2pkhSignature = ECDigitalSignature("3044022016ffdbb7c57634903c5e018fcfc48d59f4e37dc4bc3bbc9ba4e6ee39150bca030220119c2241a931819bc1a75d3596e4029d803d1cd6de123bf8a1a1a2c3665e1fac01") val p2pkSignature = ECDigitalSignature("304402200a5c6163f07b8d3b013c4d1d6dba25e780b39658d79ba37af7057a3b7f15ffa102201fd9b4eaa9943f734928b99a83592c2e7bf342ea2680f6a2bb705167966b742001") + "DERSignatureUtil" must "say that a signature taken from a p2sh transaction is a valid DER encoded signature" in { - DERSignatureUtil.isDEREncoded(p2shSignature) must be (true) + DERSignatureUtil.isValidSignatureEncoding(p2shSignature) must be (true) } it must "say that signature taken from a p2pkh transaction is a valid DER encoded signature" in { - DERSignatureUtil.isDEREncoded(p2pkhSignature) must be (true) + //need to remove the hash type byte to check for der encoding + val hashTypeByteRemoved = p2pkhSignature.bytes.slice(0,p2pkhSignature.bytes.size-1) + DERSignatureUtil.isDEREncoded(hashTypeByteRemoved) must be (true) } it must "say that a signature taken from a p2pk transaction is a valid DER encoded signature" in { - - DERSignatureUtil.isDEREncoded(p2pkSignature) must be (true) + val hashTypeByteRemoved = p2pkSignature.bytes.slice(0,p2pkSignature.bytes.size-1) + DERSignatureUtil.isDEREncoded(hashTypeByteRemoved) must be (true) } it must "retrieve the (r,s) values for a p2sh signature in bitcoin" in { @@ -43,27 +46,27 @@ class DERSignatureUtilTest extends FlatSpec with MustMatchers { } it must "say that a signature taken from a p2sh transaction is a valid stirctly DER encoded signature" in { - DERSignatureUtil.isStrictDEREncoding(p2shSignature) must be (true) + DERSignatureUtil.isValidSignatureEncoding(p2shSignature) must be (true) } it must "say that signature taken from a p2pkh transaction is a valid strictly DER encoded signature" in { - DERSignatureUtil.isStrictDEREncoding(p2pkhSignature) must be (true) + DERSignatureUtil.isValidSignatureEncoding(p2pkhSignature) must be (true) } it must "say that a signature taken from a p2pk transaction is a valid strictly DER encoded signature" in { - DERSignatureUtil.isStrictDEREncoding(p2pkSignature) must be (true) + DERSignatureUtil.isValidSignatureEncoding(p2pkSignature) must be (true) } it must "say that the empty signature is a valid strictly encoded DER signature" in { - DERSignatureUtil.isStrictDEREncoding(ECDigitalSignature("")) must be (true) - DERSignatureUtil.isStrictDEREncoding(EmptyDigitalSignature) must be (true) + DERSignatureUtil.isValidSignatureEncoding(ECDigitalSignature("")) must be (true) + DERSignatureUtil.isValidSignatureEncoding(EmptyDigitalSignature) must be (true) } it must "say that an overly long signature is NOT strict der encoded" in { val sig = ECDigitalSignature("0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000") - DERSignatureUtil.isStrictDEREncoding(sig) must be (false) + DERSignatureUtil.isValidSignatureEncoding(sig) must be (false) } it must "determine if a signature is encoded with a low s value" in { diff --git a/src/test/scala/org/bitcoins/core/crypto/ECDigitalSignatureTest.scala b/src/test/scala/org/bitcoins/core/crypto/ECDigitalSignatureTest.scala index 18a6867754..068b18c6c0 100644 --- a/src/test/scala/org/bitcoins/core/crypto/ECDigitalSignatureTest.scala +++ b/src/test/scala/org/bitcoins/core/crypto/ECDigitalSignatureTest.scala @@ -15,17 +15,17 @@ class ECDigitalSignatureTest extends FlatSpec with MustMatchers { } it must "say that a signature taken from a p2sh transaction is a valid DER encoded signature" in { - val signature = ECDigitalSignature("304402205b7d2c2f177ae76cfbbf14d589c113b0b35db753d305d5562dd0b61cbf366cfb02202e56f93c4f08a27f986cd424ffc48a462c3202c4902104d4d0ff98ed28f4bf8001") + val signature = ECDigitalSignature("304402205b7d2c2f177ae76cfbbf14d589c113b0b35db753d305d5562dd0b61cbf366cfb02202e56f93c4f08a27f986cd424ffc48a462c3202c4902104d4d0ff98ed28f4bf80") signature.isDEREncoded must be (true) } it must "say that signature taken from a p2pkh transaction is a valid DER encoded signature" in { - val signature = ECDigitalSignature("3044022016ffdbb7c57634903c5e018fcfc48d59f4e37dc4bc3bbc9ba4e6ee39150bca030220119c2241a931819bc1a75d3596e4029d803d1cd6de123bf8a1a1a2c3665e1fac01") + val signature = ECDigitalSignature("3044022016ffdbb7c57634903c5e018fcfc48d59f4e37dc4bc3bbc9ba4e6ee39150bca030220119c2241a931819bc1a75d3596e4029d803d1cd6de123bf8a1a1a2c3665e1fac") signature.isDEREncoded must be (true) } it must "say that a signature taken from a p2pk transaction is a valid DER encoded signature" in { - val signature = ECDigitalSignature("304402200a5c6163f07b8d3b013c4d1d6dba25e780b39658d79ba37af7057a3b7f15ffa102201fd9b4eaa9943f734928b99a83592c2e7bf342ea2680f6a2bb705167966b742001") + val signature = ECDigitalSignature("304402200a5c6163f07b8d3b013c4d1d6dba25e780b39658d79ba37af7057a3b7f15ffa102201fd9b4eaa9943f734928b99a83592c2e7bf342ea2680f6a2bb705167966b7420") signature.isDEREncoded must be (true) }