securedrop-workstation icon indicating copy to clipboard operation
securedrop-workstation copied to clipboard

Add .webp support

Open eloquence opened this issue 4 years ago • 14 comments

We should add a .webp capable image viewer to the sd-viewer template. Currently we can convert these files via ImageMagick, but they cannot be successfully opened from the SecureDrop Client.

eloquence avatar Sep 11 '20 23:09 eloquence

(Just confirming that's still on an issue on 4.1/Bullseye)

eloquence avatar Jul 06 '22 18:07 eloquence

We can confirm this is still an issue on 4.1 - the viewer VM starts up then shuts down with no warning

philmcmahon avatar Sep 23 '22 09:09 philmcmahon

Unfortunately the library that would enable eog to open .webp images isn't available in regular bullseye repos, but it is available in bullseye-backports

I'm curious what folks think about adding the backports repo for this, or maybe we want to mirror the package in our own repo?

eaon avatar Sep 23 '22 11:09 eaon

I'm curious what folks think about adding the backports repo for this, or maybe we want to mirror the package in our own repo?

I double-checked and all the dependencies of webp-pixbuf-loader are in bullseye (and not also in backports), so mirroring the package will be straightforward, if that's what we go with.

IMO -backports repos are the equivalent of testing, in that it's often changing, with only a 2-5 days advance warning. My preference is mirroring, just so we can test any updates ahead of time (and we can use the same apt-test automation we use for Tor packages).

The downside of -backports packages (regardless of whether we mirror or not) is that there's no coverage by the Debian Security Team, it's up to the maintainer, with a 2-5 day delay. Though of course the use of Qubes mitigates some of the security issues, though I defer to others on actual analysis/weighting of that.


My other naive suggestion if we're unhappy with backports would be to use something like imagemagick to convert to jpg/png/etc. on the fly and then pass that over to eog. Unsure if that kind of conversion is too much for journalists who want to see the original file.

legoktm avatar Sep 23 '22 17:09 legoktm

@l3th3 @lsd-cat What's your take on using webp-pixbuf-loader? There is https://github.com/aruiz/webp-pixbuf-loader/pull/50 which makes me wonder if it's a bit premature to add it, but conversion doesn't sound amazing either (especially since .webp supports animation IIRC and I dunno how annoying it would be dealing with that)

eaon avatar Sep 27 '22 16:09 eaon

Just to add a bit of outside context, webp-pixbuf-loader was just added as a core dependency to GNOME Shell (they're switching to .webp for backgrounds): https://gitlab.gnome.org/GNOME/gnome-build-meta/-/commit/4bf5ae30f4a3d39e6aa87d6f116e8da2bbfaae63

Eventually this would trickle down anyway, but I think that shows it's fairly battle-tested if we want to jump the gun and include it/mirror it.

nathandyer avatar Sep 27 '22 16:09 nathandyer

Took a look at the project, and it really doesn't seem battle-tested. As far as I can tell, the PR that @eaon linked seems to be the only type of security review its gotten, and it was just fuzzing (and the fixes proposed are a perfect showcase of why we should generally start favoring Rust over C/C++). As for its adoption into GNOME, looking over their issue & PR, security doesn't seem to have been a consideration at all.

I really wouldn't be surprised if there weren't more bugs in the codebase, so recommending it in its current state would be a no from me

ghost avatar Sep 28 '22 11:09 ghost

From a quick survey, I haven't found any native Rust implementations for webp decoding that we could use. Everything seems to fall back on libwebp, the same library that webp-pixbuf-loader uses. ImageMagick uses that as well, and the history of https://github.com/ImageMagick/ImageMagick/blob/main/coders/webp.c doesn't inspire all that much confidence either.

Other than a "no", from your point of view, what's an approach that would allow us to support .webp images in one way or another that you'd be OK with?

eaon avatar Sep 28 '22 15:09 eaon

(As I was cleaning up tabs I came across the post-fuzzing webp-pixbuf-loader PR again - it was closed but changes to address the issue were pushed to the repo a couple hours ago.)

eaon avatar Oct 28 '22 15:10 eaon

It's kindof basic, but in Debian adding webp support via the :drum: webp package also installs vwebp, which would seem to do the job. It uses https://chromium.googlesource.com/webm/libwebp which seems pretty active and well-supported. So we could just skip the gnome bits at the cost of some UX iffyness. (Converting files to png via dwebp and then viewing them might also be an option.)

zenmonkeykstop avatar Oct 28 '22 19:10 zenmonkeykstop

So, to recap, if I understand correctly, there are two primary options under discussion:

  • eog, which would require webp-pixbuf-loader from Bullseye backports. @l3th3 expresses concerns above about the maturity of the webp-pixbuf-loader component.
  • vwebp, which is installable via the webp package in Bullseye (but which provides no friendly GUI like eog does, nor any advanced viewer functionality like zooming; it only displays the file in a frame).

I think implicit in this choice are a couple of questions:

  • Does vwebp have any technical maturity issues similar to webp-pixbuf-loader that would make it not a suitable option?
  • What level of maturity is required for a viewer application to be considered suitable to run in a networkless, disposable VM?
  • If eog with webp-pixbuf-loader was determined to be the best option, would we consider it acceptable to install it from the backports channel?

If I understand correctly, we would have to solve separately for the printability of webp files.

What do y'all think, particularly @l3th3 (and @lsd-cat if you have a chance to weigh in) from a security perspective? I'll post my personal take as a separate comment to distinguish it from the summary above.

eloquence avatar Nov 01 '22 00:11 eloquence

Personal take: Based on the comments above, it seems like installing vwebp via the webp package seems like the pragmatic option for Bullseye (so we satisfy the immediate user need), and we could have a longer range discussion (informed by our threat model) about better defining our standards for viewer applications, which is also relevant in the context of other issues like https://github.com/freedomofpress/securedrop-workstation/issues/842.

eloquence avatar Nov 01 '22 00:11 eloquence

CVE-2023-4863 in libwebp was actively exploited by NSO Group and required critical fixes in iOS, Firefox, Chrome, Tor Browser and more so it seems both important that we can provide this functionality in safe manner as well as treading carefully on which webp rendering implementation we pick.

legoktm avatar Sep 16 '23 15:09 legoktm

FYI, it looks like https://github.com/image-rs/image can decode webp images without the use of libwebp

eaon avatar Sep 18 '23 14:09 eaon

Now that we're on bookworm, eog now depends on webp-pixbuf-loader, so it's already installed in the large template.

legoktm avatar Jul 09 '24 16:07 legoktm

I tested this and it all works fine on 1.0.0rc2.

I tried a still webp image, and an animated webp image, both rendered just like how Firefox does.

I will put it on the agenda for tomorrow's meeting just to make sure there are no unresolved security issues here, but as it stands, webp should be supported in the next SDW release.

legoktm avatar Jul 09 '24 18:07 legoktm

I will put it on the agenda for tomorrow's meeting just to make sure there are no unresolved security issues here, but as it stands, webp should be supported in the next SDW release.

There were no security concerns. There was some concern that it didn't require us to edit any of our mime lists, and that stronger control would be ideal, but that doesn't block this. Some related tickets for follow-up: https://github.com/freedomofpress/securedrop-client/issues/2007#issuecomment-2219255581, https://github.com/freedomofpress/securedrop-workstation/issues/1139.

I'll close this issue by adding a changelog entry.

legoktm avatar Jul 11 '24 14:07 legoktm