Preventing Asset Base from being the identity point on the Pallas curve (#71)

As in the title, this is done in two portions:
- A protection is added to `AssetBase::derive()`, which panics if the
output is going to be the identity point. This panic will occur with
negligible probability due to the properties of the hash.
- The `verify_supply()` function now returns an error if the Asset Base
of the notes involved is the identity point.
- A number of tests are added to ensure the `verify_supply`, `verify_issue_bundle` functions raise errors appropriately, and also to confirm that the issue bundle cannot be signed when the asset base is the identity point.

---------

Co-authored-by: Paul <3682187+PaulLaux@users.noreply.github.com>

src/issuance.rs

@@ -1,5 +1,6 @@
 //! Structs related to issuance bundles and the associated logic.
 use blake2b_simd::Hash as Blake2bHash;
+use group::Group;
 use nonempty::NonEmpty;
 use rand::{CryptoRng, RngCore};
 use std::collections::HashSet;
@@ -7,7 +8,7 @@ use std::fmt;
 
 use crate::bundle::commitments::{hash_issue_bundle_auth_data, hash_issue_bundle_txid_data};
 use crate::issuance::Error::{
-    IssueActionNotFound, IssueActionPreviouslyFinalizedAssetBase,
+    AssetBaseCannotBeIdentityPoint, IssueActionNotFound, IssueActionPreviouslyFinalizedAssetBase,
     IssueActionWithoutNoteNotFinalized, IssueBundleIkMismatchAssetBase,
     IssueBundleInvalidSignature, ValueSumOverflow, WrongAssetDescSize,
 };
@@ -134,6 +135,11 @@ impl IssueAction {
             .notes
             .iter()
             .try_fold(ValueSum::zero(), |value_sum, &note| {
+                //The asset base should not be the identity point of the Pallas curve.
+                if bool::from(note.asset().cv_base().is_identity()) {
+                    return Err(AssetBaseCannotBeIdentityPoint);
+                }
+
                 // All assets should be derived correctly
                 note.asset()
                     .eq(&issue_asset)
@@ -527,6 +533,8 @@ pub enum Error {
     WrongAssetDescSize,
     /// The `IssueAction` is not finalized but contains no notes.
     IssueActionWithoutNoteNotFinalized,
+    /// The `AssetBase` is the Pallas identity point, which is invalid.
+    AssetBaseCannotBeIdentityPoint,
 
     /// Verification errors:
     /// Invalid signature.
@@ -561,6 +569,12 @@ impl fmt::Display for Error {
                     "this `IssueAction` contains no notes but is not finalized"
                 )
             }
+            AssetBaseCannotBeIdentityPoint => {
+                write!(
+                    f,
+                    "the AssetBase is the identity point of the Pallas curve, which is invalid."
+                )
+            }
             IssueBundleInvalidSignature(_) => {
                 write!(f, "invalid signature")
             }
@@ -581,10 +595,11 @@ impl fmt::Display for Error {
 mod tests {
     use super::{AssetSupply, IssueBundle, IssueInfo};
     use crate::issuance::Error::{
-        IssueActionNotFound, IssueActionPreviouslyFinalizedAssetBase,
-        IssueBundleIkMismatchAssetBase, IssueBundleInvalidSignature, WrongAssetDescSize,
+        AssetBaseCannotBeIdentityPoint, IssueActionNotFound,
+        IssueActionPreviouslyFinalizedAssetBase, IssueBundleIkMismatchAssetBase,
+        IssueBundleInvalidSignature, WrongAssetDescSize,
     };
-    use crate::issuance::{verify_issue_bundle, IssueAction, Signed};
+    use crate::issuance::{verify_issue_bundle, IssueAction, Signed, Unauthorized};
     use crate::keys::{
         FullViewingKey, IssuanceAuthorizingKey, IssuanceKey, IssuanceValidatingKey, Scope,
         SpendingKey,
@@ -592,6 +607,9 @@ mod tests {
     use crate::note::{AssetBase, Nullifier};
     use crate::value::{NoteValue, ValueSum};
     use crate::{Address, Note};
+    use group::{Group, GroupEncoding};
+    use nonempty::NonEmpty;
+    use pasta_curves::pallas::{Point, Scalar};
     use rand::rngs::OsRng;
     use rand::RngCore;
     use reddsa::Error::InvalidSignature;
@@ -654,8 +672,49 @@ mod tests {
         )
     }
 
+    // This function computes the identity point on the Pallas curve and returns an Asset Base with that value.
+    fn identity_point() -> AssetBase {
+        let identity_point = (Point::generator() * -Scalar::one()) + Point::generator();
+        AssetBase::from_bytes(&identity_point.to_bytes()).unwrap()
+    }
+
+    fn identity_point_test_params(
+        note1_value: u64,
+        note2_value: u64,
+    ) -> (
+        OsRng,
+        IssuanceAuthorizingKey,
+        IssueBundle<Unauthorized>,
+        [u8; 32],
+    ) {
+        let (mut rng, isk, ik, recipient, sighash) = setup_params();
+
+        let note1 = Note::new(
+            recipient,
+            NoteValue::from_raw(note1_value),
+            identity_point(),
+            Nullifier::dummy(&mut rng),
+            &mut rng,
+        );
+
+        let note2 = Note::new(
+            recipient,
+            NoteValue::from_raw(note2_value),
+            identity_point(),
+            Nullifier::dummy(&mut rng),
+            &mut rng,
+        );
+
+        let action =
+            IssueAction::from_parts("arbitrary asset_desc".into(), vec![note1, note2], false);
+
+        let bundle = IssueBundle::from_parts(ik, NonEmpty::new(action), Unauthorized);
+
+        (rng, isk, bundle, sighash)
+    }
+
     #[test]
-    fn test_verify_supply_valid() {
+    fn verify_supply_valid() {
         let (ik, test_asset, action) =
             setup_verify_supply_test_params(10, 20, "Asset 1", None, false);
 
@@ -671,7 +730,17 @@ mod tests {
     }
 
     #[test]
-    fn test_verify_supply_finalized() {
+    fn verify_supply_invalid_for_asset_base_as_identity() {
+        let (_, _, bundle, _) = identity_point_test_params(10, 20);
+
+        assert_eq!(
+            bundle.actions.head.verify_supply(&bundle.ik),
+            Err(AssetBaseCannotBeIdentityPoint)
+        );
+    }
+
+    #[test]
+    fn verify_supply_finalized() {
         let (ik, test_asset, action) =
             setup_verify_supply_test_params(10, 20, "Asset 1", None, true);
 
@@ -687,7 +756,7 @@ mod tests {
     }
 
     #[test]
-    fn test_verify_supply_incorrect_asset_base() {
+    fn verify_supply_incorrect_asset_base() {
         let (ik, _, action) =
             setup_verify_supply_test_params(10, 20, "Asset 1", Some("Asset 2"), false);
 
@@ -698,7 +767,7 @@ mod tests {
     }
 
     #[test]
-    fn test_verify_supply_ik_mismatch_asset_base() {
+    fn verify_supply_ik_mismatch_asset_base() {
         let (_, _, action) = setup_verify_supply_test_params(10, 20, "Asset 1", None, false);
         let (_, _, ik, _, _) = setup_params();
 
@@ -1273,7 +1342,35 @@ mod tests {
     }
 
     #[test]
-    fn test_finalize_flag_serialization() {
+    fn issue_bundle_cannot_be_signed_with_asset_base_identity_point() {
+        let (rng, isk, bundle, sighash) = identity_point_test_params(10, 20);
+
+        assert_eq!(
+            bundle.prepare(sighash).sign(rng, &isk).unwrap_err(),
+            AssetBaseCannotBeIdentityPoint
+        );
+    }
+
+    #[test]
+    fn issue_bundle_verify_fail_asset_base_identity_point() {
+        let (mut rng, isk, bundle, sighash) = identity_point_test_params(10, 20);
+
+        let signed = IssueBundle {
+            ik: bundle.ik,
+            actions: bundle.actions,
+            authorization: Signed {
+                signature: isk.sign(&mut rng, &sighash),
+            },
+        };
+
+        assert_eq!(
+            verify_issue_bundle(&signed, sighash, &HashSet::new()).unwrap_err(),
+            AssetBaseCannotBeIdentityPoint
+        );
+    }
+
+    #[test]
+    fn finalize_flag_serialization() {
         let mut rng = OsRng;
         let (_, _, note) = Note::dummy(&mut rng, None, AssetBase::native());
 

src/note/asset_base.rs

@@ -1,5 +1,5 @@
 use blake2b_simd::{Hash as Blake2bHash, Params};
-use group::GroupEncoding;
+use group::{Group, GroupEncoding};
 use halo2_proofs::arithmetic::CurveExt;
 use pasta_curves::pallas;
 use rand::RngCore;
@@ -54,10 +54,13 @@ impl AssetBase {
     ///
     /// # Panics
     ///
-    /// Panics if `asset_desc` is empty or greater than `MAX_ASSET_DESCRIPTION_SIZE`.
+    /// Panics if `asset_desc` is empty or greater than `MAX_ASSET_DESCRIPTION_SIZE` or if the derived Asset Base is the identity point.
     #[allow(non_snake_case)]
     pub fn derive(ik: &IssuanceValidatingKey, asset_desc: &str) -> Self {
-        assert!(is_asset_desc_of_valid_size(asset_desc));
+        assert!(
+            is_asset_desc_of_valid_size(asset_desc),
+            "The asset_desc string is not of valid size"
+        );
 
         // EncodeAssetId(ik, asset_desc) = version_byte || ik || asset_desc
         let version_byte = [0x00];
@@ -65,10 +68,17 @@ impl AssetBase {
 
         let asset_digest = asset_digest(encode_asset_id);
 
+        let asset_base =
+            pallas::Point::hash_to_curve(ZSA_ASSET_BASE_PERSONALIZATION)(asset_digest.as_bytes());
+
+        // this will happen with negligible probability.
+        assert!(
+            bool::from(!asset_base.is_identity()),
+            "The Asset Base is the identity point, which is invalid."
+        );
+
         // AssetBase = ZSAValueBase(AssetDigest)
-        AssetBase(
-            pallas::Point::hash_to_curve(ZSA_ASSET_BASE_PERSONALIZATION)(asset_digest.as_bytes()),
-        )
+        AssetBase(asset_base)
     }
 
     /// Note type for the "native" currency (zec), maintains backward compatibility with Orchard untyped notes.