-
Notifications
You must be signed in to change notification settings - Fork 244
Secp256k1 in parity crypto and alternative wasm32 implementation #80
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.
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
Changes from 31 commits
6cf6ec6
07b3afc
e6a4d8a
0cc2661
c20f544
c19caa5
2da14e4
41d4c78
0083acf
a502a11
ccb7f0a
f606daa
5b2aef6
c57fb09
0ec49ac
85d2f80
58c9e72
41481dd
4b72742
1c5b4b1
3ff4e30
37a38dc
85707f9
4aed419
6ab9fb9
a976b47
e41feab
a24dbd0
9445702
6b5e6fa
30d9198
c91370b
9968c73
0117a5f
ac0eb93
aa0bdca
ed370cc
6a6a111
7a3d75d
abb0134
862533f
351b964
82b04ac
3c36377
bb81eb3
ec212b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,5 +17,5 @@ members = [ | |
| "trace-time", | ||
| "trie-standardmap", | ||
| "triehash", | ||
| "uint" | ||
| "uint", | ||
| ] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| [package] | ||
| name = "mem" | ||
| version = "0.1.0" | ||
| authors = ["Parity Technologies <admin@parity.io>"] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The crate was copied from https://github.com/paritytech/parity-ethereum/tree/master/util/mem so staying with gpl 3 license.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. Just make sure you add the required fields to the |
||
|
|
||
| [dependencies] | ||
| clear_on_drop = "0.2.3" | ||
|
cheme marked this conversation as resolved.
Outdated
|
||
|
|
||
| [features] | ||
| # when activated mem is removed through volatile primitive instead of clear_on_drop crate | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably go into the README/module docs
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure about 'README/module', just added a README file to mem.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's fine, I just meant "make sure this info is in either the README or in the module-level docs (or both)". |
||
| volatile-erase = [] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| // Copyright 2015-2018 Parity Technologies (UK) Ltd. | ||
| // This file is part of Parity. | ||
|
|
||
| // Parity is free software: you can redistribute it and/or modify | ||
| // it under the terms of the GNU General Public License as published by | ||
| // the Free Software Foundation, either version 3 of the License, or | ||
| // (at your option) any later version. | ||
|
|
||
| // Parity is distributed in the hope that it will be useful, | ||
| // but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| // GNU General Public License for more details. | ||
|
|
||
| // You should have received a copy of the GNU General Public License | ||
| // along with Parity. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
|
cheme marked this conversation as resolved.
Outdated
|
||
| extern crate clear_on_drop as cod; | ||
|
|
||
| use std::ops::{Deref, DerefMut}; | ||
|
|
||
| #[cfg(feature = "volatile-erase")] | ||
| use std::ptr; | ||
|
|
||
| #[cfg(not(feature = "volatile-erase"))] | ||
| pub use cod::clear::Clear; | ||
|
|
||
| /// reexport clear_on_drop crate | ||
| pub mod clear_on_drop { | ||
| pub use cod::*; | ||
| } | ||
|
|
||
| /// Wrapper to zero out memory when dropped. | ||
| #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] | ||
| pub struct Memzero<T: AsMut<[u8]>> { | ||
| mem: T, | ||
| } | ||
|
|
||
| impl<T: AsMut<[u8]>> From<T> for Memzero<T> { | ||
| fn from(mem: T) -> Memzero<T> { | ||
| Memzero { mem } | ||
| } | ||
| } | ||
|
|
||
| #[cfg(feature = "volatile-erase")] | ||
| impl<T: AsMut<[u8]>> Drop for Memzero<T> { | ||
| fn drop(&mut self) { | ||
| unsafe { | ||
| for byte_ref in self.mem.as_mut() { | ||
| ptr::write_volatile(byte_ref, 0) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[cfg(not(feature = "volatile-erase"))] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When should
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did use 'clear_on_drop' in my implementation (it is a bit more flexible api-wise) in other contexts and feel like using a single erasure mechanism was simpler.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think in this case "less is more". I think we should have a single way of doing memory zeroing. |
||
| impl<T: AsMut<[u8]>> Drop for Memzero<T> { | ||
| fn drop(&mut self) { | ||
| self.as_mut().clear(); | ||
| } | ||
| } | ||
|
|
||
| impl<T: AsMut<[u8]>> Deref for Memzero<T> { | ||
| type Target = T; | ||
|
|
||
| fn deref(&self) -> &Self::Target { | ||
| &self.mem | ||
| } | ||
| } | ||
|
|
||
| impl<T: AsMut<[u8]>> DerefMut for Memzero<T> { | ||
| fn deref_mut(&mut self) -> &mut Self::Target { | ||
| &mut self.mem | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,9 +5,53 @@ authors = ["Parity Technologies <admin@parity.io>"] | |
| repository = "https://github.com/paritytech/parity-common" | ||
| description = "Crypto utils used by ethstore and network." | ||
| license = "GPL-3.0" | ||
| autobenches = false | ||
|
|
||
| [[bench]] | ||
| name = "bench" | ||
| harness = false | ||
|
|
||
|
|
||
| [dependencies] | ||
| quick-error = "1.2.2" | ||
| ring = "0.13" | ||
| rust-crypto = "0.2.36" | ||
|
|
||
|
cheme marked this conversation as resolved.
Outdated
|
||
| tiny-keccak = "1.4" | ||
| scrypt = { version = "0.1.1", default-features = false } | ||
| ripemd160 = "0.8.0" | ||
| sha2 = "0.8.0" | ||
| digest = "0.8" | ||
| aes = "0.3.2" | ||
| aes-ctr = "0.3.0" | ||
| block-modes = "0.2.0" | ||
| lazy_static = "1.0" # for secp | ||
| mem = { path = "../mem" } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fine for now but needs a
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not totally sure that I want to get this crate published (there is really not a lot of content in it), currently on my branch for testing this in parity-ethereum I am using a crate reexport from crypto.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In that case I think it's best to make it a sub-crate to I'd prefer the top-level crates in |
||
|
|
||
| [target.'cfg(not(target_arch = "wasm32"))'.dependencies] | ||
| ring = "0.13" | ||
| eth-secp256k1 = { git = "https://github.com/paritytech/rust-secp256k1" } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is two missing operations here:
I applied Arkadiy modif for both on upstream, to test : With my fork on upstream rust-secp I got those nice results: So I believe we should try to merge it. @arkpar did you know if we already try in the past? |
||
| arrayvec = "0.4" # needed for eth-secp256k1 | ||
| rand = "0.4" | ||
| hmac = { version = "0.7", optional = true } | ||
| libsecp256k1 = { path = "./libsecp256k1", optional = true } | ||
| pbkdf2 = { version = "0.3", default-features = false, optional = true } | ||
|
|
||
| [target.'cfg(target_arch = "wasm32")'.dependencies] | ||
| hmac = "0.7" | ||
| subtle = { version = "1.0" } | ||
| # libsecp256k1 = { version = "0.1", package = "libsecp256k1" } // requires 'rename-dependency' in stable | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you remind me what this is about?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It allows changing lib name but before import, thus allowing to import two crate with the same declared name (in the commented line I rename sorpaas crate from 'secp256k1' to 'libsecp256k1' but it is not really explicit). It is a really nice feature, to manage crates with same name, and with rust 2018 I think it could be use to avoid using 'extern crate xxxx as yyy;' (no need for 'extern crate' in 2018). |
||
| libsecp256k1 = { path = "./libsecp256k1" } | ||
| rand = "0.5" | ||
| pbkdf2 = { version = "0.3", default-features = false } | ||
|
|
||
| [dev-dependencies] | ||
| hmac = "0.7" | ||
| # libsecp256k1 = { version = "0.1", package = "libsecp256k1" } // requires 'rename-dependency' in stable | ||
| libsecp256k1 = { path = "./libsecp256k1" } | ||
| pbkdf2 = { version = "0.3", default-features = false } | ||
| criterion = "0.2" | ||
|
|
||
| [features] | ||
| # expose alternative wasm32 implementation for testing and benchmarks | ||
| alt = ["hmac", "libsecp256k1", "pbkdf2"] | ||
| nightly = ["subtle/nightly"] | ||
| volatile-erase = ["mem/volatile-erase"] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| // Copyright 2015-2018 Parity Technologies (UK) Ltd. | ||
| // This file is part of Parity. | ||
|
|
||
| // Parity is free software: you can redistribute it and/or modify | ||
| // it under the terms of the GNU General Public License as published by | ||
| // the Free Software Foundation, either version 3 of the License, or | ||
| // (at your option) any later version. | ||
|
|
||
| // Parity is distributed in the hope that it will be useful, | ||
| // but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| // GNU General Public License for more details. | ||
|
|
||
| // You should have received a copy of the GNU General Public License | ||
| // along with Parity. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A few lines of introduction here would be nice. |
||
| use criterion::{Criterion, Bencher}; | ||
|
|
||
| use parity_crypto::traits::asym::*; | ||
|
|
||
| pub fn secp256k1(c: &mut Criterion) { | ||
| use parity_crypto::secp256k1::Secp256k1; | ||
| asym_bench::<Secp256k1>(c, "secp256k1".to_owned()) | ||
| } | ||
|
|
||
| #[cfg(feature="alt")] | ||
| pub fn secp256k1_alt(c: &mut Criterion) { | ||
| use parity_crypto::secp256k1_alt::Secp256k1; | ||
| asym_bench::<Secp256k1>(c, "secp256k1_alt".to_owned()) | ||
| } | ||
|
|
||
|
|
||
|
|
||
| fn asym_bench<A: Asym>(c: &mut Criterion, name: String) { | ||
|
|
||
| c.bench_function(&(name.clone() + "_sign_verify"), | ||
| |b: &mut Bencher| { | ||
| let mut sec_buf = vec![7; A::SECRET_SIZE]; | ||
| let message = vec![0;32]; | ||
| b.iter(|| { | ||
| let (secret, public) = A::keypair_from_slice(&mut sec_buf).unwrap(); | ||
| let signature = secret.sign(&message).unwrap(); | ||
| assert!(public.verify(&signature, &message).unwrap()); | ||
| }); | ||
| } | ||
| ); | ||
|
|
||
| c.bench_function(&(name.clone() + "_sign_recover"), | ||
| |b: &mut Bencher| { | ||
| let mut sec_buf = vec![3; A::SECRET_SIZE]; | ||
| let message = vec![0;32]; | ||
| b.iter(|| { | ||
| let (secret, public) = A::keypair_from_slice(&mut sec_buf).unwrap(); | ||
| let signature = secret.sign(&message).unwrap(); | ||
| assert!(public == A::recover(&signature, &message).unwrap()); | ||
| }); | ||
| } | ||
| ); | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| // Copyright 2015-2018 Parity Technologies (UK) Ltd. | ||
| // This file is part of Parity. | ||
|
|
||
| // Parity is free software: you can redistribute it and/or modify | ||
| // it under the terms of the GNU General Public License as published by | ||
| // the Free Software Foundation, either version 3 of the License, or | ||
| // (at your option) any later version. | ||
|
|
||
| // Parity is distributed in the hope that it will be useful, | ||
| // but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| // GNU General Public License for more details. | ||
|
|
||
| // You should have received a copy of the GNU General Public License | ||
| // along with Parity. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
|
|
||
| extern crate parity_crypto; | ||
|
|
||
| #[macro_use] | ||
| extern crate criterion; | ||
|
|
||
| mod asym; | ||
|
|
||
| use criterion::{Criterion, Bencher}; | ||
|
|
||
| #[cfg(not(feature="alt"))] | ||
| criterion_group!(benches, input_len, asym::secp256k1); | ||
|
|
||
| #[cfg(feature="alt")] | ||
| criterion_group!(benches, input_len, asym::secp256k1, asym::secp256k1_alt); | ||
|
|
||
| criterion_main!(benches); | ||
|
|
||
| /// general benches for multiple input size | ||
| fn input_len(c: &mut Criterion) { | ||
|
|
||
| c.bench_function_over_inputs("ripemd", | ||
| |b: &mut Bencher, size: &usize| { | ||
| let data = vec![0u8; *size]; | ||
| b.iter(|| parity_crypto::digest::ripemd160(&data[..])); | ||
| }, | ||
| vec![100, 500, 1_000, 10_000, 100_000] | ||
| ); | ||
|
|
||
| c.bench_function_over_inputs("aes_ctr", | ||
| |b: &mut Bencher, size: &usize| { | ||
| let data = vec![0u8; *size]; | ||
| let mut dest = vec![0; *size]; | ||
| let k = [0; 16]; | ||
| let iv = [0; 16]; | ||
|
|
||
| b.iter(||{ | ||
| parity_crypto::aes::encrypt_128_ctr(&k[..], &iv[..], &data[..], &mut dest[..]).unwrap(); | ||
| // same as encrypt but add it just in case | ||
| parity_crypto::aes::decrypt_128_ctr(&k[..], &iv[..], &data[..], &mut dest[..]).unwrap(); | ||
| }); | ||
| }, | ||
| vec![100, 500, 1_000, 10_000, 100_000] | ||
| ); | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| [package] | ||
| name = "libsecp256k1" | ||
| version = "0.0.1" | ||
| authors = ["Parity Technologies <admin@parity.io>"] | ||
| repository = "https://github.com/paritytech/parity-common" | ||
| description = "Renaming crate for libsecp256k1." | ||
| license = "GPL-3.0" | ||
|
|
||
| [dependencies] | ||
| libsecp256k1 = { version = "0.1" } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| // Copyright 2015-2018 Parity Technologies (UK) Ltd. | ||
| // This file is part of Parity. | ||
|
|
||
| // Parity is free software: you can redistribute it and/or modify | ||
| // it under the terms of the GNU General Public License as published by | ||
| // the Free Software Foundation, either version 3 of the License, or | ||
| // (at your option) any later version. | ||
|
|
||
| // Parity is distributed in the hope that it will be useful, | ||
| // but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| // GNU General Public License for more details. | ||
|
|
||
| // You should have received a copy of the GNU General Public License | ||
| // along with Parity. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
| //! Renaming crate for libsecp256k1, this crate should be drop as soon as | ||
| //! 'renaming-crates' functionality makes it to stable. | ||
|
|
||
|
|
||
| extern crate secp256k1; | ||
|
|
||
| pub use secp256k1::*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This name is taken (https://github.com/vifino/rust-mem) so we need something different.
memzero?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switching to "parity-util-mem"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, not sure I love that name, sounds a bit like a pathname! How about, as mentioned above, you move it to a
parity-cryptosubcrate and so we publish it as part of that? If you don't likememzero, then justmem-utilsseems like it could work.