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.

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

Make sure certificate is verified when required by RestApiClientAsync #1620

Merged
merged 16 commits into from
Nov 27, 2023
Merged

Make sure certificate is verified when required by RestApiClientAsync #1620

Changes from 10 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
d7310cb
Make sure certificate is verified when required
mdazam1942 Nov 21, 2023
1e9c0ae
updating ssl context codition
mdazam1942 Nov 21, 2023
1f56d9f
unittest failure debug print
mdazam1942 Nov 21, 2023
c5c3079
remove prints
mdazam1942 Nov 21, 2023
76c078a
set certificate check to false for stix bundle
mdazam1942 Nov 21, 2023
521ab2a
ssl verify mode update:
mdazam1942 Nov 21, 2023
4380c0d
revert back verify optional
mdazam1942 Nov 21, 2023
2ea9106
cert is only loaded if present otherwise set default SSL check
mdazam1942 Nov 23, 2023
221d663
Merge branch 'develop' into certificate_verification_fix
mdazam1942 Nov 23, 2023
78f9e7e
rever cert verify for stix bundle
mdazam1942 Nov 23, 2023
23d17ca
change default ssl_context
mdazam1942 Nov 23, 2023
1d6973b
update on ssl context
mdazam1942 Nov 24, 2023
aae8440
Fix unittests, removed boolean default value for certificate and adde…
mdazam1942 Nov 24, 2023
b20913b
fix unittests
mdazam1942 Nov 27, 2023
ea23a0b
fix unittest
mdazam1942 Nov 27, 2023
aadcc83
fix unittest2
mdazam1942 Nov 27, 2023
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
10 changes: 8 additions & 2 deletions stix_shifter_utils/stix_transmission/utils/RestApiClientAsync.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,15 @@ def __init__(self, host, port=None, headers={}, url_modifier_function=None, cert

self.server_cert_file_content = None
self.ssl_context = False

if isinstance(cert_verify, bool):
# verify certificate non self signed case
if cert_verify:
self.ssl_context = True
# self signed cert provided
elif isinstance(cert_verify, str):
self.server_cert_file_content = cert_verify
self.ssl_context = True

self.headers = headers
self.url_modifier_function = url_modifier_function
Expand All @@ -87,8 +88,13 @@ async def call_api(self, endpoint, method, headers=None, cookies=None, data=None
except IOError:
self.logger.error('Failed to setup certificate')

if self.ssl_context and self.server_cert_file_content:
self.ssl_context = ssl.SSLContext(ssl.PROTOCOL_TLS)
self.ssl_context.load_verify_locations(self.server_cert_name)
try:
self.ssl_context.load_verify_locations(self.server_cert_name)
except Exception as ex:
self.logger.debug('Unable to load the certificate for ssl context. Reasons: Connection does not require certificate or unexpected exception while loading the certificate: ' + str(ex))

self.ssl_context.verify_mode = ssl.CERT_REQUIRED
Copy link

Choose a reason for hiding this comment

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

verify_mode and check_hostname cannot be conditional they must be applied even in the case of using a default ssl context but I don't see a call to ssl.create_default_context() any where.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated the code to remove ambiguity around certificate validation. now certificate is verified in both custom or default cases. also remove the option to force validation to false.

self.ssl_context.check_hostname = True

Expand Down
Loading