Skip to content

Commit

Permalink
fix: fixed SRS issues
Browse files Browse the repository at this point in the history
  • Loading branch information
titanism committed Jan 8, 2025
1 parent fb77a4f commit cc87db7
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 8 deletions.
21 changes: 20 additions & 1 deletion helpers/check-srs.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,26 @@ function checkSRS(address, shouldThrow = false, ignoreHook = false) {
if (!REGEX_SRS0.test(address) && !REGEX_SRS1.test(address)) return address;

try {
const reversed = srs.reverse(address);
//
// sometimes senders send to a lowercase version of the MAIL FROM
// which is going to mess things up here because case sensitivity is needed
// therefore we will do a rewrite here if necessary
//
const index = address.indexOf('@');
const local = address.slice(0, index).split('=');
const domain = address.slice(index + 1);

//
// > local.split('=')
// [ 'srs0', '4f4d', 't7', 'example.com', 'john' ]
// and we need
// SRS0=4f4d=T7=example.com=john
// therefore keys 0 and 2 need capitalized
//
if (local[0]) local[0] = local[0].toUpperCase(); // SRS0
if (local[2]) local[2] = local[2].toUpperCase(); // T7
const srsAddress = `${local.join('=')}@${domain}`;
const reversed = srs.reverse(srsAddress);
if (_.isNull(reversed)) throw new Error('Bad signature');
return reversed;
} catch (_err) {
Expand Down
4 changes: 2 additions & 2 deletions helpers/get-forwarding-addresses.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ async function getForwardingAddresses(
if (!isSANB(record) && !hasIMAP)
throw new SMTPError(
`${address} is not yet configured with its email service provider ${config.urls.web} ;`,
{ responseCode: 421, ignore_hook: true }
{ responseCode: 421, ignore_hook: true, notConfigured: true }
);

// e.g. user@example.com => [email protected]
Expand Down Expand Up @@ -557,7 +557,7 @@ async function getForwardingAddresses(
if (forwardingAddresses.length === 0 && !hasIMAP) {
throw new SMTPError(
`${address} is not yet configured with its email service provider ${config.urls.web} ;`,
{ responseCode: 421, ignore_hook: true }
{ responseCode: 421, ignore_hook: true, notConfigured: true }
);
}

Expand Down
16 changes: 15 additions & 1 deletion helpers/get-recipients.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,19 @@ async function getRecipients(session, scan) {
webhookKey
};
} catch (err) {
if (err.notConfigured && to.srs) {
return {
address: to.address,
addresses: [to.address],
port: 25,
hasIMAP: false,
aliasPublicKey: false,
vacationResponder: false,
// TODO: only do this if MX server of sender used our service
srs: true
};
}

logger.warn(err, { session });
err.responseCode = getErrorCode(err);
bounces.push({
Expand Down Expand Up @@ -609,7 +622,8 @@ async function getRecipients(session, scan) {
port: recipient.port,
recipient: recipient.address,
to: [address.to],
replacements
replacements,
...(recipient.srs ? { srs: true } : {})
});
}
}
Expand Down
3 changes: 0 additions & 3 deletions helpers/on-data-mx.js
Original file line number Diff line number Diff line change
Expand Up @@ -1690,9 +1690,6 @@ async function onDataMX(session, headers, body) {
}
}

// TODO: fix the SRS0 thing (not rewriting properly. e.g. RCD)
// TODO: if it was SRS rewritten then it shouldn't 421 it should deliver to the address still

//
// send vacation responders if necessary in series
//
Expand Down
2 changes: 2 additions & 0 deletions helpers/on-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ async function onData(stream, _session, fn) {
// however because srs0 was rewritten from SRS0
// and because tv was rewritten from TV it is not being delivered correctly
//
const original = to.address;
to.address = checkSRS(to.address, shouldThrow);
if (original !== to.address) to.srs = true;
return to;
}),
async (to) => {
Expand Down
11 changes: 10 additions & 1 deletion test/mx/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const pWaitFor = require('p-wait-for');
const pify = require('pify');
const test = require('ava');
const { SMTPServer } = require('smtp-server');
const { SRS } = require('sender-rewriting-scheme');

const utils = require('../utils');

Expand All @@ -40,6 +41,7 @@ import('@ava/get-port').then((obj) => {
const asyncMxConnect = pify(mxConnect);
const IP_ADDRESS = ip.address();
const tls = { rejectUnauthorized: false };
const srs = new SRS(config.srs);

test.before(utils.setupMongoose);
test.before(utils.setupRedisClient);
Expand Down Expand Up @@ -333,7 +335,14 @@ test('imap/forward/webhook', async (t) => {
transporter.sendMail({
envelope: {
from: '[email protected]',
to: `test@${domain.name}`
to: [
`test@${domain.name}`,
//
// NOTE: this is purposely formatted incorrectly
// to test when mail servers send to lowercase version of MAIL FROM
// (we have logic that accounts for this, see `#helpers/check-srs`)
srs.forward(`test@${domain.name}`, env.WEB_HOST).toLowerCase()
]
},
raw: `
To: test@${domain.name}
Expand Down

0 comments on commit cc87db7

Please sign in to comment.