Skip to content

allow to update a private key from file #5294

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
rrrooommmaaa merged 3 commits into from
Jul 14, 2023
Merged

allow to update a private key from file #5294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
725dba3
allow import a key from file
Jul 13, 2023
f6ea920
test fix
Jul 13, 2023
a8dfadc
test fix
Jul 13, 2023
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
4 changes: 2 additions & 2 deletions extension/chrome/elements/add_pubkey.htm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<!-- © 2016-2018 FlowCrypt Limited. Limitations apply. Contact human@flowcrypt.com -->

<!DOCTYPE html>
<!doctype html>
<html>
<head>
<meta content="text/html;charset=utf-8" http-equiv="Content-Type" />
Expand All @@ -12,7 +12,7 @@
<link rel="stylesheet" href="/css/open-sans.css" />
<link rel="stylesheet" href="/css/mobile-menu-styling.css" />
<link rel="stylesheet" href="/css/fine-uploader-new.css" />
<script type="text/template" src="/chrome/elements/hared/attach.template.htm" id="qq-template"></script>
<script type="text/template" src="/chrome/elements/shared/attach.template.htm" id="qq-template"></script>
</head>

<body id="settings">
Expand Down
2 changes: 1 addition & 1 deletion extension/chrome/settings/modules/add_key.htm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<link rel="icon" href="/img/logo/flowcrypt-logo-19-19.png" />
<link rel="stylesheet" href="/css/fine-uploader-new.css" />
<link rel="stylesheet" href="/css/mobile-menu-styling.css" />
<script type="text/template" src="/chrome/elements/hared/attach.template.htm" id="qq-template"></script>
<script type="text/template" src="/chrome/elements/shared/attach.template.htm" id="qq-template"></script>
</head>

<body id="settings">
Expand Down
5 changes: 4 additions & 1 deletion extension/chrome/settings/modules/add_key.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,13 @@ View.run(

private saveKeyAndContinue = async (key: Key) => {
await saveKeysAndPassPhrase(this.acctEmail, [key]); // resulting new_key checked above
/* eslint-disable @typescript-eslint/naming-convention */
await setPassphraseForPrvs(this.clientConfiguration, this.acctEmail, [key], {
passphrase: String($('.input_passphrase').val()),
passphrase_save: !!$('.input_passphrase_save').prop('checked'), // eslint-disable-line , @typescript-eslint/naming-convention
passphrase_save: !!$('.input_passphrase_save').prop('checked'),
passphrase_ensure_single_copy: false, // we require KeyImportUi to rejectKnown keys
});
/* eslint-enable @typescript-eslint/naming-convention */
BrowserMsg.send.reload(this.parentTabId, { advanced: true });
};

Expand Down
2 changes: 1 addition & 1 deletion extension/chrome/settings/modules/contacts.htm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<link rel="stylesheet" href="/css/mobile-menu-styling.css" />
<link rel="icon" href="/img/logo/flowcrypt-logo-19-19.png" />
<link rel="stylesheet" href="/css/fine-uploader-new.css" />
<script type="text/template" src="/chrome/elements/hared/attach.template.htm" id="qq-template"></script>
<script type="text/template" src="/chrome/elements/shared/attach.template.htm" id="qq-template"></script>
<style>
#fineuploader_button {
overflow: initial !important;
Expand Down
2 changes: 1 addition & 1 deletion extension/chrome/settings/modules/decrypt.htm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<link rel="stylesheet" href="/css/sweetalert2.css" />
<link rel="stylesheet" href="/css/open-sans.css" />
<link rel="stylesheet" href="/css/fine-uploader-new.css" />
<script type="text/template" src="/chrome/elements/hared/attach.template.htm" id="qq-template"></script>
<script type="text/template" src="/chrome/elements/shared/attach.template.htm" id="qq-template"></script>
<style>
iframe {
width: 100%;
Expand Down
29 changes: 25 additions & 4 deletions extension/chrome/settings/modules/my_key_update.htm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
<link rel="stylesheet" href="/css/animate.css" />
<link rel="stylesheet" href="/css/mobile-menu-styling.css" />
<link rel="icon" href="/img/logo/flowcrypt-logo-19-19.png" />
<link rel="stylesheet" href="/css/fine-uploader-new.css" />
<script type="text/template" src="/chrome/elements/shared/attach.template.htm" id="qq-template"></script>
</head>

<body id="settings">
Expand All @@ -21,16 +23,35 @@
</div>
<div class="line">FlowCrypt will also recreate a new public key from your updated private key.</div>
<div class="line">
<textarea class="input_private_key armored" data-test="input-prv-key" placeholder="Updated ASCII Armored Private Key" maxlength="80000"></textarea>
<ul style="list-style: none; margin-left: 0; padding-left: 0" class="source_selector display_none">
<li>
<input type="radio" name="source" id="source_file" data-test="source-file" value="file" />
<label for="source_file"> Load from a file</label>
</li>
<li>
<input type="radio" name="source" id="source_paste" data-test="source-paste" value="paste" />
<label for="source_paste"> Paste armored key directly </label>
</li>
</ul>
</div>
<div class="line">
<input class="input_passphrase" type="password" data-test="input-passphrase" placeholder="Private Key Passphrase" maxlength="256" />
<div class="source_file_container display_none">
<a href="#" id="fineuploader_button" class="display_none">file</a>
<div class="line display_none" id="fineuploader"></div>
</div>
<div class="source_paste_container display_none">
<div class="line">
<textarea class="input_private_key armored" data-test="input-prv-key" placeholder="Updated ASCII Armored Private Key" maxlength="80000"></textarea>
</div>
<div class="line">
<input class="input_passphrase" type="password" data-test="input-passphrase" placeholder="Private Key Passphrase" maxlength="256" />
</div>
<div class="line"><span class="button green long action_update_private_key" data-test="action-update-key">update private key</span></div>
</div>
<div class="line"><span class="button green long action_update_private_key" data-test="action-update-key">update private key</span></div>
<div class="line"><a href="#" class="action_show_public_key">Back to Public Key</a></div>
</div>

<script src="/lib/purify.js"></script>
<script src="/lib/fine-uploader.js"></script>
<script src="/lib/jquery.min.js"></script>
<script src="/lib/sweetalert2.js"></script>
<script src="/lib/openpgp.js"></script>
Expand Down
21 changes: 13 additions & 8 deletions extension/chrome/settings/modules/my_key_update.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ import { PassphraseStore } from '../../../js/common/platform/store/passphrase-st
import { AcctStore } from '../../../js/common/platform/store/acct-store.js';
import { InMemoryStore } from '../../../js/common/platform/store/in-memory-store.js';
import { InMemoryStoreKeys } from '../../../js/common/core/const.js';
import { KeyImportUi } from '../../../js/common/ui/key-import-ui.js';
import { saveKeysAndPassPhrase } from '../../../js/common/helpers.js';

View.run(
class MyKeyUpdateView extends View {
protected fesUrl?: string;
private readonly acctEmail: string;
private readonly keyImportUi = new KeyImportUi({});
private readonly fingerprint: string;
private readonly showKeyUrl: string;
private readonly inputPrivateKey = $('.input_private_key');
Expand Down Expand Up @@ -57,13 +60,15 @@ View.run(
);
} else {
$('#content').show();
this.keyImportUi.initPrvImportSrcForm(this.acctEmail, undefined);
this.pubLookup = new PubLookup(this.clientConfiguration);
[this.ki] = await KeyStore.get(this.acctEmail, [this.fingerprint]);
Assert.abortAndRenderErrorIfKeyinfoEmpty(this.ki ? [this.ki] : []);
$('.action_show_public_key').attr('href', this.showKeyUrl);
$('.email').text(this.acctEmail);
$('.fingerprint').text(Str.spaced(this.ki.fingerprints[0]));
this.inputPrivateKey.attr('placeholder', this.inputPrivateKey.attr('placeholder') + ' (' + this.ki.fingerprints[0] + ')');
$('.source_selector').css('display', 'block');
}
};

Expand All @@ -76,13 +81,10 @@ View.run(
};

private storeUpdatedKeyAndPassphrase = async (updatedPrv: Key, updatedPrvPassphrase: string) => {
/* eslint-disable @typescript-eslint/no-non-null-assertion */
const shouldSavePassphraseInStorage =
!this.clientConfiguration.forbidStoringPassPhrase() && !!(await PassphraseStore.get(this.acctEmail, this.ki!, true));
await KeyStore.add(this.acctEmail, updatedPrv);
await PassphraseStore.set('local', this.acctEmail, this.ki!, shouldSavePassphraseInStorage ? updatedPrvPassphrase : undefined);
await PassphraseStore.set('session', this.acctEmail, this.ki!, shouldSavePassphraseInStorage ? undefined : updatedPrvPassphrase);
/* eslint-enable @typescript-eslint/no-non-null-assertion */
/* eslint-disable @typescript-eslint/no-non-null-assertion, @typescript-eslint/naming-convention */
const passphrase_save = !this.clientConfiguration.forbidStoringPassPhrase() && !!(await PassphraseStore.get(this.acctEmail, this.ki!, true));
await saveKeysAndPassPhrase(this.acctEmail, [updatedPrv], { passphrase: updatedPrvPassphrase, passphrase_save, passphrase_ensure_single_copy: true });
/* eslint-enable @typescript-eslint/no-non-null-assertion, @typescript-eslint/naming-convention */
if (
this.clientConfiguration.canSubmitPubToAttester() &&
(await Ui.modal.confirm('Public and private key updated locally.\n\nUpdate public records with new Public Key?'))
Expand All @@ -101,11 +103,14 @@ View.run(
};

private updatePrivateKeyHandler = async () => {
const updatedKey = await KeyUtil.parse(String(this.inputPrivateKey.val()));
const updatedKeyEncrypted = await KeyUtil.parse(String(this.inputPrivateKey.val()));
const updatedKey = await KeyUtil.parse(KeyUtil.armor(updatedKeyEncrypted)); // create a "cloned" copy to decrypt later
const updatedKeyPassphrase = String($('.input_passphrase').val());
KeyImportUi.allowReselect();
if (typeof updatedKey === 'undefined') {
await Ui.modal.warning(Lang.setup.keyFormattedWell(this.prvHeaders.begin, String(this.prvHeaders.end)), Ui.testCompatibilityLink);
} else if (updatedKeyEncrypted.identities.length === 0) {
await Ui.modal.error(Lang.setup.prvHasUseridIssue);
Copy link
Contributor Author

@rrrooommmaaa rrrooommmaaa Jul 13, 2023

Choose a reason for hiding this comment

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

This page is now using KeyImportUi for some visual handling, but it's not using KeyImportUi.checkPrv as it is not easy to adapt it here (we can try this in a separate issue)

Also, what should the message be if there is something wrong with UserIds (we currently don't support UserIds that don't contain email address in them)
I added this text
image

Copy link
Contributor Author

@rrrooommmaaa rrrooommmaaa Jul 13, 2023

Choose a reason for hiding this comment

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

@sosnovsky is there a vision on whether we should support keys with uids without emails?

Copy link
Contributor Author

@rrrooommmaaa rrrooommmaaa Jul 13, 2023

Choose a reason for hiding this comment

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

We currently support identities missing email address for public keys of recipients, but compose module assumes emails[0] matches identities[0] but that's not necessarily true. I suggest to refactor this part. Instead of "synchronized" arrays we can have an array of objects of the type returned by Str.parseEmail, it has { email, name, full } -- everything that's required. full is the original identity

Copy link
Collaborator

@sosnovsky sosnovsky Jul 14, 2023

Choose a reason for hiding this comment

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

@sosnovsky is there a vision on whether we should support keys with uids without emails?

I think it's quite rare case and we didn't get any requests for such functionality, so we can leave it as is for now. Then we'll probably also need to update our mobile apps to support such keys.

We currently support identities missing email address for public keys of recipients, but compose module assumes emails[0] matches identities[0] but that's not necessarily true. I suggest to refactor this part.

That sounds good 👍

} else if (updatedKey.isPublic) {
await Ui.modal.warning(
'This was a public key. Please insert a private key instead. It\'s a block of text starting with "' + this.prvHeaders.begin + '"'
Expand Down
2 changes: 1 addition & 1 deletion extension/chrome/settings/setup.htm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<link rel="stylesheet" href="/lib/bootstrap/bootstrap.min.css" />
<link rel="stylesheet" href="/css/mobile-menu-styling.css" />
<link rel="icon" href="/img/logo/flowcrypt-logo-19-19.png" />
<script type="text/template" src="/chrome/elements/hared/attach.template.htm" id="qq-template"></script>
<script type="text/template" src="/chrome/elements/shared/attach.template.htm" id="qq-template"></script>
</head>

<body id="settings">
Expand Down
3 changes: 2 additions & 1 deletion extension/chrome/settings/setup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ import { InMemoryStore } from '../../js/common/platform/store/in-memory-store.js
/* eslint-disable @typescript-eslint/naming-convention */
export interface PassphraseOptions {
passphrase: string;
passphrase_save: boolean;
passphrase_save: boolean; // if true and not forbidden by OrgRules, save to the local storage, otherwise to session
passphrase_ensure_single_copy: boolean; // make sure the passphrase isn't both in local storage and session
}

export interface SetupOptions extends PassphraseOptions {
Expand Down
1 change: 1 addition & 0 deletions extension/chrome/settings/setup/setup-create-key.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export class SetupCreateKeyModule {
const opts: SetupOptions = {
passphrase: String($('#step_2a_manual_create .input_password').val()),
passphrase_save: Boolean($('#step_2a_manual_create .input_passphrase_save').prop('checked')),
passphrase_ensure_single_copy: false, // there can't be any saved passphrases for the new key
submit_main: this.view.shouldSubmitPubkey('#step_2a_manual_create .input_submit_key'),
submit_all: this.view.shouldSubmitPubkey('#step_2a_manual_create .input_submit_all'),
recovered: false,
Expand Down
1 change: 1 addition & 0 deletions extension/chrome/settings/setup/setup-import-key.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export class SetupImportKeyModule {
submit_main: this.view.shouldSubmitPubkey('#step_2b_manual_enter .input_submit_key'),
submit_all: this.view.shouldSubmitPubkey('#step_2b_manual_enter .input_submit_all'),
passphrase_save: Boolean($('#step_2b_manual_enter .input_passphrase_save').prop('checked')),
passphrase_ensure_single_copy: true,
recovered: false,
};
/* eslint-enable @typescript-eslint/naming-convention */
Expand Down
1 change: 1 addition & 0 deletions extension/chrome/settings/setup/setup-key-manager-autogen.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export class SetupWithEmailKeyManagerModule {
const setupOptions: SetupOptions = {
passphrase_save:
this.view.clientConfiguration.mustAutogenPassPhraseQuietly() || Boolean($('#step_2_ekm_choose_pass_phrase .input_passphrase_save').prop('checked')),
passphrase_ensure_single_copy: false, // there can't be any saved passphrases for the new key
submit_main: this.view.clientConfiguration.canSubmitPubToAttester(),
submit_all: false,
passphrase,
Expand Down
1 change: 1 addition & 0 deletions extension/chrome/settings/setup/setup-recover-key.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export class SetupRecoverKeyModule {
submit_all: false,
passphrase,
passphrase_save: true, // todo - reevaluate saving passphrase when recovering
passphrase_ensure_single_copy: true,
recovered: true,
};
await saveKeysAndPassPhrase(this.view.acctEmail, newlyMatchingKeys, options);
Expand Down
13 changes: 8 additions & 5 deletions extension/js/common/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ export const isCustomerUrlFesUsed = async (acctEmail: string) => {
export const setPassphraseForPrvs = async (clientConfiguration: ClientConfiguration, acctEmail: string, prvs: Key[], ppOptions: PassphraseOptions) => {
const storageType = ppOptions.passphrase_save && !clientConfiguration.forbidStoringPassPhrase() ? 'local' : 'session';
for (const prv of prvs) {
await PassphraseStore.set(storageType, acctEmail, { longid: KeyUtil.getPrimaryLongid(prv) }, ppOptions.passphrase);
const keyInfo = { longid: KeyUtil.getPrimaryLongid(prv) };
await PassphraseStore.set(storageType, acctEmail, keyInfo, ppOptions.passphrase);
if (ppOptions.passphrase_ensure_single_copy) {
// delete from the other storage
await PassphraseStore.set(storageType === 'local' ? 'session' : 'local', acctEmail, { longid: KeyUtil.getPrimaryLongid(prv) }, undefined);
}
}
};

Expand All @@ -41,8 +46,6 @@ const addOrReplaceKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], ppO
}
}
if (ppOptions !== undefined) {
// todo: it would be good to check that the passphrase isn't present in the other storage type
// though this situation is not possible with current use cases
await setPassphraseForPrvs(await ClientConfiguration.newInstance(acctEmail), acctEmail, prvs, ppOptions);
}
const { sendAs, full_name: name } = await AcctStore.get(acctEmail, ['sendAs', 'full_name']);
Expand Down Expand Up @@ -125,7 +128,7 @@ export const processAndStoreKeysFromEkmLocally = async ({
let ppOptions: PassphraseOptions | undefined; // the options to pass to saveKeysAndPassPhrase
if (!originalOptions?.passphrase && (await ClientConfiguration.newInstance(acctEmail)).mustAutogenPassPhraseQuietly()) {
// eslint-disable-next-line @typescript-eslint/naming-convention
ppOptions = { passphrase: PgpPwd.random(), passphrase_save: true };
ppOptions = { passphrase: PgpPwd.random(), passphrase_save: true, passphrase_ensure_single_copy: true };
} else {
ppOptions = originalOptions;
}
Expand Down Expand Up @@ -163,7 +166,7 @@ export const processAndStoreKeysFromEkmLocally = async ({
if (encryptedKeys?.keys.length) {
// new keys are about to be added, they must be accompanied with the passphrase setting
// eslint-disable-next-line @typescript-eslint/naming-convention
const effectivePpOptions = { passphrase: encryptedKeys.passphrase, passphrase_save: passphraseInLocalStorage };
const effectivePpOptions = { passphrase: encryptedKeys.passphrase, passphrase_save: passphraseInLocalStorage, passphrase_ensure_single_copy: true };
// ppOptions have special meaning in saveKeysAndPassPhrase(), they trigger `name` updates, todo: refactor in #4545
await saveKeysAndPassPhrase(acctEmail, encryptedKeys.keys, ppOptions ? effectivePpOptions : undefined);
if (!ppOptions) {
Expand Down
1 change: 1 addition & 0 deletions extension/js/common/lang.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export const Lang = {
keyBackupsNotAllowed: 'Key backups are not allowed on this domain.',
prvHasFixableCompatIssue:
'This key has minor usability issues that can be fixed. This commonly happens when importing keys from Symantec&trade; PGP Desktop or other legacy software. It may be missing User IDs, or it may be missing a self-signature. It is also possible that the key is simply expired.',
prvHasUseridIssue: 'The set of User IDs in this key is not supported.',
ppMatchAllSet: "Your pass phrase matches. Good job! You're all set.",
noKeys: 'Keys for your account were not set up yet - please ask your systems administrator.',
},
Expand Down
9 changes: 7 additions & 2 deletions extension/js/common/ui/key-import-ui.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ export class KeyImportUi {
submitKeyForAddrs.splice(submitKeyForAddrs.indexOf(email), 1);
};

// by unselecting, we allow to click on "Load from a file" and trigger the fineuploader again
public static allowReselect = () => {
$('input[type=radio][name=source]').prop('checked', false);
};

public onBadPassphrase: VoidCallback = () => undefined;

public initPrvImportSrcForm = (acctEmail: string, parentTabId: string | undefined, submitKeyForAddrs?: string[] | undefined) => {
Expand Down Expand Up @@ -181,7 +186,7 @@ export class KeyImportUi {
} else {
$('.input_private_key').val('').change().prop('disabled', false);
await Ui.modal.error('Not able to read this key. Make sure it is a valid PGP private key.', false, Ui.testCompatibilityLink);
$('input[type=radio][name=source]').removeAttr('checked');
KeyImportUi.allowReselect();
}
},
});
Expand Down Expand Up @@ -310,7 +315,7 @@ export class KeyImportUi {
throw new UserAlert('This was a public key. Please insert a private key instead. It\'s a block of text starting with "' + headers.begin + '"');
}
if (type === 'publicKey' && !k.isPublic) {
throw new UserAlert('This was a public key. Please insert a private key instead. It\'s a block of text starting with "' + headers.begin + '"');
throw new UserAlert('This was a private key. Please insert a public key instead. It\'s a block of text starting with "' + headers.begin + '"');
}
};

Expand Down
Loading