readthedocs / readthedocs-docker-images

Docker image definitions used by Read the Docs
115 stars 70 forks source link

Is it possible to change ImageMagick policy? #155

Open mgeier opened 4 years ago

mgeier commented 4 years ago

When trying to convert a PDF file to PNG, I get the following error:

RuntimeError: Error running 'convert -density 96 /tmp/magic_call-rrtbdrjl/magic_call.pdf /tmp/magic_call-rrtbdrjl/magic_call.pdf.png' to create 'magic_call.pdf.png':
convert-im6.q16: not authorized `/tmp/magic_call-rrtbdrjl/magic_call.pdf' @ error/constitute.c/ReadImage/412.
convert-im6.q16: no images defined `/tmp/magic_call-rrtbdrjl/magic_call.pdf.png' @ error/convert.c/ConvertImageCommand/3258.

This seems to be due to a security problem that has been fixed in the meantime but a safety mechanism has not been reverted (AFAIU, but I don't really understand the details).

The suggested fix is to change the file /etc/ImageMagick-6/policy.xml from having the line

<policy domain="coder" rights="none" pattern="PDF" />

to the line

<policy domain="coder" rights="read" pattern="PDF" />

Here are some links that I have found:

https://www.scivision.dev/imagemagick-convert-not-authorized/ https://stackoverflow.com/questions/53377176/change-imagemagick-policy-on-a-dockerfile

https://stackoverflow.com/questions/52861946/imagemagick-not-authorized-to-convert-pdf-to-an-image https://askubuntu.com/questions/1081895/trouble-with-batch-conversion-of-png-to-pdf-using-convert https://stackoverflow.com/questions/42928765/convertnot-authorized-aaaa-error-constitute-c-readimage-453

Is it possible to fix this in the docker image?

An alternative to fixing this would be to install poppler-utils instead: #154.

Ideally both things could be done.

humitos commented 3 years ago

Is this still an issue on Ubuntu 20.04 LTS?

I spun up an Ubuntu 20.04 LTS instance and it seems it is:

root@bb0cd7dcac02:/home/docs# cat /etc/ImageMagick-6/policy.xml | grep coder | grep PDF
  <policy domain="coder" rights="none" pattern="PDF" />

An alternative to fixing this would be to install poppler-utils instead: #154.

After installing poppler-utils package I still see the same line in the policy.xml. Is installing this package a current working workaround?

Is it possible to fix this in the docker image?

I personally wouldn't like to deal with this problem if possible. I don't have deep enough knowledge about PDF generation to be responsible to maintain this change on Read the Docs. However, if we know there is a fix/workaround the user can do by themselves, I'd like to have it documented.

mgeier commented 2 years ago

Is this still an issue on Ubuntu 20.04 LTS?

I spun up an Ubuntu 20.04 LTS instance and it seems it is:

root@bb0cd7dcac02:/home/docs# cat /etc/ImageMagick-6/policy.xml | grep coder | grep PDF
  <policy domain="coder" rights="none" pattern="PDF" />

Yes, rights="none" is unchanged.

The suggested fix would be to use rights="read".

But for me personally, that's not really important, I prefer the poppler-utils solution anyway:

An alternative to fixing this would be to install poppler-utils instead: #154.

After installing poppler-utils package I still see the same line in the policy.xml.

Yes, installing poppler-utils doesn't change anything regarding ImageMagick. Sorry if I wasn't clear about that.

Is installing this package a current working workaround?

Yes, installing poppler-utils enables the use of pdftoppm, which is a (superior) alternative to using ImageMagick's convert.

The convert tool is great to have because it can handle a lot of formats, but for many (most?) specific formats, there are higher-quality alternatives available.

Is it possible to fix this in the docker image?

I personally wouldn't like to deal with this problem if possible. I don't have deep enough knowledge about PDF generation to be responsible to maintain this change on Read the Docs.

No problem, it's not that important, but merging #154 would be great!

astrojuanlu commented 2 years ago

Hi @mgeier , thanks for the detailed answer. About your last point:

but merging #154 would be great!

Since now users can install their own packages specifying build.apt_packages, we are pushing for that instead of preinstalling packages in the images. Therefore, we will not be merging #154.

astrojuanlu commented 2 years ago

However, there will still be a certain number of basic packages installed by default, right?

I think it would still be helpful to install the package poppler-utils by default, just like convert from ImageMagick is installed by default.

Conversion between common image formats should work out of the box, IMHO.

(From your comment on https://github.com/readthedocs/readthedocs-docker-images/pull/154#issuecomment-843401302)

It can be argued though that most of the RTD users will probably not need this, otherwise (supposedly) we would be getting requests about this quite often, which is not the case.

humitos commented 2 years ago

Thanks a lot @mgeier for the detailed explanation! 🚀

Also, thanks @astrojuanlu for chiming in. I agree with what you've said. I don't think we should install by default poppler-utils having the ability to use apt_packages now.