Keeper-Security / Commander

Keeper Commander is a python-based CLI and SDK interface to the Keeper Security platform. Provides administrative controls, reporting, import/export and vault management.
https://www.keepersecurity.com/commander.html
MIT License
182 stars 74 forks source link

ssh_agent: ed25519 keys without comment and passphrase are not loaded #1254

Closed phlibi closed 1 week ago

phlibi commented 2 weeks ago

Hi everyone,

I discovered that my ed25519 SSH private keys stored in PAM user records and generated using the automatic rotation feature are not loaded into the agent. The problem seems to be that there is a minimum assumed length of 444 characters, while the keys are shorter when there is no comment included and no passphrase set:

# Comment and passphrase: 444 characters
$ yes | ssh-keygen -t ed25519 -N 'abc' -f k -C 'comment' && wc k
Generating public/private ed25519 key pair.
k already exists.
Overwrite (y/n)? Your identification has been saved in k
Your public key has been saved in k.pub
The key fingerprint is:
...
  8  14 444 k

# Only passphrase:
$ yes | ssh-keygen -t ed25519 -N 'abc' -f k -C '' && wc k
Generating public/private ed25519 key pair.
k already exists.
Overwrite (y/n)? Your identification has been saved in k
Your public key has been saved in k.pub
...
  8  14 444 k

# Only comment: 399 characters (or more, depending on length of comment):
$ yes | ssh-keygen -t ed25519 -N '' -f k -C 'comment' && wc k
Generating public/private ed25519 key pair.
k already exists.
Overwrite (y/n)? Your identification has been saved in k
Your public key has been saved in k.pub
...
  7  13 399 k

# Neither comment nor passphrase: 387 characters
$ yes | ssh-keygen -t ed25519 -N '' -f k -C '' && wc k
Generating public/private ed25519 key pair.
k already exists.
Overwrite (y/n)? Your identification has been saved in k
Your public key has been saved in k.pub
...
  7  13 387 k

The minimum length that I could obtain was 387 characters with several tries and no comment and passphrase. I used the following patch to make the code work for me:

--- commands/ssh_agent.py.orig  2024-06-19 18:27:33.631995398 +0200
+++ commands/ssh_agent.py       2024-06-20 05:31:03.513420266 +0200
@@ -101,6 +101,8 @@

 SSH_AGENT_CONSTRAIN_LIFETIME = 1

+SSH_MIN_KEY_LENGTH = 387
+

 def delete_ssh_key(delete_request):
     try:
@@ -207,7 +209,7 @@
     # check notes field
     if not private_key:
         if isinstance(record, (vault.PasswordRecord, vault.TypedRecord)):
-            if 444 <= len(record.notes) < 4000:
+            if SSH_MIN_KEY_LENGTH <= len(record.notes) < 4000:
                 header, _, _ = record.notes.partition('\n')
                 if is_private_key(header):
                     private_key = record.notes
@@ -217,14 +219,14 @@
         if isinstance(record, vault.TypedRecord):
             try_values = (x.get_default_value() for x in itertools.chain(record.fields, record.custom) if x.type in ('text', 'multiline', 'secret', 'note'))
             for value in (x for x in try_values if x):
-                if isinstance(value, str) and 444 <= len(value) < 4000:
+                if isinstance(value, str) and SSH_MIN_KEY_LENGTH <= len(value) < 4000:
                     header, _, _ = value.partition('\n')
                     if is_private_key(header):
                         private_key = value
                         break
         elif isinstance(record, vault.PasswordRecord):
             for value in (x.value for x in record.custom if x.value):
-                if isinstance(value, str) and 444 <= len(value) < 4000:
+                if isinstance(value, str) and SSH_MIN_KEY_LENGTH <= len(value) < 4000:
                     header, _, _ = value.partition('\n')
                     if is_private_key(header):
                         private_key = value
@@ -244,7 +246,7 @@
                         if file_record.name and file_record.name != file_record.title:
                             names.append(file_record.name)
                         if any(True for x in names if is_private_key_name(x)):
-                            if 444 <= file_record.size < 4000:
+                            if SSH_MIN_KEY_LENGTH <= file_record.size < 4000:
                                 key_file_uids.append(file_uid)
                 if len(key_file_uids) == 1:
                     download_rq = next(attachment.prepare_attachment_download(params, key_file_uids[0]), None)
@@ -258,7 +260,7 @@
                     if atta.name and atta.title != atta.name:
                         names.append(atta.name)
                     if any(True for x in names if is_private_key_name(x)):
-                        if 444 <= atta.size < 4000:
+                        if SSH_MIN_KEY_LENGTH <= atta.size < 4000:
                             key_attachment_ids.append(atta.id)
             if len(key_attachment_ids) == 1:
                 download_rq = next(attachment.prepare_attachment_download(params, record.record_uid, key_attachment_ids[0]), None)

Regards, Philipp

aaunario-keeper commented 2 weeks ago

Thanks for the heads-up. This issue should be fixed in the next Commander release.

On Wed, Jun 19, 2024 at 10:54 PM phlibi @.***> wrote:

