Lock down some config options

Summary: This is just a general review of config options, to reduce the amount of damage a rogue administrator (without host access) can do. In particular: - Fix some typos. - Lock down some options which would potentially let a rogue administrator do something sketchy. - Most of the new locks relate to having them register a new service account, then redirect services to their account. This potentially allows them to read email. - Lock down some general disk stuff, which could be troublesome in combination with other vulnerabilities. Test Plan: - Read through config options. - Tried to think about how to do evil things with each one. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Differential Revision: https://secure.phabricator.com/D8928
2024-11-10 08:52:39 +01:00 · 2014-05-01 10:23:49 -07:00 · 2014-05-01 10:23:49 -07:00 · 7145587df7
commit 7145587df7
parent 68023e64a9
12 changed files with 42 additions and 16 deletions
--- a/src/applications/config/option/PhabricatorAWSConfigOptions.php
+++ b/src/applications/config/option/PhabricatorAWSConfigOptions.php
@ -14,16 +14,19 @@ final class PhabricatorAWSConfigOptions
  public function getOptions() {
    return array(
      $this->newOption('amazon-ses.access-key', 'string', null)
+        ->setLocked(true)
        ->setDescription(pht('Access key for Amazon SES.')),
      $this->newOption('amazon-ses.secret-key', 'string', null)
        ->setMasked(true)
        ->setDescription(pht('Secret key for Amazon SES.')),
      $this->newOption('amazon-s3.access-key', 'string', null)
+        ->setLocked(true)
        ->setDescription(pht('Access key for Amazon S3.')),
      $this->newOption('amazon-s3.secret-key', 'string', null)
        ->setMasked(true)
        ->setDescription(pht('Secret key for Amazon S3.')),
      $this->newOption('amazon-s3.endpoint', 'string', null)
+        ->setLocked(true)
        ->setDescription(
          pht(
            'Explicit S3 endpoint to use. Leave empty to have Phabricator '.
@ -31,6 +34,7 @@ final class PhabricatorAWSConfigOptions
        ->addExample(null, 'Use default endpoint')
        ->addExample('s3.amazon.com', 'Use specific endpoint'),
      $this->newOption('amazon-ec2.access-key', 'string', null)
+        ->setLocked(true)
        ->setDescription(pht('Access key for Amazon EC2.')),
      $this->newOption('amazon-ec2.secret-key', 'string', null)
        ->setMasked(true)
--- a/src/applications/config/option/PhabricatorCoreConfigOptions.php
+++ b/src/applications/config/option/PhabricatorCoreConfigOptions.php
@ -92,6 +92,9 @@ final class PhabricatorCoreConfigOptions
            pht('Install Beta Applications'),
            pht('Uninstall Beta Applications')
          ))
+        ->setSummary(
+          pht(
+            'Install applications which are still under development.'))
        ->setDescription(
          pht(
            "Phabricator includes 'Beta' applications which are in an early ".
@ -109,7 +112,7 @@ final class PhabricatorCoreConfigOptions
            pht('Shenanigans'), // That should be interesting to translate. :P
          ))
        ->setSummary(
-          pht("Should Phabricator be serious?"))
+          pht("Allows you to remove levity and jokes from the UI."))
        ->setDescription(
          pht(
            'By default, Phabricator includes some flavor text in the UI, '.
@ -135,6 +138,7 @@ final class PhabricatorCoreConfigOptions
            "The current value of PATH after configuration is applied is:\n\n".
            "  lang=text\n".
            "  %s", $path))
+        ->setLocked(true)
        ->addExample('/usr/local/bin', pht('Add One Path'))
        ->addExample("/usr/bin\n/usr/local/bin", pht('Add Multiple Paths')),
      $this->newOption('config.lock', 'set', array())
--- a/src/applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php
+++ b/src/applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php
@ -32,6 +32,7 @@ final class PhabricatorExtendingPhabricatorConfigOptions
         'aphront.default-application-configuration-class',
         'class',
         'AphrontDefaultApplicationConfiguration')
+        ->setLocked(true)
        ->setBaseClass('AphrontApplicationConfiguration')
        // TODO: This could probably use some better documentation.
        ->setDescription(pht("Application configuration class.")),
