Fixing a couple bugs with signature checking non standard txs, adding a object to hold our crypto params

This commit is contained in:
Chris Stewart 2016-03-29 09:59:27 -05:00
parent 4e63ce2079
commit 2b5379ba14
5 changed files with 59 additions and 26 deletions

View file

@ -0,0 +1,18 @@
package org.scalacoin.crypto
import org.spongycastle.asn1.sec.SECNamedCurves
/**
* Created by chris on 3/29/16.
* This trait represents all of the default parameters for our elliptic curve
*/
trait CryptoParams {
/**
* This is the elliptic curve bitcoin uses
* @return
*/
def params = SECNamedCurves.getByName("secp256k1")
}
object CryptoParams extends CryptoParams

View file

@ -126,7 +126,7 @@ trait TransactionSignatureChecker extends BitcoinSLogger {
case y : MultiSignatureScriptPubKey =>
//the signatures & pubkeys need to be reversed so that they are evaluated the
//same way as if they were getting pushed then popped off of a stack
multiSignatureHelper(spendingTransaction, inputIndex, y,
multiSignatureEvaluator(spendingTransaction, inputIndex, y,
p2shScriptSignature.signatures.toList.reverse, p2shScriptSignature.publicKeys.toList.reverse, requireStrictDEREncoding, y.requiredSigs)
case _ : P2PKHScriptPubKey | _ : P2PKScriptPubKey | _ : P2SHScriptPubKey | _ : NonStandardScriptPubKey | EmptyScriptPubKey =>
throw new RuntimeException("Don't know how to implement this scriptPubKeys in a redeemScript")
@ -166,7 +166,7 @@ trait TransactionSignatureChecker extends BitcoinSLogger {
//same way as if they were getting pushed then popped off of a stack
logger.info("multisig public keys: " + x.publicKeys)
logger.info("multisig sigs: " + multiSignatureScript.signatures)
multiSignatureHelper(spendingTransaction,inputIndex,x,multiSignatureScript.signatures.toList.reverse,
multiSignatureEvaluator(spendingTransaction,inputIndex,x,multiSignatureScript.signatures.toList.reverse,
x.publicKeys.toList.reverse,requireStrictDEREncoding,x.requiredSigs)
case x : P2PKHScriptPubKey =>
logger.warn("Trying to check if a multisignature scriptSig spends a p2pkh scriptPubKey properly - this is trivially false")
@ -220,7 +220,7 @@ trait TransactionSignatureChecker extends BitcoinSLogger {
* @return a boolean indicating if all of the signatures are valid against the given public keys
*/
@tailrec
private def multiSignatureHelper(spendingTransaction : Transaction, inputIndex : Int, scriptPubKey : MultiSignatureScriptPubKey,
final def multiSignatureEvaluator(spendingTransaction : Transaction, inputIndex : Int, scriptPubKey : ScriptPubKey,
sigs : List[ECDigitalSignature], pubKeys : List[ECPublicKey], requireStrictDEREncoding : Boolean,
requiredSigs : Long) : TransactionSignatureCheckerResult = {
logger.info("Signatures inside of helper: " + sigs)
@ -249,9 +249,9 @@ trait TransactionSignatureChecker extends BitcoinSLogger {
val result = pubKey.verify(hashForSig, sig)
result match {
case true =>
multiSignatureHelper(spendingTransaction,inputIndex, scriptPubKey, sigs.tail,pubKeys.tail,requireStrictDEREncoding, requiredSigs -1)
multiSignatureEvaluator(spendingTransaction,inputIndex, scriptPubKey, sigs.tail,pubKeys.tail,requireStrictDEREncoding, requiredSigs -1)
case false =>
multiSignatureHelper(spendingTransaction,inputIndex, scriptPubKey, sigs,pubKeys.tail,requireStrictDEREncoding, requiredSigs)
multiSignatureEvaluator(spendingTransaction,inputIndex, scriptPubKey, sigs,pubKeys.tail,requireStrictDEREncoding, requiredSigs)
}
}
} else if (sigs.isEmpty) {

View file

@ -174,15 +174,16 @@ trait CryptoInterpreter extends ControlOperationsInterpreter with BitcoinSLogger
logger.warn("Script flag null dummy was set however the first element in the script signature was not an OP_0")
ScriptProgramFactory.factory(program,false)
} else {
val isValidSignatures = TransactionSignatureChecker.checkSignature(program)
//these next lines remove the appropriate stack/script values after the signatures have been checked
val nPossibleSignatures : Int = program.stack.head match {
case s : ScriptNumber => s.num.toInt
case _ => throw new RuntimeException("n must be a script number for OP_CHECKMULTISIG")
}
logger.debug("nPossibleSignatures: " + nPossibleSignatures)
val stackWithoutPubKeys = program.stack.tail.slice(nPossibleSignatures,program.stack.tail.size)
val (pubKeysScriptTokens,stackWithoutPubKeys) =
(program.stack.tail.slice(0,nPossibleSignatures),program.stack.tail.slice(nPossibleSignatures,program.stack.tail.size))
val pubKeys = pubKeysScriptTokens.map(key => ECFactory.publicKey(key.hex))
logger.debug("Public keys on the stack: " + pubKeys)
val mRequiredSignatures : Int = stackWithoutPubKeys.head match {
case s: ScriptNumber => s.num.toInt
case _ => throw new RuntimeException("m must be a script number for OP_CHECKMULTISIG")
@ -190,13 +191,27 @@ trait CryptoInterpreter extends ControlOperationsInterpreter with BitcoinSLogger
logger.debug("mRequiredSignatures: " + mRequiredSignatures )
//+1 is for the fact that we have the # of sigs + the script token indicating the # of sigs
val signaturesScriptTokens : Seq[ScriptToken] = program.stack.tail.slice(nPossibleSignatures + 1, nPossibleSignatures + mRequiredSignatures + 1)
val signaturesScriptTokens = program.stack.tail.slice(nPossibleSignatures + 1, nPossibleSignatures + mRequiredSignatures + 1)
val signatures = signaturesScriptTokens.map(token => ECFactory.digitalSignature(token.bytes))
logger.debug("Signatures on the stack: " + signatures)
logger.debug("ScriptPubKey: " + program.scriptPubKey)
//+1 is for bug in OP_CHECKMULTSIG that requires an extra OP to be pushed onto the stack
val stackWithoutPubKeysAndSignatures = stackWithoutPubKeys.tail.slice(mRequiredSignatures+1, stackWithoutPubKeys.tail.size)
val restOfStack = stackWithoutPubKeysAndSignatures
val isValidSignatures : TransactionSignatureCheckerResult = program.scriptSignature match {
case EmptyScriptSignature =>
TransactionSignatureChecker.checkSignature(program)
case _ : MultiSignatureScriptSignature | _ : P2SHScriptSignature=>
TransactionSignatureChecker.checkSignature(program)
case scriptSignature : NonStandardScriptSignature =>
TransactionSignatureChecker.multiSignatureEvaluator(program.transaction,program.inputIndex,program.scriptPubKey,
signatures.toList,pubKeys,program.flags.contains(ScriptVerifyDerSig),mRequiredSignatures)
case _ : P2PKScriptSignature | _ : P2PKHScriptSignature =>
TransactionSignatureChecker.checkSignature(program)
}
isValidSignatures match {
case SignatureValidationSuccess =>
//means that all of the signatures were correctly encoded and

View file

@ -25,18 +25,18 @@ class ScriptInterpreterTest extends FlatSpec with MustMatchers with ScriptInterp
"ScriptInterpreter" must "evaluate a valid script to true" in {
//this is in asm format, not hex
/* val inputScript = TestUtil.p2pkhInputScriptAsm
val inputScript = TestUtil.p2pkhInputScriptAsm
//this is asm format, not hex
val outputScript : List[ScriptToken] = TestUtil.p2pkhOutputScriptAsm
val stack = List()
val script = inputScript ++ outputScript
val program = ScriptProgramFactory.factory(TestUtil.testProgram,stack,script)
val result = run(program)
result must be (true)*/
result must be (true)
}
/* it must "evaluate a script that asks to push 20 bytes onto the stack correctly" in {
it must "evaluate a script that asks to push 20 bytes onto the stack correctly" in {
val stack = List(ScriptConstantImpl("68ca4fec736264c13b859bac43d5173df6871682"))
val script = List(BytesToPushOntoStackImpl(20), ScriptConstantImpl("68ca4fec736264c13b859bac43d5173df6871682"), OP_EQUAL)
@ -59,7 +59,7 @@ class ScriptInterpreterTest extends FlatSpec with MustMatchers with ScriptInterp
val program = ScriptProgramFactory.factory(TestUtil.testProgram,stack,script)
run(program) must equal (true)
}*/
}
@ -72,20 +72,12 @@ class ScriptInterpreterTest extends FlatSpec with MustMatchers with ScriptInterp
* they are going forward to bitcoin - for historical purposes though these should pass
* they all have to do with DER encoded sigs
* bitcoinj currently fails on these
* ["Increase test coverage for DERSIG"],
["0x4a 0x0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", "0 CHECKSIG NOT", "", "Overly long signature is correctly encoded"],
["0x25 0x30220220000000000000000000000000000000000000000000000000000000000000000000", "0 CHECKSIG NOT", "", "Missing S is correctly encoded"],
["0x27 0x3024021077777777777777777777777777777777020a7777777777777777777777777777777701", "0 CHECKSIG NOT", "", "S with invalid S length is correctly encoded"],
["0x27 0x302403107777777777777777777777777777777702107777777777777777777777777777777701", "0 CHECKSIG NOT", "", "Non-integer R is correctly encoded"],
["0x27 0x302402107777777777777777777777777777777703107777777777777777777777777777777701", "0 CHECKSIG NOT", "", "Non-integer S is correctly encoded"],
["0x17 0x3014020002107777777777777777777777777777777701", "0 CHECKSIG NOT", "", "Zero-length R is correctly encoded"],
["0x17 0x3014021077777777777777777777777777777777020001", "0 CHECKSIG NOT", "", "Zero-length S is correctly encoded for DERSIG"],
["0x27 0x302402107777777777777777777777777777777702108777777777777777777777777777777701", "0 CHECKSIG NOT", "", "Negative S is correctly encoded"],
* ,
*/
val source = scala.io.Source.fromFile("src/test/scala/org/scalacoin/script/interpreter/script_valid.json")
//use this to represent a single test case from script_valid.json
val lines =
/* val lines =
"""
|
|[[
@ -94,9 +86,9 @@ class ScriptInterpreterTest extends FlatSpec with MustMatchers with ScriptInterp
| "",
| "2-of-2 with two identical keys and sigs pushed using OP_DUP but no SIGPUSHONLY"
|]]
""".stripMargin
""".stripMargin*/
//val lines = try source.getLines.filterNot(_.isEmpty).map(_.trim) mkString "\n" finally source.close()
val lines = try source.getLines.filterNot(_.isEmpty).map(_.trim) mkString "\n" finally source.close()
val json = lines.parseJson
val testCasesOpt : Seq[Option[CoreTestCase]] = json.convertTo[Seq[Option[CoreTestCase]]]
val testCases : Seq[CoreTestCase] = testCasesOpt.flatten

View file

@ -691,7 +691,15 @@
"2-of-2 CHECKMULTISIG NOT with both pubkeys valid, but second signature invalid. Valid pubkey fails, and CHECKMULTISIG exits early, prior to evaluation of second invalid signature."
],
["Increase test coverage for DERSIG"],
["0x4a 0x0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", "0 CHECKSIG NOT", "", "Overly long signature is correctly encoded"],
["0x25 0x30220220000000000000000000000000000000000000000000000000000000000000000000", "0 CHECKSIG NOT", "", "Missing S is correctly encoded"],
["0x27 0x3024021077777777777777777777777777777777020a7777777777777777777777777777777701", "0 CHECKSIG NOT", "", "S with invalid S length is correctly encoded"],
["0x27 0x302403107777777777777777777777777777777702107777777777777777777777777777777701", "0 CHECKSIG NOT", "", "Non-integer R is correctly encoded"],
["0x27 0x302402107777777777777777777777777777777703107777777777777777777777777777777701", "0 CHECKSIG NOT", "", "Non-integer S is correctly encoded"],
["0x17 0x3014020002107777777777777777777777777777777701", "0 CHECKSIG NOT", "", "Zero-length R is correctly encoded"],
["0x17 0x3014021077777777777777777777777777777777020001", "0 CHECKSIG NOT", "", "Zero-length S is correctly encoded for DERSIG"],
["0x27 0x302402107777777777777777777777777777777702108777777777777777777777777777777701", "0 CHECKSIG NOT", "", "Negative S is correctly encoded"],
["Automatically generated test cases"],
[