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

olm: update vulnerability description #338006

Merged
merged 1 commit into from
Aug 30, 2024

Conversation

sumnerevans
Copy link
Contributor

Description of changes

Since the known vulnerability was added, additional information has been
published by upstream about why they believe the vulnerability to not be
exploitable over the network:
https://matrix.org/blog/2024/08/libolm-deprecation/

This commit updates the text of the vulnerability warning with
additional information from the blog post as well as a link to the blog
post.

Signed-off-by: Sumner Evans [email protected]

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I am nervous about adding more wording in our own voice to this message since the whole matter has become so controversial and the message is already so long; I would prefer to keep it brief and avoid spending much copy on any specific post so that users get the necessary background and can make their own assessments from the links. I think that “It is not known that the issues can be exploited over the network in practical conditions.” is still accurate – it is, thankfully, not known that that can be done – but we can mention that upstream does not consider that likely to change. I was planning to replace the TWIM link with this statement since it supersedes it in my view (just as I replaced the HN comment with the TWIM link).

pkgs/development/libraries/olm/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/olm/default.nix Outdated Show resolved Hide resolved
Additional information has been published by upstream about why they
believe the vulnerability to not be exploitable over the network:
https://matrix.org/blog/2024/08/libolm-deprecation/

This commit

* updates the text of the vulnerability warning to indicate that
  upstream does not believe the issues to be exploitable over the
  network, and
* adds a link to the blog post.

Co-authored-by: Emily <[email protected]>
Signed-off-by: Sumner Evans <[email protected]>
@sumnerevans sumnerevans force-pushed the olm-vulnerability-notice-update branch from 40bf790 to 537d3c4 Compare August 28, 2024 20:40
Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me; I’ll let @LeSuisse take a look before merging. I’ll also incorporate this into the backport PR. Thanks!

Copy link
Contributor

@LeSuisse LeSuisse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me too.

(And yeah we finally have an official statement to link to 🎉 )

@LeSuisse LeSuisse merged commit bccb6af into NixOS:master Aug 30, 2024
24 of 25 checks passed
@LeSuisse LeSuisse mentioned this pull request Sep 1, 2024
13 tasks
@leo60228
Copy link
Member

leo60228 commented Sep 2, 2024

The wording in this PR seems much stronger than upstream's statements to me:

Upstream [...] has stated that the library should not be used going forward

vs.

Given the above, in the context of Matrix, libolm is currently safe to use in a practical sense (with the above caveats). However, we strongly encourage all app developers to chart a path away from libolm in favour of vodozemac.

I definitely don't see this as implying that they expect distributors or end users to take action, and marking the packages as vulnerable has the IMO fairly severe consequence of resulting in them not being built by Hydra.

@emilazy
Copy link
Member

emilazy commented Sep 2, 2024

The wording is inherited from #334638, and is based on the upstream statement in the libolm repository:

As such as of July 2024, libolm is now officially deprecated - please do not use it going forwards. The Matrix cryptography team sadly does not have bandwidth to maintain both vodozemac and libolm, and all of our maintenance effort will go into vodozemac.

so it’s almost a direct quote. Not much to say beyond that as all sides were pretty thoroughly hashed out in the original PR.

@emilazy
Copy link
Member

emilazy commented Sep 2, 2024

FWIW I don’t have any objection to building libolm on Hydra (we have a mechanism for exceptions to the blanket exclusion of marked‐insecure packages from Hydra builds), but I think we concluded there was no need to because none of the reverse dependencies are particularly heavy builds. If there are clients that would be particularly painful for users to build (maybe Nheko, since it’s C++?), then it seems reasonable to consider adding olm to the list of exceptions.

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Sep 14, 2024

If there are clients that would be particularly painful for users to build (maybe Nheko, since it’s C++?), then it seems reasonable to consider adding olm to the list of exceptions.

Exactly: the nheko developers said they don't have immediate plans to switch to vodozemac; no one has shown how to practically exploit these "vulnerabilities" and building nheko is quite resource intensive. I'm going to add an exception.

@rnhmjoj rnhmjoj mentioned this pull request Sep 14, 2024
13 tasks
@emilazy
Copy link
Member

emilazy commented Sep 14, 2024

An exception for the Hydra permitted insecure packages list would be okay in my view. That means it can still be built on Hydra but the user has to override the warning to use it. Hiding the message entirely on an ad-hoc basis is not.

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Sep 14, 2024

An exception for the Hydra permitted insecure packages list would be okay in my view. That means it can still be built on Hydra but the user has to override the warning to use it. Hiding the message entirely on an ad-hoc basis is not.

So, basically here we have both the upstream developers of the library and the client saying these vulnerabilities are basically non-issue; yet we are breaking setups unless they read a super technical wall of text, follow the instructions and accept the shame of having vulnerable software in their computer.

Okay, I guess this is useful if you're someone like Ed Snowden, but for the average Joe, it's a terrible user experience.

@emilazy
Copy link
Member

emilazy commented Sep 15, 2024

(Replied in #341831 (comment).)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants