Skip to content

Commit 28d6e76

Browse files
mxsashamxsasha
committed
fix duplications, fix cipher order tech table
1 parent 903be94 commit 28d6e76

File tree

4 files changed

+31
-90
lines changed

4 files changed

+31
-90
lines changed

‎checks/categories.py‎

Lines changed: 2 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,24 +1073,9 @@ def result_good(self):
10731073
self.verdict = "detail web tls cipher-order verdict good"
10741074
self.tech_data = ""
10751075

1076-
def result_bad(self):
1076+
def result_bad(self, cipher_order_violation):
10771077
self._status(STATUS_FAIL)
10781078
self.verdict = "detail web tls cipher-order verdict bad"
1079-
self.tech_data = ""
1080-
1081-
def result_seclevel_bad(self, cipher_order_violation):
1082-
self._status(STATUS_FAIL)
1083-
self.verdict = "detail web tls cipher-order verdict seclevel-bad"
1084-
self.tech_data = cipher_order_violation
1085-
1086-
def result_score_warning(self, cipher_order_violation):
1087-
self._status(STATUS_NOTICE)
1088-
self.verdict = "detail web tls cipher-order verdict warning"
1089-
self.tech_data = cipher_order_violation
1090-
1091-
def result_score_info(self, cipher_order_violation):
1092-
self._status(STATUS_INFO)
1093-
self.verdict = "detail web tls cipher-order verdict warning"
10941079
self.tech_data = cipher_order_violation
10951080

10961081
def result_na(self):
@@ -1620,28 +1605,10 @@ def result_good(self):
16201605
self.verdict = "detail mail tls cipher-order verdict good"
16211606
self.tech_data = ""
16221607

1623-
def result_bad(self):
1608+
def result_bad(self, cipher_order_violation):
16241609
self.was_tested()
16251610
self._status(STATUS_FAIL)
16261611
self.verdict = "detail mail tls cipher-order verdict bad"
1627-
self.tech_data = ""
1628-
1629-
def result_seclevel_bad(self, cipher_order_violation):
1630-
self.was_tested()
1631-
self._status(STATUS_FAIL)
1632-
self.verdict = "detail mail tls cipher-order verdict seclevel-bad"
1633-
self.tech_data = cipher_order_violation
1634-
1635-
def result_warning(self, cipher_order_violation):
1636-
self.was_tested()
1637-
self._status(STATUS_NOTICE)
1638-
self.verdict = "detail mail tls cipher-order verdict warning"
1639-
self.tech_data = cipher_order_violation
1640-
1641-
def result_info(self, cipher_order_violation):
1642-
self.was_tested()
1643-
self._status(STATUS_INFO)
1644-
self.verdict = "detail mail tls cipher-order verdict warning"
16451612
self.tech_data = cipher_order_violation
16461613

16471614
def result_na(self):

‎checks/tasks/tls/evaluation.py‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
from dataclasses import dataclass
2-
from typing import List, Optional
2+
from typing import List, Optional, Any, Set
33

44
from nassl.ephemeral_key_info import EcDhEphemeralKeyInfo, DhEphemeralKeyInfo, OpenSslEvpPkeyEnum
55
from sslyze import TlsVersionEnum, CipherSuiteAcceptedByServer, CipherSuite
@@ -75,38 +75,38 @@ class TLSForwardSecrecyParameterEvaluation:
7575
max_dh_size: Optional[int]
7676
max_ec_size: Optional[int]
7777

78-
good_str: List[str]
79-
phase_out_str: List[str]
80-
bad_str: List[str]
78+
good_str: Set[str]
79+
phase_out_str: Set[str]
80+
bad_str: Set[str]
8181

8282
@classmethod
8383
def from_ciphers_accepted(cls, ciphers_accepted: List[CipherSuiteAcceptedByServer]):
84-
good = []
85-
phase_out = []
86-
bad = []
84+
good = set()
85+
phase_out = set()
86+
bad = set()
8787

8888
# Evaluate according to NCSC table 4 and table 10
89-
for suite in ciphers_accepted:
89+
for suite in _unique_unhashable(ciphers_accepted):
9090
key = suite.ephemeral_key
9191
if not key:
9292
continue
9393

9494
if isinstance(key, EcDhEphemeralKeyInfo):
9595
if key.size < FS_ECDH_MIN_KEY_SIZE:
96-
bad.append(f"ECDH-{key.size}")
96+
bad.add(f"ECDH-{key.size}")
9797
if key.curve in FS_EC_PHASE_OUT:
98-
phase_out.append(f"ECDH-{key.curve_name}")
98+
phase_out.add(f"ECDH-{key.curve_name}")
9999
elif key.curve not in FS_EC_GOOD:
100-
bad.append(f"ECDH-{key.curve_name}")
100+
bad.add(f"ECDH-{key.curve_name}")
101101

102102
if isinstance(key, DhEphemeralKeyInfo):
103103
if key.size < FS_DH_MIN_KEY_SIZE:
104-
bad.append(f"DH-{key.size}")
104+
bad.add(f"DH-{key.size}")
105105
if key.generator == FFDHE_GENERATOR:
106106
if key.prime == FFDHE2048_PRIME:
107-
phase_out.append("FFDHE-2048")
107+
phase_out.add("FFDHE-2048")
108108
elif key.prime not in FFDHE_SUFFICIENT_PRIMES:
109-
bad.append(f"DH-{key.size}")
109+
bad.add(f"DH-{key.size}")
110110

