Cache generation of the SSH authentication keyfile for sshd

Summary:
Ref T11469. This isn't directly related, but has been on my radar for a while: building SSH keyfiles (particular for installs with a lot of keys, like ours) can be fairly slow.

At least one cluster instance is making multiple clone requests per second. While that should probably be rate limited separately, caching this should mitigate the impact of these requests.

This is pretty straightforward to cache since it's exactly the same every time, and only changes when users modify SSH keys (which is rare).

Test Plan:
  - Ran `bin/auth-ssh`, saw authfile generate.
  - Ran it again, saw it read from cache.
  - Changed an SSH key.
  - Ran it again, saw it regenerate.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11469

Differential Revision: https://secure.phabricator.com/D16744

scripts/ssh/ssh-auth.php

@@ -4,74 +4,84 @@
 $root = dirname(dirname(dirname(__FILE__)));
 require_once $root.'/scripts/__init_script__.php';
 
-$keys = id(new PhabricatorAuthSSHKeyQuery())
+$cache = PhabricatorCaches::getMutableCache();
-  ->setViewer(PhabricatorUser::getOmnipotentUser())
+$authfile_key = PhabricatorAuthSSHKeyQuery::AUTHFILE_CACHEKEY;
-  ->withIsActive(true)
+$authfile = $cache->getKey($authfile_key);
-  ->execute();
 
-if (!$keys) {
+if ($authfile === null) {
-  echo pht('No keys found.')."\n";
+  $keys = id(new PhabricatorAuthSSHKeyQuery())
-  exit(1);
+    ->setViewer(PhabricatorUser::getOmnipotentUser())
-}
+    ->withIsActive(true)
+    ->execute();
 
-$bin = $root.'/bin/ssh-exec';
+  if (!$keys) {
-foreach ($keys as $ssh_key) {
+    echo pht('No keys found.')."\n";
-  $key_argv = array();
+    exit(1);
-  $object = $ssh_key->getObject();
+  }
-  if ($object instanceof PhabricatorUser) {
+
-    $key_argv[] = '--phabricator-ssh-user';
+  $bin = $root.'/bin/ssh-exec';
-    $key_argv[] = $object->getUsername();
+  foreach ($keys as $ssh_key) {
-  } else if ($object instanceof AlmanacDevice) {
+    $key_argv = array();
-    if (!$ssh_key->getIsTrusted()) {
+    $object = $ssh_key->getObject();
-      // If this key is not a trusted device key, don't allow SSH
+    if ($object instanceof PhabricatorUser) {
-      // authentication.
+      $key_argv[] = '--phabricator-ssh-user';
+      $key_argv[] = $object->getUsername();
+    } else if ($object instanceof AlmanacDevice) {
+      if (!$ssh_key->getIsTrusted()) {
+        // If this key is not a trusted device key, don't allow SSH
+        // authentication.
+        continue;
+      }
+      $key_argv[] = '--phabricator-ssh-device';
+      $key_argv[] = $object->getName();
+    } else {
+      // We don't know what sort of key this is; don't permit SSH auth.
       continue;
     }
-    $key_argv[] = '--phabricator-ssh-device';
+
-    $key_argv[] = $object->getName();
+    $key_argv[] = '--phabricator-ssh-key';
-  } else {
+    $key_argv[] = $ssh_key->getID();
-    // We don't know what sort of key this is; don't permit SSH auth.
+
-    continue;
+    $cmd = csprintf('%s %Ls', $bin, $key_argv);
+
+    $instance = PhabricatorEnv::getEnvConfig('cluster.instance');
+    if (strlen($instance)) {
+      $cmd = csprintf('PHABRICATOR_INSTANCE=%s %C', $instance, $cmd);
+    }
+
+    // This is additional escaping for the SSH 'command="..."' string.
+    $cmd = addcslashes($cmd, '"\\');
+
+    // Strip out newlines and other nonsense from the key type and key body.
+
+    $type = $ssh_key->getKeyType();
+    $type = preg_replace('@[\x00-\x20]+@', '', $type);
+    if (!strlen($type)) {
+      continue;
+    }
+
+    $key = $ssh_key->getKeyBody();
+    $key = preg_replace('@[\x00-\x20]+@', '', $key);
+    if (!strlen($key)) {
+      continue;
+    }
+
+    $options = array(
+      'command="'.$cmd.'"',
+      'no-port-forwarding',
+      'no-X11-forwarding',
+      'no-agent-forwarding',
+      'no-pty',
+    );
+    $options = implode(',', $options);
+
+    $lines[] = $options.' '.$type.' '.$key."\n";
   }
 
-  $key_argv[] = '--phabricator-ssh-key';
+  $authfile = implode('', $lines);
-  $key_argv[] = $ssh_key->getID();
+  $ttl = phutil_units('24 hours in seconds');
-
+  $cache->setKey($authfile_key, $authfile, $ttl);
-  $cmd = csprintf('%s %Ls', $bin, $key_argv);
-
-  $instance = PhabricatorEnv::getEnvConfig('cluster.instance');
-  if (strlen($instance)) {
-    $cmd = csprintf('PHABRICATOR_INSTANCE=%s %C', $instance, $cmd);
-  }
-
-  // This is additional escaping for the SSH 'command="..."' string.
-  $cmd = addcslashes($cmd, '"\\');
-
-  // Strip out newlines and other nonsense from the key type and key body.
-
-  $type = $ssh_key->getKeyType();
-  $type = preg_replace('@[\x00-\x20]+@', '', $type);
-  if (!strlen($type)) {
-    continue;
-  }
-
-  $key = $ssh_key->getKeyBody();
-  $key = preg_replace('@[\x00-\x20]+@', '', $key);
-  if (!strlen($key)) {
-    continue;
-  }
-
-  $options = array(
-    'command="'.$cmd.'"',
-    'no-port-forwarding',
-    'no-X11-forwarding',
-    'no-agent-forwarding',
-    'no-pty',
-  );
-  $options = implode(',', $options);
-
-  $lines[] = $options.' '.$type.' '.$key."\n";
 }
 
-echo implode('', $lines);
+echo $authfile;
 exit(0);

src/applications/auth/editor/PhabricatorAuthSSHKeyEditor.php

@@ -191,6 +191,20 @@ final class PhabricatorAuthSSHKeyEditor
     return 'ssh-key-'.$object->getPHID();
   }
 
+  protected function applyFinalEffects(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+
+    // After making any change to an SSH key, drop the authfile cache so it
+    // is regenerated the next time anyone authenticates.
+    $cache = PhabricatorCaches::getMutableCache();
+    $authfile_key = PhabricatorAuthSSHKeyQuery::AUTHFILE_CACHEKEY;
+    $cache->deleteKey($authfile_key);
+
+    return $xactions;
+  }
+
+
   protected function getMailTo(PhabricatorLiskDAO $object) {
     return $object->getObject()->getSSHKeyNotifyPHIDs();
   }

src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php

@@ -3,6 +3,8 @@
 final class PhabricatorAuthSSHKeyQuery
   extends PhabricatorCursorPagedPolicyAwareQuery {
 
+  const AUTHFILE_CACHEKEY = 'ssh.authfile';
+
   private $ids;
   private $phids;
   private $objectPHIDs;

src/applications/cache/PhabricatorCaches.php

@@ -99,6 +99,23 @@ final class PhabricatorCaches extends Phobject {
     return $caches;
   }
 
+  public static function getMutableCache() {
+    static $cache;
+    if (!$cache) {
+      $caches = self::buildMutableCaches();
+      $cache = self::newStackFromCaches($caches);
+    }
+    return $cache;
+  }
+
+  private static function buildMutableCaches() {
+    $caches = array();
+
+    $caches[] = new PhabricatorKeyValueDatabaseCache();
+
+    return $caches;
+  }
+
 
 /* -(  Repository Graph Cache  )--------------------------------------------- */
 

src/applications/cache/PhabricatorKeyValueDatabaseCache.php

@@ -98,7 +98,7 @@ final class PhabricatorKeyValueDatabaseCache
         $this->establishConnection('w'),
         'DELETE FROM %T WHERE cacheKeyHash IN (%Ls)',
         $this->getTableName(),
-        $keys);
+        $map);
     }
 
     return $this;