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

Strip trailing dot off hostname used for SSL validation. #2374

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

voutilad
Copy link

@voutilad voutilad commented Jul 12, 2023

When using FQDN's with trailing dots to connect to a broker, the trailing dot in the hostname should be stripped before using the hostname for validation against the server's certificate.

For instance, this code:

producer = KafkaProducer(
        bootstrap_servers="redpanda.redpanda.svc.cluster.local.:9093",
        security_protocol="SSL",
)

Will cause certificate validation to fail when the subject name doesn't contain the trailing dot.

For some context, this is also a problem with librdkafka: confluentinc/librdkafka#4348

The current stance from the OpenSSL team is this is an application layer issue: openssl/openssl#11560


This change is Reviewable

@voutilad
Copy link
Author

For additional context, this was raised to the Python project and was decided that it, too, is a problem for the application layer: python/cpython#76178

@wbarnha wbarnha self-requested a review August 8, 2023 13:44
Copy link
Collaborator

@wbarnha wbarnha left a comment

Choose a reason for hiding this comment

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

I'm cautious about adding an invocation of rstrip(".") to each hostname attached to the socket. I think this should be the responsibility of users to add this logic. Sure, it makes sense to remove dots from the end of all hostnames, but I don't think it should be the library's responsibility to do so. The library assumes an arbitrary hostname is passed in, not specifically FQDNs.

@voutilad
Copy link
Author

voutilad commented Aug 8, 2023

Sure, it makes sense to remove dots from the end of all hostnames, but I don't think it should be the library's responsibility to do so.

The issue here is self.host is used in 2 different capacities with different semantics:

  1. passed to socket.getaddrinfo by the dns_lookup function
  2. passed to the wrap_socket method of ssl.SSLContext

There are reasons to an FQDN (terminated with .) to the DNS resolver via the getaddrinfo call. One reason may be for performance.

As for SSLContext, the trailing dot shouldn't be used. See RFC 6066's comment in Section 3: Server Name Indication:

"HostName" contains the fully qualified DNS hostname of the server,
as understood by the client. The hostname is represented as a byte
string using ASCII encoding without a trailing dot.

A real-world example is seen in the .NET Runtime: dotnet/runtime#62540

My stance is for getaddrinfo, a dot is valid. For wrap_socket and passing it to SSLContext, it's invalid. Since there's only 1 input from the application to the library, the library must do the right thing because it's impossible for the application to do so.

Copy link
Collaborator

@wbarnha wbarnha left a comment

Choose a reason for hiding this comment

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

I see now, thank you for the clarification! LGTM.

@wbarnha
Copy link
Collaborator

wbarnha commented Aug 8, 2023

Just a note: I'm waiting to get v2.0.3 released before merging this.

wbarnha and others added 14 commits March 7, 2024 10:31
…terations for Kafka 0.8.2 and Python 3.12 (dpkp#159)

* skip failing tests for PyPy since they work locally

* Reconfigure tests for PyPy and 3.12

* Skip partitioner tests in test_partitioner.py if 3.12 and 0.8.2

* Update test_partitioner.py

* Update test_producer.py

* Timeout tests after ten minutes

* Set 0.8.2.2 to be experimental from hereon

* Formally support PyPy 3.9
* Test Kafka 0.8.2.2 using Python 3.11 in the meantime

* Override PYTHON_LATEST conditionally in python-package.yml

* Update python-package.yml

* add python annotation to kafka version test matrix

* Update python-package.yml

* try python 3.10
* Remove support for EOL'ed versions of Python

* Update setup.py
Too many MRs to review... so little time.
After stop/start kafka service, kafka-python may use 100% CPU caused by
busy-retry while the socket was closed. This fix the issue by unregister
the socket if the fd is negative.

Co-authored-by: Orange Kao <[email protected]>
The former has been deprecated since setuptools 56

Co-authored-by: micwoj92 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants