Apply ssl verify mode to the new sni context #1583

tuupola · 2025-12-29T18:24:48Z

Apparently with SNI the call to LibSSL.ssl_set_ssl_ctx() does not carry the verify_mode from new_context to the ssl object. This causes connections with bad client cert always succeed even if mTLS is configured.

This PR fixes the problem by calling LibSSL.ssl_set_verify() on the SSL connection to set the verify mode after changing the context.

Not sure if there is a better way to do this but atleast the fix works for me.

Configuration

I have the following config (domain changed to example.com):

[main]
data_dir = /var/lib/lavinmq
tls_cert = /etc/lavinmq/certs/server.crt
tls_key = /etc/lavinmq/certs/server.key
log_level = debug

[mqtt]
bind = 0.0.0.0
tls_port = 8883

[sni:mqtt.example.com]
tls_cert = /etc/lavinmq/certs/server.crt
tls_key = /etc/lavinmq/certs/server.key
mqtt_tls_verify_peer = true
mqtt_tls_ca_cert = /etc/lavinmq/certs/ca.crt

Behavior before patching

Before the patch connection succeeds both without client cert and with a bad client cert.

$ openssl s_client -connect mqtt.example.com:8883 -servername mqtt.example.com -CAfile ca.crt -quiet
Connecting to xx.xxx.28.46
depth=1 CN=Testing Root CA
verify return:1
depth=0 CN=mqtt.example.com
verify return:1
^C

$ openssl req -x509 -newkey rsa:2048 -keyout bad.key -out bad.crt -days 365 -nodes -subj "/CN=baaad-mmmkay"
$ openssl s_client -connect mqtt.example.com:8883 -servername mqtt.example.com -CAfile ca.crt -cert bad.crt -key bad.key -quiet
Connecting to xx.xxx.28.46
depth=1 CN=Testing Root CA
verify return:1
depth=0 CN=mqtt.example.com
verify return:1
^C

Connection succeeds with the correct client cert as expected.

$ openssl s_client -connect mqtt.example.com:8883 -servername mqtt.example.com -CAfile ca.crt -cert client.crt -key client.key -quiet
Connecting to xx.xxx.28.46
depth=1 CN=Testing Root CA
verify return:1
depth=0 CN=mqtt.example.com
verify return:1
^C

Behavior after patching

After applying this patch connection without client cert or with bad client cert fail as expected.

$ openssl s_client -connect mqtt.example.com:8883 -servername mqtt.example.com -CAfile ca.crt -quiet
Connecting to xx.xxx.28.46
depth=1 CN=Testing Root CA
verify return:1
depth=0 CN=mqtt.example.com
verify return:1
8042B0F1F67F0000:error:0A00045C:SSL routines:ssl3_read_bytes:tlsv13 alert certificate required:ssl/record/rec_layer_s3.c:909:SSL alert number 116

$ openssl s_client -connect mqtt.example.com:8883 -servername mqtt.example.com -CAfile ca.crt -cert bad.crt -key bad.key -quiet
Connecting to xx.xxx.28.46
depth=1 CN=Testing Root CA
verify return:1
depth=0 CN=mqtt.example.com
verify return:1
80E27D14757F0000:error:0A000418:SSL routines:ssl3_read_bytes:tlsv1 alert unknown ca:ssl/record/rec_layer_s3.c:909:SSL alert number 48

Connections with client cert still work as expected.

$ openssl s_client -connect mqtt.example.com:8883 -servername mqtt.example.com -CAfile ca.crt -cert client.crt -key client.key -quiet
Connecting to xx.xxx.28.46
depth=1 CN=Testing Root CA
verify return:1
depth=0 CN=mqtt.example.com
verify return:1
^C

github-actions · 2025-12-29T18:24:56Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

tuupola · 2025-12-29T18:28:14Z

I have read the CLA Document and I hereby sign the CLA

dentarg · 2025-12-29T18:37:44Z

Thanks for reporting this and writing up a fix! Do you think it is possible to cover this scenario in the specs?

tuupola · 2025-12-31T04:45:19Z

I am not too familiar with Crystal nor the codebase but I will give it a try. After all proper PR should contain tests :)

tuupola · 2025-12-31T09:45:09Z

I did not find any easier way to check. Tests are mostly copied from mtls_spec.cr.

dentarg · 2025-12-31T10:49:07Z

Thanks for trying. The team will take a look after the holidays. :)

Happy new year! 🎆

kickster97

LGTM, thanks for the contribution! :)

src/stdlib/openssl_sni.cr

viktorerlingsson · 2026-01-07T14:31:36Z

spec/sni_spec.cr

+
+    tcp_server.close
+    server_done.receive
+  end


I think the two tests in this test should be split up to separate tests, to make sure both scenarios always run and keep each test a little shorter

Yeah I agree. I just don't know Crystal well enough to make informed choices. I copied the code from other tests in sni_spec.cr and altered it slightly. I think it was copied from this:

