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

"LDConfigSetSSLCertificateAuthority" and "LDConfigSetVerifyPeer" are not available from 3.0. #385

Open
ngangomsamananda opened this issue Apr 2, 2024 · 18 comments
Labels
enhancement New feature or request package: sdk/client Issues affecting the C++ Client SDK

Comments

@ngangomsamananda
Copy link

ngangomsamananda commented Apr 2, 2024

Is your feature request related to a problem? Please describe.
We are updating client sdk from v2.5.2 to v3.4.0 (c binding) but the equivalent replacement of LDConfigSetSSLCertificateAuthority and LDConfigSetVerifyPeer are not available.

  • LDConfigSetSSLCertificateAuthority: Set the path to the SSL certificate bundle used for peer authentication.

  • LDConfigSetVerifyPeer : Set whether to verify the authenticity of the peer's certificate on network requests.

    Describe the solution you'd like
    Please provide the replacement for LDConfigSetSSLCertificateAuthority and LDConfigSetVerifyPeer.

    Describe alternatives you've considered
    A clear and concise description of any alternative solutions or features you've considered.

    Additional context
    Add any other context about the feature request here.

@ngangomsamananda ngangomsamananda added the package: sdk/client Issues affecting the C++ Client SDK label Apr 2, 2024
@cwaldren-ld cwaldren-ld added the enhancement New feature or request label Apr 2, 2024
@cwaldren-ld
Copy link
Contributor

Hi @ngangomsamananda, you are correct, these are not available in the SDK at the moment.

I have filed an internal feature request.

Filed internally as 238927.

@ngangomsamananda
Copy link
Author

@cwaldren-ld Thank you.
How long does it take a feature request to complete? Will it be available in the next SDK release?

@cwaldren-ld
Copy link
Contributor

Hi @ngangomsamananda , I cannot provide an estimate, but I will address it as soon as I am able.

@ngangomsamananda
Copy link
Author

Hi @cwaldren-ld, is there any particular reason why LDConfigSetSSLCertificateAuthority and LDConfigSetVerifyPeer are not included in v3.x?
Could you please take up this issue on priority than the other tickets (#392 and #394) which I had requested? We are not able to complete the sdk update because of these APIs.

@cwaldren-ld
Copy link
Contributor

cwaldren-ld commented Apr 17, 2024 via email

@ngangomsamananda
Copy link
Author

@cwaldren-ld No worries. In one of the tickets, you had mentioned you will be out of office this week. I have added the comment so that you can have a look when you come back.

@cwaldren-ld
Copy link
Contributor

Hi @ngangomsamananda , the reason these APIs were not included was because we didn't have a proven use-case for them.

Since you have requested them, I have begun work here: #391

I will update you when the work is complete.

@ngangomsamananda
Copy link
Author

@cwaldren-ld Thank you so much.

@cwaldren-ld
Copy link
Contributor

Hi @ngangomsamananda , I apologize this is taking longer than expected. I am still working on it.

The LDConfigSetVerifyPeer will be released first, then LDConfigSetSSLCertificateAuthority will come after.

@ngangomsamananda
Copy link
Author

@cwaldren-ld Let me know when LDConfigSetVerifyPeer is release. I appreciate you for working this on priority.

@cwaldren-ld
Copy link
Contributor

cwaldren-ld commented May 14, 2024

Hi @ngangomsamananda , we have released C++ Client v3.5.0 with support for disabling Peer Verification in TLS handshake.

Here is a link to the docs.

Example:

// First, create a new TLS Config builder
LDClientHttpPropertiesTlsBuilder tls_builder = LDClientHttpPropertiesTlsBuilder_New();

// Disable peer verification 
LDClientHttpPropertiesTlsBuilder_SkipVerifyPeer(tls_builder, true);

// Assuming you have an existing LDClientConfigBuilder
LDClientConfigBuilder_HttpProperties_Tls(config, tls_builder);

Please note, the SDK will emit logs informing you that TLS peer verification is disabled, since it can be considered a security flaw in many scenarios. You can disregard the log message if you use this feature.

@ngangomsamananda
Copy link
Author

@cwaldren-ld Thank you so much. I will check and let you know.
One thing: is LDClientHttpPropertiesTlsBuilder_Free() need to be call? I saw this in the docs you have shared.

@cwaldren-ld
Copy link
Contributor

cwaldren-ld commented May 14, 2024

Not normally.

If you pass the TLS builder into the HttpPropertiesBuilder, there is no need to call LDClientHttpPropertiesTlsBuilder_Free() (since the ConfigBuilder takes ownership of it.)

If you do not pass it into the HttpPropertiesBuilder, then yes you must call LDClientHttpPropertiesTlsBuilder_Free() to avoid memory leak.

Relevant part from the docs on LDClientHttpPropertiesTlsBuilder_Free():

Frees a TLS options builder. Do not call if the builder was consumed by the HttpProperties builder.

@ngangomsamananda
Copy link
Author

Hi @cwaldren-ld , till now the replacement for LDConfigSetVerifyPeer appears to be working fine. If I face any issue, I will reach out to you. Thanks

@cwaldren-ld
Copy link
Contributor

cwaldren-ld commented May 28, 2024

Hi @ngangomsamananda , please check out #409 branch (cw/sc-244781/custom-ca) when you have a chance and test the new API:

// First, create a new TLS Config builder
LDClientHttpPropertiesTlsBuilder tls_builder = LDClientHttpPropertiesTlsBuilder_New();

// Set path to a custom CA file
LDClientHttpPropertiesTlsBuilder_CustomCAFile(tls_builder, "path/to/your/custom_ca.pem");

// Assuming you have an existing LDClientConfigBuilder
LDClientConfigBuilder_HttpProperties_Tls(config, tls_builder);

If you are unable to test the branch, please wait for the upcoming release.

@ngangomsamananda
Copy link
Author

@cwaldren-ld I will do the testing when it is release. Thanks for working on that API.

@cwaldren-ld
Copy link
Contributor

@ngangomsamananda
Copy link
Author

Hi @cwaldren-ld, thank you so much for providing the new API's in short time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request package: sdk/client Issues affecting the C++ Client SDK
Projects
None yet
Development

No branches or pull requests

2 participants