-
-
Notifications
You must be signed in to change notification settings - Fork 790
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
Check SPF/DKIM/DMARC record on dns like MX records #3083
base: master
Are you sure you want to change the base?
Conversation
Thanks for submitting this pull request. bors try Note: if this build fails, read this. |
tryBuild succeeded: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi. Thanks for your contribution.
I was thinking about doing the checks asynchronously (by having a "spinner" image and requesting the check via src="" returning a good or bad image), but this is a good start.
I didn't run in yet, but I think the regex should be changed.
try: | ||
hostnames = app.config['HOSTNAMES'].replace(',', '|') | ||
return any( | ||
re.search(f'^"v=spf1 mx a:{hostnames} [?+-~]all"$', rset.to_text()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think hostnames should be grouped: f'^"v=spf1 mx a:({hostnames}) [?+-~]all"$'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can improve all regexes if there is expert about dns records. Yeah my mistake, i will update this.
except dns.exception.DNSException: | ||
return False | ||
|
||
def check_dmarc(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be nice to check dns_dmarc_report, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dmarc record
and DMARC report record
was grouped, check mark icon is in first column per row. That's why i checked both record in one method. And both record has only one icon in their first column. I can split and put two check mark next to record besides row
If we do this asynchronously, I will explicitly check all dns entries as well as auto-configuration entries. And check methods could return states like |
Sorry for the delay. At first I thought the simplest way to implement this would be a route to check dns entries like Alex |
Thanks for submitting this pull request. bors try Note: if this build fails, read this. |
tryBuild succeeded: |
What type of PR?
Enhancement
What does this PR do?
Checks TXT records for SPF, DKIM, DMARC same as MX record checker