Skip to content

Fix bugs in _security_tls_v1_2_server #1248

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.

Sign up for GitHub

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

Merged
jboddey merged 6 commits into from
May 15, 2025
Merged

Fix bugs in _security_tls_v1_2_server #1248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fd2efb5
fix tls bugs, added unit tests for tls 1.2 server test
Apr 16, 2025
cda95c2
Merge branch 'dev' of https://github.com/MariusBaldovin/testrun into …
Apr 16, 2025
574a0b2
uncomment tls tests
Apr 16, 2025
7101ea3
uncomment tls test
Apr 16, 2025
fc91264
corrected if the tls 1.2 is invalid and 1.3 is valid
Apr 16, 2025
1d0468d
Merge branch 'dev' into bug/tls
jboddey May 15, 2025
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
18 changes: 15 additions & 3 deletions modules/test/tls/python/src/tls_module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,12 @@ def _security_tls_v1_2_server(self):
host=self._device_ipv4_addr, port=port, tls_version='1.2')
tls_1_3_results = self._tls_util.validate_tls_server(
host=self._device_ipv4_addr, port=port, tls_version='1.3')
port_results = self._tls_util.process_tls_server_results(
tls_1_2_results, tls_1_3_results, port=port)
# If TLS 1.2 is not supported don't process the results
if tls_1_2_results[0] is not None:
port_results = self._tls_util.process_tls_server_results(
tls_1_2_results, tls_1_3_results, port=port)
else:
port_results = None
Comment on lines +350 to +355
Copy link
Contributor

@jhughesoti jhughesoti Apr 16, 2025

Choose a reason for hiding this comment

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

TLS 1.3 can still validate TLS 1.2. If a device is running just TLS 1.3, it shouldn't fail TLS 1.2 since the requirement is at least TLS 1.2 for this test so there should be no need for this additional check.

Copy link
Contributor Author

@MariusBaldovin MariusBaldovin Apr 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

