[GitHub] [trafficserver] ywkaras commented on a diff in pull request #8854: In ja3_fingerprint plugin, protect against premature VConn closure.

[GitBox <gi...@apache.org>]

ywkaras commented on code in PR #8854:
URL: https://github.com/apache/trafficserver/pull/8854#discussion_r982505611


##########
plugins/experimental/ja3_fingerprint/ja3_fingerprint.cc:
##########
@@ -359,38 +474,40 @@ req_hdr_ja3_handler(TSCont contp, TSEvent event, void *edata)
     TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
     return TS_SUCCESS;
   }
+  {
+    // Retrieve ja3_data from vconn args
+    auto a = ja3_data::access(vconn);
+    if (a.get()) {
+      // Decide global or remap
+      ja3_remap_info *info = static_cast<ja3_remap_info *>(TSContDataGet(contp));
+      bool raw_flag        = info ? info->raw : enable_raw;
+      bool log_flag        = info ? info->log : enable_log;
+      TSDebug(PLUGIN_NAME, "req_hdr_ja3_handler(): Found ja3 string.");
+
+      // Get handle to headers
+      TSMBuffer bufp;
+      TSMLoc hdr_loc;
+      TSAssert(TS_SUCCESS == TSHttpTxnServerReqGet(txnp, &bufp, &hdr_loc));
+
+      // Add JA3 md5 fingerprints
+      append_to_field(bufp, hdr_loc, "X-JA3-Sig", 9, a.get()->md5_string, 32);
+
+      // If raw string is configured, added JA3 raw string to header as well
+      if (raw_flag) {
+        append_to_field(bufp, hdr_loc, "x-JA3-RAW", 9, a.get()->ja3_string.data(), a.get()->ja3_string.size());
+      }
+      TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc);
 
-  // Retrieve ja3_data from vconn args
-  ja3_data *data = static_cast<ja3_data *>(TSUserArgGet(vconn, ja3_idx));
-  if (data) {
-    // Decide global or remap
-    ja3_remap_info *info = static_cast<ja3_remap_info *>(TSContDataGet(contp));
-    bool raw_flag        = info ? info->raw : enable_raw;
-    bool log_flag        = info ? info->log : enable_log;
-    TSDebug(PLUGIN_NAME, "req_hdr_ja3_handler(): Found ja3 string.");
-
-    // Get handle to headers
-    TSMBuffer bufp;
-    TSMLoc hdr_loc;
-    TSAssert(TS_SUCCESS == TSHttpTxnServerReqGet(txnp, &bufp, &hdr_loc));
-
-    // Add JA3 md5 fingerprints
-    append_to_field(bufp, hdr_loc, "X-JA3-Sig", 9, data->md5_string, 32);
-
-    // If raw string is configured, added JA3 raw string to header as well
-    if (raw_flag) {
-      append_to_field(bufp, hdr_loc, "x-JA3-RAW", 9, data->ja3_string.data(), data->ja3_string.size());
+      // Write to logfile
+      if (log_flag) {
+        TSTextLogObjectWrite(pluginlog, "Client IP: %s\tJA3: %.*s\tMD5: %.*s", a.get()->ip_addr,
+                             static_cast<int>(a.get()->ja3_string.size()), a.get()->ja3_string.data(), 32, a.get()->md5_string);
+      }
+    } else {
+      TSDebug(PLUGIN_NAME, "req_hdr_ja3_handler(): ja3 data not set. Not SSL vconn. Abort.");
     }
-    TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc);
+  } // This block is needed to make sure 'a' is destroyed before reeenable is called.

Review Comment:
   OK changed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org