Deprecate cass_cluster_set_use_hostname_resolution()#523
Merged
absurdfarce merged 3 commits intomasterfrom Mar 24, 2022
Merged
Conversation
added 2 commits
January 10, 2022 10:30
Add documentation that MITM is possible when using `CASS_SSL_VERIFY_PEER_IDENTITY_DNS` with `cass_cluster_set_use_hostname_resolution()`
absurdfarce
reviewed
Jan 10, 2022
| * This is useful for authentication (Kerberos) or encryption (SSL) services | ||
| * that require a valid hostname for verification. | ||
| * @deprecated Do not use. Using reverse DNS lookup to verify the certificate | ||
| * does not protect against man-in-the-middle attacks. |
Contributor
There was a problem hiding this comment.
Perhaps include mention of CPP-942 inline in comments for additional context?
Although it doesn't look like there's much reference to CPP tickets in comments elsewhere in the source so maybe not.
absurdfarce
reviewed
Jan 10, 2022
| * common name or one of its subject alternative names. This implies the | ||
| * certificate is also present. Hostname resolution must also be enabled. | ||
| * CASS_SSL_VERIFY_PEER_IDENTITY_DNS - Do not use. The requires the use of | ||
| * reverse DNS lookup which in not sufficient to protect against |
absurdfarce
reviewed
Jan 10, 2022
| * CASS_SSL_VERIFY_PEER_IDENTITY_DNS - Hostname matches the certificate's | ||
| * common name or one of its subject alternative names. This implies the | ||
| * certificate is also present. Hostname resolution must also be enabled. | ||
| * CASS_SSL_VERIFY_PEER_IDENTITY_DNS - Do not use. The requires the use of |
Contributor
There was a problem hiding this comment.
s/The/This/ or maybe s/The/This option/
absurdfarce
reviewed
Jan 10, 2022
| (reverse DNS) needs to be enabled: | ||
|
|
||
| **NOTE:** This is also disabled by default. | ||
| **Important:** This section use to suggest using reverse DNS lookup as a way to validate the peer's certificate i.e. using `CASS_SSL_VERIFY_PEER_IDENTITY_DNS` with `cass_cluster_set_use_hostname_resolution(cluster, cass_true)`. This is susceptible to man-in-the-middle (MITM) attacks and is no longer recommended. |
absurdfarce
approved these changes
Jan 10, 2022
Contributor
absurdfarce
left a comment
absurdfarce
left a comment
There was a problem hiding this comment.
A couple minor typo nits but otherwise this looks pretty good
Contributor
|
Something else just occurred to me a little bit ago: we probably still need a changelog mention for this even though it isn't actually changing (behaviour in) code, right? |
Contributor
|
Merging this in for now, will address the changelog update separately once we get closer to a release |
fsaporito
pushed a commit
to cloudian/cpp-driver
that referenced
this pull request
May 9, 2023
Deprecate the function and update docs accordingly
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add documentation that MITM is possible when using
CASS_SSL_VERIFY_PEER_IDENTITY_DNSwithcass_cluster_set_use_hostname_resolution()