Skip to content

Commit

Permalink
KeeShare: Remove checking signed container
Browse files Browse the repository at this point in the history
* Remove QuaZip dependency in favor of minizip
* Remove signature checks, but maintain signatures for backwards compatibility
* Remove UI components related to certificates except for personal certificate for backwards compatibility
* Default to unsigned containers (*.kdbx)
  • Loading branch information
droidmonkey committed Dec 15, 2021
1 parent c88d8c8 commit 12990e5
Show file tree
Hide file tree
Showing 28 changed files with 278 additions and 1,645 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ option(WITH_XC_NETWORKING "Include networking code (e.g. for downloading website
option(WITH_XC_BROWSER "Include browser integration with keepassxc-browser." OFF)
option(WITH_XC_YUBIKEY "Include YubiKey support." OFF)
option(WITH_XC_SSHAGENT "Include SSH agent support." OFF)
option(WITH_XC_KEESHARE "Sharing integration with KeeShare (requires quazip5 for secure containers)" OFF)
option(WITH_XC_KEESHARE "Sharing integration with KeeShare" OFF)
option(WITH_XC_UPDATECHECK "Include automatic update checks; disable for controlled distributions" ON)
if(UNIX AND NOT APPLE)
option(WITH_XC_FDOSECRETS "Implement freedesktop.org Secret Storage Spec server side API." OFF)
Expand Down
1 change: 0 additions & 1 deletion INSTALL.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ These steps place the compiled KeePassXC binary inside the `./build/src/` direct
-DWITH_XC_TOUCHID=[ON|OFF] (macOS Only) Enable/Disable Touch ID unlock (default:OFF)
-DWITH_XC_FDOSECRETS=[ON|OFF] (Linux Only) Enable/Disable Freedesktop.org Secrets Service support (default:OFF)
-DWITH_XC_KEESHARE=[ON|OFF] Enable/Disable KeeShare group synchronization extension (default: OFF)
-DWITH_XC_KEESHARE_SECURE=[ON|OFF] Enable/Disable KeeShare signed containers, requires libquazip5 (default: OFF)
-DWITH_XC_ALL=[ON|OFF] Enable/Disable compiling all plugins above (default: OFF)

-DWITH_XC_UPDATECHECK=[ON|OFF] Enable/Disable automatic updating checking (requires WITH_XC_NETWORKING) (default: ON)
Expand Down
9 changes: 9 additions & 0 deletions cmake/FindMinizip.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# MINIZIP_FOUND - Minizip library was found
# MINIZIP_INCLUDE_DIR - Path to Minizip include dir
# MINIZIP_LIBRARIES - List of Minizip libraries

find_library(MINIZIP_LIBRARIES NAMES minizip libminizip)
find_path(MINIZIP_INCLUDE_DIR zip.h PATH_SUFFIXES minizip)

include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(Minizip DEFAULT_MSG MINIZIP_LIBRARIES MINIZIP_INCLUDE_DIR)
24 changes: 0 additions & 24 deletions cmake/FindQuaZip.cmake

This file was deleted.

219 changes: 10 additions & 209 deletions share/translations/keepassxc_en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3083,10 +3083,6 @@ Would you like to correct it?</source>
<source>Inactive</source>
<translation>Inactive</translation>
</message>
<message>
<source>KeeShare unsigned container</source>
<translation>KeeShare unsigned container</translation>
</message>
<message>
<source>KeeShare signed container</source>
<translation>KeeShare signed container</translation>
Expand Down Expand Up @@ -3173,6 +3169,10 @@ Supported extensions are: %1.</source>
<source>Browse…</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>KeeShare container</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>EditGroupWidgetMain</name>
Expand Down Expand Up @@ -6742,18 +6742,6 @@ Kernel: %3 %4</source>
<source>Auto-Type</source>
<translation type="unfinished">Auto-Type</translation>
</message>
<message>
<source>KeeShare (signed and unsigned sharing)</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>KeeShare (only signed sharing)</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>KeeShare (only unsigned sharing)</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>YubiKey</source>
<translation type="unfinished"></translation>
Expand Down Expand Up @@ -7641,6 +7629,10 @@ Please consider generating a new key file.</source>
<source>%1 characters</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>KeeShare</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>QtIOCompressor</name>
Expand Down Expand Up @@ -8243,91 +8235,14 @@ Please consider generating a new key file.</source>
<source>Fingerprint:</source>
<translation>Fingerprint:</translation>
</message>
<message>
<source>Signer</source>
<translation>Signer</translation>
</message>
<message>
<source>Generate</source>
<translation>Generate</translation>
</message>
<message>
<source>Import</source>
<translation>Import</translation>
</message>
<message>
<source>Export</source>
<translation>Export</translation>
</message>
<message>
<source>Imported certificates</source>
<translation>Imported certificates</translation>
</message>
<message>
<source>Trust</source>
<translation>Trust</translation>
</message>
<message>
<source>Ask</source>
<translation>Ask</translation>
</message>
<message>
<source>Untrust</source>
<translation>Untrust</translation>
</message>
<message>
<source>Remove</source>
<translation>Remove</translation>
</message>
<message>
<source>Path</source>
<translation>Path</translation>
</message>
<message>
<source>Status</source>
<translation>Status</translation>
</message>
<message>
<source>Fingerprint</source>
<translation>Fingerprint</translation>
</message>
<message>
<source>Trusted</source>
<translation>Trusted</translation>
</message>
<message>
<source>Untrusted</source>
<translation>Untrusted</translation>
</message>
<message>
<source>Unknown</source>
<translation>Unknown</translation>
</message>
<message>
<source>key.share</source>
<comment>Filetype for KeeShare key</comment>
<translation>key.share</translation>
</message>
<message>
<source>KeeShare key file</source>
<translation>KeeShare key file</translation>
</message>
<message>
<source>All files</source>
<translation>All files</translation>
</message>
<message>
<source>Select path</source>
<translation>Select path</translation>
</message>
<message>
<source>Exporting changed certificate</source>
<translation>Exporting changed certificate</translation>
</message>
<message>
<source>The exported certificate is not the same as the one in use. Do you want to export the current certificate?</source>
<translation>The exported certificate is not the same as the one in use. Do you want to export the current certificate?</translation>
</message>
<message>
<source>Signer:</source>
<translation type="unfinished"></translation>
Expand All @@ -8352,132 +8267,18 @@ Please consider generating a new key file.</source>
<source>Generate new certificate</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Import existing certificate</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Export own certificate</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Known shares</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Trust selected certificate</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Ask whether to trust the selected certificate every time</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Untrust selected certificate</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Remove selected certificate</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>ShareExport</name>
<message>
<source>Overwriting signed share container is not supported - export prevented</source>
<translation type="unfinished">Overwriting signed share container is not supported - export prevented</translation>
</message>
<message>
<source>Could not write export container (%1)</source>
<translation type="unfinished">Could not write export container (%1)</translation>
</message>
<message>
<source>Could not embed signature: Could not open file to write (%1)</source>
<source>Could not write export container.</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Could not embed signature: Could not write file (%1)</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Could not embed database: Could not open file to write (%1)</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Could not embed database: Could not write file (%1)</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Overwriting unsigned share container is not supported - export prevented</source>
<translation type="unfinished">Overwriting unsigned share container is not supported - export prevented</translation>
</message>
<message>
<source>Could not write export container</source>
<translation type="unfinished">Could not write export container</translation>
</message>
</context>
<context>
<name>ShareImport</name>
<message>
<source>Not this time</source>
<translation type="unfinished">Not this time</translation>
</message>
<message>
<source>Never</source>
<translation type="unfinished">Never</translation>
</message>
<message>
<source>Always</source>
<translation type="unfinished">Always</translation>
</message>
<message>
<source>Just this time</source>
<translation type="unfinished">Just this time</translation>
</message>
<message>
<source>Signed share container are not supported - import prevented</source>
<translation type="unfinished">Signed share container are not supported - import prevented</translation>
</message>
<message>
<source>File is not readable</source>
<translation type="unfinished">File is not readable</translation>
</message>
<message>
<source>Invalid sharing container</source>
<translation type="unfinished">Invalid sharing container</translation>
</message>
<message>
<source>Untrusted import prevented</source>
<translation type="unfinished">Untrusted import prevented</translation>
</message>
<message>
<source>Successful signed import</source>
<translation type="unfinished">Successful signed import</translation>
</message>
<message>
<source>Unsigned share container are not supported - import prevented</source>
<translation type="unfinished">Unsigned share container are not supported - import prevented</translation>
</message>
<message>
<source>Successful unsigned import</source>
<translation type="unfinished">Successful unsigned import</translation>
</message>
<message>
<source>File does not exist</source>
<translation type="unfinished">File does not exist</translation>
</message>
<message>
<source>KeeShare Import</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>The source of the shared container cannot be verified because it is not signed. Do you really want to import from %1?</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Do you want to trust %1 with certificate fingerprint:
%2
%3</source>
<source>Successful import</source>
<translation type="unfinished"></translation>
</message>
</context>
Expand Down
3 changes: 1 addition & 2 deletions snap/snapcraft.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ parts:
- -DKEEPASSXC_BUILD_TYPE=Release
- -DWITH_TESTS=OFF
- -DWITH_XC_ALL=ON
- -DWITH_XC_KEESHARE_SECURE=ON
build-packages:
- g++
- libgcrypt20-dev
Expand All @@ -71,7 +70,7 @@ parts:
- libsodium-dev
- libargon2-0-dev
- libqrencode-dev
- libquazip5-dev
- libminizip-dev
- asciidoctor
stage-packages:
- dbus
Expand Down
2 changes: 1 addition & 1 deletion src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ add_feature_info(Auto-Type WITH_XC_AUTOTYPE "Automatic password typing")
add_feature_info(Networking WITH_XC_NETWORKING "Compile KeePassXC with network access code (e.g. for downloading website icons)")
add_feature_info(KeePassXC-Browser WITH_XC_BROWSER "Browser integration with KeePassXC-Browser")
add_feature_info(SSHAgent WITH_XC_SSHAGENT "SSH agent integration compatible with KeeAgent")
add_feature_info(KeeShare WITH_XC_KEESHARE "Sharing integration with KeeShare (requires quazip5 for secure containers)")
add_feature_info(KeeShare WITH_XC_KEESHARE "Sharing integration with KeeShare")
add_feature_info(YubiKey WITH_XC_YUBIKEY "YubiKey HMAC-SHA1 challenge-response")
add_feature_info(UpdateCheck WITH_XC_UPDATECHECK "Automatic update checking")
if(UNIX AND NOT APPLE)
Expand Down
2 changes: 0 additions & 2 deletions src/config-keepassx.h.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
#cmakedefine WITH_XC_YUBIKEY
#cmakedefine WITH_XC_SSHAGENT
#cmakedefine WITH_XC_KEESHARE
#cmakedefine WITH_XC_KEESHARE_INSECURE
#cmakedefine WITH_XC_KEESHARE_SECURE
#cmakedefine WITH_XC_UPDATECHECK
#cmakedefine WITH_XC_TOUCHID
#cmakedefine WITH_XC_FDOSECRETS
Expand Down
8 changes: 2 additions & 6 deletions src/core/Tools.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,8 @@ namespace Tools
#ifdef WITH_XC_SSHAGENT
extensions += "\n- " + QObject::tr("SSH Agent");
#endif
#if defined(WITH_XC_KEESHARE_SECURE) && defined(WITH_XC_KEESHARE_INSECURE)
extensions += "\n- " + QObject::tr("KeeShare (signed and unsigned sharing)");
#elif defined(WITH_XC_KEESHARE_SECURE)
extensions += "\n- " + QObject::tr("KeeShare (only signed sharing)");
#elif defined(WITH_XC_KEESHARE_INSECURE)
extensions += "\n- " + QObject::tr("KeeShare (only unsigned sharing)");
#ifdef WITH_XC_KEESHARE
extensions += "\n- " + QObject::tr("KeeShare");
#endif
#ifdef WITH_XC_YUBIKEY
extensions += "\n- " + QObject::tr("YubiKey");
Expand Down
Loading

3 comments on commit 12990e5

@RealEnder
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, what is the rationale behind removing this feature?
The .share container is encrypted with KDF(password) key, which presumably can be bruteforced. I had an idea to implement a mode, in which KeePassXC will generate separate container for every public key the user has already imported and set trust, additionally encrypted the the given public key. Then during the sync/import process, every user from the team(the idea was to share credentials between team members) will lookup the .share container, for which he's got the private key, decrypt it and continue with the sync/import process. This is rough idea and lacks the public key management part, which can be arranged.
Maybe I'm missing some security feature of KeeShare, which can avoid bruteforcing the .share containers if an attacker gets to them.

@droidmonkey
Copy link
Member Author

@droidmonkey droidmonkey commented on 12990e5 Dec 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A .share container is simply a zip file with a normal kdbx database and a signature file. The signature provides absolutely no additional security, nor were we verifying the signature against any identity provider. This makes signing files completely pointless.

@RealEnder
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I saw and hence the idea to take this further in more useful way.

Please sign in to comment.