Hi everyone,

I discovered that my ed25519 SSH private keys stored in PAM user records and generated using the automatic rotation feature are not loaded into the agent. The problem seems to be that there is a minimum assumed length of 444 characters, while the keys are shorter when there is no comment included and no passphrase set:

Comment and passphrase: 444 characters

$ yes | ssh-keygen -t ed25519 -N 'abc' -f k -C 'comment' && wc k Generating public/private ed25519 key pair. k already exists. Overwrite (y/n)? Your identification has been saved in k Your public key has been saved in k.pub The key fingerprint is: ... 8 14 444 k

Only passphrase:

$ yes | ssh-keygen -t ed25519 -N 'abc' -f k -C '' && wc k Generating public/private ed25519 key pair. k already exists. Overwrite (y/n)? Your identification has been saved in k Your public key has been saved in k.pub ... 8 14 444 k

Only comment: 399 characters (or more, depending on length of comment):

$ yes | ssh-keygen -t ed25519 -N '' -f k -C 'comment' && wc k Generating public/private ed25519 key pair. k already exists. Overwrite (y/n)? Your identification has been saved in k Your public key has been saved in k.pub ... 7 13 399 k

Neither comment nor passphrase: 387 characters

$ yes | ssh-keygen -t ed25519 -N '' -f k -C '' && wc k Generating public/private ed25519 key pair. k already exists. Overwrite (y/n)? Your identification has been saved in k Your public key has been saved in k.pub ... 7 13 387 k

The minimum length that I could obtain was 387 characters with several tries and no comment and passphrase. I used the following patch to make the code work for me:

--- commands/ssh_agent.py.orig 2024-06-19 18:27:33.631995398 +0200+++ commands/ssh_agent.py 2024-06-20 05:31:03.513420266 +0200@@ -101,6 +101,8 @@

SSH_AGENT_CONSTRAIN_LIFETIME = 1 +SSH_MIN_KEY_LENGTH = 387+

def delete_ssh_key(delete_request): try:@@ -207,7 +209,7 @@

check notes field

 if not private_key:
     if isinstance(record, (vault.PasswordRecord, vault.TypedRecord)):-            if 444 <= len(record.notes) < 4000:+            if SSH_MIN_KEY_LENGTH <= len(record.notes) < 4000:
             header, _, _ = record.notes.partition('\n')
             if is_private_key(header):
                 private_key = record.notes@@ -217,14 +219,14 @@
     if isinstance(record, vault.TypedRecord):
         try_values = (x.get_default_value() for x in itertools.chain(record.fields, record.custom) if x.type in ('text', 'multiline', 'secret', 'note'))
         for value in (x for x in try_values if x):-                if isinstance(value, str) and 444 <= len(value) < 4000:+                if isinstance(value, str) and SSH_MIN_KEY_LENGTH <= len(value) < 4000:
                 header, _, _ = value.partition('\n')
                 if is_private_key(header):
                     private_key = value
                     break
     elif isinstance(record, vault.PasswordRecord):
         for value in (x.value for x in record.custom if x.value):-                if isinstance(value, str) and 444 <= len(value) < 4000:+                if isinstance(value, str) and SSH_MIN_KEY_LENGTH <= len(value) < 4000:
                 header, _, _ = value.partition('\n')
                 if is_private_key(header):
                     private_key = value@@ -244,7 +246,7 @@
                     if file_record.name and file_record.name != file_record.title:
                         names.append(file_record.name)
                     if any(True for x in names if is_private_key_name(x)):-                            if 444 <= file_record.size < 4000:+                            if SSH_MIN_KEY_LENGTH <= file_record.size < 4000:
                             key_file_uids.append(file_uid)
             if len(key_file_uids) == 1:
                 download_rq = next(attachment.prepare_attachment_download(params, key_file_uids[0]), None)@@ -258,7 +260,7 @@
                 if atta.name and atta.title != atta.name:
                     names.append(atta.name)
                 if any(True for x in names if is_private_key_name(x)):-                        if 444 <= atta.size < 4000:+                        if SSH_MIN_KEY_LENGTH <= atta.size < 4000:
                         key_attachment_ids.append(atta.id)
         if len(key_attachment_ids) == 1:
             download_rq = next(attachment.prepare_attachment_download(params, record.record_uid, key_attachment_ids[0]), None)

Regards, Philipp

— Reply to this email directly, view it on GitHub https://github.com/Keeper-Security/Commander/issues/1254, or unsubscribe https://github.com/notifications/unsubscribe-auth/AZDPG4MAZQTE4THDDKMVUMLZIJG7DAVCNFSM6AAAAABJTDWMCCVHI2DSMVQWIX3LMV43ASLTON2WKOZSGM3DGNBUG42TQOA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

--

Ayrris Aunario | Senior Software Engineer

Mobile 773.986.1194

This email is confidential and is intended for the recipient(s) addressed herein

aaunario-keeper commented 1 week ago

Closing this issue, as it has been resolved as of release v16.11.2, available here.

phlibi commented 1 week ago

Great, thanks for the quick fix!