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.

8 files changed, 49 insertions, 71 deletions

diff --git a/checks/ec_tests.cpp b/checks/ec_tests.cpp
index b3111455d..9362aa371 100644
--- a/checks/ec_tests.cpp
+++ b/checks/ec_tests.cpp
@@ -147,7 +147,8 @@ void test_coordinates()
    PointGFp p0 = p_G;
    PointGFp p1 = p_G * 2;
    PointGFp point_exp(secp160r1, exp_affine_x, exp_affine_y);
-   point_exp.check_invariants();
+   if(!point_exp.on_the_curve())
+      throw Internal_Error("Point not on the curve");
 
    CHECK_MESSAGE( p1.get_affine_x() == exp_affine_x, " p1_x = " << p1.get_affine_x() << "\n" << "exp_x = " << exp_affine_x << "\n");
    CHECK_MESSAGE( p1.get_affine_y() == exp_affine_y, " p1_y = " << p1.get_affine_y() << "\n" << "exp_y = " << exp_affine_y << "\n");
@@ -246,7 +247,8 @@ void test_zeropoint()
                BigInt("16984103820118642236896513183038186009872590470"),
                BigInt("1373093393927139016463695321221277758035357890939"));
 
-   p1.check_invariants();
+   if(!p1.on_the_curve())
+      throw Internal_Error("Point not on the curve");
    p1 -= p1;
 
    CHECK_MESSAGE(  p1.is_zero(), "p - q with q = p is not zero!");
@@ -294,7 +296,8 @@ void test_calc_with_zeropoint()
               BigInt("16984103820118642236896513183038186009872590470"),
               BigInt("1373093393927139016463695321221277758035357890939"));
 
-   p.check_invariants();
+   if(!p.on_the_curve())
+      throw Internal_Error("Point not on the curve");
    CHECK_MESSAGE(  !p.is_zero(), "created is zeropoint, shouldn't be!");
 
    PointGFp zero(curve);
@@ -618,7 +621,8 @@ void test_enc_dec_uncompressed_521_prime_too_large()
    try
       {
       p_G = std::auto_ptr<PointGFp>(new PointGFp(OS2ECP ( sv_G_secp_uncomp, secp521r1)));
-      p_G->check_invariants();
+      if(!p_G->on_the_curve())
+         throw Internal_Error("Point not on the curve");
       }
    catch (std::exception e)
       {
@@ -626,11 +630,6 @@ void test_enc_dec_uncompressed_521_prime_too_large()
       }
 
    CHECK_MESSAGE(exc, "attempt of creation of point on curve with too high prime did not throw an exception");
-   //SecureVector<byte> sv_result = EC2OSP(p_G, PointGFp::UNCOMPRESSED);
-   //string result = hex_encode(sv_result.begin(), sv_result.size());
-   //string exp_result = hex_encode(sv_G_secp_uncomp.begin(), sv_G_secp_uncomp.size());
-
-   //CHECK_MESSAGE( sv_result == sv_G_secp_uncomp, "\ncalc. result = " << result << "\nexp. result = " << exp_result << "\n");
    }
 
 void test_gfp_store_restore()
@@ -673,7 +672,8 @@ void test_cdc_curve_33()
    bool exc = false;
    try
       {
-      p_G.check_invariants();
+      if(!p_G.on_the_curve())
+         throw Internal_Error("Point not on the curve");
       }
    catch (std::exception)
       {
@@ -698,11 +698,14 @@ void test_more_zeropoint()
                BigInt("16984103820118642236896513183038186009872590470"),
                BigInt("1373093393927139016463695321221277758035357890939"));
 
-   p1.check_invariants();
+   if(!p1.on_the_curve())
+      throw Internal_Error("Point not on the curve");
    PointGFp minus_p1 = -p1;
-   minus_p1.check_invariants();
+   if(!minus_p1.on_the_curve())
+      throw Internal_Error("Point not on the curve");
    PointGFp shouldBeZero = p1 + minus_p1;
-   shouldBeZero.check_invariants();
+   if(!shouldBeZero.on_the_curve())
+      throw Internal_Error("Point not on the curve");
 
    BigInt y1 = p1.get_affine_y();
    y1 = curve.get_p() - y1;
@@ -713,7 +716,8 @@ void test_more_zeropoint()
                  "problem with minus_p1 : y");
 
    PointGFp zero(curve);
-   zero.check_invariants();
+   if(!zero.on_the_curve())
+      throw Internal_Error("Point not on the curve");
    CHECK_MESSAGE(p1 + zero == p1, "addition of zero modified point");
 
    CHECK_MESSAGE(  shouldBeZero.is_zero(), "p - q with q = p is not zero!");
diff --git a/checks/ecdsa.cpp b/checks/ecdsa.cpp
index d46ebb25e..3d2ee37f3 100644
--- a/checks/ecdsa.cpp
+++ b/checks/ecdsa.cpp
@@ -304,7 +304,8 @@ void test_create_and_verify(RandomNumberGenerator& rng)
    PointGFp p_G = OS2ECP ( sv_G_secp_comp, curve );
 
    EC_Domain_Params dom_params(curve, p_G, bi_order_g, BigInt(1));
-   p_G.check_invariants();
+   if(!p_G.on_the_curve())
+      throw Internal_Error("Point not on the curve");
 
    ECDSA_PrivateKey key_odd_oid(rng, dom_params);
    std::string key_odd_oid_str = PKCS8::PEM_encode(key_odd_oid);
@@ -359,7 +360,6 @@ void test_curve_registry(RandomNumberGenerator& rng)
          {
          OID oid(oids[i]);
          EC_Domain_Params dom_pars(oid);
-         dom_pars.get_base_point().check_invariants();
          ECDSA_PrivateKey ecdsa(rng, dom_pars);
 
          PK_Signer signer(ecdsa, "EMSA1(SHA-1)");
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;