вт, 14 апр. 2020 г. в 21:45, Niels Möller nisse@lysator.liu.se:
Hi, at last I've had a closer look at this patch you posted mid February.
dbaryshkov@gmail.com writes:
+int +gostdsa_vko(const struct ecc_scalar *key,
const struct ecc_point *pub,size_t ukm_length, const uint8_t *ukm,size_t out_length, uint8_t *out)It would be good with some docs, and a link to the spec, either in the manual, or at least comments in the header file. And what's the relation between pub and key? The usual, pub = key * generator, or somethign different?
I will rename key to priv to remove this confusion. VKO is a variant of DH:
VKO = cofactor * ukm * priv * pub; For supported curves cofactor is always 1.
For the interface, return value is a size, and should probably have the size_t type. It fails (and returns zero) of out_length is too small, but there's no good way for the caller to know what's the needed size.
I think it would make sense with either some other function get let the application query for the appropriate size (and then one could delete the return value), or use the awkward but common interface that calling with a NULL out argument returns the needed output size.
The result size is always 2 * curve size. Changing return value to be boolean 0/1.
+{
- const struct ecc_curve *ecc = key->ecc;
- unsigned bsize = (ecc_bit_size(ecc) + 7) / 8;
- mp_size_t size = ecc->p.size;
- mp_size_t itch = 4*size + ecc->mul_itch;
- mp_limb_t *scratch;
- if (itch < 5*size + ecc->h_to_a_itch)
itch = 5*size + ecc->h_to_a_itch;- if (pub->ecc != ecc)
return 0;I think other functions taking both a ecc_scalar and an ecc_point assert that the correspond to the same curve.
- if (out_length < 2 * bsize) {
return 0;- }
- scratch = gmp_alloc_limbs (itch);
- mpn_set_base256_le (scratch, size, ukm, ukm_length);
If ukm_length is large, the ukm input is silently truncated (if I read mpn_set_base256_le correctly). Is that good?
- if (mpn_zero_p (scratch, size))
- mpn_add_1 (scratch, scratch, size, 1);
This is a bit odd, I guess it's required by the spec? mpn_add_1 is overkill, scratch[0] = 1 should do the same thing, right (or skip the below ecc_mod_mul, which becomes a nop)? It's not side-channel silent, does it need to be (i.e., is the ukm input considered secret?)
- ecc_mod_mul (&ecc->q, scratch + 3*size, key->p, scratch);
- ecc->mul (ecc, scratch, scratch + 3*size, pub->p, scratch + 4*size);
- ecc->h_to_a (ecc, 0, scratch + 3*size, scratch, scratch + 5*size);
This is also looks a bit strange to me, but I don't have the background. If key represents a scalar k, and pub represents a point P = k G, where G is the generator (that's the usual relation between private and public key in ecc context), then we compute another point Q = u k P = u k^2 G, where u is the number represented by the ukm input. That square (k^2) looks a bit weird, maybe I'm misunderstanding. And if I have the math right, then it's probably more efficient to compute k^2 using ecc_mod_sqr and use ecc->mul_g (and then the pub input argument becomes unused).
I also wonder if this really has to be a new primitive, or if it could be implemented in terms of ecc_point_mul, or if we should provide an ecc_scalar_mul wrapping ecc_mod_mul (&ecc->q, ...) ?
It can be implemented using ecc_scalar_mul and ecc_point_mul (like GnuTLS does for ecc_shared_secret). But then cofactor will come to play, so it is much easier to hide this behind API function.
+void +test_main (void) +{
- struct streebog256_ctx ctx_256;
- struct streebog256_ctx ctx_512;
- test_vko(nettle_get_gost_gc512a(),
"67b63ca4ac8d2bb32618d89296c7476dbeb9f9048496f202b1902cf2ce41dbc2f847712d960483458d4b380867f426c7ca0ff5782702dbc44ee8fc72d9ec90c9","51a6d54ee932d176e87591121cce5f395cb2f2f147114d95f463c8a7ed74a9fc5ecd2325a35fb6387831ea66bc3d2aa42ede35872cc75372073a71b983e12f19","793bde5bf72840ad22b02a363ae4772d4a52fc08ba1a20f7458a222a13bf98b53be002d1973f1e398ce46c17da6d00d9b6d0076f8284dcc42e599b4c413b8804",SHEX("1d 80 60 3c 85 44 c7 27"),&nettle_streebog256,&ctx_256,SHEX("c9 a9 a7 73 20 e2 cc 55 9e d7 2d ce 6f 47 e2 19 2c ce a9 5f a6 48 67 05 82 c0 54 c0 ef 36 c2 21"));What's the source of these test vectors?
RFC 7836, adding reference.