CSIDH by AndHell · Pull Request #1 · GiacomoPope/isogeny_rs

AndHell · 2025-06-02T10:56:24Z

In this PR, we provide a simplified implementation of the CSIDH protocol using the CSIDH-512 parameter set.

It is simplified in the sense that it is not fully optimized, but focuses on the readability of the different steps in the protocol.

Further todos:

switch to velusqrt
implement verification

GiacomoPope

Some small comments, but otherwise thank you so much!

GiacomoPope · 2025-06-02T14:04:52Z

+        let Y = V * XZ;
+        let ok = Y.is_square();
+
+        (P, ok)


This function should simply be a call to rand_point to get a valid point, then we should use to_pointx. Something like:

pub fn rand_pointx<R: CryptoRng + RngCore>(&self, rng: &mut R) -> PointX { let P = Self::rand_point(rng); P.to_pointx() }

I see that this method uses failure for the direction in CSIDH reading some more, but I think this then really needs to be a method for CSIDH rather than on the Curve type, as wanting a point on the twist instead of the curve seems a little esoteric?

Maybe we want something like rand_point on the Csidh type and then we return `P, twist) to make it obvious what we're doing for this case?

GiacomoPope · 2025-06-02T14:05:28Z

+
+#[derive(Clone, Copy, Debug)]
+pub struct CsidhParameters<const COUNT: usize> {
+    pub num_primes: usize,


is num_primes not just the same as COUNT?

GiacomoPope · 2025-06-02T14:10:32Z

+
+                }
+
+                // // For extra safty, assert the kernel is indeed of the correct degree


Suggested change

// // For extra safty, assert the kernel is indeed of the correct degree

// // For extra safety, assert the kernel is indeed of the correct degree

GiacomoPope · 2025-06-02T14:12:48Z

+
+                // check if the kernel is of the correct degree
+                // if not we skip it and try again on an new round
+                if K.is_zero() == u32::MAX {


Feels like this continue should somehow be linked in to cofactor clearing, otherwise you might have P = 0 after an evaluation and yet you spend all that time cofactor clearing only to exit afterwards.

GiacomoPope · 2025-06-02T14:13:47Z

+    ];
+
+    pub const CSIDH_PARAMS: CsidhParameters<NUM_PRIMES> = CsidhParameters {
+        num_primes: NUM_PRIMES,


yeah this can be cleaned up, we dont need num_primesas we can just useNUM_PRIMES`

GiacomoPope · 2025-06-02T14:15:12Z

+            let (mut P, direction) = curve.rand_pointX(rng);
+            let direction = direction == 0;
+
+            // clear cofactor


Suggested change

// clear cofactor

// clear 2^e cofactor

GiacomoPope · 2025-06-02T14:16:33Z

+        private_key: &CsidhPrivateKey<COUNT>,
+        rng: &mut R,) -> CsidhPublicKey<Fp> {
+
+        // todo: verify


shall we ust multiply by (p+1) a bunch of times to check supersingularity?

GiacomoPope · 2025-06-30T09:10:52Z

I'm back from leave now, did you have time to work on the comments, or would you like me to merge as is and do edits myself?

GiacomoPope · 2025-06-02T21:16:32Z

    }
