Always generate RSA keys of exact bit length (#1336)

Fix RSA key generation code used when no native crypto library is available
(i.e. no NodeCrypto or WebCrypto). Now generated keys are always of exact bit
length. This was not guaranteed before, and it was common for keys to be one
bit shorter than expected.

Also, remove leftover code related to legacy WebCrypto interfaces (for IE11 and
Safari 10).

src/crypto/mode/gcm.js

@@ -25,7 +25,7 @@
 import { AES_GCM } from '@openpgp/asmcrypto.js/dist_es8/aes/gcm';
 import util from '../../util';
 
-const webCrypto = util.getWebCrypto(); // no GCM support in IE11, Safari 9
+const webCrypto = util.getWebCrypto();
 const nodeCrypto = util.getNodeCrypto();
 const Buffer = util.getNodeBuffer();
 

src/crypto/public_key/prime.js

@@ -25,7 +25,7 @@ import util from '../../util';
 import { getRandomBigInteger } from '../random';
 
 /**
- * Probabilistic random number generator
+ * Generate a probably prime random number
  * @param {Integer} bits - Bit length of the prime
  * @param {BigInteger} e - Optional RSA exponent to check against the prime
  * @param {Integer} k - Optional number of iterations of Miller-Rabin test

src/crypto/public_key/rsa.js

@@ -32,21 +32,6 @@ const webCrypto = util.getWebCrypto();
 const nodeCrypto = util.getNodeCrypto();
 const asn1 = nodeCrypto ? require('asn1.js') : undefined;
 
-// Helper for IE11 KeyOperation objects
-function promisifyIE11Op(keyObj, err) {
-  if (typeof keyObj.then !== 'function') { // IE11 KeyOperation
-    return new Promise(function(resolve, reject) {
-      keyObj.onerror = function () {
-        reject(new Error(err));
-      };
-      keyObj.oncomplete = function (e) {
-        resolve(e.target.result);
-      };
-    });
-  }
-  return keyObj;
-}
-
 /* eslint-disable no-invalid-this */
 const RSAPrivateKey = util.detectNode() ? asn1.define('RSAPrivateKey', function () {
   this.seq().obj( // used for native NodeJS crypto
@@ -178,44 +163,19 @@ export async function generate(bits, e) {
 
   // Native RSA keygen using Web Crypto
   if (util.getWebCrypto()) {
-    let keyPair;
-    let keyGenOpt;
-    if ((globalThis.crypto && globalThis.crypto.subtle) || globalThis.msCrypto) {
-      // current standard spec
-      keyGenOpt = {
-        name: 'RSASSA-PKCS1-v1_5',
-        modulusLength: bits, // the specified keysize in bits
-        publicExponent: e.toUint8Array(), // take three bytes (max 65537) for exponent
-        hash: {
-          name: 'SHA-1' // not required for actual RSA keys, but for crypto api 'sign' and 'verify'
-        }
-      };
-      keyPair = webCrypto.generateKey(keyGenOpt, true, ['sign', 'verify']);
-      keyPair = await promisifyIE11Op(keyPair, 'Error generating RSA key pair.');
-    } else if (globalThis.crypto && globalThis.crypto.webkitSubtle) {
-      // outdated spec implemented by old Webkit
-      keyGenOpt = {
-        name: 'RSA-OAEP',
-        modulusLength: bits, // the specified keysize in bits
-        publicExponent: e.toUint8Array(), // take three bytes (max 65537) for exponent
-        hash: {
-          name: 'SHA-1' // not required for actual RSA keys, but for crypto api 'sign' and 'verify'
-        }
-      };
-      keyPair = await webCrypto.generateKey(keyGenOpt, true, ['encrypt', 'decrypt']);
-    } else {
-      throw new Error('Unknown WebCrypto implementation');
-    }
+    const keyGenOpt = {
+      name: 'RSASSA-PKCS1-v1_5',
+      modulusLength: bits, // the specified keysize in bits
+      publicExponent: e.toUint8Array(), // take three bytes (max 65537) for exponent
+      hash: {
+        name: 'SHA-1' // not required for actual RSA keys, but for crypto api 'sign' and 'verify'
+      }
+    };
+    const keyPair = await webCrypto.generateKey(keyGenOpt, true, ['sign', 'verify']);
 
     // export the generated keys as JsonWebKey (JWK)
     // https://tools.ietf.org/html/draft-ietf-jose-json-web-key-33
-    let jwk = webCrypto.exportKey('jwk', keyPair.privateKey);
-    jwk = await promisifyIE11Op(jwk, 'Error exporting RSA key pair.');
-
-    // parse raw ArrayBuffer bytes to jwk/json (WebKit/Safari/IE11 quirk)
-    if (jwk instanceof ArrayBuffer) {
-      jwk = JSON.parse(String.fromCharCode.apply(null, new Uint8Array(jwk)));
-    }
+    const jwk = await webCrypto.exportKey('jwk', keyPair.privateKey);
     // map JWK parameters to corresponding OpenPGP names
     return {
       n: b64ToUint8Array(jwk.n),
@@ -261,15 +221,23 @@ export async function generate(bits, e) {
   // RSA keygen fallback using 40 iterations of the Miller-Rabin test
   // See https://stackoverflow.com/a/6330138 for justification
   // Also see section C.3 here: https://nvlpubs.nist.gov/nistpubs/FIPS/NIST
-  let q = await randomProbablePrime(bits - (bits >> 1), e, 40);
-  let p = await randomProbablePrime(bits >> 1, e, 40);
+  let p;
+  let q;
+  let n;
+  do {
+    q = await randomProbablePrime(bits - (bits >> 1), e, 40);
+    p = await randomProbablePrime(bits >> 1, e, 40);
+    n = p.mul(q);
+  } while (n.bitLength() !== bits);
+
+  const phi = p.dec().imul(q.dec());
 
   if (q.lt(p)) {
     [p, q] = [q, p];
   }
-  const phi = p.dec().imul(q.dec());
+
   return {
-    n: p.mul(q).toUint8Array(),
+    n: n.toUint8Array(),
     e: e.toUint8Array(),
     d: e.modInv(phi).toUint8Array(),
     p: p.toUint8Array(),

src/crypto/random.js

@@ -103,8 +103,6 @@ export async function getRandomBytes(length) {
   const buf = new Uint8Array(length);
   if (typeof crypto !== 'undefined' && crypto.getRandomValues) {
     crypto.getRandomValues(buf);
-  } else if (typeof globalThis !== 'undefined' && typeof globalThis.msCrypto === 'object' && typeof globalThis.msCrypto.getRandomValues === 'function') {
-    globalThis.msCrypto.getRandomValues(buf);
   } else if (nodeCrypto) {
     const bytes = nodeCrypto.randomBytes(buf.length);
     buf.set(bytes);

test/crypto/rsa.js

@@ -10,10 +10,8 @@ chai.use(require('chai-as-promised'));
 
 const expect = chai.expect;
 
-/* eslint-disable no-unused-expressions */
 /* eslint-disable no-invalid-this */
-const native = util.getWebCrypto() || util.getNodeCrypto();
-module.exports = () => (!native ? describe.skip : describe)('basic RSA cryptography with native crypto', function () {
+module.exports = () => describe('basic RSA cryptography', function () {
   let sinonSandbox;
   let getWebCryptoStub;
   let getNodeCryptoStub;
@@ -47,6 +45,21 @@ module.exports = () => (!native ? describe.skip : describe)('basic RSA cryptogra
     expect(keyObject.p).to.exist;
     expect(keyObject.q).to.exist;
     expect(keyObject.u).to.exist;
+    expect(util.uint8ArrayBitLength(keyObject.n)).to.equal(bits);
+  });
+
+  it('generate rsa key - without native crypto', async function() {
+    const bits = 1024;
+    disableNative();
+    const keyObject = await crypto.publicKey.rsa.generate(bits, 65537);
+    enableNative();
+    expect(keyObject.n).to.exist;
+    expect(keyObject.e).to.exist;
+    expect(keyObject.d).to.exist;
+    expect(keyObject.p).to.exist;
+    expect(keyObject.q).to.exist;
+    expect(keyObject.u).to.exist;
+    expect(util.uint8ArrayBitLength(keyObject.n)).to.equal(bits);
   });
 
   it('sign and verify using generated key params', async function() {