From f8cbd5d1299ebdee4dc3e7e5237785574179d3fe Mon Sep 17 00:00:00 2001 From: the-djmaze <> Date: Fri, 28 Jan 2022 15:02:10 +0100 Subject: [PATCH] #89 Improved public/private key handling --- dev/Settings/User/OpenPgp.js | 3 +- dev/Stores/User/Pgp.js | 48 ++++++++--- dev/Styles/User/SettingsOpenPGP.less | 5 +- dev/View/Popup/OpenPgpGenerate.js | 1 - .../app/libraries/RainLoop/Actions/Pgp.php | 82 +++++++++++++------ .../app/libraries/snappymail/pgp/gnupg.php | 72 ++++------------ .../app/libraries/snappymail/pgp/gpg.php | 8 +- .../Views/User/PopupsOpenPgpGenerate.html | 4 +- .../templates/Views/User/SettingsOpenPGP.html | 46 +++++++++-- 9 files changed, 160 insertions(+), 109 deletions(-) diff --git a/dev/Settings/User/OpenPgp.js b/dev/Settings/User/OpenPgp.js index 73f18c5e4..5ebb065a2 100644 --- a/dev/Settings/User/OpenPgp.js +++ b/dev/Settings/User/OpenPgp.js @@ -13,7 +13,8 @@ import { Settings } from 'Common/Globals'; export class OpenPgpUserSettings /*extends AbstractViewSettings*/ { constructor() { - this.gnupgkeys = PgpUserStore.gnupgKeys; + this.gnupgPublicKeys = PgpUserStore.gnupgPublicKeys; + this.gnupgPrivateKeys = PgpUserStore.gnupgPrivateKeys; this.openpgpkeysPublic = PgpUserStore.openpgpPublicKeys; this.openpgpkeysPrivate = PgpUserStore.openpgpPrivateKeys; diff --git a/dev/Stores/User/Pgp.js b/dev/Stores/User/Pgp.js index b3ab19b7a..f26347014 100644 --- a/dev/Stores/User/Pgp.js +++ b/dev/Stores/User/Pgp.js @@ -58,11 +58,7 @@ class OpenPgpKeyModel { this.key = key; const aEmails = []; if (key.users) { - key.users.forEach(user => { - if (user.userID.email) { - aEmails.push(user.userID.email); - } - }); + key.users.forEach(user => user.userID.email && aEmails.push(user.userID.email)); } this.id = key.getKeyID().toHex(); this.fingerprint = key.getFingerprint(); @@ -101,7 +97,8 @@ export const PgpUserStore = new class { * [ {email, can_encrypt, can_sign}, ... ] */ this.gnupgKeyring; - this.gnupgKeys = ko.observableArray(); + this.gnupgPublicKeys = ko.observableArray(); + this.gnupgPrivateKeys = ko.observableArray(); // OpenPGP.js this.openpgpPublicKeys = ko.observableArray(); @@ -159,12 +156,42 @@ export const PgpUserStore = new class { if (Settings.capa(Capa.GnuPG)) { this.gnupgKeyring = null; - this.gnupgKeys([]); + this.gnupgPublicKeys([]); + this.gnupgPrivateKeys([]); Remote.request('GnupgGetKeys', (iError, oData) => { if (oData && oData.Result) { this.gnupgKeyring = oData.Result; - this.gnupgKeys(Object.values(oData.Result)); + const initKey = (key, isPrivate) => { + const aEmails = []; + key.id = key.subkeys[0].keyid; + key.uids.forEach(uid => uid.email && aEmails.push(uid.email)); + key.emails = aEmails; + key.askDelete = ko.observable(false); + key.openForDeletion = ko.observable(null).askDeleteHelper(); + key.remove = () => { + if (key.askDelete()) { + Remote.request('GnupgDeleteKey', + (iError, oData) => { + if (oData && oData.Result) { + if (isPrivate) { + PgpUserStore.gnupgPrivateKeys.remove(key); + } else { + PgpUserStore.gnupgPublicKeys.remove(key); + } + delegateRunOnDestroy(key); + } + }, { + KeyId: key.id, + isPrivate: isPrivate + } + ); + } + } + return key; + }; + this.gnupgPublicKeys(oData.Result.public.map(key => initKey(key, 0))); + this.gnupgPrivateKeys(oData.Result.private.map(key => initKey(key, 1))); console.log('gnupg ready'); } } @@ -200,8 +227,6 @@ export const PgpUserStore = new class { keyPair.revocationCertificate keyPair.onServer keyPair.inGnuPG - keyPair.uid.name - keyPair.uid.email */ storeKeyPair(keyPair, callback) { // if (Settings.capa(Capa.GnuPG)) { @@ -213,9 +238,6 @@ export const PgpUserStore = new class { callback && callback(iError, oData); }, keyPair ); -// storeKeys(publicKeysItem); -// storeKeys(privateKeysItem); - openpgp.readKey({armoredKey:keyPair.publicKey}).then(key => { PgpUserStore.openpgpPublicKeys.push(new OpenPgpKeyModel(keyPair.publicKey, key)); storeOpenPgpKeys(PgpUserStore.openpgpPublicKeys, publicKeysItem); diff --git a/dev/Styles/User/SettingsOpenPGP.less b/dev/Styles/User/SettingsOpenPGP.less index 57697082e..87901c72a 100644 --- a/dev/Styles/User/SettingsOpenPGP.less +++ b/dev/Styles/User/SettingsOpenPGP.less @@ -8,11 +8,12 @@ width: 1%; } - .open-pgp-key-user { + .key-user { + margin: 0 0.5em; white-space: nowrap; } - .delete-open-pgp-key:not(:hover) { + .delete-key:not(:hover) { opacity: 0.7; } } diff --git a/dev/View/Popup/OpenPgpGenerate.js b/dev/View/Popup/OpenPgpGenerate.js index 0b7277c3b..fa3a7b2e5 100644 --- a/dev/View/Popup/OpenPgpGenerate.js +++ b/dev/View/Popup/OpenPgpGenerate.js @@ -65,7 +65,6 @@ export class OpenPgpGeneratePopupView extends AbstractViewPopup { if (keyPair) { keyPair.onServer = this.saveServer() ? 1 : 0; keyPair.inGnuPG = this.saveGnuPG() ? 1 : 0; - keyPair.uid = userId; PgpUserStore.storeKeyPair(keyPair, ()=>{ this.submitRequest(false); this.cancelCommand(); diff --git a/snappymail/v/0.0.0/app/libraries/RainLoop/Actions/Pgp.php b/snappymail/v/0.0.0/app/libraries/RainLoop/Actions/Pgp.php index 0a96ecc09..8c18a8d4d 100644 --- a/snappymail/v/0.0.0/app/libraries/RainLoop/Actions/Pgp.php +++ b/snappymail/v/0.0.0/app/libraries/RainLoop/Actions/Pgp.php @@ -13,7 +13,7 @@ trait Pgp */ public function GnuPG() : ?\SnappyMail\PGP\GnuPG { - $oAccount = $this->getAccountFromToken(); + $oAccount = $this->getMainAccountFromToken(); if (!$oAccount) { return null; } @@ -95,9 +95,7 @@ trait Pgp public function DoGnupgGetKeys() : array { $GPG = $this->GnuPG(); - return $GPG - ? $this->DefaultResponse(__FUNCTION__, $GPG->keyInfo('')) - : $this->FalseResponse(__FUNCTION__); + return $this->DefaultResponse(__FUNCTION__, $GPG ? $GPG->keyInfo('') : false); } public function DoGnupgGenerateKey() : array @@ -112,9 +110,15 @@ trait Pgp $this->GetActionParam('Passphrase', '') ); } - return $fingerprint - ? $this->DefaultResponse(__FUNCTION__, $fingerprint) - : $this->FalseResponse(__FUNCTION__); + return $this->DefaultResponse(__FUNCTION__, $fingerprint); + } + + public function DoGnupgDeleteKey() : array + { + $GPG = $this->GnuPG(); + $sKeyId = $this->GetActionParam('KeyId', ''); + $bPrivate = !!$this->GetActionParam('isPrivate', 0); + return $this->DefaultResponse(__FUNCTION__, $GPG ? $GPG->deleteKey($sKeyId, $bPrivate) : false); } public function DoGnupgImportKey() : array @@ -155,9 +159,7 @@ trait Pgp } $GPG = $sKey ? $this->GnuPG() : null; - return $GPG - ? $this->DefaultResponse(__FUNCTION__, $GPG->import($sKey)) - : $this->FalseResponse(__FUNCTION__); + return $this->DefaultResponse(__FUNCTION__, $GPG ? $GPG->import($sKey) : false); } /** @@ -195,21 +197,55 @@ trait Pgp return $this->DefaultResponse(__FUNCTION__, $keys); } + /** + * Used to store generated armored key pair from OpenPGP.js + * Handy when using multiple browsers + */ + public function DoPgpStoreKeyPair() : array + { + $result = [ + 'onServer' => [false, false, false], + 'inGnuPG' => [false, false, false] + ]; + $publicKey = $this->GetActionParam('publicKey', ''); + $privateKey = $this->GetActionParam('privateKey', ''); + $revocationCertificate = $this->GetActionParam('revocationCertificate', ''); + if ($this->GetActionParam('onServer', '')) { + $result['onServer'] = [ + $this->StorePGPKey($publicKey), + $this->StorePGPKey($privateKey), + false // $this->StorePGPKey($revocationCertificate) + ]; + } + if ($this->GetActionParam('inGnuPG', '')) { + $GPG = $this->GnuPG(); + if ($GPG) { + $result['inGnuPG'] = [ + $publicKey && $GPG->import($publicKey), + $privateKey && $GPG->import($privateKey), + false // $revocationCertificate && $GPG->import($revocationCertificate) + ]; + } + } + return $this->DefaultResponse(__FUNCTION__, $result); + } + /** * Used to store key from OpenPGP.js * Handy when using multiple browsers */ public function DoStorePGPKey() : array { - $oAccount = $this->getMainAccountFromToken(); - if (!$oAccount) { - return null; - } - $key = $this->GetActionParam('Key', ''); $keyId = $this->GetActionParam('KeyId', ''); - $result = false; - if ($key && $keyId) { + return $this->DefaultResponse(__FUNCTION__, ($key && $keyId && $this->StorePGPKey($key, $keyId))); + } + + private function StorePGPKey(string $key, string $keyId = '') : bool + { + $oAccount = $this->getMainAccountFromToken(); + if ($oAccount) { + $keyId = $keyId ? "0x{$keyId}" : \sha1($key); $dir = $this->StorageProvider()->GenerateFilePath( $oAccount, \RainLoop\Providers\Storage\Enumerations\StorageType::PGP @@ -218,15 +254,13 @@ trait Pgp $hash = $oAccount->CryptKey(); $key = \SnappyMail\Crypt::Encrypt($key, $hash); $key[] = \hash_hmac('sha1', $key[2], $hash); - $result = \file_put_contents("{$dir}/0x{$keyId}.key", \json_encode($key)); - } else if (\str_contains($key, 'PGP PUBLIC KEY')) { - $result = \file_put_contents("{$dir}/0x{$keyId}_public.asc", $key); + return !!\file_put_contents("{$dir}/{$keyId}.key", \json_encode($key)); + } + if (\str_contains($key, 'PGP PUBLIC KEY')) { + return !!\file_put_contents("{$dir}/{$keyId}_public.asc", $key); } } - - return $result - ? $this->TrueResponse(__FUNCTION__) - : $this->FalseResponse(__FUNCTION__); + return false; } } diff --git a/snappymail/v/0.0.0/app/libraries/snappymail/pgp/gnupg.php b/snappymail/v/0.0.0/app/libraries/snappymail/pgp/gnupg.php index 92ce1b60a..f5d2d3122 100644 --- a/snappymail/v/0.0.0/app/libraries/snappymail/pgp/gnupg.php +++ b/snappymail/v/0.0.0/app/libraries/snappymail/pgp/gnupg.php @@ -166,6 +166,11 @@ class GnuPG : $this->GPG->decryptverifyFile($filename, $plaintext); } + public function deleteKey(string $keyId, bool $private) : bool + { + return $this->getGPG()->deleteKey($keyId, $private); + } + /** * Encrypts a given text */ @@ -285,64 +290,21 @@ class GnuPG */ public function keyInfo(string $pattern) : array { - $keys = []; + $keys = [ + 'public' => [], + 'private' => [] + ]; // Public - foreach ($this->handler()->keyinfo($pattern) as $info) { - if (!$info['disabled'] && !$info['expired'] && !$info['revoked']) { - foreach ($info['uids'] as $uid) { - $id = $uid['email']; - if (isset($keys[$id])) { - $keys[$id]['can_sign'] = $keys[$id]['can_sign'] || $info['can_sign']; - $keys[$id]['can_encrypt'] = $keys[$id]['can_encrypt'] || $info['can_encrypt']; - } else { - $keys[$id] = [ - 'name' => $uid['name'], - 'email' => $uid['email'], - // Public Key tasks - 'can_verify' => $info['can_sign'], - 'can_encrypt' => $info['can_encrypt'], - // Private Key tasks - 'can_sign' => false, - 'can_decrypt' => false, - // The keys - 'publicKeys' => [], - 'privateKeys' => [] - ]; - } - foreach ($info['subkeys'] as $key) { - $keys[$id]['publicKeys'][$key['fingerprint']] = $key; - } - } - } + foreach ($this->handler()->keyinfo($pattern) as $key) { + $key['can_verify'] = $key['can_sign']; + unset($key['can_sign']); + $keys['public'][] = $key; } // Private, read https://github.com/php-gnupg/php-gnupg/issues/5 - foreach ($this->handler()->keyinfo($pattern, 1) as $info) { - if (!$info['disabled'] && !$info['expired'] && !$info['revoked']) { - foreach ($info['uids'] as $uid) { - $id = $uid['email']; - if (isset($keys[$id])) { - $keys[$id]['can_sign'] = $keys[$id]['can_sign'] || $info['can_sign']; - $keys[$id]['can_decrypt'] = $keys[$id]['can_decrypt'] || $info['can_encrypt']; - } else { - $keys[$id] = [ - 'name' => $uid['name'], - 'email' => $uid['email'], - // Public Key tasks - 'can_verify' => false, - 'can_encrypt' => false, - // Private Key tasks - 'can_sign' => $info['can_sign'], - 'can_decrypt' => $info['can_encrypt'], - // The keys - 'publicKeys' => [], - 'privateKeys' => [] - ]; - } - foreach ($info['subkeys'] as $key) { - $keys[$id]['privateKeys'][$key['fingerprint']] = $key; - } - } - } + foreach ($this->handler()->keyinfo($pattern, 1) as $key) { + $key['can_decrypt'] = $key['can_encrypt']; + unset($key['can_encrypt']); + $keys['private'][] = $key; } return $keys; } diff --git a/snappymail/v/0.0.0/app/libraries/snappymail/pgp/gpg.php b/snappymail/v/0.0.0/app/libraries/snappymail/pgp/gpg.php index 12eaefb69..e35a9bcb0 100644 --- a/snappymail/v/0.0.0/app/libraries/snappymail/pgp/gpg.php +++ b/snappymail/v/0.0.0/app/libraries/snappymail/pgp/gpg.php @@ -87,8 +87,8 @@ class GPG throw new \Exception("socket name for '{$homedir}/S.gpg-agent.extra' is too long"); } - if (!\is_dir($homedir)) { - \mkdir($homedir, 0700, true); + if ($homedir && !\is_dir($homedir) && !\mkdir($homedir, 0700, true)) { + throw new \Exception("mkdir({$homedir}) failed"); } $this->options['homedir'] = $homedir; @@ -519,11 +519,9 @@ class GPG { $key = $this->keyInfo($keyId, $private ? 1 : 0); if (!$key) { - return false; -// throw new \Exception(($private ? 'Private' : 'Public') . ' key not found: ' . $keyId); + throw new \Exception(($private ? 'Private' : 'Public') . ' key not found: ' . $keyId); } if (!$private && $this->keyInfo($keyId, 1)) { - return false; throw new \Exception('Delete private key first: ' . $keyId); } diff --git a/snappymail/v/0.0.0/app/templates/Views/User/PopupsOpenPgpGenerate.html b/snappymail/v/0.0.0/app/templates/Views/User/PopupsOpenPgpGenerate.html index 10fa8b72e..c9ab6760d 100644 --- a/snappymail/v/0.0.0/app/templates/Views/User/PopupsOpenPgpGenerate.html +++ b/snappymail/v/0.0.0/app/templates/Views/User/PopupsOpenPgpGenerate.html @@ -7,9 +7,11 @@ × + +
- diff --git a/snappymail/v/0.0.0/app/templates/Views/User/SettingsOpenPGP.html b/snappymail/v/0.0.0/app/templates/Views/User/SettingsOpenPGP.html index b3cf8a6d4..7e8a25b4a 100644 --- a/snappymail/v/0.0.0/app/templates/Views/User/SettingsOpenPGP.html +++ b/snappymail/v/0.0.0/app/templates/Views/User/SettingsOpenPGP.html @@ -21,13 +21,43 @@
GnuPG
- + + + + + + + + + + + + @@ -42,8 +72,9 @@ @@ -60,8 +91,9 @@
Private keys
✍ - 🔒 🔓 - + + + + + + + + 🗑 +
Public keys
+ ✔ + 🔒 + + + + + + + + 🗑
🔒 + - + @@ -51,7 +82,7 @@ data-i18n="GLOBAL/ARE_YOU_SURE"> - 🗑 + 🗑
🔑 + - + @@ -69,7 +101,7 @@ data-i18n="GLOBAL/ARE_YOU_SURE"> - 🗑 + 🗑