Skip to content

Conversation

@agl
Copy link
Contributor

@aglagl 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
@mscdexmscdex 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

@nodejs/crypto

@shigeki
Copy link
Contributor

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

@indutny
Copy link
Member

LGTM, thank you @agl !

@shigeki
Copy link
Contributor

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

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

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

@shigeki ... is this appropriate for v4?

@shigeki
Copy link
Contributor

@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

Thank you @shigeki!

@agl
Copy link
ContributorAuthor

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

@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]>
@MylesBorinsMylesBorins 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 freeto 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++.tlsIssues and PRs related to the tls subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@agl@jasnell@shigeki@indutny@mscdex@MylesBorins