dvc tries to use default SSH keys even if SSH link provides username and password

[TrevorG]

Bug Report

Description

It seems that DVC is trying to use default (~/.ssh/id_rsa, ~/.ssh/id_ed25519, etc.) SSH keys even if link is provided as ssh://username:password@host. Moreover, it use provided password as passphrase. In most cases, password will not match passphrase which results in failure of calling dvc command with "Incorrect passphrase" error.

Reproduce

1. $ ssh-keygen (skip this if you have some keys with default path)
2. $ dvc get-url "ssh://username:password@hostname:/path/to/file/"
3. "ERROR: unexpected error - Incorrect passphrase"
4. $ mv ~/.ssh/id_rsa ~/.ssh/id_rsa_tmp
5. $ dvc get-url "ssh://username:password@hostname:/path/to/file/"
6. Pass

Expected

File should be downloaded using credentials provided in link.

Environment information

Output of dvc doctor:

$ dvc doctor
DVC version: 2.18.1 (pip)
---------------------------------
Platform: Python 3.10.6 on Linux-5.15.0-43-generic-x86_64-with-glibc2.31
Supports:
	http (aiohttp = 3.8.1, aiohttp-retry = 2.8.3),
	https (aiohttp = 3.8.1, aiohttp-retry = 2.8.3),
	ssh (sshfs = 2022.6.0),
	webhdfs (fsspec = 2022.7.1)
Cache types: hardlink, symlink
Cache directory: ext4 on /dev/sda1
Caches: local
Remotes: ssh
Workspace directory: ext4 on /dev/sda1
Repo: dvc, git

Additional Information (if any):

