Skip to content

Commit

Permalink
fix: fixed smtp-server logger socket issue (too many logs), reduced a…
Browse files Browse the repository at this point in the history
…dmin code bug alerts by removing MAIL FROM in hash, removed redundant pre-save hook since hash is already a unique index, fixed bounce attack check in bounce info parser
  • Loading branch information
titanism committed Feb 28, 2025
1 parent 6cb038f commit 9e53a6d
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 63 deletions.
91 changes: 38 additions & 53 deletions app/models/logs.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const config = require('#config');
const createTangerine = require('#helpers/create-tangerine');
const emailHelper = require('#helpers/email');
const isEmail = require('#helpers/is-email');
const isRetryableError = require('#helpers/is-retryable-error');
// const isErrorConstructorName = require('#helpers/is-error-constructor-name');
const logger = require('#helpers/logger');
const parseAddresses = require('#helpers/parse-addresses');
Expand Down Expand Up @@ -742,34 +743,18 @@ function getQueryHash(log) {
// if had a user
if (log?.user) set.add(log.user);

//
// if it is not connection closed message
// then don't add uniqueness from the SMTP envelope
//
if (log.message !== 'Connection is closed.') {
// make it unique by mail from
if (
isSANB(log?.meta?.session?.envelope?.mailFrom?.address) &&
isEmail(log.meta.session.envelope.mailFrom.address)
)
set.add(
`/@.*${parseRootDomain(
log.meta.session.envelope.mailFrom.address.split('@')[1]
)}$/`
);

// make it unique by rcpt to
if (
Array.isArray(log?.meta?.session?.envelope?.rcptTo) &&
log.meta.session.envelope.rcptTo.length > 0
) {
for (const rcpt of log.meta.session.envelope.rcptTo) {
if (_.isObject(rcpt) && isSANB(rcpt.address) && isEmail(rcpt.address)) {
set.add(`/@.*${parseRootDomain(rcpt.address.split('@')[1])}$/`);
}
}
}
}
/*
// make it unique by mail from
if (
isSANB(log?.meta?.session?.envelope?.mailFrom?.address) &&
isEmail(log.meta.session.envelope.mailFrom.address)
)
set.add(
`/@.*${parseRootDomain(
log.meta.session.envelope.mailFrom.address.split('@')[1]
)}$/`
);
*/

// TODO: filter if it was a code bug or not
// TODO: if err.responseCode and !err.bounces && !meta.session.resolvedClientHostname && meta.session.remoteAddress
Expand Down Expand Up @@ -813,31 +798,6 @@ Logs.pre('validate', function (next) {
next();
});

Logs.pre('save', async function (next) {
// only run this if the document was new
// or if it was run from the parse-logs job
// (which sets `skip_duplicate_check = true`)
if (
!this.isNew ||
this.skip_duplicate_check ||
this?.err?.code === 'SQLITE_CORRUPT'
)
return next();

try {
const exists = await this.constructor.exists({
hash: this.hash
});

if (exists) throw new Error('Ignored duplicate log');

next();
} catch (err) {
err.is_duplicate_log = true;
next(err);
}
});

//
// NOTE: there is a snapshot array of domains that correlated to this log at the created_at time
// and this is accomplished by both a pre('save') hook and also a job
Expand Down Expand Up @@ -987,6 +947,26 @@ Logs.pre('save', async function (next) {
Logs.pre('save', function (next) {
this.is_empty_domains =
!Array.isArray(this.domains) || this.domains.length === 0;

//
// NOTE: `smtp-server` instances of `SMTPServer` accept a `logger`
// and if a socket error occurs, it will have `err.remote` added
//
// ❯ rg "err.remote" -uuu
// node_modules/.pnpm/[email protected]/node_modules/smtp-server/lib/smtp-connection.js
// 388: err.remote = this.remoteAddress;
//
if (
typeof this?.err === 'object' &&
typeof this?.err?.remote === 'string' &&
isIP(this?.err?.remote) &&
isRetryableError(this?.err)
) {
const err = new Error('Suppressing SMTP server SMTP connection output');
err.is_duplicate_log = true;
return next(err);
}

next();
});

Expand All @@ -998,6 +978,7 @@ Logs.pre('save', function (next) {
this._isNew = this.isNew;
next();
});

Logs.post('save', async (doc, next) => {
if (!doc._isNew) return next();

Expand Down Expand Up @@ -1028,6 +1009,10 @@ Logs.post('save', async (doc, next) => {
*/

try {
//
// TODO: put this in queue instead
// otherwise it's too slow and can fail
//
// send an email to admins of the error
await emailHelper({
template: 'alert',
Expand Down
14 changes: 4 additions & 10 deletions helpers/get-bounce-info.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,10 @@ function getBounceInfo(err) {
response.includes('blocked')
) {
bounceInfo.category = 'blocklist';
} else if (response.includes('bounce attacks')) {
} else if (
response.includes('bounce attack') ||
response.includes('misdirected bounce')
) {
bounceInfo.category = response.includes(IP_ADDRESS) ? 'blocklist' : 'spam';
} else if (
err.truthSource === 'qq.com' &&
Expand Down Expand Up @@ -329,15 +332,6 @@ function getBounceInfo(err) {
else if (['virus', 'spam'].includes(bounceInfo.category))
bounceInfo.action = 'reject';

// bounce attack
// > 550 Suspected bounce attacks
// <https://service.mail.qq.com/detail/122/57>
if (
response.includes('bounce attack') ||
response.includes('misdirected bounce')
)
bounceInfo.category = 'spam';

// <https://github.com/zone-eu/zone-mta/issues/434>
if (response.startsWith('DMARC ') || response.includes(' DMARC ')) {
bounceInfo.category = 'dmarc';
Expand Down

0 comments on commit 9e53a6d

Please sign in to comment.