-
Notifications
You must be signed in to change notification settings - Fork 851
Adds metrics and log fields for tracking TLS handshake bytes #12763
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -426,6 +426,7 @@ Lengths and Sizes | |
| .. _cqcl: | ||
| .. _cqhl: | ||
| .. _cqql: | ||
| .. _cqqtl: | ||
| .. _csscl: | ||
| .. _csshl: | ||
| .. _cssql: | ||
|
|
@@ -436,6 +437,7 @@ Lengths and Sizes | |
| .. _pscl: | ||
| .. _pshl: | ||
| .. _psql: | ||
| .. _psqtl: | ||
| .. _sscl: | ||
| .. _sshl: | ||
| .. _ssql: | ||
|
|
@@ -451,6 +453,10 @@ cqcl Client Request Client request content length, in bytes. | |
| cqhl Client Request Client request header length, in bytes. | ||
| cqql Client Request Client request header and content length combined, | ||
| in bytes. | ||
| cqqtl Client Request Same as cqql_, but for the first transaction on a | ||
| TLS connection, also includes TLS handshake bytes | ||
| received from the client. Note that this metrics | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| may not always be 100% accurate. | ||
| csscl Cached Origin Response Content body length from cached origin response. | ||
| csshl Cached Origin Response Header length from cached origin response. | ||
| cssql Cached Origin Response Content and header length from cached origin | ||
|
|
@@ -466,6 +472,10 @@ pscl Proxy Response Content body length of the |TS| proxy response. | |
| pshl Proxy Response Header length of the |TS| response to client. | ||
| psql Proxy Response Content body and header length combined of the | ||
| |TS| response to client. | ||
| psqtl Proxy Response Same as psql_, but for the first transaction on a | ||
| TLS connection, also includes TLS handshake bytes | ||
| sent to the client. Note that this metric may not | ||
| always be 100% accurate. | ||
| sscl Origin Response Content body length of the origin server response | ||
| to |TS|. | ||
| sshl Origin Response Header length of the origin server response. | ||
|
|
@@ -623,6 +633,9 @@ SSL / Encryption | |
| .. _cqssc: | ||
| .. _cqssu: | ||
| .. _cqssa: | ||
| .. _cthbr: | ||
| .. _cthbt: | ||
| .. _cthb: | ||
| .. _pqssl: | ||
| .. _pscert: | ||
|
|
||
|
|
@@ -659,6 +672,17 @@ cqssg Client Request SSL Group used by |TS| to communicate with the client. | |
| OpenSSL 3.2 or later or a version of BoringSSL that | ||
| supports querying group names. | ||
| cqssa Client Request ALPN Protocol ID negotiated with the client. | ||
| cthbr Client Request TLS handshake bytes received from the client. This is the | ||
| number of bytes read from the client during the TLS | ||
| handshake. Populated for all transactions on a TLS connection, | ||
| including reused connections. | ||
| cthbt Client Request TLS handshake bytes sent to the client. This is the number | ||
| of bytes written to the client during the TLS handshake. | ||
| Populated for all transactions on a TLS connection, | ||
| including reused connections. | ||
| cthb Client Request Total TLS handshake bytes (received + sent). This is the | ||
| sum of cthbr_ and cthbt_. Populated for all transactions | ||
| on a TLS connection, including reused connections. | ||
| pqssl Proxy Request Indicates whether the connection from |TS| to the origin | ||
| was over SSL or not. | ||
| pqssr Proxy Request SSL session ticket reused status from |TS| to the origin; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,8 @@ class TLSBasicSupport | |
| std::string_view get_tls_group() const; | ||
| ink_hrtime get_tls_handshake_begin_time() const; | ||
| ink_hrtime get_tls_handshake_end_time() const; | ||
| bool get_tls_handshake_bytes(uint64_t &bytes_in, uint64_t &bytes_out); | ||
|
|
||
| /** | ||
| * Returns a certificate that need to be verified. | ||
| * | ||
|
|
@@ -103,4 +105,6 @@ class TLSBasicSupport | |
|
|
||
| ink_hrtime _tls_handshake_begin_time = 0; | ||
| ink_hrtime _tls_handshake_end_time = 0; | ||
| uint64_t _tls_handshake_bytes_in = 0; | ||
| uint64_t _tls_handshake_bytes_out = 0; | ||
|
Comment on lines
+108
to
+109
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are cache variables that are lazy initialized, right? If that's right, let's make them mutable so that get_tls_handshake_bytes can be const. As a getter, that would make more sense to the caller. |
||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -280,6 +280,8 @@ SSLInitializeStatistics() | |
| ssl_rsb.user_agent_version_too_high = Metrics::Counter::createPtr("proxy.process.ssl.user_agent_version_too_high"); | ||
| ssl_rsb.user_agent_version_too_low = Metrics::Counter::createPtr("proxy.process.ssl.user_agent_version_too_low"); | ||
| ssl_rsb.user_agent_wrong_version = Metrics::Counter::createPtr("proxy.process.ssl.user_agent_wrong_version"); | ||
| ssl_rsb.tls_handshake_bytes_in_total = Metrics::Counter::createPtr("proxy.process.ssl.total_handshake_bytes_read_in"); | ||
| ssl_rsb.tls_handshake_bytes_out_total = Metrics::Counter::createPtr("proxy.process.ssl.total_handshake_bytes_write_in"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems to be a mismatch in variable name and the metric used. Should the variable be |
||
|
|
||
| #if defined(OPENSSL_IS_BORINGSSL) | ||
| size_t n = SSL_get_all_cipher_names(nullptr, 0); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,6 +72,36 @@ TLSBasicSupport::clear() | |
| { | ||
| this->_tls_handshake_begin_time = 0; | ||
| this->_tls_handshake_end_time = 0; | ||
| this->_tls_handshake_bytes_in = 0; | ||
| this->_tls_handshake_bytes_out = 0; | ||
| } | ||
|
|
||
| bool | ||
| TLSBasicSupport::get_tls_handshake_bytes(uint64_t &bytes_in, uint64_t &bytes_out) | ||
| { | ||
| if (_tls_handshake_bytes_in > 0 || _tls_handshake_bytes_out > 0) { | ||
| bytes_in = _tls_handshake_bytes_in; | ||
| bytes_out = _tls_handshake_bytes_out; | ||
| return false; | ||
| } | ||
|
|
||
| SSL *ssl = this->_get_ssl_object(); | ||
| if (ssl == nullptr) { | ||
| bytes_in = 0; | ||
| bytes_out = 0; | ||
| return false; | ||
| } | ||
|
|
||
| BIO *rbio = SSL_get_rbio(ssl); | ||
| BIO *wbio = SSL_get_wbio(ssl); | ||
|
|
||
| uint64_t bio_in = rbio ? BIO_number_read(rbio) : 0; | ||
| uint64_t bio_out = wbio ? BIO_number_written(wbio) : 0; | ||
|
|
||
| bytes_in = _tls_handshake_bytes_in = bio_in; | ||
| bytes_out = _tls_handshake_bytes_out = bio_out; | ||
|
|
||
| return true; | ||
|
Comment on lines
+80
to
+104
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't that first return return true? It set values. Maybe this is intentional? We only return true the first time the BIO is read? Regardless, let's add doxygen comments explaining the semantics of the returned boolean. |
||
| } | ||
|
|
||
| TLSHandle | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.