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

Public key aggregation ambiguous infinite points handling #4111

Closed
veorq opened this issue Jun 24, 2021 · 2 comments · Fixed by #4372
Closed

Public key aggregation ambiguous infinite points handling #4111

veorq opened this issue Jun 24, 2021 · 2 comments · Fixed by #4372
Assignees

Comments

@veorq
Copy link

veorq commented Jun 24, 2021

BlstPublicKey.aggregate() will return infinitePublicKey in two cases:

  1. If one of the public keys to aggregate is invalid (not in the subgroup or infinity point)
  2. If the sum of (valid) public keys happens to be zero

https://github.com/ConsenSys/teku/blob/a0270026d4e70db13b0042eb057ffbdb5709e4ca/bls/src/main/java/tech/pegasys/teku/bls/impl/blst/BlstPublicKey.java#L56-L74

The two cases should be distinguished, so that the caller can handle them differently.

@benjaminion
Copy link
Contributor

Thank you for raising this.

While the distinction between the two cases is true, for all practical purposes (in Teku, at least) it makes no difference. In the astronomically unlikely event that an aggregate public key results in the point at infinity, it would be invalid for signature verification. This is because the standard's CoreVerify function (arguably) requires the secret key not to be zero. This is maybe an issue with the standard, but that's how it is.

In other applications, getting an infinite public key from aggregation likely suggests a rogue public key attack or similar, and granted that it might be useful to know about this.

I'll leave this open for a while for discussion, but we probably won't be making changes here unless/until we are doing some further work. However, if this is a blocker to another project, we can modify to return an optional, or throw an exception if invalid individual keys are present. Lmk.

@veorq
Copy link
Author

veorq commented Jun 24, 2021

Thanks @benjaminion for the quick and clear response. I agree with your evaluation of the risk.

I was less worried about the zero sum than about a single invalid key not being detected, as you suggest what about throwing an exception, as BlstSignature.aggregate() is doing in

https://github.com/ConsenSys/teku/blob/a0270026d4e70db13b0042eb057ffbdb5709e4ca/bls/src/main/java/tech/pegasys/teku/bls/impl/blst/BlstSignature.java#L71-L78

?

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 a pull request may close this issue.

2 participants