From 389b48f851822114c42a2def1ba3aa4d1b630b19 Mon Sep 17 00:00:00 2001 From: Carl Beekhuizen Date: Thu, 24 Sep 2020 15:38:39 +0200 Subject: [PATCH 1/7] Adds parameter safety checks to crypto wrappers --- eth2deposit/utils/crypto.py | 10 ++++++++++ tests/test_utils/test_crypto.py | 35 ++++++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/eth2deposit/utils/crypto.py b/eth2deposit/utils/crypto.py index 9e93308e..c6881eaa 100644 --- a/eth2deposit/utils/crypto.py +++ b/eth2deposit/utils/crypto.py @@ -19,6 +19,8 @@ def SHA256(x: bytes) -> bytes: def scrypt(*, password: str, salt: str, n: int, r: int, p: int, dklen: int) -> bytes: + if n * r * p < 2**20: # 128 MB memory usage + raise ValueError("The Scrypt parameters chosen are not secure.") if n >= 2**(128 * r / 8): raise ValueError("The given `n` should be less than `2**(128 * r / 8)`." f"\tGot `n={n}`, r={r}, 2**(128 * r / 8)={2**(128 * r / 8)}") @@ -29,6 +31,14 @@ def scrypt(*, password: str, salt: str, n: int, r: int, p: int, dklen: int) -> b def PBKDF2(*, password: bytes, salt: bytes, dklen: int, c: int, prf: str) -> bytes: if 'sha' not in prf: raise ValueError(f"String 'sha' is not in `prf`({prf})") + if 'sha256' in prf and c < 2**18: + ''' + Verify the number of rounds of SHA256-PBKDF2. SHA512 bot checked as use in BIP39 + does not require and therefore doesn't use safe parameters (c=2048). + + Ref: https://github.com/bitcoin/bips/blob/master/bip-0039.mediawiki#from-mnemonic-to-seed + ''' + raise ValueError("The PBKDF2 parameters chosen are not secure.") _hash = _sha256 if 'sha256' in prf else _sha512 res = _PBKDF2(password=password, salt=salt, dkLen=dklen, count=c, hmac_hash_module=_hash) # type: ignore return res if isinstance(res, bytes) else res[0] # PyCryptodome can return Tuple[bytes] diff --git a/tests/test_utils/test_crypto.py b/tests/test_utils/test_crypto.py index 92f6ca2d..75612193 100644 --- a/tests/test_utils/test_crypto.py +++ b/tests/test_utils/test_crypto.py @@ -10,11 +10,12 @@ @pytest.mark.parametrize( 'n, r, valid', [ - (int(2**(128 * 1 / 8)) // 2, 1, True), - (int(2**(128 * 1 / 8)), 1, False), + (int(2**(128 * 1 / 8)) * 2, 8, True), + (int(2**(128 * 1 / 8)) * 1, 8, False), # Unsafe Parameters + (int(2**(128 * 1 / 8)) * 1, 1, False), # Invalid n ] ) -def test_scrypt_invalid_n(n, r, valid): +def test_scrypt_invalid_params(n, r, valid): if valid: scrypt( password="mypassword", @@ -63,6 +64,34 @@ def test_PBKDF2_invalid_prf(prf, valid): ) +@pytest.mark.parametrize( + 'count, prf, valid', + [ + (2**18, "sha256", True), + (2**17, "sha256", False), + (2**11, "sha512", True), + ] +) +def test_PBKDF2_invalid_count(count, prf, valid): + if valid: + PBKDF2( + password="mypassword", + salt="mysalt", + dklen=64, + c=count, + prf=prf + ) + else: + with pytest.raises(ValueError): + PBKDF2( + password="mypassword", + salt="mysalt", + dklen=64, + c=2048, + prf=prf, + ) + + @pytest.mark.parametrize( 'key, iv, valid', [ From 30097acbcf17657320bbb2707b759a1a0789b497 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 30 Sep 2020 10:00:59 +0800 Subject: [PATCH 2/7] Add Zinken setting --- eth2deposit/settings.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/eth2deposit/settings.py b/eth2deposit/settings.py index 7e98c364..aa834b7c 100644 --- a/eth2deposit/settings.py +++ b/eth2deposit/settings.py @@ -13,8 +13,10 @@ class BaseChainSetting(NamedTuple): AltonaSetting = BaseChainSetting(GENESIS_FORK_VERSION=bytes.fromhex('00000121')) # Eth2 "official" public testnet (spec v0.12.2) MedallaSetting = BaseChainSetting(GENESIS_FORK_VERSION=bytes.fromhex('00000001')) -# Eth2 "dress rehearsal_" testnet (spec v0.12.3) +# Eth2 "dress rehearsal" testnet (spec v0.12.3) SpadinaSetting = BaseChainSetting(GENESIS_FORK_VERSION=bytes.fromhex('00000002')) +# Eth2 "dress rehearsal" testnet (spec v0.12.3) +ZinkenSetting = BaseChainSetting(GENESIS_FORK_VERSION=bytes.fromhex('00000003')) MAINNET = 'mainnet' @@ -22,12 +24,14 @@ class BaseChainSetting(NamedTuple): ALTONA = 'altona' MEDALLA = 'medalla' SPADINA = 'spadina' +ZINKEN = 'zinken' ALL_CHAINS: Dict[str, BaseChainSetting] = { MAINNET: MainnetSetting, WITTI: WittiSetting, ALTONA: AltonaSetting, MEDALLA: MedallaSetting, SPADINA: SpadinaSetting, + ZINKEN: ZinkenSetting, } From 45e20931166038135242662e3293553099c6cd6c Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 1 Oct 2020 01:57:23 +0800 Subject: [PATCH 3/7] Bump py-ecc to v5.0.0 --- requirements.txt | 6 +++--- tests/test_key_handling/test_key_derivation/test_tree.py | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/requirements.txt b/requirements.txt index cf65e5fe..3fb95028 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ -py-ecc==4.0.0 \ - --hash=sha256:0712a1ebc2d45417088aa613f28518c1714c99d023998e50244c91e3acbb0d6c \ - --hash=sha256:a637edcce7e31ddefae0a3c1018f16e25c9428fcd524b1ac5ceeb2adfc433276 +py-ecc==5.0.0 \ + --hash=sha256:67a6b944722408c75bb630617dfbd8062c45b72d154ed3a6891c833717c87638 \ + --hash=sha256:9d3c7ba607ef36d7f8af9944d702799014b27fc77b385d14024f96f9f610ad0a pycryptodome==3.9.8 \ --hash=sha256:02e51e1d5828d58f154896ddfd003e2e7584869c275e5acbe290443575370fba \ --hash=sha256:03d5cca8618620f45fd40f827423f82b86b3a202c8d44108601b0f5f56b04299 \ diff --git a/tests/test_key_handling/test_key_derivation/test_tree.py b/tests/test_key_handling/test_key_derivation/test_tree.py index c74d4cbf..6a6bd1c8 100644 --- a/tests/test_key_handling/test_key_derivation/test_tree.py +++ b/tests/test_key_handling/test_key_derivation/test_tree.py @@ -17,7 +17,6 @@ test_vectors = json.load(f)['kdf_tests'] -@pytest.mark.skip(reason="py_ecc doesn't support BLS v4 yet") @pytest.mark.parametrize( 'test', test_vectors @@ -27,7 +26,6 @@ def test_hkdf_mod_r(test) -> None: assert bls.KeyGen(seed) == _HKDF_mod_r(IKM=seed) -@pytest.mark.skip(reason="py_ecc doesn't support BLS v4 yet") @pytest.mark.parametrize( 'seed', [b'\x00' * 32] From ed01530a82372ad8ba5e9057a3571fb5baefa20e Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 1 Oct 2020 02:17:10 +0800 Subject: [PATCH 4/7] Add deposit_cli_version to the deposit_data*.json file --- eth2deposit/credentials.py | 2 ++ eth2deposit/settings.py | 3 +++ 2 files changed, 5 insertions(+) diff --git a/eth2deposit/credentials.py b/eth2deposit/credentials.py index e8786479..d716e804 100644 --- a/eth2deposit/credentials.py +++ b/eth2deposit/credentials.py @@ -10,6 +10,7 @@ Keystore, ScryptKeystore, ) +from eth2deposit.settings import DEPOSIT_CLI_VERSION from eth2deposit.utils.constants import ( BLS_WITHDRAWAL_PREFIX, ETH2GWEI, @@ -90,6 +91,7 @@ def deposit_datum_dict(self) -> Dict[str, bytes]: datum_dict.update({'deposit_message_root': self.deposit_message.hash_tree_root}) datum_dict.update({'deposit_data_root': signed_deposit_datum.hash_tree_root}) datum_dict.update({'fork_version': self.fork_version}) + datum_dict.update({'deposit_cli_version': DEPOSIT_CLI_VERSION}) return datum_dict def signing_keystore(self, password: str) -> Keystore: diff --git a/eth2deposit/settings.py b/eth2deposit/settings.py index aa834b7c..4481fe3a 100644 --- a/eth2deposit/settings.py +++ b/eth2deposit/settings.py @@ -1,6 +1,9 @@ from typing import Dict, NamedTuple +DEPOSIT_CLI_VERSION = "0.4.0" + + class BaseChainSetting(NamedTuple): GENESIS_FORK_VERSION: bytes From 9d9dfd261af52ce7d29af76243d1ff07351f2959 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 1 Oct 2020 18:04:50 +0800 Subject: [PATCH 5/7] Use `pkg_resources` to update `DEPOSIT_CLI_VERSION` Co-authored-by: Carl Beekhuizen --- eth2deposit/settings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/eth2deposit/settings.py b/eth2deposit/settings.py index 4481fe3a..824f1524 100644 --- a/eth2deposit/settings.py +++ b/eth2deposit/settings.py @@ -1,7 +1,8 @@ from typing import Dict, NamedTuple +import pkg_resources -DEPOSIT_CLI_VERSION = "0.4.0" +DEPOSIT_CLI_VERSION = pkg_resources.require("eth2deposit")[0].version class BaseChainSetting(NamedTuple): From b3ab7552169a40e8658760c204390a20889afd67 Mon Sep 17 00:00:00 2001 From: Carl Beekhuizen Date: Thu, 1 Oct 2020 12:24:54 +0200 Subject: [PATCH 6/7] Fix comment typos --- eth2deposit/utils/crypto.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eth2deposit/utils/crypto.py b/eth2deposit/utils/crypto.py index c6881eaa..48a9d76a 100644 --- a/eth2deposit/utils/crypto.py +++ b/eth2deposit/utils/crypto.py @@ -33,8 +33,8 @@ def PBKDF2(*, password: bytes, salt: bytes, dklen: int, c: int, prf: str) -> byt raise ValueError(f"String 'sha' is not in `prf`({prf})") if 'sha256' in prf and c < 2**18: ''' - Verify the number of rounds of SHA256-PBKDF2. SHA512 bot checked as use in BIP39 - does not require and therefore doesn't use safe parameters (c=2048). + Verify the number of rounds of SHA256-PBKDF2. SHA512 not checked as use in BIP39 + does not require, and therefore doesn't use, safe parameters (c=2048). Ref: https://github.com/bitcoin/bips/blob/master/bip-0039.mediawiki#from-mnemonic-to-seed ''' From 6474d12c7ef978807db9a76ae4ed44231d83ec36 Mon Sep 17 00:00:00 2001 From: Carl Beekhuizen Date: Thu, 1 Oct 2020 13:05:24 +0200 Subject: [PATCH 7/7] bump version number --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index ed887113..edc1725b 100644 --- a/setup.py +++ b/setup.py @@ -6,7 +6,7 @@ setup( name="eth2deposit", - version='0.3.0', + version='0.4.0', py_modules=["eth2deposit"], packages=find_packages(exclude=('tests', 'docs')), python_requires=">=3.7,<4",