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

Enable TLS SNI by setting peer_name to $host in $ssl_options #785

Merged
merged 2 commits into from Aug 17, 2020

Conversation

carlhoerberg
Copy link
Contributor

@carlhoerberg carlhoerberg commented Mar 31, 2020

Required if multiple AMQP servers are behind a TLS termintator/load-balancer.

@carlhoerberg carlhoerberg force-pushed the tls-sni branch 3 times, most recently from 257f718 to 125b6ba Compare March 31, 2020 20:36
@ramunasd
Copy link
Member

ramunasd commented Apr 2, 2020

Thanks a lot @carlhoerberg . I see those options can be provided via constructor, there is no need to define them for each secure connection. ATM we don't have automated tests for secure connection and this is the first time when somebody asks for this feature, also might be some backward incompatibility. Having all this i would like to keep it as it is now.

@ramunasd ramunasd self-assigned this Apr 2, 2020
@carlhoerberg
Copy link
Contributor Author

There's no backward incompatibility, all servers that doesn't have SNI enabled will simply ignore it, it's simply an extra field in the ClientHello package only. This change does not enable hostname verification for instance, that would be a braking change.

@carlhoerberg
Copy link
Contributor Author

Not setting the SNI header is like for web browser to not set the "Host" header, load balancers/proxys have no idea which host you actually want to reach, they can then only see which IP and port you connected to, so if you have multiple AMQP servers behind the same IP and port phpamqplib whould need a lot of hassling to get it to work, this fix will fix that, and for everybody else it's a noop.

@carlhoerberg
Copy link
Contributor Author

It also allows you to have multiple certificates for the same RabbitMQ server, which is something we see quite often. The server then knows which of the certificates to send back to the client.

@carlhoerberg
Copy link
Contributor Author

Relevant documentation: https://www.php.net/manual/en/context.ssl.php

@carlhoerberg
Copy link
Contributor Author

I've verified a number of AMQP libraries, if they have SNI enabled by default or not:

SNI enabled by default:

SNI not enabled by default:

@lukebakken lukebakken modified the milestones: 3.x.x, 2.12.0 Apr 2, 2020
@lukebakken
Copy link
Collaborator

Thanks @carlhoerberg

@lukebakken lukebakken merged commit a44b6df into php-amqplib:master Aug 17, 2020
ramunasd added a commit that referenced this pull request Sep 9, 2020
lukebakken added a commit that referenced this pull request Sep 14, 2020
ramunasd added a commit that referenced this pull request Sep 20, 2020
kratkyzobak pushed a commit to kratkyzobak/php-amqplib that referenced this pull request Feb 9, 2024
Enable TLS SNI by setting peer_name to $host in $ssl_options
kratkyzobak pushed a commit to kratkyzobak/php-amqplib that referenced this pull request Feb 9, 2024
kratkyzobak pushed a commit to kratkyzobak/php-amqplib that referenced this pull request Feb 9, 2024
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.

None yet

3 participants