lavinmq/spec/sni_spec.cr

Lines 222 to 315 in 26ded76

describe "SNI end-to-end" do

it "serves correct certificate based on SNI hostname" do

# Set up SNI manager with different certificates

sni_manager = LavinMQ::SNIManager.new

# Wildcard certificate for *.example.com

wildcard_host = LavinMQ::SNIHost.new("*.example.com")

wildcard_host.tls_cert = "spec/resources/wildcard_example_certificate.pem"

wildcard_host.tls_key = "spec/resources/wildcard_example_key.pem"

sni_manager.add_host(wildcard_host)

# Certificate for foobar.localhost (exact match)

foobar_host = LavinMQ::SNIHost.new("foobar.localhost")

foobar_host.tls_cert = "spec/resources/foobar_localhost_certificate.pem"

foobar_host.tls_key = "spec/resources/foobar_localhost_key.pem"

sni_manager.add_host(foobar_host)

# Default server context (for unmatched hostnames)

default_ctx = OpenSSL::SSL::Context::Server.new

default_ctx.certificate_chain = "spec/resources/server_certificate.pem"

default_ctx.private_key = "spec/resources/server_key.pem"

# Set up SNI callback

default_ctx.set_sni_callback do |hostname|

if sni_host = sni_manager.get_host(hostname)

sni_host.amqp_tls_context

else

nil

end

end

# Start TLS server

tcp_server = TCPServer.new("127.0.0.1", 0)

port = tcp_server.local_address.port

server_done = Channel(Nil).new

spawn do

3.times do

if client = tcp_server.accept?

begin

ssl_socket = OpenSSL::SSL::Socket::Server.new(client, default_ctx)

ssl_socket.close

rescue

# Ignore handshake errors in server

ensure

client.close

end

end

end

server_done.send(nil)

end

# Helper to create client context that trusts our self-signed certs

create_client_ctx = ->(cert_file : String) {

ctx = OpenSSL::SSL::Context::Client.new

ctx.verify_mode = OpenSSL::SSL::VerifyMode::PEER

ctx.ca_certificates = cert_file

ctx

}

# Test 1: Connect with wildcard hostname (test.example.com)

tcp_client1 = TCPSocket.new("127.0.0.1", port)

client_ctx1 = create_client_ctx.call("spec/resources/wildcard_example_certificate.pem")

begin

ssl_client1 = OpenSSL::SSL::Socket::Client.new(tcp_client1, client_ctx1, hostname: "test.example.com")

ssl_client1.close

ensure

tcp_client1.close

end

# Test 2: Connect with another wildcard subdomain (foo.example.com)

tcp_client2 = TCPSocket.new("127.0.0.1", port)

client_ctx2 = create_client_ctx.call("spec/resources/wildcard_example_certificate.pem")

begin

ssl_client2 = OpenSSL::SSL::Socket::Client.new(tcp_client2, client_ctx2, hostname: "foo.example.com")

ssl_client2.close

ensure

tcp_client2.close

end

# Test 3: Connect with exact match hostname (foobar.localhost)

tcp_client3 = TCPSocket.new("127.0.0.1", port)

client_ctx3 = create_client_ctx.call("spec/resources/foobar_localhost_certificate.pem")

begin

ssl_client3 = OpenSSL::SSL::Socket::Client.new(tcp_client3, client_ctx3, hostname: "foobar.localhost")

ssl_client3.close

ensure

tcp_client3.close

end

tcp_server.close

server_done.receive

end

👍
Yeah, that test should also be changed to 3 separate tests at some point IMO 🙂

viktorerlingsson · 2026-01-07T14:32:15Z

Added a couple of comments, besides that I think it looks good! 👍

Apply verify mode to the new context

941e477

tuupola requested a review from a team as a code owner December 29, 2025 18:24

tuupola changed the title ~~Apply verify mode to the new context~~ Apply ssl verify mode to the new sni context Dec 29, 2025

lavinmq-user-84 added a commit to cloudamqp/CLA-signatures that referenced this pull request Dec 29, 2025

@tuupola has signed the CLA in cloudamqp/lavinmq#1583

4eb5017

Add test for make sure verify mode is copied

4cee8ae

kickster97 approved these changes Jan 7, 2026

View reviewed changes

viktorerlingsson reviewed Jan 7, 2026

View reviewed changes

src/stdlib/openssl_sni.cr Outdated Show resolved Hide resolved

viktorerlingsson reviewed Jan 7, 2026

View reviewed changes

Use nil instead of Pointer(Void).null

dba8f1e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Apply ssl verify mode to the new sni context #1583

Apply ssl verify mode to the new sni context #1583

Uh oh!

tuupola commented Dec 29, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Dec 29, 2025 •

edited

Loading

Uh oh!

tuupola commented Dec 29, 2025

Uh oh!

