Add support for more PP fields #12864

maskit · 2026-02-06T18:38:08Z

This adds support for PP2_SUBTYPE_SSL_CIPHER and PP2_SUBTYPE_SSL_VERSION, and log fields for those.

maskit · 2026-02-06T20:43:46Z

autest filter_body failed

maskit · 2026-02-06T20:43:58Z

[approve ci autest 0]

maskit · 2026-02-06T22:56:41Z

Needs #12863 to make the new fields work.

Copilot

Pull request overview

Adds support for PROXY Protocol v2 SSL TLV subtypes (cipher and TLS version) and exposes them as new access log fields, enabling operators to log the upstream TLS details provided by a load balancer.

Changes:

Add ProxyProtocol::get_tlv_ssl_cipher() / get_tlv_ssl_version() to extract SSL sub-TLV values.
Add new LogAccess marshalers and register new log fields pptc / pptv.
Extend Proxy Protocol parser unit tests and update admin logging field documentation.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
src/iocore/net/ProxyProtocol.cc	Implements SSL sub-TLV parsing helpers and getters for TLS cipher/version.
include/iocore/net/ProxyProtocol.h	Declares new public getters and a private helper for SSL sub-TLV parsing.
src/proxy/logging/LogAccess.cc	Adds marshalers for the new Proxy Protocol TLS cipher/version log fields.
include/proxy/logging/LogAccess.h	Declares new LogAccess marshal methods for the new fields.
src/proxy/logging/Log.cc	Registers two new logging fields (`pptc`, `pptv`) mapped to the new marshalers.
src/iocore/net/unit_tests/test_ProxyProtocol.cc	Extends TLV parsing test coverage to include SSL cipher/version subtypes.
doc/admin-guide/logging/formatting.en.rst	Documents the new log field symbols.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-09T23:02:39Z

include/iocore/net/ProxyProtocol.h

 private:
  std::string additional_data;
+
+  std::optional<std::string_view> _get_tlv_ssl_subtype(int subtype) const;


ProxyProtocol::_get_tlv_ssl_subtype takes int subtype, but subtype IDs are 8-bit values on the wire (and get_tlv() uses uint8_t). Using uint8_t here avoids accidental sign/width issues and better communicates the expected range.

Suggested change

std::optional<std::string_view> _get_tlv_ssl_subtype(int subtype) const;

std::optional<std::string_view> _get_tlv_ssl_subtype(uint8_t subtype) const;

Copilot · 2026-02-09T23:02:40Z

src/iocore/net/ProxyProtocol.cc

+    // Find the given subtype
+    uint16_t    len = ssl.length();
+    const char *p   = ssl.data() + 5; // Skip client (uint8_t) + verify (uint32_t)
+    const char *end = p + len + 1;
+    while (p != end) {


_get_tlv_ssl_subtype() computes the end pointer incorrectly (it uses end = p + len + 1 after already advancing p by 5). This makes the loop read past the end of the PP2_TYPE_SSL TLV value and can cause out-of-bounds reads/undefined behavior. Compute end from the start of the SSL value (e.g., ssl.data() + ssl.size()) and iterate while p < end (not p != end).

Suggested change

// Find the given subtype

uint16_t len = ssl.length();

const char *p = ssl.data() + 5; // Skip client (uint8_t) + verify (uint32_t)

const char *end = p + len + 1;

while (p != end) {

// The SSL TLV must contain at least the client (uint8_t) and verify (uint32_t) fields.

if (ssl.length() < 5) {

Dbg(dbg_ctl_proxyprotocol_v2, "SSL TLV too short: %zu bytes (expected at least 5)", static_cast<size_t>(ssl.length()));

return std::nullopt;

}

// Find the given subtype

uint16_t len = ssl.length();

const char *p = ssl.data() + 5; // Skip client (uint8_t) + verify (uint32_t)

const char *end = ssl.data() + len; // End of the SSL TLV value

while (p < end) {

Copilot · 2026-02-09T23:02:40Z

src/iocore/net/ProxyProtocol.cc

+  if (auto v = tlv.find(PP2_TYPE_SSL); v != tlv.end() && v->second.length() != 0) {
+    auto ssl = v->second;
+
+    // Is the client connected over TLS
+    if ((ssl.data()[0] & 0x01) == 0) {
+      // Not over TLS
+      return std::nullopt;
+    }
+
+    // Find the given subtype
+    uint16_t    len = ssl.length();
+    const char *p   = ssl.data() + 5; // Skip client (uint8_t) + verify (uint32_t)


_get_tlv_ssl_subtype() reads ssl.data()[0] and skips 5 bytes without verifying the TLV value is at least 5 bytes long. If PP2_TYPE_SSL is present but truncated/malformed, this will access beyond the string_view. Add a ssl.size() >= 5 (and preferably >= 5 + 3 before parsing sub-TLVs) guard before dereferencing and parsing.

Copilot · 2026-02-09T23:02:40Z

src/proxy/logging/LogAccess.cc

+      len = padded_length(cipher->size() + 1);
+      if (buf) {
+        marshal_str(buf, cipher->data(), len);
+        buf[cipher->size()] = '\0';
+      }


LogAccess::marshal_proxy_protocol_tls_cipher() passes a non-NUL-terminated std::string_view buffer (cipher->data()) into marshal_str(), which uses ink_strlcpy and will read past the end of the view looking for a terminator. Use marshal_mem() with the actual length (cipher->size()) and the padded length, or otherwise ensure the input is NUL-terminated before calling marshal_str().

Copilot · 2026-02-09T23:02:40Z

src/proxy/logging/LogAccess.cc

+  int len = INK_MIN_ALIGN;
+
+  if (m_http_sm) {
+    if (auto cipher = m_http_sm->t_state.pp_info.get_tlv_ssl_cipher(); cipher) {
+      len = padded_length(cipher->size() + 1);
+      if (buf) {
+        marshal_str(buf, cipher->data(), len);
+        buf[cipher->size()] = '\0';
+      }
+    }
+  }
+  return len;
+}
+
+int
+LogAccess::marshal_proxy_protocol_tls_version(char *buf)
+{
+  int len = INK_MIN_ALIGN;
+
+  if (m_http_sm) {
+    if (auto version = m_http_sm->t_state.pp_info.get_tlv_ssl_version(); version) {
+      len = padded_length(version->size() + 1);
+      if (buf) {
+        marshal_str(buf, version->data(), len);
+        buf[version->size()] = '\0';
+      }
+    }
+  }


marshal_proxy_protocol_tls_cipher() returns INK_MIN_ALIGN when no cipher is available but does not write anything into buf in that case. This can leave uninitialized bytes in the log buffer. Consider always marshalling a default string (e.g., "-") when buf is non-null, and return the padded length that matches what was written.

Suggested change

int len = INK_MIN_ALIGN;

if (m_http_sm) {

if (auto cipher = m_http_sm->t_state.pp_info.get_tlv_ssl_cipher(); cipher) {

len = padded_length(cipher->size() + 1);

if (buf) {

marshal_str(buf, cipher->data(), len);

buf[cipher->size()] = '\0';

}

}

}

return len;

}

int

LogAccess::marshal_proxy_protocol_tls_version(char *buf)

{

int len = INK_MIN_ALIGN;

if (m_http_sm) {

if (auto version = m_http_sm->t_state.pp_info.get_tlv_ssl_version(); version) {

len = padded_length(version->size() + 1);

if (buf) {

marshal_str(buf, version->data(), len);

buf[version->size()] = '\0';

}

}

}

int len = INK_MIN_ALIGN;

bool have_data = false;

if (m_http_sm) {

if (auto cipher = m_http_sm->t_state.pp_info.get_tlv_ssl_cipher(); cipher) {

have_data = true;

len = padded_length(cipher->size() + 1);

if (buf) {

marshal_str(buf, cipher->data(), len);

buf[cipher->size()] = '\0';

}

}

}

// When no cipher is available, ensure the buffer is initialized with a default value.

if (buf && !have_data) {

buf[0] = '-';

// NUL-terminate and pad the remaining bytes in the field.

for (int i = 1; i < len; ++i) {

buf[i] = '\0';

}

}

return len;

}

int

LogAccess::marshal_proxy_protocol_tls_version(char *buf)

{

int len = INK_MIN_ALIGN;

bool have_data = false;

if (m_http_sm) {

if (auto version = m_http_sm->t_state.pp_info.get_tlv_ssl_version(); version) {

have_data = true;

len = padded_length(version->size() + 1);

if (buf) {

marshal_str(buf, version->data(), len);

buf[version->size()] = '\0';

}

}

}

// When no TLS version is available, ensure the buffer is initialized with a default value.

if (buf && !have_data) {

buf[0] = '-';

// NUL-terminate and pad the remaining bytes in the field.

for (int i = 1; i < len; ++i) {

buf[i] = '\0';

}

}

Copilot · 2026-02-09T23:02:41Z

src/proxy/logging/LogAccess.cc

+      len = padded_length(version->size() + 1);
+      if (buf) {
+        marshal_str(buf, version->data(), len);
+        buf[version->size()] = '\0';
+      }


LogAccess::marshal_proxy_protocol_tls_version() has the same issue as the cipher field: version->data() is a std::string_view that is not guaranteed NUL-terminated, but marshal_str() will read until it finds a terminator. Use marshal_mem() with the view length and padded length (or ensure NUL termination) to avoid out-of-bounds reads.

Copilot · 2026-02-09T23:02:41Z

src/proxy/logging/LogAccess.cc

+  int len = INK_MIN_ALIGN;
+
+  if (m_http_sm) {
+    if (auto version = m_http_sm->t_state.pp_info.get_tlv_ssl_version(); version) {
+      len = padded_length(version->size() + 1);
+      if (buf) {
+        marshal_str(buf, version->data(), len);
+        buf[version->size()] = '\0';
+      }
+    }
+  }
+  return len;


marshal_proxy_protocol_tls_version() returns INK_MIN_ALIGN when the TLS version TLV isn't present, but does not marshal any default value into buf in that case. This can result in uninitialized log output for the field. Align the behavior with other string marshalers by always writing a default string when buf is provided and returning the corresponding padded length.

Copilot · 2026-02-09T23:02:41Z

doc/admin-guide/logging/formatting.en.rst

+      Authority      to the |TS|
+pptv  Proxy Protocol The TLS version from Proxy Protocol context from the LB
+      Authority      to the |TS|


The new log field table entries for pptc/pptv look like copy/paste from ppa: the second column label is "Authority" for both, which doesn't match the TLS cipher/version descriptions. Update the field name in the middle column to something like "TLS Cipher" and "TLS Version" so the table is internally consistent.

Suggested change

Authority to the |TS|

pptv Proxy Protocol The TLS version from Proxy Protocol context from the LB

Authority to the |TS|

TLS Cipher to the |TS|

pptv Proxy Protocol The TLS version from Proxy Protocol context from the LB

TLS Version to the |TS|

maskit added this to the 10.2.0 milestone Feb 6, 2026

maskit self-assigned this Feb 6, 2026

maskit added Logging PROXY Protocol labels Feb 6, 2026

bryancall requested review from bryancall and Copilot February 9, 2026 22:57

Copilot started reviewing on behalf of bryancall February 9, 2026 22:58 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

maskit added 9 commits February 9, 2026 16:39

Add support for custom logging field

1dbb5f4

Add support for PP2_SUBTYPE_SSL_CIPHER

e74d912

Add pptc log field for PP2_SUBTYPE_SSL_CIPHER

ab3f1d4

Add support for PP2_SUBTYPE_SSL_VERSION

7afd3c5

Add pptv log field for PP2_SUBTYPE_SSL_VERSION

71f2fd5

Fix copy pasta in documentation

78eeeef

Change parameter type

214207a

Fix off by one error

8296706

Add a length check

2aa259f

maskit force-pushed the pp_tls_cipher branch from 0b343d0 to 2aa259f Compare February 10, 2026 00:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for more PP fields #12864

Add support for more PP fields #12864

Uh oh!

maskit commented Feb 6, 2026

Uh oh!

maskit commented Feb 6, 2026

Uh oh!

maskit commented Feb 6, 2026

Uh oh!

maskit commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	std::optional<std::string_view> _get_tlv_ssl_subtype(int subtype) const;
	std::optional<std::string_view> _get_tlv_ssl_subtype(uint8_t subtype) const;

Add support for more PP fields #12864

Are you sure you want to change the base?

Add support for more PP fields #12864

Uh oh!

Conversation

maskit commented Feb 6, 2026

Uh oh!

maskit commented Feb 6, 2026

Uh oh!

maskit commented Feb 6, 2026

Uh oh!

maskit commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant