Skip to content
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

SecretKey -> PublicKey preconditions docs + fixes. #115

Merged
merged 5 commits into from
Jul 3, 2021

Conversation

mratsim
Copy link
Contributor

@mratsim mratsim commented Jul 3, 2021

This fixes #114 and status-im/nimbus-eth2#2695

Overview

In BLSCurve, preconditions checks for:

  • SecretKey: 0 < SK < r (BLS12-381 curve order)
  • PublicKey: PK != infinity and PK in G1 subgroup

are done at deserialization time.

func fromBytes*(
obj: var SecretKey,
raw: openarray[byte] or array[32, byte]
): bool {.inline.} =
## Initialize a BLS secret key from
## its raw bytes representation.
## Returns true on success and false otherwise
const L = 32
when raw is array:
obj.scalar.blst_scalar_from_bendian(raw)
else:
if raw.len != 32:
return false
let pa = cast[ptr array[L, byte]](raw[0].unsafeAddr)
obj.scalar.blst_scalar_from_bendian(pa[])
if obj.vec_is_zero():
return false
if not obj.scalar.blst_sk_check().bool:
return false
return true

func fromBytes*(
obj: var PublicKey,
raw: array[48, byte] or array[96, byte]
): bool {.inline.} =
## Initialize a BLS signature scheme object from
## its raw bytes representation.
## Returns true on success and false otherwise
result =
when raw.len == 48:
obj.point.blst_p1_uncompress(raw) == BLST_SUCCESS
elif raw.len == 96:
obj.point.blst_p1_deserialize(raw) == BLST_SUCCESS
else: false
# Infinity public keys are not allowed
if result:
result = not bool obj.point.blst_p1_affine_is_inf()
if result:
result = bool obj.point.blst_p1_affine_in_g1()

This means that procedures that rely on those types, like publicFromSecret can assume that those preconditions are already respected.
This has been clarified in a dedicated document https://github.com/status-im/nim-blscurve/blob/9b7366cdc7df14c87930c3ed0243ab551a5daf56/docs/bls_types_guarantees.md

Tests have been added to ensure that #114 was just a documentation issue and revealed that the MIRACL backend (not used in production) could actually deserialize an invalid secret key > r.

Furthermore, for defensive programming reason, an extra precondition check in publicFromSecret has been added to recheck that the input respects 0 < SK < r. That check should never fail.

@@ -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`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can probably slap a requiresInit on it to catch any zero-initialized stragglers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Insufficient private key validation (from nimbus-eth2)
2 participants