dentarg commented Dec 29, 2025

Uh oh!

tuupola commented Dec 31, 2025

Uh oh!

tuupola commented Dec 31, 2025

Uh oh!

dentarg commented Dec 31, 2025

Uh oh!

kickster97 left a comment

Uh oh!

Uh oh!

viktorerlingsson Jan 7, 2026

Uh oh!

tuupola Jan 11, 2026

Uh oh!

viktorerlingsson Jan 12, 2026

Uh oh!

viktorerlingsson commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	describe "SNI end-to-end" do
	it "serves correct certificate based on SNI hostname" do
	# Set up SNI manager with different certificates
	sni_manager = LavinMQ::SNIManager.new

	# Wildcard certificate for *.example.com
	wildcard_host = LavinMQ::SNIHost.new("*.example.com")
	wildcard_host.tls_cert = "spec/resources/wildcard_example_certificate.pem"
	wildcard_host.tls_key = "spec/resources/wildcard_example_key.pem"
	sni_manager.add_host(wildcard_host)

	# Certificate for foobar.localhost (exact match)
	foobar_host = LavinMQ::SNIHost.new("foobar.localhost")
	foobar_host.tls_cert = "spec/resources/foobar_localhost_certificate.pem"
	foobar_host.tls_key = "spec/resources/foobar_localhost_key.pem"
	sni_manager.add_host(foobar_host)

	# Default server context (for unmatched hostnames)
	default_ctx = OpenSSL::SSL::Context::Server.new
	default_ctx.certificate_chain = "spec/resources/server_certificate.pem"
	default_ctx.private_key = "spec/resources/server_key.pem"

	# Set up SNI callback
	default_ctx.set_sni_callback do \|hostname\|
	if sni_host = sni_manager.get_host(hostname)
	sni_host.amqp_tls_context
	else
	nil
	end
	end

	# Start TLS server
	tcp_server = TCPServer.new("127.0.0.1", 0)
	port = tcp_server.local_address.port

	server_done = Channel(Nil).new

	spawn do
	3.times do
	if client = tcp_server.accept?
	begin
	ssl_socket = OpenSSL::SSL::Socket::Server.new(client, default_ctx)
	ssl_socket.close
	rescue
	# Ignore handshake errors in server
	ensure
	client.close
	end
	end
	end
	server_done.send(nil)
	end

	# Helper to create client context that trusts our self-signed certs
	create_client_ctx = ->(cert_file : String) {
	ctx = OpenSSL::SSL::Context::Client.new
	ctx.verify_mode = OpenSSL::SSL::VerifyMode::PEER
	ctx.ca_certificates = cert_file
	ctx
	}

	# Test 1: Connect with wildcard hostname (test.example.com)
	tcp_client1 = TCPSocket.new("127.0.0.1", port)
	client_ctx1 = create_client_ctx.call("spec/resources/wildcard_example_certificate.pem")
	begin
	ssl_client1 = OpenSSL::SSL::Socket::Client.new(tcp_client1, client_ctx1, hostname: "test.example.com")
	ssl_client1.close
	ensure
	tcp_client1.close
	end

	# Test 2: Connect with another wildcard subdomain (foo.example.com)
	tcp_client2 = TCPSocket.new("127.0.0.1", port)
	client_ctx2 = create_client_ctx.call("spec/resources/wildcard_example_certificate.pem")
	begin
	ssl_client2 = OpenSSL::SSL::Socket::Client.new(tcp_client2, client_ctx2, hostname: "foo.example.com")
	ssl_client2.close
	ensure
	tcp_client2.close
	end

	# Test 3: Connect with exact match hostname (foobar.localhost)
	tcp_client3 = TCPSocket.new("127.0.0.1", port)
	client_ctx3 = create_client_ctx.call("spec/resources/foobar_localhost_certificate.pem")
	begin
	ssl_client3 = OpenSSL::SSL::Socket::Client.new(tcp_client3, client_ctx3, hostname: "foobar.localhost")
	ssl_client3.close
	ensure
	tcp_client3.close
	end

	tcp_server.close
	server_done.receive
	end

Apply ssl verify mode to the new sni context #1583

Are you sure you want to change the base?

Apply ssl verify mode to the new sni context #1583

Uh oh!

Conversation

tuupola commented Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Configuration

Behavior before patching

Behavior after patching

Uh oh!

github-actions bot commented Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tuupola commented Dec 29, 2025

Uh oh!

dentarg commented Dec 29, 2025

Uh oh!

tuupola commented Dec 31, 2025

Uh oh!

tuupola commented Dec 31, 2025

Uh oh!

dentarg commented Dec 31, 2025

Uh oh!

kickster97 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

viktorerlingsson Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

tuupola Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

viktorerlingsson Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

viktorerlingsson commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tuupola commented Dec 29, 2025 •

edited

Loading

github-actions bot commented Dec 29, 2025 •

edited

Loading