postalsys / mailauth

Command line utility and a Node.js library for email authentication
Other
126 stars 10 forks source link

mailauth returning random EINVALIDKEY DKIM result for Twitter #30

Closed titanism closed 1 year ago

titanism commented 1 year ago

Twitter has issue with mailauth in that DKIM results have random "invalid public key" results.

Filing this here and will follow-up if can get more details.

titanism commented 1 year ago

It appears that this is due to custom resolver caching (the parsed value cannot be JSON.stringified to begin with).

titanism commented 1 year ago

Specifically the issue happens here:

    if (rr) {
        // prefix value for parsing as there is no default value
        let entry = parseDkimHeaders(`DNS: TXT;${rr}`);

        const publicKeyValue = entry?.parsed?.p?.value;
        if (!publicKeyValue) { // <-- this is false so it throws this error
            let err = new Error('Missing key value');
            err.code = 'EINVALIDVAL';
            err.rr = rr;
            throw err;
        }

The value of rr is: "v=DKIM1;p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwe34ubzrMzM9sT0XVkcc3UXd7W+EHCyHoqn70l2AxXox52lAZzH/UnKwAoO+5qsuP7T9QOifIJ9ddNH9lEQ95Y/GdHBsPLGdgSJIs95mXNxscD6MSyejpenMGL9TPQAcxfqY5xPViZ+1wA1qcr""yjdZKRqf1f4fpMY+x3b8k7H5Qyf/Smz0sv4xFsx1r+THNIz0rzk2LO3GvE0f1ybp6P+5eAelYU4mGeZQqsKw/eB20I3jHWEyGrXuvzB67nt6ddI+N2eD5K38wg/aSytOsb5O+bUSEe7P0zx9ebRRVknCD6uuqG3gSmQmttlD5OrMWSXzrPIXe8eTBaaPd+e/jfxwIDAQAB"

The value of entry is an Object:

entry = {
  parsed: {
    header: 'dns',
    value: 'txt',
    '"v=dkim1;p=miibijanbgkqhkig9w0baqefaaocaq8amiibcgkcaqeawe34ubzrmzm9st0xvkcc3uxd7w+ehcyhoqn70l2axxox52lazzh/unkwaoo+5qsup7t9qoifij9ddnh9leq95y/gdhbsplgdgsjis95mxnxscd6msyejpenmgl9tpqacxfqy5xpviz+1wa1qcr""yjdzkrqf1f4fpmy+x3b8k7h5qyf/smz0sv4xfsx1r+thniz0rzk2lo3gve0f1ybp6p+5eaelyu4mgezqqskw/eb20i3jhweygrxuvzb67nt6ddi+n2ed5k38wg/asytosb5o+busee7p0zx9ebrrvkncd6uuqg3gsmqmttld5ormwsxzrpixe8etbaapd+e/jfxwidaqab"': { value: '' }
  },
  original: 'DNS: TXT;"v=DKIM1;p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwe34ubzrMzM9sT0XVkcc3UXd7W+EHCyHoqn70l2AxXox52lAZzH/UnKwAoO+5qsuP7T9QOifIJ9ddNH9lEQ95Y/GdHBsPLGdgSJIs95mXNxscD6MSyejpenMGL9TPQAcxfqY5xPViZ+1wA1qcr""yjdZKRqf1f4fpMY+x3b8k7H5Qyf/Smz0sv4xFsx1r+THNIz0rzk2LO3GvE0f1ybp6P+5eAelYU4mGeZQqsKw/eB20I3jHWEyGrXuvzB67nt6ddI+N2eD5K38wg/aSytOsb5O+bUSEe7P0zx9ebRRVknCD6uuqG3gSmQmttlD5OrMWSXzrPIXe8eTBaaPd+e/jfxwIDAQAB"'
}
titanism commented 1 year ago

The issue is that rr is inaccurate.

    let list = await resolver(name, 'TXT');
    let rr =
        list &&
        []
            .concat(list[0] || [])
            .join('')
            .replace(/\s+/g, '');

The list variable is an Array such as:

list [
  [
    '"v=DKIM1; p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwe34ubzrMzM9sT0XVkcc3UXd7W+EHCyHoqn70l2AxXox52lAZzH/UnKwAoO+5qsuP7T9QOifIJ9ddNH9lEQ95Y/GdHBsPLGdgSJIs95mXNxscD6MSyejpenMGL9TPQAcxfqY5xPViZ+1wA1qcr" "yjdZKRqf1f4fpMY+x3b8k7H5Qyf/Smz0sv4xFsx1r+THNIz0rz',
    'k2LO3GvE0f1ybp6P+5eAelYU4mGeZQqsKw/eB20I3jHWEyGrXuvzB67nt6ddI+N2eD5K38wg/aSytOsb5O+bUSEe7P0zx9ebRRVknCD6uuqG3gSmQmttlD5OrMWSXzrPIXe8eTBaaPd+e/jfxwIDAQAB"'
  ]
]

As you can see, it's not properly stringifying this to begin with in the cache (which is not necessarily an issue with mailauth, but one that mailauth could detect perhaps; e.g. dummy-proofing).

titanism commented 1 year ago

The dummy-proofing fix could be to add .replace(/"/g, '').

titanism commented 1 year ago

Bad:

> [].concat(list[0]).join('').replace(/\s+/g, '')
'"v=DKIM1;p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwe34ubzrMzM9sT0XVkcc3UXd7W+EHCyHoqn70l2AxXox52lAZzH/UnKwAoO+5qsuP7T9QOifIJ9ddNH9lEQ95Y/GdHBsPLGdgSJIs95mXNxscD6MSyejpenMGL9TPQAcxfqY5xPViZ+1wA1qcr""yjdZKRqf1f4fpMY+x3b8k7H5Qyf/Smz0sv4xFsx1r+THNIz0rzk2LO3GvE0f1ybp6P+5eAelYU4mGeZQqsKw/eB20I3jHWEyGrXuvzB67nt6ddI+N2eD5K38wg/aSytOsb5O+bUSEe7P0zx9ebRRVknCD6uuqG3gSmQmttlD5OrMWSXzrPIXe8eTBaaPd+e/jfxwIDAQAB"'

Good:

> [].concat(list[0]).join('').replace(/\s+/g, '').replace(/"/g, '')
'v=DKIM1;p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwe34ubzrMzM9sT0XVkcc3UXd7W+EHCyHoqn70l2AxXox52lAZzH/UnKwAoO+5qsuP7T9QOifIJ9ddNH9lEQ95Y/GdHBsPLGdgSJIs95mXNxscD6MSyejpenMGL9TPQAcxfqY5xPViZ+1wA1qcryjdZKRqf1f4fpMY+x3b8k7H5Qyf/Smz0sv4xFsx1r+THNIz0rzk2LO3GvE0f1ybp6P+5eAelYU4mGeZQqsKw/eB20I3jHWEyGrXuvzB67nt6ddI+N2eD5K38wg/aSytOsb5O+bUSEe7P0zx9ebRRVknCD6uuqG3gSmQmttlD5OrMWSXzrPIXe8eTBaaPd+e/jfxwIDAQAB'
titanism commented 1 year ago

I've confirmed that the native dns.promises.resolve is going to have this issue.

> TANGERINE 89622: cache retrieved txt:dkim-201406._domainkey.twitter.com
TANGERINE 89622: cached result found for "txt:dkim-201406._domainkey.twitter.com"
[
  [
    '"v=DKIM1; p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwe34ubzrMzM9sT0XVkcc3UXd7W+EHCyHoqn70l2AxXox52lAZzH/UnKwAoO+5qsuP7T9QOifIJ9ddNH9lEQ95Y/GdHBsPLGdgSJIs95mXNxscD6MSyejpenMGL9TPQAcxfqY5xPViZ+1wA1qcr" "yjdZKRqf1f4fpMY+x3b8k7H5Qyf/Smz0sv4xFsx1r+THNIz0rz',
    'k2LO3GvE0f1ybp6P+5eAelYU4mGeZQqsKw/eB20I3jHWEyGrXuvzB67nt6ddI+N2eD5K38wg/aSytOsb5O+bUSEe7P0zx9ebRRVknCD6uuqG3gSmQmttlD5OrMWSXzrPIXe8eTBaaPd+e/jfxwIDAQAB"'
  ]
]
> dns.promises.resolve('dkim-201406._domainkey.twitter.com', 'TXT').then(console.log)
Promise {
  <pending>,
  [Symbol(async_id_symbol)]: 2201,
  [Symbol(trigger_async_id_symbol)]: 2199
}
> [
  [
    '"v=DKIM1; p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwe34ubzrMzM9sT0XVkcc3UXd7W+EHCyHoqn70l2AxXox52lAZzH/UnKwAoO+5qsuP7T9QOifIJ9ddNH9lEQ95Y/GdHBsPLGdgSJIs95mXNxscD6MSyejpenMGL9TPQAcxfqY5xPViZ+1wA1qcr" "yjdZKRqf1f4fpMY+x3b8k7H5Qyf/Smz0sv4xFsx1r+THNIz0rz',
    'k2LO3GvE0f1ybp6P+5eAelYU4mGeZQqsKw/eB20I3jHWEyGrXuvzB67nt6ddI+N2eD5K38wg/aSytOsb5O+bUSEe7P0zx9ebRRVknCD6uuqG3gSmQmttlD5OrMWSXzrPIXe8eTBaaPd+e/jfxwIDAQAB"'
  ]
]
titanism commented 1 year ago

The issue is because Twitter's DNS wraps with quotes:

❯ dig dkim-201406._domainkey.twitter.com txt
dkim-201406._domainkey.twitter.com. 243 IN TXT  "\"v=DKIM1; p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwe34ubzrMzM9sT0XVkcc3UXd7W+EHCyHoqn70l2AxXox52lAZzH/UnKwAoO+5qsuP7T9QOifIJ9ddNH9lEQ95Y/GdHBsPLGdgSJIs95mXNxscD6MSyejpenMGL9TPQAcxfqY5xPViZ+1wA1qcr\" \"yjdZKRqf1f4fpMY+x3b8k7H5Qyf/Smz0sv4xFsx1r+THNIz0rz" "k2LO3GvE0f1ybp6P+5eAelYU4mGeZQqsKw/eB20I3jHWEyGrXuvzB67nt6ddI+N2eD5K38wg/aSytOsb5O+bUSEe7P0zx9ebRRVknCD6uuqG3gSmQmttlD5OrMWSXzrPIXe8eTBaaPd+e/jfxwIDAQAB\""
andris9 commented 1 year ago

I’m on the phone rn so can’t check but are you sure that your custom resolver is returning properly formatted values for TXT records? This quoted structure looks like it is not properly processed. If you use the native resolver, does it return the same value with quotes as well or does it remove quotes?

andris9 commented 1 year ago

Sorry, typed too slow, saw that you already answered

titanism commented 1 year ago

Apologies for the spam comments, I'm exhausted but still trying to get a PR in for you here.

@andris9 I will submit a PR shortly that fixes this

andris9 commented 1 year ago

Is such a record even valid? Can’t check while oh phone. Do other services register it as a dkim=pass for the same key 🤔

andris9 commented 1 year ago

It is definitely not valid. This is a common double quoting mistake. https://mxtoolbox.com/SuperTool.aspx?action=dkim%3atwitter.com%3adkim-201406&run=toolpage

titanism commented 1 year ago

The DNS is randomly returning quoted and non-quoted values. Seems like issue with their DNS. I'll try to file an issue with them.

andris9 commented 1 year ago

Yeah, I see the same, sometimes it returns normal results, sometimes double quoted. Mailauth is working correctly, it’s Twitter that is in the wrong. Super weird.

andris9 commented 1 year ago

Seems like if the DNS record is returned by *.r06.twtrdns.net then it is correct. If by *.u06.twtrdns.net then it is invalid.

titanism commented 1 year ago

@andris9 do you think we should build in this dummy-proofing still?

titanism commented 1 year ago

@andris9 see https://github.com/postalsys/mailauth/pull/31 if so. I submitted a report on Hacker One already for Twitter.

Also, note that there are other cases in the mailauth codebase that perform TXT lookup that don't have this dummy-proofing that we may want to add too (not sure? your discretion).

andris9 commented 1 year ago

Double quoting is invalid, so there is no need to allow it. In this case it was good it failed and did not pass the quoted value as one of mailauth jobs is to validate the syntax of those records.

titanism commented 1 year ago

👍 Sounds good! Thank you for the assist as always! Wishing you a good evening!

andris9 commented 1 year ago

It’s already morning for me, just about to go to the office 😎