Skip to content
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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 16, 2024
+47 −1
Merged

olm: mark as vulnerable #334638

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e4767b4
olm: mark as vulnerable
emilazy Aug 14, 2024
bbfd5d1
{cinny-unwrapped,element-call,fluffychat,jitsi-meet}: inherit vulnera…
emilazy Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkgs/applications/networking/instant-messengers/fluffychat/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
, pulseaudio
, makeDesktopItem
, zenity
, olm

, targetFlutterPlatform ? "linux"
}:
Expand Down Expand Up @@ -44,6 +45,7 @@ flutter319.buildFlutterApplication (rec {
maintainers = with maintainers; [ mkg20001 gilice ];
platforms = [ "x86_64-linux" "aarch64-linux" ];
sourceProvenance = [ sourceTypes.fromSource ];
inherit (olm.meta) knownVulnerabilities;
};
} // lib.optionalAttrs (targetFlutterPlatform == "linux") {
nativeBuildInputs = [ imagemagick ];
Expand Down
2 changes: 2 additions & 0 deletions pkgs/by-name/ci/cinny-unwrapped/package.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
pango,
stdenv,
darwin,
olm,
}:

buildNpmPackage rec {
Expand Down Expand Up @@ -54,5 +55,6 @@ buildNpmPackage rec {
maintainers = with lib.maintainers; [ abbe ];
license = lib.licenses.agpl3Only;
platforms = lib.platforms.all;
inherit (olm.meta) knownVulnerabilities;
};
}
2 changes: 2 additions & 0 deletions pkgs/by-name/el/element-call/package.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
, yarnBuildHook
, nodejs
, npmHooks
, olm
}:

let
Expand Down Expand Up @@ -52,5 +53,6 @@ stdenv.mkDerivation (finalAttrs: {
license = licenses.asl20;
maintainers = with maintainers; [ kilimnik ];
mainProgram = "element-call";
inherit (olm.meta) knownVulnerabilities;
};
})
39 changes: 39 additions & 0 deletions pkgs/development/libraries/olm/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,44 @@ stdenv.mkDerivation rec {
homepage = "https://gitlab.matrix.org/matrix-org/olm";
license = licenses.asl20;
maintainers = with maintainers; [ tilpner oxzi ];
knownVulnerabilities = [ ''
The libolm end‐to‐end encryption library used in many Matrix
clients and Jitsi Meet has been deprecated upstream, and relies
on a cryptography library that has known side‐channel issues and
disclaims that its implementations are not cryptographically secure
and should not be used when cryptographic security is required.

It is not known that the issues can be exploited over the network in
practical conditions. Upstream has stated that the library should
not be used going forwards, and there are no plans to move to a
another cryptography implementation or otherwise further maintain
the library at all.

You should make an informed decision about whether to override this
security warning, especially if you critically rely on end‐to‐end
encryption. If you don’t care about that, or don’t use the Matrix
functionality of a multi‐protocol client depending on libolm,
then there should be no additional risk.

Some clients are investigating migrating away from libolm to maintained
libraries without known vulnerabilities.

For further information, see:

* The libolm deprecation notice:
<https://gitlab.matrix.org/matrix-org/olm/-/blob/6d4b5b07887821a95b144091c8497d09d377f985/README.md#important-libolm-is-now-deprecated>

* The warning from the cryptography code used by libolm:
<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/>
Copy link

@tranquillity-codes tranquillity-codes Aug 16, 2024 •
edited
Loading

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

Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Contributor

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?

Copy link
Member Author

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.

Copy link
Contributor

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

Copy link

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.

Copy link
Member Author

@emilazy emilazy Aug 16, 2024 •
edited
Loading

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.

Copy link
Contributor

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

Copy link
Contributor

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


* The Matrix.org project lead’s response to the disclosure:
<https://news.ycombinator.com/item?id=41249371>

* A (likely incomplete) aggregation of client tracking issue links:
<https://github.com/NixOS/nixpkgs/pull/334638#issuecomment-2289025802>
'' ];
};
}
3 changes: 2 additions & 1 deletion pkgs/servers/web-apps/jitsi-meet/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{ lib, stdenv, fetchurl, nixosTests }:
{ lib, stdenv, fetchurl, nixosTests, olm }:

stdenv.mkDerivation rec {
pname = "jitsi-meet";
Expand Down Expand Up @@ -34,5 +34,6 @@ stdenv.mkDerivation rec {
license = licenses.asl20;
maintainers = teams.jitsi.members;
platforms = platforms.all;
inherit (olm.meta) knownVulnerabilities;
};
}