_tls_util.process_tls_server_results is causing a bug. When TLS 1.2 is not supported the TLS 1.3 will incorrectly validate/invalidate 1.2 (#1228). With these changes when TLS 1.2 is not supported the test result will be "Feature not detected" and since this test is "Required if Applicable" it will not affect the overall test result. The TLS 1.3 will still validate TLS 1.2 but only when 1.2 is non compliant and 1.3 is compliant (line 374). Also the description will mention that tls 1.2 cert is invalid but 1.3 is valid. I have added unit tests for all use cases for _security_tls_v1_2_server.

Copy link
Contributor

@jhughesoti jhughesoti Apr 16, 2025

Choose a reason for hiding this comment

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

That bug is only if TLS 1.3 is invalid and TLS 1.2 is valid so this still needs to check both TLS 1.2 and 1.3 in the 1.2 test but not fail if TLS 1.2 is valid but not 1.3. This needs to be fixed in the validation logic, not removing that scenario from being tested.

if port_results is not None:
result = port_results[
0] if result is None else result and port_results[0]
Expand All @@ -368,6 +372,12 @@ def _security_tls_v1_2_server(self):
if result is None:
result = 'Feature Not Detected'
description = 'TLS 1.2 certificate could not be validated'
details = 'TLS 1.2 certificate could not be validated'
# If TLS 1.2 cert is not valid but TLS 1.3 is valid test is Compliant
elif result and not tls_1_2_results[0] and tls_1_3_results[0]:
ports_csv = ','.join(map(str,ports_valid))
description = 'TLS 1.2 certificate invalid and '
description += f'TLS 1.3 certificate valid on ports: {ports_csv}'
elif result:
ports_csv = ','.join(map(str,ports_valid))
description = f'TLS 1.2 certificate valid on ports: {ports_csv}'
Expand All @@ -377,7 +387,9 @@ def _security_tls_v1_2_server(self):
return result, description, details
else:
LOGGER.error('Could not resolve device IP address. Skipping')
return 'Error', 'Could not resolve device IP address'
description = 'Could not resolve device IP address'
details = 'Could not resolve device IP address'
return 'Error', description, details

def _security_tls_v1_3_server(self):
LOGGER.info('Running security.tls.v1_3_server')
Expand Down
2 changes: 1 addition & 1 deletion modules/test/tls/python/src/tls_util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ def process_tls_server_results(self, tls_1_2_results, tls_1_3_results, port):
f"""validated on port {port}: """
f"""{tls_1_2_results[1]}""")
results = tls_1_2_results[0], description
elif tls_1_3_results[0] is not None and tls_1_2_results[0] is not None:
elif tls_1_2_results[0] is not None and tls_1_3_results[0] is not None:
# Validate both results
description = (f"""TLS 1.2 {'' if tls_1_2_results[0] else 'not '}"""
f"""validated on port {port}: """
Expand Down
45 changes: 23 additions & 22 deletions testing/unit/tls/certs/_.google.com.crt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
-----BEGIN CERTIFICATE-----
MIIOCjCCDPKgAwIBAgIQQagVgnQLepAJtGHnFyDA9DANBgkqhkiG9w0BAQsFADA7
MIIOITCCDQmgAwIBAgIQO7VkQSFKhDkKyAs6+4C79DANBgkqhkiG9w0BAQsFADA7
MQswCQYDVQQGEwJVUzEeMBwGA1UEChMVR29vZ2xlIFRydXN0IFNlcnZpY2VzMQww
CgYDVQQDEwNXUjIwHhcNMjQxMDIxMDgzNjU3WhcNMjUwMTEzMDgzNjU2WjAXMRUw
EwYDVQQDDAwqLmdvb2dsZS5jb20wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAARD
7W/bU6abojd0puaRMYsiXqZjXddRl8yW2qlTpw+HOHg3bA183UxWTtZx+yeHSVQE
3k0jMGvR7C4B3FY+n92ao4IL9zCCC/MwDgYDVR0PAQH/BAQDAgeAMBMGA1UdJQQM
MAoGCCsGAQUFBwMBMAwGA1UdEwEB/wQCMAAwHQYDVR0OBBYEFIz1r4xlXzoNaxGY
xiGq0aGZuvJpMB8GA1UdIwQYMBaAFN4bHu15FdQ+NyTDIbvsNDltQrIwMFgGCCsG
CgYDVQQDEwNXUjIwHhcNMjUwMzIwMTExODM5WhcNMjUwNjEyMTExODM4WjAXMRUw
EwYDVQQDDAwqLmdvb2dsZS5jb20wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAARn
a7Tpjl88KC83iSzX7uGS2uAAmsikSJgnqD/stxmO2MBZKcqR6qb2Fa/Pa/+HmzP8
tUndvRvn46D3C0q6ENZmo4IMDjCCDAowDgYDVR0PAQH/BAQDAgeAMBMGA1UdJQQM
MAoGCCsGAQUFBwMBMAwGA1UdEwEB/wQCMAAwHQYDVR0OBBYEFK7Mj/WgPPFoPi4C
y2Fm28nqjCgtMB8GA1UdIwQYMBaAFN4bHu15FdQ+NyTDIbvsNDltQrIwMFgGCCsG
AQUFBwEBBEwwSjAhBggrBgEFBQcwAYYVaHR0cDovL28ucGtpLmdvb2cvd3IyMCUG
CCsGAQUFBzAChhlodHRwOi8vaS5wa2kuZ29vZy93cjIuY3J0MIIJzQYDVR0RBIIJ
xDCCCcCCDCouZ29vZ2xlLmNvbYIWKi5hcHBlbmdpbmUuZ29vZ2xlLmNvbYIJKi5i
CCsGAQUFBzAChhlodHRwOi8vaS5wa2kuZ29vZy93cjIuY3J0MIIJ5AYDVR0RBIIJ
2zCCCdeCDCouZ29vZ2xlLmNvbYIWKi5hcHBlbmdpbmUuZ29vZ2xlLmNvbYIJKi5i
ZG4uZGV2ghUqLm9yaWdpbi10ZXN0LmJkbi5kZXaCEiouY2xvdWQuZ29vZ2xlLmNv
bYIYKi5jcm93ZHNvdXJjZS5nb29nbGUuY29tghgqLmRhdGFjb21wdXRlLmdvb2ds
ZS5jb22CCyouZ29vZ2xlLmNhggsqLmdvb2dsZS5jbIIOKi5nb29nbGUuY28uaW6C
Expand Down Expand Up @@ -61,17 +61,18 @@ YmUuY29tghR5b3V0dWJlZWR1Y2F0aW9uLmNvbYIWKi55b3V0dWJlZWR1Y2F0aW9u
LmNvbYIPeW91dHViZWtpZHMuY29tghEqLnlvdXR1YmVraWRzLmNvbYIFeXQuYmWC
ByoueXQuYmWCGmFuZHJvaWQuY2xpZW50cy5nb29nbGUuY29tghMqLmFuZHJvaWQu
Z29vZ2xlLmNughIqLmNocm9tZS5nb29nbGUuY26CFiouZGV2ZWxvcGVycy5nb29n
bGUuY24wEwYDVR0gBAwwCjAIBgZngQwBAgEwNgYDVR0fBC8wLTAroCmgJ4YlaHR0
cDovL2MucGtpLmdvb2cvd3IyL29RNm55cjhGMG0wLmNybDCCAQQGCisGAQQB1nkC
BAIEgfUEgfIA8AB1AH1ZHhLheCp7HGFnfF79+NCHXBSgTpWeuQMv2Q6MLnm4AAAB
kq5v6a4AAAQDAEYwRAIgIqLlTU1VytekLFYe6z7B9QZvvAFEtlvZ/rQAndvx4BcC
IDhSWEOXYUPI6kr3vu50z40WJeAuKG8/FkmCKArIiuRJAHcAzxFW7tUufK/zh1vZ
aS6b6RpxZ0qwF+ysAdJbd87MOwgAAAGSrm/pugAABAMASDBGAiEA8kgns/UoNAL7
/3WsQF/6ifdMBOunXlADyPxvWbpRPe0CIQCQphqwbXlfq7EkCyUnzvMJmMs9PDoM
v7elVg36zqA5HjANBgkqhkiG9w0BAQsFAAOCAQEAOp1JjUVTIoBGg90DPzx1y/0N
3qu5UVDGbuuPlDFzHmjjnGb20C/s6pJzAuUxMvvIClk6m12PCbS/F6ESGYf9QzIU
BmU5BEiOHVSuTQrnDvUzrspB0eSe2qEqfDDvoaSb7MPX5kO7crHdBtG+DFUzUa/C
ZYsgbFW7or+ennD8oJT6NFCvPah7iIJwx5S1NIBPEot8IwzKBP9VrjVBhp9yXXr1
hWai/Z2gOacoJsLzbEifB+hlwldNgYwdSnVkTe6n5FZIygGxGm97Dp0v54g0Qs+U
4cfvwPIfx7D8oBz0EiCPlRm89TjTr0y8LqujdfiZdfbQQsUsNgsVNtIiiY0CSw==
bGUuY26CFSouYWlzdHVkaW8uZ29vZ2xlLmNvbTATBgNVHSAEDDAKMAgGBmeBDAEC
ATA2BgNVHR8ELzAtMCugKaAnhiVodHRwOi8vYy5wa2kuZ29vZy93cjIvb0JGWVlh
aHpnVkkuY3JsMIIBBAYKKwYBBAHWeQIEAgSB9QSB8gDwAHUAE0rfGrWYQgl4DG/v
THqRpBa3I0nOWFdq367ap8Kr4CIAAAGVs33VmgAABAMARjBEAiA1CilSlqONy6qX
P0bw1/9p9dA4ZFOATOsW8iD/8DjeigIgNeeb36KU09un4W7/asU13MgEKT0LE5z2
iaGd4w0fcckAdwBOdaMnXJoQwzhbbNTfP1LrHfDgjhuNacCx+mSxYpo53wAAAZWz
fdPFAAAEAwBIMEYCIQD2ZdfwECTSIFwy0PBq4aO/kfN5e3zLCYxrQ43KSHadiAIh
ALLHadjJzGnN1MGSKSoiXEPSELb6VS+P0am11M1XBujpMA0GCSqGSIb3DQEBCwUA
A4IBAQAJBz7LQ63ZDaAnJXbz5wroIj59Z5NYRAiiFfKipFPWsyQxDER0EoNFoQVv
BURN3hirNR9Qm42nt7NjqH3ATMu/xKJad7Th1Q4MgltTVP1AjGT5Qb52TkWpC48C
6ArYtoI/unK5VKT9RXSVCF8P/V+1wAs84e6qzxW1Sh4b8OkpuJRg3UqP29SJH59M
YPqKCUtMKWqivJOw9JrSgbn4phoN8Cl/WETPFUyA8mmAmT1TgLROCiJItGmBGltd
92fUmynwuyWmxvpOnwgQphERETUrFujDVRHPWctJcCQyqKBWcXEoKsyWFGt9G00Q
kWI1qr1XcZG0jBvFREo7xUpa1e10
-----END CERTIFICATE-----
Loading