Skip to content

Commit

Permalink
Revert "bpf: Add CT map flag for auth required"
Browse files Browse the repository at this point in the history
This reverts commit 1f7ed72.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
  • Loading branch information
jrajahalme authored and dylandreimerink committed Apr 10, 2023
1 parent e8bc012 commit 2216111
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 73 deletions.
46 changes: 14 additions & 32 deletions bpf/bpf_lxc.c
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,6 @@ static __always_inline int handle_ipv6_from_lxc(struct __ctx_buff *ctx, __u32 *d
trace.reason = (enum trace_reason)ret;

#if defined(ENABLE_L7_LB)
from_l7lb = ctx_load_meta(ctx, CB_FROM_HOST) == FROM_HOST_L7_LB;

if (proxy_port > 0) {
/* tuple addresses have been swapped by CT lookup */
cilium_dbg3(ctx, DBG_L7_LB, tuple->daddr.p4, tuple->saddr.p4,
Expand Down Expand Up @@ -439,12 +437,6 @@ static __always_inline int handle_ipv6_from_lxc(struct __ctx_buff *ctx, __u32 *d

if (verdict == DROP_POLICY_AUTH_REQUIRED)
verdict = auth_lookup(SECLABEL, *dst_id, node_id, (__u8)*ext_err);
/* Create CT entry if drop for auth required. */
if (verdict == DROP_POLICY_AUTH_REQUIRED && ct_status == CT_NEW) {
ct_state_new.src_sec_id = SECLABEL;
ct_create6(get_ct_map6(tuple), &CT_MAP_ANY6, tuple, ctx,
CT_EGRESS, &ct_state_new, proxy_port > 0, from_l7lb, true);
}

/* Emit verdict if drop or if allow for CT_NEW or CT_REOPENED. */
if (verdict != CTX_ACT_OK || ct_status != CT_ESTABLISHED) {
Expand All @@ -458,6 +450,9 @@ static __always_inline int handle_ipv6_from_lxc(struct __ctx_buff *ctx, __u32 *d
return verdict;

skip_policy_enforcement:
#if defined(ENABLE_L7_LB)
from_l7lb = ctx_load_meta(ctx, CB_FROM_HOST) == FROM_HOST_L7_LB;
#endif
switch (ct_status) {
case CT_NEW:
ct_recreate6:
Expand All @@ -468,7 +463,7 @@ static __always_inline int handle_ipv6_from_lxc(struct __ctx_buff *ctx, __u32 *d
*/
ct_state_new.src_sec_id = SECLABEL;
ret = ct_create6(get_ct_map6(tuple), &CT_MAP_ANY6, tuple, ctx,
CT_EGRESS, &ct_state_new, proxy_port > 0, from_l7lb, false);
CT_EGRESS, &ct_state_new, proxy_port > 0, from_l7lb);
if (IS_ERR(ret))
return ret;
trace.monitor = TRACE_PAYLOAD_LEN;
Expand Down Expand Up @@ -849,8 +844,6 @@ static __always_inline int handle_ipv4_from_lxc(struct __ctx_buff *ctx, __u32 *d
trace.reason = (enum trace_reason)ret;

#if defined(ENABLE_L7_LB)
from_l7lb = ctx_load_meta(ctx, CB_FROM_HOST) == FROM_HOST_L7_LB;

if (proxy_port > 0) {
/* tuple addresses have been swapped by CT lookup */
cilium_dbg3(ctx, DBG_L7_LB, tuple->daddr, tuple->saddr, bpf_ntohs(proxy_port));
Expand Down Expand Up @@ -887,12 +880,6 @@ static __always_inline int handle_ipv4_from_lxc(struct __ctx_buff *ctx, __u32 *d

if (verdict == DROP_POLICY_AUTH_REQUIRED)
verdict = auth_lookup(SECLABEL, *dst_id, node_id, (__u8)*ext_err);
/* Create CT entry if drop for auth required. */
if (verdict == DROP_POLICY_AUTH_REQUIRED && ct_status == CT_NEW) {
ct_state_new.src_sec_id = SECLABEL;
ct_create4(get_ct_map4(tuple), &CT_MAP_ANY4, tuple, ctx,
CT_EGRESS, &ct_state_new, proxy_port > 0, from_l7lb, true);
}

/* Emit verdict if drop or if allow for CT_NEW or CT_REOPENED. */
if (verdict != CTX_ACT_OK || ct_status != CT_ESTABLISHED) {
Expand All @@ -906,6 +893,9 @@ static __always_inline int handle_ipv4_from_lxc(struct __ctx_buff *ctx, __u32 *d
return verdict;

skip_policy_enforcement:
#if defined(ENABLE_L7_LB)
from_l7lb = ctx_load_meta(ctx, CB_FROM_HOST) == FROM_HOST_L7_LB;
#endif
switch (ct_status) {
case CT_NEW:
ct_recreate4:
Expand All @@ -928,7 +918,7 @@ static __always_inline int handle_ipv4_from_lxc(struct __ctx_buff *ctx, __u32 *d
* handling here, but turns out that verifier cannot handle it.
*/
ret = ct_create4(ct_map, ct_related_map, tuple, ctx,
CT_EGRESS, &ct_state_new, proxy_port > 0, from_l7lb, false);
CT_EGRESS, &ct_state_new, proxy_port > 0, from_l7lb);
if (IS_ERR(ret))
return ret;
break;
Expand Down Expand Up @@ -1393,7 +1383,7 @@ ipv6_policy(struct __ctx_buff *ctx, int ifindex, __u32 src_label,
{
struct ct_state *ct_state, ct_state_new = {};
struct ipv6_ct_tuple *tuple;
int ret, verdict = CTX_ACT_OK, hdrlen, zero = 0;
int ret, verdict, hdrlen, zero = 0;
struct ct_buffer6 *ct_buffer;
void *data, *data_end;
struct ipv6hdr *ip6;
Expand Down Expand Up @@ -1489,7 +1479,7 @@ ipv6_policy(struct __ctx_buff *ctx, int ifindex, __u32 src_label,
tuple->nexthdr, POLICY_INGRESS, 1,
verdict, *proxy_port, policy_match_type, audited);

if (verdict != CTX_ACT_OK && verdict != DROP_POLICY_AUTH_REQUIRED)
if (verdict != CTX_ACT_OK)
return verdict;

skip_policy_enforcement:
Expand All @@ -1516,17 +1506,13 @@ ipv6_policy(struct __ctx_buff *ctx, int ifindex, __u32 src_label,
if (ret == CT_NEW) {
ct_state_new.src_sec_id = src_label;
ret = ct_create6(get_ct_map6(tuple), &CT_MAP_ANY6, tuple, ctx, CT_INGRESS,
&ct_state_new, *proxy_port > 0, false,
verdict == DROP_POLICY_AUTH_REQUIRED);
&ct_state_new, *proxy_port > 0, false);
if (IS_ERR(ret))
return ret;

/* NOTE: tuple has been invalidated after this */
}

if (verdict != CTX_ACT_OK)
return verdict;

if (!revalidate_data(ctx, &data, &data_end, &ip6))
return DROP_INVALID;

Expand Down Expand Up @@ -1703,7 +1689,7 @@ ipv4_policy(struct __ctx_buff *ctx, int ifindex, __u32 src_label, enum ct_status
struct ct_buffer4 *ct_buffer;
__u32 monitor = 0, zero = 0;
enum trace_reason reason;
int ret, verdict = CTX_ACT_OK;
int ret, verdict;
__be32 orig_sip;
__u8 policy_match_type = POLICY_MATCH_NONE;
__u8 audited = 0;
Expand Down Expand Up @@ -1812,7 +1798,7 @@ ipv4_policy(struct __ctx_buff *ctx, int ifindex, __u32 src_label, enum ct_status
tuple->nexthdr, POLICY_INGRESS, 0,
verdict, *proxy_port, policy_match_type, audited);

if (verdict != CTX_ACT_OK && verdict != DROP_POLICY_AUTH_REQUIRED)
if (verdict != CTX_ACT_OK)
return verdict;

skip_policy_enforcement:
Expand Down Expand Up @@ -1841,17 +1827,13 @@ ipv4_policy(struct __ctx_buff *ctx, int ifindex, __u32 src_label, enum ct_status
ct_state_new.src_sec_id = src_label;
ct_state_new.from_tunnel = from_tunnel;
ret = ct_create4(get_ct_map4(tuple), &CT_MAP_ANY4, tuple, ctx, CT_INGRESS,
&ct_state_new, *proxy_port > 0, false,
verdict == DROP_POLICY_AUTH_REQUIRED);
&ct_state_new, *proxy_port > 0, false);
if (IS_ERR(ret))
return ret;

/* NOTE: tuple has been invalidated after this */
}

if (verdict != CTX_ACT_OK)
return verdict;

if (!revalidate_data(ctx, &data, &data_end, &ip4))
return DROP_INVALID;

Expand Down
4 changes: 2 additions & 2 deletions bpf/lib/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -891,7 +891,7 @@ struct ct_entry {
proxy_redirect:1, /* Connection is redirected to a proxy */
dsr:1,
from_l7lb:1, /* Connection is originated from an L7 LB proxy */
auth_required:1,
reserved1:1, /* Was auth_required, not used in production anywhere */
from_tunnel:1, /* Connection is over tunnel */
reserved:5;
__u16 rev_nat_index;
Expand Down Expand Up @@ -1080,7 +1080,7 @@ struct ct_state {
syn:1,
proxy_redirect:1, /* Connection is redirected to a proxy */
from_l7lb:1, /* Connection is originated from an L7 LB proxy */
auth_required:1,
reserved1:1, /* Was auth_required, not used in production anywhere */
from_tunnel:1, /* Connection is from tunnel */
reserved:8;
__be32 addr;
Expand Down
9 changes: 2 additions & 7 deletions bpf/lib/conntrack.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ static __always_inline __u8 __ct_lookup(const void *map, struct __ctx_buff *ctx,
ct_state->dsr = entry->dsr;
ct_state->proxy_redirect = entry->proxy_redirect;
ct_state->from_l7lb = entry->from_l7lb;
ct_state->auth_required = entry->auth_required;
ct_state->from_tunnel = entry->from_tunnel;
ct_state->ifindex = entry->ifindex;
}
Expand Down Expand Up @@ -832,8 +831,7 @@ static __always_inline int ct_create6(const void *map_main, const void *map_rela
struct ipv6_ct_tuple *tuple,
struct __ctx_buff *ctx, const enum ct_dir dir,
const struct ct_state *ct_state,
bool proxy_redirect, bool from_l7lb,
bool auth_required)
bool proxy_redirect, bool from_l7lb)
{
/* Create entry in original direction */
struct ct_entry entry = { };
Expand All @@ -853,7 +851,6 @@ static __always_inline int ct_create6(const void *map_main, const void *map_rela
*/
entry.proxy_redirect = proxy_redirect;
entry.from_l7lb = from_l7lb;
entry.auth_required = auth_required;
}

entry.rev_nat_index = ct_state->rev_nat_index;
Expand Down Expand Up @@ -903,8 +900,7 @@ static __always_inline int ct_create4(const void *map_main,
struct ipv4_ct_tuple *tuple,
struct __ctx_buff *ctx, const enum ct_dir dir,
const struct ct_state *ct_state,
bool proxy_redirect, bool from_l7lb,
bool auth_required)
bool proxy_redirect, bool from_l7lb)
{
/* Create entry in original direction */
struct ct_entry entry = { };
Expand All @@ -925,7 +921,6 @@ static __always_inline int ct_create4(const void *map_main,
*/
entry.proxy_redirect = proxy_redirect;
entry.from_l7lb = from_l7lb;
entry.auth_required = auth_required;
}

entry.rev_nat_index = ct_state->rev_nat_index;
Expand Down
30 changes: 13 additions & 17 deletions bpf/lib/host_firewall.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,11 @@ ipv6_host_policy_egress(struct __ctx_buff *ctx, __u32 src_id,
if (verdict == DROP_POLICY_AUTH_REQUIRED)
verdict = auth_lookup(src_id, dst_id, node_id, (__u8)*ext_err);

/* Only create CT entry for accepted connections, or when auth is required */
if (ret == CT_NEW && (verdict == CTX_ACT_OK || verdict == DROP_POLICY_AUTH_REQUIRED)) {
/* Only create CT entry for accepted connections */
if (ret == CT_NEW && verdict == CTX_ACT_OK) {
ct_state_new.src_sec_id = HOST_ID;
ret = ct_create6(get_ct_map6(&tuple), &CT_MAP_ANY6, &tuple,
ctx, CT_EGRESS, &ct_state_new, proxy_port > 0, false,
verdict == DROP_POLICY_AUTH_REQUIRED);
ctx, CT_EGRESS, &ct_state_new, proxy_port > 0, false);
if (IS_ERR(ret))
return ret;
}
Expand Down Expand Up @@ -156,14 +155,13 @@ ipv6_host_policy_ingress(struct __ctx_buff *ctx, __u32 *src_id,
if (verdict == DROP_POLICY_AUTH_REQUIRED)
verdict = auth_lookup(dst_id, *src_id, node_id, (__u8)ext_err);

/* Only create CT entry for accepted connections, or when auth is required */
if (ret == CT_NEW && (verdict == CTX_ACT_OK || verdict == DROP_POLICY_AUTH_REQUIRED)) {
/* Only create CT entry for accepted connections */
if (ret == CT_NEW && verdict == CTX_ACT_OK) {
/* Create new entry for connection in conntrack map. */
ct_state_new.src_sec_id = *src_id;
ct_state_new.node_port = ct_state.node_port;
ret = ct_create6(get_ct_map6(&tuple), &CT_MAP_ANY6, &tuple,
ctx, CT_INGRESS, &ct_state_new, proxy_port > 0, false,
verdict == DROP_POLICY_AUTH_REQUIRED);
ctx, CT_INGRESS, &ct_state_new, proxy_port > 0, false);
if (IS_ERR(ret))
return ret;
}
Expand Down Expand Up @@ -222,7 +220,7 @@ whitelist_snated_egress_connections(struct __ctx_buff *ctx, __u32 ipcache_srcid,
if (ret == CT_NEW) {
ret = ct_create4(get_ct_map4(&tuple), &CT_MAP_ANY4,
&tuple, ctx, CT_EGRESS, &ct_state_new,
false, false, false);
false, false);
if (IS_ERR(ret))
return ret;
}
Expand Down Expand Up @@ -293,12 +291,11 @@ ipv4_host_policy_egress(struct __ctx_buff *ctx, __u32 src_id,
if (verdict == DROP_POLICY_AUTH_REQUIRED)
verdict = auth_lookup(src_id, dst_id, node_id, (__u8)*ext_err);

/* Only create CT entry for accepted connections, or when auth is required */
if (ret == CT_NEW && (verdict == CTX_ACT_OK || verdict == DROP_POLICY_AUTH_REQUIRED)) {
/* Only create CT entry for accepted connections */
if (ret == CT_NEW && verdict == CTX_ACT_OK) {
ct_state_new.src_sec_id = HOST_ID;
ret = ct_create4(get_ct_map4(&tuple), &CT_MAP_ANY4, &tuple,
ctx, CT_EGRESS, &ct_state_new, proxy_port > 0, false,
verdict == DROP_POLICY_AUTH_REQUIRED);
ctx, CT_EGRESS, &ct_state_new, proxy_port > 0, false);
if (IS_ERR(ret))
return ret;
}
Expand Down Expand Up @@ -382,14 +379,13 @@ ipv4_host_policy_ingress(struct __ctx_buff *ctx, __u32 *src_id,
if (verdict == DROP_POLICY_AUTH_REQUIRED)
verdict = auth_lookup(dst_id, *src_id, node_id, (__u8)ext_err);

/* Only create CT entry for accepted connections, or when auth is required */
if (ret == CT_NEW && (verdict == CTX_ACT_OK || verdict == DROP_POLICY_AUTH_REQUIRED)) {
/* Only create CT entry for accepted connections */
if (ret == CT_NEW && verdict == CTX_ACT_OK) {
/* Create new entry for connection in conntrack map. */
ct_state_new.src_sec_id = *src_id;
ct_state_new.node_port = ct_state.node_port;
ret = ct_create4(get_ct_map4(&tuple), &CT_MAP_ANY4, &tuple,
ctx, CT_INGRESS, &ct_state_new, proxy_port > 0, false,
verdict == DROP_POLICY_AUTH_REQUIRED);
ctx, CT_INGRESS, &ct_state_new, proxy_port > 0, false);
if (IS_ERR(ret))
return ret;
}
Expand Down
4 changes: 2 additions & 2 deletions bpf/lib/lb.h
Original file line number Diff line number Diff line change
Expand Up @@ -877,7 +877,7 @@ static __always_inline int lb6_local(const void *map, struct __ctx_buff *ctx,
state->backend_id = backend_id;
state->rev_nat_index = svc->rev_nat_index;

ret = ct_create6(map, NULL, tuple, ctx, CT_SERVICE, state, false, false, false);
ret = ct_create6(map, NULL, tuple, ctx, CT_SERVICE, state, false, false);
/* Fail closed, if the conntrack entry create fails drop
* service lookup.
*/
Expand Down Expand Up @@ -1570,7 +1570,7 @@ static __always_inline int lb4_local(const void *map, struct __ctx_buff *ctx,
state->backend_id = backend_id;
state->rev_nat_index = svc->rev_nat_index;

ret = ct_create4(map, NULL, tuple, ctx, CT_SERVICE, state, false, false, false);
ret = ct_create4(map, NULL, tuple, ctx, CT_SERVICE, state, false, false);
/* Fail closed, if the conntrack entry create fails drop
* service lookup.
*/
Expand Down
4 changes: 2 additions & 2 deletions bpf/lib/nat.h
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ static __always_inline int snat_v4_track_connection(struct __ctx_buff *ctx,
return ret;
} else if (ret == CT_NEW) {
ret = ct_create4(get_ct_map4(&tmp), NULL, &tmp, ctx,
where, &ct_state, false, false, false);
where, &ct_state, false, false);
if (IS_ERR(ret))
return ret;
}
Expand Down Expand Up @@ -1284,7 +1284,7 @@ static __always_inline int snat_v6_track_connection(struct __ctx_buff *ctx,
return ret;
} else if (ret == CT_NEW) {
ret = ct_create6(get_ct_map6(&tmp), NULL, &tmp, ctx, where,
&ct_state, false, false, false);
&ct_state, false, false);
if (IS_ERR(ret))
return ret;
}
Expand Down
8 changes: 4 additions & 4 deletions bpf/lib/nodeport.h
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ int tail_nodeport_dsr_ingress_ipv6(struct __ctx_buff *ctx)
ct_state_new.dsr = 1;
ct_state_new.ifindex = (__u16)NATIVE_DEV_IFINDEX;
ret = ct_create6(get_ct_map6(&tuple), NULL, &tuple, ctx,
CT_EGRESS, &ct_state_new, false, false, false);
CT_EGRESS, &ct_state_new, false, false);
if (!IS_ERR(ret))
ret = snat_v6_create_dsr(&tuple, &addr, port);

Expand Down Expand Up @@ -1020,7 +1020,7 @@ static __always_inline int nodeport_lb6(struct __ctx_buff *ctx,
ct_state_new.node_port = 1;
ct_state_new.ifindex = (__u16)NATIVE_DEV_IFINDEX;
ret = ct_create6(get_ct_map6(&tuple), NULL, &tuple, ctx,
CT_EGRESS, &ct_state_new, false, false, false);
CT_EGRESS, &ct_state_new, false, false);
if (IS_ERR(ret))
return ret;
break;
Expand Down Expand Up @@ -2045,7 +2045,7 @@ int tail_nodeport_dsr_ingress_ipv4(struct __ctx_buff *ctx)
ct_state_new.dsr = 1;
ct_state_new.ifindex = (__u16)NATIVE_DEV_IFINDEX;
ret = ct_create4(get_ct_map4(&tuple), NULL, &tuple, ctx,
CT_EGRESS, &ct_state_new, false, false, false);
CT_EGRESS, &ct_state_new, false, false);
if (!IS_ERR(ret))
ret = snat_v4_create_dsr(&tuple, addr, port);

Expand Down Expand Up @@ -2254,7 +2254,7 @@ static __always_inline int nodeport_lb4(struct __ctx_buff *ctx,
ct_state_new.node_port = 1;
ct_state_new.ifindex = (__u16)NATIVE_DEV_IFINDEX;
ret = ct_create4(get_ct_map4(&tuple), NULL, &tuple, ctx,
CT_EGRESS, &ct_state_new, false, false, false);
CT_EGRESS, &ct_state_new, false, false);
if (IS_ERR(ret))
return ret;
break;
Expand Down
2 changes: 1 addition & 1 deletion bpf/tests/bpf_ct_tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ int test_ct4_rst1_check(__maybe_unused struct __ctx_buff *ctx)
ct_state_new.node_port = ct_state.node_port;
ct_state_new.ifindex = ct_state.ifindex;
ret = ct_create4(get_ct_map4(&tuple), &CT_MAP_ANY4, &tuple, ctx,
CT_EGRESS, &ct_state_new, false, false, false);
CT_EGRESS, &ct_state_new, false, false);
break;

default:
Expand Down
3 changes: 1 addition & 2 deletions bpf/tests/nat_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ static __always_inline int mock_ct_create4(__maybe_unused const void *map_main,
__maybe_unused const int dir,
__maybe_unused const struct ct_state *ct_state,
__maybe_unused bool proxy_redirect,
__maybe_unused bool from_l7lb,
__maybe_unused bool auth_required)
__maybe_unused bool from_l7lb)
{
return mock_ct_create4_response;
}
Expand Down

0 comments on commit 2216111

Please sign in to comment.