Replace PointGFp::check_invaraints, which would either return silently

or throw an exception, with PointGFp::on_the_curve, which returns a bool.

Update callers.

This showed several cases where check_invaraints was being called
multiple times, for instance when decoding a point with OS2ECP,
check_invaraints was called; many callers of OS2ECP would then call
check_invaraints again on the same object.

6 files changed, 29 insertions, 55 deletions

diff --git a/src/math/numbertheory/point_gfp.cpp b/src/math/numbertheory/point_gfp.cpp
index 4e8906dba..6e62a9a13 100644
--- a/src/math/numbertheory/point_gfp.cpp
+++ b/src/math/numbertheory/point_gfp.cpp
@@ -392,17 +392,17 @@ BigInt PointGFp::get_affine_y() const
 #endif
    }
 
-void PointGFp::check_invariants() const
+bool PointGFp::on_the_curve() const
    {
    /*
    Is the point still on the curve?? (If everything is correct, the
-   point is always on its curve; then the function will return
-   silently. If Oskar managed to corrupt this object's state, then it
-   will throw an exception.)
+   point is always on its curve; then the function will return true.
+   If somehow the state is corrupted, which suggests a fault attack
+   (or internal computational error), then return false.
    */
 
    if(is_zero())
-      return;
+      return true;
 
    const Modular_Reducer& mod_p = curve.mod_p();
 
@@ -418,7 +418,7 @@ void PointGFp::check_invariants() const
    if(z == 1)
       {
       if(mod_p.reduce(x3 + ax + curve.get_b()) != y2)
-         throw Illegal_Point("Invalid ECP point: y^2 != x^3 + a*x + b");
+         return false;
       }
 
    BigInt z2 = mod_p.square(z);
@@ -429,7 +429,9 @@ void PointGFp::check_invariants() const
    BigInt b_z6 = mod_p.multiply(curve.get_b(), mod_p.square(z3));
 
    if(y2 != mod_p.reduce(x3 + ax_z4 + b_z6))
-      throw Illegal_Point("Invalid ECP point: y^2 != x^3 + a*x*z^4 + b*z^6");
+      return false;
+
+   return true;
    }
 
 // swaps the states of *this and other, does not throw!
@@ -575,7 +577,10 @@ PointGFp OS2ECP(const byte data[], u32bit data_len,
       throw Invalid_Argument("OS2ECP: Unknown format type");
 
    PointGFp result(curve, x, y);
-   result.check_invariants();
+
+   if(!result.on_the_curve())
+      throw Illegal_Point("OS2ECP: Decoded point was not on the curve");
+
    return result;
    }
 
diff --git a/src/math/numbertheory/point_gfp.h b/src/math/numbertheory/point_gfp.h
index d92a5cbcb..0708493fe 100644
--- a/src/math/numbertheory/point_gfp.h
+++ b/src/math/numbertheory/point_gfp.h
@@ -128,12 +128,11 @@ class BOTAN_DLL PointGFp
          { return (coord_x.is_zero() && coord_z.is_zero()); }
 
       /**
-      *  Checks whether the point is to be found on the underlying curve.
-      *  Throws an Invalid_Point exception in case of detecting that the point
-      *  does not satisfy the curve equation.
-      *  To be used to ensure against fault attacks.
+      * Checks whether the point is to be found on the underlying
+      * curve; used to prevent fault attacks.
+      * @return if the point is on the curve
       */
