Skip to content

Comments

PyPy's ssl module, the last missing macro and three macro functions#3270

Merged
reaperhulk merged 3 commits intopyca:masterfrom
planrich:master
Nov 21, 2016
Merged

PyPy's ssl module, the last missing macro and three macro functions#3270
reaperhulk merged 3 commits intopyca:masterfrom
planrich:master

Conversation

@planrich
Copy link
Contributor

Here are the last missing macro/functions needed. I'm not sure why sk_GENERAL_NAME_freefunc has the type with struct GENERAL_NAME_st* instead of GENERAL_NAME*. Without that change I could not pass GENERAL_NAME_free to an argument requiring a sk_GENERAL_NAME_freefunc type.

@mention-bot
Copy link

@planrich, thanks for your PR! By analyzing the history of the files in this pull request, we identified @reaperhulk, @lvh and @cyli to be potential reviewers.

@alex
Copy link
Member

alex commented Nov 21, 2016

Jenkins, ok to test.

@alex alex added the bindings label Nov 21, 2016

static const long Cryptography_HAS_SSL_CTX_CLEAR_OPTIONS = 1;

#ifdef OPENSSL_NO_TLSEXT
Copy link
Member

Choose a reason for hiding this comment

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

cryptography doesn't support OpenSSL compiled without TLS extensions so you can always assume this is not set. Do you still need it then?

Copy link
Contributor Author

@planrich planrich Nov 21, 2016

Choose a reason for hiding this comment

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

We use e.g. SSL_CTX_set_tlsext_servername_callback, I'm not sure if that is to be called with the macro OPENSSL_NO_TLSEXT defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rethinking that, I'm fine with removing it... But this means PyPy cannot support any openssl version that would set the macro?

Copy link
Member

Choose a reason for hiding this comment

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

Correct. By using cryptography as the basis of the module you're bound by the versions we've chosen to support, but OPENSSL_NO_TLSEXT is extremely uncommon and in practice we've never had a single bug report complaining about not supporting that configuration.


int sk_X509_OBJECT_num(Cryptography_STACK_OF_X509_OBJECT *);
X509_OBJECT *sk_X509_OBJECT_value(Cryptography_STACK_OF_X509_OBJECT *, int);
X509_VERIFY_PARAM * X509_STORE_get0_param(X509_STORE *);
Copy link
Member

Choose a reason for hiding this comment

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

Style nit, this should be X509_VERIFY_PARAM *X509_STORE_get0_param(X509_STORE *); (space removed before function name)

@reaperhulk reaperhulk merged commit ca4f79e into pyca:master Nov 21, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Development

Successfully merging this pull request may close these issues.

4 participants