search

weidai11 / cryptopp

free C++ class library of cryptographic schemes
https://cryptopp.com
Other
4.89k stars 1.51k forks source link

Building cryptest.exe fails on MinGW-w64 #28

Closed IlyaBizyaev closed 9 years ago

IlyaBizyaev commented 9 years ago 
g++ -DNDEBUG -g2 -O3 -march=native -Wall -Wextra -Wno-type-limits -Wno-unknown-pragmas -pipe -c validat0.cpp
validat0.cpp: In function 'bool TestSettings()':
validat0.cpp:27:47: error: call of overloaded 'memcpy_s(CryptoPP::word32*, long long unsigned int, const char [5], int)' is ambiguous
  memcpy_s(&w, sizeof(w), "\x01\x02\x03\x04", 4);
                                               ^
validat0.cpp:27:47: note: candidates are:
In file included from stdcpp.h:12:0,
                 from validat0.cpp:6:
c:/MinGW-w64/mingw64/x86_64-w64-mingw32/include/string.h:42:27: note: errno_t memcpy_s(void*, size_t, const void*, size_t)
   _CRTIMP errno_t __cdecl memcpy_s (void *_dest,size_t _numberOfElements,const
void *_src,size_t _count);
                           ^
In file included from validat0.cpp:7:0:
misc.h:201:13: note: void CryptoPP::memcpy_s(void*, size_t, const void*, size_t)

 inline void memcpy_s(void *dest, size_t sizeInBytes, const void *src, size_t co
unt)
             ^
make: *** [validat0.o] Error 1

g++ -v output:

Using built-in specs.
COLLECT_GCC=c:\MinGW-w64\mingw64\bin\g++.exe
COLLECT_LTO_WRAPPER=c:/MinGW-w64/mingw64/bin/../libexec/gcc/x86_64-w64-mingw32/4.9.2/lto-wrapper.exe
Target: x86_64-w64-mingw32
Configured with: ../../../src/gcc-4.9.2/configure --host=x86_64-w64-mingw32 --build=x86_64-w64-mingw32 --target=x86_64-w64-mingw32 --prefix=/mingw64 --with-sysroot=/c/mingw492/x86_64-492-win32-seh-rt_v4-rev3/mingw64 --with-gxx-include-dir=/mingw64/x86_64-w64-mingw32/include/c++ --enable-shared --enable-static --disable-multilib --enable-languages=ada,c,c++,fortran,objc,obj-c++,lto --enable-libstdcxx-time=yes --enable-threads=win32 --enable-libgomp --enable-libatomic --enable-lto --enable-graphite --enable-checking=release --enable-fully-dynamic-string --enable-version-specific-runtime-libs --disable-isl-version-check --disable-cloog-version-check --disable-libstdcxx-pch --disable-libstdcxx-debug --enable-bootstrap --disable-rpath --disable-win32-registry --disable-nls --disable-werror --disable-symvers --with-gnu-as --with-gnu-ld --with-arch=nocona --with-tune=core2 --with-libiconv --with-system-zlib --with-gmp=/c/mingw492/prerequisites/x86_64-w64-mingw32-static --with-mpfr=/c/mingw492/prerequisites/x86_64-w64-mingw32-static --with-mpc=/c/mingw492/prerequisites/x86_64-w64-mingw32-static --with-isl=/c/mingw492/prerequisites/x86_64-w64-mingw32-static --with-cloog=/c/mingw492/prerequisites/x86_64-w64-mingw32-static --enable-cloog-backend=isl --with-pkgversion='x86_64-win32-seh-rev3, Built by MinGW-W64 project' --with-bugurl=http://sourceforge.net/projects/mingw-w64 CFLAGS='-O2 -pipe -I/c/mingw492/x86_64-492-win32-seh-rt_v4-rev3/mingw64/opt/include -I/c/mingw492/prerequisites/x86_64-zlib-static/include -I/c/mingw492/prerequisites/x86_64-w64-mingw32-static/include' CXXFLAGS='-O2 -pipe -I/c/mingw492/x86_64-492-win32-seh-rt_v4-rev3/mingw64/opt/include -I/c/mingw492/prerequisites/x86_64-zlib-static/include -I/c/mingw492/prerequisites/x86_64-w64-mingw32-static/include' CPPFLAGS= LDFLAGS='-pipe -L/c/mingw492/x86_64-492-win32-seh-rt_v4-rev3/mingw64/opt/lib -L/c/mingw492/prerequisites/x86_64-zlib-static/lib -L/c/mingw492/prerequisites/x86_64-w64-mingw32-static/lib '
Thread model: win32
gcc version 4.9.2 (x86_64-win32-seh-rev3, Built by MinGW-W64 project)
ThatAIGeek commented 9 years ago

so did someone fixed that?

fleith commented 9 years ago

To fix this error you can put in front of the function memcpy_s your namespace. If you put CryptoPP::memcpy_s the compiler doesn't have ambiguous problem.

noloader commented 9 years ago

@IlyaBizyaev - forgive my ignorance... When I search MinGW's site for the 64-bit download, I cannot find it. Confer: search: mingw64. I did find a quote: "In the future, other subsystems such as mingw64 may be supported...". The best I can tell, there is no MinGW-64.

Where, exactly, does MinGW make the download available?

