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.

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

mautrix: declares dep on olm as optional but depends unconditionally #336052

Closed
antifuchs opened this issue Aug 20, 2024 · 6 comments
Closed

mautrix: declares dep on olm as optional but depends unconditionally #336052

antifuchs opened this issue Aug 20, 2024 · 6 comments
Labels
0.kind: bug Something is broken

Comments

@antifuchs
Copy link
Contributor

Describe the bug

Currently, on nixpkgs-unstable when pulling in the mautrix python package without the encryption option, users get the following error (due to #334638):

       Known issues:
        - 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.
[rest omitted]

I believe this happens due to the addition of the encryption deps to mautrix's nativeCheckInputs. A more nuanced approach might be necessary there to avoid opting users into having to allowlist olm in their system configs.

Steps To Reproduce

Steps to reproduce the behavior:

  1. Try to reference mautrix with current nixpkgs. In my flake, that is doable with nixosConfigurations.gloria.pkgs.python313Packages.mautrix
  2. Watch the fireworks.

Expected behavior

A bare "mautrix" python package without the crypto deps should be installable.

Additional context

I believe the mautrix upstream is currently pretty resistant to moving off libolm, but some mautrix dependents (e.g. heisenbridge) don't need the cryptography stuff. It would make sense to me to make encryption with mautrix fully opt-in.

Notify maintainers

@nyanloutre @Ma27 @sumnerevans @NickCao

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

 - system: `"aarch64-darwin"`
 - host os: `Darwin 23.5.0, macOS 14.5`
 - multi-user?: `yes`
 - sandbox: `no`
 - version: `nix-env (Lix, like Nix) 2.91.0-pre20240812-4d04adf
System type: aarch64-darwin
Additional system types: aarch64-darwin, x86_64-darwin
Features: gc, signed-caches
System configuration file: /etc/nix/nix.conf
User configuration files: /Users/asf/.config/nix/nix.conf:/Users/asf/.nix-profile/etc/xdg/nix/nix.conf:/run/current-system/sw/etc/xdg/nix/nix.conf:/nix/var/nix/profiles/default/etc/xdg/nix/nix.conf
Store directory: /nix/store
State directory: /nix/var/nix
Data directory: /nix/store/v76ga4gbqgvpnrm02j95xnwfw73hcyj5-lix-2.91.0-pre20240812-4d04adf/share`
 - channels(root): `"nixpkgs"`
 - channels(asf): `"darwin"`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixpkgs`

Add a 👍 reaction to issues you find important.

@antifuchs antifuchs added the 0.kind: bug Something is broken label Aug 20, 2024
@Ma27
Copy link
Member

Ma27 commented Aug 20, 2024

fwiw mautrix-whatsapp and mautrix-signal are written in Go these days and I'm not sure about their state.

That said, wouldn't that break the bridge's ability to work with encrypted rooms? If so, I'd like to see this as opt-out at best since I don't think that no encryption is really the better alternative here. Especially if that happens by flipping a switch which usually goes unnoticed by people not following upstream development closely.

@sumnerevans
Copy link
Contributor

sumnerevans commented Aug 20, 2024

All of the main bridges are being rewritten in Go. https://mau.fi/blog/2024-h1-mautrix-updates/

The signal and WhatsApp bridges aren't yet ready for release.

All of the go bridges will rely on libolm until we can figure out the issues with goolm.

We should not disable encryption by default in mautrix. The general consensus in the matrix community is that the attacks soatok resurfaced have been known for a while and are not practically exploitable, thus most people are going to continue using libolm. Migrating to vodozemac is going continue to be fairly gradual.

@emilazy
Copy link
Member

emilazy commented Aug 20, 2024

We could disable the tests that use libolm so that downstream packages that don’t already pull in optional-dependencies.encryption don’t trigger the warning. That won’t remove encryption support from any package that already has it.

@nh2
Copy link
Contributor

nh2 commented Dec 22, 2024

As of NixOS 24.11, mautrix-signal still seems to require olm, see comments here:

https://github.com/NixOS/nixpkgs/pull/337571/files#diff-ee35d03b308f7960d553be540b87696bb05b8cdc6d83f335af7bd3643de43087

@Ma27
Copy link
Member

Ma27 commented Dec 22, 2024

I think this issue was about mautrix, i.e. the Python library and thus mautrix-signal (and probably whatsapp, too) still have that requirement.

That said, I think an Olm implementation is still required for mautrix-signal? When I remove olm from buildInputs, I get

Building subPackage ./cmd/mautrix-signal
# maunium.net/go/mautrix/crypto/libolm
vendor/maunium.net/go/mautrix/crypto/libolm/account.go:4:11: fatal error: olm/olm.h: No such file or directory
    4 | // #include <olm/olm.h>
      |           ^~~~~~~~~~~
compilation terminated.

So the only alternative you have is withGoolm = true;, but I don't know about the state of that in terms of stability.

@sumnerevans
Copy link
Contributor

Goolm is not stable. It should not be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: bug Something is broken
Projects
None yet
Development

No branches or pull requests

5 participants