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

Feature/e2ee use hardware token secure storage #5877

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

mgallien
Copy link
Collaborator

Close #5685

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ clang-tidy found issue(s) with the introduced code (1/1)

@mgallien mgallien force-pushed the feature/e2eeUseHardwareTokenSecureStorage branch from 820e33a to 2273a3f Compare July 27, 2023 13:09
@sonarcloud
Copy link

sonarcloud bot commented Jul 27, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 6 Code Smells

61.9% 61.9% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Attention: Patch coverage is 30.88235% with 188 lines in your changes missing coverage. Please review.

Project coverage is 59.59%. Comparing base (3dc583c) to head (d254c34).
Report is 1054 commits behind head on master.

Current head d254c34 differs from pull request most recent head 2000d62

Please upload reports for the commit 2000d62 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5877      +/-   ##
==========================================
- Coverage   60.79%   59.59%   -1.21%     
==========================================
  Files         145      145              
  Lines       18836    19011     +175     
==========================================
- Hits        11451    11329     -122     
- Misses       7385     7682     +297     
Files Coverage Δ
src/libsync/account.h 42.85% <ø> (ø)
src/libsync/clientsideencryption.h 47.05% <100.00%> (+11.34%) ⬆️
src/libsync/clientsideencryptionjobs.h 10.00% <ø> (ø)
src/libsync/discovery.cpp 86.39% <ø> (-0.14%) ⬇️
src/libsync/updatefiledropmetadata.cpp 58.77% <100.00%> (ø)
...libsync/abstractpropagateremotedeleteencrypted.cpp 0.00% <0.00%> (ø)
src/libsync/encryptfolderjob.cpp 0.00% <0.00%> (ø)
src/libsync/propagateuploadencrypted.cpp 0.00% <0.00%> (ø)
src/libsync/syncengine.cpp 80.05% <0.00%> (+0.13%) ⬆️
src/libsync/account.cpp 42.75% <0.00%> (-1.69%) ⬇️
... and 2 more

... and 32 files with indirect coverage changes

@mgallien mgallien force-pushed the feature/e2eeUseHardwareTokenSecureStorage branch 2 times, most recently from 7b12e07 to eb9dcfd Compare July 28, 2023 10:55
@mgallien mgallien force-pushed the feature/e2eeUseHardwareTokenSecureStorage branch 4 times, most recently from 13d5810 to bf78e6a Compare August 18, 2023 08:11
@mgallien mgallien force-pushed the feature/e2eeUseHardwareTokenSecureStorage branch from bf78e6a to 1ce0ffb Compare August 22, 2023 20:10
@mgallien
Copy link
Collaborator Author

updating the linux CI images to add support for this PR
nextcloud/docker-ci#582

@mgallien mgallien force-pushed the feature/e2eeUseHardwareTokenSecureStorage branch from 1ce0ffb to 6b3beec Compare August 29, 2023 11:26
@mgallien
Copy link
Collaborator Author

add support to build libp11 in KDE's Craft nextcloud/desktop-client-blueprints#8
temporarily put into our own blueprints repository
pending review upstream

@mgallien
Copy link
Collaborator Author

upstream review of the Craft blueprint
https://invent.kde.org/packaging/craft-blueprints-kde/-/merge_requests/654

@mgallien mgallien force-pushed the feature/e2eeUseHardwareTokenSecureStorage branch 3 times, most recently from d254c34 to 0567e97 Compare September 4, 2023 13:44
@sonarcloud
Copy link

sonarcloud bot commented Sep 4, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell B 64 Code Smells

24.3% 24.3% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@nextcloud nextcloud deleted a comment from github-actions bot Sep 4, 2023
@mgallien mgallien force-pushed the feature/e2eeUseHardwareTokenSecureStorage branch from 0567e97 to 81d494a Compare September 8, 2023 08:34
NEXTCLOUD.cmake Outdated Show resolved Hide resolved
NEXTCLOUD.cmake Outdated Show resolved Hide resolved
@claucambra
Copy link
Collaborator

Realised this was still a draft half-way through

@mgallien mgallien force-pushed the feature/e2eeUseHardwareTokenSecureStorage branch 2 times, most recently from ff7edeb to 85e00fc Compare September 18, 2023 13:42
@mgallien mgallien force-pushed the feature/e2eeUseHardwareTokenSecureStorage branch 4 times, most recently from e5d881d to cd56880 Compare September 21, 2023 21:05
@mgallien mgallien force-pushed the feature/e2eeUseHardwareTokenSecureStorage branch from 2000d62 to 39ff954 Compare June 14, 2024 15:17
@mgallien mgallien force-pushed the feature/e2eeUseHardwareTokenSecureStorage branch from 39ff954 to 45bb571 Compare September 4, 2024 14:06
@mgallien mgallien marked this pull request as ready for review September 25, 2024 07:17
@mgallien mgallien force-pushed the feature/e2eeUseHardwareTokenSecureStorage branch from 45bb571 to 5cc95f1 Compare September 25, 2024 07:38
if we cannot encrypt because that is not working or because we lack keys
and certificate needed to enable the feature makes no difference

Signed-off-by: Matthieu Gallien <[email protected]>
ensure we do not emit a "certificate migration is needed" signal when
this is not the case

Signed-off-by: Matthieu Gallien <[email protected]>
…lector

will prevent access to internal objects not meant for reuse outside
internal code

Signed-off-by: Matthieu Gallien <[email protected]>
better check that PKCS11 keys can be converted to use them

improve logs to be able to get a better insight on crypto operations
during testing of crypto

improve some API method names

fix some log output that were using teh worng category

Signed-off-by: Matthieu Gallien <[email protected]>
should avoid calling a crash prone libp11 API to get public key from
hardware token

Signed-off-by: Matthieu Gallien <[email protected]>
even if the certificate already exists abnd is stored on USB token
hardware, we need to upload the certificate to allow sharing between
users

Signed-off-by: Matthieu Gallien <[email protected]>
should make it clearer what is the code and encryption keys and settings
in use

should make the code simpler to read and follow

Signed-off-by: Matthieu Gallien <[email protected]>
this used to rely on a QSslKey object to convey the receiving user of a
shared folder encryption material

because we added CertificateInformation that is more generic and can
work with USB token hardware stored private keys or pure software keys,
we want to use that instead of a simple QSslKey

Signed-off-by: Matthieu Gallien <[email protected]>
will fix some automated tests failing due to stack overflow when
creating `ActivityListModel` instances

Signed-off-by: Matthieu Gallien <[email protected]>
I may need to ensure other parts behave as expected in case we can only
decrypt but not encrypt

Signed-off-by: Matthieu Gallien <[email protected]>
the previous certificates were linked to a now invalid libp11 context

using any of them will lead to troubles

Signed-off-by: Matthieu Gallien <[email protected]>
if you use a new certificate and access metadata of an older one, we
pick up this previous certificate to access the metadata

Signed-off-by: Matthieu Gallien <[email protected]>
@mgallien mgallien force-pushed the feature/e2eeUseHardwareTokenSecureStorage branch from 5cc95f1 to 8b9f325 Compare October 2, 2024 07:55
@nextcloud-desktop-bot
Copy link

AppImage file: nextcloud-PR-5877-8b9f3252171c5ac11a4cfeaa302a29b2299640a6-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

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

Successfully merging this pull request may close these issues.

Add option to store key on user device when end-to-end encryption is enabled
3 participants