From 044844c0a0ec79b699ad84ef2c722a09113ea504 Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@electriccoin.co>
Date: Wed, 15 Dec 2021 13:48:59 +0000
Subject: [PATCH] Reject the identity in `SpendValidatingKey::from_bytes`

`ak_P` is not allowed to be the identity in the Orchard protocol. We
were enforcing this by construction in most places, except for the
parsing of an Orchard full viewing key.

Closes zcash/orchard#261.
---
 src/keys.rs | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/keys.rs b/src/keys.rs
index 2c9d7b34..e8bd210a 100644
--- a/src/keys.rs
+++ b/src/keys.rs
@@ -181,14 +181,17 @@ impl SpendValidatingKey {
     pub(crate) fn from_bytes(bytes: &[u8]) -> Option<Self> {
         <[u8; 32]>::try_from(bytes)
             .ok()
-            .and_then(|b|
-                // check that the sign of the y-coordinate is positive
-                if b[31] & 0x80 == 0 {
+            .and_then(|b| {
+                // Structural validity checks for ak_P:
+                // - The point must not be the identity
+                //   (which for Pallas is canonically encoded as all-zeroes).
+                // - The sign of the y-coordinate must be positive.
+                if b != [0; 32] && b[31] & 0x80 == 0 {
                     <redpallas::VerificationKey<SpendAuth>>::try_from(b).ok()
                 } else {
                     None
                 }
-            )
+            })
             .map(SpendValidatingKey)
     }
 }
@@ -838,6 +841,12 @@ mod tests {
         Note,
     };
 
+    #[test]
+    fn spend_validating_key_from_bytes() {
+        // ak_P must not be the identity.
+        assert!(SpendValidatingKey::from_bytes(&[0; 32]).is_none());
+    }
+
     #[test]
     fn parsers_reject_invalid() {
         assert!(bool::from(