$ dvc get-url "ssh://username:password@hostname:/path/to/file/ local_file -v" #with default SSH key existing
2022-08-17 13:41:34,627 ERROR: unexpected error - Incorrect passphrase
------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/dvc/cli/__init__.py", line 185, in main
    ret = cmd.do_run()
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/dvc/cli/command.py", line 36, in do_run
    return self.run()
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/dvc/commands/get_url.py", line 18, in run
    Repo.get_url(self.args.url, out=self.args.out, jobs=self.args.jobs)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/dvc/repo/get_url.py", line 17, in get_url
    if not dep.exists:
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/dvc/output.py", line 453, in exists
    return self.fs.exists(self.fs_path)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/dvc_objects/fs/base.py", line 269, in exists
    return self.fs.exists(path)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/funcy/objects.py", line 50, in __get__
    return prop.__get__(instance, type)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/funcy/objects.py", line 28, in __get__
    res = instance.__dict__[self.fget.__name__] = self.fget(instance)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/dvc_objects/fs/implementations/ssh.py", line 115, in fs
    return _SSHFileSystem(**self.fs_args)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/fsspec/spec.py", line 76, in __call__
    obj = super().__call__(*args, **kwargs)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/sshfs/spec.py", line 76, in __init__
    self._client, self._pool = self.connect(
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/fsspec/asyn.py", line 111, in wrapper
    return sync(self.loop, func, *args, **kwargs)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/fsspec/asyn.py", line 96, in sync
    raise return_result
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/fsspec/asyn.py", line 53, in _runner
    result[0] = await coro
  File "/usr/lib/python3.10/asyncio/tasks.py", line 445, in wait_for
    return fut.result()
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/sshfs/utils.py", line 27, in wrapper
    return await func(*args, **kwargs)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/sshfs/spec.py", line 91, in _connect
    client = await self._stack.enter_async_context(_raw_client)
  File "/usr/lib/python3.10/contextlib.py", line 619, in enter_async_context
    result = await _cm_type.__aenter__(cm)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/asyncssh/misc.py", line 223, in __aenter__
    self._result = await self._coro
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/asyncssh/connection.py", line 6887, in connect
    options = SSHClientConnectionOptions(options, config=config, host=host,
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/asyncssh/connection.py", line 5596, in __init__
    super().__init__(options=options, last_config=last_config, **kwargs)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/asyncssh/misc.py", line 271, in __init__
    self.prepare(**self.kwargs)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/asyncssh/connection.py", line 6334, in prepare
    client_keys = load_default_keypairs(passphrase, client_certs)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/asyncssh/public_key.py", line 3239, in load_default_keypairs
    result.extend(load_keypairs(file, passphrase, certlist,
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/asyncssh/public_key.py", line 3121, in load_keypairs
    keys = read_private_key_list(keylist, passphrase)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/asyncssh/public_key.py", line 3035, in read_private_key_list
    keys = _decode_list(read_file(filename), _decode_private, passphrase)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/asyncssh/public_key.py", line 2594, in _decode_list
    obj, end = decoder(data, *args, **kwargs)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/asyncssh/public_key.py", line 2519, in _decode_private
    key = _decode_pem_private(pem_name, headers, data, passphrase)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/asyncssh/public_key.py", line 2433, in _decode_pem_private
    return _decode_openssh_private(data, passphrase)
  File "/mnt/repos/ml-deployment/.venv/lib/python3.10/site-packages/asyncssh/public_key.py", line 2326, in _decode_openssh_private
    raise KeyEncryptionError('Incorrect passphrase') from None
asyncssh.pbe.KeyEncryptionError: Incorrect passphrase
------------------------------------------------------------
2022-08-17 13:41:34,831 DEBUG: link type reflink is not available ([Errno 95] no more link types left to try out)
2022-08-17 13:41:34,832 DEBUG: Removing '/mnt/repos/.P2UNygP9QVPudZv2EdUt77.tmp'
2022-08-17 13:41:34,832 DEBUG: Removing '/mnt/repos/.P2UNygP9QVPudZv2EdUt77.tmp'
2022-08-17 13:41:34,832 DEBUG: Removing '/mnt/repos/.P2UNygP9QVPudZv2EdUt77.tmp'
2022-08-17 13:41:34,832 DEBUG: Removing '/mnt/repos/ml-deployment/.dvc/cache/.D3VrJtEp3MJvSTdHawHs6R.tmp'
2022-08-17 13:41:34,840 DEBUG: Version info for developers:
DVC version: 2.18.1 (pip)
---------------------------------
Platform: Python 3.10.6 on Linux-5.15.0-43-generic-x86_64-with-glibc2.31
Supports:
	http (aiohttp = 3.8.1, aiohttp-retry = 2.8.3),
	https (aiohttp = 3.8.1, aiohttp-retry = 2.8.3),
	ssh (sshfs = 2022.6.0),
	webhdfs (fsspec = 2022.7.1)
Cache types: hardlink, symlink
Cache directory: ext4 on /dev/sda1
Caches: local
Remotes: ssh
Workspace directory: ext4 on /dev/sda1
Repo: dvc, git

Having any troubles? Hit us up at https://dvc.org/support, we are always happy to help!
2022-08-17 13:41:34,842 DEBUG: Analytics is enabled.
2022-08-17 13:41:34,868 DEBUG: Trying to spawn '['daemon', '-q', 'analytics', '/tmp/tmpule9cdwx']'
2022-08-17 13:41:34,869 DEBUG: Spawned '['daemon', '-q', 'analytics', '/tmp/tmpule9cdwx']'

[daavoo]

Hi @TrevorG .
Do you have any dvc config setup?
Could you share the output of dvc config -l --show-origin

[TrevorG]

Yeah, sure. I have applied some minor privacy filter on domain name.

$ dvc config -l --show-origin
/home/trevorg/.config/dvc/config	remote.pulsar.url=ssh://dvc@pulsar.domain:/tank/services/dvc/storage
/home/trevorg/.config/dvc/config	remote.pulsar.keyfile=/home/trevorg/.ssh/pulsar-dvc.key
.dvc/config	core.remote=pulsar

[daavoo]

We are hardcoding the passphrase arg to be the same as password in:

https://github.com/iterative/dvc-objects/blob/c61fc37e94de57ee249feb886f7df283b03a7234/src/dvc_objects/fs/implementations/ssh.py#L72

Introduced in #6566 and looks like a legacy leftover from paramiko

This is causing the error because asyncssh tries to decrypt the private SSH keys with the password provided in the URL.

I believe we should have separate config options for password and passphrase, as they have different meanings and can cause issues (like this one) when considering them equivalent and passing them to asyncssh args.

[pmrowla]

This change will break existing behavior for anyone that has been using password to set SSH key passphrases.

Also, how do we differentiate between password and passphrase when users have enabled the interactive ask_password option? Do we need an entirely separate prompt and SSH remote specific ask_passphrase option as well? (which would then break existing behavior for anyone using ask_password to set SSH key passphrases)

I think it might be better to do

if not (login_info["username"] and login_info["password"]):
    login_info["passphrase"] = config.get("password")

rather than adding separate config options

[daavoo]

This change will break existing behavior for anyone that has been using password to set SSH key passphrases.

Also, how do we differentiate between password and passphrase when users have enabled the interactive ask_password option? Do we need an entirely separate prompt and SSH remote specific ask_passphrase option as well? (which would then break existing behavior for anyone using ask_password to set SSH key passphrases)

Valid points. I can try to introduce fix in backwards compatible way. Still feel like we should deprecate this behavior, as it was just drawed from paramiko implementation and is causing a misuse of asyncssh API.

I think it might be better to do

if not (login_info["username"] and login_info["password"]):
login_info["passphrase"] = config.get("password")

rather than adding separate config options

I don't really know (and we don't test) the full implications of misusing the passphrase argument in asyncssh (even new implications might be added in future releases).

I think we could instead introduce a more explicit password_is_passphrase (or similar) option, defaulting to current behavior, in addition to a new passphrase / ask_passphrase options. So users can deactivate the old behavior and use the new config option.