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

k8s: Restrict configuring reserved:init policy via CNP #28007

Merged
merged 3 commits into from
Sep 8, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions Documentation/operations/upgrade.rst
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,11 @@ Annotations:
Upgrading from Cilium 1.14.x or earlier to 1.15.y or later does not
trigger this problem. Downgrading from Cilium 1.15.y or later to Cilium
1.14.x or earlier may trigger this problem.
* ``CiliumNetworkPolicy`` cannot match the ``reserved:init`` labels any more.
If you have ``CiliumNetworkPolicy`` resources that have a match for
labels ``reserved:init``, these policies must be converted to
``CiliumClusterwideNetworkPolicy`` by changing the resource type for the
policy.

.. _upgrade_cilium_cli_helm_mode:

Expand Down
2 changes: 1 addition & 1 deletion examples/policies/l4/init.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
kind: CiliumClusterwideNetworkPolicy
metadata:
name: init
specs:
Expand Down
16 changes: 9 additions & 7 deletions pkg/k8s/apis/cilium.io/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,16 @@ func getEndpointSelector(namespace string, labelSelector *slim_metav1.LabelSelec
// Those pods don't have any labels, so they don't have a namespace label either.
// Don't add a namespace label to those endpoint selectors, or we wouldn't be
// able to match on those pods.
if !matchesInit && !es.HasKey(podPrefixLbl) && !es.HasKey(podAnyPrefixLbl) {
if !es.HasKey(podPrefixLbl) && !es.HasKey(podAnyPrefixLbl) {
if namespace == "" {
// For a clusterwide policy if a namespace is not specified in the labels we add
// a selector to only match endpoints that contains a namespace label.
// This is to make sure that we are only allowing traffic for cilium managed k8s endpoints
// and even if a wildcard is provided in the selector we don't proceed with a truly
// empty(allow all) endpoint selector for the policy.
es.AddMatchExpression(podPrefixLbl, slim_metav1.LabelSelectorOpExists, []string{})
if !matchesInit {
es.AddMatchExpression(podPrefixLbl, slim_metav1.LabelSelectorOpExists, []string{})
}
} else {
es.AddMatch(podPrefixLbl, namespace)
}
Expand Down Expand Up @@ -301,11 +303,11 @@ func ParseToCiliumRule(namespace, name string, uid types.UID, r *api.Rule) *api.
// the policy is being stored, thus we add the namespace to
// the MatchLabels map.
//
// Policies applying on initializing pods are a special case.
// Those pods don't have any labels, so they don't have a namespace label either.
// Don't add a namespace label to those endpoint selectors, or we wouldn't be
// able to match on those pods.
if !retRule.EndpointSelector.HasKey(podInitLbl) && namespace != "" {
// Policies applying to all namespaces are a special case.
// Such policies can match on any traffic from Pods or Nodes,
// so it wouldn't make sense to inject a namespace match for
// those policies.
if namespace != "" {
userNamespace, present := r.EndpointSelector.GetMatch(podPrefixLbl)
if present && !namespacesAreValid(namespace, userNamespace) {
log.WithFields(logrus.Fields{
Expand Down
83 changes: 80 additions & 3 deletions pkg/k8s/apis/cilium.io/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,7 @@ func Test_ParseToCiliumRule(t *testing.T) {
// is for init policies.
name: "parse-init-policy",
args: args{
namespace: slim_metav1.NamespaceDefault,
uid: uuid,
uid: uuid,
rule: &api.Rule{
EndpointSelector: api.NewESFromMatchRequirements(
map[string]string{
Expand All @@ -178,14 +177,92 @@ func Test_ParseToCiliumRule(t *testing.T) {
labels.LabelArray{
{
Key: "io.cilium.k8s.policy.derived-from",
Value: "CiliumNetworkPolicy",
Value: "CiliumClusterwideNetworkPolicy",
Source: labels.LabelSourceK8s,
},
{
Key: "io.cilium.k8s.policy.name",
Value: "parse-init-policy",
Source: labels.LabelSourceK8s,
},
{
Key: "io.cilium.k8s.policy.uid",
Value: string(uuid),
Source: labels.LabelSourceK8s,
},
},
),
},
{
// CNP with endpoint selectors should always select the
// current namespace
name: "parse-init-policy-namespaced",
args: args{
namespace: slim_metav1.NamespaceDefault,
uid: uuid,
rule: &api.Rule{
EndpointSelector: api.NewESFromMatchRequirements(
nil,
[]slim_metav1.LabelSelectorRequirement{
{
Key: "reserved.init",
Operator: slim_metav1.LabelSelectorOpDoesNotExist,
},
},
),
Ingress: []api.IngressRule{
{
IngressCommonRule: api.IngressCommonRule{
FromEndpoints: []api.EndpointSelector{
{
LabelSelector: &slim_metav1.LabelSelector{},
},
},
},
},
},
},
},
want: api.NewRule().WithEndpointSelector(
api.NewESFromMatchRequirements(
map[string]string{
namespace: "default",
},
[]slim_metav1.LabelSelectorRequirement{
{
Key: "reserved.init",
Operator: slim_metav1.LabelSelectorOpDoesNotExist,
},
},
),
).WithIngressRules(
[]api.IngressRule{
{
IngressCommonRule: api.IngressCommonRule{
FromEndpoints: []api.EndpointSelector{
api.NewESFromK8sLabelSelector(
labels.LabelSourceK8sKeyPrefix,
&slim_metav1.LabelSelector{
MatchLabels: map[string]string{
k8sConst.PodNamespaceLabel: "default",
},
}),
},
},
},
},
).WithLabels(
labels.LabelArray{
{
Key: "io.cilium.k8s.policy.derived-from",
Value: "CiliumNetworkPolicy",
Source: labels.LabelSourceK8s,
},
{
Key: "io.cilium.k8s.policy.name",
Value: "parse-init-policy-namespaced",
Source: labels.LabelSourceK8s,
},
{
Key: "io.cilium.k8s.policy.namespace",
Value: "default",
Expand Down
53 changes: 52 additions & 1 deletion pkg/k8s/apis/cilium.io/v2/validator/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,30 @@ package validator
import (
"encoding/json"
"fmt"
"sync"

"github.com/cilium/cilium/pkg/k8s/apis/cilium.io/client"
cilium_v2 "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2"
"github.com/cilium/cilium/pkg/labels"
"github.com/cilium/cilium/pkg/logging/logfields"

"github.com/sirupsen/logrus"
apiextensionsinternal "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apiextensions-apiserver/pkg/apiserver/validation"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

"github.com/cilium/cilium/pkg/k8s/apis/cilium.io/client"
var (
// We can remove the check for this warning once 1.15 is the oldest supported Cilium version.
logInitPolicyCNP = "It seems you have a CiliumNetworkPolicy with a " +
"match on the 'reserved:init' labels. This label is not " +
"supported in CiliumNetworkPolicy any more. If you wish to " +
"define a policy for endpoints before they receive a full " +
"security identity, change the resource type for the policy " +
"to CiliumClusterwideNetworkPolicy."
errInitPolicyCNP = fmt.Errorf("CiliumNetworkPolicy incorrectly matches reserved:init label")
logOnce sync.Once
)

// NPValidator is a validator structure used to validate CNP and CCNP.
Expand Down Expand Up @@ -96,6 +113,10 @@ func (n *NPValidator) ValidateCNP(cnp *unstructured.Unstructured) error {
return err
}

if err := checkInitLabelsPolicy(cnp); err != nil {
return err
}

return nil
}

Expand All @@ -111,3 +132,33 @@ func (n *NPValidator) ValidateCCNP(ccnp *unstructured.Unstructured) error {

return nil
}

func checkInitLabelsPolicy(cnp *unstructured.Unstructured) error {
cnpBytes, err := cnp.MarshalJSON()
if err != nil {
return err
}

resCNP := cilium_v2.CiliumNetworkPolicy{}
err = json.Unmarshal(cnpBytes, &resCNP)
if err != nil {
return err
}

for _, spec := range append(resCNP.Specs, resCNP.Spec) {
if spec == nil {
continue
}
podInitLbl := labels.LabelSourceReservedKeyPrefix + labels.IDNameInit
if spec.EndpointSelector.HasKey(podInitLbl) {
logOnce.Do(func() {
log.WithFields(logrus.Fields{
logfields.CiliumNetworkPolicyName: cnp.GetName(),
}).Error(logInitPolicyCNP)
})
return errInitPolicyCNP
}
}

return nil
}
29 changes: 29 additions & 0 deletions pkg/k8s/apis/cilium.io/v2/validator/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,3 +469,32 @@ specs:
}
}
}

func (s *CNPValidationSuite) Test_GH28007(c *C) {
cnp := []byte(`apiVersion: cilium.io/v2
kind: CiliumNetworkPolicy
metadata:
name: exampleapp
namespace: examplens
spec:
egress:
- toEntities:
- world
endpointSelector:
matchExpressions:
- key: reserved:init
operator: DoesNotExist
`)
jsnByte, err := yaml.YAMLToJSON(cnp)
c.Assert(err, IsNil)

us := unstructured.Unstructured{}
err = json.Unmarshal(jsnByte, &us)
c.Assert(err, IsNil)

validator, err := NewNPValidator()
c.Assert(err, IsNil)
err = validator.ValidateCNP(&us)
// Err can't be nil since validation should detect the policy is not correct.
c.Assert(err, Equals, errInitPolicyCNP)
}