Skip to content

[Fix] - Made public key from signature recover method safer #819

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 27, 2020
Merged

[Fix] - Made public key from signature recover method safer #819

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 23 additions & 19 deletions src/main/scala/io/iohk/ethereum/crypto/ECDSASignature.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import org.bouncycastle.crypto.params.ECPublicKeyParameters
import org.bouncycastle.crypto.signers.{ECDSASigner, HMacDSAKCalculator}
import org.bouncycastle.math.ec.{ECCurve, ECPoint}

import scala.util.Try

object ECDSASignature {

val SLength = 32
Expand Down Expand Up @@ -107,26 +109,28 @@ object ECDSASignature {
chainId: Option[Byte],
messageHash: Array[Byte]
): Option[Array[Byte]] = {
val order = curve.getCurve.getOrder
//ignore case when x = r + order because it is negligibly improbable
//says: https://github.com/paritytech/rust-secp256k1/blob/f998f9a8c18227af200f0f7fdadf8a6560d391ff/depend/secp256k1/src/ecdsa_impl.h#L282
val xCoordinate = r
val curveFp = curve.getCurve.asInstanceOf[ECCurve.Fp]
val prime = curveFp.getQ

getRecoveredPointSign(recId, chainId).flatMap { recovery =>
if (xCoordinate.compareTo(prime) < 0) {
val R = constructPoint(xCoordinate, recovery)
if (R.multiply(order).isInfinity) {
val e = BigInt(1, messageHash)
val rInv = r.modInverse(order)
//Q = r^(-1)(sR - eG)
val q = R.multiply(s.bigInteger).subtract(curve.getG.multiply(e.bigInteger)).multiply(rInv.bigInteger)
//byte 0 of encoded ECC point indicates that it is uncompressed point, it is part of bouncycastle encoding
Some(q.getEncoded(false).tail)
Try {
val order = curve.getCurve.getOrder
//ignore case when x = r + order because it is negligibly improbable
//says: https://github.com/paritytech/rust-secp256k1/blob/f998f9a8c18227af200f0f7fdadf8a6560d391ff/depend/secp256k1/src/ecdsa_impl.h#L282
val xCoordinate = r
val curveFp = curve.getCurve.asInstanceOf[ECCurve.Fp]
val prime = curveFp.getQ

getRecoveredPointSign(recId, chainId).flatMap { recovery =>
if (xCoordinate.compareTo(prime) < 0) {
val R = constructPoint(xCoordinate, recovery)
if (R.multiply(order).isInfinity) {
val e = BigInt(1, messageHash)
val rInv = r.modInverse(order)
//Q = r^(-1)(sR - eG)
val q = R.multiply(s.bigInteger).subtract(curve.getG.multiply(e.bigInteger)).multiply(rInv.bigInteger)
//byte 0 of encoded ECC point indicates that it is uncompressed point, it is part of bouncycastle encoding
Some(q.getEncoded(false).tail)
} else None
} else None
} else None
}
}
}.toOption.flatten
}

private def constructPoint(xCoordinate: BigInt, recId: Int): ECPoint = {
Expand Down
8 changes: 8 additions & 0 deletions src/test/scala/io/iohk/ethereum/crypto/ECDSASignatureSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package io.iohk.ethereum.crypto

import akka.util.ByteString
import io.iohk.ethereum.nodebuilder.SecureRandomBuilder
import io.iohk.ethereum.utils.ByteStringUtils
import org.scalacheck.Arbitrary
import org.scalacheck.Arbitrary.arbitrary
import org.bouncycastle.crypto.params.ECPublicKeyParameters
Expand Down Expand Up @@ -35,6 +36,13 @@ class ECDSASignatureSpec extends AnyFlatSpec with Matchers with ScalaCheckProper
sig.publicKey(bytesToSign).isEmpty shouldBe true
}

it should "fail public key recover without throwing when signature is bad (Invalid point compression)" in {
val sig = ECDSASignature(ByteStringUtils.string2hash("149a2046f51f5d043633664d76eef4f99cdba8e53851dcda57224dfe8770f98a"), ByteStringUtils.string2hash("a8898478e9aae9fadb71c7ab5451d47d2efa4199fc26ecc1da62ce8fb77e06f1"), 28.toByte)
val messageHash = ByteStringUtils.string2hash("a1ede9cdf0b6fe37a384b265dce6b74a7464f11799dcee022f628450a19cf4eb")

sig.publicKey(messageHash).isEmpty shouldBe true
}

it should "sign message and recover public key" in {
forAll(arbitrary[Array[Byte]], Arbitrary.arbitrary[Unit].map(_ => generateKeyPair(secureRandom))) {
(message, keys) =>
Expand Down