PeculiarVentures/webcrypto

@peculiar/webcrypto is vulnerable to the Marvin Attack

tomato42 opened this issue · 5 comments

tomato42 commented

I've tested @peculiar/webcrypto package 1.4.3 on node 21.1.0 and verified that's it's vulnerable to the Marvin Attack.

The size of the side channel is very large, so even remote exploitation will be rather easy. Both correctness of the PKCS#1 v1.5 ciphertext is leaking as well as the length of the returned message. Less than 100 measurements per probe are necessary for highly statistically significant results, so a local attack taking as little as few hours is quite realistic.

I've executed the reproducer in the marvin-toolkit repo on an AMD Ryze 5 5600X CPU running with core isolation.

After collecting 10 thousand measurements per sample, I've got the following results:

Sign test mean p-value: 0.3282, median p-value: 0.2048, min p-value: 0.0
Friedman test (chisquare approximation) for all samples
p-value: 0.0
Worst pair: 6(valid_48), 11(zero_byte_in_padding_48_4)
Mean of differences: 1.39286e-05s, 95% CI: -3.92232e-06s, 3.116315e-05s (±1.754e-05s)
Median of differences: 2.03415e-05s, 95% CI: 2.01400e-05s, 2.054000e-05s (±2.000e-07s)
Trimmed mean (5%) of differences: 2.02399e-05s, 95% CI: 2.00630e-05s, 2.041578e-05s (±1.764e-07s)
Trimmed mean (25%) of differences: 2.02831e-05s, 95% CI: 2.01000e-05s, 2.045879e-05s (±1.794e-07s)
Trimmed mean (45%) of differences: 2.03165e-05s, 95% CI: 2.01283e-05s, 2.050997e-05s (±1.908e-07s)
Trimean of differences: 2.02808e-05s, 95% CI: 2.01023e-05s, 2.045544e-05s (±1.766e-07s)
Layperson explanation: Definite side-channel detected, implementation is VULNERABLE

With a graph of confidence intervals of:
conf_interval_plot_trim_mean_05
legend for the graph:

ID,Name
0,header_only
1,no_header_with_payload_48
2,no_padding_48
3,no_structure
4,signature_padding_8
5,valid_0
6,valid_48
7,valid_192
8,valid_246
9,valid_repeated_byte_payload_246_1
10,valid_repeated_byte_payload_246_255
11,zero_byte_in_padding_48_4

explanation of the probes is in the step2.py script.

Results of the pairwise statistical tests are in this file:
report.txt

note: the p-values of 0 mean that the actual calculated p-value is smaller than what double precision floating point number can represent

tomato42 commented

@microshine any progress on this?

rmhrisk commented

@tomato42, thank you for your detailed report on the vulnerability of @peculiar/webcrypto package 1.4.3 to the Marvin Attack, and I apologize for the delayed response.

Your findings regarding the side-channel vulnerability, are concerning. We understand the gravity of the situation.

To clarify our project's scope, we do not implement our own cryptography but provide a layer over the NodeJS cryptography API to enable compatibility with WebCrypto (peculiar/webcrypto -> NodeJS -> OpenSSL) projects. For reference your test specifically exercises this block of code.

Addressing the issue at our layer, such as by introducing a counter-based rate limit, poses significant challenges. Without detailed insights into how various applications utilize our library, such changes would likely unintentionally, and potentially unnecessarily break these applications. We say this because this would include scenarios where an attacker doesn't have adequate control or visibility to perform the attack.

The option to switch to a different cryptography layer, moving away from Nodejs Crypto (and as a result OpenSSL), has been considered as well. However, this library was initially created to minimize maintenance costs relative to its predecessor which directly consumed OpenSSL, and with our current funding limitations, returning to a more integrated approach may not be sustainable.

We understand this is a complex issue, and unfortunately, there might be more harm than good in attempting a solution at our layer. Do we have any insights from the NodeJS and OpenSSL teams on how they intend to address this?

We are open to suggestions and would welcome any recommendations you might have for addressing this vulnerability, considering it is not inherent to our codebase.

Thank you once again for bringing this to our attention and for your contribution to the security of open-source software.

tomato42 commented

sorry, the code you linked is used for signing with RSA-PSS: https://github.com/PeculiarVentures/webcrypto/blob/master/src/mechs/rsa/crypto.ts#L196-L213
I was testing Crypto.subtle.decrypt("RSAES-PKCS1-v1_5", keyObj, buffer); call...

as far as fixes go: the thing is that current webcrypto doesn't support RSAES-PKCS1-v1_5 mechanism... only RSA-OAEP is supported. So, if compatibility with webcrypto is the goal, I don't think there's a need to continue support for the RSAES-PKCS1-v1_5 mechanism.

For OpenSSL the fix is limited to openssl/openssl#13817, which shipped upstream only in the 3.2.x branch.

rmhrisk commented

My apologies, I shared that link in a hurry. You're right; it was for the PSS wrapper code, but the answer remains the same, we just call Node's crypto libraries.

The original design of the library aimed to provide support for web crypto before Node.js had its own support. It has since evolved to also offer algorithms not supported by webcrypto, to enable those libraries that use WebCrypto to interoperate with the rest of the world. it is our understanding the majority of the consumers of this library do so in the name of achieving that interoperability. Since we don't know which applications are using it, removing this functionality could potentially break applications, including those that are not vulnerable.

Do you know what Node's plan is?