111111
dh_sizes = [
112112
suite.ephemeral_key.size
@@ -150,7 +150,7 @@ def from_ciphers_accepted(cls, ciphers_accepted: List[CipherSuiteAcceptedByServe
150150
ciphers_sufficient = []
151151
ciphers_phase_out = []
152152
ciphers_bad = []
153-
for suite in ciphers_accepted:
153+
for suite in _unique_unhashable(ciphers_accepted):
154154
if suite.cipher_suite.name in CIPHERS_GOOD:
155155
ciphers_good.append(suite.cipher_suite)
156156
elif suite.cipher_suite.name in CIPHERS_SUFFICIENT:
@@ -191,3 +191,13 @@ class TLSCipherOrderEvaluation:
191191
violation: List[str]
192192
status: CipherOrderStatus
193193
score: scoring.Score
194+
195+
196+
def _unique_unhashable(ciphers: List[Any]) -> List[Any]:
197+
# CipherSuite is not hashable, so can't always use set()
198+
result = []
199+
for cipher in ciphers:
200+
if cipher not in result:
201+
result.append(cipher)
202+
return result
203+

‎checks/tasks/tls/scans.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -769,6 +769,7 @@ def test_cipher_order(
769769
# Sort CHACHA as later in the list, in case SSL_OP_PRIORITIZE_CHACHA is enabled #461
770770
expected_less_preferred.sort(key=lambda c: "CHACHA" in c.name)
771771
if cipher_order_violation:
772+
print("break out, got bad")
772773
break
773774
for expected_more_preferred in expected_more_preferred_list:
774775
print(
@@ -781,8 +782,8 @@ def test_cipher_order(
781782
server_connectivity_info, tls_version, expected_less_preferred + [expected_more_preferred]
782783
)
783784
if preferred_suite != expected_more_preferred:
784-
# TODO: check which name to report
785785
cipher_order_violation = [preferred_suite.name, expected_more_preferred.name]
786+
print(f"break out, got bad inner: {cipher_order_violation}")
786787
break
787788

788789
return TLSCipherOrderEvaluation(

‎checks/tasks/tls/tasks_reports.py‎

Lines changed: 2 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -436,27 +436,9 @@ def annotate_and_combine_all(good_items, sufficient_items, bad_items, phaseout_i
436436
category.subtests["tls_ciphers"].result_good()
437437

438438
if dttls.cipher_order == CipherOrderStatus.bad:
439-
category.subtests["tls_cipher_order"].result_bad()
439+
category.subtests["tls_cipher_order"].result_bad(dttls.cipher_order_violation)
440440
elif dttls.cipher_order == CipherOrderStatus.na:
441441
category.subtests["tls_cipher_order"].result_na()
442-
elif dttls.cipher_order == CipherOrderStatus.not_seclevel:
443-
(cipher1, cipher2, violated_rule) = dttls.cipher_order_violation
444-
category.subtests["tls_cipher_order"].result_seclevel_bad([[cipher1, cipher2]])
445-
elif dttls.cipher_order == CipherOrderStatus.not_prescribed:
446-
# Provide tech_data that supplies values for two rows each of
447-
# two cells to fill in a table like so:
448-
# Web server IP address | Ciphers | Rule #
449-
# -------------------------------------------------------------
450-
# 1.2.3.4 | cipher1 | ' '
451-
# ... | cipher2 | violated_rule
452-
(cipher1, cipher2, violated_rule) = dttls.cipher_order_violation
453-
# The 5th rule is only informational.
454-
if violated_rule == 5:
455-
category.subtests["tls_cipher_order"].result_score_info([[cipher1, cipher2], [" ", violated_rule]])
456-
else:
457-
category.subtests["tls_cipher_order"].result_score_warning(
458-
[[cipher1, cipher2], [" ", violated_rule]]
459-
)
460442
else:
461443
category.subtests["tls_cipher_order"].result_good()
462444

@@ -585,29 +567,10 @@ def annotate_and_combine_all(good_items, sufficient_items, bad_items, phaseout_i
585567
category.subtests["tls_ciphers"].result_phase_out(ciphers_all)
586568
else:
587569
category.subtests["tls_ciphers"].result_good()
588-
589570
if dttls.cipher_order == CipherOrderStatus.bad:
590-
category.subtests["tls_cipher_order"].result_bad()
571+
category.subtests["tls_cipher_order"].result_bad(dttls.cipher_order_violation)
591572
elif dttls.cipher_order == CipherOrderStatus.na:
592573
category.subtests["tls_cipher_order"].result_na()
593-
elif dttls.cipher_order == CipherOrderStatus.not_seclevel:
594-
(cipher1, cipher2, violated_rule) = dttls.cipher_order_violation
595-
category.subtests["tls_cipher_order"].result_seclevel_bad([[cipher1, cipher2]])
596-
elif dttls.cipher_order == CipherOrderStatus.not_prescribed:
597-
# Provide tech_data that supplies values for two rows each of
598-
# two cells to fill in a table like so:
599-
# Web server IP address | Ciphers | Rule #
600-
# -------------------------------------------------------------
601-
# 1.2.3.4 | cipher1 | ' '
602-
# ... | cipher2 | violated_rule
603-
(cipher1, cipher2, violated_rule) = dttls.cipher_order_violation
604-
# The 5th rule is only informational.
605-
if violated_rule == 5:
606-
category.subtests["tls_cipher_order"].result_score_info([[cipher1, cipher2], [" ", violated_rule]])
607-
else:
608-
category.subtests["tls_cipher_order"].result_score_warning(
609-
[[cipher1, cipher2], [" ", violated_rule]]
610-
)
611574
else:
612575
category.subtests["tls_cipher_order"].result_good()
613576

0 commit comments

Comments
 (0)