General Improvements (#47)

* Introduce `Result` shorthand, re-export and rename `InternalError`

* Re-export some public API relevant types

* Move `deserialize`

* Remove branch in `i2osp`

* Make `serialize` and `serialize_owned` methods

* Update p256

src/group/expand.rs

@@ -13,8 +13,8 @@ use generic_array::sequence::Concat;
 use generic_array::typenum::{Unsigned, U1, U2};
 use generic_array::{ArrayLength, GenericArray};
 
-use crate::errors::InternalError;
 use crate::util::i2osp;
+use crate::{Error, Result};
 
 // Computes ceil(x / y)
 fn div_ceil(x: usize, y: usize) -> usize {
@@ -37,14 +37,14 @@ pub fn expand_message_xmd<
 >(
     msg: M,
     dst: GenericArray<u8, D>,
-) -> Result<GenericArray<u8, L>, InternalError>
+) -> Result<GenericArray<u8, L>>
 where
     <D as Add<U1>>::Output: ArrayLength<u8>,
 {
     let digest_len = H::OutputSize::USIZE;
     let ell = div_ceil(L::USIZE, digest_len);
     if ell > 255 {
-        return Err(InternalError::HashToCurveError);
+        return Err(Error::HashToCurveError);
     }
     let dst_prime = dst.concat(i2osp::<U1>(D::USIZE)?);
     let z_pad = i2osp::<H::BlockSize>(0)?;

src/group/mod.rs

@@ -24,7 +24,7 @@ use rand_core::{CryptoRng, RngCore};
 use subtle::ConstantTimeEq;
 use zeroize::Zeroize;
 
-use crate::errors::InternalError;
+use crate::{Error, Result};
 
 /// A prime-order subgroup of a base field (EC, prime-order field ...). This
 /// subgroup is noted additively — as in the draft RFC — in this trait.
@@ -43,7 +43,7 @@ pub trait Group:
     fn hash_to_curve<H: BlockSizeUser + Digest + FixedOutputReset, D: ArrayLength<u8> + Add<U1>>(
         msg: &[u8],
         dst: GenericArray<u8, D>,
-    ) -> Result<Self, InternalError>
+    ) -> Result<Self>
     where
         <D as Add<U1>>::Output: ArrayLength<u8>;
 
@@ -56,7 +56,7 @@ pub trait Group:
     >(
         input: I,
         dst: GenericArray<u8, D>,
-    ) -> Result<Self::Scalar, InternalError>
+    ) -> Result<Self::Scalar>
     where
         <D as Add<U1>>::Output: ArrayLength<u8>;
 
@@ -74,16 +74,16 @@ pub trait Group:
     /// checking if the scalar is zero.
     fn from_scalar_slice_unchecked(
         scalar_bits: &GenericArray<u8, Self::ScalarLen>,
-    ) -> Result<Self::Scalar, InternalError>;
+    ) -> Result<Self::Scalar>;
 
     /// Return a scalar from its fixed-length bytes representation. If the
     /// scalar is zero, then return an error.
     fn from_scalar_slice<'a>(
         scalar_bits: impl Into<&'a GenericArray<u8, Self::ScalarLen>>,
-    ) -> Result<Self::Scalar, InternalError> {
+    ) -> Result<Self::Scalar> {
         let scalar = Self::from_scalar_slice_unchecked(scalar_bits.into())?;
         if scalar.ct_eq(&Self::scalar_zero()).into() {
-            return Err(InternalError::ZeroScalarError);
+            return Err(Error::ZeroScalarError);
         }
         Ok(scalar)
     }
@@ -101,20 +101,19 @@ pub trait Group:
     /// Return an element from its fixed-length bytes representation. This is
     /// the unchecked version, which does not check for deserializing the
     /// identity element
-    fn from_element_slice_unchecked(
-        element_bits: &GenericArray<u8, Self::ElemLen>,
-    ) -> Result<Self, InternalError>;
+    fn from_element_slice_unchecked(element_bits: &GenericArray<u8, Self::ElemLen>)
+        -> Result<Self>;
 
     /// Return an element from its fixed-length bytes representation. If the
     /// element is the identity element, return an error.
     fn from_element_slice<'a>(
         element_bits: impl Into<&'a GenericArray<u8, Self::ElemLen>>,
