Skip to content

Commit f84e0ca

Browse files
relrodepriestley
authored andcommitted
Add the "Security" config group options.
Summary: Added all the "Security" group options listed in T2255. Test Plan: - Looked at all the options. - Tested validation on `security.alternate-file-domain` Reviewers: epriestley, btrahan Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2255 Differential Revision: https://secure.phabricator.com/D4334
1 parent acadf51 commit f84e0ca

File tree

2 files changed

+184
-0
lines changed

2 files changed

+184
-0
lines changed

src/__phutil_library_map__.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1142,6 +1142,7 @@
11421142
'PhabricatorSearchResultView' => 'applications/search/view/PhabricatorSearchResultView.php',
11431143
'PhabricatorSearchScope' => 'applications/search/constants/PhabricatorSearchScope.php',
11441144
'PhabricatorSearchSelectController' => 'applications/search/controller/PhabricatorSearchSelectController.php',
1145+
'PhabricatorSecurityConfigOptions' => 'applications/config/option/PhabricatorSecurityConfigOptions.php',
11451146
'PhabricatorSettingsAdjustController' => 'applications/settings/controller/PhabricatorSettingsAdjustController.php',
11461147
'PhabricatorSettingsMainController' => 'applications/settings/controller/PhabricatorSettingsMainController.php',
11471148
'PhabricatorSettingsPanel' => 'applications/settings/panel/PhabricatorSettingsPanel.php',
@@ -2451,6 +2452,7 @@
24512452
'PhabricatorSearchQuery' => 'PhabricatorSearchDAO',
24522453
'PhabricatorSearchResultView' => 'AphrontView',
24532454
'PhabricatorSearchSelectController' => 'PhabricatorSearchBaseController',
2455+
'PhabricatorSecurityConfigOptions' => 'PhabricatorApplicationConfigOptions',
24542456
'PhabricatorSettingsAdjustController' => 'PhabricatorController',
24552457
'PhabricatorSettingsMainController' => 'PhabricatorController',
24562458
'PhabricatorSettingsPanelAccount' => 'PhabricatorSettingsPanel',
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
<?php
2+
3+
final class PhabricatorSecurityConfigOptions
4+
extends PhabricatorApplicationConfigOptions {
5+
6+
public function getName() {
7+
return pht("Security");
8+
}
9+
10+
public function getDescription() {
11+
return pht("Security options.");
12+
}
13+
14+
public function getOptions() {
15+
return array(
16+
$this->newOption('security.alternate-file-domain', 'string', null)
17+
->setSummary(pht("Alternate domain to serve files from."))
18+
->setDescription(
19+
pht(
20+
"IMPORTANT: By default, Phabricator serves files from the same ".
21+
"domain the application lives on. This is convenient but not ".
22+
"secure: it creates a large class of vulnerabilities which can ".
23+
"not be generally mitigated.\n\n".
24+
25+
"To avoid this, you should configure a second domain in the same ".
26+
"way you have the primary domain configured (i.e., point it at ".
27+
"the same machine and set up the same vhost rules) and provide ".
28+
"it here. For instance, if your primary install is on ".
29+
"'http://www.phabricator-example.com/', you could configure ".
30+
"'http://www.phabricator-files.com/' and specify the entire ".
31+
"domain (with protocol) here. This will enforce that files are ".
32+
"served only from the alternate domain. Ideally, you should use ".
33+
"a completely separate domain name rather than just a different ".
34+
"subdomain.\n\n".
35+
36+
"It is **STRONGLY RECOMMENDED** that you configure this. Your ".
37+
"install is **NOT SECURE** unless you do so."))
38+
->addExample('http://www.phabricator-files.com/', pht('Valid Setting')),
39+
$this->newOption(
40+
'security.hmac-key',
41+
'string',
42+
'[D\t~Y7eNmnQGJ;rnH6aF;m2!vJ8@v8C=Cs:aQS\.Qw')
43+
->setSummary(
44+
pht("Key for HMAC digests."))
45+
->setDescription(
46+
pht(
47+
"Default key for HMAC digests where the key is not important ".
48+
"(i.e., the hash itself is secret). You can change this if you ".
49+
"want (to any other string), but doing so will break existing ".
50+
"sessions and CSRF tokens.")),
51+
$this->newOption('security.require-https', 'bool', false)
52+
->setSummary(
53+
pht("Force users to connect via https instead of http."))
54+
->setDescription(
55+
pht(
56+
"If the web server responds to both HTTP and HTTPS requests but ".
57+
"you want users to connect with only HTTPS, you can set this ".
58+
"to true to make Phabricator redirect HTTP requests to HTTPS.\n\n".
59+
60+
"Normally, you should just configure your server not to accept ".
61+
"HTTP traffic, but this setting may be useful if you originally ".
62+
"used HTTP and have now switched to HTTPS but don't want to ".
63+
"break old links, or if your webserver sits behind a load ".
64+
"balancer which terminates HTTPS connections and you can not ".
65+
"reasonably configure more granular behavior there.\n\n".
66+
67+
"NOTE: Phabricator determines if a request is HTTPS or not by ".
68+
"examining the PHP \$_SERVER['HTTPS'] variable. If you run ".
69+
"Apache/mod_php this will probably be set correctly for you ".
70+
"automatically, but if you run Phabricator as CGI/FCGI (e.g., ".
71+
"through nginx or lighttpd), you need to configure your web ".
72+
"server so that it passes the value correctly based on the ".
73+
"connection type. Alternatively, you can add a PHP snippet to ".
74+
"the top of this configuration file to directly set ".
75+
"\$_SERVER['HTTPS'] to the correct value."))
76+
->setOptions(
77+
array(
78+
pht('Allow HTTP'),
79+
pht('Force HTTPS'),
80+
)),
81+
$this->newOption(
82+
'phabricator.csrf-key',
83+
'string',
84+
'0b7ec0592e0a2829d8b71df2fa269b2c6172eca3')
85+
->setSummary(
86+
pht("Hashed with other inputs to generate CSRF tokens."))
87+
->setDescription(
88+
pht(
89+
"This is hashed with other inputs to generate CSRF tokens. If ".
90+
"you want, you can change it to some other string which is ".
91+
"unique to your install. This will make your install more secure ".
92+
"in a vague, mostly theoretical way. But it will take you like 3 ".
93+
"seconds of mashing on your keyboard to set it up so you might ".
94+
"as well.")),
95+
$this->newOption(
96+
'phabricator.mail-key',
97+
'string',
98+
'5ce3e7e8787f6e40dfae861da315a5cdf1018f12')
99+
->setSummary(
100+
pht("Hashed with other inputs to generate mail tokens."))
101+
->setDescription(
102+
pht(
103+
"This is hashed with other inputs to generate mail tokens. If ".
104+
"you want, you can change it to some other string which is ".
105+
"unique to your install. In particular, you will want to do ".
106+
"this if you accidentally send a bunch of mail somewhere you ".
107+
"shouldn't have, to invalidate all old reply-to addresses.")),
108+
// TODO: This should really be dict<string,bool> but that doesn't exist
109+
// yet.
110+
$this->newOption('uri.allowed-protocols', 'wild', null)
111+
->setSummary(
112+
pht("Determines which URI protocols are auto-linked."))
113+
->setDescription(
114+
pht(
115+
"When users write comments which have URIs, they'll be ".
116+
"automatically linked if the protocol appears in this set. This ".
117+
"whitelist is primarily to prevent security issues like ".
118+
"javascript:// URIs."))
119+
->addExample(
120+
'{"http": true, "https": true"}', pht('Valid Setting')),
121+
$this->newOption(
122+
'celerity.resource-hash',
123+
'string',
124+
'd9455ea150622ee044f7931dabfa52aa')
125+
->setSummary(
126+
pht("An input to the hash function when building resource hashes."))
127+
->setDescription(
128+
pht(
129+
"This value is an input to the hash function when building ".
130+
"resource hashes. It has no security value, but if you ".
131+
"accidentally poison user caches (by pushing a bad patch or ".
132+
"having something go wrong with a CDN, e.g.) you can change this ".
133+
"to something else and rebuild the Celerity map to break user ".
134+
"caches. Unless you are doing Celerity development, it is ".
135+
"exceptionally unlikely that you need to modify this.")),
136+
);
137+
}
138+
139+
protected function didValidateOption(
140+
PhabricatorConfigOption $option,
141+
$value) {
142+
143+
$key = $option->getKey();
144+
if ($key == 'security.alternate-file-domain') {
145+
146+
$uri = new PhutilURI($value);
147+
$protocol = $uri->getProtocol();
148+
if ($protocol !== 'http' && $protocol !== 'https') {
149+
throw new PhabricatorConfigValidationException(
150+
pht(
151+
"Config option '%s' is invalid. The URI must start with ".
152+
"'http://' or 'https://'.",
153+
$key));
154+
}
155+
156+
$domain = $uri->getDomain();
157+
if (strpos($domain, '.') === false) {
158+
throw new PhabricatorConfigValidationException(
159+
pht(
160+
"Config option '%s' is invalid. The URI must contain a dot ('.'), ".
161+
"like 'http://example.com/', not just a bare name like ".
162+
"'http://example/'. Some web browsers will not set cookies on ".
163+
"domains with no TLD.",
164+
$key));
165+
}
166+
167+
$path = $uri->getPath();
168+
if ($path !== '' && $path !== '/') {
169+
throw new PhabricatorConfigValidationException(
170+
pht(
171+
"Config option '%s' is invalid. The URI must NOT have a path, ".
172+
"e.g. 'http://phabricator.example.com/' is OK, but ".
173+
"'http://example.com/phabricator/' is not. Phabricator must be ".
174+
"installed on an entire domain; it can not be installed on a ".
175+
"path.",
176+
$key));
177+
}
178+
}
179+
}
180+
181+
182+
}

0 commit comments

Comments
 (0)