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 <
[email protected]> 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 <[email protected]> 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 <
> > [email protected]> 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);