On Aug 19, 2014, at 10:13 AM, Alan M. Carroll <a...@network-geographics.com> wrote:
> James, > > I can answer a few of these. > >> Thanks for the docs, this looks very promising. When you are ready to submit >> patches, this will need API review >> <https://cwiki.apache.org/confluence/display/TS/API+Review+Process>. > > Actually, Susan forgot to mention that you can review the code at > https://github.com/shinrich/trafficserver/tree/ts-3006 > >> There's already and API to get the SSL *, TSHttpSsnSSLConnectionGet. I don't >> see the need for a TSSslVConn, the TSHttpSsn should be sufficient. > > I don't think that works, because the pre handshake hook is called before a > client session exists so TSHttpSsnSSLConnectionGet() will fail in that > context. > >> Are TS_SSL_CLIENT_PRE_HANDSHAKE_HOOK and TS_SSL_SNI_HOOK session hooks? If >> so, they should follow the session hook naming conventions. If not, how do >> do register these hooks? > > Not really, as there's no client session instance at that time. Both of these > hooks are called before any of the session accept logic is executed. This > makes for some ugly choices in the API. Hmm. I would like to avoid additional API object is possible. Could you use a TSVConn rather than creating TSSslVConn? >> 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. > > I recommended that because there was a discussion about potentially using a > different SSL library. The expectation is that these wrapper types can be > directly cast to a SSL library type, which is dependent on how ATS was built. I'm happy to assume that we will always use OpenSSL ;) >> There's no TSSslVConnOp value for the default action? ie. to just accept the >> SSL session? > > In that case, don't call TSSslVConnOpSet. But I suppose you might want to > undo, so that should be added. I think it's better to include the default for completeness and documentation. >> 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? > > Ah, yes. It should return a void* and size value. There's also an issue on > what maximum length can be presumed and how storage for that is handled. The > spec, as I read it, puts a limit of 64K, but in practice openSSL seems to > limit it to 256 (TS_MAX_HOST_NAME_LEN). > >> 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? > > Write a plugin :-).
