Curve25519-donna changes.

crypto/curve25519_unittest.cc

Index: crypto/curve25519_unittest.cc
===================================================================
--- crypto/curve25519_unittest.cc	(revision 0)
+++ crypto/curve25519_unittest.cc	(revision 0)
@@ -0,0 +1,48 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "crypto/curve25519.h"
+
+#include <string>
+
+#include "crypto/random.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace crypto {
+
+// SharedKey just tests that the basic key exchange identity holds: that both
+// parties end up with the same key.
+TEST(Curve25519, SharedKey) {
+  for (int i = 0; i < 5; i++) {
+    uint8 alice_private_key[curve25519::kScalarBytes];
+    crypto::RandBytes(alice_private_key, sizeof(alice_private_key));

[Ryan Sleevi, 2013/03/06 21:18:32]

This creates a non-deterministic unit test (as does line 26). Additionally, it
is not clear how or why the magic value 5 was chosen as the loop - why not
simply --gtest_repeat=5?

Better comments explaining that motivation is good - and perhaps a few
fixed/KATs that ensure identities hold (eg: for forwards/backwards
compatibility, if it ever becomes an issue)

The loop seems unnecessary, and if you are going to use it, you should be using
SCOPED_TRACE as well - but I would recommend not using it.

[agl, 2013/03/06 21:47:50]

Removing the loop is fine. This is coming from the server side of the code where
I ran it for a few hundred thousand iterations and then turned it down, but not
to zero.

The standard test for curve25519 runs for too long to be reasonable in a
unittest, but an easy test is to start with {1}, run scalar_base_mult and then
feed the answer back into scalar_base_mult as another scalar for n iterations
and then just check the final result.

[ramant (doing other things), 2013/03/08 00:10:15]

Removed the loop. Used a fixed private key.

[ramant (doing other things), 2013/03/08 00:10:15]

Changed the test to feed back answer from scalar_base_mult  to modify the
private key and ran the test again.

Did this for 5 times (derived this test from agl's test-curve25519.c).
Done.

+    curve25519::ConvertToPrivateKey(alice_private_key);
+
+    uint8 alice_public_key[curve25519::kBytes];
+    curve25519::ScalarBaseMult(alice_private_key, alice_public_key);
+
+    uint8 bob_private_key[curve25519::kScalarBytes];
+    crypto::RandBytes(bob_private_key, sizeof(bob_private_key));
+    curve25519::ConvertToPrivateKey(bob_private_key);
+
+    uint8 bob_public_key[curve25519::kBytes];
+    curve25519::ScalarBaseMult(bob_private_key, bob_public_key);
+
+    uint8 alice_shared_key[curve25519::kBytes];
+    curve25519::ScalarMult(alice_private_key, bob_public_key, alice_shared_key);
+    std::string alice_shared;
+    alice_shared.assign(reinterpret_cast<char*>(alice_shared_key),
+                        sizeof(alice_shared_key));
+
+    uint8 bob_shared_key[curve25519::kBytes];
+    curve25519::ScalarMult(bob_private_key, alice_public_key, bob_shared_key);
+    std::string bob_shared;
+    bob_shared.assign(reinterpret_cast<char*>(bob_shared_key),
+                      sizeof(bob_shared_key));
+
+    ASSERT_EQ(alice_shared, bob_shared);
+  }
+}
+
+}  // namespace crypto