From 47e9d68ef1879cbdf95cb892d7dcd700486ff643 Mon Sep 17 00:00:00 2001 From: parazyd Date: Tue, 20 Feb 2024 14:55:57 +0100 Subject: [PATCH] sdk/crypto: Forbid PublicKey to ever be the identity point --- bin/drk/src/dao.rs | 2 +- src/contract/dao/src/client/auth_xfer.rs | 5 ++-- src/contract/dao/src/client/vote.rs | 2 +- src/contract/dao/tests/integration.rs | 6 ++--- src/sdk/src/crypto/diffie_hellman.rs | 5 ++-- src/sdk/src/crypto/ecvrf.rs | 5 ---- src/sdk/src/crypto/keypair.rs | 30 +++++++++++++++++++----- src/sdk/src/crypto/note.rs | 22 +++++++++-------- tests/halo2_vk_ser.rs | 3 ++- tests/zkvm_opcodes.rs | 3 ++- 10 files changed, 51 insertions(+), 32 deletions(-) diff --git a/bin/drk/src/dao.rs b/bin/drk/src/dao.rs index 3e231d3dc..ecae72668 100644 --- a/bin/drk/src/dao.rs +++ b/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; diff --git a/src/contract/dao/src/client/auth_xfer.rs b/src/contract/dao/src/client/auth_xfer.rs index 784609352..fde39f3f4 100644 --- a/src/contract/dao/src/client/auth_xfer.rs +++ b/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 }; diff --git a/src/contract/dao/src/client/vote.rs b/src/contract/dao/src/client/vote.rs index 770a092f2..68a11b0ed 100644 --- a/src/contract/dao/src/client/vote.rs +++ b/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, diff --git a/src/contract/dao/tests/integration.rs b/src/contract/dao/tests/integration.rs index 94ea3f3ef..13deb4164 100644 --- a/src/contract/dao/tests/integration.rs +++ b/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; diff --git a/src/sdk/src/crypto/diffie_hellman.rs b/src/sdk/src/crypto/diffie_hellman.rs index f28c7b0e4..f815d0abc 100644 --- a/src/sdk/src/crypto/diffie_hellman.rs +++ b/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 { 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. diff --git a/src/sdk/src/crypto/ecvrf.rs b/src/sdk/src/crypto/ecvrf.rs index 13e3b8625..aaeae6f72 100644 --- a/src/sdk/src/crypto/ecvrf.rs +++ b/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); diff --git a/src/sdk/src/crypto/keypair.rs b/src/sdk/src/crypto/keypair.rs index cb882b52e..4960d77fb 100644 --- a/src/sdk/src/crypto/keypair.rs +++ b/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 { - match pallas::Point::from_bytes(&bytes).into() { - Some(k) => Ok(Self(k)), + match as Into>>::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 for PublicKey { - fn from(x: pallas::Point) -> Self { - Self(x) +impl TryFrom for PublicKey { + type Error = ContractError; + + fn try_from(x: pallas::Point) -> Result { + if bool::from(x.is_identity()) { + return Err(ContractError::IoError( + "Could not convert identity point to PublicKey".to_string(), + )) + } + + Ok(Self(x)) } } diff --git a/src/sdk/src/crypto/note.rs b/src/sdk/src/crypto/note.rs index a4c0488dd..3f20ff637 100644 --- a/src/sdk/src/crypto/note.rs +++ b/src/sdk/src/crypto/note.rs @@ -45,7 +45,7 @@ impl AeadEncryptedNote { ) -> Result { 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(&self, secret: &SecretKey) -> Result { - 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 ElGamalEncryptedNote { values: [pallas::Base; N], ephem_secret: &SecretKey, public: &PublicKey, - ) -> Self { + ) -> Result { // 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 ElGamalEncryptedNote { 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 ElGamalEncryptedNote { /// 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 ElGamalEncryptedNote { 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); } diff --git a/tests/halo2_vk_ser.rs b/tests/halo2_vk_ser.rs index 9231698bb..e807ead10 100644 --- a/tests/halo2_vk_ser.rs +++ b/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())), diff --git a/tests/zkvm_opcodes.rs b/tests/zkvm_opcodes.rs index c6338d1bd..b78d599b8 100644 --- a/tests/zkvm_opcodes.rs +++ b/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))),