First draft of switching to Bitcoin::Secp256k1

cpanfile

@@ -8,6 +8,7 @@ requires 'Mooish::AttributeBuilder' => '1.001';
 requires 'Type::Tiny' => '2';
 requires 'List::Util' => '1.33';
 requires 'CryptX' => '0.074';
+requires 'Bitcoin::Secp256k1' => 0;
 requires 'Bitcoin::BIP39' => '0.002';
 requires 'namespace::clean' => '0.27';
 requires 'Try::Tiny' => 0;

lib/Bitcoin/Crypto/Helpers.pm

@@ -4,9 +4,9 @@ use v5.10;
 use strict;
 use warnings;
 use Exporter qw(import);
-use Crypt::PK::ECC;
 use Carp qw(carp);
 use MIME::Base64;
+use Bitcoin::Secp256k1;
 
 use Bitcoin::Crypto::Constants;
 use Bitcoin::Crypto::Exception;
@@ -29,6 +29,7 @@ our @EXPORT_OK = qw(
 	add_ec_points
 	carp_once
 	parse_formatdesc
+	ecc
 );
 
 our @CARP_NOT;
@@ -64,78 +65,6 @@ sub ensure_length
 	return pack("x$missing") . $packed;
 }
 
-# Self-contained implementation on elliptic curve points addition.
-# This is only a partial implementation, but should be good enough for key
-# derivation needs. Code borrowed from the archived Math::EllipticCurve::Prime
-# module. Returns undef for infinity points, expects to get a valid uncompressed
-# point data on input
-sub add_ec_points
-{
-	my ($point1, $point2) = @_;
-
-	my $curve_size = Bitcoin::Crypto::Constants::key_max_length;
-	my $curve_data = Crypt::PK::ECC->new->generate_key(Bitcoin::Crypto::Constants::curve_name)->curve2hash;
-	my $p = Math::BigInt->from_hex($curve_data->{prime});
-	my $a = Math::BigInt->from_hex($curve_data->{A});
-
-	my $add_points = sub {
-		my ($x1, $x2, $y1, $lambda) = @_;
-
-		my $x = $lambda->copy->bmodpow(2, $p);
-		$x->bsub($x1);
-		$x->bsub($x2);
-		$x->bmod($p);
-
-		my $y = $x1->copy->bsub($x);
-		$y->bmul($lambda);
-		$y->bsub($y1);
-		$y->bmod($p);
-
-		return {x => $x, y => $y};
-	};
-
-	my $double = sub {
-		my ($x, $y) = @_;
-		my $lambda = $x->copy->bmodpow(2, $p);
-		$lambda->bmul(3);
-		$lambda->badd($a);
-		my $bottom = $y->copy->bmul(2)->bmodinv($p);
-		$lambda->bmul($bottom)->bmod($p);
-
-		return $add_points->($x, $x, $y, $lambda);
-	};
-
-	my $format = "(a$curve_size)*";
-	my ($px1, $py1) = map { Math::BigInt->from_bytes($_) } unpack $format, substr $point1, 1;
-	my ($px2, $py2) = map { Math::BigInt->from_bytes($_) } unpack $format, substr $point2, 1;
-
-	my $ret = sub {
-		if ($px1->bcmp($px2)) {
-			my $lambda = $py2->copy->bsub($py1);
-			my $bottom = $px2->copy->bsub($px1)->bmodinv($p);
-			$lambda->bmul($bottom)->bmod($p);
-
-			return $add_points->($px1, $px2, $py1, $lambda);
-		}
-		elsif ($py1->is_zero || $py2->is_zero || $py1->bcmp($py2)) {
-			return undef;
-		}
-		else {
-			return $double->($px1, $py1);
-		}
-		}
-		->();
-
-	my $exp_x = $ret->{x}->to_bytes;
-	my $exp_y = $ret->{y}->to_bytes;
-
-	return defined $ret
-		? "\x04" .
-		ensure_length($exp_x, $curve_size) .
-		ensure_length($exp_y, $curve_size)
-		: undef;
-}
-
 # default operation is to decode based on formatdesc
 # passing $reverse makes it encode instead
 sub parse_formatdesc
@@ -165,6 +94,22 @@ sub parse_formatdesc
 	return $data;
 }
 
+sub ecc
+{
+	state $secp;
+	state $used_times = 'inf';
+
+	# define an arbitrary number of times a single secp256k1 context can be
+	# used. Create a new context after that. This gives an increased security
+	# according to libsecp256k1 documentation.
+	if ($used_times++ > 20) {
+		$secp = Bitcoin::Secp256k1->new;
+		$used_times = 0;
+	}
+
+	return $secp;
+}
+
 1;
 
 # Internal use only

lib/Bitcoin/Crypto/Key/ExtPrivate.pm

@@ -12,7 +12,7 @@ use Carp qw(carp);
 use Bitcoin::Crypto::BIP44;
 use Bitcoin::Crypto::Key::ExtPublic;
 use Bitcoin::Crypto::Constants;
-use Bitcoin::Crypto::Helpers qw(ensure_length);
+use Bitcoin::Crypto::Helpers qw(ensure_length ecc);
 use Bitcoin::Crypto::Util qw(mnemonic_to_seed);
 use Bitcoin::Crypto::Types -types;
 use Bitcoin::Crypto::Exception;
