Use @ signs consistently for GitHub users

Use secure random number generator, and 128bits of randomness

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
+RUN apt-get update && apt-get install -y curl gcc make git libpcre3-dev zlib1g-dev libbsd-dev
 
 ADD ipscrub /ipscrub/
 ADD Makefile /

Makefile

@@ -61,3 +61,6 @@ 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
+	$<

README.md

@@ -9,17 +9,18 @@
 * [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` as `HASH(ngx_random() ++ timestamp)`.
+1. On initialization, and again every `PERIOD`, generate `salt` using 128bits from `arc4random_buf()`.
 2. On each request, generate masked IP address as `HASH(salt ++ IP address)`.
 3. Log masked IP address.
 
-`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.
+`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). 
 
 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.
 
@@ -34,8 +35,18 @@ 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).
@@ -43,6 +54,23 @@ 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.

ipscrub/config

@@ -4,7 +4,13 @@ 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"
-ngx_module_libs=SHA1
+
+# On Linux, link with libbsd, to get arc4random.
+if [ `uname` = Linux ]; then
+    ngx_module_libs="SHA1 -lbsd"
+else
+    ngx_module_libs="SHA1"
+fi
 
 . auto/module
 

ipscrub/src/ngx_ipscrub_module.c

@@ -42,7 +42,8 @@ static ngx_command_t  ngx_ipscrub_commands[] = {
 // Globals.
 const int default_period_seconds = 10 * 60; // Period between salt changes.
 time_t period_start = -1;
-long nonce = -1; // Input to salt generation.
+#define num_nonce_bytes 16
+u_char nonce[num_nonce_bytes]; // Input to salt generation.
 
 
 static ngx_http_module_t  ngx_ipscrub_module_ctx = {
@@ -146,13 +147,17 @@ 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) {
-    nonce = ngx_random();
+    rc = randbytes((u_char *) &nonce, num_nonce_bytes);
+    if (rc != NGX_OK) {
+        return NGX_HTTP_INTERNAL_SERVER_ERROR;
+    }
+
     // TODO: actually calculate when period_start should have been.
     period_start = now;
   }
 
   salt.data = (u_char *) &nonce;
-  salt.len = sizeof(nonce);
+  salt.len = num_nonce_bytes;
 
   // 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);

ipscrub/src/ngx_ipscrub_support.c

@@ -3,6 +3,15 @@
 
 #include "ngx_ipscrub_support.h"
 
+#if (NGX_FREEBSD || NGX_SOLARIS || NGX_DARWIN)
+// arc4random is built-in on these platforms.
+#elif (NGX_LINUX)
+#include <bsd/stdlib.h>
+#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)
 {
@@ -36,3 +45,20 @@ 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;
+}

ipscrub/src/ngx_ipscrub_support.h

@@ -8,5 +8,6 @@
 
 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_ */

script/check-up-to-date.sh

@@ -0,0 +1,12 @@
+#!/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