Bug 178827 - [PATCH] security/cryptopp: Bug in libc++ causes key generation to get stuck in endless loop
Summary: [PATCH] security/cryptopp: Bug in libc++ causes key generation to get stuck i...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Xin LI
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-22 02:40 UTC by Michael Gmelin
Modified: 2013-05-22 23:50 UTC (History)
1 user (show)

See Also:


Attachments
cryptopp-5.6.1_3.patch (1.33 KB, patch)
2013-05-22 02:40 UTC, Michael Gmelin
no flags Details | Diff
cryptopp-5.6.1_3.patch (2.82 KB, patch)
2013-05-22 12:37 UTC, Michael Gmelin
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Gmelin 2013-05-22 02:40:00 UTC
Due to a bug in the version of libc++ shipping with FreeBSD 9.1-RELEASE,
RSA key generation - and most likely other operations depending on
class PrimeSieve - gets stuck in an endless loop when compiled using
libc++ (CXXFLAGS+= -std=c++11 -stdlib=libc++).

The problem has been reported last year:

http://sourceforge.net/apps/trac/cryptopp/ticket/17
http://old.nabble.com/keygeneration-on-ios-using-clang(libc%2B%2B)-never-terminates-td34614981.html

It is caused by libc++'s implementation of std::find returning invalid
results when operating on std::vector<bool> (read: bitsets). The
problem has already been fixed in libc++:

http://llvm.org/viewvc/llvm-project?view=revision&revision=156546

which is also part of release 3.2 of the library. Unfortunately this is
not available in any release version of FreeBSD yet.

The attached patch works around this problem by conditionally replacing
the call to std::find with a hand written loop in case clang and an
affected version libc++ are detected. For all other compilers as well
as versions of libc++ the code is not altered at all. The ABI stays
stable, so there won't be any unpleasant surprises when upgrading the
port at a later point in time using an updated version of libc++, e.g.
after upgrading to 9.2-RELEASE.

The maintainer might want to communicate this issue upstream as well,
it's not limited to FreeBSD.

Added file(s):
- files/patch-nbtheory.cpp

Port maintainer (delphij@FreeBSD.org) is cc'd.

Generated with FreeBSD Port Tools 0.99_6 (mode: change, diff: suffix)

Fix: Apply the patch attached to this PR and update the package:

cd /usr/ports/security/cryptopp
patch -p1 </path/to/cryptopp-5.6.1_3.patch
portmaster cryptopp
How-To-Repeat: 
Requires a clang and libc++ enabled FreeBSD 9.1-RELEASE installation.

cd /usr/ports/security/cryptopp
CXX=clang++ CXXFLAGS="-std=c++11 -stdlib=libc++" \
make WITHOUT=STATIC install clean

cat >/tmp/test.cpp <<EOF
#include <cryptopp/rsa.h>
#include <cryptopp/osrng.h>
#include <cryptopp/base64.h>
#include <cryptopp/files.h>
using namespace CryptoPP;
int main()
{
  AutoSeededRandomPool rng;
  InvertibleRSAFunction privkey;
  privkey.Initialize(rng, 2048, 4);
}
EOF

clang++ -pthread -std=c++11 -stdlib=libc++ -I/usr/local/include \
-L/usr/local/lib -lcryptopp -o /tmp/test /tmp/test.cpp

/tmp/test

This is supposed to return immediately, but is hanging indefinitely
instead.
Comment 1 Edwin Groothuis freebsd_committer 2013-05-22 02:40:08 UTC
Responsible Changed
From-To: freebsd-ports-bugs->delphij

Over to maintainer (via the GNATS Auto Assign Tool)
Comment 2 Michael Gmelin 2013-05-22 12:37:58 UTC
Please find attached an altered version of the patch. This has nothing
to do with the original problem (the patch above addresses this just
fine), but instead it fixes one of the most obnoxious warnings of the
many cryptopp causes while building, e.g. clang emits the following
(gcc warns about it as well, but in a different way).

/usr/local/include/cryptopp/misc.h:414:8: warning: comparison of
unsigned /  expression < 0 is
      always false [-Wtautological-compare]
        if (a < 0)
            ~ ^ ~
/usr/local/include/cryptopp/simple.h:42:113: note: in instantiation of
function template
      specialization 'CryptoPP::IntToString<unsigned int>' requested
