Skip to content

Commit 6ab9afc

Browse files
authored
Fix mx_mech handling (#26)
Fix mx_mech evaluations; currently it will only look up the first MX record address and compare that against the list of valid IPs due to the old async pending checks being left in place which cannot be used when async/await is used as they will always evaluate to a truthy value. Checklist: - [ ] docs updated - [X] tests updated - [ ] Changes.md updated
1 parent 1112fcc commit 6ab9afc

File tree

2 files changed

+47
-42
lines changed

2 files changed

+47
-42
lines changed

lib/spf.js

Lines changed: 38 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ class SPF {
3838

3939
this.mech_ip4 = this.mech_ip
4040
this.mech_ip6 = this.mech_ip
41+
42+
// Used for tests only
43+
this._found_mx_addrs = [];
4144
}
4245

4346
const_translate(value) {
@@ -486,17 +489,15 @@ class SPF {
486489
}
487490
}
488491

489-
let pending = 0
490492
let addresses = []
491493
// RFC 4408 Section 10.1
492494
if (mxes.length > this.LIMIT) return this.SPF_PERMERROR
493495

496+
let cidr;
494497
for (const element of mxes) {
495-
pending++
496498
const mx = element.exchange
497499
// Calculate which IP method to use
498500
let resolve_method
499-
let cidr
500501
if (this.ip_ver === 'ipv4') {
501502
cidr = cidr4
502503
resolve_method = 'resolve4'
@@ -519,49 +520,44 @@ class SPF {
519520
}
520521
}
521522

522-
pending--
523-
if (addrs) {
524-
this.log_debug(`mech_mx: mx=${mx} addresses=${addrs.join(',')}`)
525-
addresses = addrs.concat(addresses)
526-
}
527-
if (pending === 0) {
528-
if (!addresses.length) return this.SPF_NONE
529-
// All queries run; see if our IP matches
530-
if (cidr) {
531-
// CIDR match type
532-
for (const address of addresses) {
533-
const range = ipaddr.parse(address)
534-
if (this.ipaddr.match(range, cidr)) {
535-
this.log_debug(
536-
`mech_mx: ${this.ip} => ${address}/${cidr}: MATCH!`,
537-
)
538-
return this.return_const(qualifier)
539-
} else {
540-
this.log_debug(
541-
`mech_mx: ${this.ip} => ${address}/${cidr}: NO MATCH`,
542-
)
543-
}
544-
}
545-
// No matches
546-
return this.SPF_NONE
523+
this.log_debug(`mech_mx: mx=${mx} addresses=${addrs.join(',')}`)
524+
addresses = addrs.concat(addresses)
525+
}
526+
527+
if (!addresses.length) return this.SPF_NONE
528+
this._found_mx_addrs = addresses;
529+
530+
// All queries run; see if our IP matches
531+
if (cidr) {
532+
// CIDR match type
533+
for (const address of addresses) {
534+
const range = ipaddr.parse(address)
535+
if (this.ipaddr.match(range, cidr)) {
536+
this.log_debug(
537+
`mech_mx: ${this.ip} => ${address}/${cidr}: MATCH!`,
538+
)
539+
return this.return_const(qualifier)
547540
} else {
548-
if (addresses.includes(this.ip)) {
549-
this.log_debug(
550-
`mech_mx: ${this.ip} => ${addresses.join(',')}: MATCH!`,
551-
)
552-
return this.return_const(qualifier)
553-
} else {
554-
this.log_debug(
555-
`mech_mx: ${this.ip} => ${addresses.join(',')}: NO MATCH`,
556-
)
557-
return this.SPF_NONE
558-
}
541+
this.log_debug(
542+
`mech_mx: ${this.ip} => ${address}/${cidr}: NO MATCH`,
543+
)
559544
}
560545
}
561-
// In case we didn't run any queries...
562-
if (pending === 0) return this.SPF_NONE
546+
// No matches
547+
return this.SPF_NONE
548+
} else {
549+
if (addresses.includes(this.ip)) {
550+
this.log_debug(
551+
`mech_mx: ${this.ip} => ${addresses.join(',')}: MATCH!`,
552+
)
553+
return this.return_const(qualifier)
554+
} else {
555+
this.log_debug(
556+
`mech_mx: ${this.ip} => ${addresses.join(',')}: NO MATCH`,
557+
)
558+
return this.SPF_NONE
559+
}
563560
}
564-
if (pending === 0) this.SPF_NONE
565561
}
566562

567563
async mech_ptr(qualifier, args) {

test/spf.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,15 @@ describe('SPF', function () {
5353
}
5454
})
5555

56+
it('resolves more than one IP in mech_mx', async function () {
57+
this.timeout = 4000
58+
this.SPF.domain = 'gmail.com'
59+
this.SPF.ip_ver = 'ipv4'
60+
61+
await this.SPF.mech_mx()
62+
assert.equal((this.SPF._found_mx_addrs.length > 1), true)
63+
})
64+
5665
it('check_host, gmail.com, fail', async function () {
5766
this.timeout = 3000
5867
this.SPF.count = 0

0 commit comments

Comments
 (0)