Alan, I've been working on the issues we discussed. I have most of it working. I've added two things:
- Allow plugins to add user data to conn. bool TSVConnSetUserData(TSVConn connp, const char* name, void* data); void* TSVConnGetUserData(TSVConn connp, const char* name); Did not bother with reserving since user can get the data using name. - Added close hook for SSLNetVConnection. I couldn't add it to lower levels because, the lower levels (vconn, netvconn) are called after the ssl data-structures are cleaned up and I wanted the plugin callback to be synchronously so that user data can be cleaned up. Things are working for the most part. I am getting a crash on the second ssl request. I suspect I am missing something. Would appreciate it if you or someone more familiar with the code can let me know what I am doing wrong. I've attached the diff (the diff is based on 6.2.1 code base). Thanks, Dk PS: Stack trace... 0 0x00000000004afd82 crash_logger_invoke(int, siginfo_t*, void*) + 0x82 1 0x00002afa38eec390 __restore_rt + (nil) 2 0x00002afa39bd2428 gsignal + 0x38 3 0x00002afa39bd402a abort + 0x16a 4 0x00002afa37ba0adf ink_fatal_va(char const*, __va_list_tag*) + 0x9f 5 0x00002afa37ba0b79 ink_fatal(char const*, ...) + 0x99 6 0x00002afa37b9ec35 _ink_assert + 0x15 7 0x00000000004ca26d INKContInternal::handle_event(int, void*) + 0x13d 8 0x0000000000780b27 SSLNetVConnection::do_io_close(int) + 0x147 9 0x00000000005a7636 Http1ClientSession::do_io_close(int) + 0x116 10 0x00000000005ab86f Http1ClientSession::state_keep_alive(int, void*) + 0x3f 11 0x00000000007adbb0 UnixNetVConnection::readSignalDone(int, NetHandler*) + 0x50 12 0x00000000007891e8 SSLNetVConnection::net_read_io(NetHandler*, EThread*) + 0x898 13 0x0000000000798654 NetHandler::mainNetEvent(int, Event*) + 0x214 14 0x00000000007e0f12 EThread::process_event(Event*, int) + 0x92 15 0x00000000007e1bc9 EThread::execute() + 0x639 16 0x00000000007e0995 spawn_thread_internal(void*) + 0x55 17 0x00002afa38ee26ba start_thread + 0xca 18 0x00002afa39ca43dd clone + 0x6d 19 0x0000000000000000 0x0 + 0x6d On Wed, Sep 27, 2017 at 6:19 PM, Alan Carroll < solidwallofc...@oath.com.invalid> wrote: > I don't think I'd do it for TLS_CLOSE, but VCONN_CLOSE would likely be > worth it. There's an issue on that, linked from the issue I mentioned > above. It's about the same thing - plugin wants to attach data during TLS > handshake for later retrieval at the HTTP level that can be cleaned up if > something goes wrong. > > On Wed, Sep 27, 2017 at 4:29 PM, Dk Jack <dnj0...@gmail.com> wrote: > > > Yeah, I was thinking along similar lines... > > > > Do you think, it'd be a worth while effort to add a ssl-close event hook? > > > > On Wed, Sep 27, 2017 at 1:31 PM, Alan Carroll < > > solidwallofc...@oath.com.invalid> wrote: > > > > > I'd probably put a time stamp in and update it during the hook > > processing. > > > If you waited 5 minutes or so before clearing, that should be enough. > If > > > it's been 5 minutes since the last activity the connection should be > > dead, > > > unless you're shipping big files. > > > > > > Or, alternatively, in READ_REQUEST_HDR_HOOK pull the data out and > attach > > it > > > to the txn, cleaning it up in TXN_CLOSE. Set a time stamp on entry in > the > > > table and if that's been 5 minutes, the connection has definitely timed > > > out. > > > > > >
--- ./iocore/net/P_NetVConnection.h 2016-12-16 18:14:58.000000000 +0000 +++ ../../trafficserver-6.2.1/./iocore/net/P_NetVConnection.h 2017-10-09 23:40:35.464116072 +0000 @@ -21,6 +21,9 @@ limitations under the License. */ +#ifndef __P_NETVCONNECTION_H__ +#define __P_NETVCONNECTION_H__ + #include "I_NetVConnection.h" TS_INLINE sockaddr const * @@ -74,3 +77,33 @@ NetVConnection::get_local_port() { return ats_ip_port_host_order(this->get_local_addr()); } + +TS_INLINE void * +NetVConnection::get_user_data(const char* name) const +{ + int len = strlen(name); + + for (int ix = 0; ix < user_data_next_index; ++ix) { + if ((len == user_data_table[ix].name_len) && (0 == strcmp(name, user_data_table[ix].name))) { + return user_data_table[ix].data; + } + } + + return NULL; +} + +TS_INLINE bool +NetVConnection::set_user_data(const char* name, void *arg) +{ + if (user_data_next_index >= MAX_CONN_USER_DATA) + return false; + + user_data_table[user_data_next_index].name = ats_strdup(name); + user_data_table[user_data_next_index].name_len = strlen(name); + user_data_table[user_data_next_index].data = arg; + ink_atomic_increment(&user_data_next_index, 1); // index values are 0 - (MAX_CONN_USER_DATA-1) + + return true; +} + +#endif // __P_NETVCONNECTION_H__ --- ./iocore/net/SSLNetVConnection.cc 2016-12-16 18:14:58.000000000 +0000 +++ ../../trafficserver-6.2.1/./iocore/net/SSLNetVConnection.cc 2017-10-09 23:14:58.735844304 +0000 @@ -855,6 +855,10 @@ SSLNetVConnection::SSLNetVConnection() void SSLNetVConnection::do_io_close(int lerrno) { + if (this->ssl != NULL) + callHooks(TS_SSL_CLOSE_HOOK); // calls ssl close hook before ssl + // data-structures are cleared. + if (this->ssl != NULL && sslHandShakeComplete) { int shutdown_mode = SSL_get_shutdown(ssl); Debug("ssl-shutdown", "previous shutdown state 0x%x", shutdown_mode); @@ -1455,7 +1459,7 @@ SSLNetVConnection::callHooks(TSHttpHookI { // Only dealing with the SNI/CERT hook so far. // TS_SSL_SNI_HOOK and TS_SSL_CERT_HOOK are the same value - ink_assert(eventId == TS_SSL_CERT_HOOK); + ink_assert(eventId == TS_SSL_CERT_HOOK || eventId == TS_SSL_CLOSE_HOOK); Debug("ssl", "callHooks sslHandshakeHookState=%d", this->sslHandshakeHookState); // First time through, set the type of the hook that is currently being invoked @@ -1466,6 +1470,8 @@ SSLNetVConnection::callHooks(TSHttpHookI this->sslHandshakeHookState = HANDSHAKE_HOOKS_CERT; // get Hooks curHook = ssl_hooks->get(TS_SSL_CERT_INTERNAL_HOOK); + } else if ((HANDSHAKE_HOOKS_DONE == sslHandshakeHookState) && (eventId == TS_SSL_CLOSE_HOOK)) { + curHook = ssl_hooks->get(TS_SSL_CLOSE_INTERNAL_HOOK); } else { // Not in the right state // reenable and continue --- ./iocore/net/I_NetVConnection.h 2016-12-16 18:14:58.000000000 +0000 +++ ../../trafficserver-6.2.1/./iocore/net/I_NetVConnection.h 2017-10-09 23:41:16.588201435 +0000 @@ -228,6 +228,10 @@ private: class NetVConnection : public VConnection { public: + enum { + MAX_CONN_USER_DATA=8 + }; + /** Initiates read. Thread safe, may be called when not handling an event from the NetVConnection, or the NetVConnection creation @@ -515,7 +519,7 @@ public: virtual void reenable_re(VIO *vio) = 0; /// PRIVATE - virtual ~NetVConnection() {} + virtual ~NetVConnection(); /** PRIVATE: instances of NetVConnection cannot be created directly by the state machines. The objects are created by NetProcessor @@ -565,6 +569,9 @@ public: is_transparent = state; } + void* get_user_data(const char* name) const; + bool set_user_data(const char* name, void *arg); + private: NetVConnection(const NetVConnection &); NetVConnection &operator=(const NetVConnection &); @@ -581,6 +588,16 @@ protected: bool is_transparent; /// Set if the next write IO that empties the write buffer should generate an event. int write_buffer_empty_event; + +private: + struct user_data { + uint8_t name_len; + char *name; + void *data; + }; + + uint8_t user_data_next_index; + struct user_data user_data_table[MAX_CONN_USER_DATA]; }; inline NetVConnection::NetVConnection() @@ -591,12 +608,21 @@ inline NetVConnection::NetVConnection() got_remote_addr(0), is_internal_request(false), is_transparent(false), - write_buffer_empty_event(0) + write_buffer_empty_event(0), + user_data_next_index(0) { ink_zero(local_addr); ink_zero(remote_addr); } +inline NetVConnection::~NetVConnection() +{ + for (uint8_t ix = 0; ix < user_data_next_index; ++ix) { + if (user_data_table[ix].name) + ats_free(user_data_table[ix].name); + } +} + inline void NetVConnection::trapWriteBufferEmpty(int event) { --- ./iocore/eventsystem/I_VConnection.h 2016-12-16 18:14:58.000000000 +0000 +++ ../../trafficserver-6.2.1/./iocore/eventsystem/I_VConnection.h 2017-10-09 17:46:24.608918906 +0000 @@ -366,6 +366,20 @@ public: return false; } + virtual void* + get_user_data(const char *name) const + { + ink_assert(!"VConnection::get_user_arg -- cannot use default implementation"); + return NULL; + } + + virtual bool + set_user_data(const char *name, void *arg) + { + ink_assert(!"VConnection::set_user_arg -- cannot use default implementation"); + return false; + } + public: /** The error code from the last error. --- ./lib/ts/apidefs.h 2017-10-04 23:37:19.045038578 +0000 +++ ../../trafficserver-6.2.1/./lib/ts/apidefs.h 2017-10-03 20:21:14.107946286 +0000 @@ -274,7 +274,8 @@ typedef enum { TS_VCONN_PRE_ACCEPT_HOOK = TS_SSL_FIRST_HOOK, TS_SSL_SNI_HOOK, TS_SSL_CERT_HOOK = TS_SSL_SNI_HOOK, - TS_SSL_LAST_HOOK = TS_SSL_CERT_HOOK, + TS_SSL_CLOSE_HOOK, + TS_SSL_LAST_HOOK = TS_SSL_CLOSE_HOOK, TS_HTTP_LAST_HOOK } TSHttpHookID; #define TS_HTTP_READ_REQUEST_PRE_REMAP_HOOK TS_HTTP_PRE_REMAP_HOOK /* backwards compat */ --- ./proxy/InkAPIInternal.h 2016-12-16 18:14:58.000000000 +0000 +++ ../../trafficserver-6.2.1/./proxy/InkAPIInternal.h 2017-10-03 20:47:09.103809284 +0000 @@ -285,6 +285,7 @@ typedef enum { TS_SSL_INTERNAL_FIRST_HOOK, TS_VCONN_PRE_ACCEPT_INTERNAL_HOOK = TS_SSL_INTERNAL_FIRST_HOOK, TS_SSL_CERT_INTERNAL_HOOK, + TS_SSL_CLOSE_INTERNAL_HOOK, TS_SSL_INTERNAL_LAST_HOOK } TSSslHookInternalID; --- ./proxy/InkAPI.cc 2016-12-16 18:14:58.000000000 +0000 +++ ../../trafficserver-6.2.1/./proxy/InkAPI.cc 2017-10-09 17:45:05.968907146 +0000 @@ -5746,6 +5746,21 @@ TSHttpSsnArgGet(TSHttpSsn ssnp, int arg_ return cs->get_user_arg(arg_idx); } +void * +TSHttpSsnGetUserDataFromConnection(TSHttpSsn ssnp, const char* name) +{ + sdk_assert(sdk_sanity_check_http_ssn(ssnp) == TS_SUCCESS); + + ProxyClientSession *cs = reinterpret_cast<ProxyClientSession *>(ssnp); + NetVConnection *vc = cs->get_netvc(); + + if (vc != NULL) { + return vc->get_user_data(name); + } + + return NULL; +} + void TSHttpTxnSetHttpRetStatus(TSHttpTxn txnp, TSHttpStatus http_retstatus) { @@ -6332,6 +6347,24 @@ TSVConnWrite(TSVConn connp, TSCont contp return reinterpret_cast<TSVIO>(vc->do_io_write((INKContInternal *)contp, nbytes, (IOBufferReader *)readerp)); } +bool +TSVConnSetUserData(TSVConn connp, const char* name, void* data) +{ + sdk_assert(sdk_sanity_check_iocore_structure(connp) == TS_SUCCESS); + + NetVConnection *vc = (NetVConnection *)connp; + return vc->set_user_data(name, data); +} + +void* +TSVConnGetUserData(TSVConn connp, const char* name) +{ + sdk_assert(sdk_sanity_check_iocore_structure(connp) == TS_SUCCESS); + + NetVConnection *vc = (NetVConnection *)connp; + return vc->get_user_data(name); +} + void TSVConnClose(TSVConn connp) { --- ./proxy/InkAPITest.cc 2016-12-16 18:14:58.000000000 +0000 +++ ../../trafficserver-6.2.1/./proxy/InkAPITest.cc 2017-10-03 20:58:15.830400326 +0000 @@ -5430,7 +5430,8 @@ typedef enum { ORIG_TS_SSL_FIRST_HOOK, ORIG_TS_VCONN_PRE_ACCEPT_HOOK = ORIG_TS_SSL_FIRST_HOOK, ORIG_TS_SSL_SNI_HOOK, - ORIG_TS_SSL_LAST_HOOK = ORIG_TS_SSL_SNI_HOOK, + ORIG_TS_SSL_CLOSE_HOOK, + ORIG_TS_SSL_LAST_HOOK = ORIG_TS_SSL_CLOSE_HOOK, ORIG_TS_HTTP_LAST_HOOK } ORIG_TSHttpHookID; --- ./proxy/http/HttpDebugNames.cc 2016-12-16 18:14:58.000000000 +0000 +++ ../../trafficserver-6.2.1/./proxy/http/HttpDebugNames.cc 2017-10-03 20:18:28.133179254 +0000 @@ -482,6 +482,8 @@ HttpDebugNames::get_api_hook_name(TSHttp return "TS_VCONN_PRE_ACCEPT_HOOK"; case TS_SSL_CERT_HOOK: return "TS_SSL_CERT_HOOK"; + case TS_SSL_CLOSE_HOOK: + return "TS_SSL_CLOSE_HOOK"; } return "unknown hook"; --- ./proxy/api/ts/ts.h 2016-12-16 18:14:58.000000000 +0000 +++ ../../trafficserver-6.2.1/./proxy/api/ts/ts.h 2017-10-07 00:17:26.183702168 +0000 @@ -1295,6 +1295,7 @@ tsapi void TSHttpTxnClientIncomingPortSe @return SSL object of this session */ tsapi void *TSHttpSsnSSLConnectionGet(TSHttpSsn ssnp); // returns SSL * +tsapi int TSHttpSsnSSLConnectionGetSocket(TSHttpSsn ssnp); // returns SSL * /** Get client address for transaction @a txnp. Retrieves the socket address of the remote client that has @@ -1498,6 +1499,7 @@ tsapi void TSHttpTxnArgSet(TSHttpTxn txn tsapi void *TSHttpTxnArgGet(TSHttpTxn txnp, int arg_idx); tsapi void TSHttpSsnArgSet(TSHttpSsn ssnp, int arg_idx, void *arg); tsapi void *TSHttpSsnArgGet(TSHttpSsn ssnp, int arg_idx); +tsapi void *TSHttpSsnGetUserDataFromConnection(TSHttpSsn ssnp, const char* name); /* The reserve API should only be use in TSAPI plugins, during plugin initialization! */ /* The lookup methods can be used anytime, but are best used during initialization as well, @@ -1684,6 +1686,9 @@ tsapi TSVIO TSVConnReadVIOGet(TSVConn co tsapi TSVIO TSVConnWriteVIOGet(TSVConn connp); tsapi int TSVConnClosedGet(TSVConn connp); +tsapi bool TSVConnSetUserData(TSVConn connp, const char* name, void* data); +tsapi void* TSVConnGetUserData(TSVConn connp, const char* name); + tsapi TSVIO TSVConnRead(TSVConn connp, TSCont contp, TSIOBuffer bufp, int64_t nbytes); tsapi TSVIO TSVConnWrite(TSVConn connp, TSCont contp, TSIOBufferReader readerp, int64_t nbytes); tsapi void TSVConnClose(TSVConn connp);