Skip to content

Commit

Permalink
Merge pull request #654 from systemli/align-mailcrypt-handler
Browse files Browse the repository at this point in the history 
Fix persistence of hasMailCrypt attribute
  • Loading branch information
@y3n4
y3n4 authored Oct 18, 2024
2 parents d8b5d95 + f98fe21 commit 6e1ac1e
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 21 deletions.
5 changes: 2 additions & 3 deletions src/Command/UsersCheckPasswordCommand.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,12 @@ protected function execute(InputInterface $input, OutputInterface $output): ?int
$envVars['userdb_quota_rule'] = sprintf('*:storage=%dM', $user->getQuota());
}

// Optionally create mail_crypt key pair and recovery token
// (when MAIL_CRYPT >= 3 and not $userDbLookup)
// Optionally create mail_crypt key pair (when MAIL_CRYPT >= 3 and not $userDbLookup)
if ($this->mailCrypt >= 3 &&
false === $userDbLookup &&
false === $user->hasMailCrypt() &&
null === $user->getMailCryptPublicKey()) {
$this->mailCryptKeyHandler->create($user, $password);
$this->mailCryptKeyHandler->create($user, $password, true);
}

// Optionally set mail_crypt environment variables for checkpassword-reply command
Expand Down
22 changes: 15 additions & 7 deletions src/Handler/MailCryptKeyHandler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ class MailCryptKeyHandler
// Use elliptic curve type 'secp521r1' for MailCrypt keys
private const MAIL_CRYPT_PRIVATE_KEY_TYPE = OPENSSL_KEYTYPE_EC;
private const MAIL_CRYPT_CURVE_NAME = 'secp521r1';
const MESSAGE_OPENSSL_EXITED_UNSUCCESSFULLY = 'Transforming key to PKCS#8 with OpenSSL failed. OpenSSL exited unsuccessfully: ';
const MESSAGE_OPENSSL_OUTPUT_INVALID = 'Transforming key to PKCS#8 with OpenSSL failed. OpenSSL output is no valid PKCS#8 key: ';
const MESSAGE_SECRET_IS_NULL = 'secret should not be null';
const MESSAGE_DECRYPTION_FAILED = 'decryption of mailCryptSecretBox failed';

