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

Remove ed-25519-java move to Bouncy Castle #118

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Korbik
Copy link
Contributor

@Korbik Korbik commented Mar 30, 2025

* Remove ed-25519-java because of the security issue
* Split the public keys representation to avoid the use of the
  SecurityProvide and heavy exception
@Korbik Korbik requested review from divarvel and Geal April 1, 2025 05:13
@@ -157,7 +156,7 @@ private static SerializedBiscuit deserialize(Schema.Biscuit data)
external));
}

if (!(data.getProof().hasNextSecret() ^ data.getProof().hasFinalSignature())) {
if (data.getProof().hasNextSecret() == data.getProof().hasFinalSignature()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we should not allw the case where there's both a next secret and a final signature. The previous XOR was confusing but right.
Maybe split the two conditions? "empty proof" where both are false, and "invalid proof" where both are true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Auto fix from the IDE. The result is the same than before but we have a case we shouldn’t have.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Splitting the condition is the clearer solution.

Copy link
Contributor

@Geal Geal left a comment

Choose a reason for hiding this comment

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

everything looks good, there's just the bit about the proof format check to fix, and that will be good to go

Korbik added 2 commits April 8, 2025 14:54
* Empty proof: no flag of the proof is set
* Invalid proof: both flags of the proof are set (impossible case)
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.

2 participants