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

More Pod attributes for PDP: SA and labels #665

Merged
merged 10 commits into from
Jul 17, 2024
32 changes: 30 additions & 2 deletions pkg/controlplane/authz/connectivitypdp/connectivity_pdp.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func NewPDP() *PDP {
}
}

// Returns a slice of copies of the non-privileged policies stored in the PDP.
// GetPrivilegedPolicies returns a slice of copies of the non-privileged policies stored in the PDP.
func (pdp *PDP) GetPrivilegedPolicies() []v1alpha1.PrivilegedAccessPolicy {
pols := pdp.privilegedPolicies.getPolicies()
res := []v1alpha1.PrivilegedAccessPolicy{}
Expand All @@ -88,7 +88,7 @@ func (pdp *PDP) GetPrivilegedPolicies() []v1alpha1.PrivilegedAccessPolicy {
return res
}

// Returns a slice of copies of the non-privileged policies stored in the PDP.
// GetPolicies returns a slice of copies of the non-privileged policies stored in the PDP.
func (pdp *PDP) GetPolicies() []v1alpha1.AccessPolicy {
pols := pdp.regularPolicies.getPolicies()
res := []v1alpha1.AccessPolicy{}
Expand All @@ -101,6 +101,13 @@ func (pdp *PDP) GetPolicies() []v1alpha1.AccessPolicy {
return res
}

// DependsOnClientAttrs returns whether the PDP holds a policy which depends on attributes
// the client workload (From field) may or may not have.
func (pdp *PDP) DependsOnClientAttrs() bool {
return pdp.privilegedPolicies.dependsOnClientAttrs() ||
pdp.regularPolicies.dependsOnClientAttrs()
}

// AddOrUpdatePolicy adds an AccessPolicy to the PDP.
// If a policy with the same name already exists in the PDP,
// it is updated (including updating the Action field).
Expand Down Expand Up @@ -172,6 +179,11 @@ func (pt *policyTier) getPolicies() connPolicyMap {
return res
}

// dependsOnClientAttrs returns whether any of the tier's policies has a From field which depends on specific attributes.
func (pt *policyTier) dependsOnClientAttrs() bool {
return pt.denyPolicies.dependsOnClientAttrs() || pt.allowPolicies.dependsOnClientAttrs()
}

// addPolicy adds an access policy to the given tier, based on its action.
// Note that within a tier, no two policies can have the same name, even if one is deny and the other is allow.
func (pt *policyTier) addPolicy(policyName types.NamespacedName, policySpec *v1alpha1.AccessPolicySpec) {
Expand Down Expand Up @@ -259,6 +271,22 @@ func (cpm connPolicyMap) decide(src WorkloadAttrs, dest *DestinationDecision, pr
return false, nil
}

// dependsOnClientAttrs returns whether any of the policies has a From field which depends on specific attributes.
func (cpm connPolicyMap) dependsOnClientAttrs() bool {
for _, policySpec := range cpm {
for i := range policySpec.From {
selector := policySpec.From[i].WorkloadSelector
if selector == nil {
continue
}
if len(selector.MatchExpressions) > 0 || len(selector.MatchLabels) > 0 {
return true
}
}
}
return false
}

// accessPolicyDecide returns a policy's decision on a given connection.
// If the policy matches the connection, a decision based on its Action is returned.
// Otherwise, it returns an "undecided" value.
Expand Down
55 changes: 55 additions & 0 deletions pkg/controlplane/authz/connectivitypdp/connectivity_pdp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,61 @@ func getNameOfFirstPolicyInPDP(pdp *connectivitypdp.PDP, action v1alpha1.AccessP
return ""
}

func TestDependsOnClientAttrs(t *testing.T) {
emptyWorkloadSet := []v1alpha1.WorkloadSetOrSelector{{WorkloadSelector: &metav1.LabelSelector{}}}
nonEmptyWorkloadSet := []v1alpha1.WorkloadSetOrSelector{trivialWorkloadSet}
allowFromAllClientsPol := v1alpha1.AccessPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "allow-from-all-clients",
Namespace: defaultNS,
},
Spec: v1alpha1.AccessPolicySpec{
Action: v1alpha1.AccessPolicyActionAllow,
From: emptyWorkloadSet,
To: nonEmptyWorkloadSet,
},
}
allowFromSomeClientsPol := v1alpha1.AccessPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "reg",
Namespace: defaultNS,
},
Spec: v1alpha1.AccessPolicySpec{
Action: v1alpha1.AccessPolicyActionAllow,
From: nonEmptyWorkloadSet,
To: nonEmptyWorkloadSet,
},
}
allowFromSomeClientsPrivPol := v1alpha1.PrivilegedAccessPolicy{
ObjectMeta: metav1.ObjectMeta{Name: "priv"},
Spec: v1alpha1.AccessPolicySpec{
Action: v1alpha1.AccessPolicyActionDeny,
From: nonEmptyWorkloadSet,
To: nonEmptyWorkloadSet,
},
}

