Skip to content

Replace Imagick with something better #13099

Description

@enoch85

EDIT (SEO): The PHP module "imagick" is not enabled although the theming app is. For favicon generation to work correctly, you need to install and enable this module.

A few days ago it was brought up to my attention that using Imagick could have very negative effects on security. The Nextcloud snap decided to not using it due to that fact, and I've now mitigated the same threat(s) as well by not using it in the Nextcloud VM.

Here are the discussion regarding the decision in the Nextcloud snap, and I think it totally makes sense not to use it in the Nextcloud Server as well.

The situation now though is that it's recomended and the setup checks will inform the user that the package is missing. As Nextcloud is advertising it's secure, then why use a package that is prune to a lot of CVEs in the past?

Regarding alternatives I think this post sums it up quite well.

Please consider removing the recommendation in future versions, and please also consider replacing the use of Imagick with something better and more secure.

EDIT 2: We now install Imaginary as a replacement for this in the Nextcloud VM.

Activity

  1. kesselb commented on Dec 16, 2018

    @kesselb
    Contributor

    Ref #12821

    I see the concerns and could imagine to move the warning into the theming app that some feature are not working because imagick is not present.

    @skjnldsv @juliushaertl looks like @nextcloud/vm and @nextcloud/snap not going to add imagick. @nextcloud/docker added imagick a few days ago.

    Edit: There is already a warning that some things does not work:

    <?php if (!$_['canThemeIcons']) { ?>

  2. skjnldsv commented on Dec 16, 2018

    @skjnldsv
    Member

    Please consider removing the recommendation in future versions, and please also consider replacing the use of Imagick with something better and more secure.

    Nothing allow us to properly convert images types (especially svg) with php other than imagick (that I'm aware of) unfortunately.

  3. kesselb commented on Dec 16, 2018

    @kesselb
    Contributor

    Apart from this favicon svg generation the theming app works fine without imagick?

  4. skjnldsv commented on Dec 16, 2018

    @skjnldsv
    Member

    @danielkesselberg avatars will be not as great looking without imagick.
    Previews too :)

  5. J0WI commented on Dec 17, 2018

    @J0WI
    Contributor

    How about the performance without imagick?
    This does also affect the gallery app: https://github.com/nextcloud/gallery#supporting-more-media-types

  6. cyphar commented on Dec 17, 2018

    @cyphar

    From what I've read, most of the ImageMagick CVEs come from individual filters or filetypes that we probably don't care about -- is there a way to whitelist policy.xml so that we only allow the few formats that we want to support? In addition, checking that the magic header of files is actually correct (for the formats we want to support) would protect against most malicious files.

    ImageMagick (and GraphicsMagick to a lesser extent) have had quite a large number of CVEs, mostly due to the sheer amount of formats and features that users need to process images. I would say a good first step would be to figure out a whitelist hardening configuration that we can use across the board, and then we can evaluate switching away from ImageMagick if that's not sufficient.

  7. J0WI commented on Dec 17, 2018

    @J0WI
    Contributor
  8. cyphar commented on Dec 17, 2018

    @cyphar

    There are also a couple of ways we could restrict ImageMagick through seccomp or by putting it inside an empty network namespace (and a mount namespace which has everything mounted-over except /lib64 and the file that is being accessed -- though this will require a bit of work since it requires having some sort of unveil feature).

  9. enoch85 commented on Dec 17, 2018

    @enoch85
    MemberAuthor

    @skjnldsv So something like https://github.com/flyimg/flyimg wouldn't work?

  10. skjnldsv commented on Dec 17, 2018

    @skjnldsv
    Member

    @enoch85 that would require shell_exec. Yes, we could rely on external software (like inkscape for example). But this is not really recommended to do on php. @rullzer ?

    EDIT: sorry, I thought it was another software. Yes, we can use an external docker as well. We actually have a PoC somewhere for that. But this is a really heavy dependency and this would not scale to every setup. Also, most people don't use docker :/

  11. kyrofa commented on Dec 18, 2018

    @kyrofa
    Member

    Thanks for this, @enoch85. It's probably no surprise that I completely agree on this issue. In the snap it's not even possible for people to use it, so folks will just see the warning forever and be unable to do anything. So at the very least, packagers should be able to disable this warning without triggering an integrity failure. Even better, Nextcloud should just stop suggesting it be installed if it's not. If one doesn't miss the functionality it provides, all it does is make the general populous less secure. Best yet: find an alternative so everyone can enjoy the functionality without trading security for it.

  12. skjnldsv commented on Dec 18, 2018

    @skjnldsv
    Member

    Best yet: find an alternative so everyone can enjoy the functionality without trading security for it.

    Let's be clear here, we all agree 😝
    Having another php lib that could do the job would be ideal, but I don't have anything else to suggest unfortunately. I'm open to suggestion though :)

  13. 40 remaining items

  14. changed the title [-]Don't use Imagick in server, and don't recommend it [/-] [+]Replace Imagick with Graphicsmagick[/+] on Jul 27, 2021
  15. szaimen commented on Jul 27, 2021

    @szaimen
    Contributor

    cc @nextcloud/server @nextcloud/security

  16. J0WI commented on Jul 27, 2021

    @J0WI
    Contributor

    Anyone familiar with libvips? The benchmarks and reviews look great.

  17. tianon commented on Jul 27, 2021

    @tianon

    My experience with it was in the context of Ghost via "sharp" requiring too new of a libvips (Debian Buster has 8.7 and they required 8.9+) which caused a host of issues for getting it successfully installed, so I'd suggest surveying what version of libvips is available in the expected target environments and ensuring the lowest common denominator meets the needs of the project before committing (but that's just my 2c; no real stake here 😇).

  18. changed the title [-]Replace Imagick with Graphicsmagick[/-] [+]Replace Imagick with something better and[/+] on Aug 1, 2021
  19. changed the title [-]Replace Imagick with something better and[/-] [+]Replace Imagick with something better[/+] on Aug 1, 2021
  20. enoch85 commented on Aug 1, 2021

    @enoch85
    MemberAuthor

    I still think this is an ongoing discussion though.

    Mainly this issue exist due to the security concerns, and whatever replacing Imagick needs to be better OR not produce a warning.

  21. PVince81 commented on Aug 2, 2021

    @PVince81
    Member

    also note: some research done with an external preview generator: #24166
    (note: I don't have time right now to continue this, feel free to take over)

  22. added and removed
    1. to developAccepted and waiting to be taken care of
    on Aug 8, 2021
  23. adripo commented on Nov 13, 2021

    @adripo

    While someone will continue developing this feature, what do you think if we proceed by marking the warning as INFO in the Administration Overview as suggested by @kerberizer in nextcloud/docker/1414#issuecomment-945842317?

  24. solracsf commented on Mar 19, 2022

    @solracsf
    Member

    Closing as per #24166

  25. Fuseteam commented on Mar 19, 2022

    @Fuseteam

    oh man, that's awesome

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions