beetlebugorg / mod_dims

Apache HTTP dynamic image resizing module
Other
43 stars 27 forks source link

Add support for AES/GCM/NoPadding decryption of eurl parameter #50

Closed atarnvik closed 3 months ago

atarnvik commented 3 months ago

Doubts:

  1. Should the new Encryption Algorithm Setting be a client setting or global setting? Was unsure on the distinction between those.
  2. Is encryption_algorithm and appropriate name for the configuration? There's technically two distinct algos used, one for the signature, which this doesn't impact, and one for the watermarked URL, which this PR addresses.
  3. Is the error logging helpful or excessive/unnecessary?
atarnvik commented 3 months ago

@beetlebugorg One thing I should mention. I had trouble running DIMS locally using the latest code on master (even without my changes in this PR). I had to test against the 3.3.26 tag (which is the version we have been running in our environments).

Error when building a docker container against master branch:

[+] Building 3.3s (12/30)                                                                                                                                           docker:desktop-linux
 => [internal] load build definition from Dockerfile                                                                                                                                0.0s
 => => transferring dockerfile: 2.40kB                                                                                                                                              0.0s
 => [internal] load metadata for docker.io/library/ubuntu:focal                                                                                                                     0.2s
 => [internal] load .dockerignore                                                                                                                                                   0.0s
 => => transferring context: 2B                                                                                                                                                     0.0s
 => [internal] load build context                                                                                                                                                   0.0s
 => => transferring context: 162.34kB                                                                                                                                               0.0s
 => [deps 1/2] FROM docker.io/library/ubuntu:focal@sha256:0b897358ff6624825fb50d20ffb605ab0eaea77ced0adb8c6a4b756513dec6fc                                                          0.0s
 => CACHED [deps 2/2] RUN apt-get -y update && apt-get -y install apache2 && rm -rf /var/lib/apt /var/lib/dpkg/info/*                                                               0.0s
 => CACHED [builder 1/7] RUN apt-get -y update && apt-get install -y checkinstall automake libtool autoconf git apache2-dev     libcurl4-openssl-dev libfreetype6-dev libopenexr-d  0.0s
 => CACHED [builder 2/7] RUN mkdir /work                                                                                                                                            0.0s
 => CACHED [builder 3/7] WORKDIR /work                                                                                                                                              0.0s
 => [builder 4/7] COPY ./mod_dims /work/mod_dims                                                                                                                                    0.0s
 => [builder 5/7] WORKDIR mod_dims                                                                                                                                                  0.0s
 => ERROR [builder 6/7] RUN ./autorun.sh --with-apxs=`which apxs2` && make                                                                                                          2.9s
------
 > [builder 6/7] RUN ./autorun.sh --with-apxs=`which apxs2` && make:
1.831 libtoolize: putting auxiliary files in '.'.
1.831 libtoolize: copying file './ltmain.sh'
1.836 libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'.
1.836 libtoolize: copying file 'm4/libtool.m4'
1.842 libtoolize: copying file 'm4/ltoptions.m4'
1.850 libtoolize: copying file 'm4/ltsugar.m4'
1.857 libtoolize: copying file 'm4/ltversion.m4'
1.864 libtoolize: copying file 'm4/lt~obsolete.m4'
2.709 configure.ac:7: installing './compile'
2.709 configure.ac:9: installing './config.guess'
2.710 configure.ac:9: installing './config.sub'
2.711 configure.ac:3: installing './install-sh'
2.712 configure.ac:3: installing './missing'
2.714 Makefile.am: installing './INSTALL'
2.726 src/Makefile.am: installing './depcomp'
2.890 make: *** No targets specified and no makefile found.  Stop.
------
Dockerfile:23
--------------------
  21 |     WORKDIR mod_dims
  22 |
  23 | >>> RUN ./autorun.sh --with-apxs=`which apxs2` && make
  24 |     RUN checkinstall --maintainer=REDACTED --pkgrelease="REDACTED.$(lsb_release -sc)" \
  25 |         --pkglicense="Apache 2.0" --pkgversion=$DIMS_VERSION --pkgname=mod-dims --default --requires=imagemagick,libcurl4,libpangocairo-1.0-0
--------------------
ERROR: failed to solve: process "/bin/sh -c ./autorun.sh --with-apxs=`which apxs2` && make" did not complete successfully: exit code: 2

I did not dig into it much yet.

beetlebugorg commented 3 months ago

Can you provide the command you ran to build?

atarnvik commented 3 months ago

Can you provide the command you ran to build?

RUN ./autorun.sh --with-apxs=`which apxs2` && make

I took a second to look at that and realized what's causing the issue:

https://github.com/beetlebugorg/mod_dims/compare/release/3.3.26...release/3.3.27#diff-253f4a27d4724a9bcfb270dc9fcc07a22d5264e6721a14de977a715d408a0cfdL16

3.3.27 removed the ./configure call from autorun.sh. I've confirmed adding it to the RUN command above fixes the issue.

So we can update the command on our end to include it, unless you think it should be added back to autorun.sh.

beetlebugorg commented 3 months ago

I think it's reasonable to add it back to the end of autorun.sh. Probably just an oversight.

I thought you were building ./docker/Dockerfile, I realize now this your own Dockerfile. :)

atarnvik commented 3 months ago

Is it ok to run ./configure twice? Noticed in another repo that has upgraded past DIMS 3.3.27 that they already added the ./configure after the ./autorun command.

And yes, sorry, should have said, this is my own Dockerfile. I went with the path of least resistance (at least for me) to get testing going.

beetlebugorg commented 3 months ago

It doesn't hurt to run it multiple times. Just extra time.

atarnvik commented 3 months ago

Ok, added it back to autorun.sh. I also triple checked that everything is functioning against latest code.