ec: expose low-level point arithmetic

[samoht]

This is useful for accessing those constant-time operations in other protocols (for instance spake2).

[samoht]

Note: the tests have been written with the help of Claude Code. Not sure what's the policy on this repo for this kind of contibutions.

[hannesm]

I barely remember the code and the invariants. Since you have looked into it more deeply, are there all the necessary checks (i.e. length and value) done that there are no surprises (exceptions) when these primitive operations are used with any input?

[hannesm]

What was the idea behind the tests? I mean did you generate them based on a RFC or another implementation, or just "let the AI do some tests"?

[hannesm]

I'm confused by your naming. You introduce a module Point, with two types, point and scalar...

Now, is this really a point?

Then there is an of_octets and to_octets, which work on point. And in addition scalar_of/to_octets. This naming doesn't scale well.

Also, there is already a Point module, thus you introduce shadowing. I'm not in favour.

And then you define it for NIST curves, is there a story for 25519?

[samoht]

I've just exposed the existing Point module but with a more abstract API (and the operations I needed for spake2). I'm happy to do something a bit more intrusive if you think that's useful!

[samoht]

Now with a proper "Group" name + test vectors from the RFC + negative tests for invalid inputs

[hannesm]

Hard to review when you force-push. Still there are some questions remaining:

• I still don't understand what the story behind the tests are? What is the relationship with RFC 5903?
• What are the invariants that are crucial for these operations? Is e.g. a point at infinity a nice value to pass to add / scalar_mult
• Choice of function names: why Scalar.mult? Wouldn't OCaml be more consistent if mul was chosen?

From your code, as I understand this could be mostly implemented on top of mirage-crypto-ec:

module Group (D : Mirage_crypto_ec.Dh_dsa) = struct
  module Point = struct
      type t = D.Dsa.pub
      let of_octets = D.Dsa.pub_of_octets
      let to_octets = D,Dsa.pub_to_octets
      let generator = `<here we need to expose the Make_point.params_g>` 
      let add = `<here we need to expose the Make_point.add>`
    end
    module Scalar = struct
      type t = D.Dh.secret
      let of_octets = D.Dh.secret_of_octets
      let to_octets = D.Dh.secret_to_octets
      let mult s p =
        let p' = Point.to_octets p in
        Result.map Point.of_octets (D.Dh.key_exchange s p')
    end
  end

But I see that exposing some functionality would be more efficient.

Coming back here, my trouble with this PR is:

• Adding complexity (a functor and a module), which all is already defined (so it's just aliases);
• The name Group is well-defined in maths https://en.wikipedia.org/wiki/Group_(mathematics) - and I'm not sure whether what you want to expose here is a group.

Another approach could be to extend the Dh_dsa interface with (actually, this could be an extension of the Dsa interface for the NIST curves - then the Dsa. prefix could as well be dropped):

module Primitive : sig
  val generator : Dsa.pub
  val add : Dsa.pub -> Dsa.pub -> Dsa.pub
  val scalar_mul(t?) : Dsa.priv -> Dsa.pub -> Dsa.pub
end

This would mean: no new functors, nothing to add to each curve definition; no new types.

Would this work for you?

[hannesm]

To get a better idea what I have in mind, please take a look at #278 (documentation comments taken from this PR)