diff --git a/CMakeLists.txt b/CMakeLists.txt index 407a275424..0a84ef18c0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,7 +41,7 @@ option(WITH_ASAN "Enable address sanitizer checks (Linux / macOS only)" OFF) option(WITH_COVERAGE "Use to build with coverage tests (GCC only)." OFF) option(WITH_APP_BUNDLE "Enable Application Bundle for macOS" ON) -set(WITH_XC_ALL OFF CACHE BOOLEAN "Build in all available plugins") +set(WITH_XC_ALL OFF CACHE BOOL "Build in all available plugins") option(WITH_XC_AUTOTYPE "Include Auto-Type." ON) option(WITH_XC_NETWORKING "Include networking code (e.g. for downlading website icons)." OFF) @@ -163,11 +163,15 @@ if("${CMAKE_SIZEOF_VOID_P}" EQUAL "4") set(IS_32BIT TRUE) endif() -if("${CMAKE_C_COMPILER}" MATCHES "clang$" OR "${CMAKE_C_COMPILER_ID}" STREQUAL "Clang") +if("${CMAKE_C_COMPILER}" MATCHES "clang$" + OR "${CMAKE_EXTRA_GENERATOR_C_SYSTEM_DEFINED_MACROS}" MATCHES "__clang__" + OR "${CMAKE_C_COMPILER_ID}" STREQUAL "Clang") set(CMAKE_COMPILER_IS_CLANG 1) endif() -if("${CMAKE_CXX_COMPILER}" MATCHES "clang(\\+\\+)?$" OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") +if("${CMAKE_CXX_COMPILER}" MATCHES "clang(\\+\\+)?$" + OR "${CMAKE_EXTRA_GENERATOR_CXX_SYSTEM_DEFINED_MACROS}" MATCHES "__clang__" + OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") set(CMAKE_COMPILER_IS_CLANGXX 1) endif() @@ -264,6 +268,11 @@ endif() add_gcc_compiler_cflags("-std=c99") add_gcc_compiler_cxxflags("-std=c++11") +if((CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 4.9.99) OR + (CMAKE_COMPILER_IS_CLANGXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 3.6.99)) + add_gcc_compiler_cxxflags("-fsized-deallocation") +endif() + if(APPLE) add_gcc_compiler_cxxflags("-stdlib=libc++") endif() @@ -387,6 +396,7 @@ 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}) @@ -394,7 +404,7 @@ 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 ${ARGON2_INCLUDE_DIR}) +include_directories(SYSTEM ${ARGON2_INCLUDE_DIR} ${sodium_INCLUDE_DIR}) # Optional if(WITH_XC_KEESHARE) diff --git a/INSTALL.md b/INSTALL.md index d3927536fd..3bc9185d9d 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -25,7 +25,7 @@ The following libraries are required: * zlib * libmicrohttpd * libxi, libxtst, qtx11extras (optional for auto-type on X11) -* libsodium (>= 1.0.12, optional for KeePassXC-Browser support) +* libsodium (>= 1.0.12) * libargon2 Prepare the Building Environment diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 9b009b6986..16d0ef89c6 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -27,6 +27,7 @@ if(NOT ZXCVBN_LIBRARIES) endif(NOT ZXCVBN_LIBRARIES) set(keepassx_SOURCES + core/Alloc.cpp core/AutoTypeAssociations.cpp core/AutoTypeMatch.cpp core/Compare.cpp @@ -254,6 +255,8 @@ endif() if(WITH_XC_TOUCHID) list(APPEND keepassx_SOURCES touchid/TouchID.mm) + # TODO: Remove -Wno-error once deprecation warnings have been resolved. + set_source_files_properties(touchid/TouchID.mm PROPERTY COMPILE_FLAGS "-Wno-old-style-cast -Wno-error") endif() add_library(autotype STATIC ${autotype_SOURCES}) @@ -270,6 +273,7 @@ target_link_libraries(keepassx_core Qt5::Concurrent Qt5::Network Qt5::Widgets + ${sodium_LIBRARY_RELEASE} ${YUBIKEY_LIBRARIES} ${ZXCVBN_LIBRARIES} ${ARGON2_LIBRARIES} diff --git a/src/browser/CMakeLists.txt b/src/browser/CMakeLists.txt index 10189d931c..7e813eb5bf 100755 --- a/src/browser/CMakeLists.txt +++ b/src/browser/CMakeLists.txt @@ -16,7 +16,6 @@ if(WITH_XC_BROWSER) include_directories(${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR}) - find_package(sodium 1.0.12 REQUIRED) set(keepassxcbrowser_SOURCES BrowserAccessControlDialog.cpp @@ -33,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) + target_link_libraries(keepassxcbrowser Qt5::Core Qt5::Concurrent Qt5::Widgets Qt5::Network ${sodium_LIBRARY_RELEASE}) endif() diff --git a/src/cli/CMakeLists.txt b/src/cli/CMakeLists.txt index c3f97a2cdc..2f4a7275e3 100644 --- a/src/cli/CMakeLists.txt +++ b/src/cli/CMakeLists.txt @@ -38,6 +38,7 @@ target_link_libraries(keepassxc-cli keepassx_core Qt5::Core ${GCRYPT_LIBRARIES} + ${sodium_LIBRARY_RELEASE} ${ARGON2_LIBRARIES} ${GPGERROR_LIBRARIES} ${ZLIB_LIBRARIES} diff --git a/src/core/Alloc.cpp b/src/core/Alloc.cpp new file mode 100644 index 0000000000..a33b561962 --- /dev/null +++ b/src/core/Alloc.cpp @@ -0,0 +1,89 @@ +/* + * Copyright (C) 2019 KeePassXC Team + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include +#include +#include +#ifdef Q_OS_MACOS +#include +#else +#include +#endif + +#if defined(NDEBUG) && !defined(__cpp_sized_deallocation) +#warning "KeePassXC is being compiled without sized deallocation support. Deletes may be slow." +#endif + +/** + * Custom sized delete operator which securely zeroes out allocated + * memory before freeing it (requires C++14 sized deallocation support). + */ +void operator delete(void* ptr, std::size_t size) noexcept +{ + if (!ptr) { + return; + } + + sodium_memzero(ptr, size); + std::free(ptr); +} + +void operator delete[](void* ptr, std::size_t size) noexcept +{ + ::operator delete(ptr, size); +} + +/** + * Custom delete operator which securely zeroes out + * allocated memory before freeing it. + */ +void operator delete(void* ptr) noexcept +{ + if (!ptr) { + return; + } + +#if defined(Q_OS_WIN) + ::operator delete(ptr, _msize(ptr)); +#elif defined(Q_OS_MACOS) + ::operator delete(ptr, malloc_size(ptr)); +#elif defined(Q_OS_UNIX) + ::operator delete(ptr, malloc_usable_size(ptr)); +#else + // whatever OS this is, give up and simply free stuff + std::free(ptr); +#endif +} + +void operator delete[](void* ptr) noexcept +{ + ::operator delete(ptr); +} + +/** + * Custom insecure delete operator that does not zero out memory before + * freeing a buffer. Can be used for better performance. + */ +void operator delete(void* ptr, bool) noexcept +{ + std::free(ptr); +} + +void operator delete[](void* ptr, bool) noexcept +{ + ::operator delete(ptr, false); +} diff --git a/src/keys/FileKey.cpp b/src/keys/FileKey.cpp index 9d1e8f50fb..da25ef4aef 100644 --- a/src/keys/FileKey.cpp +++ b/src/keys/FileKey.cpp @@ -18,19 +18,35 @@ #include "FileKey.h" -#include - #include "core/Tools.h" #include "crypto/CryptoHash.h" #include "crypto/Random.h" +#include + +#include +#include +#include +#include + QUuid FileKey::UUID("a584cbc4-c9b4-437e-81bb-362ca9709273"); +constexpr int FileKey::SHA256_SIZE; + FileKey::FileKey() : Key(UUID) + , m_key(static_cast(gcry_malloc_secure(SHA256_SIZE))) { } +FileKey::~FileKey() +{ + if (m_key) { + gcry_free(m_key); + m_key = nullptr; + } +} + /** * Read key file from device while trying to detect its file format. * @@ -148,7 +164,10 @@ bool FileKey::load(const QString& fileName, QString* errorMsg) */ QByteArray FileKey::rawKey() const { - return m_key; + if (!m_key) { + return {}; + } + return QByteArray::fromRawData(m_key, SHA256_SIZE); } /** @@ -223,12 +242,15 @@ bool FileKey::loadXml(QIODevice* device) } } + bool ok = false; if (!xmlReader.error() && correctMeta && !data.isEmpty()) { - m_key = data; - return true; + std::memcpy(m_key, data.data(), std::min(SHA256_SIZE, data.size())); + ok = true; } - return false; + sodium_memzero(data.data(), static_cast(data.capacity())); + + return ok; } /** @@ -293,7 +315,8 @@ bool FileKey::loadBinary(QIODevice* device) if (!Tools::readAllFromDevice(device, data) || data.size() != 32) { return false; } else { - m_key = data; + std::memcpy(m_key, data.data(), std::min(SHA256_SIZE, data.size())); + sodium_memzero(data.data(), static_cast(data.capacity())); return true; } } @@ -321,12 +344,15 @@ bool FileKey::loadHex(QIODevice* device) } QByteArray key = QByteArray::fromHex(data); + sodium_memzero(data.data(), static_cast(data.capacity())); if (key.size() != 32) { return false; } - m_key = key; + std::memcpy(m_key, key.data(), std::min(SHA256_SIZE, key.size())); + sodium_memzero(key.data(), static_cast(key.capacity())); + return true; } @@ -348,7 +374,9 @@ bool FileKey::loadHashed(QIODevice* device) cryptoHash.addData(buffer); } while (!buffer.isEmpty()); - m_key = cryptoHash.result(); + auto result = cryptoHash.result(); + std::memcpy(m_key, result.data(), std::min(SHA256_SIZE, result.size())); + sodium_memzero(result.data(), static_cast(result.capacity())); return true; } diff --git a/src/keys/FileKey.h b/src/keys/FileKey.h index d7486467b3..290a04af05 100644 --- a/src/keys/FileKey.h +++ b/src/keys/FileKey.h @@ -40,6 +40,7 @@ class FileKey : public Key }; FileKey(); + ~FileKey() override; bool load(QIODevice* device); bool load(const QString& fileName, QString* errorMsg = nullptr); QByteArray rawKey() const override; @@ -48,6 +49,8 @@ class FileKey : public Key static bool create(const QString& fileName, QString* errorMsg = nullptr, int size = 128); private: + static constexpr int SHA256_SIZE = 32; + bool loadXml(QIODevice* device); bool loadXmlMeta(QXmlStreamReader& xmlReader); QByteArray loadXmlKey(QXmlStreamReader& xmlReader); @@ -55,7 +58,7 @@ class FileKey : public Key bool loadHex(QIODevice* device); bool loadHashed(QIODevice* device); - QByteArray m_key; + char* m_key = nullptr; Type m_type = None; }; diff --git a/src/keys/PasswordKey.cpp b/src/keys/PasswordKey.cpp index 35ecb99897..2d0416af8f 100644 --- a/src/keys/PasswordKey.cpp +++ b/src/keys/PasswordKey.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010 Felix Geyer + * Copyright (C) 2019 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -16,35 +16,51 @@ */ #include "PasswordKey.h" +#include "core/Tools.h" #include "crypto/CryptoHash.h" +#include +#include +#include QUuid PasswordKey::UUID("77e90411-303a-43f2-b773-853b05635ead"); +constexpr int PasswordKey::SHA256_SIZE; + PasswordKey::PasswordKey() : Key(UUID) + , m_key(static_cast(gcry_malloc_secure(SHA256_SIZE))) { } PasswordKey::PasswordKey(const QString& password) : Key(UUID) + , m_key(static_cast(gcry_malloc_secure(SHA256_SIZE))) { setPassword(password); } +PasswordKey::~PasswordKey() +{ + if (m_key) { + gcry_free(m_key); + m_key = nullptr; + } +} + QSharedPointer PasswordKey::fromRawKey(const QByteArray& rawKey) { auto result = QSharedPointer::create(); - result->m_key = rawKey; + std::memcpy(result->m_key, rawKey.data(), std::min(SHA256_SIZE, rawKey.size())); return result; } QByteArray PasswordKey::rawKey() const { - return m_key; + return QByteArray::fromRawData(m_key, SHA256_SIZE); } void PasswordKey::setPassword(const QString& password) { - m_key = CryptoHash::hash(password.toUtf8(), CryptoHash::Sha256); + std::memcpy(m_key, CryptoHash::hash(password.toUtf8(), CryptoHash::Sha256).data(), SHA256_SIZE); } diff --git a/src/keys/PasswordKey.h b/src/keys/PasswordKey.h index 68ab79895e..4408cabcf3 100644 --- a/src/keys/PasswordKey.h +++ b/src/keys/PasswordKey.h @@ -30,13 +30,16 @@ class PasswordKey : public Key PasswordKey(); explicit PasswordKey(const QString& password); + ~PasswordKey() override; QByteArray rawKey() const override; void setPassword(const QString& password); static QSharedPointer fromRawKey(const QByteArray& rawKey); private: - QByteArray m_key; + static constexpr int SHA256_SIZE = 32; + + char* m_key = nullptr; }; #endif // KEEPASSX_PASSWORDKEY_H diff --git a/src/keys/YkChallengeResponseKey.cpp b/src/keys/YkChallengeResponseKey.cpp index f9cbe31747..759d8d1bc8 100644 --- a/src/keys/YkChallengeResponseKey.cpp +++ b/src/keys/YkChallengeResponseKey.cpp @@ -1,6 +1,6 @@ /* + * Copyright (C) 2019 KeePassXC Team * Copyright (C) 2014 Kyle Manna - * Copyright (C) 2017 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -32,6 +32,10 @@ #include #include +#include +#include +#include + QUuid YkChallengeResponseKey::UUID("e092495c-e77d-498b-84a1-05ae0d955508"); YkChallengeResponseKey::YkChallengeResponseKey(int slot, bool blocking) @@ -45,9 +49,18 @@ YkChallengeResponseKey::YkChallengeResponseKey(int slot, bool blocking) } } +YkChallengeResponseKey::~YkChallengeResponseKey() +{ + if (m_key) { + gcry_free(m_key); + m_keySize = 0; + m_key = nullptr; + } +} + QByteArray YkChallengeResponseKey::rawKey() const { - return m_key; + return QByteArray::fromRawData(m_key, static_cast(m_keySize)); } /** @@ -67,14 +80,22 @@ bool YkChallengeResponseKey::challenge(const QByteArray& challenge, unsigned int emit userInteractionRequired(); } + QByteArray key; auto result = AsyncTask::runAndWaitForFuture( - [this, challenge]() { return YubiKey::instance()->challenge(m_slot, true, challenge, m_key); }); + [this, challenge, &key]() { return YubiKey::instance()->challenge(m_slot, true, challenge, key); }); if (m_blocking) { emit userConfirmed(); } if (result == YubiKey::SUCCESS) { + if (m_key) { + gcry_free(m_key); + } + m_keySize = static_cast(key.size()); + m_key = static_cast(gcry_malloc_secure(m_keySize)); + std::memcpy(m_key, key.data(), m_keySize); + sodium_memzero(key.data(), static_cast(key.capacity())); return true; } } while (retries > 0); diff --git a/src/keys/YkChallengeResponseKey.h b/src/keys/YkChallengeResponseKey.h index b8467e7a62..5f7c40e721 100644 --- a/src/keys/YkChallengeResponseKey.h +++ b/src/keys/YkChallengeResponseKey.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2017 KeePassXC Team + * Copyright (C) 2019 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -32,6 +32,7 @@ class YkChallengeResponseKey : public QObject, public ChallengeResponseKey static QUuid UUID; explicit YkChallengeResponseKey(int slot = -1, bool blocking = false); + ~YkChallengeResponseKey() override; QByteArray rawKey() const override; bool challenge(const QByteArray& challenge) override; @@ -52,7 +53,8 @@ class YkChallengeResponseKey : public QObject, public ChallengeResponseKey void userConfirmed(); private: - QByteArray m_key; + char* m_key = nullptr; + std::size_t m_keySize = 0; int m_slot; bool m_blocking; }; diff --git a/src/proxy/CMakeLists.txt b/src/proxy/CMakeLists.txt index ff645dadb4..bdbfa3b740 100755 --- a/src/proxy/CMakeLists.txt +++ b/src/proxy/CMakeLists.txt @@ -18,12 +18,13 @@ if(WITH_XC_BROWSER) include_directories(${BROWSER_SOURCE_DIR}) set(proxy_SOURCES + ../core/Alloc.cpp keepassxc-proxy.cpp ${BROWSER_SOURCE_DIR}/NativeMessagingBase.cpp NativeMessagingHost.cpp) add_library(proxy STATIC ${proxy_SOURCES}) - target_link_libraries(proxy Qt5::Core Qt5::Network) + target_link_libraries(proxy Qt5::Core Qt5::Network ${sodium_LIBRARY_RELEASE}) add_executable(keepassxc-proxy keepassxc-proxy.cpp) target_link_libraries(keepassxc-proxy proxy) diff --git a/src/touchid/TouchID.mm b/src/touchid/TouchID.mm index 9ef72189b2..7df5ad556b 100644 --- a/src/touchid/TouchID.mm +++ b/src/touchid/TouchID.mm @@ -15,6 +15,7 @@ inline void debug(const char* message, ...) { + Q_UNUSED(message); // qWarning(...); } @@ -258,6 +259,7 @@ inline QString hash(const QString& value) NSString* authMessage = msg.toNSString(); // autoreleased [context evaluatePolicy:LAPolicyDeviceOwnerAuthenticationWithBiometrics localizedReason:authMessage reply:^(BOOL success, NSError* error) { + Q_UNUSED(error); result = success ? kTouchIDResultAllowed : kTouchIDResultFailed; CFRunLoopWakeUp(CFRunLoopGetCurrent()); }];