nheko: clean up #341831
nheko: clean up #341831
Conversation
pkgs/by-name/nh/nheko/package.nix
Outdated
| lmdb | ||
| mtxclient | ||
| nlohmann_json | ||
| # See discussion at https://github.com/Nheko-Reborn/nheko/issues/1786 |
There was a problem hiding this comment.
If I read this correctly, the author considers the entire issue to not be a big issue.
This contradicts the decision we took, i.e. marking olm as insecure.
I'm not sure if we want to revert this on a per-package basis given on what upstream's opinion on the olm topic is.
cc @emilazy @NixOS/security for opinions.
There was a problem hiding this comment.
Well, I think it makes sense to have that warning:
- olm is not used exclusively for Matrix: though unlikely, other application may be vulnerable.
- the library is now unmaintained upstream.
There was a problem hiding this comment.
The referenced issue sounds a lot like wishful thinking and hoping for a great future, where some kind soul forks and maintains libolm. Or were you referencing something specific in that issue?
I don't think it is reasonable to override knownVulnerabilities, unless the upstream says they are not affected by the issues that affect libolm.
There was a problem hiding this comment.
unless the upstream says they are not affected by the issues that affect libolm.
I'm referencing this comment in particular: Nheko-Reborn/nheko#1786 (comment) (deepbluev7 is one of the main dev of Nheko). They are saying it's ok for their use case, so I think it's stupid to mark it as unsafe on the sole basis of a CVE about some theoretical attack.
More importantly, this decision broke an application that people rely for communications on a daily basis without any notice. Worse, it results in an obscure error message that doesn't even refer to Nheko when updating your system.
There was a problem hiding this comment.
It doesn't break anything to provide a detailed and balanced warning that specifically refers to the library's use in Matrix, points to upstream advisories, and tells the user exactly how to override it. The advisory was discussed extensively on the original PR. There's no argument for overriding this individually at the client level and it's hard not to see this as an inappropriate attempt to revert the hard-won consensus on that PR through the back door.
There was a problem hiding this comment.
To be clear: it was the express and specific intent to show this advisory to people using Matrix clients that rely on this library with known weaknesses and an upstream expressing that there will be no maintenance or fixes and that the library should not be used. The display of it to users of those clients is the point, not an unintended side effect. Nothing is broken about keeping users informed and the ability to override the warning and the choice to leave that decision to end users is deliberate.
There was a problem hiding this comment.
For context and with extensive discussion: #334638
There was a problem hiding this comment.
It doesn't break anything to provide a detailed and balanced warning that specifically refers to the library's use in Matrix, points to upstream advisories, and tells the user exactly how to override it.
Yes it does, add Nheko to your system and try to rebuild it: it fails. The message is bad because it does not refers explicitely to which package is causing it (this is a more general UX problem in NixOS, though). It is also borderline incomprehensible for the average user (you need to know that your chat uses a protocol called Matrix, that Matrix uses an end-to-end encryption thing called olm, and you need to know that a side-channel is a bad thing).
I'm not saying this because I'm angry with you or anything: I just had friends asking me what the error meant and what to do.
There was a problem hiding this comment.
you need to know that your chat uses a protocol called Matrix, that Matrix uses an end-to-end encryption thing called olm, and you need to know that a side-channel is a bad thing
If you're that much of a non-technical user, you shouldn't nixos-rebuild switch a system.
There was a problem hiding this comment.
Using NixOS does not imply knowledge of the internals of a chat program or cryptography.
|
Removed the controversial overrides and added an exception in |
|
I believe the Hydra override may also need to go in About the vulnerability message: I’d be open to ways to make it more accessible and less technical, but it is a difficult situation. The issues with libolm are highly controversial, with multiple security researchers, the Matrix.org libolm upstream, client developers, and Matrix fans all having wildly different and contradictory views about the topic. A lot of that was discussed in the original PR. It’s been very unpleasant to deal with as a downstream, where my priority is making sure users are informed and warned about potential security vulnerabilities and maintenance issues in libraries they may be depending on for security and privacy, and it took a lot of work to write a message that was neutral and pointed to various resources on the topic, including the CVEs issued for the flaws, extensive descriptions of the problems in the code, and statements from upstream. I think we reached about as much consensus on it as any action on this topic could hope to gather. The stress of dealing with remediating the situation and trying to find a way forward that would satisfy a large subset of parties, while the PR was being canvassed beyond the NixOS community, may have made me more snappy than I should have been in response to this one, especially if you hadn’t seen the full context of the prior discussion, so I’m sorry for that. If we could make the message less technical and easier to understand for end users, I would appreciate and support that. But I found it very difficult to do more than neutrally describe a nuts‐and‐bolts overview, because interpreting it further risks making editorial statements on the matter that are inevitably going to be even more controversial than the message already is. Hopefully the client ecosystem will move to vodozemac sooner rather than later and we won’t have to worry about it any more… |
There is at least some olm bindings for python, so yes it's worth looking into.
Uhm, there's not much point in having these minor changes if Nheko remains quite inaccessible. I think they can wait.
Ok, understood.
I wouldn't change the message now, it's probably too late and we risk creating more confusion.
Since the issue is very technical, I would have preferred if the security team had made a stronger decision: either disregarding the vulnerability reports or marking individual packages as unsafe. But if even the "experts" are not agreeing on this issue, I can't blame you.
I understand, no problem.
Yes, I think most already did, unfortunately that's not the case for Nheko. |
rnhmjoj
force-pushed
the
pr-nheko
branch
2 times, most recently
from
739055a
to
f716d5c
Compare
| @@ -34,7 +34,20 @@ let | |||
| systems | |||
| ; | |||
|
|
|||
| pkgs = packageSet (recursiveUpdate { inherit system; config.allowUnsupportedSystem = true; } nixpkgsArgs); | |||
| allNixpkgsArgs = lib.recursiveUpdate | |||
There was a problem hiding this comment.
This should cover all the release-*.nix files providing their own nixpkgsArgs.
Olm was marked as vulnerable in e4767b4. The issue, however, is controversial and the practical ability to exploit its vulnerabilities has not been demostrated: ultimately the users are invited to decide for themselves. Given that, hydra should still build and distribute the packages depending on olm to avoid expensive builds in the event the users decide it's acceptable to continue using these packages.
Description of changes
Clean up nheko and related packages. Most importantly, fix the build failure caused by the olm deprecation (#338006).
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truemtxclient coeurl nheko)Add a 👍 reaction to pull requests you find important.
cc: @emilazy