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

Issue #75: WOPI proof #79

Merged
merged 26 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f6dcaf4
Issue 75: Fix an assertion message.
donquixote Dec 19, 2024
e5c9092
Issue 75: Let WopiController check if token is NULL.
donquixote Dec 12, 2024
6697965
Issue 75: Spell URL uppercase in docs in CollaboraDiscovery*.
donquixote Dec 3, 2024
dcf431a
Issue 75: Extract getParsedXml() from getWopiClientUrl().
donquixote Nov 21, 2024
8265c3a
Issue 75: Extract parseXml() from getWopiClientURL() in CollaboraDisc…
donquixote Dec 17, 2024
9125d29
Issue 75: Intercept errors from XML parsing.
donquixote Dec 12, 2024
00911b5
Issue 75: Add CollaboraDiscoveryTest.
donquixote Dec 12, 2024
5dd05c9
Issue 75: Add proof key methods in CollaboraDiscovery.
donquixote Dec 12, 2024
fca29b8
Issue 75: Set custom response format for WOPI routes.
donquixote Dec 17, 2024
845564b
Issue 75: Throw AccessDeniedHttpException in WopiController instead o…
donquixote Dec 17, 2024
068299f
Issue 75: Extract WopiControllerTestBase from WopiControllerTest.
donquixote Dec 18, 2024
d1155ee
Issue 75: Fix query parameters in WopiControllerTestBase::createReque…
donquixote Dec 19, 2024
3ef1c4a
Issue 75: Require 'phpseclib/phpseclib' to handle RSA keys.
donquixote Dec 13, 2024
7400188
Issue 75: Add setting for wopi_proof.
donquixote Dec 17, 2024
a08be93
Issue 75: Add proofing to WOPI routes.
donquixote Dec 12, 2024
9f017f0
Issue 75: Generate proof key for demo and ExistingSite tests.
donquixote Dec 19, 2024
7ead437
Issue 75: Recommend to generate a proof key when this is installed in…
donquixote Dec 19, 2024
8703772
Issue 75: Add a post-update hook to set 'wopi_proof' setting.
donquixote Dec 20, 2024
aa43d11
Issue 75: Combine WopiTimeoutAccessCheck back into WopiProofAccessCheck.
donquixote Dec 20, 2024
6424f84
Issue 75: Use ->checkboxChecked() and ->checkboxNotChecked() in Confi…
donquixote Dec 20, 2024
2812bb4
Issue 75: Simplify the data provider in CollaboraDiscoveryTest.
donquixote Dec 20, 2024
46608d6
Issue 75: Fix class doc on CollaboraDiscovery.
donquixote Dec 20, 2024
3c10a1c
Issue 75: Add missing link in README.md.
donquixote Dec 20, 2024
2fc47cb
Issue 75: Don't check timeout in JwtTranscoderBase, rely on JWT class…
donquixote Jan 7, 2025
15dad30
Issue 75: Set a fake timestamp in JWT class.
donquixote Jan 7, 2025
2d0896c
Issue 75: Use is_numeric instead of preg_match().
donquixote Jan 8, 2025
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
7 changes: 7 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@ jobs:
run: |
docker-compose exec -T web ./vendor/bin/phpcs -s

- name: Generate proof key
run: |
set -x
docker-compose exec -T collabora coolconfig generate-proof-key
# Collabora becomes unavailable after the above command.
docker-compose restart collabora

- name: Drupal site install
run: |
docker-compose exec -T web ./vendor/bin/run drupal:site-install
Expand Down
14 changes: 12 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,14 @@ For a local demo or test installation, [see below](#development--demo-installati
- (optional) For the 'Collabora Online Group' sub-module, you also need the
[Group](https://drupa.org/project/group) and [Group Media](https://drupa.org/project/groupmedia) modules.

### Installation steps
### Collabora Online server preparation

It is recommended to generate a proof key as documented here:\
brummbar marked this conversation as resolved.
Show resolved Hide resolved
docker-compose exec collabora coolconfig generate-proof-key

If not, the proof mechanism needs to be disabled in the module settings.

### Module installation steps

See the [Drupal guide to install
modules](https://www.drupal.org/docs/extending-drupal/installing-modules).
Expand All @@ -39,7 +46,7 @@ field in the `composer.json` of your project.
Then you can go into Drupal logged as an admin and go to _Extend_. In
the list you should be able to find _Collabora Online_ and enable it.

From there you can access the module specific configuration.
From there you can access the module-specific configuration.

Please check the "Configuration" section below!

Expand Down Expand Up @@ -69,6 +76,9 @@ docker-compose up -d

docker-compose exec web composer install
docker-compose exec web ./vendor/bin/run drupal:site-install

docker-compose exec collabora coolconfig generate-proof-key
docker-compose restart collabora
```

Optionally, generate an admin login link.
Expand Down
9 changes: 6 additions & 3 deletions collabora_online.routing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ collabora-online.wopi.info:
action: 'info'
methods: [ GET ]
requirements:
_permission: 'access content'
_collabora_online_wopi_access: 'TRUE'
_format: 'collabora_online_wopi'
options:
parameters:
action:
Expand All @@ -62,7 +63,8 @@ collabora-online.wopi.contents:
action: 'content'
methods: [ GET ]
requirements:
_permission: 'access content'
_collabora_online_wopi_access: 'TRUE'
_format: 'collabora_online_wopi'
options:
parameters:
action:
Expand All @@ -77,7 +79,8 @@ collabora-online.wopi.save:
action: 'save'
methods: [ POST ]
requirements:
_permission: 'access content'
_collabora_online_wopi_access: 'TRUE'
_format: 'collabora_online_wopi'
options:
parameters:
action:
Expand Down
7 changes: 7 additions & 0 deletions collabora_online.services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,10 @@ services:
class: Drupal\collabora_online\Jwt\JwtTranscoder
Drupal\collabora_online\MediaHelperInterface:
class: Drupal\collabora_online\MediaHelper
Drupal\collabora_online\EventSubscriber\ExceptionWopiSubscriber: { }
Drupal\collabora_online\Access\WopiProofAccessCheck:
tags:
- { name: access_check, applies_to: _collabora_online_wopi_access }
Drupal\collabora_online\Access\WopiTimeoutAccessCheck:
tags:
- { name: access_check, applies_to: _collabora_online_wopi_access }
4 changes: 3 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
"require": {
"php": ">=8.1",
"drupal/key": "^1.19",
"firebase/php-jwt": "^6.10"
"firebase/php-jwt": "^6.10",
"phpseclib/phpseclib": "^3.0",
"symfony/error-handler": "^6.4|^7.1"
},
"require-dev": {
"composer/installers": "^2",
Expand Down
1 change: 1 addition & 0 deletions config/install/collabora_online.settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ cool:
access_token_ttl: 86400
# Disable cert checks.
disable_cert_check: false
wopi_proof: true
brummbar marked this conversation as resolved.
Show resolved Hide resolved
# Allow fullscreen - Enabled by default for functionality.
allowfullscreen: true
3 changes: 3 additions & 0 deletions config/schema/collabora_online.schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ collabora_online.settings:
disable_cert_check:
type: boolean
label: 'Disable cert checks.'
wopi_proof:
type: boolean
label: 'Verify WOPI proof header and timestamp.'
allowfullscreen:
type: boolean
label: 'Allow full-screen.'
Expand Down
229 changes: 229 additions & 0 deletions src/Access/WopiProofAccessCheck.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,229 @@
<?php

/*
* Copyright the Collabora Online contributors.
*
* SPDX-License-Identifier: MPL-2.0
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

declare(strict_types=1);

namespace Drupal\collabora_online\Access;

use Drupal\collabora_online\Cool\CollaboraDiscoveryInterface;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Access\AccessResultInterface;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Routing\Access\AccessInterface;
use Drupal\Core\Utility\Error;
use phpseclib3\Crypt\PublicKeyLoader;
use phpseclib3\Crypt\RSA;
use phpseclib3\Crypt\RSA\PublicKey;
use Psr\Log\LoggerInterface;
use Symfony\Component\DependencyInjection\Attribute\Autowire;
use Symfony\Component\HttpFoundation\Request;

/**
* Access checker to verify the WOPI token.
*
* This does not check the X-WOPI-Timestamp expiration.
*
* This is inspired by the wopi-lib package, see
* https://github.com/Champs-Libres/wopi-lib/blob/master/src/Service/ProofValidator.php.
*/
class WopiProofAccessCheck implements AccessInterface {

public function __construct(
protected readonly CollaboraDiscoveryInterface $discovery,
#[Autowire(service: 'logger.channel.collabora_online')]
protected readonly LoggerInterface $logger,
protected readonly ConfigFactoryInterface $configFactory,
) {}

/**
* Checks if the request has a WOPI proof.
*
* @param \Symfony\Component\HttpFoundation\Request $request
* The request.
*
* @return \Drupal\Core\Access\AccessResultInterface
* The access result.
*/
public function access(Request $request): AccessResultInterface {
$config = $this->configFactory->get('collabora_online.settings');
if (!($config->get('cool')['wopi_proof'] ?? TRUE)) {
return AccessResult::allowed()
->addCacheableDependency($config);
}
// Each incoming request will have a different proof and timestamp, so there
// is no point in caching.
return $this->doCheckAccess($request)
->setCacheMaxAge(0);
}

/**
* Checks if the request has a WOPI proof.
*
* @param \Symfony\Component\HttpFoundation\Request $request
* The request.
*
* @return \Drupal\Core\Access\AccessResult
* The access result.
*/
protected function doCheckAccess(Request $request): AccessResult {
$keys = $this->getKeys();
if (!isset($keys['current'])) {
return AccessResult::forbidden('Missing or incomplete WOPI proof keys.');
}
$signatures = $this->getSignatures($request);
if (!isset($signatures['current'])) {
return AccessResult::forbidden('Missing or incomplete WOPI proof headers.');
}
$subject = $this->getSubject($request);

// Try different key and signature combinations.
foreach ($keys as $key_name => $key) {
foreach ($signatures as $signature_name => $signature) {
if ($key_name === 'old' && $signature_name === 'old') {
// Don't verify an old signature with an old key.
continue;
}
$success = $key->verify($subject, $signature);
if ($success) {
return AccessResult::allowed();
}
}
}
return AccessResult::forbidden('WOPI proof mismatch.');
}

/**
* Gets the message to be signed.
*
* @param \Symfony\Component\HttpFoundation\Request $request
* The request.
*
* @return string
* The message to be signed.
*/
protected function getSubject(Request $request): string {
// This class is not responsible for checking the expiration, but it still
// needs the WOPI timestamp to build the message for the signature.
$timestamp_ticks = $request->headers->get('X-WOPI-Timestamp');
$token = $request->query->get('access_token', '');
$url = $request->getUri();
return sprintf(
'%s%s%s%s%s%s',
pack('N', strlen($token)),
$token,
pack('N', strlen($url)),
strtoupper($url),
pack('N', 8),
pack('J', $timestamp_ticks)
);
}

/**
* Gets RSA public keys from the discovery.xml.
*
* The discovery.xml has a current and an old key.
* This is to support situations when the key has been recently changed, but
* the incoming request was signed with the older key.
*
* @return array<'current'|'old', \phpseclib3\Crypt\RSA\PublicKey>
* Current and old public key, or just the current if they are the same, or
* empty array if none found.
*/
protected function getKeys(): array {
// Get current and old key.
// Remove empty values.
// If both are the same, keep only the current one.
$public_keys = array_unique(array_filter([
'current' => $this->discovery->getProofKey(),
'old' => $this->discovery->getProofKeyOld(),
brummbar marked this conversation as resolved.
Show resolved Hide resolved
]));
$key_objects = [];
foreach ($public_keys as $key_name => $key_str) {
$key_obj = $this->prepareKey($key_str, $key_name);
if ($key_obj === NULL) {
continue;
}
$key_objects[$key_name] = $key_obj;
}
return $key_objects;
}

/**
* Gets an RSA key object based on a string value.
*
* @param string $key_str
* Key string value from discovery.xml.
* @param 'current'|'old' $key_name
* Key name, only used for logging.
*
* @return \phpseclib3\Crypt\RSA\PublicKey|null
* An RSA public key object, or NULL on failure.
*/
protected function prepareKey(string $key_str, string $key_name): ?PublicKey {
try {
$key_object = PublicKeyLoader::loadPublicKey($key_str);
}
catch (\Throwable $e) {
$log_message = "Problem with the @name key from discovery.yml:<br>\n"
. Error::DEFAULT_ERROR_MESSAGE;
$log_args = Error::decodeException($e);
$log_args['@name'] = $key_name;
$this->logger->error($log_message, $log_args);
return NULL;
}
if (!$key_object instanceof PublicKey) {
$log_message = "Problem with the @name key from discovery.yml:<br>\n"
. "Expected RSA public key, found @type.";
$log_args = [
'@name' => $key_name,
'@type' => get_debug_type($key_object),
];
$this->logger->error($log_message, $log_args);
return NULL;
}
return $key_object
->withHash('sha256')
->withPadding(RSA::SIGNATURE_RELAXED_PKCS1);
}

/**
* Gets the current and old signature from the request.
*
* The request will have a current and an old signature.
* This is to support situations when the key has been recently changed, but
* the cached discovery.xml still has the old key.
*
* @param \Symfony\Component\HttpFoundation\Request $request
* Incoming request that may have a signature to be verified.
*
* @return array{current?: string, old?: string}
* Current and old signature from the request, decoded and ready for use.
* If they are the same, only one of them is returned.
* If no signatures are found, an empty array is returned.
*/
protected function getSignatures(Request $request): array {
// Get the current and old proof header.
// Remove empty values.
// If both are the same, keep only the current one.
brummbar marked this conversation as resolved.
Show resolved Hide resolved
$proof_headers = array_unique(array_filter([
'current' => $request->headers->get('X-WOPI-Proof'),
'old' => $request->headers->get('X-WOPI-ProofOld'),
]));
$decoded_proof_headers = array_map(
fn (string $header_value) => base64_decode($header_value, TRUE),
$proof_headers,
);
// Remove false values where decoding failed.
return array_filter($decoded_proof_headers);
}

}
Loading
Loading