Skip to content

Fixed bug in bpf where IPv4 destination address was stored as IPv6#648

Merged
kmesh-bot merged 3 commits intokmesh-net:mainfrom
LiZhenCheng9527:rbac-fix
Aug 1, 2024
Merged

Fixed bug in bpf where IPv4 destination address was stored as IPv6#648
kmesh-bot merged 3 commits intokmesh-net:mainfrom
LiZhenCheng9527:rbac-fix

Conversation

@LiZhenCheng9527
Copy link
Copy Markdown
Contributor

@LiZhenCheng9527 LiZhenCheng9527 commented Jul 29, 2024

What type of PR is this?
/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #653

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


@kmesh-bot kmesh-bot added the kind/bug Something isn't working label Jul 29, 2024
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.97%. Comparing base (68d7f31) to head (ccb0848).
Report is 4 commits behind head on main.

Files Coverage Δ
pkg/auth/rbac.go 79.77% <100.00%> (-1.00%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 75a7312...ccb0848. Read the comment docs.

IP6_COPY(key.addr.ip6, skops->local_ip6);
if (skops->family == AF_INET6) {
if (is_ipv4_mapped_addr(skops->local_ip6))
key.addr.ip4 = skops->local_ip4;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is skops->local_ip4 and skops->local_ip6 set simutaneously?

tuple_key->ipv6.dport = GET_SKOPS_REMOTE_PORT(skops);
}

if (is_ipv4_mapped_addr(tuple_key->ipv6.daddr)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt this is not right. If the dst address is mapped address. Assume you want to extract to ipv4, i am not sure why operating on ip6 below, since is it a union of ipv4 and ipv6.

And BTW, since the family == AF_INET6, how can we make it work when we dnat to v4?

Can you test with a real example

Signed-off-by: LiZhenCheng9527 <lizhencheng6@huawei.com>
Signed-off-by: LiZhenCheng9527 <lizhencheng6@huawei.com>
Signed-off-by: LiZhenCheng9527 <lizhencheng6@huawei.com>
}
conn.dstPort = uint32(tupleV6.DstPort)
// conn.dstIp = restoreIPv4(conn.dstIp)
// conn.srcIp = restoreIPv4(conn.srcIp)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

Copy link
Copy Markdown
Member

@hzxuzhonghu hzxuzhonghu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@kmesh-bot
Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@kmesh-bot kmesh-bot merged commit 9c10c28 into kmesh-net:main Aug 1, 2024
@hzxuzhonghu
Copy link
Copy Markdown
Member

Should be backported

@kmesh-bot
Copy link
Copy Markdown
Collaborator

In response to a cherrypick label: new pull request created: #660

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Does not pewrform when using tcp-echo to test the l4 authorizationPolicy

4 participants