(I can see where this ambiguity comes from, and we are going to clear it. I'm asking about the download because I want to get a test environment setup for this platform).

IlyaBizyaev commented 9 years ago

The download page is here: http://mingw-w64.org/doku.php/download If I remember it right, you need the "Mingw-build" one for 4.9.2.

noloader commented 9 years ago

Thanks Ilya.

Something looks very fishy here... MinGW does not recognize the project. And it appears different individuals have administrative control, and the projects are based in different countries. At minimum, I would expect email addresses to be consistent. I.e., both use _mingw-users-owner@lists.sourceforge.net_ because its MinGW. And I definetly don't trust these garbage addresses: _b6wnwyoptcoe5n6efws9@e.o-w-o.info_.

$ whois mingw-w64.org | grep Registrant
Registrant ID:ovh555cd417awjj
Registrant Name:Adrien Nader
Registrant Organization:
Registrant Street: mingw-w64.org, office #8303039
Registrant Street: c/o OwO, BP80157
Registrant City:Roubaix Cedex 1
Registrant State/Province:
Registrant Postal Code:59053
Registrant Country:FR
Registrant Phone:+33.899498765
Registrant Phone Ext:
Registrant Fax:
Registrant Fax Ext:
Registrant Email:b6wnwyoptcoe5n6efws9@e.o-w-o.info

$ whois mingw.org | grep Registrant
Registrant ID:fed46a4501b17109
Registrant Name:Mumit  Khan
Registrant Organization:Mumit Khan
Registrant Street: 520 N. Pinckney St., Suite 1
Registrant City:Madison
Registrant State/Province:WI
Registrant Postal Code:53703
Registrant Country:US
Registrant Phone:+1.6082588241
Registrant Phone Ext:
Registrant Fax: +1.6082588241
Registrant Fax Ext:
Registrant Email:mingw-users-owner@lists.sourceforge.net
IlyaBizyaev commented 9 years ago

Yes, MinGW-w64 is a fork, but a popular one. It seems to me that lots of former MinGW project participants moved there.

IlyaBizyaev commented 9 years ago

Just read more on the main page: http://mingw-w64.org/doku.php/start

IlyaBizyaev commented 9 years ago

Here is their mailing list: mingw-w64-public@lists.sourceforge.net

noloader commented 9 years ago

Thanks again.

I followed the link you provided, then followed to downloads. It took me to http://mingw-w64.org/doku.php/download. In the download matrix, I tried to download MinGW builds (http://mingw-w64.org/doku.php/download/mingw-builds), Win-Builds (http://mingw-w64.org/doku.php/download/win-builds) and Msys2 (http://mingw-w64.org/doku.php/download/msys2). None of them appear to provide a MinGW64 download. It looks like they sent me to empty wiki pages.

Where, exactly, is there download in that table?

Or, can you test RC6 at https://sourceforge.net/projects/cryptopp/files/cryptopp/5.6.3/ ?

Or, can you provide me with remote access to one of your machines that has it installed?

Zireael-N commented 9 years ago

Msys2 (http://mingw-w64.org/doku.php/download/msys2). None of them appear to provide a MinGW64 download. It looks like they sent me to empty wiki pages.

Assuming you installed Msys2, you need to do the following on the first launch:

5. Update the system packages with
pacman --needed -Sy bash pacman pacman-mirrors msys2-runtime
6. Close MSYS2, run it again from Start menu and update the rest with
pacman -Su

Then you can install mingw-w64-i686-gcc and mingw-w64-x86_64-gcc with pacman -S <package>. Don't forget to install make as well.

Msys2 provides 3 shortcuts: "MSYS2 Shell", "MinGW-w64 Win32 Shell" and "MinGW-w64 Win64 Shell", you need to use the 2nd to compile 32-bit binaries and the 3rd to compile 64-bit binaries.

Zireael-N commented 9 years ago

This happens because MinGW-w64 has its own implementation of memcpy_s, yet doesn't have these defined: _MEMORY_S_DEFINED and __STDC_WANT_SECURE_LIB__.

You can fix this by replacing:

#if (!__STDC_WANT_SECURE_LIB__ && !defined(_MEMORY_S_DEFINED))

with:

#if (!__STDC_WANT_SECURE_LIB__ && !defined(_MEMORY_S_DEFINED) && !defined(__MINGW64__))

in misc.h.

IlyaBizyaev commented 9 years ago

Sorry, I cannot remember the exact download. It seems to me that I've used the official MinGW-w64 Installer for that purpose. You can find it here: http://sourceforge.net/projects/mingw-w64/ Here is my full gcc -v output:

Using built-in specs.
COLLECT_GCC=c:\MinGW-w64\mingw64\bin\gcc.exe
COLLECT_LTO_WRAPPER=c:/MinGW-w64/mingw64/bin/../libexec/gcc/x86_64-w64-mingw32/4
.9.2/lto-wrapper.exe
Target: x86_64-w64-mingw32
Configured with: ../../../src/gcc-4.9.2/configure --host=x86_64-w64-mingw32 --bu
ild=x86_64-w64-mingw32 --target=x86_64-w64-mingw32 --prefix=/mingw64 --with-sysr
oot=/c/mingw492/x86_64-492-win32-seh-rt_v4-rev3/mingw64 --with-gxx-include-dir=/
mingw64/x86_64-w64-mingw32/include/c++ --enable-shared --enable-static --disable
-multilib --enable-languages=ada,c,c++,fortran,objc,obj-c++,lto --enable-libstdc
xx-time=yes --enable-threads=win32 --enable-libgomp --enable-libatomic --enable-
lto --enable-graphite --enable-checking=release --enable-fully-dynamic-string --
enable-version-specific-runtime-libs --disable-isl-version-check --disable-cloog
-version-check --disable-libstdcxx-pch --disable-libstdcxx-debug --enable-bootst
rap --disable-rpath --disable-win32-registry --disable-nls --disable-werror --di
sable-symvers --with-gnu-as --with-gnu-ld --with-arch=nocona --with-tune=core2 -
-with-libiconv --with-system-zlib --with-gmp=/c/mingw492/prerequisites/x86_64-w6
4-mingw32-static --with-mpfr=/c/mingw492/prerequisites/x86_64-w64-mingw32-static
 --with-mpc=/c/mingw492/prerequisites/x86_64-w64-mingw32-static --with-isl=/c/mi
ngw492/prerequisites/x86_64-w64-mingw32-static --with-cloog=/c/mingw492/prerequi
sites/x86_64-w64-mingw32-static --enable-cloog-backend=isl --with-pkgversion='x8
6_64-win32-seh-rev3, Built by MinGW-W64 project' --with-bugurl=http://sourceforg
e.net/projects/mingw-w64 CFLAGS='-O2 -pipe -I/c/mingw492/x86_64-492-win32-seh-rt
_v4-rev3/mingw64/opt/include -I/c/mingw492/prerequisites/x86_64-zlib-static/incl
ude -I/c/mingw492/prerequisites/x86_64-w64-mingw32-static/include' CXXFLAGS='-O2
 -pipe -I/c/mingw492/x86_64-492-win32-seh-rt_v4-rev3/mingw64/opt/include -I/c/mi
ngw492/prerequisites/x86_64-zlib-static/include -I/c/mingw492/prerequisites/x86_
64-w64-mingw32-static/include' CPPFLAGS= LDFLAGS='-pipe -L/c/mingw492/x86_64-492
-win32-seh-rt_v4-rev3/mingw64/opt/lib -L/c/mingw492/prerequisites/x86_64-zlib-st
atic/lib -L/c/mingw492/prerequisites/x86_64-w64-mingw32-static/lib '
Thread model: win32
gcc version 4.9.2 (x86_64-win32-seh-rev3, Built by MinGW-W64 project)

By the way, @Zireael-N, I simply use MSYS that comes with MinGW-32.

Zireael-N commented 9 years ago

By the way, @Zireael-N, I simply use MSYS that comes with MinGW-32.

The Msys2 part was addressed to Jeffrey. Have you tried doing what I suggested here?
Here's the exact line: https://github.com/weidai11/cryptopp/blob/1d5bcc08fb618897bce687c152033b93dffa633e/misc.h#L212.

noloader commented 9 years ago

@IlyaBizyaev, @Zireael-N - I have more bad news.... I tried to install MinGW and MinGW-64 on two different machines - Windows 7 and Windows 8. Only one of them ever worked for me (MinGW/Windows 7); the other three never worked.

Now the WIndows 7/MinGW machine is broken:

$ cd cryptopp
$ ./cryptest.sh
sh: ./cryptest.sh: /bin/bash: bad interpreter: No such file or directory

The same tests ran fine yesterday. I've tried reinstalling MinGW, but that did not help.

MinGW and MinGW-64 don't seem to have the stability we expect or need (or at least for me under the role of tester). MinGW and MinGW-64 cause me more problems than Debian Unstable :) I'm going to remove MinGW and MinGW-64 from my testing regime. It causes too many problems (and a lot of frustration), and the time can be better spent elsewhere (for me).

We talked about similar on the Crypto++ mailing list recently; see Strategy for Accommodating Unstable Platforms and Tools. Consensus was we should make an effort, but not waste too much time on platforms like these. Group consensus is important because it gives me authority to act. Without consensus, I have no authority and I cannot act.

Don't read too much into it. I only said "... remove MinGW and MinGW-64 from my testing regime" because my time is better spent elsewhere. I did not say we should drop support altogether.

Would either of you guys pick up the torch and take responsibility for this platform?

Would either of you guys pick up the torch and take responsibility for this platform?

I should probably qualify this now... Sadly, this is not in a vaccuum. MinGW and MinGW-64 changes could affect down level MinGW versions, Windows and Cygwin. So testing should include something for those platforms, too.

I can test the changes under the older platforms, like Windows XP, Windows Vista, Visual Studio 2005 and Visual Studio 2008.

noloader commented 9 years ago

@Zireael-N, @IlyaBizyaev, @Liberus

Change:

if (!STDC_WANT_SECURE_LIB && !defined(_MEMORY_S_DEFINED))

with:

if (!STDC_WANT_SECURE_LIB__ && !defined(_MEMORY_S_DEFINED) && !defined(MINGW64__))

I believe the library provides memcpy_s and memmove_s for platforms like Linux. I believe the Windows-compatibles are supposed to use Microsoft's implementation.

I think the change is consistent with what's supposed to happen. Does anyone see any problems with it?

If there are no objections, then can someone test it and perform a merge request?

Zireael-N commented 9 years ago

sh: ./cryptest.sh: /bin/bash: bad interpreter: No such file or directory

I don't understand whether it says it can't find /bin/bash or ./cryptest.sh.

But the repository certainly doesn't have the script: 2015-11-05_07-39-35

I'm experiencing this when trying to build tests:

g++ -o cryptest.exe -DNDEBUG -g2 -O3 -fPIC -march=native -Wall -Wextra -Wno-type-limits -Wno-unknown-pragmas -pipe bench.o bench2.o test.o validat0.o validat1.o validat2.o validat3.o adhoc.o datatest.o regtest.o fipsalgt.o dlltest.o ./libcryptopp.a  -lws2_32
./libcryptopp.a(winpipes.o): In function `ZN8CryptoPP19WindowsPipeReceiver16GetReceiveResultEv':
C:\GitHub\cryptopp/winpipes.cpp:131: undefined reference to `non-virtual thunk to CryptoPP::WindowsPipeSource::GetHandle() const'
C:\GitHub\cryptopp/winpipes.cpp:131: undefined reference to `non-virtual thunk to CryptoPP::WindowsPipeSource::GetHandle() const'
./libcryptopp.a(winpipes.o): In function `ZN8CryptoPP19WindowsPipeReceiver7ReceiveEPhj':
C:\GitHub\cryptopp/winpipes.cpp:94: undefined reference to `non-virtual thunk to CryptoPP::WindowsPipeSource::GetHandle() const'
C:\GitHub\cryptopp/winpipes.cpp:94: undefined reference to `non-virtual thunk to CryptoPP::WindowsPipeSource::GetHandle() const'
collect2.exe: error: ld returned 1 exit status

Was getting warning: -fPIC ignored for target (all code is position independent) as well for every file.

I'll try different compilation options to see if it can be fixed.

And about __MINGW64__, it doesn't seem to be defined if you use i686 toolchain, to build a 32-bit version. Though it still uses the same library that does have memcpy_s implemented.

There's a diff in default defines between MinGW and MinGW-w64 with i686 toolchain. The ones that are worth looking at:

+#define __SIZEOF_FLOAT80__ 12
+#define __SIZEOF_FLOAT128__ 16
+#define __STDC_UTF_16__ 1
+#define __STDC_UTF_32__ 1
+#define __STDC_VERSION__ 201112L
+#define __GNUC_STDC_INLINE__ 1
-#define __GNUC_GNU_INLINE__ 1

And the whole list: https://gist.github.com/Zireael-N/15240b72b30796b5184f

Zireael-N commented 9 years ago

Okay, that error had nothing to do with compilation options:

Notice how WindowsPipeReceiver inherits virtual HANDLE GetHandle() const =0 but doesn't define it: https://github.com/weidai11/cryptopp/blob/697ccb452b58a796dabc47736b87ecad5be47e27/winpipes.h#L69

See https://github.com/Zireael-N/cryptopp/commit/f9bf7190cc4821b0d35e6c230ee35008f80e4ea2 ; if it's a correct guess as to what it's supposed to do, I'll make a pull request.

Edit: I found the script on Crypto++'s Wiki. Takes an eternity to run. :(

The results are:

Testing started: Thu,  5 Nov, 2015 08:39:17
Testing finished: Thu,  5 Nov, 2015 09:49:04

19 configurations tested

18 errors detected

15 of them are g++ -shared -o libcryptopp.so ... failing with undefined reference to '_imp__send@16'. Can be easily fixed by appending -lws2_32. For example:

g++ -shared -o libcryptopp.so -DDEBUG -g2 -O2 -fPIC -march=native -Wall -Wextra -Wno-type-limits -Wno-unknown-pragmas -pipe shacal2.o seed.o shark.o zinflate.o gf2n.o socketft.o oaep.o rc2.o default.o wait.o wake.o twofish.o safer.o iterhash.o adler32.o marss.o blowfish.o ecp.o strciphr.o dh2.o ida.o zlib.o elgamal.o algparam.o tea.o rijndael.o eax.o network.o sha.o emsa2.o pkcspad.o squaretb.o idea.o authenc.o hmac.o zdeflate.o xtrcrypt.o simple.o mars.o rc5.o fipstest.o queue.o hrtimer.o vmac.o eprecomp.o winpipes.o polynomi.o dsa.o gzip.o dessp.o files.o base32.o sharkbox.o randpool.o esign.o hex.o sosemanuk.o arc4.o osrng.o skipjack.o gcm.o integer.o xtr.o fips140.o cpu.o filters.o bfinit.o rabin.o gf2_32.o 3way.o rdtables.o rsa.o cbcmac.o tftables.o gost.o md5.o nbtheory.o panama.o sha3.o modes.o casts.o algebra.o cryptlib.o gfpcrypt.o dll.o ec2n.o blumshub.o des.o basecode.o base64.o rc6.o gf256.o mqueue.o misc.o pssr.o channels.o rng.o tiger.o cast.o rw.o square.o asn.o whrlpool.o md4.o dh.o ccm.o md2.o mqv.o tigertab.o crc.o ttmac.o luc.o seal.o salsa.o trdlocal.o pubkey.o camellia.o ripemd.o eccrypto.o serpent.o cmac.o -lws2_32

3 of them are:

g++ -DNDEBUG -g2 -O2 -DCRYPTOPP_MAINTAIN_BACKWARDS_COMPATIBILITY  -fPIC -march=native -Wall -Wextra -Wno-type-limits -Wno-unknown-pragmas -pipe -c elgamal.cpp
elgamal.cpp:1:0: warning: -fPIC ignored for target (all code is position independent)
 // elgamal.cpp - written and placed in the public domain by Wei Dai
 ^
In file included from elgamal.h:7:0,
                 from elgamal.cpp:4:
dsa.h:26:34: error: 'MIN_PRIME_LENGTH' is not a member of 'CryptoPP::DSA {aka CryptoPP::DSA2<CryptoPP::SHA1>}'
 const int MIN_DSA_PRIME_LENGTH = DSA::MIN_PRIME_LENGTH;
                                  ^
dsa.h:27:34: error: 'MAX_PRIME_LENGTH' is not a member of 'CryptoPP::DSA {aka CryptoPP::DSA2<CryptoPP::SHA1>}'
 const int MAX_DSA_PRIME_LENGTH = DSA::MAX_PRIME_LENGTH;
                                  ^
dsa.h:28:39: error: 'PRIME_LENGTH_MULTIPLE' is not a member of 'CryptoPP::DSA {aka CryptoPP::DSA2<CryptoPP::SHA1>}'
 const int DSA_PRIME_LENGTH_MULTIPLE = DSA::PRIME_LENGTH_MULTIPLE;
                                       ^
In file included from elgamal.h:7:0,
                 from elgamal.cpp:4:
dsa.h: In function 'bool CryptoPP::GenerateDSAPrimes(const byte*, size_t, int&, CryptoPP::Integer&, unsigned int, CryptoPP::Integer&)':
dsa.h:31:10: error: 'GeneratePrimes' is not a member of 'CryptoPP::DSA {aka CryptoPP::DSA2<CryptoPP::SHA1>}'
  {return DSA::GeneratePrimes(seed, seedLength, counter, p, primeLength, q);}
          ^
GNUmakefile:529: recipe for target 'elgamal.o' failed

Another edit: I see that you've updated the repo with RC6, now I'm getting this:

rdrand.cpp:435:5: error: #error "Please report on the Crypto++ user group"
 #   error "Please report on the Crypto++ user group"
     ^
GNUmakefile:449: recipe for target 'rdrand.o' failed
IlyaBizyaev commented 9 years ago

Hello, @noloader ! Frankly speaking, I'm not good at troubleshooting, but I'll do my best to help you with that installation problems. As for the MinGW-32, I used the official installer that can be found here. On installation, I also choose MSYS. Don't then forget to specify the path to C:\MinGW\bin folder in your PATH environmental variable. I also usually run this mingwvars.bat script from the C:\MinGW folder, don't know if it helps:

@echo.
@echo Setting up environment for using MinGW with GCC from %~dp0.
@set PATH=%~dp0bin;%PATH%

As for your script, I don't quite understand it. 1) Do you run it from MSYS or from cmd.exe? 2) Why do you use the cd cryptopp command? In my case, cryptopp is located in the root of C: drive, and I use cd /c/cryptopp 3) Finally, does this file really exists? If yes, try explicit sh ./cryptest.sh or bash ./cryptest.sh call. Try to launch other commands like cat, vim etc.

As for your suggestion to take responsibility on the platform, I can only say that if I could fix it myself, I wouldn't have asked for solution here :wink: That's not the area I'm good at (at least for now).

And finally, which alternatives to MinGW would you suggest?

IlyaBizyaev commented 9 years ago

The fixed, proposed by @Zireael-N, works fine, and both libcryptopp and cryptest.exe compile fine under MinGW-w64 (no matter if unalign data access is allowed or not) :smile: . However, tests still fail when CRYPTOPP_NO_UNALIGNED_DATA_ACCESS is defined. I'll propose a PR.

IlyaBizyaev commented 9 years ago

What's interesting is that the Crypto++ that I clone with Git and the one I download in archive from GitHub differ really much! Here is how this line looks in misc.h from the Git repository:

// ************** misc functions ***************

#if (!__STDC_WANT_SECURE_LIB__ && !defined(_MEMORY_S_DEFINED)) && !(defined(__MINGW__) || defined(__MINGW32__))

//! \brief Bounds checking replacement for \p memcpy
//! \param dest pointer to the desination memory block
//! \param sizeInBytes the size of the desination memory block, in bytes
//! \param src pointer to the source memory block
//! \param count the size of the source memory block, in bytes
//! \throws InvalidArgument
//! \details ISO/IEC TR-24772 provides bounds checking interfaces for potentially unsafe functions like \p memcpy, \p strcpy and \p memmove. However, not all standard libraries provides them, like Glibc. The library's \p memcpy_s is a near-drop in replacement. Its only a near-replacement because the library's version throws an \p InvalidArgument on a bounds violation.
//! \note \p memcpy_s will \p assert the pointers \p src and \p dest are not \p NULL in debug builds. Passing \p NULL for either pointer is undefined behavior.

And that's how the one from *.zip looks:

// ************** misc functions ***************

#if (!__STDC_WANT_SECURE_LIB__ && !defined(_MEMORY_S_DEFINED)
inline void memcpy_s(void *dest, size_t sizeInBytes, const void *src, size_t count)
{
    // NULL pointers to memcpy is undefined behavior
    CRYPTOPP_ASSERT(dest); CRYPTOPP_ASSERT(src);

    if (count > sizeInBytes)
        throw InvalidArgument("memcpy_s: buffer overflow");

    memcpy(dest, src, count);
}
noloader commented 9 years ago

What's interesting is that the Crypto++ that I clone with Git and the one I download in archive from GitHub differ really much!

Hmmm.... I'm not sure what GitHub is providing as a ZIP. You should probably abandon it if it does not pass the sniff test.

The ZIP download from the website is OK. You can find it at http://www.cryptopp.com/cryptopp563rc6.zip . I verified http://www.cryptopp.com/cryptopp563rc6.zip provides the changes.

Can you switch to the clone and verify the issues still exist (or no longer exist) with the latest check-in?

Or, can you verify the issues still exist (or no longer exist) with cryptopp563rc6.zip?

noloader commented 9 years ago

@Zireael-N,@IlyaBizyaev

I found the script on Crypto++'s Wiki. Takes an eternity to run. :(

Oh man... Try a Debian Chroot. it takes 6 to 12 hours to verify a change set on, say, S/390 or ARMHF.

15 of them are g++ -shared -o libcryptopp.so ... failing with undefined reference to '_imp__send@16'. Can be easily fixed by appending -lws2_32....

This was because GNUmakefile did not include LDLIBS for the libcryptopp.so recipe. It never included it. See, for example, GNUMakefile for Crypto++ 5.6.2.

The only thing that really changed is we are now testing it. I believe the fix has been checked in.

noloader commented 9 years ago

and both libcryptopp and cryptest.exe compile fine under MinGW-w64 (no matter if unalign data access is allowed or not).

However, tests still fail when CRYPTOPP_NO_UNALIGNED_DATA_ACCESS is defined.

OK, we will need more details. "no matter if unalign data access is allowed or not..." - that's controlled by CRYPTOPP_NO_UNALIGNED_DATA_ACCESS:

$ grep -n CRYPTOPP_NO_UNALIGNED_DATA_ACCESS *.h *.cpp
config.h:37:#ifndef CRYPTOPP_NO_UNALIGNED_DATA_ACCESS
config.h:38:// # define CRYPTOPP_NO_UNALIGNED_DATA_ACCESS
config.h:459:#if !defined(CRYPTOPP_NO_UNALIGNED_DATA_ACCESS) && !defined(CRYPTOPP_ALLOW_UNALIGNED_DATA_ACCESS)

And then:

$ awk 'NR >= 459 && NR <= 464' config.h
#if !defined(CRYPTOPP_NO_UNALIGNED_DATA_ACCESS) && !defined(CRYPTOPP_ALLOW_UNALIGNED_DATA_ACCESS)
#if (CRYPTOPP_BOOL_X64 || CRYPTOPP_BOOL_X86 || CRYPTOPP_BOOL_X32 || defined(__powerpc__) || (__ARM_FEATURE_UNALIGNED >= 1))
    #define CRYPTOPP_ALLOW_UNALIGNED_DATA_ACCESS
#endif
#endif

I'm not sure how it can work one way but not another because everything pivots on CRYPTOPP_ALLOW_UNALIGNED_DATA_ACCESS:

$ grep CRYPTOPP_ALLOW_UNALIGNED_DATA_ACCESS *.h *.cpp | wc -l
      31
IlyaBizyaev commented 9 years ago

The build now looks completely different! That seems to be a GitHub issue... Both aligned and unaligned versions build fine and pass all tests on MinGW-w64. MinGW-32 still experiences problems:

g++ -DNDEBUG -g2 -O2 -march=native -pipe -c cryptlib.cpp
In file included from cryptlib.cpp:19:0:
misc.h: In function 'void CryptoPP::IncrementCounterByOne(byte*, const byte*, un
signed int)':
misc.h:744:35: error: 'memcpy_s' was not declared in this scope
  memcpy_s(output, size, input, i+1);
                                   ^
In file included from filters.h:15:0,
                 from cryptlib.cpp:20:
secblock.h: In instantiation of 'void CryptoPP::SecBlock<T, A>::Assign(const T*,
 CryptoPP::SecBlock<T, A>::size_type) [with T = unsigned char; A = CryptoPP::All
ocatorWithCleanup<unsigned char>; CryptoPP::SecBlock<T, A>::size_type = unsigned
 int]':