pdp := connectivitypdp.NewPDP()
// no policy -> no dependency on client attributes
require.False(t, pdp.DependsOnClientAttrs())

// adding a policy with allow-all From field -> still no dependency on client attributes
require.Nil(t, pdp.AddOrUpdatePolicy(connectivitypdp.PolicyFromCR(&allowFromAllClientsPol)))
require.False(t, pdp.DependsOnClientAttrs())

// adding a policy that only allows clients with specific labels -> now we have a dependency on client attributes
require.Nil(t, pdp.AddOrUpdatePolicy(connectivitypdp.PolicyFromCR(&allowFromSomeClientsPol)))
require.True(t, pdp.DependsOnClientAttrs())

// deleting this last policy -> no dependency on client attributes again
require.Nil(t, pdp.DeletePolicy(types.NamespacedName{Namespace: defaultNS, Name: allowFromSomeClientsPol.Name}, false))
require.False(t, pdp.DependsOnClientAttrs())

// adding a privileged policy that only allows clients with specific labels -> a dependency on client attributes
require.Nil(t, pdp.AddOrUpdatePolicy(connectivitypdp.PolicyFromPrivilegedCR(&allowFromSomeClientsPrivPol)))
require.True(t, pdp.DependsOnClientAttrs())
}

func TestDeleteNonexistingPolicies(t *testing.T) {
pdp := connectivitypdp.NewPDP()
err := pdp.DeletePolicy(types.NamespacedName{Name: "no-such-policy"}, true)
Expand Down
62 changes: 49 additions & 13 deletions pkg/controlplane/authz/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ const (
// the number of seconds a JWT access token is valid before it expires.
jwtExpirySeconds = 5

ClientNamespaceLabel = "clusterlink/metadata.clientNamespace"
ClientSALabel = "clusterlink/metadata.clientServiceAccount"
ClientLabelsPrefix = "client/metadata.labels."
ServiceNameLabel = "clusterlink/metadata.serviceName"
ServiceNamespaceLabel = "clusterlink/metadata.serviceNamespace"
ServiceLabelsPrefix = "service/metadata."
Expand Down Expand Up @@ -84,9 +87,10 @@ type ingressAuthorizationResponse struct {
}

type podInfo struct {
name string
namespace string
labels map[string]string
name string
namespace string
serviceAccount string
labels map[string]string
}

// Manager manages the authorization dataplane connections.
Expand Down Expand Up @@ -166,7 +170,12 @@ func (m *Manager) addPod(pod *v1.Pod) {
defer m.podLock.Unlock()

podID := types.NamespacedName{Name: pod.Name, Namespace: pod.Namespace}
m.podList[podID] = podInfo{name: pod.Name, namespace: pod.Namespace, labels: pod.Labels}
m.podList[podID] = podInfo{
name: pod.Name,
namespace: pod.Namespace,
labels: pod.Labels,
serviceAccount: pod.Spec.ServiceAccountName,
}
for _, ip := range pod.Status.PodIPs {
// ignoring host-networked Pod IPs
if ip.IP != pod.Status.HostIP {
Expand Down Expand Up @@ -211,19 +220,40 @@ func (m *Manager) getPodInfoByIP(ip string) *podInfo {
return nil
}

func (m *Manager) getClientAttributes(req *egressAuthorizationRequest) connectivitypdp.WorkloadAttrs {
podInfo := m.getPodInfoByIP(req.IP)
if podInfo == nil {
m.logger.Infof("Pod has no info: IP=%v.", req.IP)
return nil
}

clientAttrs := connectivitypdp.WorkloadAttrs{
GatewayNameLabel: m.getPeerName(),
ServiceNamespaceLabel: podInfo.namespace, // deprecated
ClientNamespaceLabel: podInfo.namespace,
ClientSALabel: podInfo.serviceAccount,
}

if src, ok := podInfo.labels["app"]; ok {
clientAttrs[ServiceNameLabel] = src // deprecated
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does it take to remove the deprecated attributes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think they are being used currently in some experiments, so better wait a bit.
They will not appear in the documentation.

}

for k, v := range podInfo.labels {
clientAttrs[ClientLabelsPrefix+k] = v
}

m.logger.Infof("Client attributes: %v.", clientAttrs)

return clientAttrs
}

// authorizeEgress authorizes a request for accessing an imported service.
func (m *Manager) authorizeEgress(ctx context.Context, req *egressAuthorizationRequest) (*egressAuthorizationResponse, error) {
m.logger.Infof("Received egress authorization request: %v.", req)

srcAttributes := connectivitypdp.WorkloadAttrs{GatewayNameLabel: m.getPeerName()}
podInfo := m.getPodInfoByIP(req.IP)
if podInfo != nil {
srcAttributes[ServiceNamespaceLabel] = podInfo.namespace

if src, ok := podInfo.labels["app"]; ok { // TODO: Add support for labels other than just the "app" key.
m.logger.Infof("Received egress authorization srcLabels[app]: %v.", podInfo.labels["app"])
srcAttributes[ServiceNameLabel] = src
}
srcAttributes := m.getClientAttributes(req)
if len(srcAttributes) == 0 && m.connectivityPDP.DependsOnClientAttrs() {
return nil, fmt.Errorf("failed to extract client attributes, however, access policies depend on such attributes")
}

var imp v1alpha1.Import
Expand Down Expand Up @@ -377,6 +407,12 @@ func (m *Manager) authorizeIngress(
dstAttributes[ServiceLabelsPrefix+k] = v
}

// do not allow requests from clients with no attributes if the PDP has attribute-dependent policies
if len(req.SrcAttributes) == 0 && m.connectivityPDP.DependsOnClientAttrs() {
resp.Allowed = false
return resp, nil
}

decision, err := m.connectivityPDP.Decide(req.SrcAttributes, dstAttributes, req.ServiceName.Namespace)
if err != nil {
return nil, fmt.Errorf("error deciding on an ingress connection: %w", err)
Expand Down
7 changes: 5 additions & 2 deletions tests/e2e/k8s/services/httpecho/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import (
"github.com/clusterlink-net/clusterlink/tests/e2e/k8s/util"
)

const EchoClientPodName = "echo-client"

func GetEchoValue(cluster *util.KindCluster, server *util.Service) (string, error) {
port, err := cluster.ExposeNodeport(server)
if err != nil {
Expand Down Expand Up @@ -76,11 +78,12 @@ func GetEchoValue(cluster *util.KindCluster, server *util.Service) (string, erro
}

func RunClientInPod(cluster *util.KindCluster, server *util.Service) (string, error) {
url := "http://" + server.Name
body, err := cluster.RunPod(&util.Pod{
Name: "echo-client",
Name: EchoClientPodName,
Namespace: server.Namespace,
Image: "curlimages/curl",
Args: []string{"curl", "-s", "-m", "1", "http://" + server.Name},
Args: []string{"curl", "-s", "-m", "10", "--retry", "10", "--retry-delay", "1", "--retry-all-errors", url},
})
return strings.TrimSpace(body), err
}
Loading
Loading