Fix failed self test when NO_OS_DEPENDENCE (GH #761)

Add is_clamped for secret key validation.

Cleanup paramter names in Donna::curve25519 to follow function.

Overload Donna::curve25519 to implicitly use base point if not provided.

Add additional asserts to let the code debug itself.

Update documentation.

donna.h

@@ -57,7 +57,25 @@
 NAMESPACE_BEGIN(CryptoPP)
 NAMESPACE_BEGIN(Donna)
 
-int curve25519(byte pubkey[32], const byte seckey[32], const byte basepoint[32]);
+/// \brief Generate public key
+/// \param publicKey byte array for the public key
+/// \param secretKey byte array with the private key
+/// \returns 0 on success, non-0 otherwise
+/// \details This curve25519() overload generates a public key from an existing
+///   secret key. Internally curve25519() performs a scalar multiplication
+///   using the base point and writes the result to <tt>pubkey</tt>.
+int curve25519(byte publicKey[32], const byte secretKey[32]);
+
+/// \brief Generate shared key
+/// \param sharedKey byte array for the shared secret
+/// \param secretKey byte array with the private key
+/// \param othersKey byte array with the peer's public key
+/// \returns 0 on success, non-0 otherwise
+/// \details This curve25519() overload generates a shared key from an existing
+///   a secret key and the other party's public key. Internally curve25519()
+///   performs a scalar multiplication using the two keys and writes the result
+///   to <tt>sharedKey</tt>.
+int curve25519(byte sharedKey[32], const byte secretKey[32], const byte othersKey[32]);
 
 NAMESPACE_END  // Donna
 NAMESPACE_END  // CryptoPP

donna_32.cpp

@@ -873,23 +873,29 @@ ANONYMOUS_NAMESPACE_END
 NAMESPACE_BEGIN(CryptoPP)
 NAMESPACE_BEGIN(Donna)
 
-int curve25519(byte pubkey[32], const byte seckey[32], const byte basepoint[32])
+int curve25519(byte publicKey[32], const byte secretKey[32])
+{
+  const byte basePoint[32] = {9};
+  return curve25519(publicKey, secretKey, basePoint);
+}
+
+int curve25519(byte sharedKey[32], const byte secretKey[32], const byte othersKey[32])
 {
   limb bp[10], x[10], z[11], zmone[10];
-  byte e[32]; int i;
+  byte e[32];
 
-  for (i = 0; i < 32; ++i)
+  for (unsigned int i = 0; i < 32; ++i)
-    e[i] = seckey[i];
+    e[i] = secretKey[i];
 
   e[0] &= 248;
   e[31] &= 127;
   e[31] |= 64;
 
-  fexpand(bp, basepoint);
+  fexpand(bp, othersKey);
   cmult(x, z, e, bp);
   crecip(zmone, z);
   fmul(z, x, zmone);
-  fcontract(pubkey, z);
+  fcontract(sharedKey, z);
   return 0;
 }
 

donna_64.cpp

@@ -482,24 +482,29 @@ ANONYMOUS_NAMESPACE_END
 NAMESPACE_BEGIN(CryptoPP)
 NAMESPACE_BEGIN(Donna)
 
-int curve25519(byte pubkey[32], const byte seckey[32], const byte basepoint[32])
+int curve25519(byte publicKey[32], const byte secretKey[32])
+{
+  const byte basePoint[32] = {9};
+  return curve25519(publicKey, secretKey, basePoint);
+}
+
+int curve25519(byte sharedKey[32], const byte secretKey[32], const byte othersKey[32])
 {
   limb bp[5], x[5], z[5], zmone[5];
   uint8_t e[32];
-  int i;
 
-  for (i = 0;i < 32;++i)
+  for (unsigned int i = 0;i < 32;++i)
-    e[i] = seckey[i];
+    e[i] = secretKey[i];
 
   e[0] &= 248;
   e[31] &= 127;
   e[31] |= 64;
 
-  fexpand(bp, basepoint);
+  fexpand(bp, othersKey);
   cmult(x, z, e, bp);
   crecip(zmone, z);
   fmul(z, x, zmone);
-  fcontract(pubkey, z);
+  fcontract(sharedKey, z);
   return 0;
 }
 

validat0.cpp

@@ -437,27 +437,30 @@ bool TestCurve25519()
     SecByteBlock priv1(32), priv2(32), pub1(32), pub2(32), share1(32), share2(32);
     for (unsigned int i=0; i<AGREE_COUNT; ++i)
     {
-        // Langley's curve25519-donna
         GlobalRNG().GenerateBlock(priv1, priv1.size());
         GlobalRNG().GenerateBlock(priv2, priv2.size());
 
         priv1[0] &= 248; priv1[31] &= 127; priv1[31] |= 64;
         priv2[0] &= 248; priv2[31] &= 127; priv2[31] |= 64;
 
-        const byte base[32] = {9};
+        // Langley's curve25519-donna
-        Donna::curve25519(pub1, priv1, base);
+        Donna::curve25519(pub1, priv1);
-        Donna::curve25519(pub2, priv2, base);
+        Donna::curve25519(pub2, priv2);
 
         int ret1 = Donna::curve25519(share1, priv1, pub2);
         int ret2 = Donna::curve25519(share2, priv2, pub1);
         int ret3 = std::memcmp(share1, share2, 32);
 
-        // Bernstein's Tweet NaCl
+#if defined(NO_OS_DEPENDENCE)
+        int ret4=0, ret5=0, ret6=0;
+#else
+        // Bernstein's NaCl requires DefaultAutoSeededRNG.
         NaCl::crypto_box_keypair(pub2, priv2);
 
         int ret4 = Donna::curve25519(share1, priv1, pub2);
         int ret5 = NaCl::crypto_scalarmult(share2, priv2, pub1);
         int ret6 = std::memcmp(share1, share2, 32);
+#endif
 
         bool fail = ret1 != 0 || ret2 != 0 || ret3 != 0 || ret4 != 0 || ret5 != 0 || ret6 != 0;
         pass = pass && !fail;

xed25519.cpp

@@ -63,6 +63,11 @@ int is_small_order(const byte s[32])
   return (int) ((k >> 8) & 1);
 }
 
+int is_clamped(const byte s[32])
+{
+  return (s[0] & 248) == s[0] && (s[31] & 127) == s[31] && (s[31] | 64) == s[31];
+}
+
 ANONYMOUS_NAMESPACE_END
 
 NAMESPACE_BEGIN(CryptoPP)
@@ -71,12 +76,18 @@ x25519::x25519(const byte y[32], const byte x[32])
 {
     std::memcpy(m_pk, y, 32);
     std::memcpy(m_sk, x, 32);
+
+    CRYPTOPP_ASSERT(is_clamped(m_sk) != 0);
+    CRYPTOPP_ASSERT(is_small_order(m_pk) == 0);
 }
 
 x25519::x25519(const byte x[32])
 {
     std::memcpy(m_sk, x, 32);
     GeneratePublicKey(NullRNG(), m_sk, m_pk);
+
+    CRYPTOPP_ASSERT(is_clamped(m_sk) != 0);
+    CRYPTOPP_ASSERT(is_small_order(m_pk) == 0);
 }
 
 x25519::x25519(const Integer &y, const Integer &x)
@@ -86,6 +97,9 @@ x25519::x25519(const Integer &y, const Integer &x)
 
     ArraySink xs(m_sk, 32);
     x.Encode(xs, 32);
+
+    CRYPTOPP_ASSERT(is_clamped(m_sk) != 0);
+    CRYPTOPP_ASSERT(is_small_order(m_pk) == 0);
 }
 
 x25519::x25519(const Integer &x)
