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

KeeShare: Remove checking signed container and QuaZip #7223

Merged
merged 1 commit into from
Dec 15, 2021

Conversation

droidmonkey
Copy link
Member

@droidmonkey droidmonkey commented Dec 13, 2021

  • 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)

Screenshots

This is the original KeeShare settings in 2.6.6 (showing trusted keys generated in 2.7.0 at the bottom):
image

This is the new settings view in 2.7.0:
image

Testing strategy

Tested KeeShare between 2.7.0 and 2.6.6, worked perfectly. Signatures are still recognized and validated in 2.6.6 so backwards compatibility is maintained. In 2.7.0 we no longer care about verifying the signature.

Type of change

  • ✅ Refactor (significant modification to existing code)

@droidmonkey droidmonkey added pr: refactoring Pull request that refactors code feature: KeeShare labels Dec 13, 2021
@droidmonkey droidmonkey added this to the v2.7.0 milestone Dec 13, 2021
@droidmonkey droidmonkey force-pushed the refactor/keeshare-part1 branch 5 times, most recently from 5a00683 to 77a8230 Compare December 14, 2021 00:36
Copy link
Member

@phoerious phoerious left a comment

Choose a reason for hiding this comment

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

No major objections. Great net code reduction.

src/keeshare/ShareExport.cpp Outdated Show resolved Hide resolved
@droidmonkey
Copy link
Member Author

Need to fix the TestSharing build to include the core library.

* 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)
@droidmonkey
Copy link
Member Author

I ended up reverting the BinaryStream file move and used a QDataStream to write the ssh-rsa key signature file.

@droidmonkey droidmonkey force-pushed the refactor/keeshare-part1 branch from 77a8230 to dc39b31 Compare December 15, 2021 02:57
@codecov-commenter
Copy link

codecov-commenter commented Dec 15, 2021

Codecov Report

Merging #7223 (dc39b31) into develop (4567265) will increase coverage by 0.35%.
The diff coverage is 3.31%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #7223      +/-   ##
===========================================
+ Coverage    64.22%   64.57%   +0.35%     
===========================================
  Files          336      335       -1     
  Lines        42377    42032     -345     
===========================================
- Hits         27214    27138      -76     
+ Misses       15163    14894     -269     
Impacted Files Coverage Δ
src/core/Tools.cpp 70.63% <0.00%> (-0.57%) ⬇️
src/keeshare/KeeShareSettings.cpp 71.79% <0.00%> (-1.73%) ⬇️
src/keeshare/KeeShareSettings.h 60.00% <ø> (+45.71%) ⬆️
src/keeshare/SettingsWidgetKeeShare.cpp 47.06% <ø> (+20.03%) ⬆️
src/keeshare/ShareExport.cpp 0.00% <0.00%> (ø)
src/keeshare/ShareImport.cpp 0.00% <0.00%> (ø)
src/keeshare/ShareObserver.cpp 11.16% <0.00%> (-0.05%) ⬇️
src/keeshare/group/EditGroupWidgetKeeShare.cpp 34.47% <0.00%> (+1.29%) ⬆️
src/keeshare/KeeShare.cpp 44.03% <100.00%> (+3.12%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4567265...dc39b31. Read the comment docs.

@droidmonkey droidmonkey merged commit 12990e5 into develop Dec 15, 2021
@droidmonkey droidmonkey deleted the refactor/keeshare-part1 branch December 15, 2021 04:23
yan12125 pushed a commit to archlinuxcn/repo that referenced this pull request Dec 15, 2021
In keepassxreboot/keepassxc#7223, quazip is
replaced by minizip, and the option -DWITH_XC_KEESHARE_SECURE is gone.
yan12125 pushed a commit to archlinuxcn/repo that referenced this pull request Mar 2, 2022
archlinux-github pushed a commit to archlinux/aur that referenced this pull request Jul 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: KeeShare pr: refactoring Pull request that refactors code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants