Add point validity check for pk-aggregation

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Check validity of `PublicKey` and `Signature` points in signature verification [#7]
+- Check validity of `PublicKey` points when aggregating them [#8]
 
 ### Added
 
@@ -34,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add initial commit, this package continues the development of [dusk-bls12_381-sign](https://github.com/dusk-network/bls12_381-sign/) at version `0.6.0` under the new name: `bls12_381-bls` and without the go related code.
 
 <!-- ISSUES -->
+[#8]: https://github.com/dusk-network/bls12_381-bls/issues/8
 [#7]: https://github.com/dusk-network/bls12_381-bls/issues/7
 [#5]: https://github.com/dusk-network/bls12_381-bls/issues/5
 [#3]: https://github.com/dusk-network/bls12_381-bls/issues/3

src/keys/apk.rs

@@ -58,17 +58,34 @@ impl From<&SecretKey> for APK {
 
 impl APK {
     /// Aggregate a set of [`PublicKey`] into the [`APK`].
-    pub fn aggregate(&mut self, pks: &[PublicKey]) {
+    ///
+    /// # Errors
+    ///
+    /// The aggregation errors when one of the [`PublicKey`]s is made of the
+    /// identity or an otherwise invalid point.
+    pub fn aggregate(&mut self, pks: &[PublicKey]) -> Result<(), Error> {
         #[cfg(feature = "parallel")]
         let iter = pks.par_iter();
 
         #[cfg(not(feature = "parallel"))]
         let iter = pks.iter();
 
+        let mut is_valid = self.0.is_valid();
         let sum: G2Projective = iter
-            .map(|pk| dusk_bls12_381::G2Projective::from(pk.pk_t()))
+            .map(|pk| {
+                if !pk.is_valid() {
+                    is_valid = false;
+                }
+                G2Projective::from(pk.pk_t())
+            })
             .sum();
-        (self.0).0 = ((self.0).0 + sum).into();
+
+        if !is_valid {
+            return Err(Error::InvalidPoint);
+        }
+
+        self.0 .0 = (self.0 .0 + sum).into();
+        Ok(())
     }
 
     /// Verify a [`Signature`].

tests/keys.rs

@@ -0,0 +1,66 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+//
+// Copyright (c) DUSK NETWORK. All rights reserved.
+
+use bls12_381_bls::{Error, PublicKey, SecretKey, APK};
+use dusk_bls12_381::BlsScalar;
+use dusk_bytes::Serializable;
+use rand::rngs::StdRng;
+use rand::SeedableRng;
+use zeroize::Zeroize;
+
+#[test]
+fn sk_zeroize() {
+    let secret = BlsScalar::from(42);
+    let mut sk = SecretKey::from(secret);
+
+    sk.zeroize();
+    assert_eq!(sk, SecretKey::default());
+}
+
+#[test]
+fn keys_encoding() {
+    let mut rng = StdRng::seed_from_u64(0xbeef);
+    let sk = SecretKey::random(&mut rng);
+    let pk = PublicKey::from(&sk);
+    let apk = APK::from(&pk);
+
+    assert_eq!(sk, SecretKey::from_bytes(&sk.to_bytes()).unwrap());
+    assert_eq!(pk, PublicKey::from_bytes(&pk.to_bytes()).unwrap());
+    assert_eq!(apk, APK::from_bytes(&apk.to_bytes()).unwrap());
+}
+
+#[test]
+fn apk_identity_fails() {
+    let mut rng = StdRng::seed_from_u64(0xba0bab);
+
+    let sk = SecretKey::random(&mut rng);
+    let pk = PublicKey::from(&sk);
+    let sk2 = SecretKey::random(&mut rng);
+    let pk2 = PublicKey::from(&sk2);
+    let sk3 = SecretKey::random(&mut rng);
+    let pk3 = PublicKey::from(&sk3);
+    let identity = PublicKey::from(&SecretKey::from(BlsScalar::zero()));
+
+    let mut apk = APK::from(&pk);
+    assert_eq!(
+        apk.aggregate(&[identity, pk2, pk3]).unwrap_err(),
+        Error::InvalidPoint
+    );
+    assert_eq!(
+        apk.aggregate(&[pk2, identity, pk3]).unwrap_err(),
+        Error::InvalidPoint
+    );
+    assert_eq!(
+        apk.aggregate(&[pk2, pk3, identity]).unwrap_err(),
+        Error::InvalidPoint
+    );
+
+    let mut apk = APK::from(&identity);
+    assert_eq!(
+        apk.aggregate(&[pk, pk2, pk3]).unwrap_err(),
+        Error::InvalidPoint
+    );
+}

tests/signature.rs

@@ -124,7 +124,8 @@ fn sign_verify_aggregated() {
         agg_sig = agg_sig.aggregate(&[sig]);
         pks.push(pk)
     }
-    apk.aggregate(&pks[..]);
+    apk.aggregate(&pks[..])
+        .expect("public keys should be valid");
 
     assert!(apk.verify(&agg_sig, &msg).is_ok());
 }
@@ -146,7 +147,7 @@ fn sign_verify_aggregated_incorrect_message() {
         let pk = PublicKey::from(&sk);
         let sig = sk.sign(&pk, &msg);
         agg_sig = agg_sig.aggregate(&[sig]);
-        apk.aggregate(&[pk]);
+        apk.aggregate(&[pk]).expect("public keys should be valid");
     }
 
     // Verification should fail with a different message.
@@ -171,7 +172,7 @@ fn sign_verify_aggregated_incorrect_apk() {
         let pk = PublicKey::from(&sk);
         let sig = sk.sign(&pk, &msg);
         agg_sig = agg_sig.aggregate(&[sig]);
-        apk.aggregate(&[pk]);
+        apk.aggregate(&[pk]).expect("public keys should be valid");
     }
 
     // Verification with the wrong APK should fail.