Skip to content

Commit 53d0ad2

Browse files
committed
Fix various small bugs
- kex_hash_func was actually not enabled for mail - leftover debug lines - mx_status mismatch - some error hardening
1 parent 8d178aa commit 53d0ad2

File tree

6 files changed

+22
-23
lines changed

6 files changed

+22
-23
lines changed

checks/tasks/shared.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ def do_mail_get_servers(self, url, *args, **kwargs):
143143
# Invalid NULL MX next to other MX.
144144
return [(None, MxStatus.null_mx_with_other_mx)]
145145
elif not do_resolve_single_a_aaaa(url):
146-
return [(None, None, MxStatus.null_mx_without_a_aaaa)]
146+
return [(None, MxStatus.null_mx_without_a_aaaa)]
147147
return [(None, MxStatus.null_mx)]
148148

149149
rdata = rdata.lower().strip()

checks/tasks/tls/evaluation.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ def from_certificate_deployments(cls, certificate_deployment: CertificateDeploym
223223
ocsp_in_cert = False
224224

225225
has_ocsp_response = certificate_deployment.ocsp_response is not None
226-
ocsp_response_trusted = certificate_deployment.ocsp_response is True
226+
ocsp_response_trusted = certificate_deployment.ocsp_response_is_trusted is True
227227

228228
return cls(
229229
ocsp_in_cert=ocsp_in_cert,

checks/tasks/tls/scans.py

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ def check_pubkey(certificates: List[Certificate], mode: ChecksMode):
490490
message = f"{common_name}: {failed_key_type}-{key_size} key_size"
491491
if curve:
492492
message += f", curve: {curve}"
493-
if public_key.curve in CERT_EC_CURVES_PHASE_OUT:
493+
if isinstance(public_key, EllipticCurvePublicKey) and type(public_key.curve) in CERT_EC_CURVES_PHASE_OUT:
494494
phase_out_pubkey.append(message)
495495
else:
496496
bad_pubkey.append(message)
@@ -599,15 +599,17 @@ def check_mail_tls(result: ServerScanResult, all_suites: List[CipherSuitesScanAt
599599
protocol_evaluation = TLSProtocolEvaluation.from_protocols_accepted(prots_accepted)
600600
fs_evaluation = TLSForwardSecrecyParameterEvaluation.from_ciphers_accepted(ciphers_accepted)
601601
cipher_evaluation = TLSCipherEvaluation.from_ciphers_accepted(ciphers_accepted)
602+
server_connectivity_info = ServerConnectivityInfo(
603+
server_location=result.server_location,
604+
network_configuration=result.network_configuration,
605+
tls_probing_result=result.connectivity_result,
606+
)
602607
cipher_order_evaluation = test_cipher_order(
603-
ServerConnectivityInfo(
604-
server_location=result.server_location,
605-
network_configuration=result.network_configuration,
606-
tls_probing_result=result.connectivity_result,
607-
),
608+
server_connectivity_info,
608609
prots_accepted,
609610
cipher_evaluation,
610611
)
612+
key_exchange_hash_evaluation = test_key_exchange_hash(server_connectivity_info)
611613
cert_results = cert_checks(result.server_location.hostname, ChecksMode.MAIL)
612614

613615
# HACK for DANE-TA(2) and hostname mismatch!
@@ -646,7 +648,8 @@ def check_mail_tls(result: ServerScanResult, all_suites: List[CipherSuitesScanAt
646648
else None,
647649
compression_score=(
648650
scoring.WEB_TLS_COMPRESSION_BAD
649-
if result.scan_result.tls_compression.result.supports_compression
651+
if result.scan_result.tls_compression.result
652+
and result.scan_result.tls_compression.result.supports_compression
650653
else scoring.WEB_TLS_COMPRESSION_GOOD
651654
),
652655
dh_param=fs_evaluation.max_dh_size,
@@ -664,8 +667,8 @@ def check_mail_tls(result: ServerScanResult, all_suites: List[CipherSuitesScanAt
664667
if result.scan_result.tls_1_3_early_data.result.supports_early_data
665668
else scoring.WEB_TLS_ZERO_RTT_GOOD
666669
),
667-
kex_hash_func=KexHashFuncStatus.good,
668-
kex_hash_func_score=scoring.WEB_TLS_KEX_HASH_FUNC_OK,
670+
kex_hash_func=key_exchange_hash_evaluation.status,
671+
kex_hash_func_score=key_exchange_hash_evaluation.score,
669672
)
670673
results.update(cert_results)
671674
return results
@@ -768,10 +771,13 @@ def check_web_tls(url, af_ip_pair=None, *args, **kwargs):
768771
if result.scan_result.session_renegotiation.result.is_vulnerable_to_client_renegotiation_dos
769772
else scoring.WEB_TLS_CLIENT_RENEG_GOOD
770773
),
771-
compression=result.scan_result.tls_compression.result.supports_compression,
774+
compression=result.scan_result.tls_compression.result.supports_compression
775+
if result.scan_result.tls_compression.result
776+
else None,
772777
compression_score=(
773778
scoring.WEB_TLS_COMPRESSION_BAD
774-
if result.scan_result.tls_compression.result.supports_compression
779+
if result.scan_result.tls_compression.result
780+
and result.scan_result.tls_compression.result.supports_compression
775781
else scoring.WEB_TLS_COMPRESSION_GOOD
776782
),
777783
dh_param=fs_evaluation.max_dh_size,
@@ -842,7 +848,7 @@ def raise_sslyze_errors(result: ServerScanResult) -> None:
842848
"""
843849
last_error_trace = None
844850
for scan_result in vars(result.scan_result).values():
845-
error_trace = getattr(scan_result, "error_trace")
851+
error_trace = getattr(scan_result, "error_trace", None)
846852
if error_trace:
847853
last_error_trace = error_trace
848854
log.info(f"TLS scan on {result.server_location} failed: {error_trace}: {''.join(error_trace.format())}")
@@ -1013,7 +1019,7 @@ def check_supported_tls_versions(server_connectivity_info: ServerConnectivityInf
10131019
try:
10141020
ssl_connection.connect()
10151021
supported_tls_versions.append(tls_version)
1016-
except (ConnectionToServerFailed, OpenSSLError) as exc:
1022+
except (ConnectionToServerFailed, OpenSSLError, TlsHandshakeTimedOut) as exc:
10171023
log.debug(
10181024
f"Server {server_connectivity_info.server_location.hostname}"
10191025
f"/{server_connectivity_info.server_location.ip_address}"

checks/tasks/tls/tasks_reports.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ def callback(results, domain, test_type):
118118
if "mx_status" in results:
119119
return callback_null_mx(results, domain, test_type)
120120
testdomain = test_map[test_type]["model"](domain=domain)
121-
if testdomain is MailTestTls:
121+
if isinstance(testdomain, MailTestTls):
122122
testdomain.mx_status = MxStatus.has_mx
123123
testdomain.save()
124124
category = test_map[test_type]["category"]
@@ -831,16 +831,13 @@ def do_mail_smtp_starttls(mailservers, url, *args, **kwargs):
831831
# Pull in any cached results
832832
cache_id = redis_id.mail_starttls.id.format(server)
833833
results[server] = cache.get(cache_id, False)
834-
log.debug(f"=========== pulled {cache_id=} for {server=} data {results[server]}")
835834
while timer() - start < cache_ttl and (not results or not all(results.values())):
836835
servers_to_check = [server for server, _ in mailservers if not results[server]]
837-
log.debug(f"=========== checking remaining {servers_to_check=}")
838836
results.update(check_mail_tls_multiple(servers_to_check))
839837
time.sleep(1)
840838
for server, server_result in results.items():
841839
cache_id = redis_id.mail_starttls.id.format(server)
842840
cache.set(cache_id, server_result, cache_ttl)
843-
log.debug(f"=========== writing to {cache_id=} for {server=}")
844841
if results[server] is False:
845842
results[server] = dict(tls_enabled=False, could_not_test_smtp_starttls=True)
846843
except SoftTimeLimitExceeded:

checks/tasks/tls/tls_constants.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,6 @@
103103
"TLS_RSA_WITH_AES_128_CBC_SHA256",
104104
"TLS_RSA_WITH_AES_128_CBC_SHA",
105105
"TLS_RSA_WITH_3DES_EDE_CBC_SHA",
106-
# CCM is not in appendix C, but footnote 31 makes it Good (on its own)
107-
"TLS_RSA_WITH_AES_128_CCM",
108-
"TLS_RSA_WITH_AES_256_CCM",
109106
]
110107

111108
# NCSC table 1

requirements.in

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ abnf
4646
pyopenssl
4747
dnspython
4848

49-
# sslyze dependencies, which is installed from outside this file
5049
sslyze
5150

5251
# https://stackoverflow.com/questions/73933432/django-celery-cannot-import-name-celery-from-celery-after-rebuilding-dockerf

0 commit comments

Comments
 (0)