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

ngtcp2: add thread safety to SSLKEYLOGFILE implementation #4311

Closed
wants to merge 1 commit into from

Conversation

jay
Copy link
Member

@jay jay commented Sep 9, 2019

  • Implement a thread-safe SSLKEYLOGFILE just like in openssl.c.

Follow-up to aae22fd which added SSLKEYLOGFILE support for ngtcp2.

Closes #xxxx


This is essentially a copy and paste from openssl.c. Untested since I have no http3 builds yet.

@jay jay added TLS HTTP/3 h3 or quic related labels Sep 9, 2019
- Implement a thread-safe SSLKEYLOGFILE just like in openssl.c.

Follow-up to aae22fd which added SSLKEYLOGFILE support for ngtcp2.

Closes #xxxx
@jay jay force-pushed the ngtcp2_threadsafe_sslkeylogfile branch from bee00d4 to 2c8934d Compare September 9, 2019 18:48
@@ -42,6 +42,8 @@
#include "curl_memory.h"
#include "memdebug.h"

#define ENABLE_SSLKEYLOGFILE
Copy link
Member

Choose a reason for hiding this comment

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

Why this?

Copy link
Member Author

Choose a reason for hiding this comment

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

MIrrored from openssl. I think I did that in openssl so it could be disabled for possible compliance issues? I really don't remember. I think it would be better to wrap in #ifndef DISABLE_SSLKEYLOGFILE , but if you prefer I'll remove it entirely.

Copy link
Member

Choose a reason for hiding this comment

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

I would rather imagine that it'd be within CURL_DISABLE_SSLKEYLOGFILE as then we could offer a disable option for configure in "normal" style.

I don't think it is needed for compliance since QUIC will only work with OpenSSL versions that also have the keylog support.

@bagder
Copy link
Member

bagder commented Sep 13, 2019

Maybe this should instead be made to use the same function instead of duplicating it, since they both work for OpenSSL?

@jay
Copy link
Member Author

jay commented Sep 14, 2019

If they are both using openssl wouldn't they both call the function or how does that work with ngtcp2? I assume not otherwise you wouldn't have added it. How about I move everything to sslkeylogfile.c

@bagder
Copy link
Member

bagder commented Sep 14, 2019

vtls/openssl.c is only used for TLS, vquic/ngtcp2.c is used for QUIC/h3 but both depend on OpenSSL.

How about I move everything to sslkeylogfile.c

Seems reasonable. The question is then probably where it belongs, since both vquic/ and vtls/ files would use it...

@stale
Copy link

stale bot commented Mar 12, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 12, 2020
@stale stale bot closed this Mar 26, 2020
@jay jay mentioned this pull request May 2, 2020
3 tasks
@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020
@jay jay deleted the ngtcp2_threadsafe_sslkeylogfile branch November 27, 2022 23:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
HTTP/3 h3 or quic related stale TLS
Development

Successfully merging this pull request may close these issues.

2 participants