Fix tokens encrypted with Node crypto implementation being undecryptable in new Rust implementation. (#930)

* Encrypt with new padding algo when possible.

* formatting

* changelog

Cargo.lock

@@ -191,6 +191,15 @@ version = "0.8.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "06ea2b9bc92be3c2baa9334a323ebca2d6f074ff852cd1d7b11064035cd3868f"
 
+[[package]]
+name = "cpufeatures"
+version = "0.2.12"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "53fe5e26ff1b7aef8bca9c6080520cfb8d9333c7568e1829cef191a9723e5504"
+dependencies = [
+ "libc",
+]
+
 [[package]]
 name = "crypto-common"
 version = "0.1.6"
@@ -727,6 +736,7 @@ dependencies = [
  "serde",
  "serde_derive",
  "serde_json",
+ "sha1",
  "url",
 ]
 
@@ -1316,6 +1326,7 @@ dependencies = [
  "pkcs1",
  "pkcs8",
  "rand_core",
+ "sha2",
  "signature",
  "spki",
  "subtle",
@@ -1579,6 +1590,28 @@ dependencies = [
  "serde",
 ]
 
+[[package]]
+name = "sha1"
+version = "0.10.6"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e3bf829a2d51ab4a5ddf1352d8470c140cadc8301b2ae1789db023f01cedd6ba"
+dependencies = [
+ "cfg-if",
+ "cpufeatures",
+ "digest",
+]
+
+[[package]]
+name = "sha2"
+version = "0.10.8"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "793db75ad2bcafc3ffa7c68b215fee268f537982cd901d132f89c6343f3a3dc8"
+dependencies = [
+ "cfg-if",
+ "cpufeatures",
+ "digest",
+]
+
 [[package]]
 name = "signature"
 version = "2.2.0"

Cargo.toml

@@ -22,7 +22,8 @@ atom_syndication = "0.12"
 ruma = { version = "0.9", features = ["events", "html"] }
 reqwest = "0.11"
 rand = "0.8.5"
-rsa = "0.9.6"
+rsa = { version = "0.9.6", features = ["sha2"] }
 base64ct = { version = "1.6.0", features = ["alloc"] }
+sha1 = "0.10.6"
 [build-dependencies]
 napi-build = "2"

changelog.d/930.bugfix

@@ -0,0 +1,2 @@
+Track which key was used to encrypt secrets in storage, and encrypt/decrypt secrets in Rust.
+

src/tokens/UserTokenStore.ts

@@ -16,7 +16,7 @@ import { JiraOnPremClient } from "../jira/client/OnPremClient";
 import { JiraCloudClient } from "../jira/client/CloudClient";
 import { TokenError, TokenErrorCode } from "../errors";
 import { TypedEmitter } from "tiny-typed-emitter";
-import { hashId, TokenEncryption } from "../libRs"; 
+import { hashId, TokenEncryption, stringToAlgo } from "../libRs"; 
 
 const ACCOUNT_DATA_TYPE = "uk.half-shot.matrix-hookshot.github.password-store:";
 const ACCOUNT_DATA_GITLAB_TYPE = "uk.half-shot.matrix-hookshot.gitlab.password-store:";
@@ -32,7 +32,7 @@ export const AllowedTokenTypes = ["github", "gitlab", "jira"];
 interface StoredTokenData {
     encrypted: string|string[];
     keyId: string;
-    algorithm: 'rsa';
+    algorithm: 'rsa'|'rsa-pkcs1v15';
     instance?: string;
 }
 
@@ -102,7 +102,7 @@ export class UserTokenStore extends TypedEmitter<Emitter> {
         const data: StoredTokenData = {
             encrypted: tokenParts,
             keyId: this.keyId,
-            algorithm: "rsa",
+            algorithm: "rsa-pkcs1v15",
             instance: instanceUrl,
         };
         await this.intent.underlyingClient.setAccountData(key, data);
@@ -148,18 +148,15 @@ export class UserTokenStore extends TypedEmitter<Emitter> {
                 return null;
             }
             // For legacy we just assume it's the current configured key.
-            const algorithm = obj.algorithm ?? "rsa";
+            const algorithm = stringToAlgo(obj.algorithm ?? "rsa");
             const keyId = obj.keyId ?? this.keyId;
 
-            if (algorithm !== 'rsa') {
-                throw new Error(`Algorithm for stored data is '${algorithm}', but we only support RSA`);
-            }
             if (keyId !== this.keyId) {
                 throw new Error(`Stored data was encrypted with a different key to the one currently configured`);
             }
 
             const encryptedParts = typeof obj.encrypted === "string" ? [obj.encrypted] : obj.encrypted;
-            const token = this.tokenEncryption.decrypt(encryptedParts);
+            const token = this.tokenEncryption.decrypt(encryptedParts, algorithm);
             this.userTokens.set(key, token);
             return token;
         } catch (ex) {

src/tokens/mod.rs

@@ -5,7 +5,8 @@ use napi::bindgen_prelude::Buffer;
 use napi::Error;
 use rsa::pkcs1::DecodeRsaPrivateKey;
 use rsa::pkcs8::DecodePrivateKey;
-use rsa::{Pkcs1v15Encrypt, RsaPrivateKey, RsaPublicKey};
+use rsa::{Oaep, Pkcs1v15Encrypt, RsaPrivateKey, RsaPublicKey};
+use sha1::Sha1;
 
 static MAX_TOKEN_PART_SIZE: usize = 128;
 
@@ -31,6 +32,24 @@ enum DecryptError {
     FromUtf8(FromUtf8Error),
 }
 
+#[napi]
+pub enum Algo {
+    RSAOAEP,
+    RSAPKCS1v15,
+}
+
+#[napi]
+pub fn string_to_algo(algo_str: String) -> Result<Algo, Error> {
+    match algo_str.as_str() {
+        "rsa" => Ok(Algo::RSAOAEP),
+        "rsa-pkcs1v15" => Ok(Algo::RSAPKCS1v15),
+        _ => Err(Error::new(
+            napi::Status::GenericFailure,
+            "Unknown algorithm",
+        )),
+    }
+}
+
 impl TokenEncryption {
     pub fn new(private_key_data: Vec<u8>) -> Result<Self, TokenEncryptionError> {
         let data = String::from_utf8(private_key_data).map_err(TokenEncryptionError::FromUtf8)?;
@@ -72,11 +91,11 @@ impl JsTokenEncryption {
     }
 
     #[napi]
-    pub fn decrypt(&self, parts: Vec<String>) -> Result<String, Error> {
+    pub fn decrypt(&self, parts: Vec<String>, algo: Algo) -> Result<String, Error> {
         let mut result = String::new();
 
         for v in parts {
-            match self.decrypt_value(v) {
+            match self.decrypt_value(v, algo) {
                 Ok(new_value) => {
                     result += &new_value;
                     Ok(())
@@ -90,13 +109,22 @@ impl JsTokenEncryption {
         Ok(result)
     }
 
-    fn decrypt_value(&self, value: String) -> Result<String, DecryptError> {
+    fn decrypt_value(&self, value: String, algo: Algo) -> Result<String, DecryptError> {
         let raw_value = Base64::decode_vec(&value).map_err(DecryptError::Base64)?;
-        let decrypted_value = self
-            .inner
-            .private_key
-            .decrypt(Pkcs1v15Encrypt, &raw_value)
-            .map_err(DecryptError::Decryption)?;
+        let decrypted_value = match algo {
+            Algo::RSAOAEP => {
+                let padding = Oaep::new::<Sha1>();
+                self.inner
+                    .private_key
+                    .decrypt(padding, &raw_value)
+                    .map_err(DecryptError::Decryption)
+            }
+            Algo::RSAPKCS1v15 => self
+                .inner
+                .private_key
+                .decrypt(Pkcs1v15Encrypt, &raw_value)
+                .map_err(DecryptError::Decryption),
+        }?;
         let utf8_value = String::from_utf8(decrypted_value).map_err(DecryptError::FromUtf8)?;
         Ok(utf8_value)
     }

tests/tokens/tokenencryption.spec.ts

@@ -1,5 +1,5 @@
-import { TokenEncryption } from "../../src/libRs";
-import { RSAKeyPairOptions, generateKeyPair } from "node:crypto";
+import { Algo, TokenEncryption } from "../../src/libRs";
+import { RSAKeyPairOptions, generateKeyPair, publicEncrypt } from "node:crypto";
 import { expect } from "chai";
 
 describe("TokenEncryption", () => {
@@ -8,6 +8,18 @@ describe("TokenEncryption", () => {
     async function createTokenEncryption() {
         return new TokenEncryption(await keyPromise);
     }
+
+    async function legacyEncryptFunction(token: string) {
+        const MAX_TOKEN_PART_SIZE = 128;
+        const tokenParts: string[] = [];
+        let tokenSource = token;
+        while (tokenSource && tokenSource.length > 0) {
+            const part = tokenSource.slice(0, MAX_TOKEN_PART_SIZE);
+            tokenSource = tokenSource.substring(MAX_TOKEN_PART_SIZE);
+            tokenParts.push(publicEncrypt(await keyPromise, Buffer.from(part)).toString("base64"));
+        }
+        return tokenParts;
+    }
     
     before('generate RSA key', () => {
         // Generate this once since it will take an age.
@@ -49,7 +61,7 @@ describe("TokenEncryption", () => {
     it('should be able to decrypt from a single part into a string', async() => {
         const tokenEncryption = await createTokenEncryption();
         const value = tokenEncryption.encrypt('hello world');
-        const result = tokenEncryption.decrypt(value);
+        const result = tokenEncryption.decrypt(value, Algo.RSAPKCS1v15);
         expect(result).to.equal('hello world');
     });
     it('should be able to decrypt from many parts into string', async() => {
@@ -58,7 +70,7 @@ describe("TokenEncryption", () => {
         const tokenEncryption = await createTokenEncryption();
         const value = tokenEncryption.encrypt(plaintext);
         expect(value).to.have.lengthOf(2);
-        const result = tokenEncryption.decrypt(value);
+        const result = tokenEncryption.decrypt(value, Algo.RSAPKCS1v15);
         expect(result).to.equal(plaintext);
     });
     it('should support pkcs1 format keys', async() => {
@@ -66,4 +78,9 @@ describe("TokenEncryption", () => {
         const result = tokenEncryption.encrypt('hello world');
         expect(result).to.have.lengthOf(1);
     });
+    it('should be to decrypt a string from the old crypto implementation', async() => {
+        const legacyString = await legacyEncryptFunction('hello world');
+        const tokenEncryption = await createTokenEncryption();
+        expect(tokenEncryption.decrypt(legacyString, Algo.RSAOAEP)).to.equal('hello world');
+    });
 });