selectors: support ipv4-mapped ipv6 addresses

[kobrineli]

Fixes: #3712

Description

Currently Tetragon parses ip address as only ipv4 or ipv6, while all ipv4 address can be used as ipv6 in ::ffff:X.X.X.X form.
In Go IP.To4() method works correctly if the address have such form, so Tetragon will add this address only in IPv4 map.
Later, when this address is used with AF_INET6, Tetragon BPF code will try to use only ipv6 map, leading to false positive event.

To fix this, we need to add ipv4 address in both ipv4 and ipv6 maps in case the original string contains colon (meaning IPv4-mapped form).

Changelog

tracingpolicy: support IPv4-mapped IPv6 address form in selectors.

[kobrineli]

With this fix policy from the issue works correctly:

apiVersion: cilium.io/v1alpha1
kind: TracingPolicy
metadata:
  name: "tcp-connect"
spec:
  kprobes:
    - call: "tcp_connect"
      syscall: false
      args:
        - index: 0
          type: "sock"
      selectors:
        - matchArgs:
            - index: 0
              operator: "NotDAddr"
              values:
                - 127.0.0.1
                - ::1
                - ::ffff:127.0.0.1

[kobrineli]

@kkourt
Hi! Could you look at these changes, please?

[kkourt]

Thanks, the change makes sense to me but I believe the implementation could be improved.

CC: @kevsecurity

[netlify]

✅ Deploy Preview for tetragon ready!

Name	Link
🔨 Latest commit	2f5d19f
🔍 Latest deploy log	https://app.netlify.com/sites/tetragon/deploys/681def6a2ba77600084db00c
😎 Deploy Preview	https://deploy-preview-3714--tetragon.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[kobrineli]

@kkourt
Hi! Thanks for the review. I've made adjustments but with some changes.
Moreover, I've noticed that my original proposal had a flaw: in case of invalid ipv4 address, if the original string didn't contain colon, err6 would remain nil and the error wouldn't be returned.

Added unit tests, covering every case of the function.

[kkourt]

@kkourt Hi! Thanks for the review. I've made adjustments but with some changes. Moreover, I've noticed that my original proposal had a flaw: in case of invalid ipv4 address, if the original string didn't contain colon, err6 would remain nil and the error wouldn't be returned.

Added unit tests, covering every case of the function.

Thanks!

Can you please squash the changes from my feedback into the relevant original commits? (git rebase --interactive using the squash and fixup actions should help). if you want, feel free to keep the unit test in a separate patch but if you opt to do so please add a descriptive commit message.

Also, once done, please force-push the new branch and notify the reviewers by clicking on the github button "Re-request review".

Thanks!

[kobrineli]

@kkourt
Done

[kkourt]

Thanks, this looks great to me!

Please find a minor comment below.

[kkourt]

Thanks, LGTM!

I kicked off the CI as well. Assuming everything's green I'll merge the PR when the CI is done.

[kobrineli]

@kkourt
Hi! Could you restart CI, please?
Errors don't seem to be related to PR changes.

[kkourt]

@kkourt Hi! Could you restart CI, please? Errors don't seem to be related to PR changes.

The errors keep happening, even though I agree that they do not seem to be related. Would need to investigate fruther.
I created an issue #3732 for this. CC: @tpapagian

[kobrineli]

@kkourt @kevsecurity
Changed parseAddr method to make it use netip.ParseAddr and net.ParseCIDR methods, now it looks much better.
Unit tests for IPv4-mapped IPv6 address pass successfully.

[kevsecurity]

Yep, that looks much better.

[kevsecurity]

Thanks. I hit merge as @kkourt had previously approved, and I wasn't sure if you have merge permissions.

[kobrineli]

I don't, thanks!

[kkourt]

Thanks. I hit merge as @kkourt had previously approved, and I wasn't sure if you have merge permissions.

There were some CI issues that I was hoping to resolve before merging the PR.
And it seems that the CI never finished after the new changes were pushed (or maybe I'm missing something?)

[kevsecurity]

Thanks. I hit merge as @kkourt had previously approved, and I wasn't sure if you have merge permissions.

There were some CI issues that I was hoping to resolve before merging the PR. And it seems that the CI never finished after the new changes were pushed (or maybe I'm missing something?)

Sorry. It looked complete here, except with quay failures.