Skip to content

Commit

Permalink
Fix mx_mech handling (haraka#26)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
smfreegard authored Aug 14, 2024
1 parent 1112fcc commit 6ab9afc
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 42 deletions.
80 changes: 38 additions & 42 deletions lib/spf.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ class SPF {

this.mech_ip4 = this.mech_ip
this.mech_ip6 = this.mech_ip

// Used for tests only
this._found_mx_addrs = [];
}

const_translate(value) {
Expand Down Expand Up @@ -486,17 +489,15 @@ class SPF {
}
}

let pending = 0
let addresses = []
// RFC 4408 Section 10.1
if (mxes.length > this.LIMIT) return this.SPF_PERMERROR

let cidr;
for (const element of mxes) {
pending++
const mx = element.exchange
// Calculate which IP method to use
let resolve_method
let cidr
if (this.ip_ver === 'ipv4') {
cidr = cidr4
resolve_method = 'resolve4'
Expand All @@ -519,49 +520,44 @@ class SPF {
}
}

pending--
if (addrs) {
this.log_debug(`mech_mx: mx=${mx} addresses=${addrs.join(',')}`)
addresses = addrs.concat(addresses)
}
if (pending === 0) {
if (!addresses.length) return this.SPF_NONE
// All queries run; see if our IP matches
if (cidr) {
// CIDR match type
for (const address of addresses) {
const range = ipaddr.parse(address)
if (this.ipaddr.match(range, cidr)) {
this.log_debug(
`mech_mx: ${this.ip} => ${address}/${cidr}: MATCH!`,
)
return this.return_const(qualifier)
} else {
this.log_debug(
`mech_mx: ${this.ip} => ${address}/${cidr}: NO MATCH`,
)
}
}
// No matches
return this.SPF_NONE
this.log_debug(`mech_mx: mx=${mx} addresses=${addrs.join(',')}`)
addresses = addrs.concat(addresses)
}

if (!addresses.length) return this.SPF_NONE
this._found_mx_addrs = addresses;

// All queries run; see if our IP matches
if (cidr) {
// CIDR match type
for (const address of addresses) {
const range = ipaddr.parse(address)
if (this.ipaddr.match(range, cidr)) {
this.log_debug(
`mech_mx: ${this.ip} => ${address}/${cidr}: MATCH!`,
)
return this.return_const(qualifier)
} else {
if (addresses.includes(this.ip)) {
this.log_debug(
`mech_mx: ${this.ip} => ${addresses.join(',')}: MATCH!`,
)
return this.return_const(qualifier)
} else {
this.log_debug(
`mech_mx: ${this.ip} => ${addresses.join(',')}: NO MATCH`,
)
return this.SPF_NONE
}
this.log_debug(
`mech_mx: ${this.ip} => ${address}/${cidr}: NO MATCH`,
)
}
}
// In case we didn't run any queries...
if (pending === 0) return this.SPF_NONE
// No matches
return this.SPF_NONE
} else {
if (addresses.includes(this.ip)) {
this.log_debug(
`mech_mx: ${this.ip} => ${addresses.join(',')}: MATCH!`,
)
return this.return_const(qualifier)
} else {
this.log_debug(
`mech_mx: ${this.ip} => ${addresses.join(',')}: NO MATCH`,
)
return this.SPF_NONE
}
}
if (pending === 0) this.SPF_NONE
}

async mech_ptr(qualifier, args) {
Expand Down
9 changes: 9 additions & 0 deletions test/spf.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ describe('SPF', function () {
}
})

it('resolves more than one IP in mech_mx', async function () {
this.timeout = 4000
this.SPF.domain = 'gmail.com'
this.SPF.ip_ver = 'ipv4'

await this.SPF.mech_mx()
assert.equal((this.SPF._found_mx_addrs.length > 1), true)
})

it('check_host, gmail.com, fail', async function () {
this.timeout = 3000
this.SPF.count = 0
Expand Down

0 comments on commit 6ab9afc

Please sign in to comment.