@@ -131,44 +131,37 @@ sub derive_key_bip44
 sub _derive_key_partial
 {
 	my ($self, $child_num, $hardened) = @_;
+	my $key = $self->raw_key;
 
 	my $hmac_data;
 	if ($hardened) {
 
-		# zero byte
-		$hmac_data .= "\x00";
-
-		# key data - 32 bytes
-		$hmac_data .= ensure_length $self->raw_key, Bitcoin::Crypto::Constants::key_max_length;
+		# zero byte + key data - 32 bytes
+		$hmac_data = "\x00" . $key;
 	}
 	else {
+
 		# public key data - SEC compressed form
-		$hmac_data .= $self->raw_key('public_compressed');
+		$hmac_data = $self->raw_key('public_compressed');
 	}
 
 	# child number - 4 bytes
 	$hmac_data .= ensure_length pack('N', $child_num), 4;
 
 	my $data = hmac('SHA512', $self->chain_code, $hmac_data);
+	my $tweak = substr $data, 0, 32;
 	my $chain_code = substr $data, 32, 32;
 
-	my $number = Math::BigInt->from_bytes(substr $data, 0, 32);
-	my $key_num = Math::BigInt->from_bytes($self->raw_key);
-	my $n_order = $self->curve_order;
-
-	Bitcoin::Crypto::Exception::KeyDerive->raise(
-		"key $child_num in sequence was found invalid"
-	) if $number->bge($n_order);
-
-	$number->badd($key_num);
-	$number->bmod($n_order);
-
-	Bitcoin::Crypto::Exception::KeyDerive->raise(
+	Bitcoin::Crypto::Exception::KeyDerive->trap_into(
+		sub {
+			$key = ecc->add_private_key($key, $tweak);
+			die 'verification failed' unless ecc->verify_private_key($key);
+		},
 		"key $child_num in sequence was found invalid"
-	) if $number->beq(0);
+	);
 
 	return $self->new(
-		key_instance => $number->as_bytes,
+		key_instance => $key,
 		chain_code => $chain_code,
 		child_number => $child_num,
 		parent_fingerprint => $self->get_fingerprint,

lib/Bitcoin/Crypto/Key/ExtPublic.pm

@@ -8,7 +8,7 @@ use Crypt::Mac::HMAC qw(hmac);
 use Types::Common -sigs, -types;
 
 use Bitcoin::Crypto::Constants;
-use Bitcoin::Crypto::Helpers qw(ensure_length add_ec_points);
+use Bitcoin::Crypto::Helpers qw(ensure_length ecc);
 use Bitcoin::Crypto::Exception;
 use Bitcoin::Crypto::BIP44;
 
@@ -44,31 +44,24 @@ sub _derive_key_partial
 	) if $hardened;
 
 	# public key data - SEC compressed form
-	my $hmac_data = $self->raw_key('public_compressed');
+	my $key = $self->raw_key('public_compressed');
 
-	# child number - 4 bytes
-	$hmac_data .= ensure_length pack('N', $child_num), 4;
+	# key + child number - 4 bytes
+	my $hmac_data = $key . ensure_length pack('N', $child_num), 4;
 
 	my $data = hmac('SHA512', $self->chain_code, $hmac_data);
+	my $tweak = substr $data, 0, 32;
 	my $chain_code = substr $data, 32, 32;
 
-	my $n_order = Math::BigInt->from_hex($self->key_instance->curve2hash->{order});
-	my $number = Math::BigInt->from_bytes(substr $data, 0, 32);
-	Bitcoin::Crypto::Exception::KeyDerive->raise(
-		"key $child_num in sequence was found invalid"
-	) if $number->bge($n_order);
-
-	my $key = $self->_create_key(substr $data, 0, 32);
-	my $point = $key->export_key_raw('public');
-	my $parent_point = $self->raw_key('public');
-	$point = add_ec_points($point, $parent_point);
-
-	Bitcoin::Crypto::Exception::KeyDerive->raise(
+	Bitcoin::Crypto::Exception::KeyDerive->trap_into(
+		sub {
+			$key = ecc->add_public_key($key, $tweak);
+		},
 		"key $child_num in sequence was found invalid"
-	) unless defined $point;
+	);
 
 	return $self->new(
-		key_instance => $point,
+		key_instance => $key,
 		chain_code => $chain_code,
 		child_number => $child_num,
 		parent_fingerprint => $self->get_fingerprint,

lib/Bitcoin/Crypto/Key/Private.pm

@@ -4,7 +4,6 @@ use v5.10;
 use strict;
 use warnings;
 use Moo;
-use Crypt::PK::ECC;
 use Bitcoin::BIP39 qw(bip39_mnemonic_to_entropy entropy_to_bip39_mnemonic);
 use Types::Common -sigs, -types;
 use List::Util qw(none);
@@ -269,13 +268,6 @@ function to fail. You can encode like this (for UTF-8):
 	use Encode qw(encode);
 	$message = encode('UTF-8', $message);
 
-Caution: libtomcrypt cryptographic package that is generating signatures does
-not currently offer a deterministic mechanism. For this reason the sign_message
-method will complain with a warning. You should install an optional
-L<Crypt::Perl> package, which supports deterministic signatures, which will
-disable the warning. Non-deterministic signatures can lead to leaking private
-keys if the random number generator's entropy is insufficient.
-
 =head2 sign_transaction
 
 	$object->sign_transaction($tx, %params)

lib/Bitcoin/Crypto/Manual/Transactions.pod

@@ -133,10 +133,8 @@ Contributions sorting out any of those are welcome!
 
 =item * Transaction malleability is not fully solved
 
-Strict DER signatures (BIP66) are not checked explicitly (CryptX may check them
-while verifying the signature though). Low S signatures mentioned in BIP62 are
-generated, but not verified. Some others like minimal push opcodes and
-OP_PUSHDATA 520 byte push limit are not implemented.
+Some details like minimal push opcodes and OP_PUSHDATA 520 byte push limit are
+not implemented.
 
 =item * legacy SIGHASH_SINGLE with more inputs than outputs