We don't need the CSR in the repo I think.

I would go for some slightly more verbose add_sni_hostname. Documentation would be great too, especially since having to pass a Context::Server to a Context::Server quickly gets confusing.

Also how about a variant that takes multiple hostnames? Since at least the bare and the www hostname in the same certificate is very common.

You can group accessors, type and documentation in a single definition, and @sni is an internal detail (the public API is #add_sni_hostname):

# :nodoc:
getter! sni : Hash(String, Server)

# ...
property sni_fail_hard = false

Furthermore you shouldn't expose @ctxbox, it's an unsafe internal detail. It probably doesn't have to be typed either.

Got it. Where should I place developer notes? If I come back to this in six months, I want to know that, say, child contexts shouldn't have @sni set.

Maybe set_tlsext_servername_callback itself could start with return if @sni?

I'm not sure I understand sni_fail_hard. If true then it will accept any hostname, which is weird.

Should OK and NOACK be reversed or should OK be something else?

fixed in latest push. I wasn't initializing the contexts correctly throughout the spec, and that coupled with the sni_fail_ahrd issue let tests pass. sni_fail_hard, if set, should immediately throw an error to any SSL client connecting (see any of cloudflares sites).

Maybe avoid a double check:

if hostname_ptr
  hostname = String.new(hostname_ptr)
  # ...
end

The documentation will be rendered as one big paragraph. A bit a markdown information would help.

I hope this is replying to the right comment. Regarding having self as the default context, that's what the current functionality does. SNI just allows different hostnames to be added, and with each new hostname comes a new context that the client will be attached to. I can reexplain this if I've botched the explanation.

Maybe use Enumerable(String) instead of Array(String) so the method would accept more collection types.

please, use snake_case (ditto all below) instead of camelCase.

Thanks. I reread the style guide again. Don't know where I got this idea from.

I'm not sure these two branches should be the same interface, that is for an interface that asks me for hostnames explicitly I would expect the certificate to only be presented for that exact hostname.

On the other hand being able to just give a bunch of certificates and have the valid hostnames figured out from that is quite useful, so a separate interface for that does make sense to me. That should then clearly document that in case of certificates with overlapping hostnames, say you add one for example.org and example.com and one for example.org and example.net, the first one that matches win.

Finally I think mixing both approaches should also work but still a certificate that was added via the "figure the hostnames from it" interface shouldn't be presented for an explicitly given hostname, likewise an explicitly given hostname's certificate should always win over the implicit one if present.

Now this starts to accumulate quite some logic to the point where I start to wonder whether a Context::Server subclass makes sense, perhaps with some nice self yielding constructor so you can do something like

server.context = OpenSSL::SSL::Context::SniServer.new do |ctx|
  ctx.add_certifcate cert_a, key_a
  ctx.add_certificate "example.org", cert_b, key_b
  ctx.add_certificate "*.example.org", cert_c, key_c # explicit hostnames with wildcard matching probably warrants its own PR already
  ctx.add_certificate "example.net", some_context
  ctx.add_certificate another_context # not sure about that one, would need to validate the context has a certificate etc.
end

I don't want to ask you to do all of that now and I am mostly brainstorming here. But if there's some general agreement along those thoughts we should maybe figure out what the minimal thing is we can do here without introducing weird behaviours that we have to break later.

Isn't this branch redundant with return if @sni above?