@@ -93,46 +107,72 @@ x25519::x25519(const Integer &x)
     ArraySink xs(m_sk, 32);
     x.Encode(xs, 32);
     GeneratePublicKey(NullRNG(), m_sk, m_pk);
+
+    CRYPTOPP_ASSERT(is_clamped(m_sk) != 0);
+    CRYPTOPP_ASSERT(is_small_order(m_pk) == 0);
 }
 
 x25519::x25519(RandomNumberGenerator &rng)
 {
     GeneratePrivateKey(rng, m_sk);
     GeneratePublicKey(NullRNG(), m_sk, m_pk);
+
+    CRYPTOPP_ASSERT(is_clamped(m_sk) != 0);
+    CRYPTOPP_ASSERT(is_small_order(m_pk) == 0);
 }
 
 x25519::x25519(BufferedTransformation &params)
 {
-    // TODO: Fix the on-disk format once we know what it is.
+    // TODO: Fix the on-disk format once we determine what it is.
     BERSequenceDecoder seq(params);
 
-      BERGeneralDecoder x(seq, BIT_STRING);
+      size_t read; byte unused;
-      if (!x.IsDefiniteLength() || x.MaxRetrievable() < 32)
-        BERDecodeError();
-      x.Get(m_sk, 32);
-      x.MessageEnd();
 
-      BERGeneralDecoder y(seq, OCTET_STRING);
+      BERSequenceDecoder sk(seq, BIT_STRING);
-      if (!y.IsDefiniteLength() || y.MaxRetrievable() < 32)
+      read = sk.Get(unused);  // unused bits
-        BERDecodeError();
+      CRYPTOPP_ASSERT(read == 1 && unused == 0);
-      y.Get(m_pk, 32);
+
-      y.MessageEnd();
+      CRYPTOPP_ASSERT(sk.MaxRetrievable() >= 32);
+      read = sk.Get(m_sk, 32);
+      sk.MessageEnd();
+
+      if (read != 32)
+          throw BERDecodeErr();
+
+      if (seq.EndReached())
+      {
+          GeneratePublicKey(NullRNG(), m_sk, m_pk);
+      }
+      else
+      {
+          BERSequenceDecoder pk(seq, OCTET_STRING);
+          CRYPTOPP_ASSERT(pk.MaxRetrievable() >= 32);
+          read = pk.Get(m_pk, 32);
+          pk.MessageEnd();
+
+          if (read != 32)
+              throw BERDecodeErr();
+      }
 
     seq.MessageEnd();
+
+    CRYPTOPP_ASSERT(is_clamped(m_sk) != 0);
+    CRYPTOPP_ASSERT(is_small_order(m_pk) == 0);
 }
 
 void x25519::DEREncode(BufferedTransformation &params) const
 {
-    // TODO: Fix the on-disk format once we know what it is.
+    // TODO: Fix the on-disk format once we determine what it is.
     DERSequenceEncoder seq(params);
 
-      DERSequenceEncoder x(seq, BIT_STRING);
+      DERSequenceEncoder sk(seq, BIT_STRING);
-      x.Put(m_sk, 32);
+      sk.Put((byte)0);   // unused bits
-      x.MessageEnd();
+      sk.Put(m_sk, 32);
+      sk.MessageEnd();
 
-      DERSequenceEncoder y(seq, OCTET_STRING);
+      DERSequenceEncoder pk(seq, OCTET_STRING);
-      y.Put(m_pk, 32);
+      pk.Put(m_pk, 32);
-      y.MessageEnd();
+      pk.MessageEnd();
 
     seq.MessageEnd();
 }
@@ -140,8 +180,12 @@ void x25519::DEREncode(BufferedTransformation &params) const
 bool x25519::Validate(RandomNumberGenerator &rng, unsigned int level) const
 {
     CRYPTOPP_UNUSED(rng);
+    CRYPTOPP_ASSERT(is_clamped(m_sk) != 0);
+    CRYPTOPP_ASSERT(is_small_order(m_pk) == 0);
 
-    if (level >= 2 && is_small_order(m_pk) != 0)
+    if (level >= 1 && is_clamped(m_sk) == 0)
+        return false;
+    else if (level >= 2 && is_small_order(m_pk) != 0)
         return false;
 
     return true;