Skip to content
This repository has been archived by the owner on Dec 14, 2020. It is now read-only.

Add SkipTLSNegotiation switch #9

Closed
wants to merge 1 commit into from
Closed

Add SkipTLSNegotiation switch #9

wants to merge 1 commit into from

Conversation

amenzhinsky
Copy link
Contributor

@amenzhinsky amenzhinsky commented Jan 21, 2018

Hi there.
Ran into issue that some brokers expect connection previously TLS encrypted before SASL/AMQP negotiation.

https://docs.microsoft.com/en-us/azure/service-bus-messaging/service-bus-amqp-protocol-guide

Azure Service Bus requires the use of TLS at all times. It supports connections over TCP port 5671, whereby the TCP connection is first overlaid with TLS before entering the AMQP protocol handshake, and also supports connections over TCP port 5672 whereby the server immediately offers a mandatory upgrade of connection to TLS using the AMQP-prescribed model. The AMQP WebSockets binding creates a tunnel over TCP port 443 that is then equivalent to AMQP 5671 connections.

But 5672 endpoint is missing for iothub AMQP brokers.
Dug into existing iouthub SDKs and figured out that amqp library for nodejs doesn't even implement TLS negotiation at all (what I find very strange).

Made it work this way, any comment on this?

@vcabbage
Copy link
Owner

Hi @amenzhinsky! Thanks for looking into this.

This is one of the scenarios amqp.New is for. It allows for establishing AMQP over an existing connection. I should add an example of it's usage.

It may be that the amqps scheme should automatically establish the TLS connection over port 5671 instead of toggling TLS negotiation within AMQP over port 5672. I need to do a little more research to verify that. If so, that can be done inside of Dial, before any of the AMQP negotiation happens.

In the mean time, here's an example using New. Please let me know if you have any issues with it.

host := "my-namespace.servicebus.windows.net"
tlsConn, err := tls.Dial("tcp", host+":5671", nil)
if err != nil {
	panic(err)
}

client, err := amqp.New(tlsConn,
	amqp.ConnSASLPlain("name", "key"),
	amqp.ConnServerHostname(host), // this important to communicate the hostname to the server
)

@amenzhinsky
Copy link
Contributor Author

@vcabbage thank you for the quick response, it's working for me. (didn't notice that a conn can be passed to New).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants