Enable modsecurity feature

[aledbf]

Temporal image: quay.io/aledbf/nginx-ingress-controller:0.262

fixes #116

[k8s-reviewable]

This change is 

[coveralls]

Coverage remained the same at 33.582% when pulling 0cdc27b50275f1481405394d2eee05e0beb2fd92 on aledbf:enable-modsec into c873ef6 on kubernetes:master.

[rikatz]

@aledbf Are you aware of this?

This might help in keeping the base image small, while keeping mod_security decoupled of the entire NGINX code :)

Thanks!

[aledbf]

@rikatz yes but is not that easy :) the nginx module uses the libmodsecurity library and that has too many dependencies, so the sidecar is not an option.

[rikatz]

@aledbf Yeap, but my approach is to have the module AND the libmodsecurity as a side car. The module can be compiled as a Dynamic Library, and used only when enabled by the sysadmin :)

I think the big issue here is to keep the NGINX Slim and the Sidecar container synced, as this module relies on NGINX version :)

[aledbf]

@rikatz I think you need to test this to see what I mean. If you have the library in one pod and nginx in another you need to install the libraries in the nginx image. Most of the images are installes in /usr/lib so you end with two big images and no way to ensure it will work after any change

Edit: ldd

root@3f9741a2c71e:/# ldd /usr/local/modsecurity/lib/libmodsecurity.so
	linux-vdso.so.1 =>  (0x00007ffe03fd1000)
	librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007f90cffe8000)
	libcurl.so.4 => /usr/lib/x86_64-linux-gnu/libcurl.so.4 (0x00007f90cfd79000)
	libGeoIP.so.1 => /usr/lib/x86_64-linux-gnu/libGeoIP.so.1 (0x00007f90cfb47000)
	libyajl.so.2 => /usr/lib/x86_64-linux-gnu/libyajl.so.2 (0x00007f90cf93c000)
	libxml2.so.2 => /usr/lib/x86_64-linux-gnu/libxml2.so.2 (0x00007f90cf581000)
	libpcre.so.3 => /lib/x86_64-linux-gnu/libpcre.so.3 (0x00007f90cf310000)
	libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f90cef8e000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f90cec85000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f90ce8ba000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f90ce6a4000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f90ce487000)
	libidn.so.11 => /usr/lib/x86_64-linux-gnu/libidn.so.11 (0x00007f90ce253000)
	librtmp.so.1 => /usr/lib/x86_64-linux-gnu/librtmp.so.1 (0x00007f90ce037000)
	libssl.so.1.0.0 => /lib/x86_64-linux-gnu/libssl.so.1.0.0 (0x00007f90cddce000)
	libcrypto.so.1.0.0 => /lib/x86_64-linux-gnu/libcrypto.so.1.0.0 (0x00007f90cd989000)
	libgssapi_krb5.so.2 => /usr/lib/x86_64-linux-gnu/libgssapi_krb5.so.2 (0x00007f90cd73f000)
	liblber-2.4.so.2 => /usr/lib/x86_64-linux-gnu/liblber-2.4.so.2 (0x00007f90cd530000)
	libldap_r-2.4.so.2 => /usr/lib/x86_64-linux-gnu/libldap_r-2.4.so.2 (0x00007f90cd2de000)
	libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007f90cd0c4000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f90ccec0000)
	libicuuc.so.55 => /usr/lib/x86_64-linux-gnu/libicuuc.so.55 (0x00007f90ccb2b000)
	liblzma.so.5 => /lib/x86_64-linux-gnu/liblzma.so.5 (0x00007f90cc909000)
	/lib64/ld-linux-x86-64.so.2 (0x00005614a6d39000)
	libgnutls.so.30 => /usr/lib/x86_64-linux-gnu/libgnutls.so.30 (0x00007f90cc5d8000)
	libhogweed.so.4 => /usr/lib/x86_64-linux-gnu/libhogweed.so.4 (0x00007f90cc3a5000)
	libnettle.so.6 => /usr/lib/x86_64-linux-gnu/libnettle.so.6 (0x00007f90cc16f000)
	libgmp.so.10 => /usr/lib/x86_64-linux-gnu/libgmp.so.10 (0x00007f90cbeee000)
	libkrb5.so.3 => /usr/lib/x86_64-linux-gnu/libkrb5.so.3 (0x00007f90cbc1c000)
	libk5crypto.so.3 => /usr/lib/x86_64-linux-gnu/libk5crypto.so.3 (0x00007f90cb9ed000)
	libcom_err.so.2 => /lib/x86_64-linux-gnu/libcom_err.so.2 (0x00007f90cb7e8000)
	libkrb5support.so.0 => /usr/lib/x86_64-linux-gnu/libkrb5support.so.0 (0x00007f90cb5dd000)
	libresolv.so.2 => /lib/x86_64-linux-gnu/libresolv.so.2 (0x00007f90cb3c2000)
	libsasl2.so.2 => /usr/lib/x86_64-linux-gnu/libsasl2.so.2 (0x00007f90cb1a6000)
	libgssapi.so.3 => /usr/lib/x86_64-linux-gnu/libgssapi.so.3 (0x00007f90caf65000)
	libicudata.so.55 => /usr/lib/x86_64-linux-gnu/libicudata.so.55 (0x00007f90c94ae000)
	libp11-kit.so.0 => /usr/lib/x86_64-linux-gnu/libp11-kit.so.0 (0x00007f90c9249000)
	libtasn1.so.6 => /usr/lib/x86_64-linux-gnu/libtasn1.so.6 (0x00007f90c9036000)
	libkeyutils.so.1 => /lib/x86_64-linux-gnu/libkeyutils.so.1 (0x00007f90c8e31000)
	libheimntlm.so.0 => /usr/lib/x86_64-linux-gnu/libheimntlm.so.0 (0x00007f90c8c28000)
	libkrb5.so.26 => /usr/lib/x86_64-linux-gnu/libkrb5.so.26 (0x00007f90c899d000)
	libasn1.so.8 => /usr/lib/x86_64-linux-gnu/libasn1.so.8 (0x00007f90c86fb000)
	libhcrypto.so.4 => /usr/lib/x86_64-linux-gnu/libhcrypto.so.4 (0x00007f90c84c8000)
	libroken.so.18 => /usr/lib/x86_64-linux-gnu/libroken.so.18 (0x00007f90c82b1000)
	libffi.so.6 => /usr/lib/x86_64-linux-gnu/libffi.so.6 (0x00007f90c80a9000)
	libwind.so.0 => /usr/lib/x86_64-linux-gnu/libwind.so.0 (0x00007f90c7e7f000)
	libheimbase.so.1 => /usr/lib/x86_64-linux-gnu/libheimbase.so.1 (0x00007f90c7c70000)
	libhx509.so.5 => /usr/lib/x86_64-linux-gnu/libhx509.so.5 (0x00007f90c7a25000)
	libsqlite3.so.0 => /usr/lib/x86_64-linux-gnu/libsqlite3.so.0 (0x00007f90c7750000)
	libcrypt.so.1 => /lib/x86_64-linux-gnu/libcrypt.so.1 (0x00007f90c7517000)