-    ) -> Result<Self, InternalError> {
+    ) -> Result<Self> {
         let elem = Self::from_element_slice_unchecked(element_bits.into())?;
 
         if Self::ct_eq(&elem, &<Self as Group>::identity()).into() {
             // found the identity element
-            return Err(InternalError::PointError);
+            return Err(Error::PointError);
         }
 
         Ok(elem)

src/group/p256.rs

@@ -26,6 +26,7 @@ use num_traits::{One, ToPrimitive, Zero};
 use once_cell::unsync::Lazy;
 use p256_::elliptic_curve::group::prime::PrimeCurveAffine;
 use p256_::elliptic_curve::group::GroupEncoding;
+use p256_::elliptic_curve::ops::Reduce;
 use p256_::elliptic_curve::sec1::{FromEncodedPoint, ToEncodedPoint};
 use p256_::elliptic_curve::Field;
 use p256_::{AffinePoint, EncodedPoint, ProjectivePoint};
@@ -33,7 +34,7 @@ use rand_core::{CryptoRng, RngCore};
 use subtle::{Choice, ConditionallySelectable};
 
 use super::Group;
-use crate::errors::InternalError;
+use crate::{Error, Result};
 
 // https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-hash-to-curve-11#section-8.2
 // `L: 48`
@@ -48,7 +49,7 @@ impl Group for ProjectivePoint {
     fn hash_to_curve<H: BlockSizeUser + Digest + FixedOutputReset, D: ArrayLength<u8> + Add<U1>>(
         msg: &[u8],
         dst: GenericArray<u8, D>,
-    ) -> Result<Self, InternalError>
+    ) -> Result<Self>
     where
         <D as Add<U1>>::Output: ArrayLength<u8>,
     {
@@ -85,15 +86,15 @@ impl Group for ProjectivePoint {
         let (q1x, q1y) = hash_to_curve_simple_swu(&uniform_bytes[L::USIZE..], &A, &B, &P, &Z);
 
         // convert to `p256` types
-        let p0 = AffinePoint::from_encoded_point(&EncodedPoint::from_affine_coordinates(
-            &q0x, &q0y, false,
+        let p0 = Option::<AffinePoint>::from(AffinePoint::from_encoded_point(
+            &EncodedPoint::from_affine_coordinates(&q0x, &q0y, false),
         ))
-        .ok_or(InternalError::PointError)?
+        .ok_or(Error::PointError)?
         .to_curve();
-        let p1 = AffinePoint::from_encoded_point(&EncodedPoint::from_affine_coordinates(
-            &q1x, &q1y, false,
+        let p1 = Option::<AffinePoint>::from(AffinePoint::from_encoded_point(
+            &EncodedPoint::from_affine_coordinates(&q1x, &q1y, false),
         ))
-        .ok_or(InternalError::PointError)?;
+        .ok_or(Error::PointError)?;
 
         Ok(p0 + p1)
     }
@@ -107,7 +108,7 @@ impl Group for ProjectivePoint {
     >(
         input: I,
         dst: GenericArray<u8, D>,
-    ) -> Result<Self::Scalar, InternalError>
+    ) -> Result<Self::Scalar>
     where
         <D as Add<U1>>::Output: ArrayLength<u8>,
     {
@@ -132,7 +133,7 @@ impl Group for ProjectivePoint {
         let mut result = GenericArray::default();
         result[..bytes.len()].copy_from_slice(&bytes);
 
-        Ok(p256_::Scalar::from_bytes_reduced(&result))
+        Ok(p256_::Scalar::from_be_bytes_reduced(result))
     }
 
     type ElemLen = U33;
@@ -141,8 +142,8 @@ impl Group for ProjectivePoint {
 
     fn from_scalar_slice_unchecked(
         scalar_bits: &GenericArray<u8, Self::ScalarLen>,
-    ) -> Result<Self::Scalar, InternalError> {
-        Ok(Self::Scalar::from_bytes_reduced(scalar_bits))
+    ) -> Result<Self::Scalar> {
+        Ok(Self::Scalar::from_be_bytes_reduced(*scalar_bits))
     }
 
     fn random_nonzero_scalar<R: RngCore + CryptoRng>(rng: &mut R) -> Self::Scalar {
@@ -159,8 +160,8 @@ impl Group for ProjectivePoint {
 
     fn from_element_slice_unchecked(
         element_bits: &GenericArray<u8, Self::ElemLen>,
-    ) -> Result<Self, InternalError> {
-        Option::from(Self::from_bytes(element_bits)).ok_or(InternalError::PointError)
+    ) -> Result<Self> {
+        Option::from(Self::from_bytes(element_bits)).ok_or(Error::PointError)
     }
 
     fn to_arr(&self) -> GenericArray<u8, Self::ElemLen> {

src/group/ristretto.rs

@@ -19,7 +19,7 @@ use generic_array::{ArrayLength, GenericArray};
 use rand_core::{CryptoRng, RngCore};
 
 use super::Group;
-use crate::errors::InternalError;
+use crate::{Error, Result};
 
 // `cfg` here is only needed because of a bug in Rust's crate feature documentation. See: https://github.com/rust-lang/rust/issues/83428
 #[cfg(feature = "ristretto255")]
@@ -32,7 +32,7 @@ impl Group for RistrettoPoint {
     fn hash_to_curve<H: BlockSizeUser + Digest + FixedOutputReset, D: ArrayLength<u8> + Add<U1>>(
         msg: &[u8],
         dst: GenericArray<u8, D>,
-    ) -> Result<Self, InternalError>
+    ) -> Result<Self>
     where
         <D as Add<U1>>::Output: ArrayLength<u8>,
     {
@@ -42,7 +42,7 @@ impl Group for RistrettoPoint {
             uniform_bytes
                 .as_slice()
                 .try_into()
-                .map_err(|_| InternalError::HashToCurveError)?,
+                .map_err(|_| Error::HashToCurveError)?,
         ))
     }
 
@@ -56,7 +56,7 @@ impl Group for RistrettoPoint {
     >(
         input: I,
         dst: GenericArray<u8, D>,
-    ) -> Result<Self::Scalar, InternalError>
+    ) -> Result<Self::Scalar>
     where
         <D as Add<U1>>::Output: ArrayLength<u8>,
     {
@@ -66,7 +66,7 @@ impl Group for RistrettoPoint {
             uniform_bytes
                 .as_slice()
                 .try_into()
-                .map_err(|_| InternalError::HashToCurveError)?,
+                .map_err(|_| Error::HashToCurveError)?,
         ))
     }
 
@@ -74,7 +74,7 @@ impl Group for RistrettoPoint {
     type ScalarLen = U32;
     fn from_scalar_slice_unchecked(
         scalar_bits: &GenericArray<u8, Self::ScalarLen>,
-    ) -> Result<Self::Scalar, InternalError> {
+    ) -> Result<Self::Scalar> {
         Ok(Scalar::from_bytes_mod_order(*scalar_bits.as_ref()))
     }
 
@@ -104,10 +104,10 @@ impl Group for RistrettoPoint {
     type ElemLen = U32;
     fn from_element_slice_unchecked(
         element_bits: &GenericArray<u8, Self::ElemLen>,
-    ) -> Result<Self, InternalError> {
+    ) -> Result<Self> {
         CompressedRistretto::from_slice(element_bits)
             .decompress()
-            .ok_or(InternalError::PointError)
+            .ok_or(Error::PointError)
     }
     // serialization of a group element
     fn to_arr(&self) -> GenericArray<u8, Self::ElemLen> {

src/group/tests.rs

@@ -7,14 +7,13 @@
 
 //! Includes a series of tests for the group implementations
 
-use crate::errors::InternalError;
-use crate::group::Group;
+use crate::{Error, Group, Result};
 
 // Test that the deserialization of a group element should throw an error if the
 // identity element can be deserialized properly
 
 #[test]
-fn test_group_properties() -> Result<(), InternalError> {
+fn test_group_properties() -> Result<()> {
     #[cfg(feature = "ristretto255")]
     {
         use curve25519_dalek::ristretto::RistrettoPoint;
@@ -35,19 +34,19 @@ fn test_group_properties() -> Result<(), InternalError> {
 }
 
 // Checks that the identity element cannot be deserialized
-fn test_identity_element_error<G: Group>() -> Result<(), InternalError> {
+fn test_identity_element_error<G: Group>() -> Result<()> {
     let identity = G::identity();
     let result = G::from_element_slice(&identity.to_arr());
-    assert!(matches!(result, Err(InternalError::PointError)));
+    assert!(matches!(result, Err(Error::PointError)));
 
     Ok(())
 }
 
 // Checks that the zero scalar cannot be deserialized
-fn test_zero_scalar_error<G: Group>() -> Result<(), InternalError> {
+fn test_zero_scalar_error<G: Group>() -> Result<()> {
     let zero_scalar = G::scalar_zero();
     let result = G::from_scalar_slice(&G::scalar_as_bytes(zero_scalar));
-    assert!(matches!(result, Err(InternalError::ZeroScalarError)));
+    assert!(matches!(result, Err(Error::ZeroScalarError)));
 
     Ok(())
 }