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

Rpc pubkey #3489

Merged
merged 1 commit into from
Mar 13, 2022
Merged

Rpc pubkey #3489

merged 1 commit into from
Mar 13, 2022

Conversation

mratsim
Copy link
Contributor

@mratsim mratsim commented Mar 12, 2022

This centralizes all crypto handling in crypto.nim instead of having it sprawled out.

In particular, for REST and RPC, pubkeys are supplied by users and due to lazy validation, those are unchecked (no check that they are infnity, on curve or in subgroup).
Fortunately:

  1. We require the endpoint to be exposed only to trusted services (either local or in a safe network)
  2. All codepaths in REST and RPC compares the pubkey with the known one from the Ethereum Beacon State, hence invalid keys cannot slip in.

Alternative: since RPC is scheduled for deprecation, we can also deleted the related files instead of merging this PR.

Regarding REST, validators pubkeys go through ValidatorIdent and/or node.restKeysCache and those are compared to the known validators key before use so no issue there:

let vindex =
block:
let vid = validator_id.get()
case vid.kind
of ValidatorQueryKind.Key:
let optIndices = keysToIndices(node.restKeysCache, stateData.data,
[vid.key])
if optIndices[0].isNone():
return RestApiResponse.jsonError(Http404, ValidatorNotFoundError)
optIndices[0].get()
of ValidatorQueryKind.Index:
let vres = vid.index.toValidatorIndex()
if vres.isErr():
case vres.error()
of ValidatorIndexError.TooHighValue:
return RestApiResponse.jsonError(Http400,
TooHighValidatorIndexValueError)
of ValidatorIndexError.UnsupportedValue:
return RestApiResponse.jsonError(Http500,
UnsupportedValidatorIndexValueError)
let index = vres.get()
if uint64(index) >= validatorsCount:
return RestApiResponse.jsonError(Http404, ValidatorNotFoundError)
index

Closes #1715

@mratsim mratsim changed the base branch from stable to unstable March 12, 2022 09:57
Copy link
Member

@arnetheduck arnetheduck left a comment

Choose a reason for hiding this comment

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

lgtm - debt of this kind has a tendency to spread, even for deprecated stuff

@github-actions
Copy link

Unit Test Results

     12 files  ±  0     829 suites  +8   26m 41s ⏱️ - 2m 30s
1 674 tests +  2  1 626 ✔️ +1    48 💤 +1  0 ±0 
9 775 runs  +16  9 663 ✔️ +8  112 💤 +8  0 ±0 

Results for commit 3351d3e. ± Comparison against base commit 13b264d.

@arnetheduck arnetheduck merged commit 9fd7305 into unstable Mar 13, 2022
@arnetheduck arnetheduck deleted the rpc-pubkey branch March 13, 2022 07:12
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.

[SEC] Observations on Point Validation
2 participants