Skip to content

Conversation

@wu3rstle
Copy link
Contributor

corrected argument name to expect to prevent the error: option --private-key not recognized

@jherbel
Copy link
Contributor

jherbel commented Sep 30, 2021

Hi, thanks for this PR. Are you actually using this argument? I am asking since it is not configurable via the GUI and it anyway does the same as --secret (which also looks like a bug to me).

@wu3rstle
Copy link
Contributor Author

Hi, i am using this argument via Integrate Nagios plugins because we are connecting to an sftp server that only allows sftp authentication via ssh-key.

check_sftp --host $HOSTADDRESS$ --user <SFTP_USER> --private-key ~/.ssh/id_rsa.openssh

The two arguments are for different use cases:

  • --secret: password for authentication
  • --private-key: using ssh key for authentication

@jherbel
Copy link
Contributor

jherbel commented Sep 30, 2021

Ok, this makes sense. However, then I think there is another bug in check_sftp. The code says:

elif opt in ["--secret"]:
	opt_pass = arg
elif opt in ["--private-key"]:
	opt_pass = arg

So --secret and --private-key effectively do the same thing (unintentionally). They are both passed on to paramiko.client.SSHClient.connect as password. I think --private-key should be set as pkey in paramiko.client.SSHClient.connect, right? The solution would be:

elif opt in ["--private-key"]:
	opt_key = arg

Could you please take a look and check if I am correct? If I am, could you please include this in your fix and also briefly test it?

@wu3rstle
Copy link
Contributor Author

i added the missing commit which was only local.

@wu3rstle
Copy link
Contributor Author

Now the check on my local check_mk installation works as expected with the copied file.

Now i tell paramiko.client.SSHClient.connect to use the key as argument for key_filename to allow usage of the given ssh key path.
I also adjusted the command's help message to make clear which key-format has to be used for the dependency.

Copy link
Contributor

@jherbel jherbel left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the changes, I would suggest to slightly rename the argument for clarity

@wu3rstle wu3rstle requested a review from jherbel September 30, 2021 09:57
Copy link
Contributor

@jherbel jherbel left a comment

Choose a reason for hiding this comment

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

Looks good, thanks, this will be merged shortly.

LarsMichelsen pushed a commit that referenced this pull request Oct 1, 2021
The active check <tt>check_sftp</tt> claimed to accept <tt>--private-key</tt>
as a command line argument for specifying a private key to be used for the
SFTP login. However, attempting to use this option resulted in the error
<tt>option --private-key not recognized</tt>.

As of this werk, <tt>check_sftp</tt> accepts <tt>--private-key-file</tt> as
a command line argument. This can be used to specify the path to a private
key or certificate file which will be used for authentication.

Closes #398

Change-Id: I8c4f70d2f0831ab4f054283f95d87afdab3def80
@wu3rstle wu3rstle deleted the patch-1 branch October 1, 2021 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants