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

Fix sharing with user-key encryption #39908

Merged
merged 1 commit into from
Mar 22, 2022
Merged

Conversation

jvillafanez
Copy link
Member

Description

Having user-key encryption enabled, shares being received by the user might not be properly decrypted.

The cause was a wrong path and a length-base check. Basically a //files/path/to/file was used (notice the double //) and the check was using a strlen('/files'); the resulting path was s/path/to/file instead of the expected /path/to/file.
When we checked for the shares of the target path (s/path/to/file), we didn't find it, so we returned an access list containing only the owner of the file instead of the owner and the sharees. In the end, the key file for the sharee wasn't present in the right place.

Related Issue

owncloud/encryption#329

Motivation and Context

How Has This Been Tested?

Manually tested

  1. Setup up user-key encryption
  2. user1 shares a file with user2
  3. user2 tries to download the file.

With the patch, user2 can preview the file in the file list and he can also download the file without problems

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@ownclouders
Copy link
Contributor

ownclouders commented Mar 22, 2022

💥 Acceptance tests pipeline apiShareManagementBasicToShares-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/35034/66

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

100.0% 100.0% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

Looks sensible anyway, to remove the accidental // that was being produced if mountPoint was the empty string. Let's merge this, and we will get a new result from nightly CI. If there are other issues, we can followup with another PR(s).

@owncloud owncloud deleted a comment from update-docs bot Mar 22, 2022
@phil-davis phil-davis merged commit dca13d6 into master Mar 22, 2022
@delete-merged-branch delete-merged-branch bot deleted the fix_userkey_encryption_share branch March 22, 2022 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants