fix(ecc): checked from_be_bytes (#1746)

Fixes some panics in `from_sec1_bytes` where it should error instead.

However the root cause of many places is that
`IntMod::from_{le,be}_bytes` previously does not require the input bytes
to be reduced. While this is a feature of how we represent field
elements up to equivalence, in practice in cryptographic applications
other implementations always require the input to be in canonical
(reduced) from. To match other implementations and prevent future
errors, I have renamed the former `from_{le,be}_bytes` to
`from_{le,be}_bytes_unchecked` and added new checked versions that
return `Option`.

Also fixed implementations of `Decompress` for p256,k256 such that it
rejects when the input `x` bytes are not reduced.

Author: jonathanpwang

benchmarks/guest/kitchen-sink/src/main.rs

@@ -97,7 +97,7 @@ pub fn main() {
 
     let mut bytes = [0u8; 32];
     bytes[7] = 1 << 5; // 2^61 = modulus + 1
-    let mut res = Mersenne61::from_le_bytes(&bytes); // No need to start from reduced representation
+    let mut res = Mersenne61::from_le_bytes_unchecked(&bytes); // No need to start from reduced representation
     for _ in 0..61 {
         res += res.clone();
     }

benchmarks/guest/pairing/src/main.rs

@@ -43,7 +43,7 @@ pub fn main() {
             // SAFETY: We're reading `6 * 32 == PAIR_ELEMENT_LEN` bytes from `input[idx..]`
             // per iteration. This is guaranteed to be in-bounds.
             let slice = unsafe { input.get_unchecked(start..start + 32) };
-            Fp::from_be_bytes(&slice[..32])
+            Fp::from_be_bytes_unchecked(&slice[..32])
         };
         // https://eips.ethereum.org/EIPS/eip-197, Fp2 is encoded as (a, b) where a * i + b
         let g1_x = read_fq_at(0);

book/src/custom-extensions/algebra.md

@@ -12,7 +12,7 @@ The functional part is provided by the `openvm-algebra-guest` crate, which is a
   - `Repr` typically is `[u8; NUM_LIMBS]`, representing the number's underlying storage.
   - `MODULUS` is the compile-time known modulus.
   - `ZERO` and `ONE` represent the additive and multiplicative identities, respectively.
-  - Constructors include `from_repr`, `from_le_bytes`, `from_be_bytes`, `from_u8`, `from_u32`, and `from_u64`.
+  - Constructors include `from_repr`, `from_le_bytes`, `from_be_bytes`, `from_le_bytes_unchecked`, `from_be_bytes_unchecked`, `from_u8`, `from_u32`, and `from_u64`.
 
 - `Field` trait:
   Provides constants `ZERO` and `ONE` and methods for basic arithmetic operations within a field.

examples/ecc/src/main.rs

@@ -22,13 +22,13 @@ openvm_ecc_guest::sw_macros::sw_init! {
 // ANCHOR: main
 pub fn main() {
     let x1 = Secp256k1Coord::from_u32(1);
-    let y1 = Secp256k1Coord::from_le_bytes(&hex!(
+    let y1 = Secp256k1Coord::from_le_bytes_unchecked(&hex!(
         "EEA7767E580D75BC6FDD7F58D2A84C2614FB22586068DB63B346C6E60AF21842"
     ));
     let p1 = Secp256k1Point::from_xy_nonidentity(x1, y1).unwrap();
 
     let x2 = Secp256k1Coord::from_u32(2);
-    let y2 = Secp256k1Coord::from_le_bytes(&hex!(
+    let y2 = Secp256k1Coord::from_le_bytes_unchecked(&hex!(
         "D1A847A8F879E0AEE32544DA5BA0B3BD1703A1F52867A5601FF6454DD8180499"
     ));
     let p2 = Secp256k1Point::from_xy_nonidentity(x2, y2).unwrap();

examples/pairing/src/main.rs

@@ -27,31 +27,31 @@ openvm_algebra_complex_macros::complex_init! {
 // ANCHOR: main
 pub fn main() {
     let p0 = AffinePoint::new(
-        Fp::from_be_bytes(&hex!("17f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb")),
-        Fp::from_be_bytes(&hex!("08b3f481e3aaa0f1a09e30ed741d8ae4fcf5e095d5d00af600db18cb2c04b3edd03cc744a2888ae40caa232946c5e7e1"))
+        Fp::from_be_bytes_unchecked(&hex!("17f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb")),
+        Fp::from_be_bytes_unchecked(&hex!("08b3f481e3aaa0f1a09e30ed741d8ae4fcf5e095d5d00af600db18cb2c04b3edd03cc744a2888ae40caa232946c5e7e1"))
     );
     let p1 = AffinePoint::new(
         Fp2::from_coeffs([
-            Fp::from_be_bytes(&hex!("1638533957d540a9d2370f17cc7ed5863bc0b995b8825e0ee1ea1e1e4d00dbae81f14b0bf3611b78c952aacab827a053")),
-            Fp::from_be_bytes(&hex!("0a4edef9c1ed7f729f520e47730a124fd70662a904ba1074728114d1031e1572c6c886f6b57ec72a6178288c47c33577"))
+            Fp::from_be_bytes_unchecked(&hex!("1638533957d540a9d2370f17cc7ed5863bc0b995b8825e0ee1ea1e1e4d00dbae81f14b0bf3611b78c952aacab827a053")),
+            Fp::from_be_bytes_unchecked(&hex!("0a4edef9c1ed7f729f520e47730a124fd70662a904ba1074728114d1031e1572c6c886f6b57ec72a6178288c47c33577"))
         ]),
         Fp2::from_coeffs([
-            Fp::from_be_bytes(&hex!("0468fb440d82b0630aeb8dca2b5256789a66da69bf91009cbfe6bd221e47aa8ae88dece9764bf3bd999d95d71e4c9899")),
-            Fp::from_be_bytes(&hex!("0f6d4552fa65dd2638b361543f887136a43253d9c66c411697003f7a13c308f5422e1aa0a59c8967acdefd8b6e36ccf3"))
+            Fp::from_be_bytes_unchecked(&hex!("0468fb440d82b0630aeb8dca2b5256789a66da69bf91009cbfe6bd221e47aa8ae88dece9764bf3bd999d95d71e4c9899")),
+            Fp::from_be_bytes_unchecked(&hex!("0f6d4552fa65dd2638b361543f887136a43253d9c66c411697003f7a13c308f5422e1aa0a59c8967acdefd8b6e36ccf3"))
         ]),
     );
     let q0 = AffinePoint::new(
-        Fp::from_be_bytes(&hex!("0572cbea904d67468808c8eb50a9450c9721db309128012543902d0ac358a62ae28f75bb8f1c7c42c39a8c5529bf0f4e")),
-        Fp::from_be_bytes(&hex!("166a9d8cabc673a322fda673779d8e3822ba3ecb8670e461f73bb9021d5fd76a4c56d9d4cd16bd1bba86881979749d28"))
+        Fp::from_be_bytes_unchecked(&hex!("0572cbea904d67468808c8eb50a9450c9721db309128012543902d0ac358a62ae28f75bb8f1c7c42c39a8c5529bf0f4e")),
+        Fp::from_be_bytes_unchecked(&hex!("166a9d8cabc673a322fda673779d8e3822ba3ecb8670e461f73bb9021d5fd76a4c56d9d4cd16bd1bba86881979749d28"))
     );
     let q1 = AffinePoint::new(
         Fp2::from_coeffs([
-            Fp::from_be_bytes(&hex!("024aa2b2f08f0a91260805272dc51051c6e47ad4fa403b02b4510b647ae3d1770bac0326a805bbefd48056c8c121bdb8")),
-            Fp::from_be_bytes(&hex!("13e02b6052719f607dacd3a088274f65596bd0d09920b61ab5da61bbdc7f5049334cf11213945d57e5ac7d055d042b7e"))
+            Fp::from_be_bytes_unchecked(&hex!("024aa2b2f08f0a91260805272dc51051c6e47ad4fa403b02b4510b647ae3d1770bac0326a805bbefd48056c8c121bdb8")),
+            Fp::from_be_bytes_unchecked(&hex!("13e02b6052719f607dacd3a088274f65596bd0d09920b61ab5da61bbdc7f5049334cf11213945d57e5ac7d055d042b7e"))
         ]),
         Fp2::from_coeffs([
-            Fp::from_be_bytes(&hex!("0ce5d527727d6e118cc9cdc6da2e351aadfd9baa8cbdd3a76d429a695160d12c923ac9cc3baca289e193548608b82801")),
-            Fp::from_be_bytes(&hex!("0606c4a02ea734cc32acd2b02bc28b99cb3e287e85a763af267492ab572e99ab3f370d275cec1da1aaa9075ff05f79be"))
+            Fp::from_be_bytes_unchecked(&hex!("0ce5d527727d6e118cc9cdc6da2e351aadfd9baa8cbdd3a76d429a695160d12c923ac9cc3baca289e193548608b82801")),
+            Fp::from_be_bytes_unchecked(&hex!("0606c4a02ea734cc32acd2b02bc28b99cb3e287e85a763af267492ab572e99ab3f370d275cec1da1aaa9075ff05f79be"))
         ]),
     );
 

extensions/algebra/guest/src/lib.rs

@@ -151,13 +151,21 @@ pub trait IntMod:
     /// Does not enforce the integer value of `bytes` must be less than the modulus.
     fn from_repr(repr: Self::Repr) -> Self;
 
+    /// Creates a new IntMod from an array of bytes, little endian.
+    /// Returns `None` if the integer value of `bytes` is greater than or equal to the modulus.
+    fn from_le_bytes(bytes: &[u8]) -> Option<Self>;
+
+    /// Creates a new IntMod from an array of bytes, big endian.
+    /// Returns `None` if the integer value of `bytes` is greater than or equal to the modulus.
+    fn from_be_bytes(bytes: &[u8]) -> Option<Self>;
+
     /// Creates a new IntMod from an array of bytes, little endian.
     /// Does not enforce the integer value of `bytes` must be less than the modulus.
-    fn from_le_bytes(bytes: &[u8]) -> Self;
+    fn from_le_bytes_unchecked(bytes: &[u8]) -> Self;
 
     /// Creates a new IntMod from an array of bytes, big endian.
     /// Does not enforce the integer value of `bytes` must be less than the modulus.
-    fn from_be_bytes(bytes: &[u8]) -> Self;
+    fn from_be_bytes_unchecked(bytes: &[u8]) -> Self;
 
     /// Creates a new IntMod from a u8.
     /// Does not enforce the integer value of `bytes` must be less than the modulus.

extensions/algebra/moduli-macros/src/lib.rs

@@ -391,13 +391,31 @@ pub fn moduli_declare(input: TokenStream) -> TokenStream {
                         Self(repr)
                     }
 
-                    fn from_le_bytes(bytes: &[u8]) -> Self {
+                    fn from_le_bytes(bytes: &[u8]) -> Option<Self> {
+                        let elt = Self::from_le_bytes_unchecked(bytes);
+                        if elt.is_reduced() {
+                            Some(elt)
+                        } else {
+                            None
+                        }
+                    }
+
+                    fn from_be_bytes(bytes: &[u8]) -> Option<Self> {
+                        let elt = Self::from_be_bytes_unchecked(bytes);
+                        if elt.is_reduced() {
+                            Some(elt)
+                        } else {
+                            None
+                        }
+                    }
+
+                    fn from_le_bytes_unchecked(bytes: &[u8]) -> Self {
                         let mut arr = [0u8; #limbs];
                         arr.copy_from_slice(bytes);
                         Self(arr)
                     }
 
-                    fn from_be_bytes(bytes: &[u8]) -> Self {
+                    fn from_be_bytes_unchecked(bytes: &[u8]) -> Self {
                         let mut arr = [0u8; #limbs];
                         for (a, b) in arr.iter_mut().zip(bytes.iter().rev()) {
                             *a = *b;
@@ -761,11 +779,12 @@ pub fn moduli_declare(input: TokenStream) -> TokenStream {
                 fn reduce_le_bytes(bytes: &[u8]) -> Self {
                     let mut res = <Self as openvm_algebra_guest::IntMod>::ZERO;
                     // base should be 2 ^ #limbs which exceeds what Self can represent
-                    let mut base = <Self as openvm_algebra_guest::IntMod>::from_le_bytes(&[255u8; #limbs]);
+                    let mut base = <Self as openvm_algebra_guest::IntMod>::from_le_bytes_unchecked(&[255u8; #limbs]);
                     base += <Self as openvm_algebra_guest::IntMod>::ONE;
                     for chunk in bytes.chunks(#limbs).rev() {
-                        res = res * &base + <Self as openvm_algebra_guest::IntMod>::from_le_bytes(chunk);
+                        res = res * &base + <Self as openvm_algebra_guest::IntMod>::from_le_bytes_unchecked(chunk);
                     }
+                    openvm_algebra_guest::IntMod::assert_reduced(&res);
                     res
                 }
             }

extensions/algebra/tests/programs/examples/little.rs

@@ -34,7 +34,7 @@ pub fn main() {
     assert_eq!(res, inv);
 
     let two = Secp256k1Coord::from_u32(2);
-    let minus_two = Secp256k1Coord::from_le_bytes(&pow);
+    let minus_two = Secp256k1Coord::from_le_bytes_unchecked(&pow);
 
     assert_eq!(res - &minus_two, inv + &two);
 

extensions/algebra/tests/programs/examples/moduli_setup.rs

@@ -32,18 +32,18 @@ pub fn main() {
     }
     assert_eq!(res, Mersenne61::from_u32(1));
 
-    let mut non_reduced = Mersenne61::from_le_bytes(&[0xff; 32]);
+    let mut non_reduced = Mersenne61::from_le_bytes_unchecked(&[0xff; 32]);
     assert!(!non_reduced.is_reduced());
     let reduced = &non_reduced + &Mersenne61::ZERO;
     reduced.assert_reduced();
 
     assert_eq!(&non_reduced + &non_reduced, reduced.double());
 
-    non_reduced = Mersenne61::from_le_bytes(&Mersenne61::MODULUS);
+    non_reduced = Mersenne61::from_le_bytes_unchecked(&Mersenne61::MODULUS);
     assert!(!non_reduced.is_reduced());
 
     let mut bytes = [0u8; 32];
     bytes[7] = 1 << 5; // 2^61 = 2^{8*7 + 5} = modulus + 1
-    non_reduced = Mersenne61::from_le_bytes(&bytes);
+    non_reduced = Mersenne61::from_le_bytes_unchecked(&bytes);
     assert!(!non_reduced.is_reduced());
 }

extensions/ecc/guest/src/ecdsa.rs

@@ -128,16 +128,16 @@ where
             }
 
             Tag::CompressedEvenY | Tag::CompressedOddY => {
-                let x = Coordinate::<C>::from_be_bytes(&bytes[1..]);
+                let x = Coordinate::<C>::from_be_bytes(&bytes[1..]).ok_or_else(Error::new)?;
                 let rec_id = bytes[0] & 1;
                 let point = FromCompressed::decompress(x, &rec_id).ok_or_else(Error::new)?;
                 Ok(Self { point })
             }
 
             Tag::Uncompressed => {
                 let (x_bytes, y_bytes) = bytes[1..].split_at(Coordinate::<C>::NUM_LIMBS);
-                let x = Coordinate::<C>::from_be_bytes(x_bytes);
-                let y = Coordinate::<C>::from_be_bytes(y_bytes);
+                let x = Coordinate::<C>::from_be_bytes(x_bytes).ok_or_else(Error::new)?;
+                let y = Coordinate::<C>::from_be_bytes(y_bytes).ok_or_else(Error::new)?;
                 let point = <C as IntrinsicCurve>::Point::from_xy(x, y).ok_or_else(Error::new)?;
                 Ok(Self { point })
             }
@@ -441,11 +441,8 @@ where
         // Signature is default encoded in big endian bytes
         let (r_be, s_be) = sig.split_at(<C as IntrinsicCurve>::Scalar::NUM_LIMBS);
         // Note: Scalar internally stores using little endian
-        let r = Scalar::<C>::from_be_bytes(r_be);
-        let s = Scalar::<C>::from_be_bytes(s_be);
-        if !r.is_reduced() || !s.is_reduced() {
-            return Err(Error::new());
-        }
+        let r = Scalar::<C>::from_be_bytes(r_be).ok_or_else(Error::new)?;
+        let s = Scalar::<C>::from_be_bytes(s_be).ok_or_else(Error::new)?;
         if r == Scalar::<C>::ZERO || s == Scalar::<C>::ZERO {
             return Err(Error::new());
         }
@@ -456,7 +453,7 @@ where
         let trim = prehash_bytes.len().saturating_sub(Scalar::<C>::NUM_LIMBS);
         // from_be_bytes still works if len < Scalar::NUM_LIMBS
         // we don't need to reduce because IntMod is up to modular equivalence
-        let z = Scalar::<C>::from_be_bytes(&prehash_bytes[..prehash_bytes.len() - trim]);
+        let z = Scalar::<C>::from_be_bytes_unchecked(&prehash_bytes[..prehash_bytes.len() - trim]);
 
         // `r` is in the Scalar field, we now possibly add C::ORDER to it to get `x`
         // in the Coordinate field.
@@ -481,10 +478,7 @@ where
             };
         }
         assert!(FieldBytesSize::<C>::USIZE <= Coordinate::<C>::NUM_LIMBS);
-        let x = Coordinate::<C>::from_be_bytes(&r_bytes);
-        if !x.is_reduced() {
-            return Err(Error::new());
-        }
+        let x = Coordinate::<C>::from_be_bytes(&r_bytes).ok_or_else(Error::new)?;
         let rec_id = recovery_id.to_byte();
         // The point R decompressed from x-coordinate `r`
         let R: C::Point = FromCompressed::decompress(x, &rec_id).ok_or_else(Error::new)?;
@@ -518,11 +512,8 @@ where
     // Signature is default encoded in big endian bytes
     let (r_be, s_be) = sig.split_at(<C as IntrinsicCurve>::Scalar::NUM_LIMBS);
     // Note: Scalar internally stores using little endian
-    let r = Scalar::<C>::from_be_bytes(r_be);
-    let s = Scalar::<C>::from_be_bytes(s_be);
-    if !r.is_reduced() || !s.is_reduced() {
-        return Err(Error::new());
-    }
+    let r = Scalar::<C>::from_be_bytes(r_be).ok_or_else(Error::new)?;
+    let s = Scalar::<C>::from_be_bytes(s_be).ok_or_else(Error::new)?;
     if r == Scalar::<C>::ZERO || s == Scalar::<C>::ZERO {
         return Err(Error::new());
     }
@@ -533,7 +524,7 @@ where
     let trim = prehash_bytes.len().saturating_sub(Scalar::<C>::NUM_LIMBS);
     // from_be_bytes still works if len < Scalar::NUM_LIMBS
     // we don't need to reduce because IntMod is up to modular equivalence
-    let z = Scalar::<C>::from_be_bytes(&prehash_bytes[..prehash_bytes.len() - trim]);
+    let z = Scalar::<C>::from_be_bytes_unchecked(&prehash_bytes[..prehash_bytes.len() - trim]);
 
     let u1 = z.div_unsafe(&s);
     let u2 = (&r).div_unsafe(&s);