NETOBSERV-2471: TLS usage tracking #815

jotak · 2025-10-14T15:37:03Z

Start implementing TLS, by reading the TLS header when present
Extract SSL version
Report the TLS version in output records

Dependencies

openshift-ci · 2025-10-14T15:37:07Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

bpf/utils.h

openshift-ci-robot · 2025-10-30T16:00:25Z

@jotak: This pull request references NETOBSERV-2471 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target the "4.21.0" version, but no target version was set.

Details

In response to this:

(wip status = pretty much done, except for handshake version (see code comment), maybe needs an experimental feature gate; correctness to be verified, as I'm getting less TLS flows than expected - am I missing something?)

use tls.VersionName

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2025-10-30T16:09:04Z

@jotak: This pull request references NETOBSERV-2471 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target the "4.21.0" version, but no target version was set.

Details

In response to this:

Start implementing TLS, by reading the TLS header when present

Extract SSL version (not done yet for the handshake message)

Report the TLS version in output records

TODO: handshake message: version is stored a couple of bits further

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2025-10-31T09:11:00Z

@jotak: This pull request references NETOBSERV-2471 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target the "4.21.0" version, but no target version was set.

Details

In response to this:

Start implementing TLS, by reading the TLS header when present

Extract SSL version (not done yet for the handshake message)

Report the TLS version in output records

TODO: handshake message: version is stored a couple of bits further

Dependencies

FLP: NETOBSERV-2471: TLS usage tracking (bump agent) flowlogs-pipeline#1119

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2025-10-31T15:27:26Z

@jotak: This pull request references NETOBSERV-2471 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target the "4.21.0" version, but no target version was set.

Details

In response to this:

Start implementing TLS, by reading the TLS header when present

Extract SSL version

Report the TLS version in output records

Dependencies

FLP: NETOBSERV-2471: TLS usage tracking (bump agent) flowlogs-pipeline#1119

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2025-10-31T15:43:49Z

@jotak: This pull request references NETOBSERV-2471 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target the "4.21.0" version, but no target version was set.

Details

In response to this:

Start implementing TLS, by reading the TLS header when present

Extract SSL version

Report the TLS version in output records

Dependencies

Operator: NETOBSERV-2471: TLS fields network-observability-operator#2124

FLP: NETOBSERV-2471: TLS usage tracking (bump agent) flowlogs-pipeline#1119

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

msherif1234 · 2025-10-31T16:51:27Z

bpf/flows.c

    if (enable_dns_tracking) {
        dns_errno = track_dns_packet(skb, &pkt);
    }
+    track_tls_version(skb, &pkt);


don't think its better if we have feature config for tracker like we do with all other features ? instead of enabling it by default ?

yes I'll do that (although I would like to have it enabled by default if it doesn't show visible impact on perfs)

msherif1234 · 2025-10-31T16:51:59Z

bpf/flows.c

+                if (pkt->ssl_version < aggregate_flow->ssl_version) {
+                    aggregate_flow->ssl_version = pkt->ssl_version;
+                }
+                aggregate_flow->misc_flags |= MISC_FLAGS_SSL_MISMATCH;


is it better to have mismatch counter instead of flag ?

the flag allows us to correlate precisely with a particular flow, and report that up to the UI

msherif1234 · 2025-10-31T16:53:36Z

bpf/tls_tracker.h

