diff --git a/vlib/x/crypto/curve25519/curve25519.v b/vlib/x/crypto/curve25519/curve25519.v index f233b01f0b..2be680a5d9 100644 --- a/vlib/x/crypto/curve25519/curve25519.v +++ b/vlib/x/crypto/curve25519/curve25519.v @@ -101,6 +101,12 @@ pub fn (mut pv PrivateKey) x25519(point []u8) ![]u8 { if point.len != point_size { return error('bad point size, should be 32') } + // We reject and disallow zero-bytes point to be passed + // and check it here as a quick exit before heavy math + // calculation on `x25519` call + if is_zero(point) { + return error('x25519: get zeros point') + } // even technically its possible, but we limit to unallow it if subtle.constant_time_compare(pv.key, point) == 1 { return error('pv.key identical with point') @@ -121,6 +127,12 @@ pub fn (pv PrivateKey) bytes() ![]u8 { // free releases underlying key. Dont use the key after calling .free @[unsafe] pub fn (mut pv PrivateKey) free() { + // when private key has been marked as done (freed), + // calling free on already freed key would lead to undefined behavior. + // so, we check it + if pv.done { + return + } unsafe { pv.key.free() } // sets flag to finish pv.done = true @@ -143,7 +155,7 @@ pub fn PublicKey.new_from_bytes(bytes []u8) !&PublicKey { // in curve25519 is generally not needed for Diffie-Hellman key exchange. // See https://cr.yp.to/ecdh.html#validate // But there are availables suggestion to do validation on them spreads on the internet, likes - // bytes return the clone of the bytes of the underlying PublicKey + // - blacklisting the known bad public keys // - check the shared value and to raise exception if it is zero. // - You can also bind the exchanged public keys to the shared keys, i.e., // instead of using H(abG) as the shared keys, you should use H(aG || bG || abG) @@ -209,6 +221,7 @@ fn (rd RawDerivator) derive(sec []u8, opt DeriveOpts) ![]u8 { // 6. Diffie-Hellman with Curve25519 // See https://datatracker.ietf.org/doc/html/rfc7748#section-6 pub fn derive_shared_secret(mut local PrivateKey, remote PublicKey, opt SharedOpts) ![]u8 { + // TODO: should this check be relaxed ? // check for safety local_pubkey := local.public_key()! if local_pubkey.equal(remote) { @@ -220,6 +233,8 @@ pub fn derive_shared_secret(mut local PrivateKey, remote PublicKey, opt SharedOp // Both now share shared = X25519(local_privkey, remote_pubkey) = X25519(remote_privkey, local_pubkey) // as a shared secret, which is then used as a key or input to a key derivation function. sec := local.x25519(remote.key)! + // Internally, x25519 has builtin check for zeros result + // but only for non base point branch on x25519_generic routine if is_zero(sec) { return error('zeroes shared secret') } @@ -247,6 +262,11 @@ pub fn derive_shared_secret(mut local PrivateKey, remote PublicKey, opt SharedOp // be either `base_point` or the output of another `x25519` call. @[direct_array_access] pub fn x25519(mut scalar []u8, point []u8) ![]u8 { + // likes the previous comment, we add zeroes point check here + // and reject if it happen. + if is_zero(point) || is_zero(scalar) { + return error('x25519: unallowed zeros/scalar point') + } mut dst := []u8{len: 32} // we do bytes clamping here, to make sure scalar was ready to use clamp(mut scalar)! @@ -264,8 +284,8 @@ fn x25519_generic(mut dst []u8, mut scalar []u8, point []u8) ![]u8 { return error('dst: get base_point') } } else { - // base := point.clone() scalar_mult(mut dst, mut scalar, point)! + // check for zeros point result if is_zero_point(dst) { return error('bad input point: low order point') } diff --git a/vlib/x/crypto/curve25519/curve25519_test.v b/vlib/x/crypto/curve25519/curve25519_test.v index 230a5abc82..4536b8ce75 100644 --- a/vlib/x/crypto/curve25519/curve25519_test.v +++ b/vlib/x/crypto/curve25519/curve25519_test.v @@ -99,9 +99,10 @@ struct LowOrderPoint { } const loworder_points = [ + // zeros point LowOrderPoint{[u8(0x00), 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00], error('bad input point: low order point')}, + 0x00, 0x00, 0x00, 0x00, 0x00], error('x25519: unallowed zeros/scalar point')}, LowOrderPoint{[u8(0x01), 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], error('bad input point: low order point')}, @@ -354,55 +355,3 @@ const tests_vectors = [ 0x61, 0x96, 0x1e, 0xfc, 0xb] }, ] - -// Please be aware, this is long running times test, its loop until 1.000.000 times. -// so, please be patient. See the detail of the type 2 test from rfc -// see at https://datatracker.ietf.org/doc/html/rfc7748#section-5.2 -// -// Currently, this test was disabled due to its takes long time to complete, -// please uncomment it if you need run the test. -// On my test machine, its pass successfully in 2225590.282 ms -// ```v -// ... (previous line) -// ... -// start i: 999995 -// start i: 999996 -// start i: 999997 -// start i: 999998 -// start i: 999999 -// -// OK 2225590.282 ms 3 asserts | curve25519.test_x25519_after_iteration_from_rfc_vector_type2() -// Summary for running V tests in "curve25519_test.v": 3 passed, 3 total. Elapsed time: 2225590 ms. -// ``` -/* -fn test_x25519_after_iteration_from_rfc_vector_type2() ! { - iteration1 := hex.decode('422c8e7a6227d7bca1350b3e2bb7279f7897b87bb6854b783c60e80311ae3079')! - iteration1000 := hex.decode('684cf59ba83309552800ef566f2f4d3c1c3887c49360e3875f2eb94d99532c51') ! - iteration1000000 := hex.decode('7c3911e0ab2586fd864497297e575e6f3bc601c0883c30df5f4dd2d24f665424') ! - - // Initially, set k and u to be the following values - key := '0900000000000000000000000000000000000000000000000000000000000000' - mut k := hex.decode(key)! - mut u := k.clone() - mut r := []u8{len: 32} - - // For each iteration, set k to be the result of calling the function and u - // to be the old value of k. The final result is the value left in k. - // - for i in 0..1000000 { - println("start i: $i") - tmp_k := k.clone() - r = x25519(mut k, u) ! - unsafe {u = tmp_k} - unsafe {k = r} - if i == 0 { - assert k == iteration1 - } else if i == 999 { - assert k == iteration1000 - } else if i == 999999 { - assert k == iteration1000000 - } - - } -} -*/