[XrdCrypto] Generate DH parameters on first call to XrdCryptosslCipher #1595
Conversation
|
Looks good, thanks JT. Let me know when it passes your tests and I will merge it. |
|
Please note that this PR updates a file that exists in both an old pre openssl 3 version src/XrdCrypto/XrdCryptosslCipher.cc and a new openssl 3 compatible version src/XrdCrypto/openssl3/XrdCryptosslCipher.cc Until the new code (that also works on older openssl versions) replaces the old code, all changes must be done to both versions to keep them in sync. |
|
Yes, we are aware of this. We are not merging this until a) final tests have been performed, and b) the openssl3 update is included in the pr. This is just a preliminary pr. |
|
At Nebraska, we've been running with this patch in production on our local redirector for about 27 hours now. Redirector CPU use looks OK, CMS ETF and HammerCloud look OK, and we haven't received reports of issues. Since things look promising, I'll look at getting the equivalent openssl3 changes into this PR. Though at present, we don't have a suitable environment for testing. |
|
Sounds good. I think we can manage verifying the SSL3 part with our Ubuntu collaborators. |
- Move DH parameter generation inside server scope
- Avoid overhead of generating DH parameters on client
- Remove DH_check() call on server side
- Adds overhead, and the server-generated key should already be safe
- Include cassert instead of assert.h
|
This adds openssl3 code that should be equivalent to the pre-openssl3 code. I'd greatly appreciate feedback from @ellert. It also adds a bit of cleanup. Time permitting, I'll put it into production at Nebraska tomorrow. |
|
@ellert Could you check if the openssl3 changes are not problematic? We really need to push this out ASAP. |
|
You don't have to port anything the PR already contains the port for
openssl3. We just wanted to make sure it was OK as we can't even compile
it. So, I will merge it and we go from there.
…On Tue, 25 Jan 2022, simonmichal wrote:
@abh3 : if the PR is ready to be merged and @ellert don't have time to have a look at it, then please go ahead with the merge and once done I will port the changes to openssl3
--
Reply to this email directly or view it on GitHub:
#1595 (comment)
You are receiving this because you are subscribed to this thread.
Message ID: ***@***.***>
########################################################################
Use REPLY-ALL to reply to list
To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1
|
This should address the increased load issue discussed in #1556. I've very lightly tested this change in production, and it seems OK. I'm planning more thorough tests tomorrow.
We'll also need to update
openssl3/XrdCryptosslCipher.ccto match, but it should be fairly straightforward.