algparam.h:49:29:   required from here
secblock.h:532:57: error: 'memcpy_s' was not declared in this scope
    {memcpy_s(m_ptr, m_size*sizeof(T), ptr, len*sizeof(T));}
                                                         ^
secblock.h: In instantiation of 'typename A::pointer CryptoPP::StandardReallocat
e(A&, T*, typename A::size_type, typename A::size_type, bool) [with T = unsigned
 char; A = CryptoPP::AllocatorWithCleanup<unsigned char>; typename A::pointer =
unsigned char*; typename A::size_type = unsigned int]':
secblock.h:220:70:   required from 'CryptoPP::AllocatorWithCleanup<T, T_Align16>
::pointer CryptoPP::AllocatorWithCleanup<T, T_Align16>::reallocate(T*, CryptoPP:
:AllocatorWithCleanup<T, T_Align16>::size_type, CryptoPP::AllocatorWithCleanup<T
, T_Align16>::size_type, bool) [with T = unsigned char; bool T_Align16 = false;
CryptoPP::AllocatorWithCleanup<T, T_Align16>::pointer = unsigned char*; CryptoPP
::AllocatorWithCleanup<T, T_Align16>::size_type = unsigned int]'
secblock.h:629:9:   required from 'void CryptoPP::SecBlock<T, A>::New(CryptoPP::
SecBlock<T, A>::size_type) [with T = unsigned char; A = CryptoPP::AllocatorWithC
leanup<unsigned char>; CryptoPP::SecBlock<T, A>::size_type = unsigned int]'
filters.h:79:30:   required from here
secblock.h:127:77: error: 'memcpy_s' was not declared in this scope
   if (oldPtr && newPointer) {memcpy_s(newPointer, copySize, oldPtr, copySize);}

                                                                             ^
