-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
olm: mark as vulnerable #334638
olm: mark as vulnerable #334638
Conversation
c95f41b
to
1ed56ac
Compare
Result of 48 packages removed: 😢
|
None of the issues described by soatok are practically exploitable. We definitely should not disable libolm support, as having no encryption is way worse than having "vulnerabilities" that cannot be exploited. |
Just to clarify: this PR is about giving a warning to nixpkgs libolm users that issues have been raised and might impact their threat model. It is not about disabling encryption for the users of said software but warning them and giving them a choice. At a Linux distribution level, it is hard to judge what might be the usage of a specific software for a potential user (to put it another way, it is hard to evaluate the threat model of a specific user). In the nixpkgs/NixOS case we do not have a lot of other choices to alert users :/ . The impacted packages are leaf packages so people allowing them should not burn too much CPU time rebuilding them if this PR gets merged. In any cases |
So, I agree that at the time of writing there’s no known way to exploit these vulnerabilities over the network in practice. I also agree that it’s perfectly possible that there might not be a way to exploit them at all. (And I agree that having no encryption is worse: my hope was that, given that Matrix DMs are E2E by default, users would set up an E2E proxy without the issues in the interim while the ecosystem is in flux; I thought this may be better than having to bypass the security warning. Still, I was wrong: for one thing, Pantalaimon still depends on libolm, and for another, that’s probably more churn than just bypassing the warning or switching to another client. So yes, I no longer think that part of my previous comment is a good idea at all.) However, there’s a history here with timing attacks. They were known to be a theoretical possibility for many years, but the idea of being able to use them in a practical attack was dismissed; they were considered entirely hypothetical and not worth worrying about. Then, in 2003, we got the paper Remote Timing Attacks are Practical, and everything changed. Exploits of timing attacks have only improved in the years since, and people have managed to wring private key material in fairly realistic network conditions out of some pretty marginal exploits. So there’s two things here:
To quote Matthew Garrett – who is far more of an expert than me – on Hacker News:
I think it would be great if it turns out that these vulnerabilities are not practically exploitable under most threat models! I want users to be safe; that’s my whole motivation here! But, still: the arguments for why it’s definitely not exploitable that I’ve seen are pretty handwave‐y and look a lot like similar historical arguments that turned out to be way overconfident. And the fact that the library has been deprecated rather than getting a fix for an issue this basic is concerning, because cryptography code that messes up table‐stakes things like this is very likely to have other issues lurking, and if anyone ever does develop a practical exploit for even this known issue, users will have no recourse. And, I mean, here’s an excerpt from the statement in the upstream libolm repository:
When we click the link to the README file for the cryptography algorithm implementations they use, we get this terrifying statement:
I mean, what more can you say to that? “These are not cryptographically secure implementations … and should not be used in contexts that need cryptographically secure implementations”. In my view, that right there is sufficient for It doesn’t really matter how long a vulnerability has existed or if it was known or not. We set I also want to be clear that I’m not trying to participate in any kind of pro‐Matrix vs. anti‐Matrix thing in this pull request, or penalize users of these clients. I use Matrix on a daily basis to work on NixOS and socialize, and am consequently invested in its ecosystem. I sincerely hope that the ecosystem can move to maintained libraries for E2E. I think it sucks that a library that is depended on by basically every non‐Element client for encryption is in this state. I really wish this whole situation wasn’t a thing. But it is, and I don’t think we can make it go away just by ignoring it. I fully expect many users will react to this security notice by shrugging, adding the package to their list of permitted insecure packages, and going about their day. That’s okay: they got the opportunity to make an informed choice, which is what matters to me. I genuinely look forward to clients resolving this issue and leaving Matrix users safer. Rather than debating whether known vulnerabilities that won’t be fixed pass a threshold of severity and practically‐demonstrated exploits, it’d be better if we simply had confidence that clients were using safe, well‐maintained E2E implementations that will proactively address any security issues that arise in the future. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this PR. As one of the package maintainers of olm
I have just started the same until I stumbled over this PR.
@@ -27,5 +27,8 @@ stdenv.mkDerivation rec { | |||
homepage = "https://gitlab.matrix.org/matrix-org/olm"; | |||
license = licenses.asl20; | |||
maintainers = with maintainers; [ tilpner oxzi ]; | |||
knownVulnerabilities = [ | |||
"libolm is deprecated upstream with an AES cache timing vulnerability that upstream doesn't plan to fix" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add more details, for example, linking the olm commit stating it as deprecated - https://gitlab.matrix.org/matrix-org/olm/-/commit/6d4b5b07887821a95b144091c8497d09d377f985 - and Soatok's blog post.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thanks for nudging me to do this. I avoided doing it because I was worried about being sufficiently neutral and describing the potential risks and trade‐offs sufficiently well, but it’s important in a situation like this. I’ve pushed a long but hopefully unbiased and informative explanation of the issues and maintenance status of the package that should let users make an informed decision. Please let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I enjoyed reading your deprecation information text. It informs the user with enough details to look into it.
Thanks again for all the work you are putting into this :)
1ed56ac
to
42003e9
Compare
I’ve pushed this with a much more elaborate and nuanced There is one big remaining issue: Thunderbird. Thunderbird grew chat support at some point and added support for Matrix a few years back. That code currently vendors libolm for encryption support. I’ve linked their tracking issue and it will hopefully be addressed soon, given the strength of Mozilla’s security team. At first, I hoped we could simply patch out Matrix support from the Thunderbird package until then, on the assumption that the vast majority of people use Thunderbird exclusively as an email client. That worked, but unfortunately it turns out that rolling back to a version with Matrix support leaves you without the account data you had previously added. That’s obviously not acceptable, and I don’t have enough knowledge of the Thunderbird code to stub it out in a more sophisticated manner. I have opted not to mark the Thunderbird packages with
This is not a sustainable solution; If Thunderbird don’t move off libolm quickly, we’ll have to do something, but this was the best resolution I could think of to warn users of Matrix clients without causing widespread chaos. If the issue isn’t addressed in a timely fashion, hopefully we can figure out how to patch Matrix support in a more reliable fashion or something. I’m not entirely happy with this, but it’s the best compromise I can think of at this time. |
The Thunderbird patch to move off libolm has been updated, so I’m feeling confident that they will address this in a timely fashion and that we can hold off on I think this is ready to merge once the new message has been reviewed. |
implementations, or to otherwise further maintain in any capacity. | ||
|
||
You should make an informed decision about whether to override | ||
this security warning, especially if you rely criticially on Matrix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this security warning, especially if you rely criticially on Matrix | |
this security warning, especially if you rely critically on Matrix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, thanks! Fixed and pushed a few other minor rewordings I had locally but forgot to squash.
42003e9
to
bdcccfe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the detailed new message, the tone seems appropriate, and it gives enough context that users don't have to start from scratch to decide whether they care. And good catch with the vendored libolms, I probably would have missed them :)
<https://gitlab.matrix.org/matrix-org/olm/-/blob/6d4b5b07887821a95b144091c8497d09d377f985/lib/crypto-algorithms/README.md> | ||
|
||
* The blog post disclosing the details of the known vulnerabilities: | ||
<https://soatok.blog/2024/08/14/security-issues-in-matrixs-olm-library/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend against linking soatok's FUD campaign blog post. The author's motivation is to "make the Matrix evangelists shut up", not to inform the public about vulnerabilities. Thus, they use manipulative tactics which they have used in the past to overblow the impact of the vulnerabilities. Additionally, they further fail to do basic research w.r.t. the client coverage. All clients that I know of that are actively worked on are in various stages of being migrated to vodozemac. Vodozemac bindings for various languages are being worked on (including C, C++, Python...) and improved.
As an aside, libolm has been known to be deprecated for as long as I am involved with Matrix (it only really takes a basic look at the repo to tell). New clients do not utilize libolm for this reason.
EDIT: Soatok took down their gist, here's a webarchive link https://web.archive.org/web/20240815171614/https://gist.github.com/soatok/8aef6f67fec9c702f510ee24d19ef92b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we try and prefer CVE #s anyway? That way there's a standardized reference to the vuln.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no CVEs. Presumably upstream has no intention of getting them assigned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. Seems like a bit of a gray area. That being said:
- Preferring CVE #s is probably a good thing, but with the absence of them, I'm not sure where this falls
- The characterization of the blogpost as "FUD" is a little dismissive and has people talking past each other
Given that this is a "maybe multiple people are right" scenario, can we point to any existing policy that expresses whether we shouldn't use the library or, conversely, shouldn't heavily weigh the blogpost's case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There’s never been any rule that says we need CVE numbers assigned to act on known vulnerabilities, especially when upstream acknowledges the vulnerabilities and openly says that the library should not be used. Many vulnerabilities don’t get CVE numbers assigned. We routinely set knownVulnerabilities
for security‐critical packages that are end‐of‐life, as this one is, even when they don’t have specific known vulnerabilities, as this one does.
I think the upstream statements and the documentation of the cryptography code it relies on would be sufficient for this, but the blog post provides additional detail about specific concrete timing attacks the code is vulnerable to.
Note that the wording hopefully makes it very clear that the purpose is to warn and inform users. The error message will contain specific instructions on how to bypass the warning and continue to use the packages that depend on libolm until the ecosystem finishes migrating.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, just relying on CVEs seems like a derailment to me.
If you'd like to directly address what I was asking, which read:
can we point to any existing policy that expresses whether we shouldn't use the library or, conversely, shouldn't heavily weigh the blogpost's case
Then you might say:
see for examples, gogs or googleearth-pro
Thanks for the answer, those are good examples. Here's gogs, which had an announcement: https://github.com/NixOS/nixpkgs/blob/nixos-unstable/pkgs/applications/version-management/gogs/default.nix#L49
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what is important to put this into context is that at least some of these vulnerabilities were intentional choices done at the start of libolms development and haven't been considered critical so far. There is a lot of buzz around them now, but arguably the impact of those issues hasn't changed. And while libolm was deprecated now, that has mostly been a decision based on how much maintenance bandwidth the maintainers for it have and them focusing on vodozemac now.
If you consider that important enough to warrant a manual override by users to install software based on libolm, you should probably also consider to warn for things like Nheko, that explicitly warn in the README, that the implementation isn't done by professionals: https://github.com/Nheko-Reborn/nheko/?tab=readme-ov-file#note-regarding-end-to-end-encryption
I believe the latter to have a much higher impact to users. While the libolm deprecation will be a problem at some point, it happened only recently and projects didn't have much time yet to figure out how they want to deal with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate that many in the Matrix development community had already known about these issues for years, and so the recent flurry of urgency might seem misguided to them. However, I think there’s a fundamental disconnect here: we don’t consider security issues less severe based on their age, and the fact that the libolm maintainers and the clients depending on it were aware of them and didn’t choose to fix them or more actively deprecate and warn about the library before compounds the problem, rather than mitigating it. I’ve used Matrix for 6 years now, and I never knew that the encryption library I was relying on for a substantial portion of that time was using cryptography code whose security was explicitly disclaimed; I would have been shocked to find that out, and I don’t think you can reasonably expect end users to read README files buried in a subdirectory of a library dependency.
If I had learned about these issues sooner, I would have made an active effort to keep users informed, try to get upstream to act one way or another on them, and make my own personal risk assessment decisions relating to use of the library. The main effect of the blog post, it seems, has been to publicize to a wider audience, including cryptography engineers, what the Matrix development ecosystem was already aware of.
Nheko depends on libolm, so it will already trigger a warning with this pull request. In an ideal world perhaps we would warn about such things beyond that (especially if we got the problems
infrastructure to have a less blunt tool to use for these things), but I think that there’s a distance between a non‐audited implementation of a cryptographic protocol on top of a library and the library itself being deprecated with known concrete issues. We inherently have to make some judgement calls about security as a distro, but I don’t think we’re really in a position to review the general coding practices of every application separate to any upstream deprecations and known published vulnerabilities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just FYI. There are now CVEs:
- CVE-2024-45191 - AES implementation is vulnerable to cache-timing attacks
- CVE-2024-45192 - Base64 implementation used for decoding sensitive data has a timing side-channel
- CVE-2024-45193 - Ed25519 signatures are malleable
— Source: Matrix Foundation blog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The message was adjusted in #338006
bdcccfe
to
aff7952
Compare
@tilpner I just found another one in the form of @tranquillity-codes suggested I remove the link to Soatok’s blog post on Matrix. I replied that I had done my best to write a balanced advisory on this clearly controversial issue, that I don’t think it would do a service to users to omit the link to the public coordinated disclosure of the known vulnerabilities in libolm, and that I linked several statements from Matrix.org including the project lead’s reply to the blog post for balance. Based on the conversation, I do not think that we are likely to be able to resolve our disagreement, so I don’t think further discussion on the matter would be productive. If there was a formal CVE advisory then perhaps we could link to that instead, but that doesn’t seem likely to happen. I’ve already elaborated on my position about assumptions of the severity or exploitability of the bugs in my previous comment. Having discussed this matter extensively on Matrix over the past couple days and with the positive feedback from the maintainers of this package, I think this PR is the best way we can handle a bad situation, should be ready to merge, and will likely hit the button within a day pending other feedback if nobody else does. I will continue to follow the tracking issues in various clients and do my bit to help bump to versions that do not depend on libolm as they are released. |
843268b
to
45bf5e2
Compare
Updated the |
45bf5e2
to
0734fc6
Compare
The manual backport is up at #335189. I will give it a couple days before merging to get a sense of the fallout and continue to track upstream developments. |
Just so you are aware there is some more statement from the foundation side at https://matrix.org/blog/2024/08/16/this-week-in-matrix-2024-08-16/#dept-of-encryption-closed-lock-with-key |
Thanks for the pointer; I appreciate the acknowledgement from upstream that more proactive action would have hopefully helped to avoid the recent kerfuffle. vodozemac seems like a much better base for future development; I’m really happy that it was audited and I’m actively tracking the process of client adoption. Hopefully the ecosystem can put this behind it soon. |
What's the plan for pantalaimon, given I don't see that being updated to use vodozemac any time soon, but is part of my infrastructure. |
You’ll have to make your own risk assessment based on the information in the message and decide whether or not to permit |
Yeah those issues were posted an hour ago, though I've known that it probably wont happens as it's been dead for ages. It is worrying to think about having to throw out a decent chunk of my infrastructure in the event olm/pantalaimon get removed from nixpkgs, or stick to using nixos 24.05 as an overlay to get it... |
Nothing that depends on libolm has been removed and there is definitely not going to be any rush to make that happen. The In the longer term, depending on the movement in the rest of the ecosystem, if there is totally unmaintained software that relies critically on libolm and will never move off it then we might consider removing it, yes, especially if anyone finds any worse vulnerabilities, but that’s not anything there is any kind of agreement to do at this time. This pull request is not intended as a prelude to imminent removal. |
Is there a known good alternative for a desktop matrix client, which can be used without |
At least element and fractal work. |
Just to be clear: I meant |
#335753 unbreaks element-desktop |
Eek! I should have checked the reverse dependencies of Jitsi Meet when finding it at the last minute, I didn’t expect to break Element 😬 Very sorry for that, especially for giving users an inaccurate impression of Element’s security with the inherited warning… Not sure what the best way to remediate that is. |
This comment was marked as spam.
This comment was marked as spam.
This reverts commit ecffb72. blame NixOS/nixpkgs#334638 (comment)
Olm has a known vulnerability (NixOS#334638) but is only an optional dependency of nio, so in theory nio should by default be unaffected. nio’s tests, however, cover its full suite of extra features, so Olm is still evaluated as a dependency of the check phase. Since the check phase doesn’t process user data or access the network this vulnerability isn’t relevant and can be ignored, allowing nio to evaluate and ultimately be run without Olm.
After olm gained knownVulnerabilities in NixOS#334638, allow building these bridges using the pure-Go goolm library instead of libolm bindings.
Description of changes
See https://soatok.blog/2024/08/14/security-issues-in-matrixs-olm-library/.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.