--- a/src/applications/config/option/PhabricatorMailgunConfigOptions.php
+++ b/src/applications/config/option/PhabricatorMailgunConfigOptions.php
@ -13,14 +13,15 @@ final class PhabricatorMailgunConfigOptions

  public function getOptions() {
    return array(
-      $this->newOption('mailgun.api-key', 'string', null)
-        ->setMasked(true)
-        ->setDescription(pht('Mailgun API key.')),
      $this->newOption('mailgun.domain', 'string', null)
+        ->setLocked(true)
        ->setDescription(
          pht(
            'Mailgun domain name. See https://mailgun.com/cp/domains'))
        ->addExample('mycompany.com', 'Use specific domain'),
+      $this->newOption('mailgun.api-key', 'string', null)
+        ->setMasked(true)
+        ->setDescription(pht('Mailgun API key.')),
    );

  }
--- a/src/applications/config/option/PhabricatorPHDConfigOptions.php
+++ b/src/applications/config/option/PhabricatorPHDConfigOptions.php
@ -42,6 +42,7 @@ final class PhabricatorPHDConfigOptions
            "mode with 'phd debug' are always launched in verbose mode. See ".
            "also 'phd.trace'.")),
      $this->newOption('phd.user', 'string', null)
+        ->setLocked(true)
        ->setSummary(pht("System user to run daemons as."))
        ->setDescription(
          pht(
--- a/src/applications/config/option/PhabricatorPHPMailerConfigOptions.php
+++ b/src/applications/config/option/PhabricatorPHPMailerConfigOptions.php
@ -14,6 +14,7 @@ final class PhabricatorPHPMailerConfigOptions
  public function getOptions() {
    return array(
      $this->newOption('phpmailer.mailer', 'string', 'smtp')
+        ->setLocked(true)
        ->setSummary(pht("Configure mailer used by PHPMailer."))
        ->setDescription(
          pht(
@ -23,11 +24,14 @@ final class PhabricatorPHPMailerConfigOptions
            "You need it when you want to use SMTP instead of sendmail as the ".
            "mailer.")),
      $this->newOption('phpmailer.smtp-host', 'string', null)
+        ->setLocked(true)
        ->setDescription(pht('Host for SMTP.')),
      $this->newOption('phpmailer.smtp-port', 'int', 25)
+        ->setLocked(true)
        ->setDescription(pht('Port for SMTP.')),
      // TODO: Implement "enum"? Valid values are empty, 'tls', or 'ssl'.
      $this->newOption('phpmailer.smtp-protocol', 'string', null)
+        ->setLocked(true)
        ->setSummary(pht('Configure TLS or SSL for SMTP.'))
        ->setDescription(
          pht(
@ -35,6 +39,7 @@ final class PhabricatorPHPMailerConfigOptions
            "'ssl' to use TLS or SSL, respectively. Leave it blank for ".
            "vanilla SMTP. If you're sending via Gmail, set it to 'ssl'.")),
      $this->newOption('phpmailer.smtp-user', 'string', null)
+        ->setLocked(true)
        ->setDescription(pht('Username for SMTP.')),
      $this->newOption('phpmailer.smtp-password', 'string', null)
        ->setMasked(true)
--- a/src/applications/config/option/PhabricatorSecurityConfigOptions.php
+++ b/src/applications/config/option/PhabricatorSecurityConfigOptions.php
@ -16,6 +16,7 @@ final class PhabricatorSecurityConfigOptions

    return array(
      $this->newOption('security.alternate-file-domain', 'string', null)
+        ->setLocked(true)
        ->setSummary(pht("Alternate domain to serve files from."))
        ->setDescription(
          pht(
@ -43,6 +44,7 @@ final class PhabricatorSecurityConfigOptions
        'string',
        '[D\t~Y7eNmnQGJ;rnH6aF;m2!vJ8@v8C=Cs:aQS\.Qw')
        ->setMasked(true)
+        ->setLocked(true)
        ->setSummary(
          pht("Key for HMAC digests."))
        ->setDescription(
@ -85,6 +87,7 @@ final class PhabricatorSecurityConfigOptions
        'string',
        '0b7ec0592e0a2829d8b71df2fa269b2c6172eca3')
        ->setMasked(true)
+        ->setLocked(true)
        ->setSummary(
          pht("Hashed with other inputs to generate CSRF tokens."))
        ->setDescription(
@ -100,6 +103,7 @@ final class PhabricatorSecurityConfigOptions
         'string',
         '5ce3e7e8787f6e40dfae861da315a5cdf1018f12')
        ->setMasked(true)
+        ->setLocked(true)
        ->setSummary(
          pht("Hashed with other inputs to generate mail tokens."))
        ->setDescription(
@ -191,17 +195,18 @@ final class PhabricatorSecurityConfigOptions
            "referrers to YouTube) and is pretty silly (but sort of ".
            "awesome).")),
        $this->newOption('security.allow-outbound-http', 'bool', true)
-        ->setBoolOptions(
-          array(
-            pht("Allow"),
-            pht("Disallow"),
-          ))
-        ->setSummary(
-          pht("Allow outbound HTTP requests"))
-        ->setDescription(
-          pht(
-            "If you enable this, you are allowing Phabricator to potentially ".
-            "make requests to external servers.")),
+          ->setBoolOptions(
+            array(
+              pht("Allow"),
+              pht("Disallow"),
+            ))
+          ->setLocked(true)
+          ->setSummary(
+            pht("Allow outbound HTTP requests"))
+          ->setDescription(
+            pht(
+              "If you enable this, you are allowing Phabricator to ".
+              "potentially make requests to external servers.")),
    );
  }

--- a/src/applications/config/option/PhabricatorSendGridConfigOptions.php
+++ b/src/applications/config/option/PhabricatorSendGridConfigOptions.php
@ -14,6 +14,7 @@ final class PhabricatorSendGridConfigOptions
  public function getOptions() {
    return array(
      $this->newOption('sendgrid.api-user', 'string', null)
+        ->setLocked(true)
        ->setDescription(pht('SendGrid API username.')),
      $this->newOption('sendgrid.api-key', 'string', null)
        ->setMasked(true)
--- a/src/applications/files/config/PhabricatorFilesConfigOptions.php
+++ b/src/applications/files/config/PhabricatorFilesConfigOptions.php
@ -118,6 +118,7 @@ final class PhabricatorFilesConfigOptions
            'Configure the largest file which will be put into the MySQL '.
            'storage engine.')),
      $this->newOption('storage.local-disk.path', 'string', null)
+        ->setLocked(true)
        ->setSummary(pht('Local storage disk path.'))
        ->setDescription(
          pht(
@ -190,7 +191,7 @@ final class PhabricatorFilesConfigOptions
           pht('Disable')
         ))->setDescription(
             pht("This option will enable animated gif images".
-                  "to be set as profile pictures. The \'convert\' binary ".
+                  "to be set as profile pictures. The 'convert' binary ".
                  "should be available to the webserver for this to work")),

    );
--- a/src/applications/phame/config/PhabricatorPhameConfigOptions.php
+++ b/src/applications/phame/config/PhabricatorPhameConfigOptions.php
@ -19,6 +19,7 @@ final class PhabricatorPhameConfigOptions
        array(
          'externals/skins/',
        ))
+        ->setLocked(true)
        ->setDescription(
          pht('List of directories where Phame will look for skins.')),
    );
--- a/src/applications/phortune/option/PhabricatorPhortuneConfigOptions.php
+++ b/src/applications/phortune/option/PhabricatorPhortuneConfigOptions.php
@ -61,6 +61,7 @@ final class PhabricatorPhortuneConfigOptions
        ->setHidden(true)
        ->setDescription(pht('WePay access token.')),
      $this->newOption('phortune.wepay.account-id', 'string', null)
+        ->setLocked(true)
        ->setHidden(true)
        ->setDescription(pht('WePay account ID.')),
    );
--- a/src/applications/repository/PhabricatorRepositoryConfigOptions.php
+++ b/src/applications/repository/PhabricatorRepositoryConfigOptions.php
@ -17,6 +17,7 @@ final class PhabricatorRepositoryConfigOptions
  public function getOptions() {
    return array(
      $this->newOption('repository.default-local-path', 'string', '/var/repo/')
+        ->setLocked(true)
        ->setSummary(
          pht("Default location to store local copies of repositories."))
        ->setDescription(