James,
Thanks for the feedback. I think Alan already addressed most of the
issues. Here are my comments on the remaining items.
Why do TSSslCertFindByName() and TSSslCertFindByAddress() take a TSSslVConn
argument?
I'm using the TSSslVConn to cache a pointer to the global cert table
(loaded from ssl_multicert.config). Since in theory the
ssl_multicert.config could be reloaded at any point, we acquire() a copy
of the pointer before going into the PRE_HANDSHAKE and SNI hooks, and
release() that pointer at the end. Originally I thought of having a
TSSslCertTableGet() and then pass that into the TSSslCertFind*()
functions, but decided to squish out the explicit LookupTable call.
Actually, as I'm writing this up, I realize that since I removed the
explicit reference to the Lookup table from the API, I don't need to
cache a copy of the lookup table in the framework. The TSSslCertFind*()
functions can just acquire and release as they go. I'll go ahead and do
that API simplification. That will remove the TSSslVConn argument.
Previously, then we looked at SSL integration APIs, we eschewed wrapped types
like TSSslContext in favour of directly returning OpenSSL pointers (as void *).
My argument at the time was that there's a lot of API surface needed to deal
with SSL and not much value in just wrapping OpenSSL.
Can definitely go either way here. Another benefit of the opaque types
is that the compiler can do some minimal type checking for you. Not a
huge issue for me either way.
There's no TSSslVConnOp value for the default action? ie. to just accept the
SSL session?
Definitely would be good to have the default to undo. Will get that added
IIRC, OpenSSL doesn't guarantee anything about the SNI name except that is is a
bag of bytes. Is it OK for TSSslVConnServernameGet() to present that as a C
string?
The servername is not null terminated in the packet, but OpenSSL does
null terminate it before handing back the value via
SSL_get_servername(). I went back through the openssl to verify the
null termination. Interestingly some data structures in there are
storing the servername as buffer plus length, but the one returned is
NULL terminated and data only. Internally they are doing many strlen
and strcmp operations on it.
Finally, I'm not thrilled with the idea of using ssl_multicert.config to
specify the tunneling action. What would you do if you had a wildcard cert and
only wanted to tunnel a specific name?
In the wildcard case, e.g, a cert for *.bob.com, you could create a self
signed cert to represent fred.bob.com and then use that cert to enter a
config line to blind tunnel traffic to fred.bob.com. In the tunnel case
the cert is only used select traffic, so it need not be signed by a
'real' CA.
Of course, I'm open to other options. As Alan noted, one could write a
plugin for any complex scenarios, but adding a config-only option for
simple cases seems like a good idea.
Thanks,
Susan