+
+    /// Return a new random X only point. 
+    pub fn rand_point<R: CryptoRng + RngCore>(rng: &mut R) -> PointX<Fq> {


This doesnt always return a valid point on the curve... I think we should just delete this for now as it might be confusing?

GiacomoPope · 2025-06-02T21:17:04Z

+use rand_core::{CryptoRng, RngCore};
+
+#[derive(Clone, Copy, Debug)]
+pub struct CsidhParameters<const COUNT: usize> {


maybe rename to PRIMES or PRIME_COUNT?

GiacomoPope · 2025-07-17T09:30:59Z

+
+
+
+fp2::define_fp2_from_modulus!(


We dont need the extension, right? We can use define_fp_core https://docs.rs/fp2/latest/fp2/macro.define_fp_core.html

GiacomoPope · 2025-07-17T12:32:18Z

        max_exponent: MAX_EXPONENT,
        two_cofactor: COFACTOR,
        primes: PRIMES,
+        four_sqrt_p: FOUR_SQRT_P,


Cant we have this value as a Fp and then avoid the need for , 5? Similar to how we have constants in the sidh impl

Yeah, this part is quite ugly.

The issue is that we need 4sqrt(p) as a bigint (bn), not as a Fp, in the verification, to check against the order of the random point.
The cleanest way would probably be to have a struct for bn.

I'll try to come up with something.

We do we need it to be a big number? For scalar multiplication? One option would be to use a Fp element and then encode this to bytes for scalar multiplication? Another, yes, would be to make a BN type

We only use this constant to check if the order of a point is > 4sqrt(p).

Oh I see, this is alg 2 of (https://eprint.iacr.org/2022/880.pdf)...

I think the "best" method at the moment is have $4 \sqrt{p}$ as a Fp type in the structure and then in verify we can do something like

bound = bytes_to_bn_vartime(&self.four_sqrt_p.encode())

where &self.four_sqrt_p.encode() will have type &[u8] and bytes_to_bn_vartime can convert from base 2^8 to base 2^64?

I ended up adding a small Bn struct as a wrapper.
Let me know what you think about this approach.
If you think it's suitable I can also extend the struct.

GiacomoPope · 2025-07-17T12:34:21Z

+
+        // if RHS i a sqrt, it is on the curve
+        let RHS = (Cxx+four_Ax-two_Cx+(*C24))*x;
+        let (_, is_sqrt) = RHS.sqrt();     


We can use https://docs.rs/fp2/latest/fp2/traits/fp/trait.Fp.html#tymethod.is_square

GiacomoPope · 2025-07-22T10:10:00Z

-        let four_Ax = tmp+tmp;
+        // projective curve: x^3C+4Ax^2−2x^2C+XC
+        let tmp = *A24*x;
+        let four_Ax = tmp+tmp+tmp+tmp;


we have a mul4() method available in fp2_rs

GiacomoPope · 2025-07-22T10:10:13Z

+        let four_Ax = tmp+tmp+tmp+tmp;
        let tmp = *C24*x;
        let Cxx = tmp*x;
        let two_Cx = tmp+tmp;


Same here, we have a mul2()

GiacomoPope · 2025-07-22T10:11:29Z

        let A24 = public_key.A + Fp::TWO;
        let C24 = Fp::FOUR;

+        let fsqrtp = &self.four_sqrt_p.limbs[..self.four_sqrt_p.len];


This looks like we might want to make a method out of it? maybe a Bn to_slice() ?

GiacomoPope · 2025-07-22T10:12:02Z

        0x856f1399d91d6592,
        0x0000000000000002,
-    ];
+        0x0000000000000000,


why the extra 0x00?

GiacomoPope · 2025-07-22T10:12:46Z

+
+
+// This is a small wrapper to use a bn as constant
+const BN_MAXLIMBS : usize = 8;


oooh this is why... Maybe we should use a vector and dynamically allocate? This could be refactored later I suppose though.

Vectors and constants don't like each other much.

What about putting Bn behind a macro, similar to Fp?
It's quite a bit of overhead for a single constant, but it should be the cleanest solution and might be helpful for other protocols.

AndHell and others added 8 commits May 26, 2025 21:24

add csidh field

d53031c

[WIP] first (still buggy) basic csidh

b97a802

Merge branch 'GiacomoPope:main' into csidh

28fe7c9

Merge branch 'GiacomoPope:main' into csidh

fa05bd2

Merge branch 'GiacomoPope:main' into csidh

e29fe00

[csidh] use const generic for array lenght

2a82e65

working csidh 512

05eed1f

[csidh] expand key range to [-B..B] and add some comments

c8548a7

GiacomoPope reviewed Jun 2, 2025

View reviewed changes

AndHell and others added 6 commits June 2, 2025 20:04

[csidh] move rand_pointx to csidh instead of curve

3e5ab17

Merge branch 'GiacomoPope:main' into csidh

0976758

[csidh] add a simple benchmark

c7abbc4

Merge branch 'GiacomoPope:main' into csidh

b67a7e6

Merge branch 'GiacomoPope:main' into csidh

7bdf7ca

[csidh] use velu sqrt

7cadbb0

[WIP] csidh verify

6a9d901

GiacomoPope reviewed Jul 17, 2025

View reviewed changes

AndHell added 2 commits July 17, 2025 11:45

csidh verify

ef1a717

cleanup and faster point sampling

f90629c

GiacomoPope reviewed Jul 17, 2025

View reviewed changes

[csidh] add struct Bn for constants

7318773

GiacomoPope reviewed Jul 22, 2025

View reviewed changes

AndHell added 2 commits July 22, 2025 17:08

struct Bn from macro

2a633fc

[wip] use BnTrait in velu

4b3e848


		}

		// // For extra safty, assert the kernel is indeed of the correct degree

	// // For extra safty, assert the kernel is indeed of the correct degree
	// // For extra safety, assert the kernel is indeed of the correct degree



		// This is a small wrapper to use a bn as constant
		const BN_MAXLIMBS : usize = 8;




		fp2::define_fp2_from_modulus!(

Conversation

AndHell commented Jun 2, 2025

Uh oh!

GiacomoPope left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GiacomoPope commented Jun 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants