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

SSL_CTX_set_ciphersuites for tlsv1.3 context #1292

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

BillyGLW
Copy link

@BillyGLW BillyGLW commented Feb 3, 2024

Linking: #1224

As cffi binding allows to use SSL_CTX_set_ciphersuites - adaptation for setting the ciphers in context of TLSv1.3 can be made.

@BillyGLW BillyGLW changed the title SSL_CTX_set_ciphersuites for tlsv3 context SSL_CTX_set_ciphersuites for tlsv1.3 context Feb 5, 2024
@@ -1320,6 +1320,26 @@ def set_tmp_ecdh(self, curve):
"""
_lib.SSL_CTX_set_tmp_ecdh(self._context, curve._to_EC_KEY())

def set_ciphersuites(self, cipher_list):
Copy link
Contributor

Choose a reason for hiding this comment

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

def set_ciphersuites(self, cipher_list: bytes) -> None :)

Copy link
Author

Choose a reason for hiding this comment

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

Done

@jlaine
Copy link
Contributor

jlaine commented Jun 24, 2024

Isn't it a bit unusual to hijack the test of another method, just because the two methods require bytes as input?

@BillyGLW
Copy link
Author

Isn't it a bit unusual to hijack the test of another method, just because the two methods require bytes as input?

for me it would make more sense if there would be additional test that calls both of these methods simultaneously (as it is intended), so that parametrized ones can test it individually based on bytes input and context that changes (or raises exception due to typerror).. but it also require test_set_ciphersuites to be parametrized. So, i also think that in current form it will be better to separate them

@BillyGLW BillyGLW force-pushed the main branch 3 times, most recently from 4998df7 to f84b711 Compare June 25, 2024 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants