Skip to content

Check host against private IP range before sending mail #151

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.

Sign up for GitHub

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

Closed
wants to merge 8 commits into from
Closed

Check host against private IP range before sending mail #151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0063401
Anchor regexp to prevent it from misfiring
smfreegard Jan 19, 2012
e7a5648
Only run init_master in the master
smfreegard Jan 19, 2012
d93fc49
Add missing drain event to SpamAssassin plugin
smfreegard Jan 19, 2012
48fb2b0
Update smtp_forward and clamd plugins to use non-recursive send_data …
smfreegard Jan 19, 2012
f4f8d8d
Remove old drain event handler...
smfreegard Jan 19, 2012
aa57955
added check to prevent outbound mail to private ip ranges
sstur Jan 23, 2012
6172a58
updated outbound to emit when outgoing socket is connected
sstur Jan 23, 2012
6a7b6ed
added try/catch around socket.write for outgoing mail
sstur Jan 23, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion net_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ var config = require('./config');
var isIPv4 = require('net').isIPv4;

// Regexp to match private IPv4 ranges
var re_private_ipv4 = /(?:10|127|169\.254|172\.(?:1[6-9]|2[0-9]|3[01])|192\.168)\..*/;
var re_private_ipv4 = /^(?:10|127|169\.254|172\.(?:1[6-9]|2[0-9]|3[01])|192\.168)\..*/;

var top_level_tlds = {};
config.get('top-level-tlds','list').forEach(function (tld) {
Expand Down
12 changes: 11 additions & 1 deletion outbound.js
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ var constants = require('./constants');
var trans = require('./transaction');
var plugins = require('./plugins');
var date_to_str = require('./utils').date_to_str;
var is_rfc1918 = require('./net_utils').is_rfc1918;
var Address = require('./address').Address;

var delivery_concurrency = 0;
Expand Down Expand Up @@ -667,6 +668,9 @@ HMailItem.prototype.try_deliver = function () {
delivery_concurrency--;
return self.try_deliver(); // try next MX
}
addresses = addresses.filter(function(host) {
return !is_rfc1918(host);
Copy link
Collaborator

Choose a reason for hiding this comment

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

So a bug here is that this check should only be done for addresses not returned by a get_mx hook. This allows get_mx to be used for testing, or for delivering to a local outbound relay host.

Copy link
Author

Choose a reason for hiding this comment

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

Good call. I'll refactor accordingly..

});
if (addresses.length === 0) {
// NODATA or empty host list
self.logerror("DNS lookup of " + host + " resulted in no data");
Expand Down Expand Up @@ -725,7 +729,12 @@ HMailItem.prototype.try_deliver_host = function () {
line = '.';
}
self.logprotocol("C: " + line);
this.write(line + "\r\n");
try {
this.write(line + "\r\n");
} catch(e) {
//socket might have gone away in rare case
this.emit('error', new Error('Socket write failed'));
}
command = cmd.toLowerCase();
response = [];
};
Expand Down Expand Up @@ -769,6 +778,7 @@ HMailItem.prototype.try_deliver_host = function () {
});

socket.on('connect', function () {
self.emit('connect', socket);
});

socket.on('line', function (line) {
Expand Down
21 changes: 13 additions & 8 deletions plugins/clamd.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,26 +74,31 @@ exports.hook_data_post = function (next, connection) {
}

var data_marker = 0;
var in_data = false;
var close_pending = true;

var send_data = function () {
if (data_marker < transaction.data_lines.length) {
in_data = true;
var wrote_all = true;
while (wrote_all && (data_marker < transaction.data_lines.length)) {
var line = transaction.data_lines[data_marker];
var len = pack_len(Buffer.byteLength(line));
var wrote_all = socket.write(len, function() {
wrote_all = socket.write(len, function() {
socket.write(line);
data_marker++;
});
if (wrote_all) {
send_data(); // TODO: Fix to be non-recursive like queue/proxy
}
if (!wrote_all) return;
}
else {
if (close_pending) {
close_pending = false;
socket.end(pack_len(0));
}
};

socket.on('drain', function () {
connection.logdebug(plugin, 'drain');
send_data();
if (close_pending && in_data) {
process.nextTick(function () { send_data() });
}
});
socket.on('timeout', function () {
connection.logerror(plugin, "connection timed out");
Expand Down
38 changes: 18 additions & 20 deletions plugins/queue/smtp_forward.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,35 @@ exports.smtp_forward = function (next, connection) {
var recipients = connection.transaction.rcpt_to.map(function(item) { return item });
var data_marker = 0;
var dot_pending = true;
var got_data_response = false;

var send_data = function () {
if (data_marker < connection.transaction.data_lines.length) {
var wrote_all = socket.write(connection.transaction.data_lines[data_marker].replace(/^\./, '..').replace(/\r?\n/g, '\r\n'));
var wrote_all = true;
while (wrote_all && (data_marker < connection.transaction.data_lines.length)) {
var line = connection.transaction.data_lines[data_marker];
data_marker++;
if (wrote_all) {
send_data();
}
}
else if (dot_pending) {
wrote_all = socket.write(line.replace(/^\./, '..').replace(/\r?\n/g, '\r\n'));
if (!wrote_all) return;
}
// we get here if wrote_all still true, and we got to end of data_lines
if (dot_pending) {
dot_pending = false;
socket.send_command('dot');
}
}

};

socket.on('drain', function () {
connection.logdebug(self, 'drain');
if (dot_pending && command === 'databody') {
process.nextTick(function () { send_data() });
}
});

socket.send_command = function (cmd, data) {
var line = cmd + (data ? (' ' + data) : '');
if (cmd === 'dot') {
line = '.';
}
connection.logprotocol(self, "C: " + line);
// Set this before we write() in case 'drain' is called
// to stop send_data() form calling 'dot' twice.
command = cmd.toLowerCase();
this.write(line + "\r\n");
// Clear response buffer from previous command
Expand Down Expand Up @@ -148,7 +153,7 @@ exports.smtp_forward = function (next, connection) {
socket.send_command('DATA');
break;
case 'data':
got_data_response = true;
command = 'databody';
send_data();
break;
case 'dot':
Expand All @@ -172,11 +177,4 @@ exports.smtp_forward = function (next, connection) {
return next();
}
});
socket.on('drain', function() {
connection.logdebug(self, "Drained");
if (got_data_response && dot_pending && command === 'data') {
process.nextTick(function () { send_data() });
}
});
};

33 changes: 24 additions & 9 deletions plugins/spamassassin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,23 +63,38 @@ exports.hook_data_post = function (next, connection) {

socket.setTimeout(300 * 1000);

var username = config.main.spamd_user || process.getuid();
var username = config.main.spamd_user ||
connection.transaction.notes.spamd_user ||
process.getuid();

var data_marker = 0;
var in_data = false;
var end_pending = true;

var send_data = function () {
if (data_marker < connection.transaction.data_lines.length) {
var wrote_all = socket.write(connection.transaction.data_lines[data_marker]);
in_data = true;
var wrote_all = true;
while (wrote_all && (data_marker < connection.transaction.data_lines.length)) {
var line = connection.transaction.data_lines[data_marker];
data_marker++;
if (wrote_all) {
send_data();
}
// dot-stuffing not necessary for spamd
wrote_all = socket.write(line);
if (!wrote_all) return;
}
else {
// we get here if wrote_all still true, and we got to end of data_lines
if (end_pending) {
end_pending = false;
socket.end("\r\n");
}
};

socket.on('drain', function () {
connection.logdebug(plugin, 'drain');
if (end_pending && in_data) {
process.nextTick(function () { send_data() });
}
});

socket.on('timeout', function () {
connection.logerror(plugin, "spamd connection timed out");
socket.end();
Expand Down Expand Up @@ -129,13 +144,13 @@ exports.hook_data_post = function (next, connection) {
}
else if (state === 'tests') {
spamd_response.tests = line;
socket.destroy();
socket.end();
}
});

socket.on('end', function () {
// Now we do stuff with the results...

plugin.fixup_old_headers(config.main.old_headers_action, connection.transaction);

if (spamd_response.flag === 'Yes') {
Expand Down
6 changes: 3 additions & 3 deletions server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,14 @@ Server.createServer = function (params) {
c.use(cluster[module]());
}
}

c.set('host', config_data.main.listen_host);
c.listen(parseInt(config_data.main.port));
c.on('listening', listening);
Server.cluster = c;
c.on('start', function () {
if (c.isMaster) {
plugins.run_hooks('init_master', Server);
});
}
if (c.isWorker) {
plugins.run_hooks('init_child', Server);
}
Expand Down
Empty file modified utils.js
100644 → 100755
View file
Open in desktop
Empty file.