+
+// Extract TLS info
+static inline void track_tls_version(struct __sk_buff *skb, pkt_info *pkt) {
+    if (pkt->id->transport_protocol == IPPROTO_TCP) {


do u think this work for STCP protocol too ?, not sure what is the story for DTLS ?

I haven't digged into other protocols.. might be something to do if someone asks for it, but I believe TCP covers most of the expectations

msherif1234 · 2025-10-31T16:57:04Z

bpf/tls_tracker.h

+        struct tcphdr *tcp = (struct tcphdr *)pkt->l4_hdr;
+        if (!tcp || ((void *)tcp + sizeof(*tcp) > data_end)) {
+            return;
+        }


does the verifier needs the above check given how late we call this tracker all headers should have been sane already ?

Yes I had a verifier error without those checks

msherif1234 · 2025-10-31T17:05:33Z

bpf/tls_tracker.h

+};
+
+// Extract TLS info
+static inline void track_tls_version(struct __sk_buff *skb, pkt_info *pkt) {


might be good idea to add return code and track failure at the caller since there many conditions check in this function and if things doesn't work it will be hard to debug.
might be good adding BPF_PRINTK() on failing checks ?

msherif1234 · 2025-10-31T17:08:01Z

bpf/tls_tracker.h

+
+        switch (rec.content_type) {
+        case CONTENT_TYPE_HANDSHAKE: {
+            pkt->ssl_version = ((u16)rec.major) << 8 | rec.minor;


why setting it here if u really want handshake ssl version ?

that's a fallback for the case where handshake header couldn't be read ... But I'm going to change that, actually perhaps we don't want the client-hello version at all, it doesn't tell what's actually going to be used

msherif1234 · 2025-10-31T17:08:42Z

bpf/tls_tracker.h

+        case CONTENT_TYPE_ALERT:
+        case CONTENT_TYPE_APP_DATA:
+            pkt->ssl_version = ((u16)rec.major) << 8 | rec.minor;
+            break;


do u think adding SSL record type to the follow will be adding value to end user ?

you mean adding it to the flow, like, as a bitfield? hmm why not ..

bpf/tls_tracker.h

msherif1234

Nice work!!, I left some comments/suggestions, nothing major small enhancements . Also pls share some screenshots for this new feature

msherif1234 · 2025-11-01T11:48:34Z

bpf/tls_tracker.h

+            return;
+        }
+
+        switch (rec.content_type) {


This logic assume there is always tls record after tcp header this is not safe assumption we could have dns header here or any other header ??

right, I should harden that a bit more; I've found unexpected TLSVersion during my tests probably because of that.

github-actions · 2025-11-03T10:52:19Z

New images:
quay.io/netobserv/ebpf-bytecode:16db482
quay.io/netobserv/netobserv-ebpf-agent:16db482

These will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=16db482 make set-agent-image

- Start implementing TLS, by reading the TLS header when present - Extract SSL version (not done yet for the handshake message) - Report the TLS version in output records

openshift-ci · 2026-01-09T11:14:23Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jpinsonneau for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

DOWNSTREAM_OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2026-01-10T05:41:27Z

@jotak: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/qe-e2e-tests	`84ed5b6`	link	false	`/test qe-e2e-tests`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

rhmdnd · 2026-02-04T15:13:02Z

bpf/tls_tracker.h

+                tls->version = bpf_ntohs(version);
+                // Stop reading here
+                return TLSTRACKER_MATCHED;
+            }


Something else that would be nice to pull out of the extensions is the key share. This would tell users if the server is using classical or hybrid key exchange algorithms, which is useful for finding out which applications are PQC-ready (e.g., looking for X25519 + Kyber768).

We might need to do something like:

if (ext_hdr.type == 0x002b) { // Supported Versions: single version expected u16 version; if (bpf_skb_load_bytes(skb, offset + ext_offset, &version, sizeof(version)) < 0) { return TLSTRACKER_UNKNOWN; } tls->version = bpf_ntohs(version); } else if (ext_hdr.type == 0x0033) { // Key Share Extension (TLS 1.3) // The Server Hello Key Share contains: // Group (2 bytes) + Key Exchange Length (2 bytes) + Key Exchange Data u16 selected_group; if (bpf_skb_load_bytes(skb, offset + ext_offset, &selected_group, sizeof(selected_group)) < 0) { // not sure what would be best to return here? return KEYSHARE_UNKNOWN; } tls->key_share_group = bpf_ntohs(selected_group); }

The tls_info would need to be updated to relay that information though.

Thoughts?

yep, definitely, if that's useful to figure out PQC-readiness, it's something that we want here
I'll add it, thanks for the hint!

openshift-ci bot added the do-not-merge/work-in-progress label Oct 14, 2025

jotak commented Oct 14, 2025

View reviewed changes

bpf/utils.h Outdated Show resolved Hide resolved

jotak changed the title ~~WIP TLS~~ NETOBSERV-2471: TLS usage tracking Oct 30, 2025

openshift-ci-robot added the jira/valid-reference label Oct 30, 2025

jotak force-pushed the tls branch 2 times, most recently from 9797fe9 to abc2fd7 Compare October 30, 2025 16:04

jotak mentioned this pull request Oct 31, 2025

NETOBSERV-2471: TLS usage tracking (bump agent) netobserv/flowlogs-pipeline#1119

Open

jotak marked this pull request as ready for review October 31, 2025 15:27

openshift-ci bot removed the do-not-merge/work-in-progress label Oct 31, 2025

jotak requested review from jpinsonneau, luisjira and msherif1234 October 31, 2025 15:27

jotak mentioned this pull request Oct 31, 2025

NETOBSERV-2471: TLS fields netobserv/network-observability-operator#2124

Open

10 tasks

msherif1234 reviewed Oct 31, 2025

View reviewed changes

bpf/tls_tracker.h Show resolved Hide resolved

msherif1234 reviewed Oct 31, 2025

View reviewed changes

msherif1234 reviewed Nov 1, 2025

View reviewed changes

jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Nov 3, 2025

jotak added 3 commits January 9, 2026 11:14

NETOBSERV-2471: TLS usage tracking

2c98c98

- Start implementing TLS, by reading the TLS header when present - Extract SSL version (not done yet for the handshake message) - Report the TLS version in output records

Check handshake version, use bpf_skb_load_bytes

5311877

Show ~ on version mismatch

7e32299

jotak force-pushed the tls branch from 9d32f26 to 7e32299 Compare January 9, 2026 11:14

github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Jan 9, 2026

jotak added 2 commits January 10, 2026 00:47

Implement 1.3 / handshake extension reading

18a9863

bump flp

84ed5b6

jotak force-pushed the tls branch from 1ccad62 to 84ed5b6 Compare January 9, 2026 23:47

rhmdnd reviewed Feb 4, 2026

View reviewed changes

NETOBSERV-2471: TLS usage tracking #815

Are you sure you want to change the base?

NETOBSERV-2471: TLS usage tracking #815

Uh oh!

Conversation

jotak commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependencies

Uh oh!

openshift-ci bot commented Oct 14, 2025

Uh oh!

Uh oh!

openshift-ci-robot commented Oct 30, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Oct 30, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Oct 31, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependencies

Uh oh!

openshift-ci-robot commented Oct 31, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependencies

Uh oh!

openshift-ci-robot commented Oct 31, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependencies

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

msherif1234 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 3, 2025

Uh oh!

openshift-ci bot commented Jan 9, 2026

Uh oh!

openshift-ci bot commented Jan 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

jotak commented Oct 14, 2025 •

edited

Loading

openshift-ci-robot commented Oct 30, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Oct 30, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Oct 31, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Oct 31, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Oct 31, 2025 •

edited by openshift-ci bot

Loading

msherif1234 left a comment •

edited

Loading