From 7145587df7fb43fb269cb0fc0dd942fdd91ce914 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 1 May 2014 10:23:49 -0700 Subject: [PATCH] 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 --- .../option/PhabricatorAWSConfigOptions.php | 4 +++ .../option/PhabricatorCoreConfigOptions.php | 6 ++++- ...catorExtendingPhabricatorConfigOptions.php | 1 + .../PhabricatorMailgunConfigOptions.php | 7 ++--- .../option/PhabricatorPHDConfigOptions.php | 1 + .../PhabricatorPHPMailerConfigOptions.php | 5 ++++ .../PhabricatorSecurityConfigOptions.php | 27 +++++++++++-------- .../PhabricatorSendGridConfigOptions.php | 1 + .../config/PhabricatorFilesConfigOptions.php | 3 ++- .../config/PhabricatorPhameConfigOptions.php | 1 + .../PhabricatorPhortuneConfigOptions.php | 1 + .../PhabricatorRepositoryConfigOptions.php | 1 + 12 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/applications/config/option/PhabricatorAWSConfigOptions.php b/src/applications/config/option/PhabricatorAWSConfigOptions.php index a8e3bced1c..88042c410c 100644 --- 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) diff --git a/src/applications/config/option/PhabricatorCoreConfigOptions.php b/src/applications/config/option/PhabricatorCoreConfigOptions.php index d492a3b715..184def1554 100644 --- 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()) diff --git a/src/applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php b/src/applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php index aa8ea620f6..3c6678cf2f 100644 --- 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.")), diff --git a/src/applications/config/option/PhabricatorMailgunConfigOptions.php b/src/applications/config/option/PhabricatorMailgunConfigOptions.php index cc4fb56d6d..7c5b629227 100644 --- 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.')), ); } diff --git a/src/applications/config/option/PhabricatorPHDConfigOptions.php b/src/applications/config/option/PhabricatorPHDConfigOptions.php index 934684b91b..a15844f4d2 100644 --- 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( diff --git a/src/applications/config/option/PhabricatorPHPMailerConfigOptions.php b/src/applications/config/option/PhabricatorPHPMailerConfigOptions.php index d6ecd39844..c49ae1c2fd 100644 --- 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) diff --git a/src/applications/config/option/PhabricatorSecurityConfigOptions.php b/src/applications/config/option/PhabricatorSecurityConfigOptions.php index 5c5235074e..57a1921003 100644 --- 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.")), ); } diff --git a/src/applications/config/option/PhabricatorSendGridConfigOptions.php b/src/applications/config/option/PhabricatorSendGridConfigOptions.php index 33bf53bb23..898268b598 100644 --- 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) diff --git a/src/applications/files/config/PhabricatorFilesConfigOptions.php b/src/applications/files/config/PhabricatorFilesConfigOptions.php index 5eae1e8277..c942b9254a 100644 --- 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")), ); diff --git a/src/applications/phame/config/PhabricatorPhameConfigOptions.php b/src/applications/phame/config/PhabricatorPhameConfigOptions.php index 4931623d94..1ec6f2aeae 100644 --- 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.')), ); diff --git a/src/applications/phortune/option/PhabricatorPhortuneConfigOptions.php b/src/applications/phortune/option/PhabricatorPhortuneConfigOptions.php index fcea8e3746..02bc1d468b 100644 --- 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.')), ); diff --git a/src/applications/repository/PhabricatorRepositoryConfigOptions.php b/src/applications/repository/PhabricatorRepositoryConfigOptions.php index a641647be2..cae49f60db 100644 --- 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(