Updated documents and code to add default hook op and remove the
TSSslVConn argument from the FindBy functions.
On 8/19/2014 2:46 PM, Susan Hinrichs wrote:
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
