From a248885f23c4062307af0ca85d8a396ffe205bf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Sat, 3 Jul 2021 10:23:10 +0200 Subject: [PATCH 1/5] Add test to confirm that the SecretKey type implies < curve order (https://github.com/status-im/nimbus-eth2/issues/2693, https://github.com/status-im/nim-blscurve/issues/114) --- tests/priv_to_pub.nim | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/priv_to_pub.nim b/tests/priv_to_pub.nim index 4dcc9b5..ea7f153 100644 --- a/tests/priv_to_pub.nim +++ b/tests/priv_to_pub.nim @@ -79,3 +79,12 @@ test_sk_to_pk( seckey = "47faea55fe00a78306449165c017c9db86411a4c2467b4b89e21323c746406a0", pubkey = "a18e29d0185a5a6d19edf052ae098fd2924f579b6dfb4905332b8f4fc78adeb3188ad8315bf279a144be026ac08f3441" ) + +# Ensure that secret keys with key > BLS12-381 curve order cannot be deserialized +# BLS12-381 curve order is 0x73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001 + +block: + var sk{.noInit.}: SecretKey + doAssert not sk.fromHex("0x73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001") + doAssert not sk.fromHex("0x73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000002") + echo "SUCCESS - secret keys > curve order are refused" From 408220c656bef6544b2d57bb33769a05eba2e2f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Sat, 3 Jul 2021 10:28:35 +0200 Subject: [PATCH 2/5] fix MIRACL backend --- blscurve/miracl/bls_sig_io.nim | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/blscurve/miracl/bls_sig_io.nim b/blscurve/miracl/bls_sig_io.nim index ef4d094..15f96a3 100644 --- a/blscurve/miracl/bls_sig_io.nim +++ b/blscurve/miracl/bls_sig_io.nim @@ -23,11 +23,14 @@ func fromHex*[T: SecretKey|PublicKey|Signature|ProofOfPossession]( ## its hex raw bytes representation. ## Returns true on a success and false otherwise ## For secret key deserialization - ## A zero key is invalid + ## The key must be 0 < key < CURVE_Order when obj is SecretKey: result = obj.intVal.fromHex(hexStr) if obj.intVal.isZilch(): return false + {.noSideEffect.}: + if obj.intVal.cmp(CURVE_Order) != -1: + return false else: result = obj.point.fromHex(hexStr) when obj is PublicKey: @@ -50,6 +53,9 @@ func fromBytes*[T: SecretKey|PublicKey|Signature|ProofOfPossession]( result = obj.intVal.fromBytes(raw) if obj.intVal.isZilch(): return false + {.noSideEffect.}: + if obj.intVal.cmp(CURVE_Order) != -1: + return false else: result = obj.point.fromBytes(raw) when obj is PublicKey: From 794e1f10213638364627bfd8735a81b2da0626ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Sat, 3 Jul 2021 10:34:42 +0200 Subject: [PATCH 3/5] [Doc] clarify that `SecretKey` type are always instantiated < BLS12-381 curve order. --- blscurve/blst/blst_min_pubkey_sig_core.nim | 9 +++++++++ blscurve/miracl/miracl_min_pubkey_sig_core.nim | 12 ++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/blscurve/blst/blst_min_pubkey_sig_core.nim b/blscurve/blst/blst_min_pubkey_sig_core.nim index 85ca630..50aa8c2 100644 --- a/blscurve/blst/blst_min_pubkey_sig_core.nim +++ b/blscurve/blst/blst_min_pubkey_sig_core.nim @@ -60,6 +60,10 @@ type ## Long-term storage of this key also requires adequate protection. ## ## At the moment, the nim-blscurve library does not guarantee such protections + ## + ## Guarantees: + ## - SecretKeys are always created (via hkdf_mod_r) or deserialized (via `fromBytes`) + ## so that SK < BLS12-381 curve order. scalar: blst_scalar PublicKey* = object @@ -124,6 +128,11 @@ func publicFromSecret*(pubkey: var PublicKey, seckey: SecretKey): bool = ## This requires some -O3 compiler optimizations to be off ## as such {.passC: "-fno-tree-vectorize".} ## is automatically added to the compiler flags in blst_lowlevel + ## + ## Assumptions: + ## - On creation or deserialization of the `SecretKey` type + ## there was a check to ensure that SK < CurveOrder. + ## see `fromBytes` and `blst_sk_check` if seckey.vec_is_zero(): return false var pk {.noInit.}: blst_p1 diff --git a/blscurve/miracl/miracl_min_pubkey_sig_core.nim b/blscurve/miracl/miracl_min_pubkey_sig_core.nim index 153473f..877caa2 100644 --- a/blscurve/miracl/miracl_min_pubkey_sig_core.nim +++ b/blscurve/miracl/miracl_min_pubkey_sig_core.nim @@ -60,6 +60,10 @@ type ## Long-term storage of this key also requires adequate protection. ## ## At the moment, the nim-blscurve library does not guarantee such protections + ## + ## Guarantees: + ## - SecretKeys are always created (via hkdf_mod_r) or deserialized (via `fromBytes`) + ## so that SK < BLS12-381 curve order intVal: BIG_384 PublicKey* = object @@ -116,13 +120,17 @@ func publicFromSecret*(pubkey: var PublicKey, seckey: SecretKey): bool = ## Side-channel/Constant-time considerations: ## The SK content is not revealed unless its value ## is exactly 0 + ## + ## Assumptions: + ## - On creation or deserialization of the `SecretKey` type + ## there was a check to ensure that SK < CurveOrder. + ## see `fromBytes`/`fromHex`. # # Procedure: # 1. xP = SK * P # 2. PK = point_to_pubkey(xP) # 3. return PK - - + # # Always != 0: # keyGen, deriveChild_secretKey, fromHex, fromBytes guarantee that. if seckey.intVal.isZilch(): From 9b7366cdc7df14c87930c3ed0243ab551a5daf56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Sat, 3 Jul 2021 10:57:19 +0200 Subject: [PATCH 4/5] Document the SecretKey -> PublicKey validation flow and guarantees. https://github.com/status-im/nim-blscurve/issues/114, https://github.com/status-im/nimbus-eth2/issues/2693 --- blscurve/blst/blst_min_pubkey_sig_core.nim | 11 +++-- .../miracl/miracl_min_pubkey_sig_core.nim | 13 +++--- docs/bls_types_guarantees.md | 42 +++++++++++++++++++ 3 files changed, 56 insertions(+), 10 deletions(-) create mode 100644 docs/bls_types_guarantees.md diff --git a/blscurve/blst/blst_min_pubkey_sig_core.nim b/blscurve/blst/blst_min_pubkey_sig_core.nim index 50aa8c2..63c30f5 100644 --- a/blscurve/blst/blst_min_pubkey_sig_core.nim +++ b/blscurve/blst/blst_min_pubkey_sig_core.nim @@ -129,12 +129,15 @@ func publicFromSecret*(pubkey: var PublicKey, seckey: SecretKey): bool = ## as such {.passC: "-fno-tree-vectorize".} ## is automatically added to the compiler flags in blst_lowlevel ## - ## Assumptions: - ## - On creation or deserialization of the `SecretKey` type - ## there was a check to ensure that SK < CurveOrder. - ## see `fromBytes` and `blst_sk_check` + ## Returns: + ## - false is secret key is invalid (SK == 0 or >= BLS12-381 curve order), + ## true otherwise + ## By construction no public API should ever instantiate + ## an invalid secretkey in the first place. if seckey.vec_is_zero(): return false + if not obj.scalar.blst_sk_check().bool: + return false var pk {.noInit.}: blst_p1 pk.blst_sk_to_pk_in_g1(seckey.scalar) pubkey.point.blst_p1_to_affine(pk) diff --git a/blscurve/miracl/miracl_min_pubkey_sig_core.nim b/blscurve/miracl/miracl_min_pubkey_sig_core.nim index 877caa2..26bc019 100644 --- a/blscurve/miracl/miracl_min_pubkey_sig_core.nim +++ b/blscurve/miracl/miracl_min_pubkey_sig_core.nim @@ -115,16 +115,14 @@ func publicFromSecret*(pubkey: var PublicKey, seckey: SecretKey): bool = ## - PK, a public key encoded as an octet string. ## ## Returns: - ## - false is secret key is invalid (SK == 0), true otherwise + ## - false is secret key is invalid (SK == 0 or >= BLS12-381 curve order), + ## true otherwise + ## By construction no public API should ever instantiate + ## an invalid secretkey in the first place. ## ## Side-channel/Constant-time considerations: ## The SK content is not revealed unless its value ## is exactly 0 - ## - ## Assumptions: - ## - On creation or deserialization of the `SecretKey` type - ## there was a check to ensure that SK < CurveOrder. - ## see `fromBytes`/`fromHex`. # # Procedure: # 1. xP = SK * P @@ -135,6 +133,9 @@ func publicFromSecret*(pubkey: var PublicKey, seckey: SecretKey): bool = # keyGen, deriveChild_secretKey, fromHex, fromBytes guarantee that. if seckey.intVal.isZilch(): return false + {.noSideEffect.}: + if obj.intVal.cmp(CURVE_Order) != -1: + return false pubkey.point = generator1() pubkey.point.mul(secKey.intVal) return true diff --git a/docs/bls_types_guarantees.md b/docs/bls_types_guarantees.md new file mode 100644 index 0000000..21b1e46 --- /dev/null +++ b/docs/bls_types_guarantees.md @@ -0,0 +1,42 @@ +# BLS types guarantees + +This document outlines the assumptions when working with BLS curves types. + +## Secret Keys / private keys. + +Spec: 0 < SK < r (BLS12-381 curve order) + +**On creation via:** +- Lamport key (EIP2333): `derive_master_secretKey` or `derive_child_secretKey` +- `keyGen` + +An internal procedure `hkdf_mod_r` guarantees that the secret keys is below the BLS12-381 curve order + +**On deserialization via:** +- `fromBytes` and `fromHex` + +The input is checked for `0 < SK < r` with both BLST and Miracl backend. + +There is no other public API to create a SecretKey instance. + +## Public keys + +Spec: PK != infinity point & PK in G1 subgroup + +**On creation via:** +- `publicFromSecret` + +Given a secret key `0 < SK < r`, and the generator point G, we have the scalar multiplication \[SK\]G != infinity and in the G1 subgroup. + +Note: `publicFromSecret` rechecks the `0 < SK < r` guaranteed by the `SecretKey` type internally. + +**On deserialization via:** +- `fromBytes` and `fromHex` + +The input is checked against both conditions with both BLST and MIRACL backend. +- Procedures in `bls_sig_io.nim`: + - `fromBytes` and `fromHex` +- BLST: `blst_p1_affine_is_inf` and subgroup checks `blst_p1_affine_in_g1` +- MIRACL: `isInf` and `subgroupCheck` + +There is no other public API to create a PublicKey instance. From 6a4c55f501d4670bafd405202a0240c500534230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Sat, 3 Jul 2021 11:43:25 +0200 Subject: [PATCH 5/5] typo --- blscurve/blst/blst_min_pubkey_sig_core.nim | 2 +- blscurve/miracl/miracl_min_pubkey_sig_core.nim | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/blscurve/blst/blst_min_pubkey_sig_core.nim b/blscurve/blst/blst_min_pubkey_sig_core.nim index 63c30f5..9464cda 100644 --- a/blscurve/blst/blst_min_pubkey_sig_core.nim +++ b/blscurve/blst/blst_min_pubkey_sig_core.nim @@ -136,7 +136,7 @@ func publicFromSecret*(pubkey: var PublicKey, seckey: SecretKey): bool = ## an invalid secretkey in the first place. if seckey.vec_is_zero(): return false - if not obj.scalar.blst_sk_check().bool: + if not seckey.scalar.blst_sk_check().bool: return false var pk {.noInit.}: blst_p1 pk.blst_sk_to_pk_in_g1(seckey.scalar) diff --git a/blscurve/miracl/miracl_min_pubkey_sig_core.nim b/blscurve/miracl/miracl_min_pubkey_sig_core.nim index 26bc019..3a770c2 100644 --- a/blscurve/miracl/miracl_min_pubkey_sig_core.nim +++ b/blscurve/miracl/miracl_min_pubkey_sig_core.nim @@ -134,7 +134,7 @@ func publicFromSecret*(pubkey: var PublicKey, seckey: SecretKey): bool = if seckey.intVal.isZilch(): return false {.noSideEffect.}: - if obj.intVal.cmp(CURVE_Order) != -1: + if seckey.intVal.cmp(CURVE_Order) != -1: return false pubkey.point = generator1() pubkey.point.mul(secKey.intVal)