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

check for empty string #10778

Merged
merged 1 commit into from
Oct 11, 2018
Merged

check for empty string #10778

merged 1 commit into from
Oct 11, 2018

Conversation

suntorytimed
Copy link
Contributor

@suntorytimed suntorytimed commented Aug 21, 2018

Adding a check to see if keyFileContents is empty:

Signed-off-by: Stefan Weiberg sweiberg@suse.com

* this fixes a download error and an exception if the data content
  for encryption is empty
* #3958: for recovering encrypted files with a damaged signature
  this is necessary in addition to turning the signature check off

Signed-off-by: Stefan Weiberg <sweiberg@suse.com>
@ChristophWurst
Copy link
Member

@schiessle 🏓

@MorrisJobke
Copy link
Member

@schiessle 🏓

@schiessle
Copy link
Member

schiessle commented Oct 4, 2018

Makes sense not to try to decrypt an empty string, so for the pr as such 👍

but...
@suntorytimed could you elaborate on what this exactly fixes and how to reproduce it? While I see the point that it doesn't make sense to decrypt an empty string I fail to see how this is related to an broken signature. Thanks!

@schiessle schiessle requested a review from rullzer October 4, 2018 15:27
@suntorytimed
Copy link
Contributor Author

@schiessle this is pointed out in the linked ticket #3958

If for some reason the signature of an encrypted file is corrupted and I switch the signature check off to bypass this check the download of the file won't finish as the last data block of the file is empty and Nextcloud throws an exception due to an empty string that can't be decrypted.

#3958 (comment)

@schiessle
Copy link
Member

Thanks for clarifying, @suntorytimed as soon as we have a second review this can be merged. I would also suggest to backport it. Can you prepare a second PR against the stable14 branch? Thanks!

@rullzer rullzer merged commit dd8350b into nextcloud:master Oct 11, 2018
@suntorytimed
Copy link
Contributor Author

@schiessle done: #11783

MorrisJobke added a commit that referenced this pull request Oct 24, 2018
@MorrisJobke MorrisJobke added this to the Nextcloud 15 milestone Oct 24, 2018
@MorrisJobke MorrisJobke mentioned this pull request Nov 13, 2018
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.

6 participants