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

do rbac in xdp prog #712

Merged
merged 21 commits into from
Sep 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/v2-c/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ CFLAGS += -fstack-protector-strong -fPIC
CFLAGS += -Wall -Werror
CFLAGS += -D_FORTIFY_SOURCE=2 -O2

SOURCES = $(wildcard */*.c)
SOURCES = $(wildcard */*.c) $(wildcard workloadapi/security/*.c)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does previous /.c already includes workloadapi/security/*.c?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/.c will only include files in the current directory

OBJECTS = $(subst .c,.o,$(SOURCES))
# target
APPS := libkmesh_api_v2_c.so
Expand Down
52 changes: 51 additions & 1 deletion bpf/include/bpf_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@
/* Ip(0.0.0.2 | ::2) used for control command, e.g. KmeshControl */
#define CONTROL_CMD_IP 2

#define MAP_SIZE_OF_OUTTER_MAP 8192

#define BPF_DATA_MAX_LEN \
192 /* this value should be \
small that make compile success */
#define BPF_INNER_MAP_DATA_LEN 1300

struct manager_key {
union {
__u64 netns_cookie;
Expand Down Expand Up @@ -48,6 +55,22 @@ struct {
__type(value, struct sock_storage_data);
} map_of_sock_storage SEC(".maps");

struct {
__uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
__uint(key_size, sizeof(__u32));
Copy link
Member

Choose a reason for hiding this comment

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

why this is u32 while L176 use u64

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm a little confused about this too,The map definition is the original one, just the position is moved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original design is confirmed. Only 32-bit information is used. __u64 is used to solve the alarm of ptr forced forwarding.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

commented on L175

__uint(value_size, sizeof(__u32));
__uint(max_entries, MAP_SIZE_OF_OUTTER_MAP);
__uint(map_flags, 0);
} outer_map SEC(".maps");
Copy link
Member

Choose a reason for hiding this comment

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

nit: should add kmesh prefix


struct {
__uint(type, BPF_MAP_TYPE_ARRAY);
__uint(key_size, sizeof(__u32));
__uint(value_size, BPF_INNER_MAP_DATA_LEN);
__uint(max_entries, 1);
__uint(map_flags, 0);
} inner_map SEC(".maps");
Copy link
Member

Choose a reason for hiding this comment

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

ditto


/*
* From v5.4, bpf_get_netns_cookie can be called for bpf cgroup hooks, from v5.15, it can be called for bpf sockops
* hook. Therefore, ensure that function is correctly used.
Expand Down Expand Up @@ -137,4 +160,31 @@ static inline bool handle_kmesh_manage_process(struct kmesh_context *kmesh_ctx)
return false;
}

#endif
static inline void *kmesh_get_ptr_val(const void *ptr)
{
/*
map_in_map -- outer_map:
key value
idx1 inner_map_fd1 // point to inner map1
idx2 inner_map_fd2 // point to inner map2

Copy link
Member

Choose a reason for hiding this comment

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

what does structA.ptr_member1 represent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pointer-type element

structA.ptr_member1 = idx1; // store idx in outer_map
*/
void *inner_map_instance = NULL;
__u32 inner_idx = 0;
__u32 idx = (__u32)(uintptr_t)ptr;

Copy link
Member

Choose a reason for hiding this comment

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

nit:should be put in front of this function

if (!ptr) {
return NULL;
}

/* get inner_map_instance by idx */
inner_map_instance = kmesh_map_lookup_elem(&outer_map, &idx);
if (!inner_map_instance) {
return NULL;
}

/* get inner_map_instance value */
return kmesh_map_lookup_elem(inner_map_instance, &inner_idx);
}
#endif
49 changes: 1 addition & 48 deletions bpf/kmesh/ads/include/kmesh_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "bpf_log.h"
#include "common.h"
#include "bpf_common.h"
#include "config.h"
#include "core/address.pb-c.h"

Expand All @@ -17,11 +18,6 @@
#define BPF_LOGTYPE_ROUTER_CONFIG BPF_DEBUG_ON
#define BPF_LOGTYPE_COMMON BPF_DEBUG_ON

#define BPF_DATA_MAX_LEN \
192 /* this value should be \
small that make compile success */
#define BPF_INNER_MAP_DATA_LEN 1300

#define BPF_OK 1

#define _(P) \
Expand Down Expand Up @@ -73,22 +69,6 @@ static inline char *bpf_strncpy(char *dst, int n, const char *src)
}
#endif

struct {
__uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
__uint(key_size, sizeof(__u32));
__uint(value_size, sizeof(__u32));
__uint(max_entries, MAP_SIZE_OF_OUTTER_MAP);
__uint(map_flags, 0);
} outer_map SEC(".maps");

struct {
__uint(type, BPF_MAP_TYPE_ARRAY);
__uint(key_size, sizeof(__u32));
__uint(value_size, BPF_INNER_MAP_DATA_LEN);
__uint(max_entries, 1);
__uint(map_flags, 0);
} inner_map SEC(".maps");

typedef enum {
KMESH_TAIL_CALL_LISTENER = 1,
KMESH_TAIL_CALL_FILTER_CHAIN,
Expand All @@ -112,31 +92,4 @@ enum kmesh_l7_msg_type { MSG_UNKNOW = 0, MSG_REQUEST, MSG_MID_REPONSE, MSG_FINAL
#define GET_RET_PROTO_TYPE(n) ((n)&0xff)
#define GET_RET_MSG_TYPE(n) (((n) >> KMESH_PROTO_TYPE_WIDTH) & 0xff)

static inline void *kmesh_get_ptr_val(const void *ptr)
{
/*
map_in_map -- outer_map:
key value
idx1 inner_map_fd1 // point to inner map1
idx2 inner_map_fd2 // point to inner map2

structA.ptr_member1 = idx1; // store idx in outer_map
*/
void *inner_map_instance = NULL;
__u32 inner_idx = 0;
__u64 idx = (__u64)ptr;

if (!ptr) {
return NULL;
}

/* get inner_map_instance by idx */
inner_map_instance = kmesh_map_lookup_elem(&outer_map, &idx);
if (!inner_map_instance) {
return NULL;
}

/* get inner_map_instance value */
return kmesh_map_lookup_elem(inner_map_instance, &inner_idx);
}
#endif // _KMESH_COMMON_H_
2 changes: 1 addition & 1 deletion bpf/kmesh/bpf2go/bpf2go.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@ package bpf2go
//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -cc clang --cflags $EXTRA_CFLAGS --cflags $EXTRA_CDEFINE KmeshSockops ../ads/sockops.c -- -I../ads/include -I../../include -I../../../api/v2-c
//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -cc clang --cflags $EXTRA_CFLAGS --cflags $EXTRA_CDEFINE KmeshTracePoint ../ads/tracepoint.c -- -I../ads/include -I../../include
//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -cc clang --cflags $EXTRA_CFLAGS --cflags $EXTRA_CDEFINE KmeshSockopsWorkload ../workload/sockops.c -- -I../workload/include -I../../include -I../probes
//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -cc clang --cflags $EXTRA_CFLAGS --cflags $EXTRA_CDEFINE KmeshXDPAuth ../workload/xdp.c -- -I../workload/include -I../../include
//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -cc clang --cflags $EXTRA_CFLAGS --cflags $EXTRA_CDEFINE KmeshXDPAuth ../workload/xdp.c -- -I../workload/include -I../../include -I../../../api/v2-c
//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -cc clang --cflags $EXTRA_CFLAGS --cflags $EXTRA_CDEFINE KmeshSendmsg ../workload/sendmsg.c -- -I../workload/include -I../../include
214 changes: 214 additions & 0 deletions bpf/kmesh/workload/include/authz.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,214 @@
/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
/* Copyright Authors of Kmesh */

#ifndef __AUTHZ_H__
#define __AUTHZ_H__

#include "workload_common.h"
#include "bpf_log.h"
#include "xdp.h"
#include "workloadapi/security/authorization.pb-c.h"

#define AUTH_ALLOW 0
#define AUTH_DENY 1
#define UNMATCHED 0
#define MATCHED 1

struct {
__uint(type, BPF_MAP_TYPE_HASH);
__uint(key_size, sizeof(__u32));
__uint(value_size, sizeof(Istio__Security__Authorization));
__uint(map_flags, BPF_F_NO_PREALLOC);
__uint(max_entries, MAP_SIZE_OF_AUTH_POLICY);
} map_of_authz SEC(".maps");

static inline Istio__Security__Authorization *map_lookup_authz(__u32 policyKey)
{
return (Istio__Security__Authorization *)kmesh_map_lookup_elem(&map_of_authz, &policyKey);
}

static inline wl_policies_v *get_workload_policies_by_uid(__u32 workload_uid)
{
return (wl_policies_v *)kmesh_map_lookup_elem(&map_of_wl_policy, &workload_uid);
}

static inline int matchDstPorts(Istio__Security__Match *match, struct xdp_info *info, struct bpf_sock_tuple *tuple_info)
{
__u32 *notPorts = NULL;
__u32 *ports = NULL;
__u32 i;

if (match->n_destination_ports == 0 && match->n_not_destination_ports == 0) {
return MATCHED;
}

if (match->n_not_destination_ports != 0) {
notPorts = kmesh_get_ptr_val(match->not_destination_ports);
if (!notPorts) {
return UNMATCHED;
}
#pragma unroll
for (i = 0; i < MAX_MEMBER_NUM_PER_POLICY; i++) {
if (i >= match->n_not_destination_ports) {
break;
}
if (info->iph->version == 4) {
if (bpf_htons(notPorts[i]) == tuple_info->ipv4.dport) {
return UNMATCHED;
}
} else {
if (bpf_htons(notPorts[i]) == tuple_info->ipv6.dport) {
return UNMATCHED;
}
}
}
}
// if not match not_destination_ports && has no destination_ports, return MATCHED
if (match->n_destination_ports == 0) {
return MATCHED;
}

ports = kmesh_get_ptr_val(match->destination_ports);
if (!ports) {
return UNMATCHED;
}
#pragma unroll
for (i = 0; i < MAX_MEMBER_NUM_PER_POLICY; i++) {
if (i >= match->n_destination_ports) {
break;
}
if (info->iph->version == 4) {
if (bpf_htons(ports[i]) == tuple_info->ipv4.dport) {
return MATCHED;
}
} else {
if (bpf_htons(ports[i]) == tuple_info->ipv6.dport) {
return MATCHED;
}
}
}
return UNMATCHED;
Copy link
Contributor

@nlgwcy nlgwcy Aug 31, 2024

Choose a reason for hiding this comment

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

If the not_destination_ports rules are not matched, should it return matched?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

check the begin of this function, line 42 solved the scenario you mentioned

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix

}

static inline int match_check(Istio__Security__Match *match, struct xdp_info *info, struct bpf_sock_tuple *tuple_info)
{
__u32 matchResult;

// if multiple types are set, they are AND-ed, all matched is a match
// todo: add other match types
matchResult = matchDstPorts(match, info, tuple_info);
return matchResult;
}

static inline int
clause_match_check(Istio__Security__Clause *cl, struct xdp_info *info, struct bpf_sock_tuple *tuple_info)
{
void *matchsPtr = NULL;
Istio__Security__Match *match = NULL;
__u32 i;

if (cl->n_matches == 0) {
return UNMATCHED;
}
matchsPtr = kmesh_get_ptr_val(cl->matches);
if (!matchsPtr) {
return MATCHED;
}

#pragma unroll
for (i = 0; i < MAX_MEMBER_NUM_PER_POLICY; i++) {
if (i >= cl->n_matches) {
break;
}
match = (Istio__Security__Match *)kmesh_get_ptr_val((void *)*((__u64 *)matchsPtr + i));
if (!match) {
continue;
}
// if any match matches, it is a match
if (match_check(match, info, tuple_info) == MATCHED) {
return MATCHED;
}
}
return UNMATCHED;
}

static inline int
rule_match_check(Istio__Security__Rule *rule, struct xdp_info *info, struct bpf_sock_tuple *tuple_info)
{
void *clausesPtr = NULL;
Istio__Security__Clause *clause = NULL;
__u32 i;

if (rule->n_clauses == 0) {
BPF_LOG(ERR, AUTH, "rule has no clauses\n");
return UNMATCHED;
}
// Clauses are AND-ed.
clausesPtr = kmesh_get_ptr_val(rule->clauses);
if (!clausesPtr) {
BPF_LOG(ERR, AUTH, "failed to get clauses from rule\n");
return UNMATCHED;
}

#pragma unroll
for (i = 0; i < MAX_MEMBER_NUM_PER_POLICY; i++) {
if (i >= rule->n_clauses) {
break;
}
clause = (Istio__Security__Clause *)kmesh_get_ptr_val((void *)*((__u64 *)clausesPtr + i));
if (!clause) {
continue;
}
if (clause_match_check(clause, info, tuple_info) == UNMATCHED) {
return UNMATCHED;
}
}
return MATCHED;
}

static inline int
do_auth(Istio__Security__Authorization *policy, struct xdp_info *info, struct bpf_sock_tuple *tuple_info)
{
void *rulesPtr = NULL;
Istio__Security__Rule *rule = NULL;
int matchFlag = 0;
__u32 i = 0;

if (policy->n_rules == 0) {
BPF_LOG(ERR, AUTH, "auth policy %s has no rules\n", kmesh_get_ptr_val(policy->name));
return AUTH_ALLOW;
}

// Rules are OR-ed.
rulesPtr = kmesh_get_ptr_val(policy->rules);
Copy link
Member

Choose a reason for hiding this comment

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

I am not familiar with this, but looks weired to me, It is using the ptr address as key, we cannot assume each time the address of a policy is same. Especially when a policy is updated

if (!rulesPtr) {
BPF_LOG(ERR, AUTH, "failed to get rules from policy %s\n", kmesh_get_ptr_val(policy->name));
return AUTH_DENY;
}

for (i = 0; i < MAX_MEMBER_NUM_PER_POLICY; i++) {
if (i >= policy->n_rules) {
break;
}
rule = (Istio__Security__Rule *)kmesh_get_ptr_val((void *)*((__u64 *)rulesPtr + i));
if (!rule) {
continue;
}
if (rule_match_check(rule, info, tuple_info) == MATCHED) {
if (policy->action == ISTIO__SECURITY__ACTION__DENY) {
return AUTH_DENY;
} else {
return AUTH_ALLOW;
}
}
}

// no match rules
if (policy->action == ISTIO__SECURITY__ACTION__DENY) {
return AUTH_ALLOW;
} else {
return AUTH_DENY;
}
}

#endif
13 changes: 7 additions & 6 deletions bpf/kmesh/workload/include/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
#define _KMESH_CONFIG_H_

// map size
#define MAP_SIZE_OF_FRONTEND 105000
#define MAP_SIZE_OF_SERVICE 5000
#define MAP_SIZE_OF_ENDPOINT 105000
#define MAP_SIZE_OF_BACKEND 100000
#define MAP_SIZE_OF_AUTH 8192
Copy link
Member

Choose a reason for hiding this comment

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

I donot mean to reduce this size, it is used per socket. For xdp auth.

What i meant is the per node auth policies can be small

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok,I see what you mean. This size is also used in the auth-over-socket process. will change it

#define MAP_SIZE_OF_DSTINFO 8192
#define MAP_SIZE_OF_FRONTEND 105000
#define MAP_SIZE_OF_SERVICE 5000
#define MAP_SIZE_OF_ENDPOINT 105000
#define MAP_SIZE_OF_BACKEND 100000
#define MAP_SIZE_OF_AUTH 8192
#define MAP_SIZE_OF_DSTINFO 8192
#define MAP_SIZE_OF_AUTH_POLICY 512

// map name
#define map_of_frontend kmesh_frontend
Expand Down
Loading
Loading