From 29ba1b93b57e20a88b1b21e9a09c381a3f8fd417 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= <francois@garillot.net>
Date: Wed, 19 Aug 2020 17:18:25 -0400
Subject: [PATCH] Check for small subgroup components when using the
 EdwardsPoint group for the OPRF

A malicious attacker sending a small point could lead the server / user to leak private information.

The check avoiding mixed order points (rather than just small) is prohibitively expensive, and I don't know how an attacker would extract any data from that => we focus on a small order check.

Fixes #34.
---
 Cargo.lock   |  7 ++++++
 Cargo.toml   |  1 +
 src/group.rs | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index f579414..c2c3c2a 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1,5 +1,11 @@
 # This file is automatically @generated by Cargo.
 # It is not intended for manual editing.
+[[package]]
+name = "anyhow"
+version = "1.0.32"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "6b602bfe940d21c130f3895acd65221e8a61270debe89d628b9cb4e3ccb8569b"
+
 [[package]]
 name = "atty"
 version = "0.2.14"
@@ -451,6 +457,7 @@ checksum = "624a8340c38c1b80fd549087862da4ba43e08858af025b236e509b6649fc13d5"
 name = "opaque-ke"
 version = "0.1.0"
 dependencies = [
+ "anyhow",
  "base64",
  "criterion",
  "curve25519-dalek",
diff --git a/Cargo.toml b/Cargo.toml
index 7b7e9be..9e5571e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -31,6 +31,7 @@ x25519-dalek = { version = "0.6.0", default-features = false, features = ["std"]
 zeroize = "1.1"
 
 [dev-dependencies]
+anyhow = "1.0.32"
 base64 = "0.12.3"
 criterion = "0.3.3"
 hex = "0.4.2"
diff --git a/src/group.rs b/src/group.rs
index 4a96549..27277d5 100644
--- a/src/group.rs
+++ b/src/group.rs
@@ -131,9 +131,14 @@ impl Group for EdwardsPoint {
     fn from_element_slice(
         element_bits: &GenericArray<u8, Self::ElemLen>,
     ) -> Result<Self, InternalPakeError> {
-        CompressedEdwardsY::from_slice(element_bits)
+        let point = CompressedEdwardsY::from_slice(element_bits)
             .decompress()
-            .ok_or_else(|| InternalPakeError::PointError)
+            .ok_or_else(|| InternalPakeError::PointError)?;
+
+        if point.is_small_order() {
+            return Err(InternalPakeError::SubGroupError);
+        }
+        Ok(point)
     }
     // serialization of a group element
     fn to_arr(&self) -> GenericArray<u8, Self::ElemLen> {
@@ -164,3 +169,62 @@ impl Group for EdwardsPoint {
             .mul_by_cofactor()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use anyhow::{anyhow, Result};
+
+    const EIGHT_TORSION: [[u8; 32]; 8] = [
+        [
+            1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+            0, 0, 0,
+        ],
+        [
+            199, 23, 106, 112, 61, 77, 216, 79, 186, 60, 11, 118, 13, 16, 103, 15, 42, 32, 83, 250,
+            44, 57, 204, 198, 78, 199, 253, 119, 146, 172, 3, 122,
+        ],
+        [
+            0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+            0, 0, 128,
+        ],
+        [
+            38, 232, 149, 143, 194, 178, 39, 176, 69, 195, 244, 137, 242, 239, 152, 240, 213, 223,
+            172, 5, 211, 198, 51, 57, 177, 56, 2, 136, 109, 83, 252, 5,
+        ],
+        [
+            236, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255,
+            255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 127,
+        ],
+        [
+            38, 232, 149, 143, 194, 178, 39, 176, 69, 195, 244, 137, 242, 239, 152, 240, 213, 223,
+            172, 5, 211, 198, 51, 57, 177, 56, 2, 136, 109, 83, 252, 133,
+        ],
+        [
+            0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+            0, 0, 0,
+        ],
+        [
+            199, 23, 106, 112, 61, 77, 216, 79, 186, 60, 11, 118, 13, 16, 103, 15, 42, 32, 83, 250,
+            44, 57, 204, 198, 78, 199, 253, 119, 146, 172, 3, 250,
+        ],
+    ];
+
+    fn deserialize_point(pt: &[u8]) -> Result<EdwardsPoint> {
+        let mut bytes = [0u8; 32];
+        bytes.copy_from_slice(&pt[..32]);
+
+        curve25519_dalek::edwards::CompressedEdwardsY(bytes)
+            .decompress()
+            .ok_or_else(|| anyhow!("Point decompression failed!"))
+    }
+
+    #[test]
+    fn test_small_subgroup_edwards() {
+        for i in 0..8 {
+            let pt = &EIGHT_TORSION[i][..];
+            assert!(deserialize_point(&pt).is_ok());
+            assert!(EdwardsPoint::from_element_slice(GenericArray::from_slice(&pt)).is_err());
+        }
+    }
+}