Scp: quoting errors with spaces in dest filename

[giftig]

When trying to upload individual files with scp, I ran into some unknown errors being reported. Looking at the source, it looks like this is the host sending a response packet which the library doesn't recognise, after sending the initial request to start an scp transfer.

I was transferring relatively large files and with spaces in filenames and directories so I did a few tests to narrow down the actual problem. Filesize isn't important, nor is local filename, but remote filename seems to have issues if there are spaces. I haven't managed to construct a very simple test case which produces unknown error, but I have identified several which produce ambiguous target which seem to be errors in the client:

        for dest in vec![
            "/tmp/foo.txt",
            "/tmp/foo?.txt",
            "/tmp/foo bar.txt",
            "/tmp/foo bar/foo.txt",
            "/tmp/foo bar/foo bar.txt",
            "/tmp/foo\\ bar/foo\\ bar.txt"
        ] {
            println!("DEST: {}", dest);
            let scp = self.session.open_scp()?;
            let res = scp.upload("/tmp/foo.txt", dest);

            match res {
                Ok(_) => println!("OK"),
                Err(e) => println!("ERR: {}", e)
            }
        }

For this test setup I created "/tmp/foo bar" on the remote host. I also tried this test with raw strings and by constructing paths with Path::new first to rule out issues there, but it behaves the same in both cases.

Results were:

DEST: /tmp/foo.txt
OK
DEST: /tmp/foo?.txt
OK
DEST: /tmp/foo bar.txt
ERR: Scp error: scp: ambiguous target

DEST: /tmp/foo bar/foo.txt
ERR: Scp error: scp: ambiguous target

DEST: /tmp/foo bar/foo bar.txt
ERR: Scp error: scp: ambiguous target

DEST: /tmp/foo\ bar/foo\ bar.txt
OK

You can see I can fix the issue manually by escaping any spaces in the remote path, but especially given I'm actually trying to pass it a Path I'd expect any necessary escaping to be handled by the library, as a Path like /tmp/foo bar correctly identifies the target.

I'm not very familiar with the low-level SSH / SCP protocol here but I'm assuming this library is failing to correctly quote it in the way the protocol desires and failing in these edge cases.

If you're open to PRs I might see if I can fix this issue myself and add my test cases above to the unit tests; I see you've tested recursive uploads, implicit target dir, etc. but not any edgecases around funny filenames. I ran into this while working on a little project to learn Rust so this seems like a straightforward issue for me to tackle while I'm here.

EDIT:

I identified how to get unknown error, as well; it's if the destination path contains brackets:

DEST: /tmp/(foo)/foo.txt
ERR: Scp error: unknown error.
DEST: /tmp/(foo).txt
ERR: Scp error: unknown error.
DEST: /tmp/\(foo\).txt
OK

...and again, backslashing fixes them.

[HsuJv]

Well, thanks for pointing out this error.

You can try to have a PR to fix it if you want.
Also if you need help, please do not hesitate to say.

BRs.

[giftig]

Hi @HsuJv, thanks for the quick response.

I had a quick play with getting the project's tests running but after initially noticing I needed to set some SSH_RS_TEST_USERNAME=... env vars to repoint it at a working SSH server, I then had some troubles with the host I tested against not supporting all the algorithms the library needs it to for algorithms tests.

But since I only care about scp I worked around it by running only scp tests with cargo test -F scp --test scp and got prompted for my password a dozen or so times -- because it's invoking some sudo rm -rf commands as part of the tests. It's not the most pleasant test behaviour and I wouldn't feel comfortable running this test environment without at least containerising the environment, which would be some extra steps to get running.

With that in mind I'll probably leave it but hopefully the test cases I listed in the issue are useful.

Just a bit of feedback on potential improvements to the experience of running tests here:

• considering making the tests a bit more robust / easy to run by providing a docker command to spin up a container with the specific ssh version / algorithm support you want to test against, and point at that so the environment is the same for everyone. I had to guess a bit based on reading the tests and the env_getter macro in particular.
• avoid the need for sudo. I didn't see exactly why it's using sudo as I didn't read through all the detail of the test setup, but I would assume any test files being created and cleaned up could be owned by the running user and confined either to /tmp or to a working dir inside the project dir.

[HsuJv]

Some answers to your question:

1. I was thinking of running this test in the GitHub runner, so I didn't think too much about how to run the tests on different hosts.
2. You can get the SSH_RS_TEST... envs from

   ssh-rs/.github/workflows/build.yml

   Lines 71 to 77 in 2699b3d

   	env:
   	SSH_RS_TEST_SERVER: localhost:8888
   	SSH_RS_TEST_USER: ubuntu
   	SSH_RS_TEST_PASSWD: password
   	SSH_RS_TEST_PEM_RSA: /root/rsa_old
   	SSH_RS_TEST_OPENSSH_RSA: /root/rsa_new
   	SSH_RS_TEST_ED25519: /root/ed25519

3. docker is a good idea, but since we've already got the GitHub runners, I think it might be a bit redundant.
4. Using /tmp is a good idea. You can make this change in your PRs. Also, maybe I can do this when I'm free.
   BRs

[giftig]

Do you not normally run the tests locally at all, and just let the github agent run them for you? I guess that works, but having some minimal setup for running tests locally would make it a bit easier to develop as you wouldn't have to keep pushing unfinished changes to github to test while writing the code and the feedback loop would be that much faster.

I had configured some of those SSH_RS_* vars locally to point at a host I'm developing against, but that host didn't support all the algorithms the library covered so they were failing. The bigger problem was the use of sudo in the scp tests though; it's fine on an agent but not so much on my dev machine so it'd be nice to avoid.

Since I ran into a few additional issues (which I raised and I see you've spotted them) I've gone back to using the ssh2-rs crate and working around my openssl issues instead, so I probably won't get chance to address these issues myself, but hopefully my feedback has been useful.

[HsuJv]

I had a virtual machine that uses localhost to perform as a target server to do these tests. And as you may find, the test cases, especially the scp part, can not be performed with a remote server.
That was really my mistake, I should have made it more generic instead of brute-forcing the problem with sudo.
Anyway, thanks for all the work & suggestions you have made.
I'll fix this later. (Being occupied with my business these days)

BRs