-
Notifications
You must be signed in to change notification settings - Fork 34
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
(SSE compatibility) Fix SSE files being mistaken for E2EE #495
Conversation
Signed-off-by: Alyx <[email protected]>
Hello there, We hope that the reviewing process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR reviewing process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! |
so, i've tested it on my own installation since i made this PR, and so far everything just works fine. i didn't encounter any issues. i think i successfully made E2EE and SSE compatible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, thanks.
I left some minor code style comments :)
thanks! :) |
I would recommend doing it locally. There are other places where you use spaces instead of spaces that should be updated too. |
Signed-off-by: Alyx <[email protected]>
alright, done! |
Looks good @fluse1367 :). |
# Conflicts: # lib/E2EEnabledPathCache.php
done @artonge :) |
@fluse1367 some unit tests are failing due to your changes. Can you take a look? |
Closing in favor of #593 due to inactivity |
hey @artonge, sorry for not answering :( thanks for taking over ❤️ |
Thanks for your investigation and work :) |
@fluse1367 Have you been using both encryption since this PR got merged? Do you have any feedback? Is there some limitation pending? |
@artonge yes i have and everything's working quite well ;P one time tho (i dont know how it happened, and it didnt happen again) my instance got into a weird state where some temporary part files (from uploading stuff) remained on the FS and didnt get deleted, that caused NC to be unable to interact with the target files AT ALL. however i just deleted the part files and then everything went back to normal. idk if that has something to do with the patch tho, may be just a regular bug 🤷♀️ i didnt need to recover from any of my NC backups, and as far as i can tell, all data is still intact however, i'm still doing regular external backups of my e2ee stuff from the source (using gnome deja dup), just in case ^^ |
Thanks for the feedback @fluse1367. We'll keep on eye open for such kind of bug :). Was it when uploading E2EE files? Which files did you delete? |
@artonge |
Based on my observations, I'm pretty sure to have found a somewhat reliable way to tell SSE and E2EE files apart, however i do not know if this is 100% reliable. I've tested this fix a bit and it seems like it's working fine.
What I've learned is, that with SSE, a folder will never be marked as encrypted in the filecache. With E2EE a file can only be E2EE if it's inside an E2EE folder. So, if a folder is marked as encrypted, it and all of its contents have to be E2EE.
Based on that, its fairly easy to check if a file/folder is SSE or E2EE: If a node has any parent marked as encrypted, it is E2EE. If that node however has no parent marked as encrypted but the node itself is, it is SSE.
I know this plugin is not compatible with SSE, however i still did this fix to at least increase compatibility (also, i have E2EE alongside with SSE and i do not want to disable either, so yeah this is the outcome).
This also addresses #489.