Throw on empty passphrase in encryptKey and `SecretKeyPacket.encryp…

…t` (#1508)

Breaking change: `openpgp.encryptKey` now throws if an empty string is given as
passphrase. The operation used to succeed, but the resulting key was left in an
inconsistent state, and e.g. serialization would not be possible.

Non-breaking changes:
- `options.passphrase` in `generateKey` and `reformatKey` now defaults to
`undefined` instead of empty string. Passing an empty string does not throw for
now, but this might change in the future to align with `encryptKey`'s
behaviour.
- In TS, add `GenerateKeyOptions` as alias of `KeyOptions`, to clarify its
scope.

openpgp.d.ts

@@ -20,9 +20,9 @@ export function readPrivateKey(options: { armoredKey: string, config?: PartialCo
 export function readPrivateKey(options: { binaryKey: Uint8Array, config?: PartialConfig }): Promise<PrivateKey>;
 export function readPrivateKeys(options: { armoredKeys: string, config?: PartialConfig }): Promise<PrivateKey[]>;
 export function readPrivateKeys(options: { binaryKeys: Uint8Array, config?: PartialConfig }): Promise<PrivateKey[]>;
-export function generateKey(options: KeyOptions & { format?: 'armored' }): Promise<SerializedKeyPair<string> & { revocationCertificate: string }>;
-export function generateKey(options: KeyOptions & { format: 'binary' }): Promise<SerializedKeyPair<Uint8Array> & { revocationCertificate: string }>;
-export function generateKey(options: KeyOptions & { format: 'object' }): Promise<KeyPair & { revocationCertificate: string }>;
+export function generateKey(options: GenerateKeyOptions & { format?: 'armored' }): Promise<SerializedKeyPair<string> & { revocationCertificate: string }>;
+export function generateKey(options: GenerateKeyOptions & { format: 'binary' }): Promise<SerializedKeyPair<Uint8Array> & { revocationCertificate: string }>;
+export function generateKey(options: GenerateKeyOptions & { format: 'object' }): Promise<KeyPair & { revocationCertificate: string }>;
 export function decryptKey(options: { privateKey: PrivateKey; passphrase?: MaybeArray<string>; config?: PartialConfig }): Promise<PrivateKey>;
 export function encryptKey(options: { privateKey: PrivateKey; passphrase?: MaybeArray<string>; config?: PartialConfig }): Promise<PrivateKey>;
 export function reformatKey(options: { privateKey: PrivateKey; userIDs?: MaybeArray<UserID>; passphrase?: string; keyExpirationTime?: number; date?: Date, format?: 'armored', config?: PartialConfig }): Promise<SerializedKeyPair<string> & { revocationCertificate: string }>;
@@ -662,7 +662,7 @@ interface KeyPair {
 
 export type EllipticCurveName = 'ed25519' | 'curve25519' | 'p256' | 'p384' | 'p521' | 'secp256k1' | 'brainpoolP256r1' | 'brainpoolP384r1' | 'brainpoolP512r1';
 
-interface KeyOptions {
+interface GenerateKeyOptions {
   userIDs: MaybeArray<UserID>;
   passphrase?: string;
   type?: 'ecc' | 'rsa';
@@ -674,6 +674,7 @@ interface KeyOptions {
   format?: 'armored' | 'object' | 'binary';
   config?: PartialConfig;
 }
+export type KeyOptions = GenerateKeyOptions;
 
 interface SubkeyOptions {
   type?: 'ecc' | 'rsa';

src/openpgp.js

@@ -37,7 +37,7 @@ import { checkKeyRequirements } from './key/helper';
  * @param {Object} options
  * @param {Object|Array<Object>} options.userIDs - User IDs as objects: `{ name: 'Jo Doe', email: 'info@jo.com' }`
  * @param {'ecc'|'rsa'} [options.type='ecc'] - The primary key algorithm type: ECC (default) or RSA
- * @param {String} [options.passphrase=(not protected)] - The passphrase used to encrypt the generated private key. If omitted, the key won't be encrypted.
+ * @param {String} [options.passphrase=(not protected)] - The passphrase used to encrypt the generated private key. If omitted or empty, the key won't be encrypted.
  * @param {Number} [options.rsaBits=4096] - Number of bits for RSA keys
  * @param {String} [options.curve='curve25519'] - Elliptic curve for ECC keys:
  *                                             curve25519 (default), p256, p384, p521, secp256k1,
@@ -53,7 +53,7 @@ import { checkKeyRequirements } from './key/helper';
  * @async
  * @static
  */
-export async function generateKey({ userIDs = [], passphrase = '', type = 'ecc', rsaBits = 4096, curve = 'curve25519', keyExpirationTime = 0, date = new Date(), subkeys = [{}], format = 'armored', config, ...rest }) {
+export async function generateKey({ userIDs = [], passphrase, type = 'ecc', rsaBits = 4096, curve = 'curve25519', keyExpirationTime = 0, date = new Date(), subkeys = [{}], format = 'armored', config, ...rest }) {
   config = { ...defaultConfig, ...config }; checkConfig(config);
   userIDs = toArray(userIDs);
   const unknownOptions = Object.keys(rest); if (unknownOptions.length > 0) throw new Error(`Unknown option: ${unknownOptions.join(', ')}`);
@@ -86,7 +86,7 @@ export async function generateKey({ userIDs = [], passphrase = '', type = 'ecc',
  * @param {Object} options
  * @param {PrivateKey} options.privateKey - Private key to reformat
  * @param {Object|Array<Object>} options.userIDs - User IDs as objects: `{ name: 'Jo Doe', email: 'info@jo.com' }`
- * @param {String} [options.passphrase=(not protected)] - The passphrase used to encrypt the reformatted private key. If omitted, the key won't be encrypted.
+ * @param {String} [options.passphrase=(not protected)] - The passphrase used to encrypt the reformatted private key. If omitted or empty, the key won't be encrypted.
  * @param {Number} [options.keyExpirationTime=0 (never expires)] - Number of seconds from the key creation time after which the key expires
  * @param {Date}   [options.date] - Override the creation date of the key signatures. If the key was previously used to sign messages, it is recommended
  *                                  to set the same date as the key creation time to ensure that old message signatures will still be verifiable using the reformatted key.
@@ -97,7 +97,7 @@ export async function generateKey({ userIDs = [], passphrase = '', type = 'ecc',
  * @async
  * @static
  */
-export async function reformatKey({ privateKey, userIDs = [], passphrase = '', keyExpirationTime = 0, date, format = 'armored', config, ...rest }) {
+export async function reformatKey({ privateKey, userIDs = [], passphrase, keyExpirationTime = 0, date, format = 'armored', config, ...rest }) {
   config = { ...defaultConfig, ...config }; checkConfig(config);
   userIDs = toArray(userIDs);
   const unknownOptions = Object.keys(rest); if (unknownOptions.length > 0) throw new Error(`Unknown option: ${unknownOptions.join(', ')}`);

src/packet/secret_key.js

@@ -275,11 +275,8 @@ class SecretKeyPacket extends PublicKeyPacket {
       throw new Error('Key packet is already encrypted');
     }
 
-    if (this.isDecrypted() && !passphrase) {
-      this.s2kUsage = 0;
-      return;
-    } else if (!passphrase) {
-      throw new Error('The key must be decrypted before removing passphrase protection.');
+    if (!passphrase) {
+      throw new Error('A non-empty passphrase is required for key encryption.');
     }
 
     this.s2k = new S2K(config);

test/general/openpgp.js

@@ -1192,6 +1192,14 @@ module.exports = () => describe('OpenPGP.js public api tests', function() {
       expect(unlocked.isDecrypted()).to.be.true;
     });
 
+    it('should throw on empty passphrase', async function() {
+      const { privateKey } = await openpgp.generateKey({ userIDs: [{ name: 'test', email: 'test@test.com' }], format: 'object' });
+      await expect(openpgp.encryptKey({
+        privateKey,
+        passphrase: ''
+      })).to.be.rejectedWith(/passphrase is required for key encryption/);
+    });
+
     it('should support multiple passphrases', async function() {
       const { privateKey } = await openpgp.generateKey({ userIDs: [{ name: 'test', email: 'test@test.com' }], format: 'object' });
       const passphrases = ['123', '456'];