openwrt / packages

Community maintained packages for OpenWrt. Documentation for submitting pull requests is in CONTRIBUTING.md
GNU General Public License v2.0
3.89k stars 3.4k forks source link

nginx: Dynamic modules and other changes to Makefile #16833

Open alex9434 opened 2 years ago

alex9434 commented 2 years ago

Maintainer: @Ansuel @mhei Environment: all

Description: Suggested changes to NGINX Makefile for OpenWRT.

Dear all,

Can I suggest the following 3 changes to the NGINX makefile:

1. Add MP4 pseudo-streaming: The makefile already contains the option --with-http_flv_module to add FLV pseudo streaming. NGINX has the comparable functionality to add MP4 pseudo streaming --with-http_mp4_module. The code for the module is included in the NGINX source code. Given the more widespread use of MP4 compared to FLV today that module might be very useful.

2. Add Digest authentication Because some routers/boards do not have hardware encryption support it might be helpful to be able to stream media over HTTP instead of HTTPS. Digest Authentication offers better protection than Basic Authentication over HTTP because the password is not transmitted in cleartext. https://www.nginx.com/resources/wiki/modules/auth_digest/

3. Compile Dynamic Modules instead of Static Modules In NGINX 1.9.11 onwards selected modules can be loaded into NGINX at runtime based on configuration files. This can help to reduce memory requirements by only loading modules that are needed. Currently, all modules are statically compiled into NGINX. Can we change this to dynamic loading? https://www.nginx.com/blog/compiling-dynamic-modules-nginx-plus/

Next steps? I have already made the changes for 1 and 2 by myself to the Makefile and Config-ssl.in (they are actually trivial). If you agree with these suggestions, I am happy to work on a pull request (have never done this before). On point 3 it would be good to get your view and the suggested approach.

Thank you for your kind consideration!

Ansuel commented 2 years ago

Yes adding support for dynamic loading has always been my dream but tons of work and some module can't be compiled that way...

The first thing i would do would be find the module that can't be compiled dynamically and create a list. In theory converting all of this to dynamic module should not be hard but we first need to understand the module that we can't compile

alex9434 commented 2 years ago

Hi @ansuel, I am happy to go for you through the current list of all modules and identify the ones that cannot be compiled dynamically. Would that help and would you then be willing to modify the makefile so that the rest are compiled dynamically? Would you also consider my suggestions under 1 and 2?

Ansuel commented 2 years ago

@alex9434 sure make the pr but i would give priority to dynamic module honestly. the list would make the introduction of dynamic module much easier.

alex9434 commented 2 years ago

@Ansuel, fully agree with your priorities! I have looked at the modules that Debian provides as dynamic modules. Theoretically, it would be possible to try to convert additional modules to dynamic but Debian has probably already excluded the cases where that does not make sense.

The following modules from your Makefile in Master are loaded dynamically in Debian:

--with-stream=dynamic
--add-dynamic-module=/build/nginx-UdHF0z/nginx-1.18.0/debian/modules/http-headers-more-filter
--add-dynamic-module=/build/nginx-UdHF0z/nginx-1.18.0/debian/modules/http-dav-ext
--add-dynamic-module=/build/nginx-UdHF0z/nginx-1.18.0/debian/modules/http-lua
--add-dynamic-module=/build/nginx-UdHF0z/nginx-1.18.0/debian/modules/rtmp

The blog that I linked in my earlier post also describes how to compile NAXSI dynamically which is also included in your makefile.

NGINX on Debian also has the following additional dynamic modules

--with-http_geoip_module=dynamic
--with-http_image_filter_module=dynamic
--with-http_perl_module=dynamic
--with-http_xslt_module=dynamic
--with-mail=dynamic
--with-stream_geoip_module=dynamic
--add-dynamic-module=/build/nginx-UdHF0z/nginx-1.18.0/debian/modules/http-auth-pam
--add-dynamic-module=/build/nginx-UdHF0z/nginx-1.18.0/debian/modules/http-cache-purge
--add-dynamic-module=/build/nginx-UdHF0z/nginx-1.18.0/debian/modules/http-ndk
--add-dynamic-module=/build/nginx-UdHF0z/nginx-1.18.0/debian/modules/http-echo
--add-dynamic-module=/build/nginx-UdHF0z/nginx-1.18.0/debian/modules/http-fancyindex
--add-dynamic-module=/build/nginx-UdHF0z/nginx-1.18.0/debian/modules/http-geoip2
--add-dynamic-module=/build/nginx-UdHF0z/nginx-1.18.0/debian/modules/nchan
--add-dynamic-module=/build/nginx-UdHF0z/nginx-1.18.0/debian/modules/http-uploadprogress
--add-dynamic-module=/build/nginx-UdHF0z/nginx-1.18.0/debian/modules/http-upstream-fair
--add-dynamic-module=/build/nginx-UdHF0z/nginx-1.18.0/debian/modules/http-subs-filter

Can we start with the 5+1 modules from the first list? I am happy to experiment whether further modules from the Makefile can be converted to dynamic once we have the five dynamic modules. OK for you? Any other way I can support you?

Ansuel commented 2 years ago

This night i will check the required changes for the makefile and post here a pof...

anyway in theory most of the standard module are in the nginx wiki and everyone is listed if can be loaded dynamically or not.

Ansuel commented 2 years ago

@alex9434 i made a first version of this. Can you test it?

The idea is to test the pr... merge that and with the current framework, you can add all the package you need hoping they are dynamic.

alex9434 commented 2 years ago

@alex9434 i made a first version of this. Can you test it?

The idea is to test the pr... merge that and with the current framework, you can add all the package you need hoping they are dynamic.

Will test it tonight. Thank you!

alex9434 commented 2 years ago

@Ansuel, compiling went through on arm64 and x64. However, I get the following error when starting NGINX:

nginx -T -c '/etc/nginx/uci.conf'
2021/10/10 06:51:40 [emerg] 6210#0: "load_module" directive is not allowed here in /etc/nginx/conf.d/luci.locations:19
nginx: configuration file /etc/nginx/uci.conf test failed

The error is that load_module is in the wrong context. The line: load_module /usr/modules/ngx_http_ubus_module.so; needs to go into uci.conf.template instead of conf.d/luci.locations because.

After moving it to uci.conf.template, I get the following error.

2021/10/10 07:08:11 [emerg] 5928#0: dlsym() "/usr/modules/ngx_http_ubus_module.so", "ngx_modules" failed (Symbol not found: ngx_modules) in /etc/nginx/uci.conf:7
nginx: configuration file /etc/nginx/uci.conf test failed

I then looked at the directory /usr/modules:

root@OpenWrt:/usr/modules# ls -l
-rwxr-xr-x    1 root     root          4099 Aug 31 22:20 ngx_http_brotli_filter_module.so
-rwxr-xr-x    1 root     root          4099 Aug 31 22:20 ngx_http_brotli_static_module.so
-rwxr-xr-x    1 root     root          4099 Aug 31 22:20 ngx_http_dav_ext_module.so
-rwxr-xr-x    1 root     root          4099 Aug 31 22:20 ngx_http_headers_more_filter_module.so
-rwxr-xr-x    1 root     root          4099 Aug 31 22:20 ngx_http_lua_module.so
-rwxr-xr-x    1 root     root          4099 Aug 31 22:20 ngx_http_naxsi_module.so
-rwxr-xr-x    1 root     root          4099 Aug 31 22:20 ngx_http_ts_module.so
-rwxr-xr-x    1 root     root          4099 Aug 31 22:20 ngx_http_ubus_module.so
-rwxr-xr-x    1 root     root          4099 Aug 31 22:20 ngx_rtmp_module.so
-rwxr-xr-x    1 root     root          4099 Aug 31 22:20 ngx_stream_module.so

All modules have the exact size of 4099. Do you have any idea what went wrong?

alex9434 commented 2 years ago

Regarding MP4 could you kindly add the following to the Makefile?

...
CONFIG_NGINX_MP4 \
...
  ifeq ($(CONFIG_NGINX_MP4),y)
    ADDITIONAL_MODULES += --with-http_mp4_module
  endif
...
 ADDITIONAL_MODULES += --with-ipv6 --with-http_stub_status_module --with-http_flv_module  --with-http_mp4_module
and to Config_ssl.in
config NGINX_MP4
    bool
    prompt "Enable MP4 module"
    help
        Provides the ability to seek within MP4 files using time-based offsets.
    default n

Regarding digest authentication, could you add the following to the Makefile?

...

CONFIG_NGINX_HTTP_AUTH_DIGEST \
...
  ifeq ($(CONFIG_NGINX_HTTP_AUTH_DIGEST),y)
    ADDITIONAL_MODULES += --add-module=$(PKG_BUILD_DIR)/nginx-auth-digest
  endif
...
define Download/nginx-auth-digest
  VERSION:=b3073ef3624ec0e590671399e7b8f31458218d2a
  SUBDIR:=nginx-auth-digest
  FILE:=nginx-http-auth-digest-$$(VERSION).tar.xz
  URL:=https://github.com/atomx/nginx-http-auth-digest.git
  MIRROR_HASH:=24511f8c0867ce3e0961766f33d2a4020aeeda19325ab4400c8abf545c999b93
  PROTO:=git
endef

define Prepare/nginx-auth-digest
    $(eval $(Download/nginx-auth-digest))
    xzcat $(DL_DIR)/$(FILE) | tar -C $(PKG_BUILD_DIR) $(TAR_OPTIONS)
endef

And to Config_ssl.in

config NGINX_HTTP_AUTH_DIGEST
    bool
    prompt "Enable HTTP auth digest"
    default n

That would be very helpful!