Nocrypto.Rsa.priv_of_primes with ~e:Z.zero raises Division_by_zero

[cfcs]

Should be documented in the .mli?

[cfcs]

various combinations of p and q do too:

Nocrypto.Rsa.priv_of_primes ~e:(Z.of_int 65537) ~p:(Z.of_int 3) ~q:(Z.of_int 3);;

And it doesn't do validation on the primes:

Nocrypto.Rsa.priv_of_primes ~e:(Z.of_int 9) ~p:(Z.of_int 8) ~q:(Z.of_int 9);;
- : Nocrypto.Rsa.priv = {Nocrypto.Rsa.e = <abstr>; d = <abstr>; n = <abstr>; p = <abstr>; q = <abstr>;   dp = <abstr>; dq = <abstr>; q' = <abstr>}

This is of course a completely acceptable design decision, it's just not very clear from the doc-string, and I expected it to do some validation, or alternatively that there would be functions that allowed me to check if a given pubkey/privkey looked valid.

[cfcs]

on a related note, perhaps it's worth declaring pub (and perhaps priv) as private in the .mli to enforce using conversion functions that can do some basic checking of parameters on these keys?

[pqwy]

The key material is not checked for well-formedness -- hence the key type was not wrapped in a smart constructor -- because the app can obtain it from any number of trusted or untrusted sources. The computation (3 primality, 2 relative primality) is too expensive to impose unconditionally.

I can export a separate check for (e, p, q) triples, but I wonder -- in which scenario do you obtain two untrusted primes and a public exponent?

[cfcs]

I ran into the problem in my OpenPGP implementation. When reading a secret key I get these values:

https://tools.ietf.org/html/rfc4880#section-5.5.2

     - A series of multiprecision integers comprising the key material:
           - a multiprecision integer (MPI) of RSA public modulus n;
           - an MPI of RSA public encryption exponent e

https://tools.ietf.org/html/rfc4880#section-5.5.3

       Algorithm-Specific Fields for RSA secret keys:
       - multiprecision integer (MPI) of RSA secret exponent d.
       - MPI of RSA secret prime value p.
       - MPI of RSA secret prime value q (p < q).
       - MPI of u, the multiplicative inverse of p, mod q.

The Nocrypto.Rsa.priv type:

type Nocrypto.Rsa.priv =
    {                                                                           
      e : Z.t; (* I can get this from the public key *)
      d : Z.t; (* I can get this from the private key *)
      n : Z.t; (* I can get this from the public key *)
      p : Z.t; (* I can get this from the private key *)
      q : Z.t; (* I can get this from the private key *)
      dp : Z.t;
      dq : Z.t;
      q' : Z.t; (* I believe this is the field they call "u" *)
    }

Using priv_of_primes seemed the easiest way to construct a priv (so I didn't have to duplicate the code to calculate dp and dq.

Related question: Should I be concerned about side channel attacks when using this? Would it make sense to add a ?mask parameter? Scenario would be something like "I have a PGP service that reads the secret key from storage, and optionally produces a signature provided some other factor is present in the request. The user can time their interactions with this service."

[cfcs]

(As described in the original posts above I am not opposed to not being able to do validation, but I think that it should be documented in the .mli that it can throw exceptions, since I would like to catch those exceptions and print this key is fucked when testing interoperability with other implementations rather than crashing hard).

[pqwy]

Right. There's a pre-packaged sanity check for (e, p, q) now. Checking the full private key amounts to re-computing it, and this is what you should do if unsure.

Your flow is now:

if Rsa.well_formed ~e ~p ~q then priv_of_primes ~e ~p ~q else raise This_key_is_fucked

I also added a bit more documentation.

Speaking of documentation, here's a quote from the old docstring:

Keys are taken to be trusted material, and their properties are not checked.

~mask is also described. Do read that header before opening more issues.

[cfcs]

Re: Masking: So the decision is to not provide masking of the well_formed etc operations?

[pqwy]

Does not apply. Applies only to stuff that already does it.