-31: SFTP Protocol Error when transferring file via SFTP Link #3406
Comments
|
I did a git bisect and came across [512ad7d] which was a re-factoring of the error handling.
The error originates from sftpfs_opendir in src/vfs/sftpfs/dir.c. |
"-31: SFTP Protocol Error" message can be triggered when entering a remote directory with a symlink pointed to file which located at non readable directory.
/testfile -> /non-accessible-directory/testfile
mc use libssh2_sftp_stat_ex with LIBSSH2_SFTP_LSTAT type, remote lstat() failed
first we need to pimp up sftpfs_ssherror_to_gliberror() to get knowledge about calling libssh2 function and libssh2_sftp_last_error() error code when LIBSSH2_ERROR_SFTP_PROTOCOL (aka -31) occurs. |
first patch incorporate sftp session error hint |
|
|
Just wanted to bump this ticket to say this is still a problem in:
Upon sftp to remote site, copying file from remote back to originating machine, the following error is displayed:
"Failed opening remote file (-31)"
Upon acknowledging the error by hitting return, the copy completes successfully despite the error. |
Any progress?
The version in debian/sid also segv after a while (3:4.8.18-1).
I'm trying to copy a bunch of files over a raspbian/jessie, the path I'm copying into doesn't have any symlink and any user can read and traverse every directory of the destination path.
I tried mc from sources (4.8.18-163-g67b3d6495/glib:2.50.2) without and with both the patches above, it doesn't segv but the error -31 is still present.
Ignoring the error I've copied about 5600 files (49M) without errors (md5 checked), now if I try to overwrite just one it segv, the terminal show a ridiculous alloc failed in glib/gmem.c
(I'm not sure this is the same issue)
Please let me know if I can help in some way (trying patches) |
From looking at the code, it seems that the segfault is a regression caused by
[18f6b85]
and I think that the reason why it segfaults now is is because sftpfs_{l,f}stat functions do not set st_blksize stuff.
Here is an untested patch which should remove the segfault if my guess is correct:
I hope it does compile. |
Thanks zaytsev, sorry for the delay didn't get any mail, it does compile and it does fix the overwrite segv!
About the error -31, I see "protocol error" 2 and 4, usually start with 2 then alternate with some 4 (2 and 4 should be "err").
If there's anything else I can do just let me know (strace? ltrace? enable/insert some trace in mc/ssl? enable debug on server side?) |
I've created a new ticket for the segfault: #3749 .
Re. error -31, sorry, I don't even use sftp and I don't know if and when I will get time to look into it if at all. Too bad and's patches apparently aren't enough to fix the problem :-/ |
I'm not very confident with ssl and this is the first time I see mc sources but, a breakpoint on the error message show bt like this:
If I understand correctly the error is raised within internal.c:sftpfs_stat() by libssh2_sftp_stat_ex
libssh2_sftp_stat_ex return unexpected/unhandled -2 and -4 (res is shown as err), which in my /usr/include/libssh2.h are:
But I don't know ssl enough to understand what is wrong, maybe it require some option to ignore certificates like wget? |
I'm not sure you are looking at the right errors. You should put a break on sftpfs_ssherror_to_gliberror and check the values of res and libssh2_sftp_last_error(super_data->sftp_session).
You said earlier that you get a message that res == -31, so this can't be caused by res == -2 or -4. To decode protocol errors you need to look at the LIBSSH2_FX_* constants. |
Sorry, I got the error thing a bit wrong.
libssh2_sftp_stat_ex() is a two line function which call sftp_stat(), this may exit with error -31 (ftp), 2 and 4 are errors from the server.
I tried to enable-debug in libssh2 in order to get ssh2_strace() working but got into more trouble, time to give up yet, maybe I'll try later with pbuilder.
edit
got libssh2 building :) attached the session log, below the trace mask, I took a fast peek but the log doesn't tell me much
It does really return "no such file", but if the stat() is on the server... I'm copying new files into a remote empty directory... (no clue about the very specific "failure" yet) |
|
In as far as LIBSSH2_FX_NO_SUCH_FILE is concerned, I think the reason why this happens is that the copy function tries to stat destination file to make sure it's not overwriting anything, but the stat function shows protocol error, instead of correctly propagating the status.
Untested patch to check this assumption:
To find out why LIBSSH2_FX_FAILURE happens try to find out how to reliably reproduce it and pay attention to call arguments while debugging. |
I must have too much changes, the patch applied but in the wrong function.
There are 4 call to libssh2_sftp_stat_ex in internal.c but you're right the only one failing is in sftpfs_stat, and with your patch -31:2 is gone.
Let see if a backtrace for the -31:4 can help. |
I tried some copy and every backtrace looks almost the same, the failure seems to be from sftpfs_chmod() (on created directory maybe?)
I just checked, the source directories and the directories created remote by mc have the same privilegies, the only difference is a different user:group (the user "pi" have full access in the destination directory)
https://curl.haxx.se/mail/lib-2016-01/0104.html
sftpfs_chmod() is always failing only in the second do {...} while, after attrs.permissions = mode; |
Yes, this is absolutely disgusting copy & paste programming, how about properly rewriting all this stuff and covering it with tests? ;-P
Probably changing the permissions fails, and this error is not suppressed. Try (untested) patch: |
Thumbs up! error -31:4 gone too :-D
I'm not 100% sure, I don't think is about permissions, the user connecting via sftp own the target directory with full perms +rwx.
I can take a look (or wasn't for me?) but not this year :-D
Thanks a lot for the support! I'm gonna stress sftp a bit in the next days with some daily work.
Keep up the good work, and happy new year! |
Could be UID / GID mismatch though...
It was for anyone potentially willing to take a look, including you. The plan would be:
We have some unit tests implemented with the check framework.
I'm glad we finally identified the reason behind these annoying -31 errors. Thanks for your help as well, hopefully someone will manage to clean up this mess. Happy New Year to you too! |
@zaytsev, challenge accepted :)
I took some time this evening, please see the attached internal.c, I've identified some pattern in the copy&paste code an tried to normalize it, beside the names of added functions/type/macro... let me know what you think about.
edit
I'd like to instrument the test framework you were talking about but have no idea where to start.
Also I'd like to look into @pingu note about ciphers and speed but there also I've no idea where to start (how to change cipher configuration for mc?)
edit
Uploaded latest version of internal.c, looking at the code I wonder if all those sftpfs_fix_filename() need to be in the EAGAIN loop. |
|
Thank you for your help, however, the attached file is of little use for us. Please commit your changes as separate patches and attach those to the ticket. We have a few wiki pages which explain how to work with git: GitGuideLines / WorkingGuideLines - patches from your local branch can be produced with git format-patch command. Regarding tests, all we have is in the tests directory. See the README file there. In as far as ciphers are concerned, sorry, I've got no clue; it would be awesome if sftp respected the ssh_config file, but I don't know if there are any facilities in the library you can make use of to that end. |
@zaytsev, before any official patch I'd like to know if the changes I did are aligned with your idea of cleanup, and eventually I'd really like suggestions/help naming the new functions/macro.
attached a patch, but is only for review (last commit of internal.c was 471ea78).
while there are a lot of changes at once I think they are quite easy to read, if you prefer a set of progressive patches I can do that, will require some more work, just matter of time.
the API of internal.c isn't changed, I added only static functions and a couple of macro.
the sftpfs_* functions had almost all the same pattern:
sftpfs_lstat, sftpfs_stat and sftpfs_chmod now simply call _sftpfs_stat, only stat_type may be different.
only sftpfs_lstat and sftpfs_stat call _sftpfs_attr_to_buf which I think is self explanatory.
the patch for the for regression which caused segfault is within _sftpfs_attr_to_buf (below the comment) |
|
@andrew_b thanks, done, I didn't know.
Also, as mentioned somewhere above, I think all fix statements could be moved outside of the do{...}while(EAGAIN), fixfname are declared as const, the libssh2_sftp_*_ex calls shouldn't touch its content, or I'm missing something?:
If you can confirm the patch is moving in the right direction I'll look into testing the changes.
edit
@andrew_b, only the changes below are related to #3749, then there are about two/three patches to ignore some proto error (specific to #3406), the rest is cleanup, perhaps some comment would be appropriate near the 3406's ignored proto errors (I'll add some) and test carefully that they generate errors when those are expected.
edit
@andrew_b, @zaytsev, please take a look here: https://github.com/xenogenesi/mc/commits/fix-sftp-err-31, this is the minimal patchset needed to get rid of error -31 (segfault fix and and's EACCES patches included), re-integrated from scratch and tested (sorry for me was just easier/faster to fork and share from github)
If you want me to re-integrate the mess-cleanup over those in a set of progressive patches just let me know, I'd like to, but if you want to proceed in a different way, no problem, just let me know. |
Branch: 3406_sftp_error_31
Both and's patches are applied.
The work is not finished. Refactoring of internal.c only was made. Such refactoring is required for file.c and dir.c.
Currently we still have the -31 error at least in following two cases:
|
Great work, awesome!
Nitpick:
The code in [38344f3fa68b0705d686747cd99348f6420e16b7]:
Should have been added in [10fedd5defa2d0c64bc8a1bd6d4f8c2e7a9fccf0] I think.
Question:
In [1740153a767e31523773046fffb7fe60c275f70f] maybe
should be if (res < 0) ? Otherwise these lines will have always to be kept in sync with what sftpfs_internal_stat can return? |
Replying to zaytsev:
Done. |
Replying to zaytsev:
Well, I have read man 2 stat (really, I done it!) and found out that we can't use such way because:
As a result of above, the branch should be reworked almost totally. |
Oh, my fault, I've just copied what Andreas did, sorry about that :-/ So, these two commits:
are completely wrong, and, consequently, everything that follows them has to be updated :-|
Of course, you are right about E* being positive, but I think that we shouldn't rely on that, and the second approach you suggested must be followed.
Are you planning to work on this anytime in the future? I'm asking because currently the branch does not compile due to latest commits. |
The root of the this problem is the passing the file to the opendir() callback:
panel_operate -> panel_operate_init_totals -> compute_dir_size -> do_compute_dir_size -> mc_opendir -> sftpfs_opendir -> libssh2_sftp_open_ex (..., fixfname, ..., LIBSSH2_SFTP_OPENDIR);
Here the "fixfname" is the file not a directory, but we try use it as directory.
Branch: 3406_sftp_error_31
Please review. |
|
The refactoring looks awesome, it's exactly what I had in mind plus much more, however, I can't currently test this code. Could one of the long term error -31 sufferers please check this branch? |
I've just tested this branch and it seems to fix the -31 error for me. Might need some more testing but it looks like the error is gone for me. |
Thanks for feedback. |
|
|
|
|
|
I get the same thing today (2020.11.13) - bulk operations are impossible : (
I'm running MC on Xubuntu 18.04.
$ mc --version
The destination SFTP machine has also Xubuntu 18.04 and: |
I'm getting this error while moving or copying some *.img files over sftp. Almost all other files were moved without any problems. *.img files from another folder were also moved without errors. Then I've found two relatively small files, tried to rename them and still were getting the same error. So lookes like that is not file name related. See mc-err-files.zip https://www.dropbox.com/sh/9yp5f14yqef2mc8/AAB2RVd84XCfNpKX9nw6H9-Ja?dl=0. Probably it is related to sftp server - I use ubuntu 20.04 + vsftpd 3.0.3-12 on the server side.
$ lsb_release -a
$ uname -a
$ cat /proc/cpuinfo
$ mc --version
Data types:
$ openssl version |
Replying to m_pashka:
Do you have permissions to read/write those files in those folders?
vsftpd is an FTP server not SFTP one. |
Important
This issue was migrated from Trac:
pingu(junkmail@….lv)graham@….org.za-31,sftp,error,chiperGNU Midnight Commander 4.8.13
Built with GLib 2.42.1
Using the S-Lang library with terminfo database
With builtin Editor
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With support for X11 events
With internationalization support
With multiple codepages support
Virtual File Systems: cpiofs, tarfs, sfs, extfs, ext2undelfs, ftpfs, sftpfs, fish
Data types: char: 8; int: 32; long: 32; void *: 32; size_t: 32; off_t: 64;
Package: libssh2-1
Source: libssh2
Version: 1.4.3-4
Local version string SSH-2.0-OpenSSH_6.7p1 Debian-3
Remote protocol version 2.0, remote software version dropbear_0.52
Seems that after OpenSSH Server update after infamous Heartblead bug, Im allways getting these errors when copying / moving files within SFTP built in mc:
COPY/MOVE FILE TO REMOTE: "-31: SFTP Protocol Error"
REMOVE REMOTE FILE/DIR: "-31: Failed opening remote file"
Although after I press ENTER - operation continues successfully, however when transfering lots of selected files I have to acknowledge (press ENTER) on every file operation, so batch copy / move / remove operations are inefficient.
Doing verbose CLI sftp connection to the same server, succeeds without errors.
Also to note that to speed up transfers in ssh config file I prioritised blowfish protocol, what is honoured by sftp transfers, but NOT with mc sftp, as with sftp CLI (specifying blowfish) I get allmost twice the speed, but mc sftp transfer speeds are similar to the ones when I force AES encryption on CLI sftp.
Note
Original attachments:
andonJan 7, 2016 at 18:01 UTCandonJan 7, 2016 at 20:24 UTCsand(raziel@….cc) onDec 30, 2016 at 8:16 UTCsand(raziel@….cc) onJan 9, 2017 at 11:18 UTCsand(raziel@….cc) onJan 10, 2017 at 10:46 UTCThe text was updated successfully, but these errors were encountered: