Skip to content
Permalink
Browse files

Fix ECP leakage in Add() and Double() (GH #869, PR #871)

This check-in provides the fix for leaks in ECP's Add() and Double(). The fixes were taken from Joost Renes, Craig Costello, and Lejla Batina's [Complete addition formulas for prime order elliptic curves](https://eprint.iacr.org/2015/1060.pdf).

The Pull Request includes two additional changes that were related to testing the primary fix. First, an `AuthenticatedKeyAgreementWithRolesValidate` interface was added. It allows us to test key agreement when roles are involved. Roles are "client", "server", "initiator", "recipient", etc.

Second, `SetGlobalSeed` was added to `test.cpp` to help with reproducible results. We had code in two different places that set the seed value for the random number generator. But it was sloppy and doing a poor job since results could not be reproduced under some circumstances.
  • Loading branch information
noloader committed Aug 5, 2019
1 parent b3eb4c6 commit c9ef9420e762b91cc06463d349cf06e04c749b9d
Showing with 613 additions and 130 deletions.
  1. +1 −1 cryptest.vcxproj.user
  2. +6 −5 ec2n.cpp
  3. +0 −1 ec2n.h
  4. +10 −0 eccrypto.cpp
  5. +4 −0 eccrypto.h
  6. +430 −27 ecp.cpp
  7. +36 −0 ecp.h
  8. +10 −28 fhmqv.h
  9. +13 −31 hmqv.h
  10. +14 −1 mqv.cpp
  11. +50 −27 test.cpp
  12. +1 −1 validat6.cpp
  13. +38 −8 validat7.cpp
@@ -3,4 +3,4 @@
<PropertyGroup>
<LocalDebuggerCommandArguments>v</LocalDebuggerCommandArguments>
</PropertyGroup>
</Project>
</Project>
@@ -16,7 +16,8 @@ ANONYMOUS_NAMESPACE_BEGIN
using CryptoPP::EC2N;

#if defined(HAVE_GCC_INIT_PRIORITY)
const EC2N::Point g_identity __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 50))) = EC2N::Point();
#define INIT_ATTRIBUTE __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 51)))
const EC2N::Point g_identity INIT_ATTRIBUTE = EC2N::Point();
#elif defined(HAVE_MSC_INIT_PRIORITY)
#pragma warning(disable: 4075)
#pragma init_seg(".CRT$XCU")
@@ -51,8 +52,8 @@ void EC2N::DEREncode(BufferedTransformation &bt) const
{
m_field->DEREncode(bt);
DERSequenceEncoder seq(bt);
m_field->DEREncodeElement(seq, m_a);
m_field->DEREncodeElement(seq, m_b);
m_field->DEREncodeElement(seq, m_a);
m_field->DEREncodeElement(seq, m_b);
seq.MessageEnd();
}

@@ -260,7 +261,7 @@ const EC2N::Point& EC2N::Double(const Point &P) const

// ********************************************************

/*
#if 0
EcPrecomputation<EC2N>& EcPrecomputation<EC2N>::operator=(const EcPrecomputation<EC2N> &rhs)
{
m_ec = rhs.m_ec;
@@ -312,7 +313,7 @@ EC2N::Point EcPrecomputation<EC2N>::CascadeExponentiate(const Integer &exponent,
{
return m_ep.CascadeExponentiate(exponent, static_cast<const EcPrecomputation<EC2N> &>(pc2).m_ep, exponent2);
}
*/
#endif

NAMESPACE_END

1 ec2n.h
@@ -3,7 +3,6 @@
/// \file ec2n.h
/// \brief Classes for Elliptic Curves over binary fields


#ifndef CRYPTOPP_EC2N_H
#define CRYPTOPP_EC2N_H

@@ -28,6 +28,9 @@
#include "ec2n.h"
#include "misc.h"

#include <iostream>
#include <sstream>

// Squash MS LNK4221 and libtool warnings
#ifndef CRYPTOPP_MANUALLY_INSTANTIATE_TEMPLATES
extern const char ECCRYPTO_FNAME[] = __FILE__;
@@ -683,6 +686,13 @@ OID DL_GroupParameters_EC<EC>::GetAlgorithmID() const
return ASN1::id_ecPublicKey();
}

std::ostream& operator<<(std::ostream& os, const DL_GroupParameters_EC<ECP>::Element& obj)
{
std::ostringstream oss;
oss << "(" << std::hex << obj.x << ", " << std::hex << obj.y << ")";
return os << oss.str();
}

// ******************************************************************

template <class EC>
@@ -22,6 +22,8 @@
#include "ecp.h"
#include "ec2n.h"

#include <iosfwd>

#if CRYPTOPP_MSC_VERSION
# pragma warning(push)
# pragma warning(disable: 4231 4275)
@@ -168,6 +170,8 @@ class DL_GroupParameters_EC : public DL_GroupParametersImpl<EcPrecomputation<EC>
mutable bool m_compress, m_encodeAsOID; // presentation details
};

inline std::ostream& operator<<(std::ostream& os, const DL_GroupParameters_EC<ECP>::Element& obj);

/// \brief Elliptic Curve Discrete Log (DL) public key
/// \tparam EC elliptic curve field
template <class EC>

2 comments on commit c9ef942

@mouse07410

This comment has been minimized.

Copy link
Collaborator

mouse07410 replied Aug 5, 2019

Seems to successfully work on at least one of my machines (the biggest/trickiest one). Will post reports when can test on other boxes as well.

@mouse07410

This comment has been minimized.

Copy link
Collaborator

mouse07410 replied Aug 6, 2019

@noloader tested on two out of three machines - worked. Need to test it on the last one, but you can probably merge this.

Please sign in to comment.
You can’t perform that action at this time.