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.

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

Fix l3-dependent L4/L7 rules applying to CIDR egress traffic #3434

Merged
merged 6 commits into from
Apr 2, 2018

Conversation

joestringer
Copy link
Member

Fixes: #3414

@joestringer joestringer added pending-review sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. labels Mar 31, 2018
@joestringer joestringer requested a review from a team March 31, 2018 00:29
@joestringer joestringer requested review from a team as code owners March 31, 2018 00:29
bpf/lib/l4.h Outdated
allowed = 0;
#endif /* CFG_L4_EGRESS */
#ifdef CFG_L3L4__EGRESS
if (l3dependent) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's reset allowed and treat each call to BPF_L4_MAP() independently.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, separated out into entirely different invocations.

@joestringer joestringer force-pushed the submit/l3dl4+cidr branch 2 times, most recently from 3b2d8e6 to 55ac034 Compare March 31, 2018 02:55
@tgraf
Copy link
Member

tgraf commented Mar 31, 2018

test-me-please

Refactor the way that we generate CFG_L4_INGRESS and CFG_L4_EGRESS. An
upcoming commit will split this into two, to differentiate l3-dependent
L4 vs. L3-independent L4.

Signed-off-by: Joe Stringer <joe@covalent.io>
L3-dependent L4 proxyport should only ever be found via the lookup in
the label-based policy enforcement, and never be found via the
CIDR-dependent L4 policy lookup. Split it out.

Fixes: cilium#3414

Signed-off-by: Joe Stringer <joe@covalent.io>
When policy import fails, print the poolicy as well.

Signed-off-by: Joe Stringer <joe@covalent.io>
This will be reused in an upcoming commit.

Signed-off-by: Joe Stringer <joe@covalent.io>
Signed-off-by: Joe Stringer <joe@covalent.io>
Add a test that introduces an L3-dependent L7 in-cluster egress rule
which denies all traffic, plus a separate L3 CIDR egress rule. External
access should not occur via the proxy.

This detects issue cilium#3414.

Signed-off-by: Joe Stringer <joe@covalent.io>
@joestringer
Copy link
Member Author

L3/L4 checks failed in CI but passed locally. I'm running all of the policies tests locally to see if anything crops up.

@joestringer
Copy link
Member Author

test-me-please

@tgraf tgraf merged commit 482e20b into cilium:master Apr 2, 2018
@joestringer joestringer deleted the submit/l3dl4+cidr branch April 2, 2018 20:09
@joestringer joestringer added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Apr 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants