michaelklishin / langohr

A small, feature complete Clojure client for RabbitMQ that embraces AMQP 0.9.1 model
http://clojurerabbitmq.info
355 stars 45 forks source link

(hopefully) invalid warning about TrustEverythingTrustManager #111

Open ghost opened 2 years ago

ghost commented 2 years ago

If logging for com.rabbitmq.client is enabled, this warning is printed when calling langohr.core/connect with :ssl true, even if I pass a custom :ssl-context:

SECURITY ALERT: this trust manager trusts every certificate, effectively disabling peer verification. This is convenient for local development but offers no protection against man-in-the-middle attacks. Please see https://www.rabbitmq.com/ssl.html to learn more about peer certificate verification.

That's from the rabbitmq-java-client library: https://github.com/rabbitmq/rabbitmq-java-client/blob/e32bcbb2824f7616a13acd5827a87ca92e54f08f/src/main/java/com/rabbitmq/client/TrustEverythingTrustManager.java#L34

I think it's an invalid logging statement a la what was observed for the JMS client in https://github.com/rabbitmq/rabbitmq-jms-client/issues/74 and fixed in https://github.com/rabbitmq/rabbitmq-jms-client/pull/75.

I just want to make sure that peer authentication is in fact enabled using the default SSLContext in my client application; the logging statement worries me.

michaelklishin commented 2 years ago

It's not enabled by default by Langohr. https://github.com/rabbitmq/rabbitmq-java-client/issues/229 and a few linked issues explain why enabling peer verification by default will be a usability disaster as well as a security improvement.

I don't see why your ssl-context would not be passed on to the Java client.

ghost commented 2 years ago

I think you are right that the ssl context is passed on and that peer verification settings are derived from there. However, this issue I meant to point out that the logs hint otherwise, which is misleading. I think it happens because this line https://github.com/michaelklishin/langohr/blob/master/src/clojure/langohr/core.clj#L342 uses com.rabbitmq.client.ConnectionFactory.useSslProtocol() with no args, and that instantiates a TrustEverythingManager that isn't used when an ssl-context is provided. I see it's a complex thing to addres, but it took me quite a lot of work to figure that out.