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

Adds checking for downgrade redirects, storing of redirect domain and howing redirect domain in direct tests #1542

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
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
5 changes: 5 additions & 0 deletions checks/categories.py
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,11 @@ def result_bad(self):
self.verdict = "detail web tls https-forced verdict bad"
self.tech_data = "detail tech data no"

def result_bad_redirect(self, redirect_domain):
self._status(STATUS_FAIL)
self.verdict = "detail web tls https-forced verdict bad-redirect"
self.tech_data = f"detail tech data bad-redirect {redirect_domain}"


class WebTlsHttpsHsts(Subtest):
def __init__(self):
Expand Down
17 changes: 17 additions & 0 deletions checks/migrations/0016_domaintesttls_redirect_domain.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 4.2.15 on 2024-11-08 16:03

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("checks", "0015_add_rpki_scoring"),
]

operations = [
migrations.AddField(
model_name="domaintesttls",
name="redirect_domain",
field=models.CharField(default=None, max_length=255, null=True),
),
]
4 changes: 4 additions & 0 deletions checks/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class ForcedHttpsStatus(Enum):
good = 1
no_http = 2
no_https = 3
bad_redirect = 4


class OcspStatus(Enum):
Expand Down Expand Up @@ -523,6 +524,8 @@ class DomainTestTls(BaseTestModel):
forced_https = EnumField(ForcedHttpsStatus, default=ForcedHttpsStatus.bad)
forced_https_score = models.IntegerField(null=True)

redirect_domain = models.CharField(null=True, max_length=255, default=None)

