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

crypto: use SSL_CTX_clear_extra_chain_certs. #4919

Closed
wants to merge 1 commit into from

Conversation

agl
Copy link
Contributor

@agl agl commented Jan 27, 2016

The SSL_CTX_clear_extra_chain_certs function clears the extra
certificates associated with an SSL_CTX without reaching into the
SSL_CTX structure itself (which will become impossible in OpenSSL
1.1.0). The underlying implementation in OpenSSL[1] is the same what the
code was doing and OpenSSL has provided this function since 0.9.8 so
this change should be fully compatible.

[1] https://github.com/nodejs/node/blob/master/deps/openssl/openssl/ssl/s3_lib.c#L3899

The SSL_CTX_clear_extra_chain_certs function clears the extra
certificates associated with an SSL_CTX without reaching into the
SSL_CTX structure itself (which will become impossible in OpenSSL
1.1.0). The underlying implementation in OpenSSL[1] is the same what the
code was doing and OpenSSL has provided this function since 0.9.8 so
this change should be fully compatible.

[1] https://github.com/nodejs/node/blob/master/deps/openssl/openssl/ssl/s3_lib.c#L3899
@mscdex mscdex added tls Issues and PRs related to the tls subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. labels Jan 27, 2016
@jasnell
Copy link
Member

jasnell commented Jan 29, 2016

@nodejs/crypto

@shigeki
Copy link
Contributor

shigeki commented Jan 29, 2016

LGTM. It's good to know this before upgrading to1.1.0. Thanks.

@indutny
Copy link
Member

indutny commented Jan 29, 2016

LGTM, thank you @agl !

@shigeki
Copy link
Contributor

shigeki commented Jan 29, 2016

shigeki pushed a commit that referenced this pull request Feb 2, 2016
The SSL_CTX_clear_extra_chain_certs function clears the extra
certificates associated with an SSL_CTX without reaching into the
SSL_CTX structure itself (which will become impossible in OpenSSL
1.1.0). The underlying implementation in OpenSSL[1] is the same what the
code was doing and OpenSSL has provided this function since 0.9.8 so
this change should be fully compatible.

[1] https://github.com/nodejs/node/blob/master/deps/openssl/openssl/ssl/s3_lib.c#L3899

PR-URL: #4919
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
@shigeki
Copy link
Contributor

shigeki commented Feb 2, 2016

CI was done again after openssl upgrade. Arm failures were due to Jenkins errors, otherwise were green.
Landed in c3d5b2b. Thanks.

@shigeki shigeki closed this Feb 2, 2016
@jasnell
Copy link
Member

jasnell commented Feb 2, 2016

@shigeki ... is this appropriate for v4?

@shigeki
Copy link
Contributor

shigeki commented Feb 3, 2016

@jasnell This is not a bug fix but a kind of refactoring to prepare for the future upgrade to openssl-1.1.0. I don't think it is necessary to be backported to LTS.

@jasnell
Copy link
Member

jasnell commented Feb 3, 2016

Thank you @shigeki!

@agl
Copy link
Contributor Author

agl commented Feb 3, 2016

I would actually be interested in having the LTS release build with these sorts of OpenSSL but it's no big deal to carry the patch locally for us.

@indutny
Copy link
Member

indutny commented Feb 3, 2016

@agl @jasnell changed the labels then

rvagg pushed a commit that referenced this pull request Feb 8, 2016
The SSL_CTX_clear_extra_chain_certs function clears the extra
certificates associated with an SSL_CTX without reaching into the
SSL_CTX structure itself (which will become impossible in OpenSSL
1.1.0). The underlying implementation in OpenSSL[1] is the same what the
code was doing and OpenSSL has provided this function since 0.9.8 so
this change should be fully compatible.

[1] https://github.com/nodejs/node/blob/master/deps/openssl/openssl/ssl/s3_lib.c#L3899

PR-URL: #4919
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 17, 2016
The SSL_CTX_clear_extra_chain_certs function clears the extra
certificates associated with an SSL_CTX without reaching into the
SSL_CTX structure itself (which will become impossible in OpenSSL
1.1.0). The underlying implementation in OpenSSL[1] is the same what the
code was doing and OpenSSL has provided this function since 0.9.8 so
this change should be fully compatible.

[1] https://github.com/nodejs/node/blob/master/deps/openssl/openssl/ssl/s3_lib.c#L3899

PR-URL: #4919
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 18, 2016
The SSL_CTX_clear_extra_chain_certs function clears the extra
certificates associated with an SSL_CTX without reaching into the
SSL_CTX structure itself (which will become impossible in OpenSSL
1.1.0). The underlying implementation in OpenSSL[1] is the same what the
code was doing and OpenSSL has provided this function since 0.9.8 so
this change should be fully compatible.

[1] https://github.com/nodejs/node/blob/master/deps/openssl/openssl/ssl/s3_lib.c#L3899

PR-URL: #4919
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Feb 18, 2016
MylesBorins pushed a commit that referenced this pull request Mar 2, 2016
The SSL_CTX_clear_extra_chain_certs function clears the extra
certificates associated with an SSL_CTX without reaching into the
SSL_CTX structure itself (which will become impossible in OpenSSL
1.1.0). The underlying implementation in OpenSSL[1] is the same what the
code was doing and OpenSSL has provided this function since 0.9.8 so
this change should be fully compatible.

[1] https://github.com/nodejs/node/blob/master/deps/openssl/openssl/ssl/s3_lib.c#L3899

PR-URL: #4919
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
The SSL_CTX_clear_extra_chain_certs function clears the extra
certificates associated with an SSL_CTX without reaching into the
SSL_CTX structure itself (which will become impossible in OpenSSL
1.1.0). The underlying implementation in OpenSSL[1] is the same what the
code was doing and OpenSSL has provided this function since 0.9.8 so
this change should be fully compatible.

[1] https://github.com/nodejs/node/blob/master/deps/openssl/openssl/ssl/s3_lib.c#L3899

PR-URL: nodejs#4919
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants