Page MenuHomePhabricator

D8102.diff
No OneTemporary

D8102.diff

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

Mime Type
text/plain
Expires
Fri, Nov 1, 3:32 AM (1 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6747078
Default Alt Text
D8102.diff (14 KB)

Event Timeline