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

Fixed ECDH for curve25519 so it uses X25519 #1386

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

hbs
Copy link
Collaborator

@hbs hbs commented Dec 6, 2024

No description provided.

@hbs hbs requested review from randomboolean, stggn and pi-r-p December 6, 2024 14:15
Copy link
Contributor

@stggn stggn left a comment

Choose a reason for hiding this comment

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

With ECDH and 'curve25519' the secrets are now different, while with other curves they are identical:

//'c2pnb208w1' 'curve' STORE 
'curve25519' 'curve' STORE
$curve ECGEN
ECPUBLIC 'pub1' STORE
ECPRIVATE 'priv1' STORE

$curve ECGEN
ECPUBLIC 'pub2' STORE
ECPRIVATE 'priv2' STORE

$priv1 $pub2 ECDH
$priv2 $pub1 ECDH

@hbs
Copy link
Collaborator Author

hbs commented Dec 10, 2024

With ECDH and 'curve25519' the secrets are now different, while with other curves they are identical:

//'c2pnb208w1' 'curve' STORE 
'curve25519' 'curve' STORE
$curve ECGEN
ECPUBLIC 'pub1' STORE
ECPRIVATE 'priv1' STORE

$curve ECGEN
ECPUBLIC 'pub2' STORE
ECPRIVATE 'priv2' STORE

$priv1 $pub2 ECDH
$priv2 $pub1 ECDH

This is the reason for this PR, the DH done using curve25519 was incorrect until this PR.

@hbs
Copy link
Collaborator Author

hbs commented Dec 10, 2024

PTAL had to fix public key computation from private key when using curve25519 as it was not using the correct approach.

… thus ensuring that the pubkey is correctly computed for curve25519
@hbs
Copy link
Collaborator Author

hbs commented Dec 11, 2024

The latest ECGEN fix leads to the correct shared secret in your ECDH test. Could you add that test to the ECDH doc please?

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.

3 participants