Skip to content

Allow Windows users to specify OpenSSL options in the ownCloud config #4514

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

Merged
merged 4 commits into from
Sep 2, 2013
Merged

Allow Windows users to specify OpenSSL options in the ownCloud config #4514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fb34f49
Start a branch for easier OpenSSL configuration.
ringmaster Aug 18, 2013
9a263a5
Employ config option for OpenSSL config file, if provided.
ringmaster Aug 19, 2013
df7bfa4
Don't cache the pkey, skip generation if the keyfile exists
ringmaster Aug 26, 2013
39f4538
This function doesn't cache anymore. Adjusted PHPDoc to suit.
ringmaster Aug 26, 2013
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
17 changes: 9 additions & 8 deletions apps/files_encryption/hooks/hooks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,6 @@ class Hooks {
*/
public static function login($params) {
$l = new \OC_L10N('files_encryption');
//check if all requirements are met
if(!Helper::checkRequirements() || !Helper::checkConfiguration() ) {
$error_msg = $l->t("Missing requirements.");
$hint = $l->t('Please make sure that PHP 5.3.3 or newer is installed and that OpenSSL together with the PHP extension is enabled and configured properly. For now, the encryption app has been disabled.');
\OC_App::disable('files_encryption');
\OCP\Util::writeLog('Encryption library', $error_msg . ' ' . $hint, \OCP\Util::ERROR);
\OCP\Template::printErrorPage($error_msg, $hint);
}

$view = new \OC_FilesystemView('/');

Expand All @@ -54,6 +46,15 @@ public static function login($params) {

$util = new Util($view, $params['uid']);

//check if all requirements are met
if(!$util->ready() && (!Helper::checkRequirements() || !Helper::checkConfiguration())) {
$error_msg = $l->t("Missing requirements.");
$hint = $l->t('Please make sure that PHP 5.3.3 or newer is installed and that OpenSSL together with the PHP extension is enabled and configured properly. For now, the encryption app has been disabled.');
\OC_App::disable('files_encryption');
\OCP\Util::writeLog('Encryption library', $error_msg . ' ' . $hint, \OCP\Util::ERROR);
\OCP\Template::printErrorPage($error_msg, $hint);
}

// setup user, if user not ready force relogin
if (Helper::setupUser($util, $params['password']) === false) {
return false;
Expand Down
8 changes: 5 additions & 3 deletions apps/files_encryption/lib/crypt.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ public static function createKeypair() {

$return = false;

$res = openssl_pkey_new(array('private_key_bits' => 4096));
$res = Helper::getOpenSSLPkey();

if ($res === false) {
\OCP\Util::writeLog('Encryption library', 'couldn\'t generate users key-pair for ' . \OCP\User::getUser(), \OCP\Util::ERROR);
while ($msg = openssl_error_string()) {
\OCP\Util::writeLog('Encryption library', 'openssl_pkey_new() fails: ' . $msg, \OCP\Util::ERROR);
}
} elseif (openssl_pkey_export($res, $privateKey)) {
} elseif (openssl_pkey_export($res, $privateKey, null, Helper::getOpenSSLConfig())) {
// Get public key
$keyDetails = openssl_pkey_get_details($res);
$publicKey = $keyDetails['key'];
Expand All @@ -70,7 +70,9 @@ public static function createKeypair() {
);
} else {
\OCP\Util::writeLog('Encryption library', 'couldn\'t export users private key, please check your servers openSSL configuration.' . \OCP\User::getUser(), \OCP\Util::ERROR);
\OCP\Util::writeLog('Encryption library', openssl_error_string(), \OCP\Util::ERROR);
while($errMsg = openssl_error_string()) {
\OCP\Util::writeLog('Encryption library', $errMsg, \OCP\Util::ERROR);
}
}

return $return;
Expand Down
22 changes: 21 additions & 1 deletion apps/files_encryption/lib/helper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ public static function checkRequirements() {
* @return bool true if configuration seems to be OK
*/
public static function checkConfiguration() {
if(openssl_pkey_new(array('private_key_bits' => 4096))) {
if(self::getOpenSSLPkey()) {
return true;
} else {
while ($msg = openssl_error_string()) {
Expand All @@ -275,6 +275,26 @@ public static function checkConfiguration() {
}
}

/**
* Create an openssl pkey with config-supplied settings
* WARNING: This initializes a new private keypair, which is computationally expensive
* @return resource The pkey resource created
*/
public static function getOpenSSLPkey() {
return openssl_pkey_new(self::getOpenSSLConfig());
}

/**
* Return an array of OpenSSL config options, default + config
* Used for multiple OpenSSL functions
* @return array The combined defaults and config settings
*/
public static function getOpenSSLConfig() {
$config = array('private_key_bits' => 4096);
$config = array_merge(\OCP\Config::getSystemValue('openssl', array()), $config);
Copy link

Choose a reason for hiding this comment

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

This forces RSA key bit size to be 4096. What if i want 8192?

Copy link

Choose a reason for hiding this comment

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

Swap the arguments to array_merge or add a comment explaining why users should not be able to overwrite the key size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If this is something you want, file a new issue. This was hardcoded in the code when I got to it and I don't know why, though I assume whoever wrote it had a reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For reference, the 4096-bit key setting was added by @schiesbn in cfbdad9

Copy link

Choose a reason for hiding this comment

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

You are making openssl settings configurable via the config file, but private_key_bits can not be overwritten this way. This is not an independent issue. Anything else can be overwritten (assuming you are using the configuration consistently everywhere).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is specifically an independent issue, since the code behaves the same way it did before this PR, explicitly overriding any setting that was provided via the openssl.cnf. Prior to this PR, the custom openssl.cnf could only be provided by changing system environment variables, and yet if this was done, the extant code would have overridden the key length.

Added #4698 for this independent issue so that the change this PR is meant to address can be folded in before I have to rebase it for the fifth time.

return $config;
}

/**
* @brief glob uses different pattern than regular expressions, escape glob pattern only
* @param unescaped path
Expand Down
5 changes: 5 additions & 0 deletions config/config.sample.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,4 +214,9 @@
'preview_libreoffice_path' => '/usr/bin/libreoffice',
/* cl parameters for libreoffice / openoffice */
'preview_office_cl_parameters' => '',

// Extra SSL options to be used for configuration
'openssl' => array(
//'config' => '/absolute/location/of/openssl.cnf',
),
);