Skip to content

Commit cf68991

Browse files
authored
Fix tokens encrypted with Node crypto implementation being undecryptable in new Rust implementation. (#930)
* Encrypt with new padding algo when possible. * formatting * changelog
1 parent 79bfffc commit cf68991

File tree

6 files changed

+100
-22
lines changed

6 files changed

+100
-22
lines changed

Cargo.lock

Lines changed: 33 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ atom_syndication = "0.12"
2222
ruma = { version = "0.9", features = ["events", "html"] }
2323
reqwest = "0.11"
2424
rand = "0.8.5"
25-
rsa = "0.9.6"
25+
rsa = { version = "0.9.6", features = ["sha2"] }
2626
base64ct = { version = "1.6.0", features = ["alloc"] }
27+
sha1 = "0.10.6"
2728
[build-dependencies]
2829
napi-build = "2"

changelog.d/930.bugfix

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Track which key was used to encrypt secrets in storage, and encrypt/decrypt secrets in Rust.
2+

src/tokens/UserTokenStore.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { JiraOnPremClient } from "../jira/client/OnPremClient";
1616
import { JiraCloudClient } from "../jira/client/CloudClient";
1717
import { TokenError, TokenErrorCode } from "../errors";
1818
import { TypedEmitter } from "tiny-typed-emitter";
19-
import { hashId, TokenEncryption } from "../libRs";
19+
import { hashId, TokenEncryption, stringToAlgo } from "../libRs";
2020

2121
const ACCOUNT_DATA_TYPE = "uk.half-shot.matrix-hookshot.github.password-store:";
2222
const ACCOUNT_DATA_GITLAB_TYPE = "uk.half-shot.matrix-hookshot.gitlab.password-store:";
@@ -32,7 +32,7 @@ export const AllowedTokenTypes = ["github", "gitlab", "jira"];
3232
interface StoredTokenData {
3333
encrypted: string|string[];
3434
keyId: string;
35-
algorithm: 'rsa';
35+
algorithm: 'rsa'|'rsa-pkcs1v15';
3636
instance?: string;
3737
}
3838

@@ -102,7 +102,7 @@ export class UserTokenStore extends TypedEmitter<Emitter> {
102102
const data: StoredTokenData = {
103103
encrypted: tokenParts,
104104
keyId: this.keyId,
105-
algorithm: "rsa",
105+
algorithm: "rsa-pkcs1v15",
106106
instance: instanceUrl,
107107
};
108108
await this.intent.underlyingClient.setAccountData(key, data);
@@ -148,18 +148,15 @@ export class UserTokenStore extends TypedEmitter<Emitter> {
148148
return null;
149149
}
150150
// For legacy we just assume it's the current configured key.
151-
const algorithm = obj.algorithm ?? "rsa";
151+
const algorithm = stringToAlgo(obj.algorithm ?? "rsa");
152152
const keyId = obj.keyId ?? this.keyId;
153153

154-
if (algorithm !== 'rsa') {
155-
throw new Error(`Algorithm for stored data is '${algorithm}', but we only support RSA`);
156-
}
157154
if (keyId !== this.keyId) {
158155
throw new Error(`Stored data was encrypted with a different key to the one currently configured`);
159156
}
160157

161158
const encryptedParts = typeof obj.encrypted === "string" ? [obj.encrypted] : obj.encrypted;
162-
const token = this.tokenEncryption.decrypt(encryptedParts);
159+
const token = this.tokenEncryption.decrypt(encryptedParts, algorithm);
163160
this.userTokens.set(key, token);
164161
return token;
165162
} catch (ex) {

src/tokens/mod.rs

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ use napi::bindgen_prelude::Buffer;
55
use napi::Error;
66
use rsa::pkcs1::DecodeRsaPrivateKey;
77
use rsa::pkcs8::DecodePrivateKey;
8-
use rsa::{Pkcs1v15Encrypt, RsaPrivateKey, RsaPublicKey};
8+
use rsa::{Oaep, Pkcs1v15Encrypt, RsaPrivateKey, RsaPublicKey};
9+
use sha1::Sha1;
910

1011
static MAX_TOKEN_PART_SIZE: usize = 128;
1112

@@ -31,6 +32,24 @@ enum DecryptError {
3132
FromUtf8(FromUtf8Error),
3233
}
3334

35+
#[napi]
36+
pub enum Algo {
37+
RSAOAEP,
38+
RSAPKCS1v15,
39+
}
40+
41+
#[napi]
42+
pub fn string_to_algo(algo_str: String) -> Result<Algo, Error> {
43+
match algo_str.as_str() {
44+
"rsa" => Ok(Algo::RSAOAEP),
45+
"rsa-pkcs1v15" => Ok(Algo::RSAPKCS1v15),
46+
_ => Err(Error::new(
47+
napi::Status::GenericFailure,
48+
"Unknown algorithm",
49+
)),
50+
}
51+
}
52+
3453
impl TokenEncryption {
3554
pub fn new(private_key_data: Vec<u8>) -> Result<Self, TokenEncryptionError> {
3655
let data = String::from_utf8(private_key_data).map_err(TokenEncryptionError::FromUtf8)?;
@@ -72,11 +91,11 @@ impl JsTokenEncryption {
7291
}
7392

7493
#[napi]
75-
pub fn decrypt(&self, parts: Vec<String>) -> Result<String, Error> {
94+
pub fn decrypt(&self, parts: Vec<String>, algo: Algo) -> Result<String, Error> {
7695
let mut result = String::new();
7796

7897
for v in parts {
79-
match self.decrypt_value(v) {
98+
match self.decrypt_value(v, algo) {
8099
Ok(new_value) => {
81100
result += &new_value;
82101
Ok(())
@@ -90,13 +109,22 @@ impl JsTokenEncryption {
90109
Ok(result)
91110
}
92111

93-
fn decrypt_value(&self, value: String) -> Result<String, DecryptError> {
112+
fn decrypt_value(&self, value: String, algo: Algo) -> Result<String, DecryptError> {
94113
let raw_value = Base64::decode_vec(&value).map_err(DecryptError::Base64)?;
95-
let decrypted_value = self
96-
.inner
97-
.private_key
98-
.decrypt(Pkcs1v15Encrypt, &raw_value)
99-
.map_err(DecryptError::Decryption)?;
114+
let decrypted_value = match algo {
115+
Algo::RSAOAEP => {
116+
let padding = Oaep::new::<Sha1>();
117+
self.inner
118+
.private_key
119+
.decrypt(padding, &raw_value)
120+
.map_err(DecryptError::Decryption)
121+
}
122+
Algo::RSAPKCS1v15 => self
123+
.inner
124+
.private_key
125+
.decrypt(Pkcs1v15Encrypt, &raw_value)
126+
.map_err(DecryptError::Decryption),
127+
}?;
100128
let utf8_value = String::from_utf8(decrypted_value).map_err(DecryptError::FromUtf8)?;
101129
Ok(utf8_value)
102130
}

tests/tokens/tokenencryption.spec.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { TokenEncryption } from "../../src/libRs";
2-
import { RSAKeyPairOptions, generateKeyPair } from "node:crypto";
1+
import { Algo, TokenEncryption } from "../../src/libRs";
2+
import { RSAKeyPairOptions, generateKeyPair, publicEncrypt } from "node:crypto";
33
import { expect } from "chai";
44

55
describe("TokenEncryption", () => {
@@ -8,6 +8,18 @@ describe("TokenEncryption", () => {
88
async function createTokenEncryption() {
99
return new TokenEncryption(await keyPromise);
1010
}
11+
12+
async function legacyEncryptFunction(token: string) {
13+
const MAX_TOKEN_PART_SIZE = 128;
14+
const tokenParts: string[] = [];
15+
let tokenSource = token;
16+
while (tokenSource && tokenSource.length > 0) {
17+
const part = tokenSource.slice(0, MAX_TOKEN_PART_SIZE);
18+
tokenSource = tokenSource.substring(MAX_TOKEN_PART_SIZE);
19+
tokenParts.push(publicEncrypt(await keyPromise, Buffer.from(part)).toString("base64"));
20+
}
21+
return tokenParts;
22+
}
1123

1224
before('generate RSA key', () => {
1325
// Generate this once since it will take an age.
@@ -49,7 +61,7 @@ describe("TokenEncryption", () => {
4961
it('should be able to decrypt from a single part into a string', async() => {
5062
const tokenEncryption = await createTokenEncryption();
5163
const value = tokenEncryption.encrypt('hello world');
52-
const result = tokenEncryption.decrypt(value);
64+
const result = tokenEncryption.decrypt(value, Algo.RSAPKCS1v15);
5365
expect(result).to.equal('hello world');
5466
});
5567
it('should be able to decrypt from many parts into string', async() => {
@@ -58,12 +70,17 @@ describe("TokenEncryption", () => {
5870
const tokenEncryption = await createTokenEncryption();
5971
const value = tokenEncryption.encrypt(plaintext);
6072
expect(value).to.have.lengthOf(2);
61-
const result = tokenEncryption.decrypt(value);
73+
const result = tokenEncryption.decrypt(value, Algo.RSAPKCS1v15);
6274
expect(result).to.equal(plaintext);
6375
});
6476
it('should support pkcs1 format keys', async() => {
6577
const tokenEncryption = new TokenEncryption(await keyPromisePKCS1);
6678
const result = tokenEncryption.encrypt('hello world');
6779
expect(result).to.have.lengthOf(1);
6880
});
81+
it('should be to decrypt a string from the old crypto implementation', async() => {
82+
const legacyString = await legacyEncryptFunction('hello world');
83+
const tokenEncryption = await createTokenEncryption();
84+
expect(tokenEncryption.decrypt(legacyString, Algo.RSAOAEP)).to.equal('hello world');
85+
});
6986
});

0 commit comments

Comments
 (0)