From 08c0e260200b3441c43bb529b5dbe7cdff6e37f7 Mon Sep 17 00:00:00 2001 From: Jeffrey Walton Date: Fri, 20 Jan 2017 06:10:14 -0500 Subject: [PATCH] Add CRYPTOPP_ASSERT to Validate routines Since we switched to CRYPTOPP_ASSERT we don't have to worry about an accidental assert in production. We can now assert ValidateElement and ValidateGroup and let the code warn of potential problems during development. This came about because ECGDSA inadvertently used GetGroupOrder() rather than GetSubgroupOrder(). The assert alerted to the problem area without the need for debugging --- bench2.cpp | 6 +++--- eccrypto.cpp | 16 +++++++++++++++- eccrypto.h | 17 ++++++++++++++--- gfpcrypt.cpp | 27 ++++++++++++++++++++++++++- 4 files changed, 58 insertions(+), 8 deletions(-) diff --git a/bench2.cpp b/bench2.cpp index 01c45b36..c0db1f00 100644 --- a/bench2.cpp +++ b/bench2.cpp @@ -1,11 +1,12 @@ // bench2.cpp - written and placed in the public domain by Wei Dai #include "cryptlib.h" +#include "bench.h" +#include "validate.h" + #include "pubkey.h" #include "gfpcrypt.h" #include "eccrypto.h" -#include "bench.h" -#include "validate.h" #include "files.h" #include "filters.h" @@ -15,7 +16,6 @@ #include "dsa.h" #include "luc.h" #include "rw.h" -#include "eccrypto.h" #include "ecp.h" #include "ec2n.h" #include "asn.h" diff --git a/eccrypto.cpp b/eccrypto.cpp index c1a9c7e0..133194ea 100644 --- a/eccrypto.cpp +++ b/eccrypto.cpp @@ -586,17 +586,23 @@ template bool DL_GroupParameters_EC::ValidateGroup(RandomNumberGenerator &rng, unsigned int level) const { bool pass = GetCurve().ValidateParameters(rng, level); + CRYPTOPP_ASSERT(pass); Integer q = GetCurve().FieldSize(); pass = pass && m_n!=q; + CRYPTOPP_ASSERT(pass); if (level >= 2) { Integer qSqrt = q.SquareRoot(); pass = pass && m_n>4*qSqrt; + CRYPTOPP_ASSERT(pass); pass = pass && VerifyPrime(rng, m_n, level-2); + CRYPTOPP_ASSERT(pass); pass = pass && (m_k.IsZero() || m_k == (q+2*qSqrt+1)/m_n); + CRYPTOPP_ASSERT(pass); pass = pass && CheckMOVCondition(q, m_n); + CRYPTOPP_ASSERT(pass); } return pass; @@ -605,17 +611,25 @@ bool DL_GroupParameters_EC::ValidateGroup(RandomNumberGenerator &rng, unsign template bool DL_GroupParameters_EC::ValidateElement(unsigned int level, const Element &g, const DL_FixedBasePrecomputation *gpc) const { - bool pass = !IsIdentity(g) && GetCurve().VerifyPoint(g); + bool pass = !IsIdentity(g); + CRYPTOPP_ASSERT(pass); + pass = pass && GetCurve().VerifyPoint(g); + CRYPTOPP_ASSERT(pass); + if (level >= 1) { if (gpc) + { pass = pass && gpc->Exponentiate(this->GetGroupPrecomputation(), Integer::One()) == g; + CRYPTOPP_ASSERT(pass); + } } if (level >= 2 && pass) { const Integer &q = GetSubgroupOrder(); Element gq = gpc ? gpc->Exponentiate(this->GetGroupPrecomputation(), q) : this->ExponentiateElement(g, q); pass = pass && IsIdentity(gq); + CRYPTOPP_ASSERT(pass); } return pass; } diff --git a/eccrypto.h b/eccrypto.h index 0f106228..1c178384 100644 --- a/eccrypto.h +++ b/eccrypto.h @@ -428,7 +428,11 @@ public: //! \param x the private exponent //! \details This Initialize() function overload initializes a private key from existing parameters. void Initialize(const DL_GroupParameters_EC ¶ms, const Integer &x) - {this->AccessGroupParameters() = params; this->SetPrivateExponent(x);} + { + this->AccessGroupParameters() = params; + this->SetPrivateExponent(x); + CRYPTOPP_ASSERT(x>=1 && x<=params.GetSubgroupOrder()-1); + } //! \brief Initialize an EC Private Key using {EC,G,n,x} //! \param ec the elliptic curve @@ -437,7 +441,13 @@ public: //! \param x the private exponent //! \details This Initialize() function overload initializes a private key from existing parameters. void Initialize(const EC &ec, const Element &G, const Integer &n, const Integer &x) - {this->AccessGroupParameters().Initialize(ec, G, n); this->SetPrivateExponent(x);} + { + this->AccessGroupParameters().Initialize(ec, G, n); + this->SetPrivateExponent(x); + + DL_GroupParameters_EC ¶ms = this->AccessGroupParameters(); + CRYPTOPP_ASSERT(x>=1 && x<=params.GetSubgroupOrder()-1); + } //! \brief Create an EC private key //! \param rng a RandomNumberGenerator derived class @@ -463,8 +473,9 @@ public: { const DL_GroupParameters& params = this->GetAbstractGroupParameters(); pub.AccessAbstractGroupParameters().AssignFrom(params); - const Integer &xInv = this->GetPrivateExponent().InverseMod(params.GetGroupOrder()); + const Integer &xInv = this->GetPrivateExponent().InverseMod(params.GetSubgroupOrder()); pub.SetPublicElement(params.ExponentiateBase(xInv)); + CRYPTOPP_ASSERT(xInv.NotZero()); } virtual bool GetVoidValue(const char *name, const std::type_info &valueType, void *pValue) const diff --git a/gfpcrypt.cpp b/gfpcrypt.cpp index 3d2b9251..3badb261 100644 --- a/gfpcrypt.cpp +++ b/gfpcrypt.cpp @@ -70,8 +70,12 @@ void DL_GroupParameters_DSA::GenerateRandom(RandomNumberGenerator &rng, const Na bool DL_GroupParameters_DSA::ValidateGroup(RandomNumberGenerator &rng, unsigned int level) const { bool pass = DL_GroupParameters_GFP::ValidateGroup(rng, level); - int pSize = GetModulus().BitCount(), qSize = GetSubgroupOrder().BitCount(); + CRYPTOPP_ASSERT(pass); + + const int pSize = GetModulus().BitCount(), qSize = GetSubgroupOrder().BitCount(); pass = pass && ((pSize==1024 && qSize==160) || (pSize==2048 && qSize==224) || (pSize==2048 && qSize==256) || (pSize==3072 && qSize==256)); + CRYPTOPP_ASSERT(pass); + return pass; } @@ -132,12 +136,20 @@ bool DL_GroupParameters_IntegerBased::ValidateGroup(RandomNumberGenerator &rng, bool pass = true; pass = pass && p > Integer::One() && p.IsOdd(); + CRYPTOPP_ASSERT(pass); pass = pass && q > Integer::One() && q.IsOdd(); + CRYPTOPP_ASSERT(pass); if (level >= 1) + { pass = pass && GetCofactor() > Integer::One() && GetGroupOrder() % q == Integer::Zero(); + CRYPTOPP_ASSERT(pass); + } if (level >= 2) + { pass = pass && VerifyPrime(rng, q, level-2) && VerifyPrime(rng, p, level-2); + CRYPTOPP_ASSERT(pass); + } return pass; } @@ -148,17 +160,26 @@ bool DL_GroupParameters_IntegerBased::ValidateElement(unsigned int level, const bool pass = true; pass = pass && GetFieldType() == 1 ? g.IsPositive() : g.NotNegative(); + CRYPTOPP_ASSERT(pass); + pass = pass && g < p && !IsIdentity(g); + CRYPTOPP_ASSERT(pass); if (level >= 1) { if (gpc) + { pass = pass && gpc->Exponentiate(GetGroupPrecomputation(), Integer::One()) == g; + CRYPTOPP_ASSERT(pass); + } } if (level >= 2) { if (GetFieldType() == 2) + { pass = pass && Jacobi(g*g-4, p)==-1; + CRYPTOPP_ASSERT(pass); + } // verifying that Lucas((p+1)/2, w, p)==2 is omitted because it's too costly // and at most 1 bit is leaked if it's false @@ -168,9 +189,13 @@ bool DL_GroupParameters_IntegerBased::ValidateElement(unsigned int level, const { Integer gp = gpc ? gpc->Exponentiate(GetGroupPrecomputation(), q) : ExponentiateElement(g, q); pass = pass && IsIdentity(gp); + CRYPTOPP_ASSERT(pass); } else if (GetFieldType() == 1) + { pass = pass && Jacobi(g, p) == 1; + CRYPTOPP_ASSERT(pass); + } } return pass;