Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F14005282
D8102.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
14 KB
Referenced Files
None
Subscribers
None
D8102.diff
View Options
Index: src/applications/passphrase/controller/PassphraseCredentialCreateController.php
===================================================================
--- src/applications/passphrase/controller/PassphraseCredentialCreateController.php
+++ src/applications/passphrase/controller/PassphraseCredentialCreateController.php
@@ -6,7 +6,7 @@
$request = $this->getRequest();
$viewer = $request->getUser();
- $types = PassphraseCredentialType::getAllTypes();
+ $types = PassphraseCredentialType::getAllCreateableTypes();
$types = mpull($types, null, 'getCredentialType');
$types = msort($types, 'getCredentialTypeName');
Index: src/applications/passphrase/controller/PassphraseCredentialEditController.php
===================================================================
--- src/applications/passphrase/controller/PassphraseCredentialEditController.php
+++ src/applications/passphrase/controller/PassphraseCredentialEditController.php
@@ -32,6 +32,11 @@
throw new Exception(pht('Credential has invalid type "%s"!', $type));
}
+ if (!$type->isCreateable()) {
+ throw new Exception(
+ pht('Credential has noncreateable type "%s"!', $type));
+ }
+
$is_new = false;
} else {
$type_const = $request->getStr('type');
@@ -65,93 +70,123 @@
$v_secret = $credential->getSecretID() ? str_repeat($bullet, 32) : null;
$validation_exception = null;
+ $errors = array();
+ $e_password = null;
if ($request->isFormPost()) {
+
$v_name = $request->getStr('name');
$v_desc = $request->getStr('description');
$v_username = $request->getStr('username');
- $v_secret = $request->getStr('secret');
$v_view_policy = $request->getStr('viewPolicy');
$v_edit_policy = $request->getStr('editPolicy');
- $type_name = PassphraseCredentialTransaction::TYPE_NAME;
- $type_desc = PassphraseCredentialTransaction::TYPE_DESCRIPTION;
- $type_username = PassphraseCredentialTransaction::TYPE_USERNAME;
- $type_destroy = PassphraseCredentialTransaction::TYPE_DESTROY;
- $type_secret_id = PassphraseCredentialTransaction::TYPE_SECRET_ID;
- $type_view_policy = PhabricatorTransactions::TYPE_VIEW_POLICY;
- $type_edit_policy = PhabricatorTransactions::TYPE_EDIT_POLICY;
-
- $xactions = array();
-
- $xactions[] = id(new PassphraseCredentialTransaction())
- ->setTransactionType($type_name)
- ->setNewValue($v_name);
-
- $xactions[] = id(new PassphraseCredentialTransaction())
- ->setTransactionType($type_desc)
- ->setNewValue($v_desc);
+ $v_secret = $request->getStr('secret');
+ $v_password = $request->getStr('password');
+ $v_decrypt = $v_secret;
+
+ $env_secret = new PhutilOpaqueEnvelope($v_secret);
+ $env_password = new PhutilOpaqueEnvelope($v_password);
+
+ if ($type->requiresPassword($env_secret)) {
+ if (strlen($v_password)) {
+ $v_decrypt = $type->decryptSecret($env_secret, $env_password);
+ if ($v_decrypt === null) {
+ $e_password = pht('Incorrect');
+ $errors[] = pht(
+ 'This key requires a password, but the password you provided '.
+ 'is incorrect.');
+ } else {
+ $v_decrypt = $v_decrypt->openEnvelope();
+ }
+ } else {
+ $e_password = pht('Required');
+ $errors[] = pht(
+ 'This key requires a password. You must provide the password '.
+ 'for the key.');
+ }
+ }
- $xactions[] = id(new PassphraseCredentialTransaction())
- ->setTransactionType($type_username)
- ->setNewValue($v_username);
+ if (!$errors) {
+ $type_name = PassphraseCredentialTransaction::TYPE_NAME;
+ $type_desc = PassphraseCredentialTransaction::TYPE_DESCRIPTION;
+ $type_username = PassphraseCredentialTransaction::TYPE_USERNAME;
+ $type_destroy = PassphraseCredentialTransaction::TYPE_DESTROY;
+ $type_secret_id = PassphraseCredentialTransaction::TYPE_SECRET_ID;
+ $type_view_policy = PhabricatorTransactions::TYPE_VIEW_POLICY;
+ $type_edit_policy = PhabricatorTransactions::TYPE_EDIT_POLICY;
- $xactions[] = id(new PassphraseCredentialTransaction())
- ->setTransactionType($type_view_policy)
- ->setNewValue($v_view_policy);
+ $xactions = array();
- $xactions[] = id(new PassphraseCredentialTransaction())
- ->setTransactionType($type_edit_policy)
- ->setNewValue($v_edit_policy);
+ $xactions[] = id(new PassphraseCredentialTransaction())
+ ->setTransactionType($type_name)
+ ->setNewValue($v_name);
- // Open a transaction in case we're writing a new secret; this limits
- // the amount of code which handles secret plaintexts.
- $credential->openTransaction();
+ $xactions[] = id(new PassphraseCredentialTransaction())
+ ->setTransactionType($type_desc)
+ ->setNewValue($v_desc);
- $min_secret = str_replace($bullet, '', trim($v_secret));
- if (strlen($min_secret)) {
- // If the credential was previously destroyed, restore it when it is
- // edited if a secret is provided.
$xactions[] = id(new PassphraseCredentialTransaction())
- ->setTransactionType($type_destroy)
- ->setNewValue(0);
+ ->setTransactionType($type_username)
+ ->setNewValue($v_username);
- $new_secret = id(new PassphraseSecret())
- ->setSecretData($v_secret)
- ->save();
$xactions[] = id(new PassphraseCredentialTransaction())
- ->setTransactionType($type_secret_id)
- ->setNewValue($new_secret->getID());
- }
+ ->setTransactionType($type_view_policy)
+ ->setNewValue($v_view_policy);
- try {
- $editor = id(new PassphraseCredentialTransactionEditor())
- ->setActor($viewer)
- ->setContinueOnNoEffect(true)
- ->setContentSourceFromRequest($request)
- ->applyTransactions($credential, $xactions);
-
- $credential->saveTransaction();
-
- if ($request->isAjax()) {
- return id(new AphrontAjaxResponse())->setContent(
- array(
- 'phid' => $credential->getPHID(),
- 'name' => 'K'.$credential->getID().' '.$credential->getName(),
- ));
- } else {
- return id(new AphrontRedirectResponse())
- ->setURI('/K'.$credential->getID());
+ $xactions[] = id(new PassphraseCredentialTransaction())
+ ->setTransactionType($type_edit_policy)
+ ->setNewValue($v_edit_policy);
+
+ // Open a transaction in case we're writing a new secret; this limits
+ // the amount of code which handles secret plaintexts.
+ $credential->openTransaction();
+
+ $min_secret = str_replace($bullet, '', trim($v_decrypt));
+ if (strlen($min_secret)) {
+ // If the credential was previously destroyed, restore it when it is
+ // edited if a secret is provided.
+ $xactions[] = id(new PassphraseCredentialTransaction())
+ ->setTransactionType($type_destroy)
+ ->setNewValue(0);
+
+ $new_secret = id(new PassphraseSecret())
+ ->setSecretData($v_decrypt)
+ ->save();
+ $xactions[] = id(new PassphraseCredentialTransaction())
+ ->setTransactionType($type_secret_id)
+ ->setNewValue($new_secret->getID());
}
- } catch (PhabricatorApplicationTransactionValidationException $ex) {
- $credential->killTransaction();
-
- $validation_exception = $ex;
- $e_name = $ex->getShortMessage($type_name);
- $e_username = $ex->getShortMessage($type_username);
-
- $credential->setViewPolicy($v_view_policy);
- $credential->setEditPolicy($v_edit_policy);
+ try {
+ $editor = id(new PassphraseCredentialTransactionEditor())
+ ->setActor($viewer)
+ ->setContinueOnNoEffect(true)
+ ->setContentSourceFromRequest($request)
+ ->applyTransactions($credential, $xactions);
+
+ $credential->saveTransaction();
+
+ if ($request->isAjax()) {
+ return id(new AphrontAjaxResponse())->setContent(
+ array(
+ 'phid' => $credential->getPHID(),
+ 'name' => 'K'.$credential->getID().' '.$credential->getName(),
+ ));
+ } else {
+ return id(new AphrontRedirectResponse())
+ ->setURI('/K'.$credential->getID());
+ }
+ } catch (PhabricatorApplicationTransactionValidationException $ex) {
+ $credential->killTransaction();
+
+ $validation_exception = $ex;
+
+ $e_name = $ex->getShortMessage($type_name);
+ $e_username = $ex->getShortMessage($type_username);
+
+ $credential->setViewPolicy($v_view_policy);
+ $credential->setEditPolicy($v_edit_policy);
+ }
}
}
@@ -214,6 +249,14 @@
->setLabel($type->getSecretLabel())
->setValue($v_secret));
+ if ($type->shouldShowPasswordField()) {
+ $form->appendChild(
+ id(new AphrontFormPasswordControl())
+ ->setName('password')
+ ->setLabel($type->getPasswordLabel())
+ ->setError($e_password));
+ }
+
$crumbs = $this->buildApplicationCrumbs();
if ($is_new) {
@@ -230,10 +273,13 @@
}
if ($request->isAjax()) {
+ $errors = id(new AphrontErrorView())->setErrors($errors);
+
$dialog = id(new AphrontDialogView())
->setUser($viewer)
->setWidth(AphrontDialogView::WIDTH_FORM)
->setTitle($title)
+ ->appendChild($errors)
->appendChild($form)
->addSubmitButton(pht('Create Credential'))
->addCancelButton($this->getApplicationURI());
@@ -248,6 +294,7 @@
$box = id(new PHUIObjectBoxView())
->setHeaderText($header)
+ ->setFormErrors($errors)
->setValidationException($validation_exception)
->setForm($form);
Index: src/applications/passphrase/credentialtype/PassphraseCredentialType.php
===================================================================
--- src/applications/passphrase/credentialtype/PassphraseCredentialType.php
+++ src/applications/passphrase/credentialtype/PassphraseCredentialType.php
@@ -1,5 +1,8 @@
<?php
+/**
+ * @task password Managing Encryption Passwords
+ */
abstract class PassphraseCredentialType extends Phobject {
abstract public function getCredentialType();
@@ -19,10 +22,88 @@
return $types;
}
+ public static function getAllCreateableTypes() {
+ $types = self::getAllTypes();
+ foreach ($types as $key => $type) {
+ if (!$type->isCreateable()) {
+ unset($types[$key]);
+ }
+ }
+
+ return $types;
+ }
+
public static function getTypeByConstant($constant) {
$all = self::getAllTypes();
$all = mpull($all, null, 'getCredentialType');
return idx($all, $constant);
}
+
+ /**
+ * Can users create new credentials of this type?
+ *
+ * @return bool True if new credentials of this type can be created.
+ */
+ public function isCreateable() {
+ return true;
+ }
+
+
+/* -( Passwords )---------------------------------------------------------- */
+
+
+ /**
+ * Return true to show an additional "Password" field. This is used by
+ * SSH credentials to strip passwords off private keys.
+ *
+ * @return bool True if a password field should be shown to the user.
+ *
+ * @task password
+ */
+ public function shouldShowPasswordField() {
+ return false;
+ }
+
+
+ /**
+ * Return the label for the password field, if one is shown.
+ *
+ * @return string Human-readable field label.
+ *
+ * @task password
+ */
+ public function getPasswordLabel() {
+ return pht('Password');
+ }
+
+
+ /**
+ * Return true if the provided credental requires a password to decrypt.
+ *
+ * @param PhutilOpaqueEnvelope Credential secret value.
+ * @return bool True if the credential needs a password.
+ *
+ * @task password
+ */
+ public function requiresPassword(PhutilOpaqueEnvelope $secret) {
+ return false;
+ }
+
+
+ /**
+ * Return the decrypted credential secret, or `null` if the password does
+ * not decrypt the credential.
+ *
+ * @param PhutilOpaqueEnvelope Credential secret value.
+ * @param PhutilOpaqueEnvelope Credential password.
+ * @return
+ * @task password
+ */
+ public function decryptSecret(
+ PhutilOpaqueEnvelope $secret,
+ PhutilOpaqueEnvelope $password) {
+ return $secret;
+ }
+
}
Index: src/applications/passphrase/credentialtype/PassphraseCredentialTypeSSHPrivateKeyFile.php
===================================================================
--- src/applications/passphrase/credentialtype/PassphraseCredentialTypeSSHPrivateKeyFile.php
+++ src/applications/passphrase/credentialtype/PassphraseCredentialTypeSSHPrivateKeyFile.php
@@ -25,4 +25,12 @@
return new AphrontFormTextControl();
}
+ public function isCreateable() {
+ // This credential type exists to support historic repository configuration.
+ // We don't support creating new credentials with this type, since it does
+ // not scale and managing passwords is much more difficult than if we have
+ // the key text.
+ return false;
+ }
+
}
Index: src/applications/passphrase/credentialtype/PassphraseCredentialTypeSSHPrivateKeyText.php
===================================================================
--- src/applications/passphrase/credentialtype/PassphraseCredentialTypeSSHPrivateKeyText.php
+++ src/applications/passphrase/credentialtype/PassphraseCredentialTypeSSHPrivateKeyText.php
@@ -21,4 +21,46 @@
return pht('Private Key');
}
+ public function shouldShowPasswordField() {
+ return true;
+ }
+
+ public function getPasswordLabel() {
+ return pht('Password for Key');
+ }
+
+ public function requiresPassword(PhutilOpaqueEnvelope $secret) {
+ // According to the internet, this is the canonical test for an SSH private
+ // key with a password.
+ return preg_match('/ENCRYPTED/', $secret->openEnvelope());
+ }
+
+ public function decryptSecret(
+ PhutilOpaqueEnvelope $secret,
+ PhutilOpaqueEnvelope $password) {
+
+ $tmp = new TempFile();
+ Filesystem::writeFile($tmp, $secret->openEnvelope());
+
+ if (!Filesystem::binaryExists('ssh-keygen')) {
+ throw new Exception(
+ pht(
+ 'Decrypting SSH keys requires the `ssh-keygen` binary, but it '.
+ 'is not available in PATH. Either make it available or strip the '.
+ 'password fromt his SSH key manually before uploading it.'));
+ }
+
+ list($err, $stdout, $stderr) = exec_manual(
+ 'ssh-keygen -p -P %P -N %s -f %s',
+ $password,
+ '',
+ (string)$tmp);
+
+ if ($err) {
+ return null;
+ } else {
+ return new PhutilOpaqueEnvelope(Filesystem::readFile($tmp));
+ }
+ }
+
}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Mon, Oct 28, 10:40 AM (2 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6747078
Default Alt Text
D8102.diff (14 KB)
Attached To
Mode
D8102: Detect and prompt for passwords on SSH private keys, then strip them
Attached
Detach File
Event Timeline
Log In to Comment