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.

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

Allow ssh command to work with "PAM User" records #1292

Open
phlibi opened this issue Aug 15, 2024 · 1 comment
Open

Allow ssh command to work with "PAM User" records #1292

phlibi opened this issue Aug 15, 2024 · 1 comment

Comments

@phlibi
Copy link

phlibi commented Aug 15, 2024

Hi all,

Keeper currently supports the record types "PAM Machine" and "PAM User", which are both supposed to work with password and SSH key rotation. For some reason, password rotation does not work for us for records of type "PAM Machine", so we use "PAM User" instead. Except that those user records do not contain a predefined field for the hostname/IP address, they seem to be mostly equal.

The problem with Keeper Commander is that it does not recognize "PAM User" records containing SSH credentials when using the ssh command:

My Vault> ssh backup-1/phip
ssh: Enter name of existing record

A quick fix for this is to allow the pamUser record type in the filter list in commands/connect.py:

--- a/./commands/connect.py.orig
+++ b/./commands/connect.py
@@ -231,7 +231,7 @@ class ConnectSshCommand(BaseConnectCommand):
         return ssh_parser
 
     def execute(self, params, **kwargs):
-        ssh_record_types = ['serverCredentials', 'sshKeys', 'pamMachine']
+        ssh_record_types = ['serverCredentials', 'sshKeys', 'pamMachine', 'pamUser']
         record_name = kwargs['record'] if 'record' in kwargs else None
         if not record_name:
             ls = RecordListCommand()

To make it actually work, it is required to add the hostname/IP address field to the "PAM User" record. To get the naming right, I've found it to be easiest to change the record type to "PAM Machine", fill in the hostname/IP and port fields, save the record and then change the type back to "PAM User".
With this information in place, the ssh command now also works with a "PAM User" record:

My Vault> ssh backup-1/phip
Connecting to "backup-1/phip" ...
...
phip@backup-1:~$ 

In case the hostname/IP is not provided, the error message is just fine:

My Vault> ssh compute-1/phip
Record "compute-1/phip" does not have host.

A drawback is that ssh without arguments does not pick up the hostname/IP information from the record if it is a pamUser record:

My Vault> ssh
  #  Record UID    Type               Title                         Description                         Shared
---  ------------  -----------------  ----------------------------  ----------------------------------  --------
  1  WU9...GIg     pamUser            backup-1/phip                 phip                                True
  2  -cJ...QZg     pamUser            compute-1/phip                phip                                True
  3  VfC...NmQ     serverCredentials  fallback-server-1             root @ 10.X.Y.Z:22                  True
...

This requires another fix in the generation of the record description in vault_extensions.py, so that it also looks for a "host" type in the custom fields, the same way as already done by TypedRecord.get_typed_field():

--- a/vault_extensions.py.orig
+++ b/vault_extensions.py
@@ -107,7 +107,7 @@ def get_record_description(record):   # type: (vault.KeeperRecord) -> Optional[s
                 if field:
                     comps.append(field.get_default_value())
                 else:
-                    field = next((x for x in record.fields if x.type.lower() in {'host', 'pamhostname'}), None)
+                    field = next((x for x in itertools.chain(record.fields, record.custom) if x.type.lower() in {'host', 'pamhostname'}), None)
                     if field:
                         host = field.get_default_value()
                         if isinstance(host, dict):

(maybe the line there could also be simplified to use TypedRecord.get_typed_field() instead of reimplementing part of it)

Now the description is also fine:

My Vault> ssh
  #  Record UID   Type               Title                         Description                         Shared
---  -----------  -----------------  ----------------------------  ----------------------------------  --------
  1  WU9...GIg    pamUser            backup-1/phip                 phip @ 10.X.Y.Z:22                  True
  2  -cJ...QZg    pamUser            compute-1/phip                phip                                True
  3  VfC...NmQ    serverCredentials  fallback-server-1             root @ 10.X.Y.Z:22                  True
...

Please consider integrating these changes.

Thanks!

@sk-keeper
Copy link
Collaborator

These changes make total sense.
We will add them to the next Commander release.

Thanks

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

No branches or pull requests

2 participants