# HTTP headers
http_compression_enabled = models.BooleanField(null=True, default=False)
http_compression_score = models.IntegerField(null=True)
Expand Down Expand Up @@ -592,6 +595,7 @@ def __dir__(self):
"kex_hash_func_score",
"forced_https",
"forced_https_score",
"redirect_domain",
"http_compression_enabled",
"http_compression_score",
"hsts_enabled",
Expand Down
44 changes: 36 additions & 8 deletions checks/tasks/tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,7 @@ def save_results(model, results, addr, domain, category):
elif testname == "http_checks":
model.forced_https = result.get("forced_https")
model.forced_https_score = result.get("forced_https_score")
model.redirect_domain = result.get("redirect_domain")
model.http_compression_enabled = result.get("http_compression_enabled")
model.http_compression_score = result.get("http_compression_score")
model.hsts_enabled = result.get("hsts_enabled")
Expand Down Expand Up @@ -770,6 +771,8 @@ def annotate_and_combine_all(good_items, sufficient_items, bad_items, phaseout_i
category.subtests["https_forced"].result_no_https()
elif dttls.forced_https == ForcedHttpsStatus.bad:
category.subtests["https_forced"].result_bad()
elif dttls.forced_https == ForcedHttpsStatus.bad_redirect:
category.subtests["https_forced"].result_bad_redirect(dttls.redirect_domain)

if dttls.hsts_enabled:
if dttls.hsts_score == scoring.WEB_TLS_HSTS_GOOD:
Expand Down Expand Up @@ -2931,8 +2934,8 @@ def do_web_http(af_ip_pairs, url, task, *args, **kwargs):
Start all the HTTP related checks for the web test.

"""
results = {}
try:
results = {}
for af_ip_pair in af_ip_pairs:
results[af_ip_pair[1]] = http_checks(af_ip_pair, url, task)

Expand All @@ -2943,22 +2946,23 @@ def do_web_http(af_ip_pairs, url, task, *args, **kwargs):
results[af_ip_pair[1]] = dict(
forced_https=False,
forced_https_score=scoring.WEB_TLS_FORCED_HTTPS_BAD,
redirect_domain=None,
http_compression_enabled=True,
http_compression_score=(scoring.WEB_TLS_HTTP_COMPRESSION_BAD),
http_compression_score=scoring.WEB_TLS_HTTP_COMPRESSION_BAD,
hsts_enabled=False,
hsts_policies=[],
hsts_score=scoring.WEB_TLS_HSTS_BAD,
)

return ("http_checks", results)
return "http_checks", results


def http_checks(af_ip_pair, url, task):
"""
Perform the HTTP header and HTTPS redirection checks for this webserver.

"""
forced_https_score, forced_https = forced_http_check(af_ip_pair, url, task)
forced_https_score, forced_https, redirect_domain = forced_http_check(af_ip_pair, url, task)
header_checkers = [
HeaderCheckerContentEncoding(),
HeaderCheckerStrictTransportSecurity(),
Expand All @@ -2968,6 +2972,7 @@ def http_checks(af_ip_pair, url, task):
results = {
"forced_https": forced_https,
"forced_https_score": forced_https_score,
"redirect_domain": redirect_domain,
}
results.update(header_results)
return results
Expand All @@ -2978,29 +2983,52 @@ def forced_http_check(af_ip_pair, url, task):
Check if the webserver is properly configured with HTTPS redirection.
"""
try:
http_get_ip(hostname=url, ip=af_ip_pair[1], port=443, https=True)
response_https = http_get_ip(hostname=url, ip=af_ip_pair[1], port=443, https=True)
except requests.RequestException:
# No HTTPS connection available to our HTTP client.
# Could also be too outdated config (#1130)
return scoring.WEB_TLS_FORCED_HTTPS_BAD, ForcedHttpsStatus.no_https
return scoring.WEB_TLS_FORCED_HTTPS_BAD, ForcedHttpsStatus.no_https, None

# We also check if the domain redirects to an external domain and if so store this domain.
# This has two purposes:
# 1) be able to add the redirect domain to direct tests in frontend.
# 2) check if there is a downgrade redirect (https > http).
# Note we only check based on the Location http-header (no HTML/JavaScript redirects).
redirect_url, redirect_domain = None, None

try:
response_http = http_get_ip(hostname=url, ip=af_ip_pair[1], port=80, https=False)
except requests.RequestException:
# No plain HTTP available, but HTTPS is
return scoring.WEB_TLS_FORCED_HTTPS_NO_HTTP, ForcedHttpsStatus.no_http
# Check if the HTTPS itself redirects to HTTP (downgrade redirect).
if response_https.headers.get("Location"):
redirect_url = urlparse(response_https.headers.get("Location"))
if redirect_url and redirect_url.scheme == "http":
redirect_domain = redirect_url.hostname
return scoring.WEB_TLS_FORCED_HTTPS_BAD, ForcedHttpsStatus.bad_redirect, redirect_domain
return scoring.WEB_TLS_FORCED_HTTPS_NO_HTTP, ForcedHttpsStatus.no_http, redirect_domain

forced_https = ForcedHttpsStatus.bad
forced_https_score = scoring.WEB_TLS_FORCED_HTTPS_BAD

for response in response_http.history[1:] + [response_http]:
if response.url:
parsed_url = urlparse(response.url)

if response.headers.get("Location"):
redirect_url = urlparse(response.headers.get("Location"))
if redirect_url:
redirect_domain = redirect_url.hostname

# Requirement: in case of redirecting, a domain should firstly upgrade itself by
# redirecting to its HTTPS version before it may redirect to another domain (#1208)
if parsed_url.scheme == "https" and url == parsed_url.hostname:
# Check if there is a downgrade redirect
if redirect_url and redirect_url.scheme == "http":
return scoring.WEB_TLS_FORCED_HTTPS_BAD, ForcedHttpsStatus.bad_redirect, redirect_domain

forced_https = ForcedHttpsStatus.good
forced_https_score = scoring.WEB_TLS_FORCED_HTTPS_GOOD
break

return forced_https_score, forced_https
return forced_https_score, forced_https, redirect_domain
1 change: 1 addition & 0 deletions interface/batch/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,7 @@ components:
* `bad` - HTTPS redirection is not enforced.
* `no_http` - No HTTP connection; test is not relevant.
* `no_https` - No or outdated HTTPS connection; test could not be executed.
* `bad_redirect` - Outbound redirection towards HTTP instead of HTTPS.
http_compression:
type: boolean
description: If HTTP compression is used.
Expand Down
22 changes: 20 additions & 2 deletions interface/views/domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def create_report(domain, ipv6, dnssec, tls=None, appsecpriv=None, rpki=None):
return report


def get_direct_domains(address):
def get_direct_domains(address, redirect_domain=None):
webtest_direct = []
# Add either the 'www.' version or the non 'www.' version, whichever we are
# not, to the direct links.
Expand All @@ -87,6 +87,15 @@ def get_direct_domains(address):
if domain:
webtest_direct.append(pretty_domain_name(domain))

# Add redirection domain to direct web test if stored and testable
if (
redirect_domain
and get_valid_domain_web(redirect_domain)
and redirect_domain != address
and pretty_domain_name(redirect_domain) not in webtest_direct
):
webtest_direct.append(pretty_domain_name(get_valid_domain_web(redirect_domain)))

mailtest_direct = []
# Add the current domain and the non 'www.' version, if applicable, to the
# direct links.
Expand All @@ -98,6 +107,15 @@ def get_direct_domains(address):
if domain:
mailtest_direct.append(pretty_domain_name(domain))

# Add redirection domain to direct mail test if stored and testable
if (
redirect_domain
and get_valid_domain_mail(redirect_domain)
and redirect_domain != address
and pretty_domain_name(redirect_domain) not in mailtest_direct
):
mailtest_direct.append(pretty_domain_name(get_valid_domain_mail(redirect_domain)))

return webtest_direct, mailtest_direct


Expand All @@ -107,7 +125,7 @@ def resultsrender(addr, report, request):
score = webprobes.count_probe_reports_score(probe_reports)
add_score_to_report(report, score)
retest_time = get_retest_time(report)
webtest_direct, mailtest_direct = get_direct_domains(addr)
webtest_direct, mailtest_direct = get_direct_domains(addr, report.tls.webtestset.first().redirect_domain)
prettyaddr = pretty_domain_name(addr)
return render(
request,
Expand Down
Loading