Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix egress CIDR policy enforcement #3348

Merged
merged 2 commits into from
Mar 28, 2018
Merged

Fix egress CIDR policy enforcement #3348

merged 2 commits into from
Mar 28, 2018

Conversation

tgraf
Copy link
Member

@tgraf tgraf commented Mar 28, 2018

No description provided.

@tgraf tgraf requested a review from a team March 28, 2018 00:38
@tgraf tgraf requested a review from a team as a code owner March 28, 2018 00:38
@tgraf tgraf force-pushed the fix-egress-to-world branch from f36fa59 to ac1786a Compare March 28, 2018 00:40
@tgraf
Copy link
Member Author

tgraf commented Mar 28, 2018

test-me-please

rlenglet
rlenglet approved these changes Mar 28, 2018
View reviewed changes
bpf/bpf_overlay.c Outdated
@@ -1,4 +1,4 @@
/*
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove leading space.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@tgraf tgraf force-pushed the fix-egress-to-world branch from ac1786a to 10f8a6a Compare March 28, 2018 00:57
@tgraf
Copy link
Member Author

tgraf commented Mar 28, 2018

test-me-please

1 similar comment
@tgraf
Copy link
Member Author

tgraf commented Mar 28, 2018

test-me-please

@tgraf tgraf force-pushed the fix-egress-to-world branch from 10f8a6a to 1f2afb5 Compare March 28, 2018 02:40
@tgraf
Copy link
Member Author

tgraf commented Mar 28, 2018

test-me-please

@tgraf tgraf changed the title [WIP] bpf: Fix egress CIDR enforcement Fix egress CIDR policy enforcement Mar 28, 2018
@tgraf tgraf added wip sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. priority/insane 🚨 http://cultofthepartyparrot.com/parrots/shipitparrot.gif labels Mar 28, 2018
tgraf added 2 commits March 28, 2018 04:03
@tgraf
bpf: Fix egress CIDR policy enforcement
54ab48d 
The existing egress CIDR enforcement lookup was performed correctly but
relied on a later policy check enforcing a drop and thus only marked the
packet to skip the policy check instead of dropping it directly. The
latter policy check was removed and since broke the egress CIDR policy
enforcement.

The CI test is in-effective and thus did not catch this regression. This
commit fixes the bug, the CI test will be fixed in a separate commit.

Fixes: #3345
Fixes: #3340

Signed-off-by: Thomas Graf <thomas@cilium.io>
@tgraf
bpf: Fix return value of lpm{46}_ingress_lookup
4b6ab43 
When policy enforcement is disabled, these lookup calls should always
return a positive value to indicate a match. This bug had no effect
so far as calls to these functions are currently protected by defines
only defined when policy enforcement is enabled.

Signed-off-by: Thomas Graf <thomas@cilium.io>
@tgraf tgraf force-pushed the fix-egress-to-world branch from 1f2afb5 to 4b6ab43 Compare March 28, 2018 04:19
@tgraf
Copy link
Member Author

tgraf commented Mar 28, 2018

Fixed another bug that was hidden and added commit messages, doing another CI run.

@tgraf
Copy link
Member Author

tgraf commented Mar 28, 2018

test-me-please

@tgraf tgraf added pending-review and removed wip labels Mar 28, 2018
@tgraf tgraf merged commit 63682d6 into master Mar 28, 2018
@tgraf tgraf deleted the fix-egress-to-world branch March 28, 2018 05:54
@tgraf tgraf added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Apr 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joestringer joestringer joestringer approved these changes

@rlenglet rlenglet rlenglet approved these changes

Assignees
No one assigned
Labels
priority/insane 🚨 http://cultofthepartyparrot.com/parrots/shipitparrot.gif release-note/bug This PR fixes an issue in a previous release of Cilium. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tgraf @rlenglet @joestringer