secblock.h: In instantiation of 'CryptoPP::SecBlock<T, A>::SecBlock(const Crypto
PP::SecBlock<T, A>&) [with T = unsigned char; A = CryptoPP::AllocatorWithCleanup
<unsigned char>]':
algparam.h:29:7:   required from 'CryptoPP::AlgorithmParametersTemplate<T>::Algo
rithmParametersTemplate(const char*, const T&, bool) [with T = CryptoPP::ConstBy
teArrayParameter]'
algparam.h:424:104:   required from 'CryptoPP::AlgorithmParameters& CryptoPP::Al
gorithmParameters::operator()(const char*, const T&, bool) [with T = CryptoPP::C
onstByteArrayParameter]'
algparam.h:462:58:   required from 'CryptoPP::AlgorithmParameters CryptoPP::Make
Parameters(const char*, const T&, bool) [with T = CryptoPP::ConstByteArrayParame
ter]'
filters.h:695:81:   required from here
secblock.h:443:79: error: 'memcpy_s' was not declared in this scope
    if (t.m_ptr) {memcpy_s(m_ptr, m_size*sizeof(T), t.m_ptr, t.m_size*sizeof(T))
;}
                                                                               ^
noloader commented 9 years ago

MinGW-32 still experiences problems:

Would you post the output of the following commands? They will dump the preprocessor symbols. Provide them for both MinGW-32 and MinGW-64. Make them available for download somewhere. Or ZIP them and email them to me and Zireael.

# Under MinGW-32
g++ -x c++ -dM -march=native -E - < /dev/null | sort | uniq > mingw-32.txt

# Under MinGW-64
g++ -x c++ -dM -march=native -E - < /dev/null | sort | uniq > mingw-64.txt

This uses the compiler driver to tell us what it is defining. It will invoke cplusplus1, which is the C++ preprocessor.

The MinGW-32 issue is being tracked under Issue 58: Rollup of errata items to be fixed before release.

Zireael-N commented 9 years ago

This uses the compiler driver to tell us what it is defining.

Check the end of this message.

noloader commented 9 years ago

Check the end of this message.

Too much noise in the diff. There's no need for diff tell me there's a different between SIZE_MAX or SIZE_T_MAX on X86 and X64.

Add a cut -d " " -f 2 to pull out the defines.

I am also interested in some of the usual suspects, like __MINGW__, __i386__, __x86_64__ and __LP64__.

Zireael-N commented 9 years ago

I'm not entirely sure where I need to add cut. But here are all the defines: 1) https://gist.github.com/Zireael-N/b78d94f758e4b41580e0 2) https://gist.github.com/Zireael-N/3bbe6c650c4e7d2c04ad

Too much noise in the diff

That's why I mentioned the ones that actually do matter before giving a link to the full list.

X86 and X64

I was actually comparing MinGW-w64 with a 32-bit toolchain vs MinGW. It's a bit confusing but the former can compile both 32 and 64-bit binaries while the latter can only compile 32-bit binaries.

That's why checking for __MINGW64__ was not an ideal solution: the former only defines it when you compile a 64-bit binary.

The former does have memcpy_s implemented no matter what the architecture is, the latter doesn't have it implemented.

noloader commented 9 years ago

That's why checking for MINGW64 was not an ideal solution: the former only defines it when you compile a 64-bit binary.

OK, sounds good. I'm going to leave it to you guys to figure out. Let me know when you are ready for me to check it. (In this case, I'll add it by hand to my stuff, and then test it against the Linux and VS tool chains I have. If all goes well, we'll pull the merge).

The former does have memcpy_s implemented no matter what the architecture is, the latter doesn't have it implemented.

I believe Microsoft provides it all the time. You might consider filing a bug report so it gets fixed. You should probably file it as a security bug because Microsoft has deprecated the dangerous functions in favor of their safer counter parts.

The safer functions are usually a security gate on Microsoft platforms. Microsoft has already made the call: Security Development Lifecycle (SDL) Banned Function Calls. Drepper and friends don't get to make it.

Zireael-N commented 9 years ago

Honestly, I'd just go with:

