Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Practical URL Parameter Encryption #4171

Closed
wants to merge 13 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ config/*
!config/nginx.conf
!config/php-fpm.conf
!config/php.ini
docker-compose.*

######################
## VisualStudioCode ##
Expand Down
2 changes: 1 addition & 1 deletion actions/DisplayAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ private function createResponse(Request $request, BridgeAbstract $bridge, string
'last-modified' => gmdate('D, d M Y H:i:s ', $now) . 'GMT',
'content-type' => $format->getMimeType() . '; charset=' . $format->getCharset(),
];
return new Response($format->stringify(), 200, $headers);
return new Response($format->stringify($request), 200, $headers);
}

private function createFeedItemFromException($e, BridgeAbstract $bridge): FeedItem
Expand Down
10 changes: 10 additions & 0 deletions config.default.ini.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@
;enabled_bridges[] = YouTubeCommunityTabBridge
enabled_bridges[] = *

; Encrypted URL key. A random string between 16 and 64 characters long which is used to generate
; compressed and encrypted feed URLs, to keep private information secret.
;
; NEVER SHARE THIS KEY.
; A password generator should be used to create this string. Whitespace is NOT ALLOWED.
;
; If this value is empty (default), then encrypted URLs cannot be used.
; Example key (DO NOT USE THIS): "b3c7@hsLqk)P(SJvjCBDUy]GMg6RamdHxEWV8K9nA4QN.p_5"
enc_url_key = ""

; Defines the timezone used by RSS-Bridge
; Find a list of supported timezones at
; https://www.php.net/manual/en/timezones.php
Expand Down
2 changes: 1 addition & 1 deletion formats/AtomFormat.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class AtomFormat extends FormatAbstract
protected const ATOM_NS = 'http://www.w3.org/2005/Atom';
protected const MRSS_NS = 'http://search.yahoo.com/mrss/';

public function stringify()
public function stringify(?Request $request)
{
$document = new \DomDocument('1.0', $this->getCharset());
$document->formatOutput = true;
Expand Down
31 changes: 23 additions & 8 deletions formats/HtmlFormat.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class HtmlFormat extends FormatAbstract
{
const MIME_TYPE = 'text/html';

public function stringify()
public function stringify(?Request $request)
{
// This query string is url encoded
$queryString = $_SERVER['QUERY_STRING'];
Expand All @@ -13,14 +13,27 @@ public function stringify()
$formatFactory = new FormatFactory();
$formats = [];

if (str_contains(strtolower($queryString), strtolower(UrlEncryptionService::PARAMETER_NAME . '='))) {
$encryptionToken = 'yes';
} else {
$encryptionToken = null;
}

// Create all formats (except HTML)
$formatNames = $formatFactory->getFormatNames();
foreach ($formatNames as $formatName) {
if ($formatName === 'Html') {
continue;
}
// The format url is relative, but should be absolute in order to help feed readers.
$formatUrl = '?' . str_ireplace('format=Html', 'format=' . $formatName, $queryString);
if (str_contains(strtolower($queryString), 'format=html')) {
$formatUrl = '?' . str_ireplace('format=Html', 'format=' . $formatName, $queryString);
} else {
// If we're viewing the HtmlFormat and the 'format' GET parameter isn't here, this is likely an
// encrypted URL being viewed. Handle this by reconstructing the raw URL with the new format.
$formatUrl = '?' . http_build_query($request->toArray());
$formatUrl .= (strlen($formatUrl) > 1 ? '&' : '') . 'format=' . $formatName;
}
$formatObject = $formatFactory->create($formatName);
$formats[] = [
'url' => $formatUrl,
Expand Down Expand Up @@ -48,12 +61,14 @@ public function stringify()
}

$html = render_template(__DIR__ . '/../templates/html-format.html.php', [
'charset' => $this->getCharset(),
'title' => $feedArray['name'],
'formats' => $formats,
'uri' => $feedArray['uri'],
'items' => $items,
'donation_uri' => $donationUri,
'charset' => $this->getCharset(),
'title' => $feedArray['name'],
'formats' => $formats,
'uri' => $feedArray['uri'],
'items' => $items,
'donation_uri' => $donationUri,
'encryption_token' => $encryptionToken,
'bridge_name' => $request->get('bridge'),
]);
// Remove invalid characters
ini_set('mbstring.substitute_character', 'none');
Expand Down
2 changes: 1 addition & 1 deletion formats/JsonFormat.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class JsonFormat extends FormatAbstract
'uid',
];

public function stringify()
public function stringify(?Request $request)
{
$feedArray = $this->getFeed();

Expand Down
2 changes: 1 addition & 1 deletion formats/MrssFormat.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class MrssFormat extends FormatAbstract
protected const ATOM_NS = 'http://www.w3.org/2005/Atom';
protected const MRSS_NS = 'http://search.yahoo.com/mrss/';

public function stringify()
public function stringify(?Request $request)
{
$document = new \DomDocument('1.0', $this->getCharset());
$document->formatOutput = true;
Expand Down
2 changes: 1 addition & 1 deletion formats/PlaintextFormat.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class PlaintextFormat extends FormatAbstract
{
const MIME_TYPE = 'text/plain';

public function stringify()
public function stringify(?Request $request)
{
$feed = $this->getFeed();
foreach ($this->getItems() as $item) {
Expand Down
2 changes: 1 addition & 1 deletion formats/SfeedFormat.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class SfeedFormat extends FormatAbstract
{
const MIME_TYPE = 'text/plain';

public function stringify()
public function stringify(?Request $request)
{
$text = '';
foreach ($this->getItems() as $item) {
Expand Down
22 changes: 20 additions & 2 deletions lib/BridgeAbstract.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,27 @@ public function getMaintainer(): string
/**
* A more correct method name would have been "getContexts"
*/
public function getParameters(): array
final public function getParameters(): array
{
return static::PARAMETERS;
$parameters = static::PARAMETERS;

if (UrlEncryptionService::enabled()) {
// A parameter cannot be defined which collides with the special encryption parameter.
$illegalToken = array_key_exists(UrlEncryptionService::PARAMETER_NAME, $parameters);
foreach ($parameters as $k => $v) {
$illegalToken |= array_key_exists(UrlEncryptionService::PARAMETER_NAME, $v);
}

if ($illegalToken) {
throw new \Exception(
'The parameter name "' . UrlEncryptionService::PARAMETER_NAME
. '" is reserved for encrypted URLs. Remove this from the PARAMETERS definition in bridge "'
. $this->getName() . '".'
);
}
}

return $parameters;
}

public function getItems()
Expand Down
2 changes: 1 addition & 1 deletion lib/FormatAbstract.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ abstract class FormatAbstract

protected array $feed = [];

abstract public function stringify();
abstract public function stringify(?Request $request);

public function setFeed(array $feed)
{
Expand Down
6 changes: 6 additions & 0 deletions lib/ParameterValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ public function validateInput(array &$input, $contexts): array
$errors = [];

foreach ($input as $name => $value) {
if ($name === UrlEncryptionService::PARAMETER_NAME && UrlEncryptionService::enabled()) {
// Do not validate against encrypted URL tokens.
continue;
}

$registered = false;

foreach ($contexts as $contextName => $contextParameters) {
if (!array_key_exists($name, $contextParameters)) {
continue;
Expand Down
51 changes: 36 additions & 15 deletions lib/RssBridge.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,22 @@ public function __construct(

public function main(array $argv = []): Response
{
if ($argv) {
parse_str(implode('&', array_slice($argv, 1)), $cliArgs);
$request = Request::fromCli($cliArgs);
} else {
$request = Request::fromGlobals();
}

foreach ($request->toArray() as $key => $value) {
if (!is_string($value)) {
return new Response(render(__DIR__ . '/../templates/error.html.php', [
'message' => "Query parameter \"$key\" is not a string.",
]), 400);
}
}

// The check for maintenance mode should always occur first since it has no
// dependencies, and nothing else needs to come before it for bootstrapping.
if (Configuration::getConfig('system', 'enable_maintenance_mode')) {
return new Response(render(__DIR__ . '/../templates/error.html.php', [
'title' => '503 Service Unavailable',
'message' => 'RSS-Bridge is down for maintenance.',
]), 503);
}

if ($argv) {
parse_str(implode('&', array_slice($argv, 1)), $cliArgs);
$request = Request::fromCli($cliArgs);
} else {
$request = Request::fromGlobals();
}

// HTTP Basic auth check
if (Configuration::getConfig('authentication', 'enable')) {
if (Configuration::getConfig('authentication', 'password') === '') {
Expand All @@ -65,6 +59,33 @@ public function main(array $argv = []): Response
// At this point the username and password was correct
}

// If the URL contains an encrypted token, then the rest of the current URL
// parameters are discarded and the encrypted token is decrypted, decompressed,
// and expanded into the Request object's 'get' container. The user should NEVER
// be redirected to another URL that would expose what params that are in the
// current page.
if (
$request->get(UrlEncryptionService::PARAMETER_NAME)
&& UrlEncryptionService::enabled()
) {
try {
$request->tryDecryptUrl();
} catch (\Exception $e) {
return new Response(
render(__DIR__ . '/../templates/error.html.php', ['message' => $e->getMessage()]),
401
);
}
}

foreach ($request->toArray() as $key => $value) {
if (!is_string($value)) {
return new Response(render(__DIR__ . '/../templates/error.html.php', [
'message' => "Query parameter \"$key\" is not a string.",
]), 400);
}
}

// Add token as attribute to request
$request = $request->withAttribute('token', $request->get('token'));

Expand Down
Loading
Loading