Provide a setting which forces all file views to be served from an alternate
domain
Summary:
See D758, D759.
- Provide a strongly recommended setting which permits configuration of an
alternate domain.
- Lock cookies down better: set them on the exact domain, and use SSL-only if
the configuration is HTTPS.
- Prevent Phabriator from setting cookies on other domains.
This assumes D759 will land, it is not effective without that change.
Test Plan:
- Attempted to login from a different domain and was rejected.
- Logged out, logged back in normally.
- Put install in setup mode and verified it revealed a warning.
- Configured an alterate domain.
- Tried to view an image with an old URI, got a 400.
- Went to /files/ and verified links rendered to the alternate domain.
- Viewed an alternate domain file.
- Tried to view an alternate domain file without the secret key, got a 404.
Reviewers: andrewjcg, erling, aran, tuomaspelkonen, jungejason, codeblock
CC: aran
Differential Revision: 760
2011-08-02 07:24:00 +02:00
|
|
|
<?php
|
|
|
|
|
Move ALL files to serve from the alternate file domain, not just files without
"Content-Disposition: attachment"
Summary:
We currently serve some files off the primary domain (with "Content-Disposition:
attachment" + a CSRF check) and some files off the alternate domain (without
either).
This is not sufficient, because some UAs (like the iPad) ignore
"Content-Disposition: attachment". So there's an attack that goes like this:
- Alice uploads xss.html
- Alice says to Bob "hey download this file on your iPad"
- Bob clicks "Download" on Phabricator on his iPad, gets XSS'd.
NOTE: This removes the CSRF check for downloading files. The check is nice to
have but only raises the barrier to entry slightly. Between iPad / sniffing /
flash bytecode attacks, single-domain installs are simply insecure. We could
restore the check at some point in conjunction with a derived authentication
cookie (i.e., a mini-session-token which is only useful for downloading files),
but that's a lot of complexity to drop all at once.
(Because files are now authenticated only by knowing the PHID and secret key,
this also fixes the "no profile pictures in public feed while logged out"
issue.)
Test Plan: Viewed, info'd, and downloaded files
Reviewers: btrahan, arice, alok
Reviewed By: arice
CC: aran, epriestley
Maniphest Tasks: T843
Differential Revision: https://secure.phabricator.com/D1608
2012-02-14 23:52:27 +01:00
|
|
|
final class PhabricatorFileDataController extends PhabricatorFileController {
|
Provide a setting which forces all file views to be served from an alternate
domain
Summary:
See D758, D759.
- Provide a strongly recommended setting which permits configuration of an
alternate domain.
- Lock cookies down better: set them on the exact domain, and use SSL-only if
the configuration is HTTPS.
- Prevent Phabriator from setting cookies on other domains.
This assumes D759 will land, it is not effective without that change.
Test Plan:
- Attempted to login from a different domain and was rejected.
- Logged out, logged back in normally.
- Put install in setup mode and verified it revealed a warning.
- Configured an alterate domain.
- Tried to view an image with an old URI, got a 400.
- Went to /files/ and verified links rendered to the alternate domain.
- Viewed an alternate domain file.
- Tried to view an alternate domain file without the secret key, got a 404.
Reviewers: andrewjcg, erling, aran, tuomaspelkonen, jungejason, codeblock
CC: aran
Differential Revision: 760
2011-08-02 07:24:00 +02:00
|
|
|
|
|
|
|
private $phid;
|
|
|
|
private $key;
|
2014-08-11 16:08:58 +02:00
|
|
|
private $token;
|
Provide a setting which forces all file views to be served from an alternate
domain
Summary:
See D758, D759.
- Provide a strongly recommended setting which permits configuration of an
alternate domain.
- Lock cookies down better: set them on the exact domain, and use SSL-only if
the configuration is HTTPS.
- Prevent Phabriator from setting cookies on other domains.
This assumes D759 will land, it is not effective without that change.
Test Plan:
- Attempted to login from a different domain and was rejected.
- Logged out, logged back in normally.
- Put install in setup mode and verified it revealed a warning.
- Configured an alterate domain.
- Tried to view an image with an old URI, got a 400.
- Went to /files/ and verified links rendered to the alternate domain.
- Viewed an alternate domain file.
- Tried to view an alternate domain file without the secret key, got a 404.
Reviewers: andrewjcg, erling, aran, tuomaspelkonen, jungejason, codeblock
CC: aran
Differential Revision: 760
2011-08-02 07:24:00 +02:00
|
|
|
|
|
|
|
public function willProcessRequest(array $data) {
|
|
|
|
$this->phid = $data['phid'];
|
|
|
|
$this->key = $data['key'];
|
2014-08-11 16:08:58 +02:00
|
|
|
$this->token = idx($data, 'token');
|
Provide a setting which forces all file views to be served from an alternate
domain
Summary:
See D758, D759.
- Provide a strongly recommended setting which permits configuration of an
alternate domain.
- Lock cookies down better: set them on the exact domain, and use SSL-only if
the configuration is HTTPS.
- Prevent Phabriator from setting cookies on other domains.
This assumes D759 will land, it is not effective without that change.
Test Plan:
- Attempted to login from a different domain and was rejected.
- Logged out, logged back in normally.
- Put install in setup mode and verified it revealed a warning.
- Configured an alterate domain.
- Tried to view an image with an old URI, got a 400.
- Went to /files/ and verified links rendered to the alternate domain.
- Viewed an alternate domain file.
- Tried to view an alternate domain file without the secret key, got a 404.
Reviewers: andrewjcg, erling, aran, tuomaspelkonen, jungejason, codeblock
CC: aran
Differential Revision: 760
2011-08-02 07:24:00 +02:00
|
|
|
}
|
|
|
|
|
|
|
|
public function shouldRequireLogin() {
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
|
2014-08-11 16:08:58 +02:00
|
|
|
protected function checkFileAndToken($file) {
|
|
|
|
if (!$file) {
|
|
|
|
return new Aphront404Response();
|
|
|
|
}
|
|
|
|
|
|
|
|
if (!$file->validateSecretKey($this->key)) {
|
|
|
|
return new Aphront403Response();
|
|
|
|
}
|
|
|
|
|
|
|
|
return null;
|
|
|
|
}
|
|
|
|
|
Provide a setting which forces all file views to be served from an alternate
domain
Summary:
See D758, D759.
- Provide a strongly recommended setting which permits configuration of an
alternate domain.
- Lock cookies down better: set them on the exact domain, and use SSL-only if
the configuration is HTTPS.
- Prevent Phabriator from setting cookies on other domains.
This assumes D759 will land, it is not effective without that change.
Test Plan:
- Attempted to login from a different domain and was rejected.
- Logged out, logged back in normally.
- Put install in setup mode and verified it revealed a warning.
- Configured an alterate domain.
- Tried to view an image with an old URI, got a 400.
- Went to /files/ and verified links rendered to the alternate domain.
- Viewed an alternate domain file.
- Tried to view an alternate domain file without the secret key, got a 404.
Reviewers: andrewjcg, erling, aran, tuomaspelkonen, jungejason, codeblock
CC: aran
Differential Revision: 760
2011-08-02 07:24:00 +02:00
|
|
|
public function processRequest() {
|
|
|
|
$request = $this->getRequest();
|
|
|
|
|
Move ALL files to serve from the alternate file domain, not just files without
"Content-Disposition: attachment"
Summary:
We currently serve some files off the primary domain (with "Content-Disposition:
attachment" + a CSRF check) and some files off the alternate domain (without
either).
This is not sufficient, because some UAs (like the iPad) ignore
"Content-Disposition: attachment". So there's an attack that goes like this:
- Alice uploads xss.html
- Alice says to Bob "hey download this file on your iPad"
- Bob clicks "Download" on Phabricator on his iPad, gets XSS'd.
NOTE: This removes the CSRF check for downloading files. The check is nice to
have but only raises the barrier to entry slightly. Between iPad / sniffing /
flash bytecode attacks, single-domain installs are simply insecure. We could
restore the check at some point in conjunction with a derived authentication
cookie (i.e., a mini-session-token which is only useful for downloading files),
but that's a lot of complexity to drop all at once.
(Because files are now authenticated only by knowing the PHID and secret key,
this also fixes the "no profile pictures in public feed while logged out"
issue.)
Test Plan: Viewed, info'd, and downloaded files
Reviewers: btrahan, arice, alok
Reviewed By: arice
CC: aran, epriestley
Maniphest Tasks: T843
Differential Revision: https://secure.phabricator.com/D1608
2012-02-14 23:52:27 +01:00
|
|
|
$alt = PhabricatorEnv::getEnvConfig('security.alternate-file-domain');
|
2014-08-11 16:08:58 +02:00
|
|
|
$base_uri = PhabricatorEnv::getEnvConfig('phabricator.base-uri');
|
|
|
|
$alt_uri = new PhutilURI($alt);
|
|
|
|
$alt_domain = $alt_uri->getDomain();
|
|
|
|
$req_domain = $request->getHost();
|
|
|
|
$main_domain = id(new PhutilURI($base_uri))->getDomain();
|
|
|
|
|
|
|
|
$cache_response = true;
|
|
|
|
|
|
|
|
if (empty($alt) || $main_domain == $alt_domain) {
|
|
|
|
// Alternate files domain isn't configured or it's set
|
|
|
|
// to the same as the default domain
|
|
|
|
|
|
|
|
// load the file with permissions checks;
|
|
|
|
$file = id(new PhabricatorFileQuery())
|
|
|
|
->setViewer($request->getUser())
|
|
|
|
->withPHIDs(array($this->phid))
|
|
|
|
->executeOne();
|
|
|
|
|
|
|
|
$error_response = $this->checkFileAndToken($file);
|
|
|
|
if ($error_response) {
|
|
|
|
return $error_response;
|
|
|
|
}
|
|
|
|
|
|
|
|
// when the file is not CDNable, don't allow cache
|
|
|
|
$cache_response = $file->getCanCDN();
|
|
|
|
} else if ($req_domain != $alt_domain) {
|
|
|
|
// Alternate domain is configured but this request isn't using it
|
|
|
|
|
|
|
|
// load the file with permissions checks;
|
|
|
|
$file = id(new PhabricatorFileQuery())
|
|
|
|
->setViewer($request->getUser())
|
|
|
|
->withPHIDs(array($this->phid))
|
|
|
|
->executeOne();
|
|
|
|
|
|
|
|
$error_response = $this->checkFileAndToken($file);
|
|
|
|
if ($error_response) {
|
|
|
|
return $error_response;
|
|
|
|
}
|
|
|
|
|
|
|
|
// if the user can see the file, generate a token;
|
|
|
|
// redirect to the alt domain with the token;
|
2012-08-24 21:25:51 +02:00
|
|
|
return id(new AphrontRedirectResponse())
|
2014-08-18 23:11:06 +02:00
|
|
|
->setIsExternal(true)
|
2014-08-11 16:08:58 +02:00
|
|
|
->setURI($file->getCDNURIWithToken());
|
Provide a setting which forces all file views to be served from an alternate
domain
Summary:
See D758, D759.
- Provide a strongly recommended setting which permits configuration of an
alternate domain.
- Lock cookies down better: set them on the exact domain, and use SSL-only if
the configuration is HTTPS.
- Prevent Phabriator from setting cookies on other domains.
This assumes D759 will land, it is not effective without that change.
Test Plan:
- Attempted to login from a different domain and was rejected.
- Logged out, logged back in normally.
- Put install in setup mode and verified it revealed a warning.
- Configured an alterate domain.
- Tried to view an image with an old URI, got a 400.
- Went to /files/ and verified links rendered to the alternate domain.
- Viewed an alternate domain file.
- Tried to view an alternate domain file without the secret key, got a 404.
Reviewers: andrewjcg, erling, aran, tuomaspelkonen, jungejason, codeblock
CC: aran
Differential Revision: 760
2011-08-02 07:24:00 +02:00
|
|
|
|
2014-08-11 16:08:58 +02:00
|
|
|
} else {
|
|
|
|
// We are using the alternate domain
|
2013-10-03 23:38:08 +02:00
|
|
|
|
2014-08-11 16:08:58 +02:00
|
|
|
// load the file, bypassing permission checks;
|
|
|
|
$file = id(new PhabricatorFileQuery())
|
|
|
|
->setViewer(PhabricatorUser::getOmnipotentUser())
|
|
|
|
->withPHIDs(array($this->phid))
|
|
|
|
->executeOne();
|
Provide a setting which forces all file views to be served from an alternate
domain
Summary:
See D758, D759.
- Provide a strongly recommended setting which permits configuration of an
alternate domain.
- Lock cookies down better: set them on the exact domain, and use SSL-only if
the configuration is HTTPS.
- Prevent Phabriator from setting cookies on other domains.
This assumes D759 will land, it is not effective without that change.
Test Plan:
- Attempted to login from a different domain and was rejected.
- Logged out, logged back in normally.
- Put install in setup mode and verified it revealed a warning.
- Configured an alterate domain.
- Tried to view an image with an old URI, got a 400.
- Went to /files/ and verified links rendered to the alternate domain.
- Viewed an alternate domain file.
- Tried to view an alternate domain file without the secret key, got a 404.
Reviewers: andrewjcg, erling, aran, tuomaspelkonen, jungejason, codeblock
CC: aran
Differential Revision: 760
2011-08-02 07:24:00 +02:00
|
|
|
|
2014-08-11 16:08:58 +02:00
|
|
|
$error_response = $this->checkFileAndToken($file);
|
|
|
|
if ($error_response) {
|
|
|
|
return $error_response;
|
|
|
|
}
|
|
|
|
|
Generate a 403 page with a nice dialog when a file token is invalid
Summary:
Ref T5685. Currently we just 403 on an invalid token, but we can be a little more helpful.
The issues here are:
- If we **do** redirect you on this page and something goes wrong, you might get stuck in a redirect loop.
- If we **don't** redirect you, copy/pasting the link to someone (or reloading the page) gives them a pretty confusing result, since the link doesn't work any more. Prior to this diff, they get a 403.
To mitigate this, do a little better than a bare 403: give them a link to auth and generate a new URI for the file.
If this is still confusing, the next best thing I can come up with is something like this:
- Put some modulous of the timestamp in the URI.
- If the current time is within 2 seconds of the generation time, show this dialog.
- Otherwise, redirect.
That seems like it would be okay, but I worry that "2" has to be small (so links you copy/paste -> chat -> click still work) and a small value means that a small amount of clock skew breaks things. We could use the database clock, but ehhh.
Other ideas:
- Put a hash of the remote IP in the URI, redirect if it doesn't match. Fails for companies behind a NAT gateway but should work in a lot of other cases.
- Just redirect always, there's no reason it should ever loop and browsers don't really do anything bad when there's a loop (they'll show an error after too many redirects).
I'm leaning toward letting this stabilize in the wild for a bit, then trying "always redirect".
Test Plan: {F188914}
Reviewers: btrahan, 20after4
Reviewed By: 20after4
Subscribers: epriestley
Maniphest Tasks: T5685
Differential Revision: https://secure.phabricator.com/D10215
2014-08-11 18:39:25 +02:00
|
|
|
$acquire_token_uri = id(new PhutilURI($file->getViewURI()))
|
|
|
|
->setDomain($main_domain);
|
|
|
|
|
|
|
|
|
2014-08-11 16:08:58 +02:00
|
|
|
if ($this->token) {
|
|
|
|
// validate the token, if it is valid, continue
|
|
|
|
$validated_token = $file->validateOneTimeToken($this->token);
|
|
|
|
|
|
|
|
if (!$validated_token) {
|
Generate a 403 page with a nice dialog when a file token is invalid
Summary:
Ref T5685. Currently we just 403 on an invalid token, but we can be a little more helpful.
The issues here are:
- If we **do** redirect you on this page and something goes wrong, you might get stuck in a redirect loop.
- If we **don't** redirect you, copy/pasting the link to someone (or reloading the page) gives them a pretty confusing result, since the link doesn't work any more. Prior to this diff, they get a 403.
To mitigate this, do a little better than a bare 403: give them a link to auth and generate a new URI for the file.
If this is still confusing, the next best thing I can come up with is something like this:
- Put some modulous of the timestamp in the URI.
- If the current time is within 2 seconds of the generation time, show this dialog.
- Otherwise, redirect.
That seems like it would be okay, but I worry that "2" has to be small (so links you copy/paste -> chat -> click still work) and a small value means that a small amount of clock skew breaks things. We could use the database clock, but ehhh.
Other ideas:
- Put a hash of the remote IP in the URI, redirect if it doesn't match. Fails for companies behind a NAT gateway but should work in a lot of other cases.
- Just redirect always, there's no reason it should ever loop and browsers don't really do anything bad when there's a loop (they'll show an error after too many redirects).
I'm leaning toward letting this stabilize in the wild for a bit, then trying "always redirect".
Test Plan: {F188914}
Reviewers: btrahan, 20after4
Reviewed By: 20after4
Subscribers: epriestley
Maniphest Tasks: T5685
Differential Revision: https://secure.phabricator.com/D10215
2014-08-11 18:39:25 +02:00
|
|
|
$dialog = $this->newDialog()
|
|
|
|
->setShortTitle(pht('Expired File'))
|
|
|
|
->setTitle(pht('File Link Has Expired'))
|
|
|
|
->appendParagraph(
|
|
|
|
pht(
|
|
|
|
'The link you followed to view this file is invalid or '.
|
|
|
|
'expired.'))
|
|
|
|
->appendParagraph(
|
|
|
|
pht(
|
|
|
|
'Continue to generate a new link to the file. You may be '.
|
|
|
|
'required to log in.'))
|
|
|
|
->addCancelButton(
|
|
|
|
$acquire_token_uri,
|
|
|
|
pht('Continue'));
|
|
|
|
|
|
|
|
// Build an explicit response so we can respond with HTTP/403 instead
|
|
|
|
// of HTTP/200.
|
|
|
|
$response = id(new AphrontDialogResponse())
|
|
|
|
->setDialog($dialog)
|
|
|
|
->setHTTPResponseCode(403);
|
|
|
|
|
|
|
|
return $response;
|
2014-08-11 16:08:58 +02:00
|
|
|
}
|
|
|
|
// return the file data without cache headers
|
|
|
|
$cache_response = false;
|
|
|
|
} else if (!$file->getCanCDN()) {
|
|
|
|
// file cannot be served via cdn, and no token given
|
|
|
|
// redirect to the main domain to aquire a token
|
|
|
|
|
2014-08-18 23:11:06 +02:00
|
|
|
// This is marked as an "external" URI because it is fully qualified.
|
2014-08-11 16:08:58 +02:00
|
|
|
return id(new AphrontRedirectResponse())
|
2014-08-18 23:11:06 +02:00
|
|
|
->setIsExternal(true)
|
Generate a 403 page with a nice dialog when a file token is invalid
Summary:
Ref T5685. Currently we just 403 on an invalid token, but we can be a little more helpful.
The issues here are:
- If we **do** redirect you on this page and something goes wrong, you might get stuck in a redirect loop.
- If we **don't** redirect you, copy/pasting the link to someone (or reloading the page) gives them a pretty confusing result, since the link doesn't work any more. Prior to this diff, they get a 403.
To mitigate this, do a little better than a bare 403: give them a link to auth and generate a new URI for the file.
If this is still confusing, the next best thing I can come up with is something like this:
- Put some modulous of the timestamp in the URI.
- If the current time is within 2 seconds of the generation time, show this dialog.
- Otherwise, redirect.
That seems like it would be okay, but I worry that "2" has to be small (so links you copy/paste -> chat -> click still work) and a small value means that a small amount of clock skew breaks things. We could use the database clock, but ehhh.
Other ideas:
- Put a hash of the remote IP in the URI, redirect if it doesn't match. Fails for companies behind a NAT gateway but should work in a lot of other cases.
- Just redirect always, there's no reason it should ever loop and browsers don't really do anything bad when there's a loop (they'll show an error after too many redirects).
I'm leaning toward letting this stabilize in the wild for a bit, then trying "always redirect".
Test Plan: {F188914}
Reviewers: btrahan, 20after4
Reviewed By: 20after4
Subscribers: epriestley
Maniphest Tasks: T5685
Differential Revision: https://secure.phabricator.com/D10215
2014-08-11 18:39:25 +02:00
|
|
|
->setURI($acquire_token_uri);
|
2014-08-11 16:08:58 +02:00
|
|
|
}
|
Provide a setting which forces all file views to be served from an alternate
domain
Summary:
See D758, D759.
- Provide a strongly recommended setting which permits configuration of an
alternate domain.
- Lock cookies down better: set them on the exact domain, and use SSL-only if
the configuration is HTTPS.
- Prevent Phabriator from setting cookies on other domains.
This assumes D759 will land, it is not effective without that change.
Test Plan:
- Attempted to login from a different domain and was rejected.
- Logged out, logged back in normally.
- Put install in setup mode and verified it revealed a warning.
- Configured an alterate domain.
- Tried to view an image with an old URI, got a 400.
- Went to /files/ and verified links rendered to the alternate domain.
- Viewed an alternate domain file.
- Tried to view an alternate domain file without the secret key, got a 404.
Reviewers: andrewjcg, erling, aran, tuomaspelkonen, jungejason, codeblock
CC: aran
Differential Revision: 760
2011-08-02 07:24:00 +02:00
|
|
|
}
|
|
|
|
|
|
|
|
$data = $file->loadFileData();
|
|
|
|
$response = new AphrontFileResponse();
|
|
|
|
$response->setContent($data);
|
2014-08-11 16:08:58 +02:00
|
|
|
if ($cache_response) {
|
|
|
|
$response->setCacheDurationInSeconds(60 * 60 * 24 * 30);
|
|
|
|
}
|
Provide a setting which forces all file views to be served from an alternate
domain
Summary:
See D758, D759.
- Provide a strongly recommended setting which permits configuration of an
alternate domain.
- Lock cookies down better: set them on the exact domain, and use SSL-only if
the configuration is HTTPS.
- Prevent Phabriator from setting cookies on other domains.
This assumes D759 will land, it is not effective without that change.
Test Plan:
- Attempted to login from a different domain and was rejected.
- Logged out, logged back in normally.
- Put install in setup mode and verified it revealed a warning.
- Configured an alterate domain.
- Tried to view an image with an old URI, got a 400.
- Went to /files/ and verified links rendered to the alternate domain.
- Viewed an alternate domain file.
- Tried to view an alternate domain file without the secret key, got a 404.
Reviewers: andrewjcg, erling, aran, tuomaspelkonen, jungejason, codeblock
CC: aran
Differential Revision: 760
2011-08-02 07:24:00 +02:00
|
|
|
|
Fix two issues with audio macros
Summary:
Fixes T3887. Two issues:
- Macros were generating entirely before the render cache, so audio macros worked fine in previews and the first time the cache was populated, but not afterward.
- Instead, parse them before the cache but drop them in after the cache. Clean up all the file querying, too. This makes cached remarkup generate the correct audio beahviors.
- Safari sends an HTTP request with a "Range" header, and expects a "206 Partial Content" response. If we don't give it one, it sometimes has trouble figuring out how long a piece of audio is (mostly for longer clips? Or mostly for MP3s?). I'm not exactly sure what triggers it. The net effect is that "loop" does not work when Safari gets confused. While looping a short "quack.wav" worked fine, longer MP3s didn't loop.
- Supporting "Range" and "206 Partial Content", which is straightforward, fixes this problem.
Test Plan:
- Viewed a page with lots of different cached audio macros and lots of different uncached preview audio macros, they all rendered correctly and played audio.
- Viewed a macro with a long MP3 audio loop in Safari. Verified it looped after it completed. Used Charles to check that the server received and responded to the "Range" header correctly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3887
Differential Revision: https://secure.phabricator.com/D7166
2013-09-29 00:32:48 +02:00
|
|
|
// NOTE: It's important to accept "Range" requests when playing audio.
|
|
|
|
// If we don't, Safari has difficulty figuring out how long sounds are
|
|
|
|
// and glitches when trying to loop them. In particular, Safari sends
|
|
|
|
// an initial request for bytes 0-1 of the audio file, and things go south
|
|
|
|
// if we can't respond with a 206 Partial Content.
|
|
|
|
$range = $request->getHTTPHeader('range');
|
|
|
|
if ($range) {
|
|
|
|
$matches = null;
|
|
|
|
if (preg_match('/^bytes=(\d+)-(\d+)$/', $range, $matches)) {
|
|
|
|
$response->setHTTPResponseCode(206);
|
|
|
|
$response->setRange((int)$matches[1], (int)$matches[2]);
|
|
|
|
}
|
2014-08-11 16:08:58 +02:00
|
|
|
} else if (isset($validated_token)) {
|
|
|
|
// consume the one-time token if we have one.
|
|
|
|
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
|
|
|
|
$validated_token->delete();
|
|
|
|
unset($unguarded);
|
Fix two issues with audio macros
Summary:
Fixes T3887. Two issues:
- Macros were generating entirely before the render cache, so audio macros worked fine in previews and the first time the cache was populated, but not afterward.
- Instead, parse them before the cache but drop them in after the cache. Clean up all the file querying, too. This makes cached remarkup generate the correct audio beahviors.
- Safari sends an HTTP request with a "Range" header, and expects a "206 Partial Content" response. If we don't give it one, it sometimes has trouble figuring out how long a piece of audio is (mostly for longer clips? Or mostly for MP3s?). I'm not exactly sure what triggers it. The net effect is that "loop" does not work when Safari gets confused. While looping a short "quack.wav" worked fine, longer MP3s didn't loop.
- Supporting "Range" and "206 Partial Content", which is straightforward, fixes this problem.
Test Plan:
- Viewed a page with lots of different cached audio macros and lots of different uncached preview audio macros, they all rendered correctly and played audio.
- Viewed a macro with a long MP3 audio loop in Safari. Verified it looped after it completed. Used Charles to check that the server received and responded to the "Range" header correctly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3887
Differential Revision: https://secure.phabricator.com/D7166
2013-09-29 00:32:48 +02:00
|
|
|
}
|
|
|
|
|
2012-10-23 04:06:56 +02:00
|
|
|
$is_viewable = $file->isViewableInBrowser();
|
|
|
|
$force_download = $request->getExists('download');
|
Move ALL files to serve from the alternate file domain, not just files without
"Content-Disposition: attachment"
Summary:
We currently serve some files off the primary domain (with "Content-Disposition:
attachment" + a CSRF check) and some files off the alternate domain (without
either).
This is not sufficient, because some UAs (like the iPad) ignore
"Content-Disposition: attachment". So there's an attack that goes like this:
- Alice uploads xss.html
- Alice says to Bob "hey download this file on your iPad"
- Bob clicks "Download" on Phabricator on his iPad, gets XSS'd.
NOTE: This removes the CSRF check for downloading files. The check is nice to
have but only raises the barrier to entry slightly. Between iPad / sniffing /
flash bytecode attacks, single-domain installs are simply insecure. We could
restore the check at some point in conjunction with a derived authentication
cookie (i.e., a mini-session-token which is only useful for downloading files),
but that's a lot of complexity to drop all at once.
(Because files are now authenticated only by knowing the PHID and secret key,
this also fixes the "no profile pictures in public feed while logged out"
issue.)
Test Plan: Viewed, info'd, and downloaded files
Reviewers: btrahan, arice, alok
Reviewed By: arice
CC: aran, epriestley
Maniphest Tasks: T843
Differential Revision: https://secure.phabricator.com/D1608
2012-02-14 23:52:27 +01:00
|
|
|
|
2012-10-23 04:06:56 +02:00
|
|
|
if ($is_viewable && !$force_download) {
|
Move ALL files to serve from the alternate file domain, not just files without
"Content-Disposition: attachment"
Summary:
We currently serve some files off the primary domain (with "Content-Disposition:
attachment" + a CSRF check) and some files off the alternate domain (without
either).
This is not sufficient, because some UAs (like the iPad) ignore
"Content-Disposition: attachment". So there's an attack that goes like this:
- Alice uploads xss.html
- Alice says to Bob "hey download this file on your iPad"
- Bob clicks "Download" on Phabricator on his iPad, gets XSS'd.
NOTE: This removes the CSRF check for downloading files. The check is nice to
have but only raises the barrier to entry slightly. Between iPad / sniffing /
flash bytecode attacks, single-domain installs are simply insecure. We could
restore the check at some point in conjunction with a derived authentication
cookie (i.e., a mini-session-token which is only useful for downloading files),
but that's a lot of complexity to drop all at once.
(Because files are now authenticated only by knowing the PHID and secret key,
this also fixes the "no profile pictures in public feed while logged out"
issue.)
Test Plan: Viewed, info'd, and downloaded files
Reviewers: btrahan, arice, alok
Reviewed By: arice
CC: aran, epriestley
Maniphest Tasks: T843
Differential Revision: https://secure.phabricator.com/D1608
2012-02-14 23:52:27 +01:00
|
|
|
$response->setMimeType($file->getViewableMimeType());
|
|
|
|
} else {
|
2013-12-13 04:42:12 +01:00
|
|
|
if (!$request->isHTTPPost() && !$alt_domain) {
|
|
|
|
// NOTE: Require POST to download files from the primary domain. We'd
|
|
|
|
// rather go full-bore and do a real CSRF check, but can't currently
|
|
|
|
// authenticate users on the file domain. This should blunt any
|
|
|
|
// attacks based on iframes, script tags, applet tags, etc., at least.
|
|
|
|
// Send the user to the "info" page if they're using some other method.
|
2014-08-18 23:11:06 +02:00
|
|
|
|
|
|
|
// This is marked as "external" because it is fully qualified.
|
Move ALL files to serve from the alternate file domain, not just files without
"Content-Disposition: attachment"
Summary:
We currently serve some files off the primary domain (with "Content-Disposition:
attachment" + a CSRF check) and some files off the alternate domain (without
either).
This is not sufficient, because some UAs (like the iPad) ignore
"Content-Disposition: attachment". So there's an attack that goes like this:
- Alice uploads xss.html
- Alice says to Bob "hey download this file on your iPad"
- Bob clicks "Download" on Phabricator on his iPad, gets XSS'd.
NOTE: This removes the CSRF check for downloading files. The check is nice to
have but only raises the barrier to entry slightly. Between iPad / sniffing /
flash bytecode attacks, single-domain installs are simply insecure. We could
restore the check at some point in conjunction with a derived authentication
cookie (i.e., a mini-session-token which is only useful for downloading files),
but that's a lot of complexity to drop all at once.
(Because files are now authenticated only by knowing the PHID and secret key,
this also fixes the "no profile pictures in public feed while logged out"
issue.)
Test Plan: Viewed, info'd, and downloaded files
Reviewers: btrahan, arice, alok
Reviewed By: arice
CC: aran, epriestley
Maniphest Tasks: T843
Differential Revision: https://secure.phabricator.com/D1608
2012-02-14 23:52:27 +01:00
|
|
|
return id(new AphrontRedirectResponse())
|
2014-08-18 23:11:06 +02:00
|
|
|
->setIsExternal(true)
|
Move ALL files to serve from the alternate file domain, not just files without
"Content-Disposition: attachment"
Summary:
We currently serve some files off the primary domain (with "Content-Disposition:
attachment" + a CSRF check) and some files off the alternate domain (without
either).
This is not sufficient, because some UAs (like the iPad) ignore
"Content-Disposition: attachment". So there's an attack that goes like this:
- Alice uploads xss.html
- Alice says to Bob "hey download this file on your iPad"
- Bob clicks "Download" on Phabricator on his iPad, gets XSS'd.
NOTE: This removes the CSRF check for downloading files. The check is nice to
have but only raises the barrier to entry slightly. Between iPad / sniffing /
flash bytecode attacks, single-domain installs are simply insecure. We could
restore the check at some point in conjunction with a derived authentication
cookie (i.e., a mini-session-token which is only useful for downloading files),
but that's a lot of complexity to drop all at once.
(Because files are now authenticated only by knowing the PHID and secret key,
this also fixes the "no profile pictures in public feed while logged out"
issue.)
Test Plan: Viewed, info'd, and downloaded files
Reviewers: btrahan, arice, alok
Reviewed By: arice
CC: aran, epriestley
Maniphest Tasks: T843
Differential Revision: https://secure.phabricator.com/D1608
2012-02-14 23:52:27 +01:00
|
|
|
->setURI(PhabricatorEnv::getProductionURI($file->getBestURI()));
|
|
|
|
}
|
|
|
|
$response->setMimeType($file->getMimeType());
|
|
|
|
$response->setDownload($file->getName());
|
|
|
|
}
|
|
|
|
|
Provide a setting which forces all file views to be served from an alternate
domain
Summary:
See D758, D759.
- Provide a strongly recommended setting which permits configuration of an
alternate domain.
- Lock cookies down better: set them on the exact domain, and use SSL-only if
the configuration is HTTPS.
- Prevent Phabriator from setting cookies on other domains.
This assumes D759 will land, it is not effective without that change.
Test Plan:
- Attempted to login from a different domain and was rejected.
- Logged out, logged back in normally.
- Put install in setup mode and verified it revealed a warning.
- Configured an alterate domain.
- Tried to view an image with an old URI, got a 400.
- Went to /files/ and verified links rendered to the alternate domain.
- Viewed an alternate domain file.
- Tried to view an alternate domain file without the secret key, got a 404.
Reviewers: andrewjcg, erling, aran, tuomaspelkonen, jungejason, codeblock
CC: aran
Differential Revision: 760
2011-08-02 07:24:00 +02:00
|
|
|
return $response;
|
|
|
|
}
|
|
|
|
}
|