Skip to content

Commit

Permalink
Replace all crypto libraries with Botan
Browse files Browse the repository at this point in the history
Selected the [Botan crypto library](https://github.com/randombit/botan) due to its feature list, maintainer support, availability across all deployment platforms, and ease of use. Also evaluated Crypto++ as a viable candidate, but the additional features of Botan (PKCS#11, TPM, etc) won out.

The random number generator received a backend upgrade. Botan prefers hardware-based RNG's and will provide one if available. This is transparent to KeePassXC and a significant improvement over gcrypt.

Replaced Argon2 library with built-in Botan implementation that supports i, d, and id. This requires Botan 2.11.0 or higher. Also simplified the parameter test across KDF's.

Aligned SymmetricCipher parameters with available modes. All encrypt and decrypt operations are done in-place instead of returning new objects. This allows use of secure vectors in the future with no additional overhead.

Took this opportunity to decouple KeeShare from SSH Agent. Removed leftover code from OpenSSHKey and consolidated the SSH Agent code into the same directory. Removed bcrypt and blowfish inserts since they are provided by Botan.

Additionally simplified KeeShare settings interface by removing raw certificate byte data from the user interface. KeeShare will be further refactored in a future PR.

NOTE: This PR breaks backwards compatibility with KeeShare certificates due to different RSA key storage with Botan. As a result, new "own" certificates will need to be generated and trust re-established.

Removed YKChallengeResponseKeyCLI in favor of just using the original implementation with signal/slots.

Removed TestRandom stub since it was just faking random numbers and not actually using the backend. TestRandomGenerator now uses the actual RNG.

Greatly simplified Secret Service plugin's use of crypto functions with Botan.
  • Loading branch information
droidmonkey committed Mar 2, 2021
1 parent 6c7b04f commit ba4b4ea
Show file tree
Hide file tree
Showing 117 changed files with 1,567 additions and 4,467 deletions.
32 changes: 14 additions & 18 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,6 @@ if(WITH_XC_ALL)
endif()
endif()

if(WITH_XC_SSHAGENT OR WITH_XC_KEESHARE)
set(WITH_XC_CRYPTO_SSH ON)
else()
set(WITH_XC_CRYPTO_SSH OFF)
endif()

# Prefer WITH_XC_NETWORKING setting over WITH_XC_UPDATECHECK
if(NOT WITH_XC_NETWORKING AND WITH_XC_UPDATECHECK)
message(STATUS "Disabling WITH_XC_UPDATECHECK because WITH_XC_NETWORKING is disabled")
Expand Down Expand Up @@ -250,6 +244,7 @@ if(WITH_APP_BUNDLE)
endif()

add_gcc_compiler_flags("-fno-common")
check_add_gcc_compiler_flag("-fopenmp")
add_gcc_compiler_flags("-Wall -Wextra -Wundef -Wpointer-arith -Wno-long-long")
add_gcc_compiler_flags("-Wformat=2 -Wmissing-format-attribute")
add_gcc_compiler_flags("-fvisibility=hidden")
Expand All @@ -269,7 +264,6 @@ else()
endif()
endif()

add_gcc_compiler_cxxflags("-fno-exceptions -fno-rtti")
add_gcc_compiler_cxxflags("-Wnon-virtual-dtor -Wold-style-cast -Woverloaded-virtual")
add_gcc_compiler_cflags("-Wchar-subscripts -Wwrite-strings")

Expand Down Expand Up @@ -324,7 +318,7 @@ if(APPLE AND CMAKE_COMPILER_IS_CLANGXX)
endif()

if(WITH_DEV_BUILD)
add_definitions(-DQT_DEPRECATED_WARNINGS -DGCRYPT_NO_DEPRECATED)
add_definitions(-DQT_DEPRECATED_WARNINGS)
else()
add_definitions(-DQT_NO_DEPRECATED_WARNINGS)
add_gcc_compiler_cxxflags("-Wno-deprecated-declarations")
Expand Down Expand Up @@ -447,20 +441,22 @@ endif()
# Make sure we don't enable asserts there.
set_property(DIRECTORY APPEND PROPERTY COMPILE_DEFINITIONS_NONE QT_NO_DEBUG)

find_package(LibGPGError REQUIRED)
find_package(Gcrypt 1.7.0 REQUIRED)
find_package(Argon2 REQUIRED)
find_package(ZLIB REQUIRED)
find_package(QREncode REQUIRED)
find_package(sodium 1.0.12 REQUIRED)

set(CMAKE_REQUIRED_INCLUDES ${ZLIB_INCLUDE_DIR})
# Find Botan2
find_package(Botan2 REQUIRED)
if(BOTAN2_VERSION VERSION_LESS "2.11.0")
message(FATAL_ERROR "Botan2 2.11.0 or higher is required")
endif()
include_directories(SYSTEM ${BOTAN2_INCLUDE_DIR})

#Find zlib
find_package(ZLIB REQUIRED)
if(ZLIB_VERSION_STRING VERSION_LESS "1.2.0")
message(FATAL_ERROR "zlib 1.2.0 or higher is required to use the gzip format")
endif()
include_directories(SYSTEM ${ZLIB_INCLUDE_DIR})

include_directories(SYSTEM ${ARGON2_INCLUDE_DIR} ${sodium_INCLUDE_DIR})
# QREncode required for TOTP
find_package(QREncode REQUIRED)

# Optional
if(WITH_XC_YUBIKEY)
Expand Down Expand Up @@ -499,7 +495,7 @@ if(UNIX)
endif()
endif()

include_directories(SYSTEM ${GCRYPT_INCLUDE_DIR} ${ZLIB_INCLUDE_DIR})
include_directories(SYSTEM ${ZLIB_INCLUDE_DIR})

add_subdirectory(src)
add_subdirectory(share)
Expand Down
4 changes: 4 additions & 0 deletions COPYING
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ Files: cmake/FindYubiKey.cmake
Copyright: 2014 Kyle Manna <[email protected]>
License: GPL-2 or GPL-3

Files: cmake/FindBotan2.cmake
Copyright: 2018 Ribose Inc.
License: BSD-2-clause

Files: cmake/GenerateProductVersion.cmake
Copyright: 2015 halex2005 <[email protected]>
License: MIT
Expand Down
121 changes: 121 additions & 0 deletions cmake/FindBotan2.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
# Copyright (c) 2018 Ribose Inc.
# All rights reserved.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions
# are met:
# 1. Redistributions of source code must retain the above copyright
# notice, this list of conditions and the following disclaimer.
# 2. Redistributions in binary form must reproduce the above copyright
# notice, this list of conditions and the following disclaimer in the
# documentation and/or other materials provided with the distribution.
#
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
# ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
# TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
# PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS
# BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
# POSSIBILITY OF SUCH DAMAGE.

#.rst:
# FindBotan2
# -----------
#
# Find the botan-2 library.
#
# IMPORTED Targets
# ^^^^^^^^^^^^^^^^
#
# This module defines :prop_tgt:`IMPORTED` targets:
#
# ``Botan2::Botan2``
# The botan-2 library, if found.
#
# Result variables
# ^^^^^^^^^^^^^^^^
#
# This module defines the following variables:
#
# ::
#
# BOTAN2_FOUND - true if the headers and library were found
# BOTAN2_INCLUDE_DIRS - where to find headers
# BOTAN2_LIBRARIES - list of libraries to link
# BOTAN2_VERSION - library version that was found, if any

# use pkg-config to get the directories and then use these values
# in the find_path() and find_library() calls
find_package(PkgConfig QUIET)
pkg_check_modules(PC_BOTAN2 QUIET botan-2)

# find the headers
find_path(BOTAN2_INCLUDE_DIR
NAMES botan/version.h
HINTS
${PC_BOTAN2_INCLUDEDIR}
${PC_BOTAN2_INCLUDE_DIRS}
PATH_SUFFIXES botan-2
)

# find the library
find_library(BOTAN2_LIBRARY
NAMES botan-2 libbotan-2
HINTS
${PC_BOTAN2_LIBDIR}
${PC_BOTAN2_LIBRARY_DIRS}
)

# determine the version
if(PC_BOTAN2_VERSION)
set(BOTAN2_VERSION ${PC_BOTAN2_VERSION})
elseif(BOTAN2_INCLUDE_DIR AND EXISTS "${BOTAN2_INCLUDE_DIR}/botan/build.h")
file(STRINGS "${BOTAN2_INCLUDE_DIR}/botan/build.h" botan2_version_str
REGEX "^#define[\t ]+(BOTAN_VERSION_[A-Z]+)[\t ]+[0-9]+")

string(REGEX REPLACE ".*#define[\t ]+BOTAN_VERSION_MAJOR[\t ]+([0-9]+).*"
"\\1" _botan2_version_major "${botan2_version_str}")
string(REGEX REPLACE ".*#define[\t ]+BOTAN_VERSION_MINOR[\t ]+([0-9]+).*"
"\\1" _botan2_version_minor "${botan2_version_str}")
string(REGEX REPLACE ".*#define[\t ]+BOTAN_VERSION_PATCH[\t ]+([0-9]+).*"
"\\1" _botan2_version_patch "${botan2_version_str}")
set(BOTAN2_VERSION "${_botan2_version_major}.${_botan2_version_minor}.${_botan2_version_patch}"
CACHE INTERNAL "The version of Botan which was detected")
endif()

include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(Botan2
REQUIRED_VARS BOTAN2_LIBRARY BOTAN2_INCLUDE_DIR
VERSION_VAR BOTAN2_VERSION
)

if (BOTAN2_FOUND)
set(BOTAN2_INCLUDE_DIRS ${BOTAN2_INCLUDE_DIR} ${PC_BOTAN2_INCLUDE_DIRS})
set(BOTAN2_LIBRARIES ${BOTAN2_LIBRARY})
endif()

if (BOTAN2_FOUND AND NOT TARGET Botan2::Botan2)
# create the new library target
add_library(Botan2::Botan2 UNKNOWN IMPORTED)
# set the required include dirs for the target
if (BOTAN2_INCLUDE_DIRS)
set_target_properties(Botan2::Botan2
PROPERTIES
INTERFACE_INCLUDE_DIRECTORIES "${BOTAN2_INCLUDE_DIRS}"
)
endif()
# set the required libraries for the target
if (EXISTS "${BOTAN2_LIBRARY}")
set_target_properties(Botan2::Botan2
PROPERTIES
IMPORTED_LINK_INTERFACE_LANGUAGES "C"
IMPORTED_LOCATION "${BOTAN2_LIBRARY}"
)
endif()
endif()

mark_as_advanced(BOTAN2_INCLUDE_DIR BOTAN2_LIBRARY)
2 changes: 1 addition & 1 deletion share/translations/keepassx_en_US.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7477,7 +7477,7 @@ Kernel: %3 %4</translation>
</message>
<message>
<source>Do you want to trust %1 with the fingerprint of %2 from %3?</source>
<translation>Do you want to trust %1 with the fingerprint of %2 from %3? {1 ?} {2 ?}</translation>
<translation>Do you want to trust %1 with the fingerprint of %2 from %3?</translation>
</message>
<message>
<source>Not this time</source>
Expand Down
13 changes: 2 additions & 11 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ set(keepassx_SOURCES
crypto/CryptoHash.cpp
crypto/Random.cpp
crypto/SymmetricCipher.cpp
crypto/SymmetricCipherGcrypt.cpp
crypto/kdf/Kdf.cpp
crypto/kdf/AesKdf.cpp
crypto/kdf/Argon2Kdf.cpp
Expand Down Expand Up @@ -182,7 +181,6 @@ set(keepassx_SOURCES
keys/FileKey.cpp
keys/PasswordKey.cpp
keys/YkChallengeResponseKey.cpp
keys/YkChallengeResponseKeyCLI.cpp
streams/HashedBlockStream.cpp
streams/HmacBlockStream.cpp
streams/LayeredStream.cpp
Expand Down Expand Up @@ -247,11 +245,6 @@ add_subdirectory(cli)
add_subdirectory(qrcode)
set(qrcode_LIB qrcode)

add_subdirectory(crypto/ssh)
if(WITH_XC_CRYPTO_SSH)
set(crypto_ssh_LIB crypto_ssh)
endif()

add_subdirectory(keeshare)
if(WITH_XC_KEESHARE)
set(keeshare_LIB keeshare)
Expand Down Expand Up @@ -322,10 +315,9 @@ target_link_libraries(keepassx_core
Qt5::Concurrent
Qt5::Network
Qt5::Widgets
${sodium_LIBRARY_RELEASE}
${BOTAN2_LIBRARIES}
${YUBIKEY_LIBRARIES}
${ZXCVBN_LIBRARIES}
${ARGON2_LIBRARIES}
${ZLIB_LIBRARIES}
)

Expand Down Expand Up @@ -370,7 +362,6 @@ if(MINGW)
endif()

add_executable(${PROGNAME} WIN32 ${keepassx_SOURCES_MAINEXE} ${WIN32_ProductVersionFiles})
target_link_libraries(keepassx_core ${GCRYPT_LIBRARIES} ${GPGERROR_LIBRARIES})
target_link_libraries(${PROGNAME} keepassx_core)

set_target_properties(${PROGNAME} PROPERTIES ENABLE_EXPORTS ON)
Expand Down Expand Up @@ -497,7 +488,7 @@ if(MINGW)

find_file(CRYPTO_DLL NAMES libcrypto-1_1.dll libcrypto-1_1-x64.dll)
if (NOT CRYPTO_DLL)
message(FATAL_ERROR "Cannot find libcrypto dll, ensure libgcrypt is properly installed.")
message(FATAL_ERROR "Cannot find libcrypto dll, ensure openssl is properly installed.")
endif()

install(FILES ${OPENSSL_DLL} ${CRYPTO_DLL} DESTINATION ".")
Expand Down
8 changes: 4 additions & 4 deletions src/browser/BrowserAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@

#include <QJsonDocument>
#include <QJsonParseError>
#include <sodium.h>
#include <sodium/crypto_box.h>
#include <sodium/randombytes.h>
#include <botan/sodium.h>

using namespace Botan::Sodium;

namespace
{
Expand Down Expand Up @@ -283,7 +283,7 @@ QJsonObject BrowserAction::handleGetLogins(const QJsonObject& json, const QStrin
const QString id = decrypted.value("id").toString();
const QString formUrl = decrypted.value("submitUrl").toString();
const QString auth = decrypted.value("httpAuth").toString();
const bool httpAuth = auth.compare(TRUE_STR, Qt::CaseSensitive) == 0 ? true : false;
const bool httpAuth = auth.compare(TRUE_STR, Qt::CaseSensitive) == 0;
const QJsonArray users = browserService()->findMatchingEntries(id, siteUrl, formUrl, "", keyList, httpAuth);

if (users.isEmpty()) {
Expand Down
6 changes: 0 additions & 6 deletions src/browser/BrowserHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
#include <QMutexLocker>
#include <QtNetwork>

#include "sodium.h"
#include <iostream>

#ifdef Q_OS_WIN
Expand Down Expand Up @@ -53,11 +52,6 @@ BrowserHost::~BrowserHost()

void BrowserHost::start()
{
if (sodium_init() == -1) {
qWarning() << "Failed to start browser service: libsodium failed to initialize!";
return;
}

if (!m_localServer->isListening()) {
m_localServer->listen(BrowserShared::localServerPath());
}
Expand Down
2 changes: 1 addition & 1 deletion src/browser/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,5 @@ if(WITH_XC_BROWSER)
Variant.cpp)

add_library(keepassxcbrowser STATIC ${keepassxcbrowser_SOURCES})
target_link_libraries(keepassxcbrowser Qt5::Core Qt5::Concurrent Qt5::Widgets Qt5::Network ${sodium_LIBRARY_RELEASE})
target_link_libraries(keepassxcbrowser Qt5::Core Qt5::Concurrent Qt5::Widgets Qt5::Network ${BOTAN2_LIBRARIES})
endif()
8 changes: 1 addition & 7 deletions src/cli/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,7 @@ add_executable(keepassxc-cli keepassxc-cli.cpp)
target_link_libraries(keepassxc-cli
${GPGERROR_LIBRARIES}
cli
keepassx_core
Qt5::Core
${GCRYPT_LIBRARIES}
${sodium_LIBRARY_RELEASE}
${ARGON2_LIBRARIES}
${ZLIB_LIBRARIES}
${ZXCVBN_LIBRARIES})
keepassx_core)

install(TARGETS keepassxc-cli
BUNDLE DESTINATION . COMPONENT Runtime
Expand Down
11 changes: 8 additions & 3 deletions src/cli/Utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include "Utils.h"

#ifdef WITH_XC_YUBIKEY
#include "keys/YkChallengeResponseKeyCLI.h"
#include "keys/YkChallengeResponseKey.h"
#endif

#ifdef Q_OS_WIN
Expand Down Expand Up @@ -165,9 +165,14 @@ namespace Utils
}
}

auto key = QSharedPointer<YkChallengeResponseKeyCLI>(new YkChallengeResponseKeyCLI(
{serial, slot}, QObject::tr("Please touch the button on your YubiKey to continue…"), err));
auto conn = QObject::connect(YubiKey::instance(), &YubiKey::userInteractionRequest, [&] {
err << QObject::tr("Please touch the button on your YubiKey to continue…") << "\n\n" << flush;
});

auto key = QSharedPointer<YkChallengeResponseKey>(new YkChallengeResponseKey({serial, slot}));
compositeKey->addChallengeResponseKey(key);

QObject::disconnect(conn);
}
#else
Q_UNUSED(yubiKeySlot);
Expand Down
4 changes: 2 additions & 2 deletions src/core/Alloc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
*/

#include <QtGlobal>
#include <botan/mem_ops.h>
#include <cstdint>
#include <cstdlib>
#include <sodium.h>
#if defined(Q_OS_MACOS)
#include <malloc/malloc.h>
#elif defined(Q_OS_FREEBSD)
Expand All @@ -43,7 +43,7 @@ void operator delete(void* ptr, std::size_t size) noexcept
return;
}

sodium_memzero(ptr, size);
Botan::secure_scrub_memory(ptr, size);
std::free(ptr);
}

Expand Down
Loading

0 comments on commit ba4b4ea

Please sign in to comment.