diff --git a/Dockerfile b/Dockerfile index e441598..2876c8d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,7 @@ FROM ubuntu:latest WORKDIR . -RUN apt-get update && apt-get install -y curl gcc make git libpcre3-dev zlib1g-dev libbsd-dev +RUN apt-get update && apt-get install -y curl gcc make git libpcre3-dev zlib1g-dev ADD ipscrub /ipscrub/ ADD Makefile / diff --git a/Makefile b/Makefile index ebd4dfb..2899844 100644 --- a/Makefile +++ b/Makefile @@ -61,6 +61,3 @@ run-demo: docker build -t ipscrub . docker build -t ipscrub-demo-client demo/ docker-compose up --abort-on-container-exit - -check-up-to-date: script/check-up-to-date.sh - $< diff --git a/README.md b/README.md index fdd5c98..827cc59 100644 --- a/README.md +++ b/README.md @@ -9,18 +9,17 @@ * [Security Model](#security-model) * [Threat Model](#threat-model) * [Usage](#usage) -* [Changelog](#changelog) * [GDPR](#gdpr) * [YAGNI](#yagni) * [License](#license) ## Security Model -1. On initialization, and again every `PERIOD`, generate `salt` using 128bits from `arc4random_buf()`. +1. On initialization, and again every `PERIOD`, generate `salt` as `HASH(ngx_random() ++ timestamp)`. 2. On each request, generate masked IP address as `HASH(salt ++ IP address)`. 3. Log masked IP address. -`ipscrub` uses `arc4random` to generate random nonces (see [Theo de Raat's talk on arc4random](https://www.youtube.com/watch?v=aWmLWx8ut20) for a great overview). On Linux this requires installing [libbsd](https://libbsd.freedesktop.org/wiki/) (package libbsd-dev on Ubuntu/Debian). +`ipscrub` uses `ngx_random` to generate random nonces. `ngx_random` is defined as the C `random()` function on non-Windows platforms, and `rand()` on Windows. NOTE: this is not a cryptographically secure RNG, but for the following threat model, that is ok. ALSO NOTE: the generated hash WILL change on each `PERIOD` transition, so you will only have continuity within each `PERIOD`. But because users can transition between networks at any time (e.g. wifi -> cellular), you'd have this type of issue even if you were storing raw IPs. @@ -35,18 +34,8 @@ Scenario (2) is defended against because the server operator does not know the s ## Usage -### Installation - -#### Building From Source - `ipscrub` can be built statically with nginx or as a [dynamic module](https://www.nginx.com/blog/compiling-dynamic-modules-nginx-plus/). See the `Makefile` for examples of both ways. -#### Packages - -- [Arch Linux](https://aur.archlinux.org/packages/nginx-mod-ipscrub/) (thanks to [@deep-42-thought](https://github.com/deep-42-thought)) - -#### Configuration - In your `nginx.conf`, 1. At the top-level, load the module by adding the line `load_module ngx_ipscrub_module.so;` (NOTE: only if you built as a dynamic module). @@ -54,23 +43,6 @@ In your `nginx.conf`, 1. In your `log_format` directives, replace `$remote_addr` with `$remote_addr_ipscrub`. 1. Reload your nginx config. -**NOTE**: nginx may still leak IP addresses in the error log. If this is a concern, disable error logging or wipe the log regularly. - -#### Running Tests - -`make test` - -#### Checking for Updates - -`make check-up-to-date` - -This will have a non-zero exit code if you aren't up-to-date, so you can automate regular checks. - -### Changelog - -- 1.0.1 fixed vulnerability to unmasking hashed IPs (thanks to [@marcan](https://github.com/marcan)) -- 1.0.0 initial release - ## GDPR [GDPR](https://www.eugdpr.org) goes into effect on May 25, 2018. It legislates the handling of personal data about your users, including IP addresses. diff --git a/ipscrub/config b/ipscrub/config index 8d91d40..d261e37 100644 --- a/ipscrub/config +++ b/ipscrub/config @@ -4,13 +4,7 @@ ngx_module_type=HTTP ngx_module_name=ngx_ipscrub_module ngx_module_deps="$ngx_addon_dir/src/ngx_ipscrub_support.h $ngx_addon_dir/src/ngx_ipscrub_debug.h" ngx_module_srcs="$ngx_addon_dir/src/ngx_ipscrub_module.c $ngx_addon_dir/src/ngx_ipscrub_support.c $ngx_addon_dir/src/ngx_ipscrub_debug.c" - -# On Linux, link with libbsd, to get arc4random. -if [ `uname` = Linux ]; then - ngx_module_libs="SHA1 -lbsd" -else - ngx_module_libs="SHA1" -fi +ngx_module_libs=SHA1 . auto/module diff --git a/ipscrub/src/ngx_ipscrub_module.c b/ipscrub/src/ngx_ipscrub_module.c index 8176403..09a2ef4 100644 --- a/ipscrub/src/ngx_ipscrub_module.c +++ b/ipscrub/src/ngx_ipscrub_module.c @@ -42,8 +42,7 @@ static ngx_command_t ngx_ipscrub_commands[] = { // Globals. const int default_period_seconds = 10 * 60; // Period between salt changes. time_t period_start = -1; -#define num_nonce_bytes 16 -u_char nonce[num_nonce_bytes]; // Input to salt generation. +long nonce = -1; // Input to salt generation. static ngx_http_module_t ngx_ipscrub_module_ctx = { @@ -147,17 +146,13 @@ ngx_http_variable_remote_addr_ipscrub(ngx_http_request_t *r, ngx_http_variable_v // Regenerate salt if past end of period. time_t now = time(NULL); if (period_start == -1 || now - period_start > icf->period_seconds) { - rc = randbytes((u_char *) &nonce, num_nonce_bytes); - if (rc != NGX_OK) { - return NGX_HTTP_INTERNAL_SERVER_ERROR; - } - + nonce = ngx_random(); // TODO: actually calculate when period_start should have been. period_start = now; } salt.data = (u_char *) &nonce; - salt.len = num_nonce_bytes; + salt.len = sizeof(nonce); // Although ngx_crypt provides a salted SHA function, specified by a salt beginning with {SSHA}, that function exposes the salt in its result. For our security model, this is inappropriate. Instead, we use the regular nginx SHA function specified by {SHA}, and manually combine the nonce and plaintext. rc = concat(r->pool, r->connection->addr_text, salt, &combined); diff --git a/ipscrub/src/ngx_ipscrub_support.c b/ipscrub/src/ngx_ipscrub_support.c index e83e0bf..c409d20 100644 --- a/ipscrub/src/ngx_ipscrub_support.c +++ b/ipscrub/src/ngx_ipscrub_support.c @@ -3,15 +3,6 @@ #include "ngx_ipscrub_support.h" -#if (NGX_FREEBSD || NGX_SOLARIS || NGX_DARWIN) -// arc4random is built-in on these platforms. -#elif (NGX_LINUX) -#include -#else -// TODO: test using libbsd on Windows. -#error ipscrub requires arc4random_buf. -#endif - // null_terminate allocates a new, null-terminated string based on input. ngx_int_t null_terminate(ngx_pool_t *pool, ngx_str_t input, u_char **out) { @@ -45,20 +36,3 @@ ngx_int_t concat(ngx_pool_t *pool, ngx_str_t prefix, ngx_str_t suffix, u_char ** return NGX_OK; } - -// randbytes fills out with secure random bytes. -// Return value of NGX_OK indicates success. -// Return value of NGX_ERROR indicates error. -ngx_int_t randbytes(u_char *out, int num_bytes) { - if (out == NULL) { - return NGX_ERROR; - } - if (num_bytes < 1 || num_bytes > 64) { - // Values outside these bounds may indicate parameter usage mistake. - return NGX_ERROR; - } - - arc4random_buf(out, num_bytes); - - return NGX_OK; -} diff --git a/ipscrub/src/ngx_ipscrub_support.h b/ipscrub/src/ngx_ipscrub_support.h index 38ea3f0..e509f07 100644 --- a/ipscrub/src/ngx_ipscrub_support.h +++ b/ipscrub/src/ngx_ipscrub_support.h @@ -8,6 +8,5 @@ ngx_int_t null_terminate(ngx_pool_t *pool, ngx_str_t input, u_char **hashed); ngx_int_t concat(ngx_pool_t *pool, ngx_str_t prefix, ngx_str_t suffix, u_char **out); -ngx_int_t randbytes(u_char *out, int num_bytes); #endif /* _IPSCRUB_SUPPORT_H_INCLUDED_ */ diff --git a/script/check-up-to-date.sh b/script/check-up-to-date.sh deleted file mode 100755 index a15e09a..0000000 --- a/script/check-up-to-date.sh +++ /dev/null @@ -1,12 +0,0 @@ -#!/bin/bash - -echo "Checking..." -latest=`curl "https://api.github.com/repos/masonicboom/ipscrub/releases/latest" 2>/dev/null | grep --extended-regexp -o "\"tag_name\":\s+\".*\"" | cut -d '"' -f 4` -current=`git tag | sort | tail -n 1` - -if [ $current != $latest ]; then - echo "Current version is $current; latest is $latest. Check https://github.com/masonicboom/ipscrub/releases to see if you should update." - exit 1 -else - echo "You are on the latest version of ipscrub ($current)." -fi \ No newline at end of file