Nikos Mavrogiannopoulos nmav@gnutls.org writes:
This patch adds ECC support using the GnuTLS libtomcrypt adaptation.
Nice! Needs some more work to really fit with nettle. Some initial comments based on the header file:
+/* assume curve is y^2 = x^3 - 3x + b
- instead of the generic y^2 = x^3 + ax + b
- (XXX: the generic case has been tested only
- with the SECG curves.)
- */
Maybe the naming in the itnerface should reflect that it's a special case.
+/* ---- ECC Routines ---- */ +/* size of our temp buffers for exported keys */ +#define ECC_BUF_SIZE 512
+/* max private key size */ +#define ECC_MAXSIZE 66
Where do these maximums come from?
+/** Structure defines a NIST GF(p) curve */ +typedef struct {
- /** The size of the curve in octets */
- int size;
- /** name of curve */
- const char *name;
- /** curve's OID */
- const char *oid;
- /** The prime that defines the field the curve is in (encoded in hex) */
- const char *prime;
- /** The fields A param (hex) */
- const char *A;
- /** The fields B param (hex) */
- const char *B;
- /** The order of the curve (hex) */
- const char *order;
- /** The x co-ordinate of the base point on the curve (hex) */
- const char *Gx;
- /** The y co-ordinate of the base point on the curve (hex) */
- const char *Gy;
+} ecc_set_type;
Avoid using typedef for plain structs.
I'm not sure I like using hex strings for the constants. Depends a bit on usage, of course. E.g., it's a bit difficult to define a constant mpz_t.
+/** An ECC key */ +typedef struct {
- /** Type of key, PK_PRIVATE or PK_PUBLIC */
- int type;
- mpz_t prime;
- mpz_t order;
- mpz_t A;
- mpz_t Gx;
- mpz_t Gy;
- /** The public key */
- ecc_point pubkey;
- /** The private key */
- mpz_t k;
+} ecc_key;
I'd follow nettle's DSA interface with separate structs for the public and private parameters, and eliminate the PK_PRIVATE and PK_PUBLIC constants. I think it would also make sense to move out the curve parameters to it's own struct (typed as mpz_t, then, unlike ecc_set_tyep above).
+/* Key generation */ +int ecc_make_key(void *random_ctx, nettle_random_func random, ecc_key *key, const ecc_set_type *dp); +int ecc_make_key_ex(void *random_ctx, nettle_random_func random, ecc_key *key, mpz_t prime, mpz_t order, mpz_t A, mpz_t Gx, mpz_t Gy); +void ecc_free(ecc_key *key);
I haven't figured out exactly what these do, but naming should most likele be _init and _clear, for consistency with the rest of nettle and with gmp.
+/* EC-Diffie-Hellman */ +int ecc_shared_secret(ecc_key *private_key, ecc_key *public_key,
unsigned char *out, unsigned long *outlen);
Haven't looked at this; for diffie-hellman over the normal ring one would just use gmp's powm function. I think the corresponding ecc function should also be public (maybe it already is?).
+/* ECDSA */ +int ecc_sign_hash(const unsigned char *in, unsigned long inlen,
struct dsa_signature *signature,void *random_ctx, nettle_random_func random, ecc_key *key);+int ecc_verify_hash(struct dsa_signature * signature,
const unsigned char *hash, unsigned long hashlen,int *stat, ecc_key *key);
Do these correspond to the _sign_digest and _verify_digest functions for dsa and rsa?
+/* (Internal) low level functions */ +ecc_point *ecc_new_point(void); +void ecc_del_point(ecc_point *p);
It's more nettle style to let the caller allocate the structs. ecc_point_init and ecc_point_clear would be more appropriate.
+/* point ops (mp == montgomery digit) */ +/* R = 2P */ +int ecc_projective_dbl_point(ecc_point *P, ecc_point *R, mpz_t a, mpz_t modulus);
+/* R = P + Q */ +int ecc_projective_add_point(ecc_point *P, ecc_point *Q, ecc_point *R, mpz_t A, mpz_t modulus);
Is it customary jargon and notation to think about the the ecc group operation as addition rather than multiplication? (Choice is arbitrary).
+/* R = kG */ +int ecc_mulmod(mpz_t k, ecc_point *G, ecc_point *R, mpz_t a, mpz_t modulus, int map);
I'd call the it ecc_scalar_mul or something like that, rather than mulmod. Algebraically, any (cummutative?) group is a module over Z or over Z/(group order) or something like that (sorry, I don't recall the fine details since I studied algebra), and that's the type of multiplication we're doing: Z x G -> G.
+int mp_init_multi(mpz_t *a, ...); +void mp_clear_multi(mpz_t *a, ...);
Not sure I like these.
+#define mp_isodd(a) (mpz_size(a) > 0 ? (mpz_getlimbn(a, 0) & 1 ? 1 : 0) : 0)
Just use mpz_odd_p.
+#define MP_DIGIT_BIT (sizeof(mp_limb_t) * 8 - GMP_NAIL_BITS)
Same as GMP_NUMB_BITS.
/nisse