Missing check in signature verification

[moCello]

Summary

From audit finding BLS-03:

Secure verification of BLS signatures requires several checks, as
described in the reference IETF Internet Draft
(section 2.7):

result = CoreVerify(PK, message, signature)

Inputs:
- PK, a public key in the format output by SkToPk.
- message, an octet string.
- signature, an octet string in the format output by CoreSign.

Outputs:
- result, either VALID or INVALID.

Procedure:
1. R = signature_to_point(signature)
2. If R is INVALID, return INVALID
3. If signature_subgroup_check(R) is INVALID, return INVALID
4. If KeyValidate(PK) is INVALID, return INVALID
5. xP = pubkey_to_point(PK)
6. Q = hash_to_point(message)
7. C1 = pairing(Q, xP)
8. C2 = pairing(R, P)
9. If C1 == C2, return VALID, else return INVALID

where KeyValidate() checks that a public key is a curve point in the
subgroup and is not the identity element.

However, the implementation in keys/public.rs does not do all these
required checks:

impl PublicKey {
    /// Verify a [`Signature`] by comparing the results of the two pairing
    /// operations: e(sig, g_2) == e(Hₒ(m), pk).
    pub fn verify(&self, sig: &Signature, msg: &[u8]) -> Result<(), Error> {
        let h0m = h0(msg);
        let p1 = dusk_bls12_381::pairing(&sig.0, &G2Affine::generator());
        let p2 = dusk_bls12_381::pairing(&h0m, &self.0);

        if p1.eq(&p2) {
            Ok(())
        } else {
            Err(Error::InvalidSignature)
        }
    }

Specifically, the public key may be checked to be on the curve and in
the subgroup when instantiated with from_bytes() or from a secret key,
however these do not guarantee that the point is not the identity. To
check the public key, the is_valid() defined in keys/public.rs may be
used.

The signature, as a point, would be checked to be valid (on the curve,
in the subgroup) when instantiated via the from_bytes() method of the
underlying library.

To verify that an invalid public key is accepted in signature generation
and verification, we can run the following test:

fn pktest() {

    let rng = &mut StdRng::seed_from_u64(0xbeef);
    let msg = random_message(rng);
    let sk = SecretKey::from(BlsScalar::zero());
    let pk = PublicKey::from(&sk);
    let sig = sk.sign_vulnerable(&msg);

    assert!(pk.verify(&sig, &msg).is_ok());
    assert!(pk.is_valid())
}

The first assert will pass, only the second will fail:

thread 'pktest' panicked at tests/signature.rs:24:5:
assertion failed: pk.is_valid()