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);

Reply via email to