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

Custom domain simplifaction #163

Merged
merged 21 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions lib/github-pages-health-check/domain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class Domain < Checkable
host uri nameservers dns_resolves? proxied? cloudflare_ip?
fastly_ip? old_ip_address? a_record? aaaa_record? a_record_present? aaaa_record_present?
cname_record? mx_records_present? valid_domain? apex_domain?
should_be_a_record? cname_to_github_user_domain?
should_be_a_record? cname_to_github_user_domain? cname_to_domain_to_pages?
cname_to_pages_dot_github_dot_com? cname_to_fastly?
pointed_to_github_pages_ip? non_github_pages_ip_present? pages_domain?
served_by_pages? valid? reason valid_domain? https?
Expand Down Expand Up @@ -243,6 +243,16 @@ def cname_to_github_user_domain?
cname? && !cname_to_pages_dot_github_dot_com? && cname.pages_domain?
end

# Check if the CNAME points to a Domain that points to pages
# e.g. CNAME -> Domain -> Pages
def cname_to_domain_to_pages?
Copy link
Contributor

Choose a reason for hiding this comment

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

this function seems not check the cname point to A record which points to pages.

Does this allow the user input arbitrary cname?

Copy link
Contributor Author

@tsusdere tsusdere Apr 10, 2024

Choose a reason for hiding this comment

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

It should the main thing is that we are checking for is that the CNAME doesn't already point to us then we run this check

return true if cname_to_github_user_domain? || cname_to_domain_to_pages?

I have a test that checks if it points to it but let me add a test in the event that it doesn't

Copy link
Contributor

@YiMysty YiMysty Apr 10, 2024

Choose a reason for hiding this comment

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

I mainly mean if we want to only support www.fontawesome.it => fontawesome.it => pagesxxx

or loosely support blog.fontawesome.it => fontawesome.it => pagesxxxx

hmmm loosely is fine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohh I see let me double check on that!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it looks like we just want to support www.fontawesome.it => fontawesome.it => pages where we match the host after the www

a_record_to_pages = dns.select { |d| d.type == Dnsruby::Types::A && d.name.to_s == host }.first

return false unless a_record_to_pages && cname? && !cname_to_pages_dot_github_dot_com? && @www_cname

CURRENT_IP_ADDRESSES.include?(a_record_to_pages.address.to_s.downcase)
end

# Is the given domain a CNAME to pages.github.(io|com)
# instead of being CNAME'd to the user's subdomain?
#
Expand Down Expand Up @@ -304,6 +314,7 @@ def proxied?
return true if cloudflare_ip?
return false if pointed_to_github_pages_ip?
return false if cname_to_github_user_domain?
return false if cname_to_domain_to_pages?
return false if cname_to_pages_dot_github_dot_com?
return false if cname_to_fastly? || fastly_ip?

Expand Down Expand Up @@ -399,9 +410,16 @@ def cname
cnames = dns.take_while { |answer| answer.type == Dnsruby::Types::CNAME }
return if cnames.empty?

www_cname(cnames.last)
@cname ||= Domain.new(cnames.last.cname.to_s)
end

# Check if we have a 'www.' CNAME that matches the domain
def www_cname(cname)
@www_cname ||= cname.name.to_s.start_with?("www.") &&
cname.name.to_s.end_with?(cname.domainname.to_s)
end

def mx_records_present?
return unless dns?

Expand Down Expand Up @@ -454,8 +472,7 @@ def https_eligible?
return false if host.include?("_")

# Must be a CNAME or point to our IPs.
# Only check the one domain if a CNAME. Don't check the parent domain.
return true if cname_to_github_user_domain?
return true if cname_to_github_user_domain? || cname_to_domain_to_pages?

# Check CAA records for the full domain and its parent domain.
pointed_to_github_pages_ip? && caa.lets_encrypt_allowed?
Expand Down
2 changes: 1 addition & 1 deletion lib/github-pages-health-check/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@

module GitHubPages
module HealthCheck
VERSION = "1.18.2"
VERSION = "1.18.3"
end
end
94 changes: 94 additions & 0 deletions spec/github_pages_health_check/domain_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,100 @@
end
end

context "CNAME to Domain to Pages" do
let(:cname) { "www.fontawesome.it" }
let(:domain) { "fontawesome.it" }
let(:ip) { "185.199.108.153" }
before(:each) do
allow(subject).to receive(:dns) do
[
Dnsruby::RR.create("#{cname}. 1000 IN CNAME #{domain}"),
a_packet
]
end
end

it "follows the CNAMEs all the way down" do
expect(subject.cname.host).to eq("fontawesome.it")
end

it "knows it's a Pages IP at the end" do
expect(subject).to be_a_cname_to_domain_to_pages
end
end

context "Random CNAME to Domain that goes to Pages" do
let(:cname) { "monalisa" }
let(:domain) { "fontawesome.it" }
let(:ip) { "185.199.108.153" }
before(:each) do
allow(subject).to receive(:dns) do
[
Dnsruby::RR.create("#{cname}. 1000 IN CNAME #{domain}"),
a_packet
]
end
end

it "CNAME does not start with www and no match to host" do
expect(subject).to_not be_a_cname_to_domain_to_pages
end
end

context "CNAME with same host but no www" do
let(:cname) { "blog.fontawesome.it" }
let(:domain) { "fontawesome.it" }
let(:ip) { "185.199.108.153" }
before(:each) do
allow(subject).to receive(:dns) do
[
Dnsruby::RR.create("#{cname}. 1000 IN CNAME #{domain}"),
a_packet
]
end
end

it "CNAME does not start with www and no match to host" do
expect(subject).to_not be_a_cname_to_domain_to_pages
end
end

context "CNAME starts with www but different host" do
let(:cname) { "www.fontawesome.it" }
let(:domain) { "awesomefont.it" }
let(:ip) { "185.199.108.153" }
before(:each) do
allow(subject).to receive(:dns) do
[
Dnsruby::RR.create("#{cname}. 1000 IN CNAME #{domain}"),
a_packet
]
end
end

it "CNAME does not match to host" do
expect(subject).to_not be_a_cname_to_domain_to_pages
end
end

context "CNAME to Domain that doesn't go to Pages" do
let(:cname) { "www.fontawesome.it" }
let(:domain) { "fontawesome.it" }
let(:ip) { "127.0.0.1" }
before(:each) do
allow(subject).to receive(:dns) do
[
Dnsruby::RR.create("#{cname}. 1000 IN CNAME #{domain}"),
a_packet
]
end
end

it "knows it's not a Pages IP at the end" do
expect(subject).to_not be_a_cname_to_domain_to_pages
end
end

context "broken CNAMEs" do
before do
allow(subject).to receive(:dns) do
Expand Down