paulmillr / noble-curves

Audited & minimal JS implementation of elliptic curve cryptography.
https://paulmillr.com/noble
MIT License
664 stars 62 forks source link

ed25519 edwardsToMontgomery does not produce correct output #31

Closed mirceanis closed 1 year ago

mirceanis commented 1 year ago

The function edwardsToMontgomery doesn't seem to be correctly implemented. There is a formula stated in the TSDoc (1+y)/(1-y) but the implementation uses a different formula (y-1)/(y+1)).

Indeed, when comparing outputs to what would be the equivalent code from @stablelib/ed25519 we get different results.

Here's an example test:

import { ed25519, edwardsToMontgomery } from '@noble/curves/ed25519'
import { hexToBytes, bytesToHex } from '@noble/hashes/utils'
import { deepStrictEqual } from 'assert'
import { should } from 'micro-should'

should('edwardsToMontgomery should produce correct output', () => {
  const edSecret = hexToBytes('77076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c2a');
  const edPublic = ed25519.getPublicKey(edSecret);
  const xPublic = edwardsToMontgomery(edPublic);
  deepStrictEqual(
    bytesToHex(xPublic),
    'ed7749b4d989f6957f3bfde6c56767e988e21c9f8784d91d610011cd553f9b06'
  );
});

Test vectors were computed using @stablelib/ed25519:

import { generateKeyPairFromSeed, convertPublicKeyToX25519 } from '@stablelib/ed25519'

const edSeed = hexToBytes('77076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c2a')
const edKeyPair = generateKeyPairFromSeed(edSeed)
const xPub = convertPublicKeyToX25519(edKeyPair.publicKey)
console.log(bytesToHex(xPub))

PS. I already have a fix and will make a PR soon.

ukstv commented 1 year ago

@paulmillr Please, do you mind releasing the fix?

paulmillr commented 1 year ago

@ukstv I plan to do the release schedule for non-security bugfixes once per month or so;

For now please use npm i github:paulmillr/noble-curves or so.

ukstv commented 1 year ago

@paulmillr Aah, okay. Will wait for the next release then.