From 52252c60f9914c99bc871d2bfcd33d7ee450cc14 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Fri, 31 Jul 2026 13:23:41 +0800 Subject: [PATCH] docs(security): point endpoint + credential docs at the real controls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two documentation-accuracy fixes surfaced while verifying the #299/#251 acceptance criteria; no behavior change. #299 (endpoint SSRF), acceptance #4 — the remoteControl.endpoint field godoc described only shape validation and never pointed at the control that actually bounds where the operator will dial. Add a note that confining the destination is an admin egress concern: the --remote-control-allowed-endpoint-hosts allow-list (empty = any, backward compatible) plus the egress NetworkPolicy. Flows into all four CRD copies via controller-gen. #251 (remoteControl.tls confused deputy) — the resolveRemoteControlTLS comment claimed to be "a partial mitigation, not an authorization boundary" and pointed only at a hypothetical future SubjectAccessReview. That is now stale and understates what shipped: the credentialRefPolicy ValidatingAdmissionPolicy (ADR-0009) IS the enforced boundary, requiring the CR's writer to hold `get` on the referenced Secret. Rewrite the comment to name it, while staying honest that the policy is opt-in and OFF by default, so the two in-resolver gates remain a partial mitigation out of the box. #251 stays open (automated restricted-user envtest and the default-posture decision are still outstanding). --- api/v1alpha1/ntncellconfig_types.go | 6 ++++ .../ntn.operators.dev_ntncellconfigs.yaml | 6 ++++ .../ntn.operators.dev_ntncellconfigs.yaml | 6 ++++ .../crd/ntncellconfigs.ntn.operators.dev.yaml | 6 ++++ .../controller/ntncellconfig_controller.go | 29 +++++++++++-------- .../ntncellconfigs-crd.yaml | 6 ++++ 6 files changed, 47 insertions(+), 12 deletions(-) diff --git a/api/v1alpha1/ntncellconfig_types.go b/api/v1alpha1/ntncellconfig_types.go index 821f69f3..53c93ed3 100644 --- a/api/v1alpha1/ntncellconfig_types.go +++ b/api/v1alpha1/ntncellconfig_types.go @@ -91,6 +91,12 @@ type RemoteControlRef struct { // 1-63) — a permanent admission error beats a silent tight-requeue on a mistyped // value. The pattern alone cannot bound the label/host length (a regex quantifier // would, but the DNS-1123 label form makes that unreadable), so CEL carries it. + // + // Shape validation does NOT restrict WHICH host the operator will dial; the endpoint is + // CR-author-controlled, so confining it is an admin egress control (SSRF), not a CRD rule. + // Set the operator flag --remote-control-allowed-endpoint-hosts to permit only sanctioned + // gNB hosts for the runtime push (empty = any, backward compatible), and pair it with the + // operator egress NetworkPolicy (config/network-policy/allow-egress-traffic.yaml). See #299. // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=261 // +kubebuilder:validation:Pattern=`^(\[[0-9a-fA-F:]+\]|[a-zA-Z0-9]([-a-zA-Z0-9]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([-a-zA-Z0-9]*[a-zA-Z0-9])?)*):[0-9]{1,5}$` diff --git a/bundle/manifests/ntn.operators.dev_ntncellconfigs.yaml b/bundle/manifests/ntn.operators.dev_ntncellconfigs.yaml index d2be1cb4..1b5cf254 100644 --- a/bundle/manifests/ntn.operators.dev_ntncellconfigs.yaml +++ b/bundle/manifests/ntn.operators.dev_ntncellconfigs.yaml @@ -860,6 +860,12 @@ spec: 1-63) — a permanent admission error beats a silent tight-requeue on a mistyped value. The pattern alone cannot bound the label/host length (a regex quantifier would, but the DNS-1123 label form makes that unreadable), so CEL carries it. + + Shape validation does NOT restrict WHICH host the operator will dial; the endpoint is + CR-author-controlled, so confining it is an admin egress control (SSRF), not a CRD rule. + Set the operator flag --remote-control-allowed-endpoint-hosts to permit only sanctioned + gNB hosts for the runtime push (empty = any, backward compatible), and pair it with the + operator egress NetworkPolicy (config/network-policy/allow-egress-traffic.yaml). See #299. maxLength: 261 minLength: 1 pattern: ^(\[[0-9a-fA-F:]+\]|[a-zA-Z0-9]([-a-zA-Z0-9]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([-a-zA-Z0-9]*[a-zA-Z0-9])?)*):[0-9]{1,5}$ diff --git a/config/crd/bases/ntn.operators.dev_ntncellconfigs.yaml b/config/crd/bases/ntn.operators.dev_ntncellconfigs.yaml index d2be1cb4..1b5cf254 100644 --- a/config/crd/bases/ntn.operators.dev_ntncellconfigs.yaml +++ b/config/crd/bases/ntn.operators.dev_ntncellconfigs.yaml @@ -860,6 +860,12 @@ spec: 1-63) — a permanent admission error beats a silent tight-requeue on a mistyped value. The pattern alone cannot bound the label/host length (a regex quantifier would, but the DNS-1123 label form makes that unreadable), so CEL carries it. + + Shape validation does NOT restrict WHICH host the operator will dial; the endpoint is + CR-author-controlled, so confining it is an admin egress control (SSRF), not a CRD rule. + Set the operator flag --remote-control-allowed-endpoint-hosts to permit only sanctioned + gNB hosts for the runtime push (empty = any, backward compatible), and pair it with the + operator egress NetworkPolicy (config/network-policy/allow-egress-traffic.yaml). See #299. maxLength: 261 minLength: 1 pattern: ^(\[[0-9a-fA-F:]+\]|[a-zA-Z0-9]([-a-zA-Z0-9]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([-a-zA-Z0-9]*[a-zA-Z0-9])?)*):[0-9]{1,5}$ diff --git a/dist/chart/templates/crd/ntncellconfigs.ntn.operators.dev.yaml b/dist/chart/templates/crd/ntncellconfigs.ntn.operators.dev.yaml index b3164b9f..e19f83d0 100644 --- a/dist/chart/templates/crd/ntncellconfigs.ntn.operators.dev.yaml +++ b/dist/chart/templates/crd/ntncellconfigs.ntn.operators.dev.yaml @@ -863,6 +863,12 @@ spec: 1-63) — a permanent admission error beats a silent tight-requeue on a mistyped value. The pattern alone cannot bound the label/host length (a regex quantifier would, but the DNS-1123 label form makes that unreadable), so CEL carries it. + + Shape validation does NOT restrict WHICH host the operator will dial; the endpoint is + CR-author-controlled, so confining it is an admin egress control (SSRF), not a CRD rule. + Set the operator flag --remote-control-allowed-endpoint-hosts to permit only sanctioned + gNB hosts for the runtime push (empty = any, backward compatible), and pair it with the + operator egress NetworkPolicy (config/network-policy/allow-egress-traffic.yaml). See #299. maxLength: 261 minLength: 1 pattern: ^(\[[0-9a-fA-F:]+\]|[a-zA-Z0-9]([-a-zA-Z0-9]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([-a-zA-Z0-9]*[a-zA-Z0-9])?)*):[0-9]{1,5}$ diff --git a/internal/controller/ntncellconfig_controller.go b/internal/controller/ntncellconfig_controller.go index b5a86f25..37f11e25 100644 --- a/internal/controller/ntncellconfig_controller.go +++ b/internal/controller/ntncellconfig_controller.go @@ -968,21 +968,26 @@ func (r *NTNCellConfigReconciler) resolveRemoteControlTLS( if err := reader.Get(ctx, client.ObjectKey{Namespace: namespace, Name: t.SecretName}, secret); err != nil { return nil, "", fmt.Errorf("reading remoteControl.tls secret %q: %w", t.SecretName, err) } - // SECURITY (partial confused-deputy MITIGATION, not an authorization boundary): the - // operator reads this Secret with its OWN cluster-wide secrets-get, on behalf of - // whoever authored the NTNCellConfig — who may not be able to read Secrets. Two gates - // raise the bar against a CR author pointing the operator at an arbitrary Secret and - // shipping its contents to a CR-controlled endpoint: + // SECURITY (confused deputy): the operator reads this Secret with its OWN cluster-wide + // secrets-get, on behalf of whoever authored the NTNCellConfig — who may not be able to + // read Secrets. The ENFORCED authorization boundary is the credentialRefPolicy + // ValidatingAdmissionPolicy (dist/chart, credentialRefPolicy.enable; ADR-0009, #251): + // when enabled the apiserver requires the CR's writer to hold `get` on the referenced + // Secret, so the writer cannot borrow a Secret it could not read itself. That policy is + // OPT-IN and OFF by default (enable=false), so out of the box the two gates below are a + // PARTIAL mitigation only — NOT an authorization boundary — raising the bar against a CR + // author pointing the operator at an arbitrary Secret and shipping its contents to a + // CR-controlled endpoint: // (1) refuse a Kubernetes API credential — a service-account or bootstrap token // Secret stores its apiserver token under the same "token" key we read; // (2) require the Secret's owner to opt it in via a label. - // Known LIMITS (a real per-CR/per-endpoint SubjectAccessReview or grant resource is a - // tracked follow-up): the opt-in is namespace-scoped — once labelled, ANY NTNCellConfig - // in the namespace may use the Secret; a principal with secrets `patch` but not `get` - // could add the label to a Secret it cannot read; and the type check does not stop an - // Opaque Secret from holding some other bearer token. Those limits are about WHICH Secret - // may be named; the CA-pinning gate further down bounds where its token may be SENT, - // which is what turns "named the wrong Secret" into something less than exfiltration. + // LIMITS of gates (1)/(2) — why they do not substitute for the policy: the label opt-in + // is namespace-scoped — once labelled, ANY NTNCellConfig in the namespace may use the + // Secret; a principal with secrets `patch` but not `get` could add the label to a Secret + // it cannot read; and the type check does not stop an Opaque Secret from holding some + // other bearer token. Those limits are about WHICH Secret may be named; the CA-pinning + // gate further down bounds where its token may be SENT, which is what turns "named the + // wrong Secret" into something less than exfiltration. switch secret.Type { case corev1.SecretTypeServiceAccountToken, secretTypeBootstrapToken: return nil, "", fmt.Errorf("remoteControl.tls secret %q has type %q — a Kubernetes API credential must not be used as a gNB remote-control secret", t.SecretName, secret.Type) diff --git a/nephio/packages/ntn-operators-crds/ntncellconfigs-crd.yaml b/nephio/packages/ntn-operators-crds/ntncellconfigs-crd.yaml index d2be1cb4..1b5cf254 100644 --- a/nephio/packages/ntn-operators-crds/ntncellconfigs-crd.yaml +++ b/nephio/packages/ntn-operators-crds/ntncellconfigs-crd.yaml @@ -860,6 +860,12 @@ spec: 1-63) — a permanent admission error beats a silent tight-requeue on a mistyped value. The pattern alone cannot bound the label/host length (a regex quantifier would, but the DNS-1123 label form makes that unreadable), so CEL carries it. + + Shape validation does NOT restrict WHICH host the operator will dial; the endpoint is + CR-author-controlled, so confining it is an admin egress control (SSRF), not a CRD rule. + Set the operator flag --remote-control-allowed-endpoint-hosts to permit only sanctioned + gNB hosts for the runtime push (empty = any, backward compatible), and pair it with the + operator egress NetworkPolicy (config/network-policy/allow-egress-traffic.yaml). See #299. maxLength: 261 minLength: 1 pattern: ^(\[[0-9a-fA-F:]+\]|[a-zA-Z0-9]([-a-zA-Z0-9]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([-a-zA-Z0-9]*[a-zA-Z0-9])?)*):[0-9]{1,5}$