[kfox1111]

@dims came up with an interesting volume driver that seems like a good fit for this use case:
https://github.com/dims/docker-flexvol
Rather then use a docker container as a sidecar, you use it as a volume. You could then mount it in the main container wherever needed.

[rikatz]

Yup, I'm making some tests here :)

@kfox1111 In this case, we don't need to relly in volumes, instead we could mount an 'emptyPath' in both containers with the same location ;)

I'll do some further tests here to see if this might work

[kfox1111]

If you do it as a sidecar, you have to have /bin/sh or something around in the image just to execute something, copy the data out, and to have it do nothing almost all the time. Not really much resource, but cleaner just to have a volume type that does it all for you. If you do it as a volume, the container can be literally just the plugin data. no executable, no processes hanging around after, etc. Super light weight.

[rikatz]

OK, the main problem I'm facing here is not about the libraries itself (the only additional lib I've had to install was libyajl2).

The thing is that NGINX needs the same compilation directives used in the main program to be able to load a dynamic module.

Explaining, nginx-slim image was compiled with the following:

./configure \
  --prefix=/usr/share/nginx \
  --conf-path=/etc/nginx/nginx.conf \
  --http-log-path=/var/log/nginx/access.log \
  --error-log-path=/var/log/nginx/error.log \
  --lock-path=/var/lock/nginx.lock \
  --pid-path=/run/nginx.pid \
  --http-client-body-temp-path=/var/lib/nginx/body \
  --http-fastcgi-temp-path=/var/lib/nginx/fastcgi \
  --http-proxy-temp-path=/var/lib/nginx/proxy \
  --http-scgi-temp-path=/var/lib/nginx/scgi \
  --http-uwsgi-temp-path=/var/lib/nginx/uwsgi \
  ${WITH_FLAGS} \
  --without-mail_pop3_module \
  --without-mail_smtp_module \
  --without-mail_imap_module \
  --without-http_uwsgi_module \
  --without-http_scgi_module [....] 

So, even I want to compile a dynamic module, I've to compile it against the same compilation directives + add-dynamic-modules

What can be done here, anyway is to compile mod_security as an additional step of Ingress compilation process, compile it as a dynamic module and remove anything else generated from it.

The NGINX dynamic module only needs the '.so' generated in modsecurity compilation, so it adds 30Mb into the base image. The libmodsecurity.a file can be removed, as it's the whole static compiled library (and that's why the generated image contains more than 100Mb).

So, the compilation process should be:

apt-get -y install g++ flex bison doxygen libyajl-dev libtool dh-autoreconf libcurl4-gnutls-dev libxml2 libpcre++-dev libxml2-dev  pkgconf libyajl2 # This can be added to the apt-get line in build.sh

[...]
./configure --prefix=/usr/local/modsec # This can be made so we can 'isolate' the libraries, copy the .so and remove anything else later
make && make install

[...]
cd /opt
git clone https://github.com/SpiderLabs/ModSecurity-nginx

export MODSECURITY_INC="/usr/local/modsec/include"
export MODSECURITY_LIB="/usr/local/modsec/lib"

[...]
./configure --add-dynamic-module=/opt/ModSecurity-nginx/ # And anything else to compile NGINX
make modules && make && make install
[...]
# Cleanup
rm -rf /usr/local/modsec/bin /usr/local/modsec/include /usr/local/modsec/lib/libmodsecurity.a

Additionally, this module can be loaded as needed/enabled, so it wouldn't cause additional load to NGINX when not enabled.

30Mb is an acceptable additional size to ingress controller container?

[aledbf]

@rikatz cleaning the image I see an increase of 27MB in the image.
Please check https://quay.io/repository/aledbf/nginx-ingress-controller?tab=tags

Edit: I really don't want two container for this. It makes everything more complex than should be

Edit 2: test image is quay.io/aledbf/nginx-ingress-controller:0.261

[rikatz]

@aledbf +27Mb seems really reasonable to me :)

I'm not able to test this right now, thus I'll test the image itself later today :)

[aledbf]

Test image with dynamic module: quay.io/aledbf/nginx-ingress-controller:0.262

[coveralls]

Coverage remained the same at 33.492% when pulling 7632465 on aledbf:enable-modsec into 63155ee on kubernetes:master.

[ghost]

I have tested this image and I honestly would not include it in any upcoming release. Whilst the actual implementation in the ingress controller is fine, the current state of libmodsecurity is not in any shape to be included.

I have submitted bugs upstream to modsecurity but as an example, I could not get regular expressions in rule targets to work, changing rule targets with inbuilt modifiers caused nginx segmentation faults when the rules were hit, etc. etc. Even with just the Core Rule Set enabled with no overrides will cause unexpected behaviour due to the missing features in this RC.

My only other comments on the actual ingress implementation are that perhaps we should include annotations for overriding the CRS - which is common practise. Because of the way modsecurity provides features to adapt the rulebase both BEFORE and AFTER the inclusion of CRS we should perhaps think about 2 annotations:

ingress.kubernetes.io/modsec-postcrs-snippet
ingress.kubernetes.io/modsec-precrs-snippet

both of which just add a 'modsecurity_rules' section.

[bramvdklinkenberg]

Any update on this? Would be nice to see waf functionality in the nginx controller.