Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace libTomCrypt with OpenSSL crypto library in Windows version #30

Open
brodycj opened this issue Jun 8, 2016 · 21 comments
Open

Replace libTomCrypt with OpenSSL crypto library in Windows version #30

brodycj opened this issue Jun 8, 2016 · 21 comments

Comments

@brodycj
Copy link
Collaborator

brodycj commented Jun 8, 2016

SQLCipher was originally designed to use the crypto library from OpenSSL to do the actual encryption but can now use multiple encryption libraries: OpenSSL libcrypto, Apple Security framework, or libTomCrypt. It is desired to replace libTomCrypt with the OpenSSL crypto library built for Windows.

I think it is desired to rebuild the crypto library from scratch instead of relying on a crypto library that may (or may not) be installed. We will never know if different Windows OS installations may have different crypto builds, or no crypto build in certain cases. The SQLCipher project encountered these issues with Android devices in the past.

It is highly recommended to follow how the build is done in the script at: https://github.com/sqlcipher/android-database-sqlcipher/blob/master/build-openssl-libraries.sh

I am not sure whether it would work better to have the OpenSSL crypto build done by the *.vcxproj files provided by this plugin or to invoke the build manually and then ship the build library with this project. I suspect it would be the second.

@brodycj
Copy link
Collaborator Author

brodycj commented Jun 15, 2016

@sgrebnov pointed out that a forked version of OpenSSL [1] supports WinRT/UWP properly. The stock OpenSSL apparently uses some functions that are not supported by Windows 10 (UWP) app store or Windows 8.1 Store Apps.

We only need the "libcrypto" part of OpenSSL. This would both keep the library size smaller and reduce the chance of including object code with possible security vulnerabilities [2].

Specifically I think we only need the following from the OpenSSL crypto part:

  • EVP [3] which redirects to [4]
  • RAND [5]
  • HMAC to a very limited extent [6]
  • PKCS5_PBKDF2_HMAC_SHA1() (I am not sure if it is a macro or a function) [7]

I am aware that I am mixing references from different doc versions but think the point is clear.

I hope that the original OpenSSL project will be fixed to support Windows 8.1, Windows Phone 8.1, and Windows 10 (UWP) builds in the near future.

Unfortunately the SQLCipher project will not provide support for custom OpenSSL builds. For example, a contribution to support 64-bit build [8] will not be integrated [9,10,11,12] until OpenSSL accepts the 64-bit Android build fix in [13]. So we just have to fix this ourselves and hope for convergence with the original OpenSSL and SQLCipher projects to come someday.

[1] https://github.com/Microsoft/openssl/commits/OpenSSL_1_0_2_WinRT-stable
[2] https://www.zetetic.net/blog/2014/4/10/heartbleed-security-statement-for-sqlcipher.html
[3] http://www.openssl.org/docs/crypto/evp.html
[4] https://www.openssl.org/docs/manmaster/crypto/evp.html
[5] https://www.openssl.org/docs/man1.0.1/crypto/rand.html
[6] https://www.openssl.org/docs/man1.0.2/crypto/hmac.html
[7] https://www.openssl.org/docs/manmaster/crypto/PKCS5_PBKDF2_HMAC.html
[8] sqlcipher/android-database-sqlcipher#151
[9] https://discuss.zetetic.net/t/sqlcipher-android-64bit-support/667/4
[10] sqlcipher/android-database-sqlcipher#151 (comment)
[11] sqlcipher/android-database-sqlcipher#151 (comment)
[12] sqlcipher/android-database-sqlcipher#151 (comment)
[13] openssl/openssl#251

@brodycj
Copy link
Collaborator Author

brodycj commented Jun 15, 2016

@sgrebnov I setup the following repos for the OpenSSL adaptations:

@sgrebnov please feel free to make your own repo with the build scripts if you like (ideally with a permissive licensing statement). I will take whatever format you contribute.

@sgrebnov
Copy link
Contributor

sgrebnov commented Jun 16, 2016

Hi @brodybits , if I understand correct the OpenSSL-WinRT-build should be used for everything related how to build OpenSSL for Windows (produce required libs and header files) and the Cordova-sqlcipher-adapter repo will have pre-compiled libs only and documentation section referring to OpenSSL-WinRT-build as the way to update those binaries. Please confirm my understanding is correct.

@brodycj
Copy link
Collaborator Author

brodycj commented Jun 16, 2016