#if (!__STDC_WANT_SECURE_LIB__ && !defined(_MEMORY_S_DEFINED) && !(defined(__MINGW32__) && __GNUC__ >= 5 || defined(__MINGW64__)))

Reasoning? Original MinGW hasn't been updated for 2 years and GCC used in their latest version is 4.9.2. MinGW-w64, on the other hand, still receives updates and is using GCC 5.2.0.

I believe Microsoft provides it all the time.

Microsoft does. But neither of MinGWs uses their library.

On the bright side, GCC fully supports C++14. Microsoft Visual Studio 2015 doesn't.

IlyaBizyaev commented 9 years ago

Sorry, I can't agree here. I assume I've got the latest MinGW, and it has got the 4.8.1 GCC version, and my MinGW-w64 is 4.9.2 (don't know if therr is newer one).

Zireael-N commented 9 years ago

Well, that condition screws only over people who use an outdated MinGW-w64 to build 32-bit binaries.

If you are:

- you are fine.

IlyaBizyaev commented 9 years ago

And how do I choose between 32 and 64 bit executables?

Zireael-N commented 9 years ago

Read this.

IlyaBizyaev commented 9 years ago

Thanks.

noloader commented 9 years ago

Msys2 provides 3 shortcuts: "MSYS2 Shell", "MinGW-w64 Win32 Shell" and "MinGW-w64 Win64 Shell", you need to use the 2nd to compile 32-bit binaries and the 3rd to compile 64-bit binaries.

As you are working up your patch, I would encourage you to do this in the preprocessor, if possible, If you can't do it in the prerocessor, then you might be able to do it in the makefile.

We were not able to detect MacPorts compilers in the preprocessor, so we had to do it in the makefile (see How to reliably detect a MacPorts-ported compiler?). Search for MACPORTS_COMPILER at GNUmakefile.

noloader commented 9 years ago

@IlyaBizyaev, @Zireael-N -

@Zireael-N - the 64-bit output does not look correct. Can you double check those results?

The 64-bit output claims __SIZEOF_POINTER__ 4, __SIZEOF_SIZE_T__ 4, and __SIZE_MAX__ 0xffffffffU. Something is not passing the sniff test. It almost looks like a WoW (Windows-on-Windows) configuration.

@Zireael-N - Also, can you provide the output of each command for 32, 64 and 32-on-64 configurations?

Zireael-N commented 9 years ago

Because it is not a 64-bit output. It is a MinGW-w64 that's using a toolchain to build 32-bit binaries.

I'll be home in an hour, will provide these + defines of MinGW-w64 using a 64-bit toolchain.

IlyaBizyaev commented 9 years ago

And I'm going to report a couple of problems that occur on build and seem to be connected with this issue.

IlyaBizyaev commented 9 years ago

Yesterday I tried to build my app using the Crypto++ built on MinGW-w64 with CRYPTOPP_NO_UNALIGNED_DATA_ACCESS defined. Here is the output:

||=== Build: Release in Entangle (compiler: GNU GCC Compiler) ===|
C:\cryptopp\misc.h||In function 'void CryptoPP::IncrementCounterByOne(byte*, const byte*, unsigned int)':|
C:\cryptopp\misc.h|744|error: 'memcpy_s' was not declared in this scope|
C:\cryptopp\secblock.h||In instantiation of 'void CryptoPP::SecBlock<T, A>::Assign(const T*, CryptoPP::SecBlock<T, A>::size_type) [with T = unsigned char; A = CryptoPP::AllocatorWithCleanup<unsigned char>; CryptoPP::SecBlock<T, A>::size_type = unsigned int]':|
C:\cryptopp\algparam.h|49|required from here|
C:\cryptopp\secblock.h|532|error: 'memcpy_s' was not declared in this scope|
C:\cryptopp\secblock.h||In instantiation of 'typename A::pointer CryptoPP::StandardReallocate(A&, T*, typename A::size_type, typename A::size_type, bool) [with T = unsigned char; A = CryptoPP::AllocatorWithCleanup<unsigned char>; typename A::pointer = unsigned char*; typename A::size_type = unsigned int]':|
C:\cryptopp\secblock.h|220|required from 'CryptoPP::AllocatorWithCleanup<T, T_Align16>::pointer CryptoPP::AllocatorWithCleanup<T, T_Align16>::reallocate(T*, CryptoPP::AllocatorWithCleanup<T, T_Align16>::size_type, CryptoPP::AllocatorWithCleanup<T, T_Align16>::size_type, bool) [with T = unsigned char; bool T_Align16 = false; CryptoPP::AllocatorWithCleanup<T, T_Align16>::pointer = unsigned char*; CryptoPP::AllocatorWithCleanup<T, T_Align16>::size_type = unsigned int]'|
C:\cryptopp\secblock.h|629|required from 'void CryptoPP::SecBlock<T, A>::New(CryptoPP::SecBlock<T, A>::size_type) [with T = unsigned char; A = CryptoPP::AllocatorWithCleanup<unsigned char>; CryptoPP::SecBlock<T, A>::size_type = unsigned int]'|
C:\cryptopp\filters.h|79|required from here|
C:\cryptopp\secblock.h|127|error: 'memcpy_s' was not declared in this scope|
C:\cryptopp\secblock.h||In instantiation of 'CryptoPP::SecBlock<T, A>::SecBlock(const CryptoPP::SecBlock<T, A>&) [with T = unsigned char; A = CryptoPP::AllocatorWithCleanup<unsigned char>]':|
C:\cryptopp\algparam.h|29|required from 'CryptoPP::AlgorithmParametersTemplate<T>::AlgorithmParametersTemplate(const char*, const T&, bool) [with T = CryptoPP::ConstByteArrayParameter]'|
C:\cryptopp\algparam.h|424|required from 'CryptoPP::AlgorithmParameters& CryptoPP::AlgorithmParameters::operator()(const char*, const T&, bool) [with T = CryptoPP::ConstByteArrayParameter]'|
C:\cryptopp\algparam.h|462|required from 'CryptoPP::AlgorithmParameters CryptoPP::MakeParameters(const char*, const T&, bool) [with T = CryptoPP::ConstByteArrayParameter]'|
C:\cryptopp\filters.h|695|required from here|
C:\cryptopp\secblock.h|443|error: 'memcpy_s' was not declared in this scope|
||=== Build failed: 4 error(s), 12 warning(s) (1 minute(s), 43 second(s)) ===|

So, GCC claims not to find memcpy_s. @Zireael-N , do you think that fix helps?

noloader commented 9 years ago

And I'm going to report a couple of problems that occur on build and seem to be connected with this issue.

If you do, then please open separate reports with enough information to reproduce the results. Also see Bug Report on the Crypto++ wiki.

Zireael-N commented 9 years ago

I'll start this message with a solution, so you don't miss it:

Check if g++ -dumpmachine contains w64-mingw32 and if g++ --version is 5.2.0 or newer. If both are true, add -D_MEMORY_S_DEFINED to CXXFLAGS.

So I was a bit confused. Turned out what I thought to be original MinGW (the compiler shipped with Qt installer on Windows) was an outdated MinGW-w64 with a 32-bit toolchain.

So I downloaded it from the official website. It does not have memcpy_s as well. And it's using GCC 4.8.1.

Defines:

The ones I linked previously as well:

g++ -dumpmachine:

g++ --version:

g++.exe (GCC) 4.8.1
Copyright (C) 2013 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
g++.exe (Rev4, Built by MSYS2 project) 5.2.0
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
g++.exe (Rev4, Built by MSYS2 project) 5.2.0
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
g++.exe (i686-posix-dwarf-rev1, Built by MinGW-W64 project) 4.9.2
Copyright (C) 2014 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

And here I'm thinking why they didn't give their fork a new name so it'd be less confusing.

Zireael-N commented 9 years ago

Another solution: instead of relying on defines, you can use a template trick to find if memcpy_s exists:

#include <iostream>

void function(const int&) {
        std::cout << "function() exists" << std::endl;
}

template <typename T>
void function(const T&);

template <>
void function<int>(const int&) {
        std::cout << "function() didn't exist, thus a template was made" << std::endl;
}

int main() {
        function(10);
        return 0;
}

Try commenting out non-template function, you'll see how it works.

noloader commented 9 years ago

@IlyaBizyaev, @Zireael-N,

I have to admit I'm not really sure what to do.

It seems like (to me) this is an ILP-32 data model (https://en.wikipedia.org/wiki/64-bit_computing#64-bit_data_models), and the MinGW folks should probably be defining __ILP32__ in the 32-bit on 64-bit configurations. That's how we detect it on other platforms, like Debian's X32.

We seem to be having basic problems, and that means we can't determine a course of action. For example, we can't seem to determine the scope of the problem, so we won't be able to place test cases for any mitigations we come up with. And the changes that have been proposed are breaking existing use cases.

I don't like the lack of impact information, and I don't like breaking known good configurations. I think the course of action is to do nothing and document the problems:

We can probably add a define like the following in config.h (this is what we would use if we can detect the differences in the Makefile; the Makefile would set it):

// Define this if you need the library's memcpy_s and memmove_s
// #define CRYPTOPP_WANT_SECURE_LIB 1

Then, in config.h:

#if (!__STDC_WANT_SECURE_LIB__ && !defined(_MEMORY_S_DEFINED)) || (CRYPTOPP_WANT_SECURE_LIB >= 1)
    ...
#endif

MinGW-32 and MinGW-64 users will continue to work as expected. MinGW-32-on-64 users can then perform the following and everything will just work.

export CXXFLAGS="-DNDEBUG -g2 -O2 -DCRYPTOPP_WANT_SECURE_LIB=1"
make
Zireael-N commented 9 years ago

Well, the problem was that MinGW-w64 has these functions but doesn't define _MEMORY_S_DEFINED.

So you'd do #define CRYPTOPP_WANT_SECURE_LIB 1 by default and use && instead and tell MinGW-w64 users to set it to 0.

Or just tell them to use export CXXFLAGS="-D_MEMORY_S_DEFINED ..." seeing that it's already there.

Is there any reason not to try this: https://gist.github.com/Zireael-N/1628ccc910d1741c461d

noloader commented 9 years ago

Is there any reason not to try this: https://gist.github.com/Zireael-N/1628ccc910d1741c461d

This is a maintenance release (i.e., 5.6.2 → 5.6.3). It can't tolerate the symbol change due to versioning requirements. We could probably safely add symbols, but changing and deleting them is a NO-NO. We have to wait for a major version release to change or delete symbols. Also see Release Versioning

noloader commented 9 years ago

@IlyaBizyaev, @Zireael-N,

Well, the problem was that MinGW-w64 has these functions but doesn't define _MEMORY_S_DEFINED.

If that's the only problem, then we might be able to side step it with a using statement.

Can you try a using CryptoPP::memcpy_s? Maybe something like:

#if defined(__MINGW__)
    using CryptoPP::memcpy_s;
    using std::memcpy_s;
#endif
Zireael-N commented 9 years ago

Yeah, that did the job. Added:

#if defined(__MINGW32__)
    using CryptoPP::memcpy_s;
#endif

before:

memcpy_s(&w, sizeof(w), "\x01\x02\x03\x04", 4);

in validat1.cpp.

IlyaBizyaev commented 9 years ago

Excuse me if I'm wrong, but that fixes just the test executable, and not the library itself. Maybe this code should be used in misc.h?

noloader commented 9 years ago

@IlyaBizyaev, @Zireael-N,

Yeah, that did the job. 

      #if defined(__MINGW32__)
        using CryptoPP::memcpy_s;
      #endif

before:

      memcpy_s(&w, sizeof(w), "\x01\x02\x03\x04", 4);

Perfect. Let's wait to hear back from Ilya.

If all goes well, then I'll close it. Hallelujah.