Skip to content

Commit

Permalink
Ignore encrypted keys in config by default when no passphrase is given
Browse files Browse the repository at this point in the history
This commit changes the default of the ignore_encrypted option to be
True when keys are being loaded from config files, automatically
ignoring encrypted keys when no passphrase is specified. For keys
set via client_keys, the default is to still give an error in this
case unless ignore_encrypted is explicitly set.

When loading keys from their default locations, encrypted keys were
already ignored when no passphrase is given, and this behavior isn't
changing.
  • Loading branch information
ronf committed Aug 6, 2022
1 parent 7ec0e74 commit 004799e
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 4 deletions.
13 changes: 9 additions & 4 deletions asyncssh/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -6491,9 +6491,9 @@ class SSHClientConnectionOptions(SSHConnectionOptions):
are already loaded, this argument is ignored.
:param ignore_encrypted: (optional)
Whether or not to ignore encrypted keys when no passphrase is
provided. This is intended to allow encrypted keys specified via
the IdentityFile config option to be ignored if a passphrase
is not specified, loading only unencrypted local keys. Note
specified. This defaults to `True` for keys specified via

This comment has been minimized.

Copy link
@yarikoptic

yarikoptic Aug 8, 2022

is it really for (as - considered per key) or overall

           specified. This defaults to `True` if there are keys specified via

as it seems just overall check in the diff.

This comment has been minimized.

Copy link
@ronf

ronf Aug 9, 2022

Author Owner

When client_keys is passed explicitly, it overrides any IdentityFile values from the config file. So, keys will always come from one or the other (or from the default key names if neither is specified). I can try to clarify the wording.

This comment has been minimized.

Copy link
@ronf

ronf Aug 9, 2022

Author Owner

See commit 3be488d.

This comment has been minimized.

Copy link
@yarikoptic

yarikoptic Aug 9, 2022

thank you for the explanations. I must confess I am not yet 100% sure I get the situation, but primarily due to lack of experience with asyncssh API. Reworded wording seems slightly better for me. But in general -- I trust your decision/docs on this ;) Thanks for addressing the issue!

the IdentityFile config option, causing encrypted keys in the
config to be ignored when no passphrase is specified. Note
that encrypted keys loaded into an SSH agent can still be used
when this option is set.
:param host_based_auth: (optional)
Expand Down Expand Up @@ -6870,7 +6870,7 @@ def prepare(self, last_config: Optional[SSHConfig] = None, # type: ignore
client_keys: _ClientKeysArg = (),
client_certs: Sequence[FilePath] = (),
passphrase: Optional[BytesOrStr] = None,
ignore_encrypted: bool = False,
ignore_encrypted: DefTuple[bool] = (),
gss_host: DefTuple[Optional[str]] = (),
gss_kex: DefTuple[bool] = (), gss_auth: DefTuple[bool] = (),
gss_delegate_creds: DefTuple[bool] = (),
Expand Down Expand Up @@ -7038,6 +7038,11 @@ def prepare(self, last_config: Optional[SSHConfig] = None, # type: ignore

pkcs11_provider: Optional[FilePath]

if ignore_encrypted == ():
ignore_encrypted = client_keys == ()

ignore_encrypted: bool

if client_keys == ():
client_keys = cast(_ClientKeysArg, config.get('IdentityFile', ()))

Expand Down
20 changes: 20 additions & 0 deletions tests/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,26 @@ async def test_connect_sock(self):
async with asyncssh.connect(sock=sock):
pass

@asynctest
async def test_connect_encrypted_key(self):
"""Test connecting with encrytped client key and no passphrase"""

async with self.connect(client_keys='ckey_encrypted',
ignore_encrypted=True):
pass

with self.assertRaises(asyncssh.KeyImportError):
await self.connect(client_keys='ckey_encrypted')

with open('config', 'w') as f:
f.write('IdentityFile ckey_encrypted')

async with self.connect(config='config'):
pass

with self.assertRaises(asyncssh.KeyImportError):
await self.connect(config='config', ignore_encrypted=False)

@asynctest
async def test_connect_invalid_options_type(self):
"""Test connecting using options using incorrect type of options"""
Expand Down

0 comments on commit 004799e

Please sign in to comment.