Yes (though I may rename OpenSSL-WinRT-build to openssl-windows-build-scripts or something in the end). And I will be sure that you get credit for the work.

As a detail, I will probably move the pre-compiled libs and other external dependencies to an external project to be included by npm as discussed in apache/cordova-discuss#38 when I incorporate the changes. I don't think this should have any impact on your part of the OpenSSL integration project.

@sgrebnov
Copy link
Contributor

I've tested whether we can use OpenSSL build script for VS2015/UWP to produce lib version for both Windows10/UWP and Windows8.1/Store. As a result Windows8.1 build fails (a few unresolved references) as it is based on different VC++ Platform Toolset (v120 vs v140 required). I plan to see if win81-store lib version will work for both.

@brodycj
Copy link
Collaborator Author

brodycj commented Jun 16, 2016

What unresolved references did you get? It is possible they are things we don't need for SQLCipher.

@sgrebnov
Copy link
Contributor

I've also tested arm build and it seems to work correct, at least SQLite3 project compiles fine. The only small thing here is 'error C4703: potentially uninitialized local pointer variable 'pDbPage' used' errors while compiling sqlite3.c. Temporary disabled using the following directive (to be addressed by separate work item)

#pragma warning ( disable : 4703 )

@sgrebnov
Copy link
Contributor

sgrebnov commented Jun 16, 2016

Here is the list of unresolved references (not investigated yet):

1>libeay32.lib(cryptlib.obj) : error LNK2001: unresolved external symbol __imp____acrt_iob_func
1>libeay32.lib(pem_lib.obj) : error LNK2001: unresolved external symbol __imp____acrt_iob_func
1>libeay32.lib(ui_openssl.obj) : error LNK2001: unresolved external symbol __imp____acrt_iob_func
1>libeay32.lib(cryptlib.obj) : error LNK2001: unresolved external symbol __imp____stdio_common_vfprintf
1>libeay32.lib(pem_lib.obj) : error LNK2001: unresolved external symbol __imp____stdio_common_vfprintf
1>libeay32.lib(ui_openssl.obj) : error LNK2001: unresolved external symbol __imp____stdio_common_vfprintf
1>libeay32.lib(v3_utl.obj) : error LNK2001: unresolved external symbol __imp____stdio_common_vsscanf

@brodycj
Copy link
Collaborator Author

brodycj commented Jun 16, 2016

Hi Sergey,

I did a quick search and these unresolved references seem to be caused by a difference in how VS 2015 and older versions define stdin, stdout, and stderr [1,2]. They give a solution but I think it is for the other way around.

In general I use Visual Studio 2015 Community Edition to test the builds for Windows 8.1/Windows Phone 8.1/Windows 10. I suspect and hope it would be possible to use Visual Studio 2015 (Community Edition) to build the OpenSSL lib(s) for these platforms.

I think it is good to try the win81-store lib build and see if it works for all of these platforms. Though I suspect the build script for VS2015/UWP would be more future-proof if we can solve this.

[1] http://stackoverflow.com/questions/30412951/unresolved-external-symbol-imp-fprintf-and-imp-iob-func-sdl2
[2] http://stackoverflow.com/questions/30450042/unresolved-external-symbol-imp-iob-func-referenced-in-function-openssldie

@brodycj
Copy link
Collaborator Author

brodycj commented Jun 16, 2016

I would also be OK if we use different build scripts to build the Windows 8.1/Windows Phone 8.1 and Windows 10 UWP versions of OpenSSL. Even different build scripts to build the Windows 8.1 & Windows Phone 8.1 versions would be OK.

I added a license statement to the OpenSSL-WInRT-build project with a note that is subject to change. You are more than welcome to select a different license statement. Probably doesn't matter for build scripts:)

@brodycj
Copy link
Collaborator Author

brodycj commented Jun 20, 2016

It has come to my attention that there are some downsides to using a custom version of OpenSSL. Specifically there could be a risk of inferior entropy seeding. I think it would be great if we could find some way to build the needed crypto library from the official OpenSSL library. I can think of the following alternatives:

  1. Build using a Windows GCC using Mingw-w64: http://www.blogcompiler.com/2011/12/21/openssl-for-windows/
  2. Variation of 1 but compile on Linux: http://www.blogcompiler.com/2010/07/11/compile-for-windows-on-linux/
  3. Build OpenSSL library using LLVM toolchain: variation of http://llvm.org/docs/GettingStartedVS.html#an-example-using-the-llvm-tool-chain

I would definitely favor alternative 3.

@sgrebnov
Copy link
Contributor

sgrebnov commented Jun 20, 2016

I think the main goal of using custom Microsoft fork was to fix Windows Submission issue as original OpenSSL compiles fine on Windows. Taking into account there is a risk of inferior entropy seeding in custom fork do we still want to finalize this work based on custom fork (Microsoft)? Is there a simple way/test to verify that the risk exists?

@brodycj
Copy link
Collaborator Author

brodycj commented Jun 20, 2016

Hi @sgrebnov,

I am thinking the best approach would be to start with the official OpenSSL version and make the minimum changes necessary to solve the problems with the Windows store. I would also like to see someone raise these changes as a pull request on the OpenSSL project. I would be happy to do this but in terms of IPR it may be better for the original author or maybe someone else from Microsoft to raise the PR with the changes needed.

What I need most is the build instructions. This is because I still don't understand how to build static (or dynamic) libraries for Windows 8.1, Windows Phone 8.1, and Windows 10.

The other thing you know is that we only need the libcrypto part of OpenSSL (and maybe not the entire libcrypto part). I can also take care of this part.

Thanks!

@sgrebnov
Copy link
Contributor

I worry that we will end up with the same changes as in Microsoft fork (we will just duplicate the work they did). I will see if we can get more details regarding status if their work and whether this is going to be merged/included to the original OpenSSL repo soon.

@brodycj
Copy link
Collaborator Author

brodycj commented Jun 22, 2016

I tried building the OpenSSL libraries using https://github.com/sgrebnov/OpenSSL-WinRT-build and it works with a small change to OpenSSL/ms/setVSvars.bat (I will raise an issue or PR).

I am happy to report that I was able to use the built UWP win32 library in a test project and run the Cordova-sqlcipher-adapter test suite with no errors. In addition: while the full OpenSSL library is 12MB the generated SQLite3 (SQLCipher) DLL is 2.6MB and the WINMD file is 8KB.

A solution may be to build SQLCipher DLL/WINMD separately then fix plugin.xml to just import the right DLL/WINMD into the Cordova app project. @sgrebnov I could use some help or guidance if you have time for this. It would be especially cool to have your build script build these (not sure if it should be in the same project as the OpenSSL-WinRT-build or not).

UPDATE: I just raised microsoft/openssl#31 with the change needed to the OpenSSL ms/setVSvars.bat file.

@brodycj
Copy link
Collaborator Author

brodycj commented Jun 22, 2016

FYI I am now able to build and run the Windows version to pass the test suite with the OpenSSL build in the following build scenarios:

  • Windows 10 (UWP) Debug build: x86 (win32), ARM (on my mobile device), and x64 (TODO: test Release builds)
  • Windows 8.1 Debug build: x86 (win32) (TODO: test x64, Release)
  • Windows Phone 8.1 Debug build on ARM mobile device (TODO: test with Release build)

I pushed the changes (without the OpenSSL builds) to https://github.com/brodybits/Cordova-sqlcipher-adapter-dev/tree/cb-openssl-test1 for backup and safekeeping.

The one issue I can think of is build sizes. I hope we can find a way to just build the libcrypto part.

@brodycj
Copy link
Collaborator Author

brodycj commented Jul 5, 2016

I just raised microsoft/openssl#32 in case they can help us build a smaller crypto library. I will also look at this when I get a chance. This is the blocker.

@brodycj
Copy link
Collaborator Author

brodycj commented Dec 5, 2016

According to microsoft/openssl#32 (comment) the final executable/dll size should be much smaller. To accomplish this I need to get WinMD+DLL support working as discussed in brodycj/cordova-sqlite-legacy#10 (comment) and Apache Cordova CB-12189.

@brodycj
Copy link
Collaborator Author

brodycj commented Dec 5, 2016

According to microsoft/openssl#32 (comment) the final executable/dll size should be much smaller. To accomplish this I need to get WinMD+DLL support working as discussed in brodycj/cordova-sqlite-legacy#10 (comment) and Apache Cordova CB-12189.

/cc @vladimir-kotikov @mbraude ref: brodycj/cordova-sqlite-legacy#10

@slimlime
Copy link

I is sadface 😢. Any suggestions for cross-platform (Web, Angular, Desktop, Mobile) secure storage for this? (Including Windows).

@Kepro
Copy link

Kepro commented May 28, 2020

@brodybits windows are not working totally or only encryption is disabled? :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants