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

FTP mount doesn't work with encryption #5950

Closed
PVince81 opened this issue Nov 19, 2013 · 12 comments
Closed

FTP mount doesn't work with encryption #5950

PVince81 opened this issue Nov 19, 2013 · 12 comments

Comments

@PVince81
Copy link
Contributor

Steps

  1. Mount an FTP storage (vsftpd)
  2. Enable encryption
  3. Upload file
  4. Download file

Expected result

Downloaded file is not encrypted.

Actual result

Downloaded file is still encrypted

CC @schiesbn FYI
I suspect it could be another fseek() issue.

@PVince81
Copy link
Contributor Author

FTP is using fopen() directly on the "ftp://" address, which effectively streams the content.
It also means that the fseek() detection can't work.

So to fix this we either need to:

  1. Fix the encryption to not use the fseek() approach any more
    or
  2. Modify the FTP ext storage to use a temporary file.

The second one might be overkill if we anyway plan to fix 1) in the future and maybe try and use the streaming approach for ext storage in general, as per #5949

@PVince81
Copy link
Contributor Author

I tried uploading two files, a small one and a big one. When downloading them, they are both empty.
The DB shows a size of "0" for both files, but the "unencrypted_size" is correct.

If I create a file using the text editor, I can download it properly, maybe because it's using file_get_contents instead of the stream calls.

@schiesbn please have a look.

@PVince81
Copy link
Contributor Author

Seems to be an issue with the writeback code in ftp.php.
It is only triggered when a file is opened in "r+" mode, which encryption does.
Without encryption it doesn't go there.

Getting this:
failed to open stream: Remote file already exists and overwrite context option not specified\nFTP server reports 213

@PVince81
Copy link
Contributor Author

When opening the FTP stream with fopen(), even when using the mode "w", a later call to stream_get_meta_data will return a mode of "r+" which will mess up...

http://grokbase.com/t/php/php-bugs/07c6yz4nrg/43510-new-stream-get-meta-data-does-not-return-same-mode-as-used-in-fopen

@PVince81
Copy link
Contributor Author

FYI @icewind1991 @schiesbn we need to be careful with this in the future

@PVince81
Copy link
Contributor Author

@schiesbn I tried this again but it hangs on FTP upload.
When I kill the server and browser, the file is there, uploaded correctly even though it's encrypted.

Since #7454 the encryption app now correctly remembers that the fopen mode is "w" which is a good thing.

Still, what happens is that the "postFOpen" hook calls "isEncryptedPath()" (why need this for "w" mode if we completely overwrite?) which tries to do a second "fopen" of the same file on the FTP server, and blocks. I suspect that the FTP server is probably waiting for the "write" call to finish before returning the result (some kind of locking). After a timeout, I get this in the log:

{"app":"PHP","message":"fopen(ftp:\/\/[email protected]\/home\/vincent\/temp\/ftpmount\/enc\/sub\/x\/o\/IMG_20130910_071929.jpg): failed to open stream: FTP server reports RETR at \/srv\/www\/htdocs\/owncloud\/apps\/files_external\/lib\/ftp.php#90","level":0,"time":"2014-03-26T18:45:34+00:00"}

Would it be possible to remove the "isEncryptedPath()" test when mode is "w" or do that check in "preFOpen" ?

Remember that "isEncryptedPath()", if it wouldn't block, would find out that "fseek()" doesn't work, then close, and then call getLocalFile() which would yet again open a third connection to download the full file to check if it is encrypted.

fseek(). But fseek() fails, so it tries to download the file to do the "is it encrypted" detection that open a second fopen() in "r" mode to download the file. I can't debug further, it always hangs at some point.

@schiessle
Copy link
Contributor

The isEncryptedPath() call is needed to decide if files should be encrypted or not. We could indeed move it to a postFopen proxy. I will look into it.

What FTP server do you use? The last time I tried it everything worked as expected but I remember that you use a different FTP server and they not always behave the same way.

@PVince81
Copy link
Contributor Author

I used vsftpd. I guess the difference could be that mine was using file locking (if a write operation is pending, it blocks read operations)

@PVince81 PVince81 modified the milestones: ownCloud 7, ownCloud 6 May 7, 2014
@craigpg craigpg modified the milestones: need more information, ownCloud 7 Sep 2, 2014
@LukasReschke
Copy link
Member

Is this fixed with #7912?

@MorrisJobke MorrisJobke removed this from the need more information milestone Jan 23, 2015
@PVince81
Copy link
Contributor Author

@schiesbn we should retry with the new encryption

@PVince81 PVince81 added this to the backlog milestone Mar 1, 2016
@schiessle
Copy link
Contributor

works with new encryption: #23657

@lock
Copy link

lock bot commented Aug 5, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants