k256: fallible AffinePoint to VerifyingKey conversion

The inner type of `VerifyingKey` is a `PublicKey`, which is itself a wrapper for an inner curve-specific `AffinePoint` which ensures that the inner point is *NOT* the additive identity / point at infinity. Therefore, any conversion from an `AffinePoint` is potentially fallible, so the existing `From` conversions need to be changed to `TryFrom`. This commit also adds some additional type conversions between `AffinePoint`/`ProjectivePoint` and `PublicKey`/`VerifyingKey`, and also includes them in `p256` for consistency.
RustCrypto · Mar 16, 2022 · 9a78034 · 9a78034
1 parent 55784f3
commit 9a78034
Show file tree

Hide file tree

Showing 11 changed files with 185 additions and 55 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bp256/Cargo.toml b/bp256/Cargo.toml
@@ -17,7 +17,7 @@ elliptic-curve = { version = "0.12.0-pre.1", default-features = false, features
 sec1 = { version = "0.2", default-features = false }
 
 # optional dependencies
-ecdsa = { version = "0.14.0-pre.1", optional = true, default-features = false, features = ["der"] }
+ecdsa = { version = "0.14.0-pre.2", optional = true, default-features = false, features = ["der"] }
 sha2 = { version = "0.10", optional = true, default-features = false }
 
 [features]

diff --git a/bp384/Cargo.toml b/bp384/Cargo.toml
@@ -17,7 +17,7 @@ elliptic-curve = { version = "0.12.0-pre.1", default-features = false, features
 sec1 = { version = "0.2", default-features = false }
 
 # optional dependencies
-ecdsa = { version = "0.14.0-pre.1", optional = true, default-features = false, features = ["der"] }
+ecdsa = { version = "0.14.0-pre.2", optional = true, default-features = false, features = ["der"] }
 sha2 = { version = "0.10", optional = true, default-features = false }
 
 [features]

diff --git a/k256/Cargo.toml b/k256/Cargo.toml
@@ -23,15 +23,15 @@ elliptic-curve = { version = "0.12.0-pre.1", default-features = false, features
 sec1 = { version = "0.2", default-features = false }
 
 # optional dependencies
-ecdsa-core = { version = "0.14.0-pre.1", package = "ecdsa", optional = true, default-features = false, features = ["der"] }
+ecdsa-core = { version = "0.14.0-pre.2", package = "ecdsa", optional = true, default-features = false, features = ["der"] }
 hex-literal = { version = "0.3", optional = true }
 sha2 = { version = "0.10", optional = true, default-features = false }
 sha3 = { version = "0.10", optional = true, default-features = false }
 
 [dev-dependencies]
 blobby = "0.3"
 criterion = "0.3"
-ecdsa-core = { version = "0.14.0-pre.1", package = "ecdsa", default-features = false, features = ["dev"] }
+ecdsa-core = { version = "0.14.0-pre.2", package = "ecdsa", default-features = false, features = ["dev"] }
 hex-literal = "0.3"
 num-bigint = "0.4"
 num-traits = "0.2"

diff --git a/k256/src/arithmetic/affine.rs b/k256/src/arithmetic/affine.rs
@@ -3,7 +3,7 @@
 #![allow(clippy::op_ref)]
 
 use super::{FieldElement, ProjectivePoint, CURVE_EQUATION_B};
-use crate::{CompressedPoint, EncodedPoint, FieldBytes, Scalar, Secp256k1};
+use crate::{CompressedPoint, EncodedPoint, FieldBytes, PublicKey, Scalar, Secp256k1};
 use core::ops::{Mul, Neg};
 use elliptic_curve::{
     group::{prime::PrimeCurveAffine, GroupEncoding},
@@ -144,6 +144,34 @@ impl PartialEq for AffinePoint {
 
 impl Eq for AffinePoint {}
 
+impl Mul<Scalar> for AffinePoint {
+    type Output = ProjectivePoint;
+
+    fn mul(self, scalar: Scalar) -> ProjectivePoint {
+        ProjectivePoint::from(self) * scalar
+    }
+}
+
+impl Mul<&Scalar> for AffinePoint {
+    type Output = ProjectivePoint;
+
+    fn mul(self, scalar: &Scalar) -> ProjectivePoint {
+        ProjectivePoint::from(self) * scalar
+    }
+}
+
+impl Neg for AffinePoint {
+    type Output = AffinePoint;
+
+    fn neg(self) -> Self::Output {
+        AffinePoint {
+            x: self.x,
+            y: self.y.negate(1).normalize_weak(),
+            infinity: self.infinity,
+        }
+    }
+}
+
 impl DecompressPoint<Secp256k1> for AffinePoint {
     fn decompress(x_bytes: &FieldBytes, y_is_odd: Choice) -> CtOption<Self> {
         FieldElement::from_bytes(x_bytes).and_then(|x| {
@@ -261,37 +289,42 @@ impl TryFrom<&EncodedPoint> for AffinePoint {
 }
 
 impl From<AffinePoint> for EncodedPoint {
-    /// Returns the SEC1 compressed encoding of this point.
     fn from(affine_point: AffinePoint) -> EncodedPoint {
+        EncodedPoint::from(&affine_point)
+    }
+}
+
+impl From<&AffinePoint> for EncodedPoint {
+    fn from(affine_point: &AffinePoint) -> EncodedPoint {
         affine_point.to_encoded_point(true)
     }
 }
 
-impl Mul<Scalar> for AffinePoint {
-    type Output = ProjectivePoint;
+impl From<PublicKey> for AffinePoint {
+    fn from(public_key: PublicKey) -> AffinePoint {
+        *public_key.as_affine()
+    }
+}
 
-    fn mul(self, scalar: Scalar) -> ProjectivePoint {
-        ProjectivePoint::from(self) * scalar
+impl From<&PublicKey> for AffinePoint {
+    fn from(public_key: &PublicKey) -> AffinePoint {
+        AffinePoint::from(*public_key)
     }
 }
 
-impl Mul<&Scalar> for AffinePoint {
-    type Output = ProjectivePoint;
+impl TryFrom<AffinePoint> for PublicKey {
+    type Error = Error;
 
-    fn mul(self, scalar: &Scalar) -> ProjectivePoint {
-        ProjectivePoint::from(self) * scalar
+    fn try_from(affine_point: AffinePoint) -> Result<PublicKey> {
+        PublicKey::from_affine(affine_point)
     }
 }
 
-impl Neg for AffinePoint {
-    type Output = AffinePoint;
+impl TryFrom<&AffinePoint> for PublicKey {
+    type Error = Error;
 
-    fn neg(self) -> Self::Output {
-        AffinePoint {
-            x: self.x,
-            y: self.y.negate(1).normalize_weak(),
-            infinity: self.infinity,
-        }
+    fn try_from(affine_point: &AffinePoint) -> Result<PublicKey> {
+        PublicKey::try_from(*affine_point)
     }
 }
 

diff --git a/k256/src/arithmetic/projective.rs b/k256/src/arithmetic/projective.rs
@@ -3,7 +3,7 @@
 #![allow(clippy::op_ref)]
 
 use super::{AffinePoint, FieldElement, Scalar, CURVE_EQUATION_B_SINGLE};
-use crate::{CompressedPoint, EncodedPoint, Secp256k1};
+use crate::{CompressedPoint, EncodedPoint, PublicKey, Secp256k1};
 use core::{
     iter::Sum,
     ops::{Add, AddAssign, Neg, Sub, SubAssign},
@@ -18,7 +18,7 @@ use elliptic_curve::{
     sec1::{FromEncodedPoint, ToEncodedPoint},
     subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption},
     zeroize::DefaultIsZeroes,
-    PrimeCurveArithmetic, ProjectiveArithmetic,
+    Error, PrimeCurveArithmetic, ProjectiveArithmetic, Result,
 };
 
 #[rustfmt::skip]
@@ -531,6 +531,34 @@ impl<'a> Neg for &'a ProjectivePoint {
     }
 }
 
+impl From<PublicKey> for ProjectivePoint {
+    fn from(public_key: PublicKey) -> ProjectivePoint {
+        AffinePoint::from(public_key).into()
+    }
+}
+
+impl From<&PublicKey> for ProjectivePoint {
+    fn from(public_key: &PublicKey) -> ProjectivePoint {
+        AffinePoint::from(public_key).into()
+    }
+}
+
+impl TryFrom<ProjectivePoint> for PublicKey {
+    type Error = Error;
+
+    fn try_from(point: ProjectivePoint) -> Result<PublicKey> {
+        AffinePoint::from(point).try_into()
+    }
+}
+
+impl TryFrom<&ProjectivePoint> for PublicKey {
+    type Error = Error;
+
+    fn try_from(point: &ProjectivePoint) -> Result<PublicKey> {
+        AffinePoint::from(point).try_into()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::{AffinePoint, ProjectivePoint};

diff --git a/k256/src/ecdsa/recoverable.rs b/k256/src/ecdsa/recoverable.rs
@@ -187,10 +187,10 @@ impl Signature {
         let r_inv = *r.invert();
         let u1 = -(r_inv * z);
         let u2 = r_inv * *s;
-        let pk = ProjectivePoint::lincomb(&ProjectivePoint::GENERATOR, &u1, &R, &u2).to_affine();
+        let pk = ProjectivePoint::lincomb(&ProjectivePoint::GENERATOR, &u1, &R, &u2);
 
         // TODO(tarcieri): ensure the signature verifies?
-        Ok(VerifyingKey::from(&pk))
+        VerifyingKey::try_from(pk)
     }
 
     /// Parse the `r` component of this signature to a [`NonZeroScalar`]

diff --git a/k256/src/ecdsa/verify.rs b/k256/src/ecdsa/verify.rs
@@ -154,12 +154,6 @@ impl From<&VerifyingKey> for PublicKey {
     }
 }
 
-impl From<&AffinePoint> for VerifyingKey {
-    fn from(affine_point: &AffinePoint) -> VerifyingKey {
-        VerifyingKey::from_encoded_point(&affine_point.to_encoded_point(false)).unwrap()
-    }
-}
-
 impl From<ecdsa_core::VerifyingKey<Secp256k1>> for VerifyingKey {
     fn from(verifying_key: ecdsa_core::VerifyingKey<Secp256k1>) -> VerifyingKey {
         VerifyingKey {
@@ -180,6 +174,26 @@ impl ToEncodedPoint<Secp256k1> for VerifyingKey {
     }
 }
 
+impl TryFrom<AffinePoint> for VerifyingKey {
+    type Error = Error;
+
+    fn try_from(affine_point: AffinePoint) -> Result<VerifyingKey, Error> {
+        let inner = PublicKey::try_from(affine_point)
+            .map_err(|_| Error::new())?
+            .into();
+
+        Ok(VerifyingKey { inner })
+    }
+}
+
+impl TryFrom<&AffinePoint> for VerifyingKey {
+    type Error = Error;
+
+    fn try_from(affine_point: &AffinePoint) -> Result<VerifyingKey, Error> {
+        VerifyingKey::try_from(*affine_point)
+    }
+}
+
 impl TryFrom<&EncodedPoint> for VerifyingKey {
     type Error = Error;
 
@@ -188,6 +202,34 @@ impl TryFrom<&EncodedPoint> for VerifyingKey {
     }
 }
 
+impl From<VerifyingKey> for ProjectivePoint {
+    fn from(verifying_key: VerifyingKey) -> ProjectivePoint {
+        PublicKey::from(verifying_key.inner).into()
+    }
+}
+
+impl From<&VerifyingKey> for ProjectivePoint {
+    fn from(verifying_key: &VerifyingKey) -> ProjectivePoint {
+        PublicKey::from(verifying_key.inner).into()
+    }
+}
+
+impl TryFrom<ProjectivePoint> for VerifyingKey {
+    type Error = Error;
+
+    fn try_from(point: ProjectivePoint) -> Result<VerifyingKey, Error> {
+        AffinePoint::from(point).try_into()
+    }
+}
+
+impl TryFrom<&ProjectivePoint> for VerifyingKey {
+    type Error = Error;
+
+    fn try_from(point: &ProjectivePoint) -> Result<VerifyingKey, Error> {
+        AffinePoint::from(point).try_into()
+    }
+}
+
 #[cfg(feature = "pkcs8")]
 #[cfg_attr(docsrs, doc(cfg(feature = "pkcs8")))]
 impl TryFrom<pkcs8::SubjectPublicKeyInfo<'_>> for VerifyingKey {

diff --git a/p256/Cargo.toml b/p256/Cargo.toml
@@ -21,13 +21,13 @@ elliptic-curve = { version = "0.12.0-pre.1", default-features = false, features
 sec1 = { version = "0.2", default-features = false }
 
 # optional dependencies
-ecdsa-core = { version = "0.14.0-pre.1", package = "ecdsa", optional = true, default-features = false, features = ["der"] }
+ecdsa-core = { version = "0.14.0-pre.2", package = "ecdsa", optional = true, default-features = false, features = ["der"] }
 hex-literal = { version = "0.3", optional = true }
 sha2 = { version = "0.10", optional = true, default-features = false }
 
 [dev-dependencies]
 blobby = "0.3"
-ecdsa-core = { version = "0.14.0-pre.1", package = "ecdsa", default-features = false, features = ["dev"] }
+ecdsa-core = { version = "0.14.0-pre.2", package = "ecdsa", default-features = false, features = ["dev"] }
 hex-literal = "0.3"
 proptest = "1.0"
 rand_core = { version = "0.6", features = ["getrandom"] }