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

docs: Explain DOH transfers inherit some SSL settings #6688

Closed
wants to merge 2 commits into from

Conversation

jay
Copy link
Member

@jay jay commented Mar 4, 2021

Closes #xxxx


Among other things this documents that CURLOPT_SSL_CTX_FUNCTION is inherited. Since that may be unexpected and it is not currently documented I am considering removing that behavior instead. This is similar to the discussion we had in #6605 about CURLOPT_DEBUGFUNCTION where the user may assume they only have to deal with their handle, and not internal DOH handles. The remedy, if there is actually demand for it, would be like I said in #6605 where we have a callback for advanced users that can be used to set DOH options.

Either way though I think we should document how CURLOPT_SSL_CTX_FUNCTION behaves, because it's possible the user may be doing some advanced verification or debugging through that function.

@jay jay added TLS documentation name lookup DNS and related tech labels Mar 4, 2021
@jay jay requested a review from bagder March 4, 2021 06:04
Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

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

Excellent!

@jay
Copy link
Member Author

jay commented Mar 5, 2021

but which way is excellent? Do you think we should remove that behavior where DOH handles inherit the CTX callback? I think that might be the right move.

@bagder
Copy link
Member

bagder commented Mar 5, 2021

I was primarily thinking of getting the current functionality documented. For the proposed change, I would feel much better if we'd have actual users chime in with feedback...

@jay
Copy link
Member Author

jay commented Mar 6, 2021

Ok. The SSL CTX callback is advanced and I think the number of users is probably very small. I have modified the proposed warning in the manpage to solicit feedback, let me know what you think:

WARNING: If you are using DNS-over-HTTPS (DOH) via CURLOPT_DOH_URL(3) then the CTX callback will also be called for those transfers and the curl handle is set to an internal handle. This behavior is subject to change. We recommend before performing your transfer set CURLOPT_PRIVATE(3) on your curl handle so you can identify it in the CTX callback. If you have a reason to modify DOH SSL context please let us know on the curl-library mailing list because we are considering removing this capability.

@arvids-kokins-bidstack
Copy link

I would feel much better if we'd have actual users chime in with feedback...

saw this linked in the other issue

as it happens, in addition to DoH we also use CURLOPT_SSL_CTX_FUNCTION to load bundled CA certs (though I don't recall the specifics at the moment, IIRC they're not provided by the OS on all platforms and this ensures that things work regardless of platform and device configuration)

I assume in our use case it would be necessary that the function is inherited by the DoH handle (for https to function at all), though also we do not use the CURL* handle provided in the callback in any way (only sslctx)

the same DoH handle setup callback mentioned before would probably work for us too

@jay jay marked this pull request as ready for review March 17, 2021 03:55
@jay jay closed this in 8a4ef73 Mar 17, 2021
@jay jay deleted the docs_explain_doh_inherits_from_ssl branch March 17, 2021 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants