commit 302bb3964ad8cc4fb8540ee4783a936c5515605d
author David Benjamin <davidben@google.com> Wed May 02 15:08:16 2018 -0400
committer Adam Langley <agl@google.com> Wed May 02 19:24:00 2018 +0000
tree 63d01096257d1e7501a69b0fa899f123c412b45f
parent 6e678eeb6e76171712ae00d467321b6fe196152d

Small curve25519 cleanups.

Per Brian, x25519_ge_frombytes_vartime does not match the usual
BoringSSL return value convention, and we're slightly inconsistent about
whether to mask the last byte with 63 or 127. (It then gets ANDed with
64, so it doesn't matter which.) Use 127 to align with the curve25519
RFC. Finally, when we invert the transformation, use the same constants
inverted so that they're parallel.

Bug: 243, 244
Change-Id: I0e3aca0433ead210446c58d86b2f57526bde1eac
Reviewed-on: https://boringssl-review.googlesource.com/27984
Reviewed-by: Adam Langley <agl@google.com>

crypto/curve25519/spake25519.c

diff --git a/crypto/curve25519/spake25519.c b/crypto/curve25519/spake25519.c
index e0ff9ba..650178c 100644
--- a/crypto/curve25519/spake25519.c
+++ b/crypto/curve25519/spake25519.c
@@ -481,7 +481,7 @@
   }
 
   ge_p3 Qstar;
-  if (0 != x25519_ge_frombytes_vartime(&Qstar, their_msg)) {
+  if (!x25519_ge_frombytes_vartime(&Qstar, their_msg)) {
     // Point received from peer was not on the curve.
     return 0;
   }

third_party/fiat/curve25519.c

diff --git a/third_party/fiat/curve25519.c b/third_party/fiat/curve25519.c
index 0fdf1d9..60da1c8 100644
--- a/third_party/fiat/curve25519.c
+++ b/third_party/fiat/curve25519.c
@@ -1615,7 +1615,7 @@
   if (fe_isnonzero(&check)) {
     fe_add(&check, &vxx, &u);
     if (fe_isnonzero(&check)) {
-      return -1;
+      return 0;
     }
     fe_mul_ttt(&h->X, &h->X, &sqrtm1);
   }
@@ -1627,7 +1627,7 @@
   }
 
   fe_mul_ttt(&h->T, &h->X, &h->Y);
-  return 0;
+  return 1;
 }
 
 static void ge_p2_0(ge_p2 *h) {
@@ -2996,7 +2996,7 @@
                    const uint8_t signature[64], const uint8_t public_key[32]) {
   ge_p3 A;
   if ((signature[63] & 224) != 0 ||
-      x25519_ge_frombytes_vartime(&A, public_key) != 0) {
+      !x25519_ge_frombytes_vartime(&A, public_key)) {
     return 0;
   }
 
@@ -3062,7 +3062,7 @@
   SHA512(seed, 32, az);
 
   az[0] &= 248;
-  az[31] &= 63;
+  az[31] &= 127;
   az[31] |= 64;
 
   ge_p3 A;
@@ -3184,9 +3184,9 @@
   // This does not affect security because, although we're throwing away
   // entropy, a valid implementation of scalarmult should throw away the exact
   // same bits anyway.
-  out_private_key[0] |= 7;
-  out_private_key[31] &= 63;
-  out_private_key[31] |= 128;
+  out_private_key[0] |= ~248;
+  out_private_key[31] &= ~64;
+  out_private_key[31] |= ~127;
 
   X25519_public_from_private(out_public_value, out_private_key);
 }