-      void check_invariants() const;
+      bool on_the_curve() const;
 
       /**
       * swaps the states of *this and other, does not throw!
diff --git a/src/pubkey/ec_dompar/ec_dompar.cpp b/src/pubkey/ec_dompar/ec_dompar.cpp
index 3512060d1..b0aa7a87a 100644
--- a/src/pubkey/ec_dompar/ec_dompar.cpp
+++ b/src/pubkey/ec_dompar/ec_dompar.cpp
@@ -77,7 +77,6 @@ EC_Domain_Params::EC_Domain_Params(const MemoryRegion<byte>& ber_data)
 
       curve = CurveGFp(p, a, b);
       base_point = OS2ECP(sv_base_point, curve);
-      base_point.check_invariants();
       }
    else
       throw Decoding_Error("Unexpected tag while decoding ECC domain params");
diff --git a/src/pubkey/ecc_key/ecc_key.cpp b/src/pubkey/ecc_key/ecc_key.cpp
index fdb29b29f..2c66dc97f 100644
--- a/src/pubkey/ecc_key/ecc_key.cpp
+++ b/src/pubkey/ecc_key/ecc_key.cpp
@@ -25,14 +25,8 @@ EC_PublicKey::EC_PublicKey(const EC_Domain_Params& dom_par,
    if(domain().get_curve() != public_point().get_curve())
       throw Invalid_Argument("EC_PublicKey: curve mismatch in constructor");
 
-   try
-      {
-      public_key.check_invariants();
-      }
-   catch(Illegal_Point)
-      {
-      throw Invalid_State("Public key failed invariant check");
-      }
+   if(!public_point().on_the_curve())
+      throw Invalid_State("Public key was not on the curve");
    }
 
 EC_PublicKey::EC_PublicKey(const AlgorithmIdentifier& alg_id,
@@ -41,16 +35,7 @@ EC_PublicKey::EC_PublicKey(const AlgorithmIdentifier& alg_id,
    domain_params = EC_Domain_Params(alg_id.parameters);
    domain_encoding = EC_DOMPAR_ENC_EXPLICIT;
 
-   public_key = PointGFp(OS2ECP(key_bits, domain().get_curve()));
-
-   try
-      {
-      public_point().check_invariants();
-      }
-   catch(Illegal_Point)
-      {
-      throw Decoding_Error("Invalid public point; not on curve");
-      }
+   public_key = OS2ECP(key_bits, domain().get_curve());
    }
 
 AlgorithmIdentifier EC_PublicKey::algorithm_identifier() const
@@ -111,14 +96,8 @@ EC_PrivateKey::EC_PrivateKey(RandomNumberGenerator& rng,
    private_key = BigInt::random_integer(rng, 1, domain().get_order());
    public_key = domain().get_base_point() * private_key;
 
-   try
-      {
-      public_key.check_invariants();
-      }
-   catch(Illegal_Point)
-      {
+   if(!public_key.on_the_curve())
       throw Internal_Error("ECC private key generation failed");
-      }
    }
 
 MemoryVector<byte> EC_PrivateKey::pkcs8_private_key() const
@@ -147,14 +126,8 @@ EC_PrivateKey::EC_PrivateKey(const AlgorithmIdentifier& alg_id,
 
    public_key = domain().get_base_point() * private_key;
 
-   try
-      {
-      public_key.check_invariants();
-      }
-   catch(Illegal_Point)
-      {
+   if(!public_key.on_the_curve())
       throw Internal_Error("Loaded ECC private key failed self test");
-      }
    }
 
 }
diff --git a/src/pubkey/ecdh/ecdh.cpp b/src/pubkey/ecdh/ecdh.cpp
index bf8a57b3b..8d13e7f65 100644
--- a/src/pubkey/ecdh/ecdh.cpp
+++ b/src/pubkey/ecdh/ecdh.cpp
@@ -24,7 +24,9 @@ SecureVector<byte> ECDH_KA_Operation::agree(const byte w[], u32bit w_len)
    PointGFp point = OS2ECP(w, w_len, curve);
 
    PointGFp S = (cofactor * point) * l_times_priv;
-   S.check_invariants();
+
+   if(!S.on_the_curve())
+      throw Internal_Error("ECDH: Agreed value was not on the curve");
 
    return BigInt::encode_1363(S.get_affine_x(),
                               curve.get_p().bytes());
diff --git a/src/pubkey/gost_3410/gost_3410.cpp b/src/pubkey/gost_3410/gost_3410.cpp
index e6f68526e..74b39d50b 100644
--- a/src/pubkey/gost_3410/gost_3410.cpp
+++ b/src/pubkey/gost_3410/gost_3410.cpp
@@ -74,14 +74,8 @@ GOST_3410_PublicKey::GOST_3410_PublicKey(const AlgorithmIdentifier& alg_id,
 
    public_key = PointGFp(domain().get_curve(), x, y);
 
-   try
-      {
-      public_key.check_invariants();
-      }
-   catch(Illegal_Point)
-      {
+   if(!public_key.on_the_curve())
       throw Internal_Error("Loaded GOST 34.10 public key failed self test");
-      }
    }
 
 namespace {
@@ -123,7 +117,9 @@ GOST_3410_Signature_Operation::sign(const byte msg[], u32bit msg_len,
       e = 1;
 
    PointGFp k_times_P = base_point * k;
-   k_times_P.check_invariants();
+
+   if(!k_times_P.on_the_curve())
+      throw Internal_Error("GOST 34.10 k*g not on the curve");
 
    BigInt r = k_times_P.get_affine_x() % order;