here ...rounds) : InvalidArgument(algorithm + ": " +
IntToString(rounds) + " is not a valid...
                                                  ^
2 warnings generated.

This is caused by the template function IntToString checking for a < 0
even if the template parameter is an unsigned type. There are literally
dozens of these warnings. The patch fixes this in an easy to audit way
- instead of modifying the code to use "abs" or change the semantics in
any way it creates two template functions depending on if the template
parameter is unsigned or not (thanks to Roland Bock for the patch).

It would be cool if you could get that in while updating the port,
since these warnings are all over the place not only while
building the port itself, but also when using the library in your own
code.

Cheers,
Michael

-- 
Michael Gmelin
Comment 3 dfilter service freebsd_committer 2013-05-22 23:41:50 UTC
Author: delphij
Date: Wed May 22 22:41:42 2013
New Revision: 318802
URL: http://svnweb.freebsd.org/changeset/ports/318802

Log:
  This changeset fixes two issues with crypto++ library:
  
   * patch-misc.h
  
     This fixes a warning triggered by testing an unsigned parameter against
     0.  The patch solves this by creating a different template for signed
     case.
  
   * patch-nbtheory.cpp
  
     This is a workaround for a bug with the current version of libc++ shipped
     with FreeBSD 9.x, which causes an infinite loop when generating RSA key,
     possibly also other operations.
  
  PR:		ports/178827
  Submitted by:	Michael Gmelin <freebsd grem de>

Added:
  head/security/cryptopp/files/patch-misc.h   (contents, props changed)
  head/security/cryptopp/files/patch-nbtheory.cpp   (contents, props changed)
Modified:
  head/security/cryptopp/Makefile

Modified: head/security/cryptopp/Makefile
==============================================================================
--- head/security/cryptopp/Makefile	Wed May 22 22:21:52 2013	(r318801)
+++ head/security/cryptopp/Makefile	Wed May 22 22:41:42 2013	(r318802)
@@ -3,7 +3,7 @@
 
 PORTNAME=		cryptopp
 PORTVERSION=		5.6.1
-PORTREVISION=		2
+PORTREVISION=		3
 CATEGORIES=		security
 MASTER_SITES=		SF \
 			http://www.cryptopp.com/

Added: head/security/cryptopp/files/patch-misc.h
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/security/cryptopp/files/patch-misc.h	Wed May 22 22:41:42 2013	(r318802)
@@ -0,0 +1,54 @@
+--- misc.h.orig	2010-08-06 18:46:18.000000000 +0000
++++ misc.h	2013-05-22 08:43:01.949194748 +0000
+@@ -405,17 +405,13 @@
+ 	return order == GetNativeByteOrder();
+ }
+ 
++template<bool> struct IsUnsigned {};
++
+ template <class T>
+-std::string IntToString(T a, unsigned int base = 10)
++std::string IntToStringImpl(T a, unsigned int base, IsUnsigned<true>)
+ {
+ 	if (a == 0)
+ 		return "0";
+-	bool negate = false;
+-	if (a < 0)
+-	{
+-		negate = true;
+-		a = 0-a;	// VC .NET does not like -a
+-	}
+ 	std::string result;
+ 	while (a > 0)
+ 	{
+@@ -423,11 +419,30 @@
+ 		result = char((digit < 10 ? '0' : ('a' - 10)) + digit) + result;
+ 		a /= base;
+ 	}
++	return result;
++}
++
++template <class T>
++std::string IntToStringImpl(T a, unsigned int base, IsUnsigned<false>)
++{
++	bool negate = false;
++	if (a < 0)
++	{
++		negate = true;
++		a = 0-a;	// VC .NET does not like -a
++	}
++	std::string result = IntToStringImpl(a, base, IsUnsigned<true>());
+ 	if (negate)
+ 		result = "-" + result;
+ 	return result;
+ }
+ 
++template <class T>
++std::string IntToString(T a, unsigned int base = 10)
++{
++	return IntToStringImpl(a, base, IsUnsigned<(static_cast<T>(-1) > 0)>());
++}
++
+ template <class T1, class T2>
+ inline T1 SaturatingSubtract(const T1 &a, const T2 &b)
+ {

Added: head/security/cryptopp/files/patch-nbtheory.cpp
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/security/cryptopp/files/patch-nbtheory.cpp	Wed May 22 22:41:42 2013	(r318802)
@@ -0,0 +1,21 @@
+--- nbtheory.cpp.orig	2013-05-22 00:16:26.761193859 +0000
++++ nbtheory.cpp	2013-05-22 00:15:29.401256454 +0000
+@@ -307,7 +307,18 @@
+ 
+ bool PrimeSieve::NextCandidate(Integer &c)
+ {
++#if defined(__clang__) && defined(_LIBCPP_VERSION) &&  _LIBCPP_VERSION < 1101
++	// Workaround for a bug in libc++ in std::find on std::vector<bool>
++	std::vector<bool>::iterator pos = m_sieve.begin()+m_next;
++	for (std::vector<bool>::iterator end = m_sieve.end(); pos != end; ++pos)
++	{
++		if (*pos == false)
++			break;
++	}
++	bool safe = SafeConvert(pos - m_sieve.begin(), m_next);
++#else
+ 	bool safe = SafeConvert(std::find(m_sieve.begin()+m_next, m_sieve.end(), false) - m_sieve.begin(), m_next);
++#endif
+ 	assert(safe);
+ 	if (m_next == m_sieve.size())
+ 	{
_______________________________________________
svn-ports-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-ports-all
To unsubscribe, send any mail to "svn-ports-all-unsubscribe@freebsd.org"
Comment 4 Xin LI freebsd_committer 2013-05-22 23:42:32 UTC
State Changed
From-To: open->closed

Committed, thanks!