Fix SSL verification fail bug when Kafka endpoint is a FQDN with a do… #4991
Conversation
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
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
joejulian
reviewed
Mar 14, 2025
Co-authored-by: Joe Julian <me@joejulian.name>
|
Hi team based on the discussion in openssl/openssl#11560 (comment) would it be reasonable to accept the change in behavior for ignoring the trailing . in the FQDN? Also see this here: #4348. Although folks have worked around this already it does seem like the proper behavior to follow. |
joejulian
approved these changes
Mar 28, 2025
|
@emasab Any chance we could get your take on this PR? This seems like valid behavior to have and check for in the client and doesn't sound like would break existing behavior. |
emasab
reviewed
May 30, 2025
Comment on lines
585
to
587
| char* dot = strrchr(rktrans->rktrans_rkb->rkb_nodename, '.'); | ||
| if (dot != NULL && dot[1] == ':') | ||
| memmove(dot, &dot[1], strlen(&dot[1]) + 1); |
Contributor
There was a problem hiding this comment.
It shouldn't modify the rkb_nodename but the change should be done in rd_kafka_transport_ssl_set_endpoint_id after copying the string as the name can be resolved again on rd_kafka_broker_connect
Contributor
|
@david-yu yes the change seems valid following RFC 6066 but only when using it as SNI, |
Author
|
Hi
I tried to move the dot removal in rd_kafka_transport_ssl_set_endpoint_id
function.
Using this patch it seems that everything work fine but I need someone else
test it:
diff --git a/src/rdkafka_ssl.c b/src/rdkafka_ssl.c
index 417da818..ae546af2 100644
--- a/src/rdkafka_ssl.c
+++ b/src/rdkafka_ssl.c
@@ -461,6 +461,9 @@ static int
rd_kafka_transport_ssl_set_endpoint_id(rd_kafka_transport_t *rktrans,
/* Remove ":9092" port suffix from nodename */
if ((t = strrchr(name, ':')))
*t = '\0';
+ if (name[strlen(name) - 1] == '.') {
+ name[strlen(name) - 1] = '\0';
+ }
#if (OPENSSL_VERSION_NUMBER >= 0x0090806fL) && !defined(OPENSSL_NO_TLSEXT)
/* If non-numerical hostname, send it for SNI */
…On Fri, May 30, 2025 at 11:01 AM Emanuele Sabellico < ***@***.***> wrote:
*emasab* left a comment (confluentinc/librdkafka#4991)
<#4991 (comment)>
@david-yu <https://github.com/david-yu> yes the change seems valid
following RFC 6066 but only when using it as SNI, rkb_nodename shouldn't
be changed in other places.
—
Reply to this email directly, view it on GitHub
<#4991 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACTF2FCXCQW3Y7LM4PEQQT3BAM5ZAVCNFSM6AAAAABYZGHXBSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSMRRGY4TQMRZGM>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
Author
|
Hi,
pull request updated.
…On Fri, May 30, 2025 at 3:36 PM Torello Querci ***@***.***> wrote:
Hi
I tried to move the dot removal in rd_kafka_transport_ssl_set_endpoint_id
function.
Using this patch it seems that everything work fine but I need someone
else test it:
diff --git a/src/rdkafka_ssl.c b/src/rdkafka_ssl.c
index 417da818..ae546af2 100644
--- a/src/rdkafka_ssl.c
+++ b/src/rdkafka_ssl.c
@@ -461,6 +461,9 @@ static int
rd_kafka_transport_ssl_set_endpoint_id(rd_kafka_transport_t *rktrans,
/* Remove ":9092" port suffix from nodename */
if ((t = strrchr(name, ':')))
*t = '\0';
+ if (name[strlen(name) - 1] == '.') {
+ name[strlen(name) - 1] = '\0';
+ }
#if (OPENSSL_VERSION_NUMBER >= 0x0090806fL) && !defined(OPENSSL_NO_TLSEXT)
/* If non-numerical hostname, send it for SNI */
On Fri, May 30, 2025 at 11:01 AM Emanuele Sabellico <
***@***.***> wrote:
> *emasab* left a comment (confluentinc/librdkafka#4991)
> <#4991 (comment)>
>
> @david-yu <https://github.com/david-yu> yes the change seems valid
> following RFC 6066 but only when using it as SNI, rkb_nodename shouldn't
> be changed in other places.
>
> —
> Reply to this email directly, view it on GitHub
> <#4991 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AACTF2FCXCQW3Y7LM4PEQQT3BAM5ZAVCNFSM6AAAAABYZGHXBSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSMRRGY4TQMRZGM>
> .
> You are receiving this because you modified the open/close state.Message
> ID: ***@***.***>
>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
I'm not able to connect to Kafka server when the BOOTSTRAP_BROKER is a FQDN with a dot at the end.
Different Kafka implementation provide endpoints with a dot at the end so you can use a BOOTSTRAP_BROKER without dot at the end but you ad not able to connect to the topic anyway.