/**
* MailCryptPrivateKeyHandler constructor.
Expand Down Expand Up @@ -49,10 +53,10 @@ public function toPkcs8(string $privateKey): string
sodium_memzero($privateKey);

if (!$process->isSuccessful()) {
throw new Exception('Transforming key to PKCS#8 with OpenSSL failed. OpenSSL exited unsuccessfully: ' . $process->getErrorOutput());
throw new Exception(self::MESSAGE_OPENSSL_EXITED_UNSUCCESSFULLY . $process->getErrorOutput());
}
if (!str_starts_with($process->getOutput(), '-----BEGIN PRIVATE KEY-----')) {
throw new Exception('Transforming key to PKCS#8 with OpenSSL failed. OpenSSL output is no valid PKCS#8 key: ' . $process->getOutput());
throw new Exception(self::MESSAGE_OPENSSL_OUTPUT_INVALID . $process->getOutput());
}

return $process->getOutput();
Expand All @@ -61,7 +65,7 @@ public function toPkcs8(string $privateKey): string
/**
* @throws Exception
*/
public function create(User $user, string $password): void
public function create(User $user, string $password, ?bool $mailCryptEnable = false): void
{
$pKey = openssl_pkey_new([
'private_key_type' => self::MAIL_CRYPT_PRIVATE_KEY_TYPE,
Expand All @@ -80,6 +84,10 @@ public function create(User $user, string $password): void
// Clear variables with confidential content from memory
$keyPair->erase();

if (true === $mailCryptEnable) {
$user->setMailCrypt(true);
}

$this->manager->flush();
}

Expand All @@ -89,11 +97,11 @@ public function create(User $user, string $password): void
public function update(User $user, string $oldPassword, string $newPassword): void
{
if (null === $secret = $user->getMailCryptSecretBox()) {
throw new Exception('secret should not be null');
throw new Exception(self::MESSAGE_SECRET_IS_NULL);
}

if (null === $privateKey = CryptoSecretHandler::decrypt(CryptoSecret::decode($secret), $oldPassword)) {
throw new Exception('decryption of mailCryptSecretBox failed');
throw new Exception(self::MESSAGE_DECRYPTION_FAILED);
}

$this->updateWithPrivateKey($user, $privateKey, $newPassword);
Expand All @@ -116,11 +124,11 @@ public function updateWithPrivateKey(User $user, string $privateKey, string $pas
public function decrypt(User $user, string $password): string
{
if (null === $secret = $user->getMailCryptSecretBox()) {
throw new Exception('secret should not be null');
throw new Exception(self::MESSAGE_SECRET_IS_NULL);
}

if (null === $privateKey = CryptoSecretHandler::decrypt(CryptoSecret::decode($secret), $password)) {
throw new Exception('decryption of mailCryptSecretBox failed');
throw new Exception(self::MESSAGE_DECRYPTION_FAILED);
}

sodium_memzero($password);
Expand Down
9 changes: 3 additions & 6 deletions src/Handler/RegistrationHandler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,12 @@ public function handle(Registration $registration): void
$user = $this->buildUser($registration);

// Update password, generate MailCrypt keys, generate recovery token
// key material for mailCrypt is always generated, but only enabled if MAIL_CRYPT >= 2
$mailCryptEnable = $this->mailCrypt >= 2;
$this->passwordUpdater->updatePassword($user, $registration->getPlainPassword());
$this->mailCryptKeyHandler->create($user, $registration->getPlainPassword());
$this->mailCryptKeyHandler->create($user, $registration->getPlainPassword(), $mailCryptEnable);
$this->recoveryTokenHandler->create($user);

// Enable mailbox encryption
if ($this->mailCrypt >= 2) {
$user->setMailCrypt(true);
}

// We used to erase sensitive data here, but it's now done in RegistrationController
// as we need to print the plainRecoveryToken beforehand

Expand Down
10 changes: 5 additions & 5 deletions tests/Handler/MailCryptKeyHandlerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ private function createHandler(): MailCryptKeyHandler
public function testToPkcs8ExceptionOpenSSLFailed(): void
{
$this->expectException(Exception::class);
$this->expectExceptionMessage('Transforming key to PKCS#8 with OpenSSL failed. OpenSSL exited unsuccessfully:');
$this->expectExceptionMessage(MailCryptKeyHandler::MESSAGE_OPENSSL_EXITED_UNSUCCESSFULLY);
$handler = $this->createHandler();
$privateKey = 'brokenKey';
$handler->toPkcs8($privateKey);
Expand Down Expand Up @@ -57,7 +57,7 @@ public function testCreate(): void
public function testUpdateExceptionNullSecret(): void
{
$this->expectException(Exception::class);
$this->expectExceptionMessage('secret should not be null');
$this->expectExceptionMessage(MailCryptKeyHandler::MESSAGE_SECRET_IS_NULL);
$handler = $this->createHandler();
$user = new User();
$handler->update($user, 'oldPassword', 'newPassword');
Expand All @@ -66,7 +66,7 @@ public function testUpdateExceptionNullSecret(): void
public function testUpdateExceptionDecryptionFailed(): void
{
$this->expectException(Exception::class);
$this->expectExceptionMessage('decryption of mailCryptSecretBox failed');
$this->expectExceptionMessage(MailCryptKeyHandler::MESSAGE_DECRYPTION_FAILED);
$handler = $this->createHandler();
$user = new User();
$handler->create($user, 'password');
Expand Down Expand Up @@ -103,7 +103,7 @@ public function testUpdateWithPrivateKey(): void
public function testDecryptExceptionNullSecret(): void
{
$this->expectException(Exception::class);
$this->expectExceptionMessage('secret should not be null');
$this->expectExceptionMessage(MailCryptKeyHandler::MESSAGE_SECRET_IS_NULL);
$handler = $this->createHandler();
$user = new User();

Expand All @@ -113,7 +113,7 @@ public function testDecryptExceptionNullSecret(): void
public function testDecryptExceptionDecryptionFailed(): void
{
$this->expectException(Exception::class);
$this->expectExceptionMessage('decryption of mailCryptSecretBox failed');
$this->expectExceptionMessage(MailCryptKeyHandler::MESSAGE_DECRYPTION_FAILED);
$handler = $this->createHandler();
$user = new User();
$handler->create($user, 'password');
Expand Down

0 comments on commit 6e1ac1e

Please sign in to comment.