Skip to content

Commit 4970a0c

Browse files
Robin McCorkellDeepDiver1975
Robin McCorkell
authored and
DeepDiver1975
committed
Add setup check for reverse proxy header configuration
1 parent 8944af5 commit 4970a0c

File tree

4 files changed

+125
-6
lines changed

4 files changed

+125
-6
lines changed

‎core/js/setupchecks.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,11 @@
7777
t('core', 'Your PHP version ({version}) is no longer <a href="{phpLink}">supported by PHP</a>. We encourage you to upgrade your PHP version to take advantage of performance and security updates provided by PHP.', {version: data.phpSupported.version, phpLink: 'https://secure.php.net/supported-versions.php'})
7878
);
7979
}
80+
if(!data.forwardedForHeadersWorking) {
81+
messages.push(
82+
t('core', 'The reverse proxy headers configuration is incorrect, or you are accessing ownCloud from a trusted proxy. If you are not accessing ownCloud from a trusted proxy, this is a security issue and can allow an attacker to spoof their IP address as visible to ownCloud. Further information can be found in our <a href="{docLink}">documentation</a>.', {docLink: data.reverseProxyDocs})
83+
);
84+
}
8085
} else {
8186
messages.push(t('core', 'Error occurred while checking server setup'));
8287
}

‎core/js/tests/specs/setupchecksSpec.js‎

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,12 @@ describe('OC.SetupChecks tests', function() {
6666
{
6767
'Content-Type': 'application/json'
6868
},
69-
JSON.stringify({isUrandomAvailable: true, serverHasInternetConnection: false, memcacheDocs: 'https://doc.owncloud.org/server/go.php?to=admin-performance'})
69+
JSON.stringify({
70+
isUrandomAvailable: true,
71+
serverHasInternetConnection: false,
72+
memcacheDocs: 'https://doc.owncloud.org/server/go.php?to=admin-performance',
73+
forwardedForHeadersWorking: true
74+
})
7075
);
7176

7277
async.done(function( data, s, x ){
@@ -83,7 +88,13 @@ describe('OC.SetupChecks tests', function() {
8388
{
8489
'Content-Type': 'application/json'
8590
},
86-
JSON.stringify({isUrandomAvailable: true, serverHasInternetConnection: false, dataDirectoryProtected: false, memcacheDocs: 'https://doc.owncloud.org/server/go.php?to=admin-performance'})
91+
JSON.stringify({
92+
isUrandomAvailable: true,
93+
serverHasInternetConnection: false,
94+
dataDirectoryProtected: false,
95+
memcacheDocs: 'https://doc.owncloud.org/server/go.php?to=admin-performance',
96+
forwardedForHeadersWorking: true
97+
})
8798
);
8899

89100
async.done(function( data, s, x ){
@@ -100,7 +111,13 @@ describe('OC.SetupChecks tests', function() {
100111
{
101112
'Content-Type': 'application/json',
102113
},
103-
JSON.stringify({isUrandomAvailable: true, serverHasInternetConnection: false, dataDirectoryProtected: false, isMemcacheConfigured: true})
114+
JSON.stringify({
115+
isUrandomAvailable: true,
116+
serverHasInternetConnection: false,
117+
dataDirectoryProtected: false,
118+
isMemcacheConfigured: true,
119+
forwardedForHeadersWorking: true
120+
})
104121
);
105122

106123
async.done(function( data, s, x ){
@@ -117,7 +134,14 @@ describe('OC.SetupChecks tests', function() {
117134
{
118135
'Content-Type': 'application/json',
119136
},
120-
JSON.stringify({isUrandomAvailable: false, securityDocs: 'https://docs.owncloud.org/myDocs.html', serverHasInternetConnection: true, dataDirectoryProtected: true, isMemcacheConfigured: true})
137+
JSON.stringify({
138+
isUrandomAvailable: false,
139+
securityDocs: 'https://docs.owncloud.org/myDocs.html',
140+
serverHasInternetConnection: true,
141+
dataDirectoryProtected: true,
142+
isMemcacheConfigured: true,
143+
forwardedForHeadersWorking: true
144+
})
121145
);
122146

123147
async.done(function( data, s, x ){
@@ -126,6 +150,30 @@ describe('OC.SetupChecks tests', function() {
126150
});
127151
});
128152

153+
it('should return an error if the forwarded for headers are not working', function(done) {
154+
var async = OC.SetupChecks.checkSetup();
155+
156+
suite.server.requests[0].respond(
157+
200,
158+
{
159+
'Content-Type': 'application/json',
160+
},
161+
JSON.stringify({
162+
isUrandomAvailable: true,
163+
serverHasInternetConnection: true,
164+
dataDirectoryProtected: true,
165+
isMemcacheConfigured: true,
166+
forwardedForHeadersWorking: false,
167+
reverseProxyDocs: 'https://docs.owncloud.org/foo/bar.html'
168+
})
169+
);
170+
171+
async.done(function( data, s, x ){
172+
expect(data).toEqual(['The reverse proxy headers configuration is incorrect, or you are accessing ownCloud from a trusted proxy. If you are not accessing ownCloud from a trusted proxy, this is a security issue and can allow an attacker to spoof their IP address as visible to ownCloud. Further information can be found in our <a href="https://docs.owncloud.org/foo/bar.html">documentation</a>.']);
173+
done();
174+
});
175+
});
176+
129177
it('should return an error if the response has no statuscode 200', function(done) {
130178
var async = OC.SetupChecks.checkSetup();
131179

‎settings/controller/checksetupcontroller.php‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ public function getCurlVersion() {
128128
/**
129129
* Check if the used SSL lib is outdated. Older OpenSSL and NSS versions do
130130
* have multiple bugs which likely lead to problems in combination with
131-
* functionalities required by ownCloud such as SNI.
131+
* functionality required by ownCloud such as SNI.
132132
*
133133
* @link https://github.com/owncloud/core/issues/17446#issuecomment-122877546
134134
* @link https://bugzilla.redhat.com/show_bug.cgi?id=1241172
@@ -193,6 +193,23 @@ private function isPhpSupported() {
193193
return ['eol' => $eol, 'version' => PHP_VERSION];
194194
}
195195

196+
/*
197+
* Check if the reverse proxy configuration is working as expected
198+
*
199+
* @return bool
200+
*/
201+
private function forwardedForHeadersWorking() {
202+
$trustedProxies = $this->config->getSystemValue('trusted_proxies', []);
203+
$remoteAddress = $this->request->getRemoteAddress();
204+
205+
if (is_array($trustedProxies) && in_array($remoteAddress, $trustedProxies)) {
206+
return false;
207+
}
208+
209+
// either not enabled or working correctly
210+
return true;
211+
}
212+
196213
/**
197214
* @return DataResponse
198215
*/
@@ -207,6 +224,8 @@ public function check() {
207224
'securityDocs' => $this->urlGenerator->linkToDocs('admin-security'),
208225
'isUsedTlsLibOutdated' => $this->isUsedTlsLibOutdated(),
209226
'phpSupported' => $this->isPhpSupported(),
227+
'forwardedForHeadersWorking' => $this->forwardedForHeadersWorking(),
228+
'reverseProxyDocs' => $this->urlGenerator->linkToDocs('admin-reverse-proxy'),
210229
]
211230
);
212231
}

‎tests/settings/controller/CheckSetupControllerTest.php‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,40 @@ public function testIsPhpSupportedTrue() {
246246
['eol' => false, 'version' => PHP_VERSION],
247247
self::invokePrivate($this->checkSetupController, 'isPhpSupported')
248248
);
249+
}
250+
251+
public function testForwardedForHeadersWorkingFalse() {
252+
$this->config->expects($this->once())
253+
->method('getSystemValue')
254+
->with('trusted_proxies', [])
255+
->willReturn(['1.2.3.4']);
256+
$this->request->expects($this->once())
257+
->method('getRemoteAddress')
258+
->willReturn('1.2.3.4');
259+
260+
$this->assertFalse(
261+
self::invokePrivate(
262+
$this->checkSetupController,
263+
'forwardedForHeadersWorking'
264+
)
265+
);
266+
}
249267

268+
public function testForwardedForHeadersWorkingTrue() {
269+
$this->config->expects($this->once())
270+
->method('getSystemValue')
271+
->with('trusted_proxies', [])
272+
->willReturn(['1.2.3.4']);
273+
$this->request->expects($this->once())
274+
->method('getRemoteAddress')
275+
->willReturn('4.3.2.1');
276+
277+
$this->assertTrue(
278+
self::invokePrivate(
279+
$this->checkSetupController,
280+
'forwardedForHeadersWorking'
281+
)
282+
);
250283
}
251284

252285
public function testCheck() {
@@ -258,6 +291,14 @@ public function testCheck() {
258291
->method('getSystemValue')
259292
->with('memcache.local', null)
260293
->will($this->returnValue('SomeProvider'));
294+
$this->config->expects($this->at(2))
295+
->method('getSystemValue')
296+
->with('trusted_proxies', [])
297+
->willReturn(['1.2.3.4']);
298+
299+
$this->request->expects($this->once())
300+
->method('getRemoteAddress')
301+
->willReturn('4.3.2.1');
261302

262303
$client = $this->getMockBuilder('\OCP\Http\Client\IClient')
263304
->disableOriginalConstructor()->getMock();
@@ -285,6 +326,10 @@ public function testCheck() {
285326
->with('admin-security')
286327
->willReturn('https://doc.owncloud.org/server/8.1/admin_manual/configuration_server/hardening.html');
287328
self::$version_compare = -1;
329+
$this->urlGenerator->expects($this->at(2))
330+
->method('linkToDocs')
331+
->with('admin-reverse-proxy')
332+
->willReturn('reverse-proxy-doc-link');
288333

289334
$expected = new DataResponse(
290335
[
@@ -298,7 +343,9 @@ public function testCheck() {
298343
'phpSupported' => [
299344
'eol' => true,
300345
'version' => PHP_VERSION
301-
]
346+
],
347+
'forwardedForHeadersWorking' => true,
348+
'reverseProxyDocs' => 'reverse-proxy-doc-link',
302349
]
303350
);
304351
$this->assertEquals($expected, $this->checkSetupController->check());

0 commit comments

Comments
 (0)