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

Try to print MTA-STS missing DNS policy error reason #1884

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ValdikSS
Copy link

When proper (not self-signed) TLS certificate is not provisioned on the domain yet, MiaB prints only:

MTA-STS policy is missing: STSFetchResult.NONE

which may confuse the administrator, as .well-known/mta-sts.txt file is already present and opens correctly.

Print more human-friendly reason for this case.
Code taken from dns-update.py and slightly edited.

@JoshData
Copy link
Member

Thanks for this. If the cert is missing or invalid, I think we can just skip the MTA-STS message entirely, since there will be a message about the certificate.

When proper (not self-signed) TLS certificate is not provisioned on the domain yet,
MiaB prints only:

MTA-STS policy is missing: STSFetchResult.NONE

which may confuse the administrator, as .well-known/mta-sts.txt
file is already present and opens correctly.

Print more human-friendly reason for this case.
@ValdikSS
Copy link
Author

Like this I guess? Please check.

Comment on lines +665 to +670
if not cert:
certmessage = "No TLS certificate provisioned for this domain"
else:
cert_status = check_certificate(domain, cert['certificate'], cert['private-key'])
if cert_status[0] != 'OK':
certmessage = "TLS certificate is not valid"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm suggesting having no message (no call to output.print_error) in these two conditions.

Copy link
Author

@ValdikSS ValdikSS Jan 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to remove these text messages completely or print them not as an error? If the second, how it should be printed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove completely. Sorry for the ambiguity.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to edit this merge request as is suitable for the codebase. I'm no longer use MIAB actively and can't test my edit right now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me 👍

@ValdikSS
Copy link
Author

ValdikSS commented Apr 3, 2023

@JoshData, hello, any chance to get this merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants