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

check_reverse_dns: don't force mw-lb.miraheze.org #3281

Merged
merged 6 commits into from
Mar 14, 2024

Conversation

MacFan4000
Copy link
Member

There is no technical reason that a custom domain couldn't point to some other miraheze.org subdomain. All possible subdomains will always exist and resolve to our servers. This uses regex to ensure that the CNAME is of the format .miraheze.org . The alert really comes into play if somebody has changed their DNS after the domain was setup for their wiki. And if they just changed what subdomain the record points to, I don't think it's worth chasing the user up about it. As for new domains, I always check the records beforehand (and anybody else handling SSL requests should too), and thus this check should not be relied on when adding SSL certs. We should be checking records before adding SSL, and only contacting users after if they changed it after, and for some reason the new configuration won't work. (non-miraheze.org CNAME, or improper A records, or somthing similar, or not pointed at all)

There is no technical reason that a custom domain couldn't point to some other miraheze.org subdomain. All possible subdomains will always exist and resolve to our servers. This uses regex to ensure that the CNAME is of the format <subdomain>.miraheze.org . The alert really comes into play if somebody has changed their DNS after the domain was setup for their wiki. And if they just changed what subdomain the record points to, I don't think it's worth chasing the user up about it. As for new domains, I always check the records beforehand (and anybody else handling SSL requests should too), and thus this check should not be relied on when adding SSL certs. We should be checking records before adding SSL, and only contacting users after if they changed it after, and for some reason the new configuration won't work. (non-miraheze.org CNAME, or improper A records, or somthing similar, or not pointed at all)
@MacFan4000
Copy link
Member Author

updated to disallow cp* and non-mediawiki subdomains (most) and non-mw subdomains not included, and other server names would already fail the RDNS check. (the left out non-mw subdomains are the ones not going though varnish)

@RhinosF1
Copy link
Contributor

RhinosF1 commented Jul 9, 2023

It would be nice to see tests

modules/monitoring/files/check_reverse_dns.py Outdated Show resolved Hide resolved
modules/monitoring/files/check_reverse_dns.py Outdated Show resolved Hide resolved
@MacFan4000
Copy link
Member Author

It would be nice to see tests

With the change a bit simpler now, I don’t think additional tests are needed.

@RhinosF1
Copy link
Contributor

RhinosF1 commented Feb 1, 2024

It would be nice to see tests

With the change a bit simpler now, I don’t think additional tests are needed.

Tests are always good. This is one of the early targets for Python functions so I'll write them.

@Universal-Omega Universal-Omega merged commit 1d614f3 into miraheze:master Mar 14, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants