From cdcf012a5941260ee3acb4651b3e2c649c58f532 Mon Sep 17 00:00:00 2001
From: Noah Vesely <fowlslegs@riseup.net>
Date: Mon, 26 Mar 2018 17:12:58 -0600
Subject: [PATCH] Remove `DuplicateShareData` error and validation

It's possible that two different points have the same data.

To give a concrete example consider the secret polynomial `x^2 + x + s`, where
`s` is the secret byte. Plugging in 214 and 215 (both elements of the cyclic
subgroup of order 2) for `x` will give the same result, `1 + s`.

More broadly, for any polynomial `b*x^t + b*x^(t-1) + ... + x + s`, where `t` is
the order of at least one subgroup of GF(256), for all subgroups of order `t`,
all elements of that subgroup, when chosen for `x`, will produce the same
result.

There are certainly other types of polynomials that have "share collisions."
This type was just easy to find because it exploits the nature of finite fields.
---
 src/errors.rs                 |  5 -----
 src/share/validation.rs       |  5 -----
 tests/recovery_errors.rs      | 11 -----------
 tests/ss1_recovery_errors.rs  | 26 --------------------------
 tests/thss_recovery_errors.rs | 23 -----------------------
 5 files changed, 70 deletions(-)

diff --git a/src/errors.rs b/src/errors.rs
index 6940357..315d47b 100644
--- a/src/errors.rs
+++ b/src/errors.rs
@@ -118,11 +118,6 @@ error_chain! {
             display("This share number ({}) has already been used by a previous share.", share_id)
         }
 
-        DuplicateShareData(share_id: u8) {
-            description("The data encoded in this share is the same as the one found in a previous share")
-            display("The data encoded in share #{} is the same as the one found in a previous share.", share_id)
-        }
-
         InconsistentShares {
             description("The shares are inconsistent")
             display("The shares are inconsistent")
diff --git a/src/share/validation.rs b/src/share/validation.rs
index 139f3f7..45e0e63 100644
--- a/src/share/validation.rs
+++ b/src/share/validation.rs
@@ -55,11 +55,6 @@ pub(crate) fn validate_shares<S: IsShare>(shares: Vec<S>) -> Result<(u8, Vec<S>)
             bail!(ErrorKind::ShareParsingErrorEmptyShare(id))
         }
 
-        if result.iter().any(|s| s.get_data() == share.get_data()) && share.get_threshold() != 1 {
-            // When threshold = 1, shares data can be the same
-            bail!(ErrorKind::DuplicateShareData(id));
-        }
-
         result.push(share);
     }
 
diff --git a/tests/recovery_errors.rs b/tests/recovery_errors.rs
index 7a36dbe..d986b69 100644
--- a/tests/recovery_errors.rs
+++ b/tests/recovery_errors.rs
@@ -66,17 +66,6 @@ fn test_recover_duplicate_shares_number() {
     recover_secret(&shares, false).unwrap();
 }
 
-#[test]
-#[should_panic(expected = "DuplicateShareData")]
-fn test_recover_duplicate_shares_data() {
-    let share1 = "2-1-CgnlCxRNtnkzENE".to_string();
-    let share2 = "2-2-CgnlCxRNtnkzENE".to_string();
-
-    let shares = vec![share1, share2];
-
-    recover_secret(&shares, false).unwrap();
-}
-
 #[test]
 #[should_panic(expected = "MissingShares")]
 fn test_recover_too_few_shares() {
diff --git a/tests/ss1_recovery_errors.rs b/tests/ss1_recovery_errors.rs
index 4ab71a2..1ad4d11 100644
--- a/tests/ss1_recovery_errors.rs
+++ b/tests/ss1_recovery_errors.rs
@@ -135,32 +135,6 @@ fn test_recover_duplicate_shares_number() {
     recover_secret(&shares).unwrap();
 }
 
-#[test]
-#[should_panic(expected = "DuplicateShareData")]
-fn test_recover_duplicate_shares_data() {
-    let hash = get_test_hash();
-    let share1 = Share {
-        id: 1,
-        threshold: TEST_THRESHOLD,
-        shares_count: TEST_SHARES_COUNT,
-        data: "1YAYwmOHqZ69jA".to_string().into_bytes(),
-        hash: hash.clone(),
-        metadata: None,
-    };
-    let share2 = Share {
-        id: 2,
-        threshold: TEST_THRESHOLD,
-        shares_count: TEST_SHARES_COUNT,
-        data: "1YAYwmOHqZ69jA".to_string().into_bytes(),
-        hash: hash.clone(),
-        metadata: None,
-    };
-
-    let shares = vec![share1, share2];
-
-    recover_secret(&shares).unwrap();
-}
-
 #[test]
 #[should_panic(expected = "MissingShares")]
 fn test_recover_too_few_shares() {
diff --git a/tests/thss_recovery_errors.rs b/tests/thss_recovery_errors.rs
index 4c6c58a..173680a 100644
--- a/tests/thss_recovery_errors.rs
+++ b/tests/thss_recovery_errors.rs
@@ -106,29 +106,6 @@ fn test_recover_duplicate_shares_number() {
     recover_secret(&shares).unwrap();
 }
 
-#[test]
-#[should_panic(expected = "DuplicateShareData")]
-fn test_recover_duplicate_shares_data() {
-    let share1 = Share {
-        id: 1,
-        threshold: 2,
-        shares_count: 2,
-        data: "1YAYwmOHqZ69jA".to_string().into_bytes(),
-        metadata: None,
-    };
-    let share2 = Share {
-        id: 2,
-        threshold: 2,
-        shares_count: 2,
-        data: "1YAYwmOHqZ69jA".to_string().into_bytes(),
-        metadata: None,
-    };
-
-    let shares = vec![share1, share2];
-
-    recover_secret(&shares).unwrap();
-}
-
 #[test]
 #[should_panic(expected = "MissingShares")]
 fn test_recover_too_few_shares() {