Curve25519-donna changes.

crypto/curve25519.h

+// 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.
+
+#ifndef CRYPTO_CURVE25519_H
+#define CRYPTO_CURVE25519_H
+
+#include <string>

[wtc, 2013/03/06 21:26:44]

Remove this header. It is no longer necessary.

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

Done.

+
+#include "base/basictypes.h"
+#include "crypto/crypto_export.h"
+
+namespace crypto {
+
+// Curve25519 implements an elliptic curve group, commonly known as Curve25519

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

The comma here is weird, when you read just the part before the comma

"Curve25519 implements an elliptic curve group"

...

Perhaps

// Curve25519 implements the elliptic curve group described at [url], commonly
known as Curve25519.

Still, it seems like we want something more substantive than just the URL link.

// Curve25519 implements the elliptic curve group known as
// Curve25519, as described in "Curve 25519: new Diffie-Hellman Speed Records",
by D.J. Bernstein. Additional information is available at [URL]

I want to make sure that even if the URL isn't available, it's still available -
hence the reference to the (published) paper's title.

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

Done.

+// and defined in http://cr.yp.to/ecdh.html.
+namespace curve25519 {
+
+static const size_t kBytes = 32;
+static const size_t kScalarBytes = 32;

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

nit: Comments explaining what these constants are and what they're used for.

[wtc, 2013/03/06 21:26:44]

Please document what these two constants are.

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

They can be moved into the .cc as well, now that I look at it. (They aren't part
of the API.)

For comments, since they are taken from the NaCl API:

// kBytes is the number of bytes in the result of the Diffie-Hellman operation,
which is an element of GF(2^255-19).

// kScalarBytes is the number of bytes in an element of the scalar field:
GF(2^252 + 27742317777372353535851937790883648493).

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

Thanks very much agl. Added the above comments.

Done.

+
+// ScalarMult computes the |shared_key| from |private_key| and
+// |peer_public_key|. See "Computing shared secrets" section of
+// http://cr.yp.to/ecdh.html.

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

I don't know if this is really a good comment description - that section just
describes "How to call this API" - not what it does.

The same for ScalarBaseMult.

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

Done.

+void CRYPTO_EXPORT ScalarMult(const uint8* private_key,
+                              const uint8* peer_public_key,
+                              uint8* shared_key);
+
+// ScalarBaseMult computes the |public_key| from |private_key| and a basepoint
+// as specified in "Computing public keys" section of
+// http://cr.yp.to/ecdh.html.
+void CRYPTO_EXPORT ScalarBaseMult(const uint8* private_key, uint8* public_key);
+
+// ConvertToPrivateKey converts |secret| into a private key, suitable

[wtc, 2013/03/06 21:26:44]

Please note that the conversion is done in place, i.e.,
|secret| is an in/out argument.

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

Done.

+// for passing to |ScalarBaseMult|. See "Computing secret keys" section of
+// http://cr.yp.to/ecdh.html.
+void CRYPTO_EXPORT ConvertToPrivateKey(uint8* secret);

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

For all of these functions, you could provide clear descriptions about the
expected variable lengths.

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

Done.

+
+}  // namespace curve25519
+
+}  // namespace crypto
+
+#endif  // CRYPTO_CURVE25519_H