-
Notifications
You must be signed in to change notification settings - Fork 9
Crypto impact analysis #557
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a first look and you seem comfortable working with this way of sketching requirements like @nfrisby did. You are probably still working on it so maybe not required to ask:
- What concrete changes are needed? Do we need to implement anything new or is it just integration of a new library or maybe even new functions from an existing library?
- Maybe for the introduction/giving context: this seems to rely that BLS is the scheme we are going with, could be worthwhile to point out this (reasonable) assumption
- Anything to say about Peras?
Thanks for the review, much appreciated!
Correct, also wanted @curiecrypt to take a look at it before trying to merge it. But I agree with your question; those should be added.
I tried, but please let me know if something is not in line with this way of formalizing the requirements. I just followed suit ;) |
I have not tried it myself .. let's see if it sticks :) |
I just finished reading CIP in a bit more detail. I also read the impact analysis draft of yours @perturbing and I think it seems pretty mature. Although I might have some additions, especially for the Appendix, after talking with you. |
Proof-of-Possession creation and verification to mitigate rogue-key attacks. | ||
- *REQ-BlsAggregateSignatures*. | ||
Aggregate a list of public keys and signatures into one | ||
- *REQ-BlsBatchVerify*. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@perturbing do you think this will be used for generating an aggregate signature for a batch of persistent and non-persistent votes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the node will have two contexts/actions in which it will perform aggregation/batching:
- If the node is creating the certificates, it is aggregating the signatures of the set of persistent signers.
- If the node is verifying the certificate, it is aggregating the public keys and signatures of the persistent voters in the bitmap (see this question) and the non-persistent voters. Note that they sign over the same message, so the fastest verification is just by curve addition rather than repeated pairing calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed today, we may need a tag to distinguish between signatures generated by persistent and non-persistent voters. For example:
- Persistent voters: "PV || MSG"
- Non-persistent voters: "NV || MSG"
Even so, if we consider the fields of Leios certificate given in here we will aggregate the signatures generated by these parties separately. In this case, we should consider having the batch verification, which I think is not the highest priority at this time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for such separation?
docs/ImpactAnalysis.md
Outdated
- The current code on BLS12-381 already abstracts over both curves `G1`/`G2`, we should maintain this. | ||
- The `BLST` package also exposes fast verification over many messages and signatures + public keys by doing a combined pairing check. This might be helpful, though it's currently unclear if we can use this speedup. It might be the case, since we have *linear Leios, that this is never needed. | ||
|
||
# Appendix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- We should explain how the functional requirements map onto the primitives of the voting mechanism. An appendix section that describes these requirements in more detail, with references to the relevant parts of the CIP and specifications, would be valuable. For instance, domain separation is mentioned in REQ-BlsSignVerify, but its role within the Leios voting mechanism is not fully clear. An appendix clarifying such points would help the reader. Keeping these explanations in the appendix, while providing pointers in the requirements section when needed, would keep the main text simple yet informative.
- Another appendix section pointing out the risks and the potential mitigation strategies could be added.
- An explanation of the data flow with the rest of the code could be added. For example, while we already have serde functionality for the primitives, the way this data is transferred may raise additional considerations. We also need to account for the use of external data (e.g., keys, block hashes, etc.), which could have significant impacts on both Cardano Base and the cryptography component. It may not be possible to fully evaluate these impacts at this stage, but at minimum we should highlight the considerations and note their potential relevance for future analysis.
@perturbing I just finished my pass on the ecosystem parts #560 You might want to have a look how I tried to do that requirement identification ( REQ-... items) while estimating impacted components there. Furthermore, REQ-GenerateBLSKeys etc. in the "Impact on operations" section seem connected to the cryptography chapter. Maybe I should keep it more abstract (not mention BLS keys too much, but rather that we need to allow SPOs deal with some new keys) |
e081087
to
9582874
Compare
9582874
to
9fafe16
Compare
I added a bit to the intro addressing where the expected code changes will happen, thanks.
I saw that, and I think that your section is enough for now. The crypto impact is not so much about this registration and rotation, so I'm glad you added it in the ecosystem section. I think this might be for the |
Oops, I accidentally closed it ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good list of things, I'll incorporate it with the rest and follow up some editing / consistent language in another PR.
Hi,
Here is an initial attempt at the impact analysis of Leios on the cryptography of the node. As an addition to #555
@curiecrypt @ch1bo