Skip to content

Add a flag to ingore signature check #106

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.

Sign up for GitHub

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

Closed
sharidas wants to merge 1 commit into from
Closed

Add a flag to ingore signature check #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion lib/AppInfo/Application.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ function (IAppContainer $c) {
return new Crypt($server->getLogger(),
$server->getUserSession(),
$server->getConfig(),
$server->getL10N($c->getAppName()));
$server->getL10N($c->getAppName()),
$server->getEventDispatcher());
}
});

Expand Down
28 changes: 26 additions & 2 deletions lib/Crypto/Crypt.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
use OCP\IL10N;
use OCP\ILogger;
use OCP\IUserSession;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\EventDispatcher\GenericEvent;

/**
* Class Crypt provides the encryption implementation of the default ownCloud
Expand Down Expand Up @@ -77,6 +79,9 @@ class Crypt {
/** @var IL10N */
private $l;

/** @var EventDispatcherInterface */
private $eventDispatcher;

/** @var array */
private $supportedCiphersAndKeySize = [
'AES-256-CTR' => 32,
Expand All @@ -85,17 +90,28 @@ class Crypt {
'AES-128-CFB' => 16,
];

/**
* This variable reads the custom event and updates the value accordingly
* Kindly do not manually edit the value of this variable.
* @var bool
*/
private $disableSignatureCheck = false;

/**
* @param ILogger $logger
* @param IUserSession $userSession
* @param IConfig $config
* @param IL10N $l
*/
public function __construct(ILogger $logger, IUserSession $userSession, IConfig $config, IL10N $l) {
public function __construct(ILogger $logger,
IUserSession $userSession,
IConfig $config, IL10N $l,
EventDispatcherInterface $eventDispatcher) {
$this->logger = $logger;
$this->user = $userSession && $userSession->isLoggedIn() ? $userSession->getUser()->getUID() : '"no user given"';
$this->config = $config;
$this->l = $l;
$this->eventDispatcher = $eventDispatcher;
$this->supportedKeyFormats = ['hash', 'password'];
}

Expand Down Expand Up @@ -451,9 +467,17 @@ protected function isValidPrivateKey($plainKey) {
* @throws DecryptionFailedException
*/
public function symmetricDecryptFileContent($keyFileContents, $passPhrase, $cipher = self::DEFAULT_CIPHER, $version = 0, $position = 0) {
$this->eventDispatcher->addListener('files.aftersignaturemismatch', function (GenericEvent $event) {

@PVince81 PVince81 Mar 15, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's just use a static attribute... we already have this kind of dirty practice in the decrypt all command.

@PVince81 PVince81 Mar 15, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm now thinking this is not possible because of the encryption module abstraction.

@PVince81 PVince81 Mar 15, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idea would be if there was a way to inject generic flags or configs into the encryption module

in your first attempt you used the oc_appconfig table for that, which was a bit too global.

@sharidas sharidas Mar 15, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole problem is here with the code flow

transfer commnad <----> Encryption Wrapper <-----> Crypt

The transfer needs to output the files which are problematic and then come back for a retry in the encryption wrapper to copy the file again and crypt should know that this time no signature check.

Even I was not fully satisfied with the event logic.

Will see what can be done here..

if ($event->hasArgument('retryWithIgnoreSignature')) {
$this->disableSignatureCheck = $event->getArgument('retryWithIgnoreSignature');
} else {
$this->disableSignatureCheck = false;
}
$event->stopPropagation();
}, 10);
$catFile = $this->splitMetaData($keyFileContents, $cipher);

if ($catFile['signature'] !== false) {
if (($catFile['signature'] !== false) && ($this->disableSignatureCheck === false)) {
$this->checkSignature($catFile['encrypted'], $passPhrase . $version . $position, $catFile['signature']);
}

Expand Down