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.

Sign up for GitHub

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

Jump to bottom

Clear error message when passphrase not supplied with encrypted private key #2755

Merged
merged 4 commits into from
Jun 17, 2016
Merged

Clear error message when passphrase not supplied with encrypted private key #2755

merged 4 commits into from
Jun 17, 2016

Conversation

lakshmi-kannan
Copy link
Contributor

Before

vagrant@st2test:~$ st2 run core.remote hosts=localhost cmd=date username=vagrant private_key=/home/vagrant/.ssh/test_rsa_pp
.
id: 5763024955fc8c18adf74f00
status: failed
parameters:
  cmd: date
  hosts: localhost
  private_key: '********'
  username: vagrant
result:
  error: "Unable to connect to any one of the hosts: [u'localhost'].

 connect_errors={
  "localhost": {
    "failed": true,
    "traceback": "Traceback (most recent call last):\n  File \"/opt/stackstorm/st2/local/lib/python2.7/site-packages/st2actions/runners/ssh/parallel_ssh.py\", line 241, in _connect\n    client.connect()\n  File \"/opt/stackstorm/st2/local/lib/python2.7/site-packages/st2actions/runners/ssh/paramiko_ssh.py\", line 144, in connect\n    self.client = self._connect(host=self.hostname, socket=self.bastion_socket)\n  File \"/opt/stackstorm/st2/local/lib/python2.7/site-packages/st2actions/runners/ssh/paramiko_ssh.py\", line 578, in _connect\n    client.connect(**conninfo)\n  File \"/opt/stackstorm/st2/local/lib/python2.7/site-packages/paramiko/client.py\", line 380, in connect\n    look_for_keys, gss_auth, gss_kex, gss_deleg_creds, gss_host)\n  File \"/opt/stackstorm/st2/local/lib/python2.7/site-packages/paramiko/client.py\", line 597, in _auth\n    raise saved_exception\nSSHException: not a valid EC private key file\n",
    "timeout": false,
    "succeeded": false,
    "stdout": "",
    "stderr": "",
    "error": "Cannot connect to host. not a valid EC private key file: not a valid EC private key file",
    "return_code": 255
  }
}"
  traceback: "  File "/opt/stackstorm/st2/local/lib/python2.7/site-packages/st2actions/container/base.py", line 89, in _do_run
    runner.pre_run()
  File "/opt/stackstorm/st2/local/lib/python2.7/site-packages/st2actions/runners/ssh/paramiko_ssh_runner.py", line 159, in pre_run
    self._parallel_ssh_client = ParallelSSHClient(**client_kwargs)
  File "/opt/stackstorm/st2/local/lib/python2.7/site-packages/st2actions/runners/ssh/parallel_ssh.py", line 60, in __init__
    connect_results = self.connect(raise_on_any_error=raise_on_any_error)
  File "/opt/stackstorm/st2/local/lib/python2.7/site-packages/st2actions/runners/ssh/parallel_ssh.py", line 90, in connect
    raise NoHostsConnectedToException(msg)
"
vagrant@st2test:~$

After

(virtualenv)vagrant@st2dev /m/s/s/st2 ❯❯❯ st2 run core.remote hosts=localhost username=vagrant private_key=/home/vagrant/.ssh/test_rsa_pp cmd=date
.
id: 57633ad7d9d7ed1889ad2098
status: failed
parameters:
  cmd: date
  hosts: localhost
  private_key: /home/vagrant/.ssh/test_rsa_pp
  username: vagrant
result:
  error: "Unable to connect to any one of the hosts: [u'localhost'].

 connect_errors={
  "localhost": {
    "failed": true,
    "traceback": "Traceback (most recent call last):\n  File \"/mnt/src/storm/st2/st2actions/st2actions/runners/ssh/parallel_ssh.py\", line 241, in _connect\n    client.connect()\n  File \"/mnt/src/storm/st2/st2actions/st2actions/runners/ssh/paramiko_ssh.py\", line 144, in connect\n    self.client = self._connect(host=self.hostname, socket=self.bastion_socket)\n  File \"/mnt/src/storm/st2/st2actions/st2actions/runners/ssh/paramiko_ssh.py\", line 549, in _connect\n    raise Exception(msg)\nException: Private key file /home/vagrant/.ssh/test_rsa_pp is passphrase protected. Supply a passphrase!\n",
    "timeout": false,
    "succeeded": false,
    "stdout": "",
    "stderr": "",
    "error": "Failed connecting to host localhost.: Private key file /home/vagrant/.ssh/test_rsa_pp is passphrase protected. Supply a passphrase!",
    "return_code": 255
  }
}"
  traceback: "  File "/mnt/src/storm/st2/st2actions/st2actions/container/base.py", line 89, in _do_run
    runner.pre_run()
  File "/mnt/src/storm/st2/st2actions/st2actions/runners/ssh/paramiko_ssh_runner.py", line 159, in pre_run
    self._parallel_ssh_client = ParallelSSHClient(**client_kwargs)
  File "/mnt/src/storm/st2/st2actions/st2actions/runners/ssh/parallel_ssh.py", line 60, in __init__
    connect_results = self.connect(raise_on_any_error=raise_on_any_error)
  File "/mnt/src/storm/st2/st2actions/st2actions/runners/ssh/parallel_ssh.py", line 90, in connect
    raise NoHostsConnectedToException(msg)
"
(virtualenv)vagrant@st2dev /m/s/s/st2 ❯❯❯

lakshmi-kannan
lakshmi-kannan reviewed Jun 16, 2016
View reviewed changes
st2actions/st2actions/runners/ssh/paramiko_ssh.py
except paramiko.ssh_exception.PasswordRequiredException:
msg = ('Private key file %s is passphrase protected. Supply a passphrase!' %
self.key_files)
raise Exception(msg)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default message in that exception sends out a cryptic message that's not obvious to user. Hence I send out a generic exception.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please log the original exception.

@manasdk
Copy link
Contributor

manasdk commented Jun 17, 2016

👍

@Kami
Copy link
Member

Kami commented Jun 17, 2016

Good catch - thanks!

Let's please also add a test case for it.

@lakshmi-kannan
Copy link
Contributor Author

Added unit test: 1bf5009#diff-4e2bf97e5b553c71e4a57637fb0d1253R148

Merge branch 'master' into STORM-2178/clear_error_message_for_passphr…
b762b3a 
…ase_protected_keys

* master:
  update changelog
  Test for show_secrets param
  cli support for unmasking secrets if user is an admin
  Fix imports
  change base class
  add missing import
  Use _get_mask_secrets in actionexecutions
  Ability to show_secrets for admin as applied to apikeys
  Use the right mask_secrets property

Conflicts:
	CHANGELOG.rst
@lakshmi-kannan lakshmi-kannan merged commit 9966144 into StackStorm:master Jun 17, 2016
@lakshmi-kannan lakshmi-kannan deleted the STORM-2178/clear_error_message_for_passphrase_protected_keys branch June 17, 2016 21:58
Kami
Kami reviewed Jun 18, 2016
View reviewed changes
st2actions/tests/unit/test_parallel_ssh.py
@@ -49,6 +51,8 @@ def test_connect_with_password(self):
client._hosts_client[host].client.connect.assert_called_once_with(**expected_conn)

@patch('paramiko.SSHClient', Mock)
@patch.object(ParamikoSSHClient, '_is_key_file_needs_passphrase',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needing to patch this "implementation detail" everywhere is kinda nasty and hard to maintain - is there no other way to achieve that?

Anyways, thanks for adding the test 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
WIP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lakshmi-kannan @manasdk @Kami