-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add OpenSSL::SSL::Context::Server#on_server_name for SNI
#16452
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
Add OpenSSL::SSL::Context::Server#on_server_name for SNI
#16452
Conversation
This adds Server Name Indication (SNI) callback support, allowing TLS servers
to present different certificates based on the hostname the client is connecting to.
The `set_sni_callback` method accepts a block that receives the client's requested
hostname and returns either a configured `SSL::Context::Server` for that hostname,
or `nil` to continue using the default context.
Example:
```crystal
default_context.set_sni_callback do |hostname|
case hostname
when "example.com"
example_context
else
nil
end
end
```
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Also use null pointer instead of nilable type for callback box, matching the existing pattern for @alpn_protocol. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Co-authored-by: Johannes Müller <[email protected]>
Co-authored-by: Johannes Müller <[email protected]>
Let exceptions propagate naturally instead of silently swallowing them. This is consistent with other OpenSSL callbacks in the same file (set_cert_verify_callback, alpn_protocol=) which don't have exception handling either. Exceptions will bubble up through the libssl stack and surface at the SSL_accept call, where they can be properly handled by Crystal code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
C callbacks must always return an error code, never raise - letting exceptions unwind through the C stack is unsafe as libssl cannot do its cleanup. On any exception in the SNI callback, set the alert pointer to SSL_AD_INTERNAL_ERROR and return SSL_TLSEXT_ERR_ALERT_FATAL. This lets OpenSSL handle the error properly and terminate the handshake with an appropriate alert. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Verifies that when the SNI callback raises an exception, the server returns an SSL error (via ALERT_FATAL) rather than crashing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
d56814d to
88773e2
Compare
Co-authored-by: Julien Portalier <[email protected]>
|
Apparently this is broken in the interpreter. Perhaps we could just skip the spec in interpreted mode? With a note that this leads to invalid memory access. And Crystal 1.0 doesn't seem to support the proc literal syntax. Might be best to use |
Co-authored-by: Sijawusz Pur Rahnama <[email protected]>
ysbaddaden
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a polish change to make sure the c callback is properly validated by the compiler (no closure data) when sent to the OpenSSL function.
This commit corrects the type signature for the SSL_CTX_callback_ctrl C binding to prevent memory safety issues related to garbage collection and closures. Co-authored-by: Julien Portalier <[email protected]>
OpenSSL::SSL::Context::Server#on_server_name for SNI
|
I have some ideas for improving this feature: #16494 |
|
We've started to notice |
|
I assume the issue comes from the captured block closure: it can have direct references to other contexts and trying to finalize one context leads the GC to identify a circular finalization... which isn't an issue because the finalizer only accesses the object itself. I wish there was a way to tell the GC to skip the check (unsafe finalizer). |
how about |
|
We already use Lines 338 to 343 in 37cdfa7
|
Crystal 1.19.0 includes it, which cause a conflict:
In src/stdlib/openssl_sni.cr:13:3
13 | SSL_CTRL_SET_TLSEXT_SERVERNAME_CB = 53
^--------------------------------
Error: already initialized constant LibSSL::SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
Replace custom openssl_sni.cr monkey-patch with the new
on_server_name callback added in crystal-lang/crystal#16452 by us
This has only been tested by existing specs.
Crystal 1.19.0 includes it, which cause a conflict:
In src/stdlib/openssl_sni.cr:13:3
13 | SSL_CTRL_SET_TLSEXT_SERVERNAME_CB = 53
^--------------------------------
Error: already initialized constant LibSSL::SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
Replace custom openssl_sni.cr monkey-patch with the new
on_server_name callback added in crystal-lang/crystal#16452 by us
This has only been tested by existing specs.
Crystal 1.19.0 includes it, which cause a conflict:
In src/stdlib/openssl_sni.cr:13:3
13 | SSL_CTRL_SET_TLSEXT_SERVERNAME_CB = 53
^--------------------------------
Error: already initialized constant LibSSL::SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
Replace custom openssl_sni.cr monkey-patch with the new
on_server_name callback added in crystal-lang/crystal#16452 by us
This has only been tested by existing specs.
Summary
on_server_namemethod toOpenSSL::SSL::Context::Serverfor Server Name Indication (SNI) supportExample
Test plan
on_server_namemethod in context_spec.cr🤖 Generated with Claude Code