Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: www/nginx: add njs script to detect MitM attacks on TLS connections #1070

Merged
merged 2 commits into from Dec 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
60 changes: 60 additions & 0 deletions www/nginx/src/opnsense/scripts/nginx/ngx_functions.js
@@ -0,0 +1,60 @@
var fs = require('fs');
var tls_fingerprints = JSON.parse(fs.readFileSync('/usr/local/etc/nginx/tls_fingerprints.json'));

function customArrayIndexOf(haystack, needle) {
var element, element_id;
for (element_id in haystack) {
element = haystack[element_id];
if (element == needle) return element_id;
}
return -1;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, this function is needed, Array.indexOf does not work for some reasons (don't know why)…

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, just for reference: which browser in particular?

Copy link
Member Author

@fabianfrz fabianfrz Dec 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not a browser, this runs directly inside nginx (njs). It is strange that when I use the njs comand line util it works as expected but in nginx it is strange:

var a = 'x';
var b = 'x';
a == b // true
a === b // false but should be true - indexOf probably uses this comparison

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem is fixed by this commit: nginx/njs@f0dc710
Could you build njs from master branch and test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VBart I just tried indexOf on version nginx/njs@549e51e which I have built yesterday. It still returns -1

nginx version: Nginx-1.14.0 (if it helps)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, since the problem is related to comparsion of UTF-8 and Byte strings internal representation, I think you can try to workaround it by using toBytes() or toUTF8() methods. Not sure which one will work in this particular case:

current_index = fingerprint_ciphers.indexOf(browser_cipher.toUTF8());

or

current_index = fingerprint_ciphers.indexOf(browser_cipher.toBytes());

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not so urgent as we are blocked here anyway (FreeBSD build issue):
opnsense/tools@634a086

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabianfrz jfyi, njs 0.2.7 has been released today with the fix.

As of FreeBSD build issue, that's strange, because njs works perfectly on FreeBSD. And njs module is a part of official nginx port for a long time: https://www.freshports.org/www/nginx

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VBart thanks, so let's wait for inclusion in the FreeBSD ports tree so we can use it. @fichtner fixed the build by patching out a build flag:

opnsense/ports@d188643#diff-60d367a88ba4945c90d6504cc2f25228

It seems that an unsupported compiler flag caused the issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On FreeBSD only nginx-full has the NJS option enabled normally. It could be due to our unsupported version of FreeBSD 11.1. I will re-evaluate once we are on 11.2.


function check_cipher_array(r, browser_ciphers, fingerprint_ciphers, result) {
if (result.status == 'Intercepted') {
return;
}
if (browser_ciphers.length > fingerprint_ciphers.length) {
// the proxy supports more cipers than the browser -> intercepted
result.status = "Intercepted";
return;
}
var browser_cipher;
var browser_cipher_index;
var last_index = -1;
var current_index;
for (browser_cipher_index in browser_ciphers) {
browser_cipher = browser_ciphers[browser_cipher_index];
current_index = customArrayIndexOf(fingerprint_ciphers, browser_cipher);
if (current_index === -1 || current_index <= last_index) {
// a cipher has been found, which is not supported by the browser
// such a connection is definitly intercepted
result.status = "Intercepted";
//result.status = JSON.stringify(fingerprint_ciphers[0].toBytes().toString('hex'));
return;
}
last_index = current_index;
}
if (result.status == 'Unknown') {
result.status = browser_ciphers.length === fingerprint_ciphers.length ? 'Original' : 'Hardened'
}
}

function check_intercept(r) {
var tls_result = {'status': 'Unknown'};
if (r.headersIn['User-Agent'] && r.variables.ssl_ciphers != '') {
var ua = r.headersIn['User-Agent'];
if (ua in tls_fingerprints) {
var fp = tls_fingerprints[ua];
var browser_ciphers = r.variables.ssl_ciphers.split(':');
check_cipher_array(r, browser_ciphers, fp.ciphers, tls_result);
if (r.variables.ssl_curves != '')
{
var browser_curves = r.variables.ssl_curves.split(':');
check_cipher_array(r, browser_curves, fp.curves, tls_result);
}
}
}
return tls_result.status;
}

Expand Up @@ -11,6 +11,9 @@ log_format anonymized ':: - $remote_user [$time_local] "$request" '
'"$http_user_agent" "$http_x_forwarded_for"';

#tcp_nopush on;
# https intercept detection
js_include /usr/local/opnsense/scripts/nginx/ngx_functions.js;
js_set $tls_intercepted check_intercept;

# 200M should be big enough for file servers etc.
client_max_body_size 200M;
Expand Down
Expand Up @@ -84,6 +84,7 @@ location {{ location.matchtype }} {{ location.urlpattern }} {
fastcgi_param TLS-Cipher $ssl_cipher;
fastcgi_param TLS-Protocol $ssl_protocol;
fastcgi_param TLS-SNI-Host $ssl_server_name;
fastcgi_param TLS-Client-Intercepted $tls_intercepted;
fastcgi_intercept_errors off;
{% if location.upstream is not defined %}
fastcgi_pass unix:/var/run/php-www.socket;
Expand Down Expand Up @@ -135,6 +136,7 @@ location {{ location.matchtype }} {{ location.urlpattern }} {
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;
proxy_set_header X-TLS-Client-Intercepted $tls_intercepted;
proxy_ignore_client_abort {% if location.proxy_ignore_client_abort == '1' %}on{% else %}off{% endif %};
proxy_request_buffering {% if location.proxy_request_buffering == '1' %}on{% else %}off{% endif %};
proxy_buffering {% if location.proxy_buffering == '1' %}on{% else %}off{% endif %};
Expand Down
Expand Up @@ -4,6 +4,7 @@ load_module /usr/local/libexec/nginx/ngx_http_naxsi_module.so;
load_module /usr/local/libexec/nginx/ngx_mail_module.so;
load_module /usr/local/libexec/nginx/ngx_http_brotli_filter_module.so;
load_module /usr/local/libexec/nginx/ngx_http_brotli_static_module.so;
load_module /usr/local/libexec/nginx/ngx_http_js_module.so;

user www staff;
worker_processes 1;
Expand Down