From 3dea92081bbe25af69b596ca6c6c04c139bae371 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 13 May 2020 07:59:06 -0700 Subject: [PATCH] Fix an issue where passphrase-protected private keys were stored without discarding passphrases Summary: Ref T13454. See . After changes to distinguish between invalid and passphrase-protected keys, SSH private key management code incorrectly uses "-y ..." ("print public key") when it means "-p ..." ("modify input file, removing passphrase"). This results in the command having no effect, and Passphrase stores the raw input credential, not the stripped version. We can't recover the keys because we don't store the passphrase, so no migration here is really possible. (We could add more code to detect this case, but it's presumably rare.) Also, correct the behavior of the "Show Public Key" action: this is available for users who can see the credential and does not require edit permission. Test Plan: - Created a new credential with a passphrase, then showed the public key. Maniphest Tasks: T13006, T13454 Differential Revision: https://secure.phabricator.com/D21245 --- src/applications/auth/sshkey/PhabricatorAuthSSHPrivateKey.php | 2 +- .../controller/PassphraseCredentialViewController.php | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/applications/auth/sshkey/PhabricatorAuthSSHPrivateKey.php b/src/applications/auth/sshkey/PhabricatorAuthSSHPrivateKey.php index ea601a3096..ad26da8ebb 100644 --- a/src/applications/auth/sshkey/PhabricatorAuthSSHPrivateKey.php +++ b/src/applications/auth/sshkey/PhabricatorAuthSSHPrivateKey.php @@ -80,7 +80,7 @@ final class PhabricatorAuthSSHPrivateKey extends Phobject { if (!$err) { if ($passphrase) { execx( - 'ssh-keygen -y -P %P -N %s -f %R', + 'ssh-keygen -p -P %P -N %s -f %R', $passphrase, '', $tmp); diff --git a/src/applications/passphrase/controller/PassphraseCredentialViewController.php b/src/applications/passphrase/controller/PassphraseCredentialViewController.php index 6688bef285..a5147295c8 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialViewController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialViewController.php @@ -154,7 +154,6 @@ final class PassphraseCredentialViewController extends PassphraseController { ->setName(pht('Show Public Key')) ->setIcon('fa-download') ->setHref($this->getApplicationURI("public/{$id}/")) - ->setDisabled(!$can_edit) ->setWorkflow(true)); }