sdk/crypto: Forbid PublicKey to ever be the identity point

bin/drk/src/dao.rs

@@ -799,7 +799,7 @@ impl Drk {
             for vote in new_dao_votes {
                 for dao in &daos {
                     // TODO: we shouldn't decrypt with all DAOs here
-                    let note = vote.0.note.decrypt_unsafe(&dao.secret_key);
+                    let note = vote.0.note.decrypt_unsafe(&dao.secret_key)?;
                     eprintln!("Managed to decrypt DAO proposal vote note");
                     let daos_proposals = self.get_dao_proposals(dao.id).await?;
                     let mut proposal_id = None;

src/contract/dao/src/client/auth_xfer.rs

@@ -72,7 +72,7 @@ impl DaoAuthMoneyTransferCall {
                 coin_attrs.blind.inner(),
             ];
             let enc_note =
-                ElGamalEncryptedNote::encrypt_unsafe(note, &ephem_secret, &coin_attrs.public_key);
+                ElGamalEncryptedNote::encrypt_unsafe(note, &ephem_secret, &coin_attrs.public_key)?;
 
             let prover_witnesses = vec![
                 Witness::EcNiPoint(Value::known(coin_attrs.public_key.inner())),
@@ -117,8 +117,9 @@ impl DaoAuthMoneyTransferCall {
             self.dao_coin_attrs.token_id.inner(),
             self.dao_coin_attrs.blind.inner(),
         ];
+
         let dao_change_attrs =
-            ElGamalEncryptedNote::encrypt_unsafe(note, &ephem_secret, &self.dao.public_key);
+            ElGamalEncryptedNote::encrypt_unsafe(note, &ephem_secret, &self.dao.public_key)?;
 
         let params = DaoAuthMoneyTransferParams { enc_attrs, dao_change_attrs };
 

src/contract/dao/src/client/vote.rs

@@ -250,7 +250,7 @@ impl DaoVoteCall {
 
         let note = [vote_option, yes_vote_blind.inner(), all_vote_value_fp, all_vote_blind.inner()];
         let enc_note =
-            ElGamalEncryptedNote::encrypt_unsafe(note, &ephem_secret, &self.dao_keypair.public);
+            ElGamalEncryptedNote::encrypt_unsafe(note, &ephem_secret, &self.dao_keypair.public)?;
 
         let public_inputs = vec![
             token_commit,

src/contract/dao/tests/integration.rs

@@ -392,9 +392,9 @@ fn integration_test() -> Result<()> {
         }
 
         // Gather and decrypt all vote notes
-        let vote_note_1 = alice_vote_params.note.decrypt_unsafe(&dao_keypair.secret);
-        let vote_note_2 = bob_vote_params.note.decrypt_unsafe(&dao_keypair.secret);
-        let vote_note_3 = charlie_vote_params.note.decrypt_unsafe(&dao_keypair.secret);
+        let vote_note_1 = alice_vote_params.note.decrypt_unsafe(&dao_keypair.secret).unwrap();
+        let vote_note_2 = bob_vote_params.note.decrypt_unsafe(&dao_keypair.secret).unwrap();
+        let vote_note_3 = charlie_vote_params.note.decrypt_unsafe(&dao_keypair.secret).unwrap();
 
         // Count the votes
         let mut total_yes_vote_value = 0;

src/sdk/src/crypto/diffie_hellman.rs

@@ -20,18 +20,19 @@ use blake2b_simd::{Hash as Blake2bHash, Params as Blake2bParams};
 use pasta_curves::group::{GroupEncoding, Wnaf};
 
 use super::{util::fp_mod_fv, PublicKey, SecretKey};
+use crate::error::ContractError;
 
 pub const KDF_SAPLING_PERSONALIZATION: &[u8; 16] = b"DarkFiSaplingKDF";
 
 /// Sapling key agreement for note encryption.
 /// Implements section 5.4.4.3 of the Zcash Protocol Specification
-pub fn sapling_ka_agree(esk: &SecretKey, pk_d: &PublicKey) -> PublicKey {
+pub fn sapling_ka_agree(esk: &SecretKey, pk_d: &PublicKey) -> Result<PublicKey, ContractError> {
     let esk_s = fp_mod_fv(esk.inner());
     // Windowed multiplication is constant time. Hence that is used here vs naive EC mult.
     // Decrypting notes is a an amortized operation, so you want successful rare-case note
     // decryptions to be indistinguishable from the usual case.
     let mut wnaf = Wnaf::new();
-    PublicKey::from(wnaf.scalar(&esk_s).base(pk_d.inner()))
+    PublicKey::try_from(wnaf.scalar(&esk_s).base(pk_d.inner()))
 }
 
 /// Sapling KDF for note encryption.

src/sdk/src/crypto/ecvrf.rs

@@ -54,7 +54,6 @@ impl VrfProof {
     /// and a seed input `alpha_string`.
     pub fn prove(x: SecretKey, alpha_string: &[u8]) -> Self {
         let Y = PublicKey::from_secret(x);
-        assert!(!bool::from(Y.inner().is_identity()));
 
         let mut message = vec![];
         message.extend_from_slice(&Y.to_bytes());
@@ -86,10 +85,6 @@ impl VrfProof {
 
     /// Verify a `VrfProof` given a `Publickey` and a seed input `alpha_string`.
     pub fn verify(&self, Y: PublicKey, alpha_string: &[u8]) -> bool {
-        if bool::from(Y.inner().is_identity()) {
-            return false
-        }
-
         let mut message = vec![];
         message.extend_from_slice(&Y.to_bytes());
         message.extend_from_slice(alpha_string);

src/sdk/src/crypto/keypair.rs

@@ -26,7 +26,7 @@ use pasta_curves::{
     arithmetic::CurveAffine,
     group::{
         ff::{Field, PrimeField},
-        Curve, GroupEncoding,
+        Curve, Group, GroupEncoding,
     },
     pallas,
 };
@@ -136,8 +136,18 @@ impl PublicKey {
     /// Instantiate a `PublicKey` given 32 bytes. Returns an error
     /// if the representation is noncanonical.
     pub fn from_bytes(bytes: [u8; 32]) -> Result<Self, ContractError> {
-        match pallas::Point::from_bytes(&bytes).into() {
-            Some(k) => Ok(Self(k)),
+        match <subtle::CtOption<pallas::Point> as Into<Option<pallas::Point>>>::into(
+            pallas::Point::from_bytes(&bytes),
+        ) {
+            Some(k) => {
+                if bool::from(k.is_identity()) {
+                    return Err(ContractError::IoError(
+                        "Could not convert bytes to PublicKey".to_string(),
+                    ))
+                }
+
+                Ok(Self(k))
+            }
             None => Err(ContractError::IoError("Could not convert bytes to PublicKey".to_string())),
         }
     }
@@ -164,9 +174,17 @@ impl PublicKey {
     }
 }
 
-impl From<pallas::Point> for PublicKey {
-    fn from(x: pallas::Point) -> Self {
-        Self(x)
+impl TryFrom<pallas::Point> for PublicKey {
+    type Error = ContractError;
+
+    fn try_from(x: pallas::Point) -> Result<Self, Self::Error> {
+        if bool::from(x.is_identity()) {
+            return Err(ContractError::IoError(
+                "Could not convert identity point to PublicKey".to_string(),
+            ))
+        }
+
+        Ok(Self(x))
     }
 }
 

src/sdk/src/crypto/note.rs

@@ -45,7 +45,7 @@ impl AeadEncryptedNote {
     ) -> Result<Self, ContractError> {
         let ephem_secret = SecretKey::random(rng);
         let ephem_public = PublicKey::from_secret(ephem_secret);
-        let shared_secret = diffie_hellman::sapling_ka_agree(&ephem_secret, public);
+        let shared_secret = diffie_hellman::sapling_ka_agree(&ephem_secret, public)?;
         let key = diffie_hellman::kdf_sapling(&shared_secret, &ephem_public);
 
         let mut input = Vec::new();
@@ -63,7 +63,7 @@ impl AeadEncryptedNote {
     }
 
     pub fn decrypt<D: Decodable>(&self, secret: &SecretKey) -> Result<D, ContractError> {
-        let shared_secret = diffie_hellman::sapling_ka_agree(secret, &self.ephem_public);
+        let shared_secret = diffie_hellman::sapling_ka_agree(secret, &self.ephem_public)?;
         let key = diffie_hellman::kdf_sapling(&shared_secret, &self.ephem_public);
 
         let ct_len = self.ciphertext.len();
@@ -104,10 +104,11 @@ impl<const N: usize> ElGamalEncryptedNote<N> {
         values: [pallas::Base; N],
         ephem_secret: &SecretKey,
         public: &PublicKey,
-    ) -> Self {
+    ) -> Result<Self, ContractError> {
         // Derive shared secret using DH
         let ephem_public = PublicKey::from_secret(*ephem_secret);
-        let (ss_x, ss_y) = PublicKey::from(public.inner() * fp_mod_fv(ephem_secret.inner())).xy();
+        let (ss_x, ss_y) =
+            PublicKey::try_from(public.inner() * fp_mod_fv(ephem_secret.inner()))?.xy();
         let shared_secret = poseidon_hash([ss_x, ss_y]);
 
         // Derive the blinds using the shared secret and incremental nonces
@@ -122,7 +123,7 @@ impl<const N: usize> ElGamalEncryptedNote<N> {
             encrypted_values[i] = values[i] + blinds[i];
         }
 
-        Self { encrypted_values, ephem_public }
+        Ok(Self { encrypted_values, ephem_public })
     }
 
     /// Decrypt the `ElGamalEncryptedNote` using a `SecretKey` for shared secret derivation
@@ -131,10 +132,10 @@ impl<const N: usize> ElGamalEncryptedNote<N> {
     /// Note that this does not do any message authentication.
     /// This means that alterations of the ciphertexts lead to the same alterations
     /// on the plaintexts.
-    pub fn decrypt_unsafe(&self, secret: &SecretKey) -> [pallas::Base; N] {
+    pub fn decrypt_unsafe(&self, secret: &SecretKey) -> Result<[pallas::Base; N], ContractError> {
         // Derive shared secret using DH
         let (ss_x, ss_y) =
-            PublicKey::from(self.ephem_public.inner() * fp_mod_fv(secret.inner())).xy();
+            PublicKey::try_from(self.ephem_public.inner() * fp_mod_fv(secret.inner()))?.xy();
         let shared_secret = poseidon_hash([ss_x, ss_y]);
 
         let mut blinds = [pallas::Base::ZERO; N];
@@ -147,7 +148,7 @@ impl<const N: usize> ElGamalEncryptedNote<N> {
             decrypted_values[i] = self.encrypted_values[i] - blinds[i];
         }
 
-        decrypted_values
+        Ok(decrypted_values)
     }
 }
 
@@ -181,9 +182,10 @@ mod tests {
         let ephem_secret = SecretKey::random(&mut OsRng);
 
         let encrypted_note =
-            ElGamalEncryptedNote::encrypt_unsafe(plain_values, &ephem_secret, &keypair.public);
+            ElGamalEncryptedNote::encrypt_unsafe(plain_values, &ephem_secret, &keypair.public)
+                .unwrap();
 
-        let decrypted_values = encrypted_note.decrypt_unsafe(&keypair.secret);
+        let decrypted_values = encrypted_note.decrypt_unsafe(&keypair.secret).unwrap();
 
         assert_eq!(plain_values, decrypted_values);
     }

tests/halo2_vk_ser.rs

@@ -117,7 +117,8 @@ fn halo2_vk_ser() -> Result<()> {
 
     let ephem_secret = SecretKey::random(&mut OsRng);
     let pubkey = PublicKey::from_secret(ephem_secret).inner();
-    let (ephem_x, ephem_y) = PublicKey::from(pubkey * fp_mod_fv(ephem_secret.inner())).xy();
+    let (ephem_x, ephem_y) =
+        PublicKey::try_from(pubkey * fp_mod_fv(ephem_secret.inner())).unwrap().xy();
     let prover_witnesses = vec![
         Witness::Base(Value::known(pallas::Base::from(value))),
         Witness::Scalar(Value::known(value_blind.inner())),

tests/zkvm_opcodes.rs

@@ -79,7 +79,8 @@ fn zkvm_opcodes() -> Result<()> {
 
     let ephem_secret = SecretKey::random(&mut OsRng);
     let pubkey = PublicKey::from_secret(ephem_secret).inner();
-    let (ephem_x, ephem_y) = PublicKey::from(pubkey * fp_mod_fv(ephem_secret.inner())).xy();
+    let (ephem_x, ephem_y) =
+        PublicKey::try_from(pubkey * fp_mod_fv(ephem_secret.inner())).unwrap().xy();
 
     let prover_witnesses = vec![
         Witness::Base(Value::known(pallas::Base::from(value))),