Page MenuHomePhabricator
Files F14007602

D11159.diff
No OneTemporary
Actions

D11159.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1440,6 +1440,7 @@
'PhabricatorChatLogDAO' => 'applications/chatlog/storage/PhabricatorChatLogDAO.php',
'PhabricatorChatLogEvent' => 'applications/chatlog/storage/PhabricatorChatLogEvent.php',
'PhabricatorChatLogQuery' => 'applications/chatlog/query/PhabricatorChatLogQuery.php',
+ 'PhabricatorClusterConfigOptions' => 'applications/config/option/PhabricatorClusterConfigOptions.php',
'PhabricatorCommitBranchesField' => 'applications/repository/customfield/PhabricatorCommitBranchesField.php',
'PhabricatorCommitCustomField' => 'applications/repository/customfield/PhabricatorCommitCustomField.php',
'PhabricatorCommitSearchEngine' => 'applications/audit/query/PhabricatorCommitSearchEngine.php',
@@ -4593,6 +4594,7 @@
'PhabricatorPolicyInterface',
),
'PhabricatorChatLogQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+ 'PhabricatorClusterConfigOptions' => 'PhabricatorApplicationConfigOptions',
'PhabricatorCommitBranchesField' => 'PhabricatorCommitCustomField',
'PhabricatorCommitCustomField' => 'PhabricatorCustomField',
'PhabricatorCommitSearchEngine' => 'PhabricatorApplicationSearchEngine',
diff --git a/src/aphront/configuration/AphrontApplicationConfiguration.php b/src/aphront/configuration/AphrontApplicationConfiguration.php
--- a/src/aphront/configuration/AphrontApplicationConfiguration.php
+++ b/src/aphront/configuration/AphrontApplicationConfiguration.php
@@ -275,6 +275,50 @@
final public function buildController() {
$request = $this->getRequest();
+ // If we're configured to operate in cluster mode, reject requests which
+ // were not received on a cluster interface.
+ //
+ // For example, a host may have an internal address like "170.0.0.1", and
+ // also have a public address like "51.23.95.16". Assuming the cluster
+ // is configured on a range like "170.0.0.0/16", we want to reject the
+ // requests received on the public interface.
+ //
+ // Ideally, nodes in a cluster should only be listening on internal
+ // interfaces, but they may be configured in such a way that they also
+ // listen on external interfaces, since this is easy to forget about or
+ // get wrong. As a broad security measure, reject requests received on any
+ // interfaces which aren't on the whitelist.
+
+ $cluster_addresses = PhabricatorEnv::getEnvConfig('cluster.addresses');
+ if ($cluster_addresses) {
+ $server_addr = idx($_SERVER, 'SERVER_ADDR');
+ if (!$server_addr) {
+ if (php_sapi_name() == 'cli') {
+ // This is a command line script (probably something like a unit
+ // test) so it's fine that we don't have SERVER_ADDR defined.
+ } else {
+ throw new AphrontUsageException(
+ pht('No SERVER_ADDR'),
+ pht(
+ 'Phabricator is configured to operate in cluster mode, but '.
+ 'SERVER_ADDR is not defined in the request context. Your '.
+ 'webserver configuration needs to forward SERVER_ADDR to '.
+ 'PHP so Phabricator can reject requests received on '.
+ 'external interfaces.'));
+ }
+ } else {
+ if (!PhabricatorEnv::isClusterAddress($server_addr)) {
+ throw new AphrontUsageException(
+ pht('External Interface'),
+ pht(
+ 'Phabricator is configured in cluster mode and the address '.
+ 'this request was received on ("%s") is not whitelisted as '.
+ 'a cluster address.',
+ $server_addr));
+ }
+ }
+ }
+
if (PhabricatorEnv::getEnvConfig('security.require-https')) {
if (!$request->isHTTPS()) {
$https_uri = $request->getRequestURI();
diff --git a/src/applications/conduit/controller/PhabricatorConduitAPIController.php b/src/applications/conduit/controller/PhabricatorConduitAPIController.php
--- a/src/applications/conduit/controller/PhabricatorConduitAPIController.php
+++ b/src/applications/conduit/controller/PhabricatorConduitAPIController.php
@@ -274,8 +274,16 @@
);
}
- throw new Exception(
- pht('Not Implemented: Would authenticate Almanac device.'));
+ if (!PhabricatorEnv::isClusterRemoteAddress()) {
+ return array(
+ 'ERR-INVALID-AUTH',
+ pht(
+ 'This request originates from outside of the Phabricator '.
+ 'cluster address range. Requests signed with trusted '.
+ 'device keys must originate from within the cluster.'),);
+ }
+
+ $user = PhabricatorUser::getOmnipotentUser();
}
return $this->validateAuthenticatedUser(
@@ -361,6 +369,19 @@
}
}
+ // If this is a "clr-" token, Phabricator must be configured in cluster
+ // mode and the remote address must be a cluster node.
+ if ($token->getTokenType() == PhabricatorConduitToken::TYPE_CLUSTER) {
+ if (!PhabricatorEnv::isClusterRemoteAddress()) {
+ return array(
+ 'ERR-INVALID-AUTH',
+ pht(
+ 'This request originates from outside of the Phabricator '.
+ 'cluster address range. Requests signed with cluster API '.
+ 'tokens must originate from within the cluster.'),);
+ }
+ }
+
$user = $token->getObject();
if (!($user instanceof PhabricatorUser)) {
return array(
diff --git a/src/applications/config/option/PhabricatorClusterConfigOptions.php b/src/applications/config/option/PhabricatorClusterConfigOptions.php
new file mode 100644
--- /dev/null
+++ b/src/applications/config/option/PhabricatorClusterConfigOptions.php
@@ -0,0 +1,58 @@
+<?php
+
+final class PhabricatorClusterConfigOptions
+ extends PhabricatorApplicationConfigOptions {
+
+ public function getName() {
+ return pht('Cluster Setup');
+ }
+
+ public function getDescription() {
+ return pht('Configure Phabricator to run on a cluster of hosts.');
+ }
+
+ public function getOptions() {
+ return array(
+ $this->newOption('cluster.addresses', 'list<string>', array())
+ ->setLocked(true)
+ ->setSummary(pht('Address ranges of cluster hosts.'))
+ ->setDescription(
+ pht(
+ 'To allow Phabricator nodes to communicate with other nodes '.
+ 'in the cluster, provide an address whitelist of hosts that '.
+ 'are part of the cluster.'.
+ "\n\n".
+ 'Hosts on this whitelist are permitted to use special cluster '.
+ 'mechanisms to authenticate requests. By default, these '.
+ 'mechanisms are disabled.'.
+ "\n\n".
+ 'Define a list of CIDR blocks which whitelist all hosts in the '.
+ 'cluster. See the examples below for details.',
+ "\n\n".
+ 'When cluster addresses are defined, Phabricator hosts will also '.
+ 'reject requests to interfaces which are not whitelisted.'))
+ ->addExample(
+ array(
+ '23.24.25.80/32',
+ '23.24.25.81/32',
+ ),
+ pht('Whitelist Specific Addresses'))
+ ->addExample(
+ array(
+ '1.2.3.0/24',
+ ),
+ pht('Whitelist 1.2.3.*'))
+ ->addExample(
+ array(
+ '1.2.0.0/16',
+ ),
+ pht('Whitelist 1.2.*.*'))
+ ->addExample(
+ array(
+ '0.0.0.0/0',
+ ),
+ pht('Allow Any Host (Insecure!)')),
+ );
+ }
+
+}
diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php
--- a/src/applications/people/storage/PhabricatorUser.php
+++ b/src/applications/people/storage/PhabricatorUser.php
@@ -82,6 +82,10 @@
* @return bool True if this is a standard, usable account.
*/
public function isUserActivated() {
+ if ($this->isOmnipotent()) {
+ return true;
+ }
+
if ($this->getIsDisabled()) {
return false;
}
diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/PhabricatorEnv.php
--- a/src/infrastructure/env/PhabricatorEnv.php
+++ b/src/infrastructure/env/PhabricatorEnv.php
@@ -528,6 +528,32 @@
return true;
}
+ public static function isClusterRemoteAddress() {
+ $address = idx($_SERVER, 'REMOTE_ADDR');
+ if (!$address) {
+ throw new Exception(
+ pht(
+ 'Unable to test remote address against cluster whitelist: '.
+ 'REMOTE_ADDR is not defined.'));
+ }
+
+ return self::isClusterAddress($address);
+ }
+
+ public static function isClusterAddress($address) {
+ $cluster_addresses = PhabricatorEnv::getEnvConfig('cluster.addresses');
+ if (!$cluster_addresses) {
+ throw new Exception(
+ pht(
+ 'Phabricator is not configured to serve cluster requests. '.
+ 'Set `cluster.addresses` in the configuration to whitelist '.
+ 'cluster hosts before sending requests that use a cluster '.
+ 'authentication mechanism.'));
+ }
+
+ return PhutilCIDRList::newList($cluster_addresses)
+ ->containsAddress($address);
+ }
/* -( Internals )---------------------------------------------------------- */

File Metadata

Mime Type
text/plain
Expires
Wed, Oct 30, 8:46 AM (2 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6716782
Default Alt Text
D11159.diff (9 KB)

Event Timeline

Phabricator · Built by Phacility