Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing a bug with public suffix list caching #72

Merged
merged 4 commits into from
Mar 20, 2018

Conversation

jsf9k
Copy link
Member

@jsf9k jsf9k commented Mar 17, 2018

Starting with no public_suffix_list.dat cache file and without this fix (using the current develop branch):

$ trustymail --debug --json --smtp-ports=25 --dns=8.8.8.8 dhs.gov
2018-03-16 18:07:09,133 [gov]
2018-03-16 18:07:09,269 [DMARC] In dmarc_scan at /home/jeremy_frasier/dhs-ncats/trustymail/trustymail/trustymail.py:522: None of DNS query names exist: _dmarc.gov., _dmarc.gov.
<snip>
2018-03-16 18:07:14,087 [DMARC] In handle_syntax_error at /home/jeremy_frasier/dhs-ncats/trustymail/trustymail/trustymail.py:656: hq.dhs.gov does not indicate that it accepts DMARC reports about dhs.gov - https://tools.ietf.org/html/rfc7489#section-7.1
[
  {
    "Domain": "dhs.gov",
    "Base Domain": "gov",
<snip>
    "Valid DMARC": false,
    "DMARC Results": "v=DMARC1; p=none; pct=100; rua=mailto:DMARC@hq.dhs.gov, mailto:reports@dmarc.cyber.dhs.gov",
    "DMARC Record on Base Domain": false,
    "Valid DMARC Record on Base Domain": false,
    "DMARC Results on Base Domain": null,
<snip>
    "Syntax Errors": "[DMARC] In handle_syntax_error at /home/jeremy_frasier/dhs-ncats/trustymail/trustymail/trustymail.py:656: hq.dhs.gov does not indicate that it accepts DMARC reports about dhs.gov - https://tools.ietf.org/html/rfc7489#section-7.1",
    "Debug Info": null
  }
]

Starting with no public_suffix_list.dat cache file and with this fix (using the current bugfix/problem_with_psl_caching branch):

$ trustymail --debug --json --smtp-ports=25 --dns=8.8.8.8 dhs.gov
2018-03-17 07:36:37,046 [dhs.gov]
<snip>
[
  {
    "Domain": "dhs.gov",
    "Base Domain": "dhs.gov",
<snip>
    "Valid DMARC": true,
    "DMARC Results": "v=DMARC1; p=none; pct=100; rua=mailto:DMARC@hq.dhs.gov, mailto:reports@dmarc.cyber.dhs.gov",
    "DMARC Record on Base Domain": true,
    "Valid DMARC Record on Base Domain": true,
    "DMARC Results on Base Domain": "v=DMARC1; p=none; pct=100; rua=mailto:DMARC@hq.dhs.gov, mailto:reports@dmarc.cyber.dhs.gov",
<snip>
    "Syntax Errors": null,
    "Debug Info": null
  }
]

Note that running the old (develop) code you gives the same result obtained when running the new (bugfix/problem_with_psl_caching) code the first time.

One can also specify where the PSL will be cached using the --psl-filename command-line option.

jsf9k added 2 commits March 17, 2018 07:37
Previously, if the PSL file did not exist and had to be downloaded,
the stream object was read into the cache file and then passed to
publicsuffix.PublicSuffixList().  At this point the stream object was
empty, so the code was effectively using an empty PSL until get_psl()
was called again and the PSL was read in from the disk cache.

This change fixes that by modifying get_psl().  Now that function
checks to see if the cache file exists and is less than 24 hours old
and downloads it if either of those checks fails.  Then it opens the
cache file and hands that file stream (which is guaranteed to be
unread and full of data) off to publicsuffix.PublicSuffixList().
@jsf9k jsf9k added the bug This issue or pull request addresses broken functionality label Mar 17, 2018
@jsf9k jsf9k self-assigned this Mar 17, 2018
@jsf9k jsf9k requested review from seanthegeek, h-m-f-t and dav3r March 17, 2018 11:57
@jsf9k jsf9k merged commit b70fbd9 into develop Mar 20, 2018
@jsf9k jsf9k deleted the bugfix/problem_with_psl_caching branch March 20, 2018 20:19
mcdonnnj added a commit that referenced this pull request Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue or pull request addresses broken functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant