From c986bb6a8b1c9b030c5018b3118c8bb6c1108f69 Mon Sep 17 00:00:00 2001 From: yahire Date: Wed, 10 Jun 2026 23:08:45 -0400 Subject: [PATCH 01/11] Added penetrstion tests important to Telco partners/customers --- test/extended/security/penetration.go | 737 ++++++++++++++++++++++++++ 1 file changed, 737 insertions(+) create mode 100644 test/extended/security/penetration.go diff --git a/test/extended/security/penetration.go b/test/extended/security/penetration.go new file mode 100644 index 000000000000..72879f163bec --- /dev/null +++ b/test/extended/security/penetration.go @@ -0,0 +1,737 @@ +package security + +import ( + "context" + "fmt" + "regexp" + "strings" + + g "github.com/onsi/ginkgo/v2" + o "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper" + + exutil "github.com/openshift/origin/test/extended/util" +) + +var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { + defer g.GinkgoRecover() + oc := exutil.NewCLIWithoutNamespace("security-penetration") + + // CNF-18378: Check For Plain Text Passwords + g.It("TestNoPasswordExposedInLogFiles [apigroup:config.openshift.io]", func() { + ctx := context.Background() + + g.By("Getting master node names") + nodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{ + LabelSelector: "node-role.kubernetes.io/master", + }) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(len(nodes.Items)).To(o.BeNumerically(">", 0), "No master nodes found") + + g.By("Checking log files for plain text passwords") + foundInLogs := checkLogsForPasswords(oc, nodes.Items) + o.Expect(foundInLogs).To(o.BeEmpty(), fmt.Sprintf("Plain text passwords found in logs: %v", foundInLogs)) + + g.By("Checking YAML files for plain text passwords") + foundInYamls := checkYamlsForPasswords(oc, nodes.Items) + o.Expect(foundInYamls).To(o.BeEmpty(), fmt.Sprintf("Plain text passwords found in YAMLs: %v", foundInYamls)) + }) + + // CNF-21165: Check CNI SELinux From All Nodes + g.It("TestProperSELinuxContextOnCNI", func() { + ctx := context.Background() + + g.By("Getting all node names") + nodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Finding the actual CNI path") + cniPath, found := findCNIPath(oc, nodes.Items[0].Name) + if !found { + e2eskipper.Skipf("CNI directory not found on nodes, skipping SELinux context check") + } + + g.By("Checking SELinux context on all nodes") + for _, node := range nodes.Items { + checkSELinuxContext(oc, node.Name, cniPath) + } + }) + + // CNF-22599: Combined NRHO Security Penetration Tests + g.Describe("Security Penetration Tests", func() { + g.It("TestNoSSHKeysInUnexpectedSecrets [apigroup:security.openshift.io]", func() { + ctx := context.Background() + unexpectedSecrets := findSecretsContainingSSHKeys(ctx, oc) + o.Expect(unexpectedSecrets).To(o.BeEmpty(), + fmt.Sprintf("SSH private keys found in unexpected Secrets: %v", unexpectedSecrets)) + }) + + g.It("TestNoUnexpectedPrivilegedPods", func() { + ctx := context.Background() + privilegedPods := getPrivilegedPodsInUserNamespaces(ctx, oc) + o.Expect(privilegedPods).To(o.BeEmpty(), + fmt.Sprintf("Privileged pods found in user namespaces: %v", privilegedPods)) + }) + + g.It("TestProperNodeSudoConfiguration", func() { + ctx := context.Background() + nodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + unexpectedSudoers := findUnexpectedSudoersFiles(oc, nodes.Items) + o.Expect(unexpectedSudoers).To(o.BeEmpty(), + fmt.Sprintf("Unexpected sudoers files found: %v", unexpectedSudoers)) + }) + + ctx := context.Background() + g.It("TestEtcdBackupEncryptionAndRestriction [apigroup:config.openshift.io][apigroup:operator.openshift.io]", func() { + verifyEtcdEncryptionAtRest(ctx, oc) + + masterNodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{ + LabelSelector: "node-role.kubernetes.io/master", + }) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(len(masterNodes.Items)).To(o.BeNumerically(">", 0)) + + criticalFiles := findWorldReadableCriticalEtcdFiles(oc, masterNodes.Items[0].Name) + o.Expect(criticalFiles).To(o.BeEmpty(), + fmt.Sprintf("Critical etcd files are world-readable: %v", criticalFiles)) + }) + g.It("TestAllRoutesUseTLS [apigroup:route.openshift.io]", func() { + ctx := context.Background() + routesWithoutTLS := getRoutesWithoutTLS(ctx, oc) + o.Expect(routesWithoutTLS).To(o.BeEmpty(), + fmt.Sprintf("Routes without TLS found: %v", routesWithoutTLS)) + }) + + g.It("TestEtcdDirectoryPermissions [apigroup:operator.openshift.io]", func() { + masterNodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{ + LabelSelector: "node-role.kubernetes.io/master", + }) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(len(masterNodes.Items)).To(o.BeNumerically(">", 0)) + + problems := checkEtcdDirectoryPermissions(oc, masterNodes.Items[0].Name) + o.Expect(problems).To(o.BeEmpty(), + fmt.Sprintf("Etcd data directory permission issues: %v", problems)) + }) + + g.It("TestSecurityToolingInstalled [apigroup:operators.coreos.com]", func() { + foundOperators := checkSecurityOperators(ctx, oc) + o.Expect(foundOperators).NotTo(o.BeEmpty(), + "No security operators found (Compliance, File Integrity, or ACS/Stackrox)") + + auditProfile := getAuditLogProfile(ctx, oc) + g.By(fmt.Sprintf("Audit log profile: %s", auditProfile)) + }) + + g.It("TestMonitoringStackHealthy", func() { + notRunningPods := getNonRunningMonitoringPods(ctx, oc) + o.Expect(notRunningPods).To(o.BeEmpty(), + fmt.Sprintf("Non-running monitoring pods: %v", notRunningPods)) + + rulesCount := getPrometheusRulesCount(ctx, oc) + o.Expect(rulesCount).To(o.BeNumerically(">", 0), "No Prometheus rules found") + }) + + g.It("TestNetworkTrafficEncrypted [apigroup:operator.openshift.io]", func() { + etcdUsesTLS := verifyEtcdUsesTLS(ctx, oc) + o.Expect(etcdUsesTLS).To(o.BeTrue(), "Etcd is not using TLS certificates") + }) + + g.It("TestNoUnprotectedDatabasePods", func() { + dbPods := findDatabasePods(ctx, oc) + // This test is informational - it finds database pods for manual review + if len(dbPods) > 0 { + g.By(fmt.Sprintf("Database pods found (verify credentials use Secrets): %v", dbPods)) + } + }) + + g.It("TestNoUnexpectedClusterAdminServiceAccounts [apigroup:rbac.authorization.k8s.io]", func() { + bindings := getClusterAdminServiceAccountBindings(ctx, oc) + // This test is informational - review the bindings for unexpected entries + if len(bindings) > 0 { + g.By(fmt.Sprintf("ServiceAccounts with cluster-admin: %v", bindings)) + g.By("Review the above bindings for unexpected entries") + } + }) + + g.It("TestNoNFSVolumesRisk", func() { + nfsPVs := findNFSPersistentVolumes(ctx, oc) + // This test is informational - if NFS PVs exist, verify root_squash on NFS server + if len(nfsPVs) > 0 { + g.By(fmt.Sprintf("NFS PersistentVolumes found (verify root_squash on NFS server): %v", nfsPVs)) + } + }) + + g.It("TestContainerRegistryAuthentication [apigroup:config.openshift.io]", func() { + insecureRegistries := getInsecureRegistries(ctx, oc) + o.Expect(insecureRegistries).To(o.BeEmpty(), + fmt.Sprintf("Insecure registries found: %v", insecureRegistries)) + + registryRoute := getRegistryExternalRoute(ctx, oc) + if registryRoute != "" { + g.By(fmt.Sprintf("Registry external route: %s", registryRoute)) + } + }) + }) +}) + +// Helper functions for password/secret exposure detection + +func checkLogsForPasswords(oc *exutil.CLI, nodes []corev1.Node) []string { + var foundPasswords []string + + // These are test passwords that would be checked in real implementation + // In real scenario, these would come from cluster configuration + testPasswords := []string{ + // Placeholder - in real implementation, get from cluster config + } + + logPaths := []string{ + "/var/log/containers/*.log", + "/var/log/openshift-apiserver/*.log", + "/var/log/oauth-apiserver/*.log", + "/var/log/kube-apiserver/*.log", + "/var/log/ovn-kubernetes/*.log", + "/var/log/openvswitch/*.log", + "/var/log/audit/*.log", + "/var/log/rhsm/*.log", + "/var/log/lastlog*", + } + + for _, node := range nodes { + for _, pwd := range testPasswords { + for _, logPath := range logPaths { + cmd := fmt.Sprintf("grep -nl '%s' %s", pwd, logPath) + output, err := oc.AsAdmin().Run("debug").Args( + fmt.Sprintf("node/%s", node.Name), + "--", + "/bin/bash", "-c", + cmd, + ).Output() + + // RC 0 means found (bad), RC 1 means not found (good) + if err == nil && strings.TrimSpace(output) != "" { + foundPasswords = append(foundPasswords, + fmt.Sprintf("%s:PWD=***:DIR=%s", node.Name, output)) + } + } + } + } + + return foundPasswords +} + +func checkYamlsForPasswords(oc *exutil.CLI, nodes []corev1.Node) []string { + var foundPasswords []string + + testPasswords := []string{ + // Placeholder - in real implementation, get from cluster config + } + + yamlPaths := []string{ + "/etc/kubernetes/manifests/*.yaml", + "/etc/kubernetes/kubelet.conf", + "/var/lib/kubelet/config.json", + } + + for _, node := range nodes { + for _, pwd := range testPasswords { + for _, yamlPath := range yamlPaths { + cmd := fmt.Sprintf("grep -nl '%s' %s", pwd, yamlPath) + output, err := oc.AsAdmin().Run("debug").Args( + fmt.Sprintf("node/%s", node.Name), + "--", + "/bin/bash", "-c", + cmd, + ).Output() + + if err == nil && strings.TrimSpace(output) != "" { + foundPasswords = append(foundPasswords, + fmt.Sprintf("%s:PWD=***:DIR=%s", node.Name, output)) + } + } + } + } + + return foundPasswords +} + +// Helper functions for SELinux checks + +func findCNIPath(oc *exutil.CLI, nodeName string) (string, bool) { + cmd := "ls -ld /opt/cni /usr/libexec/cni" + output, err := oc.AsAdmin().Run("debug").Args( + fmt.Sprintf("node/%s", nodeName), + "--", + "/bin/bash", "-c", + cmd, + ).Output() + + if err != nil { + return "", false + } + + // Extract the actual path from symlink output + re := regexp.MustCompile(`drwxr[A-Za-z0-9\s\.\-]+(/usr/[a-z0-9/]+)`) + matches := re.FindStringSubmatch(output) + if len(matches) > 1 { + return matches[1], true + } + + // Check if output contains valid directory listing + if strings.Contains(output, "drwx") { + // Default to /usr/libexec/cni if we found directories but couldn't parse the path + return "/usr/libexec/cni", true + } + + return "", false +} + +func checkSELinuxContext(oc *exutil.CLI, nodeName, cniPath string) { + cmd := fmt.Sprintf("ls -RZ %s", cniPath) + output, err := oc.AsAdmin().Run("debug").Args( + fmt.Sprintf("node/%s", nodeName), + "--", + "/bin/bash", "-c", + cmd, + ).Output() + + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(output).To(o.ContainSubstring("bin_t"), + fmt.Sprintf("Wrong SELinux context on %s: bin_t is missing", nodeName)) + o.Expect(output).To(o.ContainSubstring("system_u"), + fmt.Sprintf("Wrong SELinux context on %s: system_u is missing", nodeName)) +} + +// Helper functions for penetration test suite + +func findSecretsContainingSSHKeys(ctx context.Context, oc *exutil.CLI) []string { + var results []string + + secrets, err := oc.AdminKubeClient().CoreV1().Secrets("").List(ctx, metav1.ListOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + for _, secret := range secrets.Items { + for key := range secret.Data { + lowerKey := strings.ToLower(key) + if strings.Contains(lowerKey, "ssh-privatekey") || + strings.Contains(lowerKey, "id_rsa") || + strings.Contains(lowerKey, "id_ed25519") { + results = append(results, + fmt.Sprintf("%s/%s (key: %s)", secret.Namespace, secret.Name, key)) + } + } + } + + return results +} + +func getPrivilegedPodsInUserNamespaces(ctx context.Context, oc *exutil.CLI) []string { + var privilegedPods []string + + pods, err := oc.AdminKubeClient().CoreV1().Pods("").List(ctx, metav1.ListOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + systemNamespaces := map[string]bool{ + "default": true, + "kube-system": true, + "kube-public": true, + "kube-node-lease": true, + "openshift": true, + "openshift-apiserver": true, + "openshift-authentication": true, + "openshift-cloud-credential-operator": true, + "openshift-cluster-version": true, + "openshift-config": true, + "openshift-config-managed": true, + "openshift-console": true, + "openshift-controller-manager": true, + "openshift-dns": true, + "openshift-etcd": true, + "openshift-image-registry": true, + "openshift-ingress": true, + "openshift-ingress-operator": true, + "openshift-kube-apiserver": true, + "openshift-kube-controller-manager": true, + "openshift-kube-scheduler": true, + "openshift-machine-api": true, + "openshift-machine-config-operator": true, + "openshift-marketplace": true, + "openshift-monitoring": true, + "openshift-multus": true, + "openshift-network-operator": true, + "openshift-node": true, + "openshift-operator-lifecycle-manager": true, + "openshift-sdn": true, + "openshift-service-ca": true, + "openshift-user-workload-monitoring": true, + } + + for _, pod := range pods.Items { + // Skip system namespaces and namespaces starting with known prefixes + ns := pod.Namespace + if systemNamespaces[ns] || + strings.HasPrefix(ns, "openshift-") || + strings.HasPrefix(ns, "kube-") || + strings.HasPrefix(ns, "portworx") || + strings.HasPrefix(ns, "rds-") { + continue + } + + for _, container := range pod.Spec.Containers { + if container.SecurityContext != nil && + container.SecurityContext.Privileged != nil && + *container.SecurityContext.Privileged { + privilegedPods = append(privilegedPods, + fmt.Sprintf("%s/%s", pod.Namespace, pod.Name)) + break + } + } + } + + return privilegedPods +} + +func findUnexpectedSudoersFiles(oc *exutil.CLI, nodes []corev1.Node) []string { + var unexpected []string + + for _, node := range nodes { + cmd := "ls /etc/sudoers.d/" + output, err := oc.AsAdmin().Run("debug").Args( + fmt.Sprintf("node/%s", node.Name), + "--", + "/bin/bash", "-c", + cmd, + ).Output() + + if err != nil { + continue + } + + lines := strings.Split(output, "\n") + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if trimmed == "" || + strings.Contains(trimmed, "Starting pod/") || + strings.Contains(trimmed, "chroot /host") || + strings.Contains(trimmed, "Removing debug pod") || + trimmed == "coreos-sudo-group" { + continue + } + unexpected = append(unexpected, fmt.Sprintf("%s: %s", node.Name, trimmed)) + } + } + + return unexpected +} + +func verifyEtcdEncryptionAtRest(ctx context.Context, oc *exutil.CLI) { + configClient := oc.AdminConfigClient() + apiserver, err := configClient.ConfigV1().APIServers().Get(ctx, "cluster", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + encType := "identity" + if apiserver.Spec.Encryption.Type != "" { + encType = string(apiserver.Spec.Encryption.Type) + } + + g.By(fmt.Sprintf("Encryption at rest type: %s", encType)) + o.Expect(encType).NotTo(o.Equal("identity"), + "Etcd encryption at rest is not enabled (type=identity)") +} + +func findWorldReadableCriticalEtcdFiles(oc *exutil.CLI, nodeName string) []string { + var critical []string + + cmd := "find /var/lib/etcd /home/core/assets/backup -perm -o=r -type f 2>/dev/null || true" + output, err := oc.AsAdmin().Run("debug").Args( + fmt.Sprintf("node/%s", nodeName), + "--", + "/bin/bash", "-c", + cmd, + ).Output() + + if err != nil { + return critical + } + + lines := strings.Split(output, "\n") + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if trimmed == "" || + strings.HasPrefix(trimmed, "Starting") || + strings.HasPrefix(trimmed, "Removing") || + strings.HasPrefix(trimmed, "To use") { + continue + } + + if strings.HasSuffix(trimmed, ".db") || + strings.HasSuffix(trimmed, ".wal") || + strings.HasSuffix(trimmed, ".tar.gz") { + critical = append(critical, trimmed) + } + } + + return critical +} + +func getRoutesWithoutTLS(ctx context.Context, oc *exutil.CLI) []string { + var noTLS []string + + routeClient := oc.AdminRouteClient().RouteV1() + routes, err := routeClient.Routes("").List(ctx, metav1.ListOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + for _, route := range routes.Items { + if route.Spec.TLS == nil { + noTLS = append(noTLS, + fmt.Sprintf("%s/%s -> %s", route.Namespace, route.Name, route.Spec.Host)) + } + } + + return noTLS +} + +func checkEtcdDirectoryPermissions(oc *exutil.CLI, nodeName string) []string { + var problems []string + + cmd := "find /var/lib/etcd -maxdepth 2 -perm -o=r 2>/dev/null" + output, err := oc.AsAdmin().Run("debug").Args( + fmt.Sprintf("node/%s", nodeName), + "--", + "/bin/bash", "-c", + cmd, + ).Output() + + if err != nil { + return problems + } + + lines := strings.Split(output, "\n") + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if trimmed == "" || + strings.Contains(trimmed, "Starting pod/") || + strings.Contains(trimmed, "chroot /host") || + strings.Contains(trimmed, "Removing debug pod") || + trimmed == "/var/lib/etcd" || + strings.HasSuffix(trimmed, ".json") { + continue + } + problems = append(problems, trimmed) + } + + return problems +} + +func checkSecurityOperators(ctx context.Context, oc *exutil.CLI) []string { + var found []string + + // Get CSVs (ClusterServiceVersions) from all namespaces + dynamicClient := oc.AdminDynamicClient() + csvGVR := schema.GroupVersionResource{ + Group: "operators.coreos.com", + Version: "v1alpha1", + Resource: "clusterserviceversions", + } + + csvList, err := dynamicClient.Resource(csvGVR).Namespace("").List(ctx, metav1.ListOptions{}) + if err != nil { + return found + } + + for _, csv := range csvList.Items { + name := csv.GetName() + lowerName := strings.ToLower(name) + + if strings.Contains(lowerName, "compliance") { + found = append(found, fmt.Sprintf("Compliance Operator: %s", name)) + } + if strings.Contains(lowerName, "file-integrity") { + found = append(found, fmt.Sprintf("File Integrity Operator: %s", name)) + } + if strings.Contains(lowerName, "stackrox") || strings.Contains(lowerName, "rhacs") { + found = append(found, fmt.Sprintf("ACS/Stackrox: %s", name)) + } + } + + return found +} + +func getAuditLogProfile(ctx context.Context, oc *exutil.CLI) string { + configClient := oc.AdminConfigClient() + apiserver, err := configClient.ConfigV1().APIServers().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + return "Unknown" + } + + if apiserver.Spec.Audit.Profile != "" { + return string(apiserver.Spec.Audit.Profile) + } + + return "Default" +} + +func getNonRunningMonitoringPods(ctx context.Context, oc *exutil.CLI) []string { + var notRunning []string + + pods, err := oc.AdminKubeClient().CoreV1().Pods("openshift-monitoring").List(ctx, metav1.ListOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + for _, pod := range pods.Items { + if pod.Status.Phase != corev1.PodRunning && pod.Status.Phase != corev1.PodSucceeded { + notRunning = append(notRunning, fmt.Sprintf("%s (%s)", pod.Name, pod.Status.Phase)) + } + } + + return notRunning +} + +func getPrometheusRulesCount(ctx context.Context, oc *exutil.CLI) int { + dynamicClient := oc.AdminDynamicClient() + rulesGVR := schema.GroupVersionResource{ + Group: "monitoring.coreos.com", + Version: "v1", + Resource: "prometheusrules", + } + + rulesList, err := dynamicClient.Resource(rulesGVR).Namespace("").List(ctx, metav1.ListOptions{}) + if err != nil { + return 0 + } + + return len(rulesList.Items) +} + +func verifyEtcdUsesTLS(ctx context.Context, oc *exutil.CLI) bool { + dynamicClient := oc.AdminDynamicClient() + etcdGVR := schema.GroupVersionResource{ + Group: "operator.openshift.io", + Version: "v1", + Resource: "etcds", + } + + etcd, err := dynamicClient.Resource(etcdGVR).Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + return false + } + + // Check spec for TLS/cert references + spec, found, err := unstructured.NestedMap(etcd.Object, "spec") + if err == nil && found { + specStr := fmt.Sprintf("%v", spec) + if strings.Contains(strings.ToLower(specStr), "cert") || + strings.Contains(strings.ToLower(specStr), "tls") { + return true + } + } + + // Check status/conditions for TLS/cert references + status, found, err := unstructured.NestedMap(etcd.Object, "status") + if err == nil && found { + statusStr := fmt.Sprintf("%v", status) + if strings.Contains(strings.ToLower(statusStr), "cert") || + strings.Contains(strings.ToLower(statusStr), "tls") { + return true + } + } + + return false +} + +func findDatabasePods(ctx context.Context, oc *exutil.CLI) []string { + var dbPods []string + + pods, err := oc.AdminKubeClient().CoreV1().Pods("").List(ctx, metav1.ListOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + dbImages := []string{"mysql", "postgres", "mongo", "mariadb"} + + for _, pod := range pods.Items { + for _, container := range pod.Spec.Containers { + lowerImage := strings.ToLower(container.Image) + for _, dbType := range dbImages { + if strings.Contains(lowerImage, dbType) { + dbPods = append(dbPods, + fmt.Sprintf("%s/%s (%s)", pod.Namespace, pod.Name, container.Image)) + break + } + } + } + } + + return dbPods +} + +func getClusterAdminServiceAccountBindings(ctx context.Context, oc *exutil.CLI) []string { + var adminBindings []string + + bindings, err := oc.AdminKubeClient().RbacV1().ClusterRoleBindings().List(ctx, metav1.ListOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + for _, binding := range bindings.Items { + if binding.RoleRef.Name != "cluster-admin" { + continue + } + + for _, subject := range binding.Subjects { + if subject.Kind == "ServiceAccount" { + adminBindings = append(adminBindings, + fmt.Sprintf("%s: %s/%s", binding.Name, subject.Namespace, subject.Name)) + } + } + } + + return adminBindings +} + +func findNFSPersistentVolumes(ctx context.Context, oc *exutil.CLI) []string { + var nfsPVs []string + + pvs, err := oc.AdminKubeClient().CoreV1().PersistentVolumes().List(ctx, metav1.ListOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + for _, pv := range pvs.Items { + if pv.Spec.NFS != nil { + nfsPVs = append(nfsPVs, + fmt.Sprintf("%s: %s:%s", pv.Name, pv.Spec.NFS.Server, pv.Spec.NFS.Path)) + } + } + + return nfsPVs +} + +func getInsecureRegistries(ctx context.Context, oc *exutil.CLI) []string { + configClient := oc.AdminConfigClient() + imageConfig, err := configClient.ConfigV1().Images().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + return []string{} + } + + if imageConfig.Spec.RegistrySources.InsecureRegistries != nil { + return imageConfig.Spec.RegistrySources.InsecureRegistries + } + + return []string{} +} + +func getRegistryExternalRoute(ctx context.Context, oc *exutil.CLI) string { + routeClient := oc.AdminRouteClient().RouteV1() + routes, err := routeClient.Routes("openshift-image-registry").List(ctx, metav1.ListOptions{}) + if err != nil { + return "" + } + + if len(routes.Items) > 0 { + return routes.Items[0].Spec.Host + } + + return "" +} From 9f6c73b5af0297238321155ff8cdc837e78d70dd Mon Sep 17 00:00:00 2001 From: yahire Date: Thu, 11 Jun 2026 08:19:11 -0400 Subject: [PATCH 02/11] Fixed review comments by coderabbit --- test/extended/security/penetration.go | 59 ++++++++++++++++++--------- 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/test/extended/security/penetration.go b/test/extended/security/penetration.go index 72879f163bec..0d261509c4dd 100644 --- a/test/extended/security/penetration.go +++ b/test/extended/security/penetration.go @@ -49,6 +49,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { g.By("Getting all node names") nodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(len(nodes.Items)).To(o.BeNumerically(">", 0), "No nodes found") g.By("Finding the actual CNI path") cniPath, found := findCNIPath(oc, nodes.Items[0].Name) @@ -88,8 +89,8 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { fmt.Sprintf("Unexpected sudoers files found: %v", unexpectedSudoers)) }) - ctx := context.Background() g.It("TestEtcdBackupEncryptionAndRestriction [apigroup:config.openshift.io][apigroup:operator.openshift.io]", func() { + ctx := context.Background() verifyEtcdEncryptionAtRest(ctx, oc) masterNodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{ @@ -110,6 +111,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestEtcdDirectoryPermissions [apigroup:operator.openshift.io]", func() { + ctx := context.Background() masterNodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{ LabelSelector: "node-role.kubernetes.io/master", }) @@ -122,6 +124,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestSecurityToolingInstalled [apigroup:operators.coreos.com]", func() { + ctx := context.Background() foundOperators := checkSecurityOperators(ctx, oc) o.Expect(foundOperators).NotTo(o.BeEmpty(), "No security operators found (Compliance, File Integrity, or ACS/Stackrox)") @@ -131,6 +134,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestMonitoringStackHealthy", func() { + ctx := context.Background() notRunningPods := getNonRunningMonitoringPods(ctx, oc) o.Expect(notRunningPods).To(o.BeEmpty(), fmt.Sprintf("Non-running monitoring pods: %v", notRunningPods)) @@ -140,11 +144,13 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestNetworkTrafficEncrypted [apigroup:operator.openshift.io]", func() { + ctx := context.Background() etcdUsesTLS := verifyEtcdUsesTLS(ctx, oc) o.Expect(etcdUsesTLS).To(o.BeTrue(), "Etcd is not using TLS certificates") }) g.It("TestNoUnprotectedDatabasePods", func() { + ctx := context.Background() dbPods := findDatabasePods(ctx, oc) // This test is informational - it finds database pods for manual review if len(dbPods) > 0 { @@ -153,6 +159,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestNoUnexpectedClusterAdminServiceAccounts [apigroup:rbac.authorization.k8s.io]", func() { + ctx := context.Background() bindings := getClusterAdminServiceAccountBindings(ctx, oc) // This test is informational - review the bindings for unexpected entries if len(bindings) > 0 { @@ -162,6 +169,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestNoNFSVolumesRisk", func() { + ctx := context.Background() nfsPVs := findNFSPersistentVolumes(ctx, oc) // This test is informational - if NFS PVs exist, verify root_squash on NFS server if len(nfsPVs) > 0 { @@ -170,6 +178,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestContainerRegistryAuthentication [apigroup:config.openshift.io]", func() { + ctx := context.Background() insecureRegistries := getInsecureRegistries(ctx, oc) o.Expect(insecureRegistries).To(o.BeEmpty(), fmt.Sprintf("Insecure registries found: %v", insecureRegistries)) @@ -193,6 +202,11 @@ func checkLogsForPasswords(oc *exutil.CLI, nodes []corev1.Node) []string { // Placeholder - in real implementation, get from cluster config } + if len(testPasswords) == 0 { + // No passwords configured to check - skip scanning + return foundPasswords + } + logPaths := []string{ "/var/log/containers/*.log", "/var/log/openshift-apiserver/*.log", @@ -208,12 +222,11 @@ func checkLogsForPasswords(oc *exutil.CLI, nodes []corev1.Node) []string { for _, node := range nodes { for _, pwd := range testPasswords { for _, logPath := range logPaths { - cmd := fmt.Sprintf("grep -nl '%s' %s", pwd, logPath) + // Use grep directly without shell to avoid command injection output, err := oc.AsAdmin().Run("debug").Args( fmt.Sprintf("node/%s", node.Name), "--", - "/bin/bash", "-c", - cmd, + "/bin/grep", "-nl", pwd, logPath, ).Output() // RC 0 means found (bad), RC 1 means not found (good) @@ -235,6 +248,11 @@ func checkYamlsForPasswords(oc *exutil.CLI, nodes []corev1.Node) []string { // Placeholder - in real implementation, get from cluster config } + if len(testPasswords) == 0 { + // No passwords configured to check - skip scanning + return foundPasswords + } + yamlPaths := []string{ "/etc/kubernetes/manifests/*.yaml", "/etc/kubernetes/kubelet.conf", @@ -244,12 +262,11 @@ func checkYamlsForPasswords(oc *exutil.CLI, nodes []corev1.Node) []string { for _, node := range nodes { for _, pwd := range testPasswords { for _, yamlPath := range yamlPaths { - cmd := fmt.Sprintf("grep -nl '%s' %s", pwd, yamlPath) + // Use grep directly without shell to avoid command injection output, err := oc.AsAdmin().Run("debug").Args( fmt.Sprintf("node/%s", node.Name), "--", - "/bin/bash", "-c", - cmd, + "/bin/grep", "-nl", pwd, yamlPath, ).Output() if err == nil && strings.TrimSpace(output) != "" { @@ -279,7 +296,7 @@ func findCNIPath(oc *exutil.CLI, nodeName string) (string, bool) { } // Extract the actual path from symlink output - re := regexp.MustCompile(`drwxr[A-Za-z0-9\s\.\-]+(/usr/[a-z0-9/]+)`) + re := regexp.MustCompile(`d[rwx-]{9}\.?\s+\d+\s+\S+\s+\S+\s+\d+\s+\S+\s+\d+\s+\S+\s+(/usr/(?:bin|lib|libexec)[a-z0-9/_-]*)`) matches := re.FindStringSubmatch(output) if len(matches) > 1 { return matches[1], true @@ -623,23 +640,27 @@ func verifyEtcdUsesTLS(ctx context.Context, oc *exutil.CLI) bool { return false } - // Check spec for TLS/cert references - spec, found, err := unstructured.NestedMap(etcd.Object, "spec") + // Check spec.observedConfig.servingInfo for TLS configuration + servingInfo, found, err := unstructured.NestedMap(etcd.Object, "spec", "observedConfig", "servingInfo") if err == nil && found { - specStr := fmt.Sprintf("%v", spec) - if strings.Contains(strings.ToLower(specStr), "cert") || - strings.Contains(strings.ToLower(specStr), "tls") { + // Check for minTLSVersion field + if minTLSVersion, exists, _ := unstructured.NestedString(servingInfo, "minTLSVersion"); exists && minTLSVersion != "" { + return true + } + // Check for cipherSuites field + if cipherSuites, exists, _ := unstructured.NestedStringSlice(servingInfo, "cipherSuites"); exists && len(cipherSuites) > 0 { return true } } - // Check status/conditions for TLS/cert references - status, found, err := unstructured.NestedMap(etcd.Object, "status") + // Check for TLS-related fields in spec (certFile, keyFile, caFile, etc.) + spec, found, err := unstructured.NestedMap(etcd.Object, "spec") if err == nil && found { - statusStr := fmt.Sprintf("%v", status) - if strings.Contains(strings.ToLower(statusStr), "cert") || - strings.Contains(strings.ToLower(statusStr), "tls") { - return true + tlsFields := []string{"certFile", "keyFile", "caFile", "clientTLS", "peerTLS", "serverTLS"} + for _, field := range tlsFields { + if _, exists := spec[field]; exists { + return true + } } } From 3564ca45857bc00af65d99ebfc8d497504f85238 Mon Sep 17 00:00:00 2001 From: yahire Date: Thu, 11 Jun 2026 08:47:33 -0400 Subject: [PATCH 03/11] Fixed code to consider Hypershift and Microshift envs --- test/extended/security/penetration.go | 42 +++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/extended/security/penetration.go b/test/extended/security/penetration.go index 0d261509c4dd..03450bb01ce3 100644 --- a/test/extended/security/penetration.go +++ b/test/extended/security/penetration.go @@ -15,6 +15,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper" + configv1 "github.com/openshift/api/config/v1" exutil "github.com/openshift/origin/test/extended/util" ) @@ -26,6 +27,20 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { g.It("TestNoPasswordExposedInLogFiles [apigroup:config.openshift.io]", func() { ctx := context.Background() + // Skip for HyperShift - control plane is hosted separately + controlPlaneTopology, err := exutil.GetControlPlaneTopology(oc) + o.Expect(err).NotTo(o.HaveOccurred()) + if *controlPlaneTopology == configv1.ExternalTopologyMode { + e2eskipper.Skipf("HyperShift clusters with external control plane topology do not have master nodes in the data plane") + } + + // Skip for MicroShift - different architecture + isMicroShift, err := exutil.IsMicroShiftCluster(oc.AdminKubeClient()) + o.Expect(err).NotTo(o.HaveOccurred()) + if isMicroShift { + e2eskipper.Skipf("MicroShift clusters have a different architecture and do not follow the same node labeling conventions") + } + g.By("Getting master node names") nodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{ LabelSelector: "node-role.kubernetes.io/master", @@ -93,6 +108,19 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { ctx := context.Background() verifyEtcdEncryptionAtRest(ctx, oc) + // Skip master node checks for HyperShift and MicroShift + controlPlaneTopology, err := exutil.GetControlPlaneTopology(oc) + o.Expect(err).NotTo(o.HaveOccurred()) + if *controlPlaneTopology == configv1.ExternalTopologyMode { + e2eskipper.Skipf("HyperShift clusters with external control plane topology do not have master nodes in the data plane") + } + + isMicroShift, err := exutil.IsMicroShiftCluster(oc.AdminKubeClient()) + o.Expect(err).NotTo(o.HaveOccurred()) + if isMicroShift { + e2eskipper.Skipf("MicroShift clusters have a different architecture and do not follow the same node labeling conventions") + } + masterNodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{ LabelSelector: "node-role.kubernetes.io/master", }) @@ -112,6 +140,20 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { g.It("TestEtcdDirectoryPermissions [apigroup:operator.openshift.io]", func() { ctx := context.Background() + + // Skip master node checks for HyperShift and MicroShift + controlPlaneTopology, err := exutil.GetControlPlaneTopology(oc) + o.Expect(err).NotTo(o.HaveOccurred()) + if *controlPlaneTopology == configv1.ExternalTopologyMode { + e2eskipper.Skipf("HyperShift clusters with external control plane topology do not have master nodes in the data plane") + } + + isMicroShift, err := exutil.IsMicroShiftCluster(oc.AdminKubeClient()) + o.Expect(err).NotTo(o.HaveOccurred()) + if isMicroShift { + e2eskipper.Skipf("MicroShift clusters have a different architecture and do not follow the same node labeling conventions") + } + masterNodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{ LabelSelector: "node-role.kubernetes.io/master", }) From 54d5f12aec3a4d861b733c7a0fb0cc370bc6567f Mon Sep 17 00:00:00 2001 From: yahire Date: Thu, 11 Jun 2026 08:57:05 -0400 Subject: [PATCH 04/11] Modified tests not to expose sensitive info in logs --- test/extended/security/penetration.go | 92 ++++++++++++++------------- 1 file changed, 49 insertions(+), 43 deletions(-) diff --git a/test/extended/security/penetration.go b/test/extended/security/penetration.go index 03450bb01ce3..9e9e6af743a5 100644 --- a/test/extended/security/penetration.go +++ b/test/extended/security/penetration.go @@ -82,16 +82,16 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { g.Describe("Security Penetration Tests", func() { g.It("TestNoSSHKeysInUnexpectedSecrets [apigroup:security.openshift.io]", func() { ctx := context.Background() - unexpectedSecrets := findSecretsContainingSSHKeys(ctx, oc) - o.Expect(unexpectedSecrets).To(o.BeEmpty(), - fmt.Sprintf("SSH private keys found in unexpected Secrets: %v", unexpectedSecrets)) + unexpectedSecretCount := countSecretsContainingSSHKeys(ctx, oc) + o.Expect(unexpectedSecretCount).To(o.Equal(0), + fmt.Sprintf("Found %d unexpected Secret(s) containing SSH private keys (details redacted for security)", unexpectedSecretCount)) }) g.It("TestNoUnexpectedPrivilegedPods", func() { ctx := context.Background() - privilegedPods := getPrivilegedPodsInUserNamespaces(ctx, oc) - o.Expect(privilegedPods).To(o.BeEmpty(), - fmt.Sprintf("Privileged pods found in user namespaces: %v", privilegedPods)) + privilegedPodCount := countPrivilegedPodsInUserNamespaces(ctx, oc) + o.Expect(privilegedPodCount).To(o.Equal(0), + fmt.Sprintf("Found %d privileged pod(s) in user namespaces (details redacted for security)", privilegedPodCount)) }) g.It("TestProperNodeSudoConfiguration", func() { @@ -193,29 +193,29 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { g.It("TestNoUnprotectedDatabasePods", func() { ctx := context.Background() - dbPods := findDatabasePods(ctx, oc) + dbPodCount := countDatabasePods(ctx, oc) // This test is informational - it finds database pods for manual review - if len(dbPods) > 0 { - g.By(fmt.Sprintf("Database pods found (verify credentials use Secrets): %v", dbPods)) + if dbPodCount > 0 { + g.By(fmt.Sprintf("Found %d database pod(s) - verify credentials use Secrets (details redacted for security)", dbPodCount)) } }) g.It("TestNoUnexpectedClusterAdminServiceAccounts [apigroup:rbac.authorization.k8s.io]", func() { ctx := context.Background() - bindings := getClusterAdminServiceAccountBindings(ctx, oc) + bindingCount := countClusterAdminServiceAccountBindings(ctx, oc) // This test is informational - review the bindings for unexpected entries - if len(bindings) > 0 { - g.By(fmt.Sprintf("ServiceAccounts with cluster-admin: %v", bindings)) - g.By("Review the above bindings for unexpected entries") + if bindingCount > 0 { + g.By(fmt.Sprintf("Found %d ServiceAccount(s) with cluster-admin role (details redacted for security)", bindingCount)) + g.By("Review cluster-admin bindings for unexpected ServiceAccount entries") } }) g.It("TestNoNFSVolumesRisk", func() { ctx := context.Background() - nfsPVs := findNFSPersistentVolumes(ctx, oc) + nfsPVCount := countNFSPersistentVolumes(ctx, oc) // This test is informational - if NFS PVs exist, verify root_squash on NFS server - if len(nfsPVs) > 0 { - g.By(fmt.Sprintf("NFS PersistentVolumes found (verify root_squash on NFS server): %v", nfsPVs)) + if nfsPVCount > 0 { + g.By(fmt.Sprintf("Found %d NFS PersistentVolume(s) - verify root_squash is enabled on NFS servers (details redacted for security)", nfsPVCount)) } }) @@ -371,8 +371,10 @@ func checkSELinuxContext(oc *exutil.CLI, nodeName, cniPath string) { // Helper functions for penetration test suite -func findSecretsContainingSSHKeys(ctx context.Context, oc *exutil.CLI) []string { - var results []string +// countSecretsContainingSSHKeys returns the count of secrets containing SSH keys +// Details are not returned to avoid information disclosure in test logs +func countSecretsContainingSSHKeys(ctx context.Context, oc *exutil.CLI) int { + count := 0 secrets, err := oc.AdminKubeClient().CoreV1().Secrets("").List(ctx, metav1.ListOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) @@ -383,17 +385,19 @@ func findSecretsContainingSSHKeys(ctx context.Context, oc *exutil.CLI) []string if strings.Contains(lowerKey, "ssh-privatekey") || strings.Contains(lowerKey, "id_rsa") || strings.Contains(lowerKey, "id_ed25519") { - results = append(results, - fmt.Sprintf("%s/%s (key: %s)", secret.Namespace, secret.Name, key)) + count++ + break // Count each secret only once } } } - return results + return count } -func getPrivilegedPodsInUserNamespaces(ctx context.Context, oc *exutil.CLI) []string { - var privilegedPods []string +// countPrivilegedPodsInUserNamespaces returns the count of privileged pods in user namespaces +// Details are not returned to avoid information disclosure in test logs +func countPrivilegedPodsInUserNamespaces(ctx context.Context, oc *exutil.CLI) int { + count := 0 pods, err := oc.AdminKubeClient().CoreV1().Pods("").List(ctx, metav1.ListOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) @@ -448,14 +452,13 @@ func getPrivilegedPodsInUserNamespaces(ctx context.Context, oc *exutil.CLI) []st if container.SecurityContext != nil && container.SecurityContext.Privileged != nil && *container.SecurityContext.Privileged { - privilegedPods = append(privilegedPods, - fmt.Sprintf("%s/%s", pod.Namespace, pod.Name)) - break + count++ + break // Count each pod only once } } } - return privilegedPods + return count } func findUnexpectedSudoersFiles(oc *exutil.CLI, nodes []corev1.Node) []string { @@ -709,8 +712,10 @@ func verifyEtcdUsesTLS(ctx context.Context, oc *exutil.CLI) bool { return false } -func findDatabasePods(ctx context.Context, oc *exutil.CLI) []string { - var dbPods []string +// countDatabasePods returns the count of database pods +// Details are not returned to avoid information disclosure in test logs +func countDatabasePods(ctx context.Context, oc *exutil.CLI) int { + count := 0 pods, err := oc.AdminKubeClient().CoreV1().Pods("").List(ctx, metav1.ListOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) @@ -722,19 +727,20 @@ func findDatabasePods(ctx context.Context, oc *exutil.CLI) []string { lowerImage := strings.ToLower(container.Image) for _, dbType := range dbImages { if strings.Contains(lowerImage, dbType) { - dbPods = append(dbPods, - fmt.Sprintf("%s/%s (%s)", pod.Namespace, pod.Name, container.Image)) - break + count++ + break // Count each pod only once } } } } - return dbPods + return count } -func getClusterAdminServiceAccountBindings(ctx context.Context, oc *exutil.CLI) []string { - var adminBindings []string +// countClusterAdminServiceAccountBindings returns the count of ServiceAccounts with cluster-admin role +// Details are not returned to avoid information disclosure in test logs +func countClusterAdminServiceAccountBindings(ctx context.Context, oc *exutil.CLI) int { + count := 0 bindings, err := oc.AdminKubeClient().RbacV1().ClusterRoleBindings().List(ctx, metav1.ListOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) @@ -746,29 +752,29 @@ func getClusterAdminServiceAccountBindings(ctx context.Context, oc *exutil.CLI) for _, subject := range binding.Subjects { if subject.Kind == "ServiceAccount" { - adminBindings = append(adminBindings, - fmt.Sprintf("%s: %s/%s", binding.Name, subject.Namespace, subject.Name)) + count++ } } } - return adminBindings + return count } -func findNFSPersistentVolumes(ctx context.Context, oc *exutil.CLI) []string { - var nfsPVs []string +// countNFSPersistentVolumes returns the count of NFS-backed PersistentVolumes +// Details are not returned to avoid information disclosure in test logs +func countNFSPersistentVolumes(ctx context.Context, oc *exutil.CLI) int { + count := 0 pvs, err := oc.AdminKubeClient().CoreV1().PersistentVolumes().List(ctx, metav1.ListOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) for _, pv := range pvs.Items { if pv.Spec.NFS != nil { - nfsPVs = append(nfsPVs, - fmt.Sprintf("%s: %s:%s", pv.Name, pv.Spec.NFS.Server, pv.Spec.NFS.Path)) + count++ } } - return nfsPVs + return count } func getInsecureRegistries(ctx context.Context, oc *exutil.CLI) []string { From 3797039cefc463dac93b95fed5536211393d7d28 Mon Sep 17 00:00:00 2001 From: yahire Date: Thu, 11 Jun 2026 09:08:39 -0400 Subject: [PATCH 05/11] Addressed more review comments --- test/extended/security/penetration.go | 56 +++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 7 deletions(-) diff --git a/test/extended/security/penetration.go b/test/extended/security/penetration.go index 9e9e6af743a5..b098e8e0b1c8 100644 --- a/test/extended/security/penetration.go +++ b/test/extended/security/penetration.go @@ -127,9 +127,13 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { o.Expect(err).NotTo(o.HaveOccurred()) o.Expect(len(masterNodes.Items)).To(o.BeNumerically(">", 0)) - criticalFiles := findWorldReadableCriticalEtcdFiles(oc, masterNodes.Items[0].Name) - o.Expect(criticalFiles).To(o.BeEmpty(), - fmt.Sprintf("Critical etcd files are world-readable: %v", criticalFiles)) + var allCriticalFiles []string + for _, node := range masterNodes.Items { + criticalFiles := findWorldReadableCriticalEtcdFiles(oc, node.Name) + allCriticalFiles = append(allCriticalFiles, criticalFiles...) + } + o.Expect(allCriticalFiles).To(o.BeEmpty(), + fmt.Sprintf("Critical etcd files are world-readable: %v", allCriticalFiles)) }) g.It("TestAllRoutesUseTLS [apigroup:route.openshift.io]", func() { ctx := context.Background() @@ -160,9 +164,13 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { o.Expect(err).NotTo(o.HaveOccurred()) o.Expect(len(masterNodes.Items)).To(o.BeNumerically(">", 0)) - problems := checkEtcdDirectoryPermissions(oc, masterNodes.Items[0].Name) - o.Expect(problems).To(o.BeEmpty(), - fmt.Sprintf("Etcd data directory permission issues: %v", problems)) + var allProblems []string + for _, node := range masterNodes.Items { + problems := checkEtcdDirectoryPermissions(oc, node.Name) + allProblems = append(allProblems, problems...) + } + o.Expect(allProblems).To(o.BeEmpty(), + fmt.Sprintf("Etcd data directory permission issues: %v", allProblems)) }) g.It("TestSecurityToolingInstalled [apigroup:operators.coreos.com]", func() { @@ -448,12 +456,43 @@ func countPrivilegedPodsInUserNamespaces(ctx context.Context, oc *exutil.CLI) in continue } + privilegedFound := false + + // Check regular containers for _, container := range pod.Spec.Containers { if container.SecurityContext != nil && container.SecurityContext.Privileged != nil && *container.SecurityContext.Privileged { count++ - break // Count each pod only once + privilegedFound = true + break + } + } + if privilegedFound { + continue + } + + // Check init containers + for _, container := range pod.Spec.InitContainers { + if container.SecurityContext != nil && + container.SecurityContext.Privileged != nil && + *container.SecurityContext.Privileged { + count++ + privilegedFound = true + break + } + } + if privilegedFound { + continue + } + + // Check ephemeral containers + for _, container := range pod.Spec.EphemeralContainers { + if container.SecurityContext != nil && + container.SecurityContext.Privileged != nil && + *container.SecurityContext.Privileged { + count++ + break } } } @@ -474,6 +513,9 @@ func findUnexpectedSudoersFiles(oc *exutil.CLI, nodes []corev1.Node) []string { ).Output() if err != nil { + // Record inspection failure instead of silently skipping + unexpected = append(unexpected, + fmt.Sprintf("%s: failed to inspect sudoers.d (error: %v)", node.Name, err)) continue } From d907310095487a177346d91f0037cd9163458bb6 Mon Sep 17 00:00:00 2001 From: yahire Date: Thu, 11 Jun 2026 11:44:26 -0400 Subject: [PATCH 06/11] Addressed more review comments --- test/extended/security/penetration.go | 29 ++++++++++++++------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/test/extended/security/penetration.go b/test/extended/security/penetration.go index b098e8e0b1c8..bbbd0a0b704f 100644 --- a/test/extended/security/penetration.go +++ b/test/extended/security/penetration.go @@ -202,29 +202,25 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { g.It("TestNoUnprotectedDatabasePods", func() { ctx := context.Background() dbPodCount := countDatabasePods(ctx, oc) - // This test is informational - it finds database pods for manual review - if dbPodCount > 0 { - g.By(fmt.Sprintf("Found %d database pod(s) - verify credentials use Secrets (details redacted for security)", dbPodCount)) - } + // Fail if database pods are found - they should use Secrets for credentials + o.Expect(dbPodCount).To(o.Equal(0), + fmt.Sprintf("Found %d database pod(s) - verify credentials use Secrets (details redacted for security)", dbPodCount)) }) g.It("TestNoUnexpectedClusterAdminServiceAccounts [apigroup:rbac.authorization.k8s.io]", func() { ctx := context.Background() bindingCount := countClusterAdminServiceAccountBindings(ctx, oc) - // This test is informational - review the bindings for unexpected entries - if bindingCount > 0 { - g.By(fmt.Sprintf("Found %d ServiceAccount(s) with cluster-admin role (details redacted for security)", bindingCount)) - g.By("Review cluster-admin bindings for unexpected ServiceAccount entries") - } + // Fail if unexpected cluster-admin service accounts are found + o.Expect(bindingCount).To(o.Equal(0), + fmt.Sprintf("Found %d ServiceAccount(s) with cluster-admin role - review for unexpected entries (details redacted for security)", bindingCount)) }) g.It("TestNoNFSVolumesRisk", func() { ctx := context.Background() nfsPVCount := countNFSPersistentVolumes(ctx, oc) - // This test is informational - if NFS PVs exist, verify root_squash on NFS server - if nfsPVCount > 0 { - g.By(fmt.Sprintf("Found %d NFS PersistentVolume(s) - verify root_squash is enabled on NFS servers (details redacted for security)", nfsPVCount)) - } + // Fail if NFS PVs are found - verify root_squash is enabled on NFS servers + o.Expect(nfsPVCount).To(o.Equal(0), + fmt.Sprintf("Found %d NFS PersistentVolume(s) - verify root_squash is enabled on NFS servers (details redacted for security)", nfsPVCount)) }) g.It("TestContainerRegistryAuthentication [apigroup:config.openshift.io]", func() { @@ -765,14 +761,19 @@ func countDatabasePods(ctx context.Context, oc *exutil.CLI) int { dbImages := []string{"mysql", "postgres", "mongo", "mariadb"} for _, pod := range pods.Items { + found := false for _, container := range pod.Spec.Containers { lowerImage := strings.ToLower(container.Image) for _, dbType := range dbImages { if strings.Contains(lowerImage, dbType) { count++ - break // Count each pod only once + found = true + break } } + if found { + break // Count each pod only once + } } } From 03435dc2c2bcf0ef3b19db24981b854b5b43b73b Mon Sep 17 00:00:00 2001 From: yahire Date: Thu, 11 Jun 2026 18:12:03 -0400 Subject: [PATCH 07/11] Addressed latest review comments --- test/extended/security/penetration.go | 51 ++++++++++++++------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/test/extended/security/penetration.go b/test/extended/security/penetration.go index bbbd0a0b704f..eaec864f37e1 100644 --- a/test/extended/security/penetration.go +++ b/test/extended/security/penetration.go @@ -137,9 +137,9 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestAllRoutesUseTLS [apigroup:route.openshift.io]", func() { ctx := context.Background() - routesWithoutTLS := getRoutesWithoutTLS(ctx, oc) - o.Expect(routesWithoutTLS).To(o.BeEmpty(), - fmt.Sprintf("Routes without TLS found: %v", routesWithoutTLS)) + routesWithoutTLSCount := countRoutesWithoutTLS(ctx, oc) + o.Expect(routesWithoutTLSCount).To(o.Equal(0), + fmt.Sprintf("Found %d route(s) without TLS (details redacted for security)", routesWithoutTLSCount)) }) g.It("TestEtcdDirectoryPermissions [apigroup:operator.openshift.io]", func() { @@ -225,13 +225,13 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { g.It("TestContainerRegistryAuthentication [apigroup:config.openshift.io]", func() { ctx := context.Background() - insecureRegistries := getInsecureRegistries(ctx, oc) - o.Expect(insecureRegistries).To(o.BeEmpty(), - fmt.Sprintf("Insecure registries found: %v", insecureRegistries)) + insecureRegistryCount := countInsecureRegistries(ctx, oc) + o.Expect(insecureRegistryCount).To(o.Equal(0), + fmt.Sprintf("Found %d insecure registr(y/ies) (details redacted for security)", insecureRegistryCount)) - registryRoute := getRegistryExternalRoute(ctx, oc) - if registryRoute != "" { - g.By(fmt.Sprintf("Registry external route: %s", registryRoute)) + hasRegistryRoute := hasRegistryExternalRoute(ctx, oc) + if hasRegistryRoute { + g.By("Registry external route exists (details redacted for security)") } }) }) @@ -582,8 +582,10 @@ func findWorldReadableCriticalEtcdFiles(oc *exutil.CLI, nodeName string) []strin return critical } -func getRoutesWithoutTLS(ctx context.Context, oc *exutil.CLI) []string { - var noTLS []string +// countRoutesWithoutTLS returns the count of routes without TLS +// Details are not returned to avoid information disclosure in test logs +func countRoutesWithoutTLS(ctx context.Context, oc *exutil.CLI) int { + count := 0 routeClient := oc.AdminRouteClient().RouteV1() routes, err := routeClient.Routes("").List(ctx, metav1.ListOptions{}) @@ -591,12 +593,11 @@ func getRoutesWithoutTLS(ctx context.Context, oc *exutil.CLI) []string { for _, route := range routes.Items { if route.Spec.TLS == nil { - noTLS = append(noTLS, - fmt.Sprintf("%s/%s -> %s", route.Namespace, route.Name, route.Spec.Host)) + count++ } } - return noTLS + return count } func checkEtcdDirectoryPermissions(oc *exutil.CLI, nodeName string) []string { @@ -820,30 +821,30 @@ func countNFSPersistentVolumes(ctx context.Context, oc *exutil.CLI) int { return count } -func getInsecureRegistries(ctx context.Context, oc *exutil.CLI) []string { +// countInsecureRegistries returns the count of insecure registries +// Details are not returned to avoid information disclosure in test logs +func countInsecureRegistries(ctx context.Context, oc *exutil.CLI) int { configClient := oc.AdminConfigClient() imageConfig, err := configClient.ConfigV1().Images().Get(ctx, "cluster", metav1.GetOptions{}) if err != nil { - return []string{} + return 0 } if imageConfig.Spec.RegistrySources.InsecureRegistries != nil { - return imageConfig.Spec.RegistrySources.InsecureRegistries + return len(imageConfig.Spec.RegistrySources.InsecureRegistries) } - return []string{} + return 0 } -func getRegistryExternalRoute(ctx context.Context, oc *exutil.CLI) string { +// hasRegistryExternalRoute returns whether an external registry route exists +// Details are not returned to avoid information disclosure in test logs +func hasRegistryExternalRoute(ctx context.Context, oc *exutil.CLI) bool { routeClient := oc.AdminRouteClient().RouteV1() routes, err := routeClient.Routes("openshift-image-registry").List(ctx, metav1.ListOptions{}) if err != nil { - return "" - } - - if len(routes.Items) > 0 { - return routes.Items[0].Spec.Host + return false } - return "" + return len(routes.Items) > 0 } From a098abcf8d8f51811d65844c699dd76ff336d40c Mon Sep 17 00:00:00 2001 From: yahire Date: Thu, 11 Jun 2026 20:51:30 -0400 Subject: [PATCH 08/11] Fixed gofmt formatting --- test/extended/security/penetration.go | 62 +++++++++++++-------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/test/extended/security/penetration.go b/test/extended/security/penetration.go index eaec864f37e1..36b8047a5862 100644 --- a/test/extended/security/penetration.go +++ b/test/extended/security/penetration.go @@ -407,38 +407,38 @@ func countPrivilegedPodsInUserNamespaces(ctx context.Context, oc *exutil.CLI) in o.Expect(err).NotTo(o.HaveOccurred()) systemNamespaces := map[string]bool{ - "default": true, - "kube-system": true, - "kube-public": true, - "kube-node-lease": true, - "openshift": true, - "openshift-apiserver": true, - "openshift-authentication": true, - "openshift-cloud-credential-operator": true, - "openshift-cluster-version": true, - "openshift-config": true, - "openshift-config-managed": true, - "openshift-console": true, - "openshift-controller-manager": true, - "openshift-dns": true, - "openshift-etcd": true, - "openshift-image-registry": true, - "openshift-ingress": true, - "openshift-ingress-operator": true, - "openshift-kube-apiserver": true, - "openshift-kube-controller-manager": true, - "openshift-kube-scheduler": true, - "openshift-machine-api": true, - "openshift-machine-config-operator": true, - "openshift-marketplace": true, - "openshift-monitoring": true, - "openshift-multus": true, - "openshift-network-operator": true, - "openshift-node": true, + "default": true, + "kube-system": true, + "kube-public": true, + "kube-node-lease": true, + "openshift": true, + "openshift-apiserver": true, + "openshift-authentication": true, + "openshift-cloud-credential-operator": true, + "openshift-cluster-version": true, + "openshift-config": true, + "openshift-config-managed": true, + "openshift-console": true, + "openshift-controller-manager": true, + "openshift-dns": true, + "openshift-etcd": true, + "openshift-image-registry": true, + "openshift-ingress": true, + "openshift-ingress-operator": true, + "openshift-kube-apiserver": true, + "openshift-kube-controller-manager": true, + "openshift-kube-scheduler": true, + "openshift-machine-api": true, + "openshift-machine-config-operator": true, + "openshift-marketplace": true, + "openshift-monitoring": true, + "openshift-multus": true, + "openshift-network-operator": true, + "openshift-node": true, "openshift-operator-lifecycle-manager": true, - "openshift-sdn": true, - "openshift-service-ca": true, - "openshift-user-workload-monitoring": true, + "openshift-sdn": true, + "openshift-service-ca": true, + "openshift-user-workload-monitoring": true, } for _, pod := range pods.Items { From a91462360dd4eef4a2f458b59c79c0a2ef61fc38 Mon Sep 17 00:00:00 2001 From: yahire Date: Fri, 12 Jun 2026 11:19:16 -0400 Subject: [PATCH 09/11] Made penetration tests informing for now --- test/extended/security/penetration.go | 37 +++++++++++++++------------ 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/test/extended/security/penetration.go b/test/extended/security/penetration.go index 36b8047a5862..5a220e2add4b 100644 --- a/test/extended/security/penetration.go +++ b/test/extended/security/penetration.go @@ -8,6 +8,7 @@ import ( g "github.com/onsi/ginkgo/v2" o "github.com/onsi/gomega" + ote "github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -24,7 +25,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { oc := exutil.NewCLIWithoutNamespace("security-penetration") // CNF-18378: Check For Plain Text Passwords - g.It("TestNoPasswordExposedInLogFiles [apigroup:config.openshift.io]", func() { + g.It("TestNoPasswordExposedInLogFiles [apigroup:config.openshift.io]", ote.Informing(), func() { ctx := context.Background() // Skip for HyperShift - control plane is hosted separately @@ -58,7 +59,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) // CNF-21165: Check CNI SELinux From All Nodes - g.It("TestProperSELinuxContextOnCNI", func() { + g.It("TestProperSELinuxContextOnCNI", ote.Informing(), func() { ctx := context.Background() g.By("Getting all node names") @@ -80,21 +81,21 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { // CNF-22599: Combined NRHO Security Penetration Tests g.Describe("Security Penetration Tests", func() { - g.It("TestNoSSHKeysInUnexpectedSecrets [apigroup:security.openshift.io]", func() { + g.It("TestNoSSHKeysInUnexpectedSecrets [apigroup:security.openshift.io]", ote.Informing(), func() { ctx := context.Background() unexpectedSecretCount := countSecretsContainingSSHKeys(ctx, oc) o.Expect(unexpectedSecretCount).To(o.Equal(0), fmt.Sprintf("Found %d unexpected Secret(s) containing SSH private keys (details redacted for security)", unexpectedSecretCount)) }) - g.It("TestNoUnexpectedPrivilegedPods", func() { + g.It("TestNoUnexpectedPrivilegedPods", ote.Informing(), func() { ctx := context.Background() privilegedPodCount := countPrivilegedPodsInUserNamespaces(ctx, oc) o.Expect(privilegedPodCount).To(o.Equal(0), fmt.Sprintf("Found %d privileged pod(s) in user namespaces (details redacted for security)", privilegedPodCount)) }) - g.It("TestProperNodeSudoConfiguration", func() { + g.It("TestProperNodeSudoConfiguration", ote.Informing(), func() { ctx := context.Background() nodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) @@ -104,7 +105,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { fmt.Sprintf("Unexpected sudoers files found: %v", unexpectedSudoers)) }) - g.It("TestEtcdBackupEncryptionAndRestriction [apigroup:config.openshift.io][apigroup:operator.openshift.io]", func() { + g.It("TestEtcdBackupEncryptionAndRestriction [apigroup:config.openshift.io][apigroup:operator.openshift.io]", ote.Informing(), func() { ctx := context.Background() verifyEtcdEncryptionAtRest(ctx, oc) @@ -135,14 +136,14 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { o.Expect(allCriticalFiles).To(o.BeEmpty(), fmt.Sprintf("Critical etcd files are world-readable: %v", allCriticalFiles)) }) - g.It("TestAllRoutesUseTLS [apigroup:route.openshift.io]", func() { + g.It("TestAllRoutesUseTLS [apigroup:route.openshift.io]", ote.Informing(), func() { ctx := context.Background() routesWithoutTLSCount := countRoutesWithoutTLS(ctx, oc) o.Expect(routesWithoutTLSCount).To(o.Equal(0), fmt.Sprintf("Found %d route(s) without TLS (details redacted for security)", routesWithoutTLSCount)) }) - g.It("TestEtcdDirectoryPermissions [apigroup:operator.openshift.io]", func() { + g.It("TestEtcdDirectoryPermissions [apigroup:operator.openshift.io]", ote.Informing(), func() { ctx := context.Background() // Skip master node checks for HyperShift and MicroShift @@ -173,7 +174,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { fmt.Sprintf("Etcd data directory permission issues: %v", allProblems)) }) - g.It("TestSecurityToolingInstalled [apigroup:operators.coreos.com]", func() { + g.It("TestSecurityToolingInstalled [apigroup:operators.coreos.com]", ote.Informing(), func() { ctx := context.Background() foundOperators := checkSecurityOperators(ctx, oc) o.Expect(foundOperators).NotTo(o.BeEmpty(), @@ -183,7 +184,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { g.By(fmt.Sprintf("Audit log profile: %s", auditProfile)) }) - g.It("TestMonitoringStackHealthy", func() { + g.It("TestMonitoringStackHealthy", ote.Informing(), func() { ctx := context.Background() notRunningPods := getNonRunningMonitoringPods(ctx, oc) o.Expect(notRunningPods).To(o.BeEmpty(), @@ -193,13 +194,13 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { o.Expect(rulesCount).To(o.BeNumerically(">", 0), "No Prometheus rules found") }) - g.It("TestNetworkTrafficEncrypted [apigroup:operator.openshift.io]", func() { + g.It("TestNetworkTrafficEncrypted [apigroup:operator.openshift.io]", ote.Informing(), func() { ctx := context.Background() etcdUsesTLS := verifyEtcdUsesTLS(ctx, oc) o.Expect(etcdUsesTLS).To(o.BeTrue(), "Etcd is not using TLS certificates") }) - g.It("TestNoUnprotectedDatabasePods", func() { + g.It("TestNoUnprotectedDatabasePods", ote.Informing(), func() { ctx := context.Background() dbPodCount := countDatabasePods(ctx, oc) // Fail if database pods are found - they should use Secrets for credentials @@ -207,7 +208,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { fmt.Sprintf("Found %d database pod(s) - verify credentials use Secrets (details redacted for security)", dbPodCount)) }) - g.It("TestNoUnexpectedClusterAdminServiceAccounts [apigroup:rbac.authorization.k8s.io]", func() { + g.It("TestNoUnexpectedClusterAdminServiceAccounts [apigroup:rbac.authorization.k8s.io]", ote.Informing(), func() { ctx := context.Background() bindingCount := countClusterAdminServiceAccountBindings(ctx, oc) // Fail if unexpected cluster-admin service accounts are found @@ -215,7 +216,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { fmt.Sprintf("Found %d ServiceAccount(s) with cluster-admin role - review for unexpected entries (details redacted for security)", bindingCount)) }) - g.It("TestNoNFSVolumesRisk", func() { + g.It("TestNoNFSVolumesRisk", ote.Informing(), func() { ctx := context.Background() nfsPVCount := countNFSPersistentVolumes(ctx, oc) // Fail if NFS PVs are found - verify root_squash is enabled on NFS servers @@ -223,7 +224,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { fmt.Sprintf("Found %d NFS PersistentVolume(s) - verify root_squash is enabled on NFS servers (details redacted for security)", nfsPVCount)) }) - g.It("TestContainerRegistryAuthentication [apigroup:config.openshift.io]", func() { + g.It("TestContainerRegistryAuthentication [apigroup:config.openshift.io]", ote.Informing(), func() { ctx := context.Background() insecureRegistryCount := countInsecureRegistries(ctx, oc) o.Expect(insecureRegistryCount).To(o.Equal(0), @@ -509,7 +510,11 @@ func findUnexpectedSudoersFiles(oc *exutil.CLI, nodes []corev1.Node) []string { ).Output() if err != nil { - // Record inspection failure instead of silently skipping + // If directory doesn't exist, that's fine - no unexpected sudoers files + if strings.Contains(output, "No such file or directory") { + continue + } + // Record other inspection failures unexpected = append(unexpected, fmt.Sprintf("%s: failed to inspect sudoers.d (error: %v)", node.Name, err)) continue From a4c7ba5297735d0ef3b5337c7afa9b277155f8ac Mon Sep 17 00:00:00 2001 From: yahire Date: Fri, 12 Jun 2026 12:23:51 -0400 Subject: [PATCH 10/11] Penetration tests will run only on Baremetal platform --- test/extended/security/penetration.go | 32 +++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/extended/security/penetration.go b/test/extended/security/penetration.go index 5a220e2add4b..a854733baf45 100644 --- a/test/extended/security/penetration.go +++ b/test/extended/security/penetration.go @@ -26,6 +26,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { // CNF-18378: Check For Plain Text Passwords g.It("TestNoPasswordExposedInLogFiles [apigroup:config.openshift.io]", ote.Informing(), func() { + skipIfNotBaremetal(oc) ctx := context.Background() // Skip for HyperShift - control plane is hosted separately @@ -60,6 +61,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { // CNF-21165: Check CNI SELinux From All Nodes g.It("TestProperSELinuxContextOnCNI", ote.Informing(), func() { + skipIfNotBaremetal(oc) ctx := context.Background() g.By("Getting all node names") @@ -82,6 +84,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { // CNF-22599: Combined NRHO Security Penetration Tests g.Describe("Security Penetration Tests", func() { g.It("TestNoSSHKeysInUnexpectedSecrets [apigroup:security.openshift.io]", ote.Informing(), func() { + skipIfNotBaremetal(oc) ctx := context.Background() unexpectedSecretCount := countSecretsContainingSSHKeys(ctx, oc) o.Expect(unexpectedSecretCount).To(o.Equal(0), @@ -89,6 +92,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestNoUnexpectedPrivilegedPods", ote.Informing(), func() { + skipIfNotBaremetal(oc) ctx := context.Background() privilegedPodCount := countPrivilegedPodsInUserNamespaces(ctx, oc) o.Expect(privilegedPodCount).To(o.Equal(0), @@ -96,6 +100,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestProperNodeSudoConfiguration", ote.Informing(), func() { + skipIfNotBaremetal(oc) ctx := context.Background() nodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) @@ -106,6 +111,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestEtcdBackupEncryptionAndRestriction [apigroup:config.openshift.io][apigroup:operator.openshift.io]", ote.Informing(), func() { + skipIfNotBaremetal(oc) ctx := context.Background() verifyEtcdEncryptionAtRest(ctx, oc) @@ -137,6 +143,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { fmt.Sprintf("Critical etcd files are world-readable: %v", allCriticalFiles)) }) g.It("TestAllRoutesUseTLS [apigroup:route.openshift.io]", ote.Informing(), func() { + skipIfNotBaremetal(oc) ctx := context.Background() routesWithoutTLSCount := countRoutesWithoutTLS(ctx, oc) o.Expect(routesWithoutTLSCount).To(o.Equal(0), @@ -144,6 +151,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestEtcdDirectoryPermissions [apigroup:operator.openshift.io]", ote.Informing(), func() { + skipIfNotBaremetal(oc) ctx := context.Background() // Skip master node checks for HyperShift and MicroShift @@ -175,6 +183,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestSecurityToolingInstalled [apigroup:operators.coreos.com]", ote.Informing(), func() { + skipIfNotBaremetal(oc) ctx := context.Background() foundOperators := checkSecurityOperators(ctx, oc) o.Expect(foundOperators).NotTo(o.BeEmpty(), @@ -185,6 +194,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestMonitoringStackHealthy", ote.Informing(), func() { + skipIfNotBaremetal(oc) ctx := context.Background() notRunningPods := getNonRunningMonitoringPods(ctx, oc) o.Expect(notRunningPods).To(o.BeEmpty(), @@ -195,12 +205,14 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestNetworkTrafficEncrypted [apigroup:operator.openshift.io]", ote.Informing(), func() { + skipIfNotBaremetal(oc) ctx := context.Background() etcdUsesTLS := verifyEtcdUsesTLS(ctx, oc) o.Expect(etcdUsesTLS).To(o.BeTrue(), "Etcd is not using TLS certificates") }) g.It("TestNoUnprotectedDatabasePods", ote.Informing(), func() { + skipIfNotBaremetal(oc) ctx := context.Background() dbPodCount := countDatabasePods(ctx, oc) // Fail if database pods are found - they should use Secrets for credentials @@ -209,6 +221,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestNoUnexpectedClusterAdminServiceAccounts [apigroup:rbac.authorization.k8s.io]", ote.Informing(), func() { + skipIfNotBaremetal(oc) ctx := context.Background() bindingCount := countClusterAdminServiceAccountBindings(ctx, oc) // Fail if unexpected cluster-admin service accounts are found @@ -217,6 +230,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestNoNFSVolumesRisk", ote.Informing(), func() { + skipIfNotBaremetal(oc) ctx := context.Background() nfsPVCount := countNFSPersistentVolumes(ctx, oc) // Fail if NFS PVs are found - verify root_squash is enabled on NFS servers @@ -225,6 +239,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { }) g.It("TestContainerRegistryAuthentication [apigroup:config.openshift.io]", ote.Informing(), func() { + skipIfNotBaremetal(oc) ctx := context.Background() insecureRegistryCount := countInsecureRegistries(ctx, oc) o.Expect(insecureRegistryCount).To(o.Equal(0), @@ -514,6 +529,10 @@ func findUnexpectedSudoersFiles(oc *exutil.CLI, nodes []corev1.Node) []string { if strings.Contains(output, "No such file or directory") { continue } + // Skip namespace errors - these are test infrastructure issues, not security findings + if strings.Contains(output, "unable to get namespace") || strings.Contains(output, "not found") { + continue + } // Record other inspection failures unexpected = append(unexpected, fmt.Sprintf("%s: failed to inspect sudoers.d (error: %v)", node.Name, err)) @@ -853,3 +872,16 @@ func hasRegistryExternalRoute(ctx context.Context, oc *exutil.CLI) bool { return len(routes.Items) > 0 } + +// skipIfNotBaremetal skips the test if not running on baremetal platform +func skipIfNotBaremetal(oc *exutil.CLI) { + g.By("checking platform type") + + infra, err := oc.AdminConfigClient().ConfigV1().Infrastructures().Get( + context.Background(), "cluster", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + if infra.Status.PlatformStatus.Type != configv1.BareMetalPlatformType { + e2eskipper.Skipf("Security penetration tests only run on baremetal platform") + } +} From ae2362afdd49096d207c06400be19907797a453f Mon Sep 17 00:00:00 2001 From: yahire Date: Tue, 16 Jun 2026 22:21:11 -0400 Subject: [PATCH 11/11] Rewrite password check test to match Robot Framework approach --- test/extended/security/penetration.go | 444 +++++++++++++++++++++----- 1 file changed, 356 insertions(+), 88 deletions(-) diff --git a/test/extended/security/penetration.go b/test/extended/security/penetration.go index a854733baf45..4e50fa94de7e 100644 --- a/test/extended/security/penetration.go +++ b/test/extended/security/penetration.go @@ -31,14 +31,14 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { // Skip for HyperShift - control plane is hosted separately controlPlaneTopology, err := exutil.GetControlPlaneTopology(oc) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get control plane topology") if *controlPlaneTopology == configv1.ExternalTopologyMode { e2eskipper.Skipf("HyperShift clusters with external control plane topology do not have master nodes in the data plane") } // Skip for MicroShift - different architecture isMicroShift, err := exutil.IsMicroShiftCluster(oc.AdminKubeClient()) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to check if cluster is MicroShift") if isMicroShift { e2eskipper.Skipf("MicroShift clusters have a different architecture and do not follow the same node labeling conventions") } @@ -47,7 +47,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { nodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{ LabelSelector: "node-role.kubernetes.io/master", }) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to list master nodes") o.Expect(len(nodes.Items)).To(o.BeNumerically(">", 0), "No master nodes found") g.By("Checking log files for plain text passwords") @@ -66,7 +66,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { g.By("Getting all node names") nodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to list nodes") o.Expect(len(nodes.Items)).To(o.BeNumerically(">", 0), "No nodes found") g.By("Finding the actual CNI path") @@ -103,7 +103,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { skipIfNotBaremetal(oc) ctx := context.Background() nodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to list nodes for sudo configuration check") unexpectedSudoers := findUnexpectedSudoersFiles(oc, nodes.Items) o.Expect(unexpectedSudoers).To(o.BeEmpty(), @@ -117,13 +117,13 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { // Skip master node checks for HyperShift and MicroShift controlPlaneTopology, err := exutil.GetControlPlaneTopology(oc) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get control plane topology") if *controlPlaneTopology == configv1.ExternalTopologyMode { e2eskipper.Skipf("HyperShift clusters with external control plane topology do not have master nodes in the data plane") } isMicroShift, err := exutil.IsMicroShiftCluster(oc.AdminKubeClient()) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to check if cluster is MicroShift") if isMicroShift { e2eskipper.Skipf("MicroShift clusters have a different architecture and do not follow the same node labeling conventions") } @@ -131,7 +131,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { masterNodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{ LabelSelector: "node-role.kubernetes.io/master", }) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to list master nodes") o.Expect(len(masterNodes.Items)).To(o.BeNumerically(">", 0)) var allCriticalFiles []string @@ -156,13 +156,13 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { // Skip master node checks for HyperShift and MicroShift controlPlaneTopology, err := exutil.GetControlPlaneTopology(oc) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get control plane topology") if *controlPlaneTopology == configv1.ExternalTopologyMode { e2eskipper.Skipf("HyperShift clusters with external control plane topology do not have master nodes in the data plane") } isMicroShift, err := exutil.IsMicroShiftCluster(oc.AdminKubeClient()) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to check if cluster is MicroShift") if isMicroShift { e2eskipper.Skipf("MicroShift clusters have a different architecture and do not follow the same node labeling conventions") } @@ -170,7 +170,7 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { masterNodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{ LabelSelector: "node-role.kubernetes.io/master", }) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to list master nodes") o.Expect(len(masterNodes.Items)).To(o.BeNumerically(">", 0)) var allProblems []string @@ -189,8 +189,11 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { o.Expect(foundOperators).NotTo(o.BeEmpty(), "No security operators found (Compliance, File Integrity, or ACS/Stackrox)") - auditProfile := getAuditLogProfile(ctx, oc) + auditProfile, err := getAuditLogProfile(ctx, oc) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get audit log profile") g.By(fmt.Sprintf("Audit log profile: %s", auditProfile)) + o.Expect(auditProfile).NotTo(o.Equal("None"), + "Audit log profile is set to None - auditing is disabled") }) g.It("TestMonitoringStackHealthy", ote.Informing(), func() { @@ -200,14 +203,16 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { o.Expect(notRunningPods).To(o.BeEmpty(), fmt.Sprintf("Non-running monitoring pods: %v", notRunningPods)) - rulesCount := getPrometheusRulesCount(ctx, oc) + rulesCount, err := getPrometheusRulesCount(ctx, oc) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get Prometheus rules") o.Expect(rulesCount).To(o.BeNumerically(">", 0), "No Prometheus rules found") }) g.It("TestNetworkTrafficEncrypted [apigroup:operator.openshift.io]", ote.Informing(), func() { skipIfNotBaremetal(oc) ctx := context.Background() - etcdUsesTLS := verifyEtcdUsesTLS(ctx, oc) + etcdUsesTLS, err := verifyEtcdUsesTLS(ctx, oc) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to verify etcd TLS configuration") o.Expect(etcdUsesTLS).To(o.BeTrue(), "Etcd is not using TLS certificates") }) @@ -241,11 +246,13 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { g.It("TestContainerRegistryAuthentication [apigroup:config.openshift.io]", ote.Informing(), func() { skipIfNotBaremetal(oc) ctx := context.Background() - insecureRegistryCount := countInsecureRegistries(ctx, oc) + insecureRegistryCount, err := countInsecureRegistries(ctx, oc) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get insecure registries") o.Expect(insecureRegistryCount).To(o.Equal(0), fmt.Sprintf("Found %d insecure registr(y/ies) (details redacted for security)", insecureRegistryCount)) - hasRegistryRoute := hasRegistryExternalRoute(ctx, oc) + hasRegistryRoute, err := hasRegistryExternalRoute(ctx, oc) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to check for registry external route") if hasRegistryRoute { g.By("Registry external route exists (details redacted for security)") } @@ -255,19 +262,55 @@ var _ = g.Describe("[sig-auth][Feature:SecurityPenetration] ", func() { // Helper functions for password/secret exposure detection -func checkLogsForPasswords(oc *exutil.CLI, nodes []corev1.Node) []string { - var foundPasswords []string +// collectPasswords retrieves actual password values from cluster secrets +// Returns a list of passwords to search for in logs and YAMLs +func collectPasswords(oc *exutil.CLI) ([]string, error) { + ctx := context.Background() + var passwords []string + seen := make(map[string]bool) // Track unique passwords + + // Try to get kubeadmin password from kube-system namespace + secret, err := oc.AdminKubeClient().CoreV1().Secrets("kube-system").Get( + ctx, "kubeadmin", metav1.GetOptions{}) + if err == nil && secret.Data != nil { + // The field name is 'kubeadmin' based on oc extract output + if pwd, ok := secret.Data["kubeadmin"]; ok && len(pwd) > 0 { + pwdStr := string(pwd) + if !seen[pwdStr] { + passwords = append(passwords, pwdStr) + seen[pwdStr] = true + } + } + } - // These are test passwords that would be checked in real implementation - // In real scenario, these would come from cluster configuration - testPasswords := []string{ - // Placeholder - in real implementation, get from cluster config + // Try to get BMC/redfish passwords from openshift-machine-api namespace + // Look for metal3-ironic-password secret specifically + secrets, err := oc.AdminKubeClient().CoreV1().Secrets("openshift-machine-api").List( + ctx, metav1.ListOptions{}) + if err == nil { + for _, secret := range secrets.Items { + // Look specifically for metal3-ironic-password or secrets with "password" in the name + if strings.Contains(strings.ToLower(secret.Name), "password") { + // Look for password field in BMC credentials + if pwd, ok := secret.Data["password"]; ok && len(pwd) > 0 { + pwdStr := string(pwd) + if !seen[pwdStr] { + passwords = append(passwords, pwdStr) + seen[pwdStr] = true + } + } + } + } } - if len(testPasswords) == 0 { - // No passwords configured to check - skip scanning - return foundPasswords + if len(passwords) > 0 { + g.By(fmt.Sprintf("Collected %d unique password(s) from cluster secrets", len(passwords))) } + return passwords, nil +} + +func checkLogsForPasswords(oc *exutil.CLI, nodes []corev1.Node) []string { + var foundPasswords []string logPaths := []string{ "/var/log/containers/*.log", @@ -281,71 +324,135 @@ func checkLogsForPasswords(oc *exutil.CLI, nodes []corev1.Node) []string { "/var/log/lastlog*", } - for _, node := range nodes { - for _, pwd := range testPasswords { + // Collect actual password values from the cluster + passwords, err := collectPasswords(oc) + if err != nil { + g.By(fmt.Sprintf("Warning: failed to collect passwords: %v", err)) + return foundPasswords + } + + if len(passwords) == 0 { + g.By("No passwords collected from cluster secrets - skipping password check") + return foundPasswords + } + + g.By(fmt.Sprintf("Checking %d password(s) across %d nodes", len(passwords), len(nodes))) + + // Check each password in each log path on each node + for pwdIdx, pwd := range passwords { + for _, node := range nodes { for _, logPath := range logPaths { - // Use grep directly without shell to avoid command injection + // Escape the password for single quotes in shell + escapedPwd := strings.ReplaceAll(pwd, "'", "'\\''") + + // Use chroot directly as the command, with sh -c for the grep + // Use -F for fixed string and -w for whole word to avoid false positives + cmd := fmt.Sprintf("sh -c 'grep -Fnl \"%s\" %s 2>/dev/null || true'", + escapedPwd, logPath) + output, err := oc.AsAdmin().Run("debug").Args( fmt.Sprintf("node/%s", node.Name), "--", - "/bin/grep", "-nl", pwd, logPath, + "chroot", "/host", + "/bin/sh", "-c", + cmd, ).Output() - // RC 0 means found (bad), RC 1 means not found (good) + // rc=0 means password was found, rc=1 means not found if err == nil && strings.TrimSpace(output) != "" { - foundPasswords = append(foundPasswords, - fmt.Sprintf("%s:PWD=***:DIR=%s", node.Name, output)) + lines := strings.Split(output, "\n") + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if trimmed == "" || + strings.Contains(trimmed, "Starting pod/") || + strings.Contains(trimmed, "chroot /host") || + strings.Contains(trimmed, "Removing debug pod") { + continue + } + // Password found in this file + foundPasswords = append(foundPasswords, + fmt.Sprintf("%s:PWD=%d:DIR=%s", node.Name, pwdIdx+1, trimmed)) + } } } } } + g.By(fmt.Sprintf("checkLogsForPasswords: found %d instances", len(foundPasswords))) return foundPasswords } func checkYamlsForPasswords(oc *exutil.CLI, nodes []corev1.Node) []string { var foundPasswords []string - testPasswords := []string{ - // Placeholder - in real implementation, get from cluster config + yamlPaths := []string{ + "/etc/kubernetes/manifests/*.yaml", + "/etc/kubernetes/kubelet.conf", + "/var/lib/kubelet/config.json", } - if len(testPasswords) == 0 { - // No passwords configured to check - skip scanning + // Collect actual password values from the cluster + passwords, err := collectPasswords(oc) + if err != nil { + g.By(fmt.Sprintf("Warning: failed to collect passwords: %v", err)) return foundPasswords } - yamlPaths := []string{ - "/etc/kubernetes/manifests/*.yaml", - "/etc/kubernetes/kubelet.conf", - "/var/lib/kubelet/config.json", + if len(passwords) == 0 { + g.By("No passwords collected from cluster secrets - skipping password check") + return foundPasswords } - for _, node := range nodes { - for _, pwd := range testPasswords { + g.By(fmt.Sprintf("Checking %d password(s) in YAMLs across %d nodes", len(passwords), len(nodes))) + + // Check each password in each YAML path on each node + for pwdIdx, pwd := range passwords { + for _, node := range nodes { for _, yamlPath := range yamlPaths { - // Use grep directly without shell to avoid command injection + // Escape the password for single quotes in shell + escapedPwd := strings.ReplaceAll(pwd, "'", "'\\''") + + // Use chroot directly as the command, with sh -c for the grep + // Use -F for fixed string and -w for whole word to avoid false positives + cmd := fmt.Sprintf("sh -c 'grep -Fnl \"%s\" %s 2>/dev/null || true'", + escapedPwd, yamlPath) + output, err := oc.AsAdmin().Run("debug").Args( fmt.Sprintf("node/%s", node.Name), "--", - "/bin/grep", "-nl", pwd, yamlPath, + "chroot", "/host", + "/bin/sh", "-c", + cmd, ).Output() + // rc=0 means password was found, rc=1 means not found if err == nil && strings.TrimSpace(output) != "" { - foundPasswords = append(foundPasswords, - fmt.Sprintf("%s:PWD=***:DIR=%s", node.Name, output)) + lines := strings.Split(output, "\n") + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if trimmed == "" || + strings.Contains(trimmed, "Starting pod/") || + strings.Contains(trimmed, "chroot /host") || + strings.Contains(trimmed, "Removing debug pod") { + continue + } + // Password found in this file + foundPasswords = append(foundPasswords, + fmt.Sprintf("%s:PWD=%d:DIR=%s", node.Name, pwdIdx+1, trimmed)) + } } } } } + g.By(fmt.Sprintf("checkYamlsForPasswords: found %d instances", len(foundPasswords))) return foundPasswords } // Helper functions for SELinux checks func findCNIPath(oc *exutil.CLI, nodeName string) (string, bool) { - cmd := "ls -ld /opt/cni /usr/libexec/cni" + cmd := "chroot /host ls -ld /opt/cni /usr/libexec/cni" output, err := oc.AsAdmin().Run("debug").Args( fmt.Sprintf("node/%s", nodeName), "--", @@ -367,14 +474,16 @@ func findCNIPath(oc *exutil.CLI, nodeName string) (string, bool) { // Check if output contains valid directory listing if strings.Contains(output, "drwx") { // Default to /usr/libexec/cni if we found directories but couldn't parse the path + g.By(fmt.Sprintf("findCNIPath: using default /usr/libexec/cni on %s", nodeName)) return "/usr/libexec/cni", true } + g.By(fmt.Sprintf("findCNIPath: CNI directory not found on %s", nodeName)) return "", false } func checkSELinuxContext(oc *exutil.CLI, nodeName, cniPath string) { - cmd := fmt.Sprintf("ls -RZ %s", cniPath) + cmd := fmt.Sprintf("chroot /host ls -RZ %s", cniPath) output, err := oc.AsAdmin().Run("debug").Args( fmt.Sprintf("node/%s", nodeName), "--", @@ -382,7 +491,7 @@ func checkSELinuxContext(oc *exutil.CLI, nodeName, cniPath string) { cmd, ).Output() - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), fmt.Sprintf("Failed to check SELinux context on %s", nodeName)) o.Expect(output).To(o.ContainSubstring("bin_t"), fmt.Sprintf("Wrong SELinux context on %s: bin_t is missing", nodeName)) o.Expect(output).To(o.ContainSubstring("system_u"), @@ -397,7 +506,7 @@ func countSecretsContainingSSHKeys(ctx context.Context, oc *exutil.CLI) int { count := 0 secrets, err := oc.AdminKubeClient().CoreV1().Secrets("").List(ctx, metav1.ListOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to list secrets") for _, secret := range secrets.Items { for key := range secret.Data { @@ -411,6 +520,7 @@ func countSecretsContainingSSHKeys(ctx context.Context, oc *exutil.CLI) int { } } + g.By(fmt.Sprintf("countSecretsContainingSSHKeys: found %d secrets", count)) return count } @@ -420,7 +530,7 @@ func countPrivilegedPodsInUserNamespaces(ctx context.Context, oc *exutil.CLI) in count := 0 pods, err := oc.AdminKubeClient().CoreV1().Pods("").List(ctx, metav1.ListOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to list pods") systemNamespaces := map[string]bool{ "default": true, @@ -462,9 +572,7 @@ func countPrivilegedPodsInUserNamespaces(ctx context.Context, oc *exutil.CLI) in ns := pod.Namespace if systemNamespaces[ns] || strings.HasPrefix(ns, "openshift-") || - strings.HasPrefix(ns, "kube-") || - strings.HasPrefix(ns, "portworx") || - strings.HasPrefix(ns, "rds-") { + strings.HasPrefix(ns, "kube-") { continue } @@ -509,6 +617,7 @@ func countPrivilegedPodsInUserNamespaces(ctx context.Context, oc *exutil.CLI) in } } + g.By(fmt.Sprintf("countPrivilegedPodsInUserNamespaces: found %d pods", count)) return count } @@ -516,7 +625,7 @@ func findUnexpectedSudoersFiles(oc *exutil.CLI, nodes []corev1.Node) []string { var unexpected []string for _, node := range nodes { - cmd := "ls /etc/sudoers.d/" + cmd := "chroot /host ls /etc/sudoers.d/" output, err := oc.AsAdmin().Run("debug").Args( fmt.Sprintf("node/%s", node.Name), "--", @@ -553,13 +662,14 @@ func findUnexpectedSudoersFiles(oc *exutil.CLI, nodes []corev1.Node) []string { } } + g.By(fmt.Sprintf("findUnexpectedSudoersFiles: found %d unexpected files", len(unexpected))) return unexpected } func verifyEtcdEncryptionAtRest(ctx context.Context, oc *exutil.CLI) { configClient := oc.AdminConfigClient() apiserver, err := configClient.ConfigV1().APIServers().Get(ctx, "cluster", metav1.GetOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get API server configuration for encryption check") encType := "identity" if apiserver.Spec.Encryption.Type != "" { @@ -574,7 +684,7 @@ func verifyEtcdEncryptionAtRest(ctx context.Context, oc *exutil.CLI) { func findWorldReadableCriticalEtcdFiles(oc *exutil.CLI, nodeName string) []string { var critical []string - cmd := "find /var/lib/etcd /home/core/assets/backup -perm -o=r -type f 2>/dev/null || true" + cmd := "chroot /host find /var/lib/etcd /home/core/assets/backup -perm -o=r -type f 2>/dev/null || true" output, err := oc.AsAdmin().Run("debug").Args( fmt.Sprintf("node/%s", nodeName), "--", @@ -583,6 +693,8 @@ func findWorldReadableCriticalEtcdFiles(oc *exutil.CLI, nodeName string) []strin ).Output() if err != nil { + critical = append(critical, + fmt.Sprintf("%s: failed to check world-readable etcd files (error: %v)", nodeName, err)) return critical } @@ -603,6 +715,7 @@ func findWorldReadableCriticalEtcdFiles(oc *exutil.CLI, nodeName string) []strin } } + g.By(fmt.Sprintf("findWorldReadableCriticalEtcdFiles: found %d files on %s", len(critical), nodeName)) return critical } @@ -613,7 +726,7 @@ func countRoutesWithoutTLS(ctx context.Context, oc *exutil.CLI) int { routeClient := oc.AdminRouteClient().RouteV1() routes, err := routeClient.Routes("").List(ctx, metav1.ListOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to list routes") for _, route := range routes.Items { if route.Spec.TLS == nil { @@ -621,13 +734,14 @@ func countRoutesWithoutTLS(ctx context.Context, oc *exutil.CLI) int { } } + g.By(fmt.Sprintf("countRoutesWithoutTLS: found %d routes", count)) return count } func checkEtcdDirectoryPermissions(oc *exutil.CLI, nodeName string) []string { var problems []string - cmd := "find /var/lib/etcd -maxdepth 2 -perm -o=r 2>/dev/null" + cmd := "chroot /host find /var/lib/etcd -maxdepth 2 -perm -o=r 2>/dev/null" output, err := oc.AsAdmin().Run("debug").Args( fmt.Sprintf("node/%s", nodeName), "--", @@ -636,6 +750,8 @@ func checkEtcdDirectoryPermissions(oc *exutil.CLI, nodeName string) []string { ).Output() if err != nil { + problems = append(problems, + fmt.Sprintf("%s: failed to check etcd directory permissions (error: %v)", nodeName, err)) return problems } @@ -653,6 +769,7 @@ func checkEtcdDirectoryPermissions(oc *exutil.CLI, nodeName string) []string { problems = append(problems, trimmed) } + g.By(fmt.Sprintf("checkEtcdDirectoryPermissions: found %d problems on %s", len(problems), nodeName)) return problems } @@ -669,6 +786,8 @@ func checkSecurityOperators(ctx context.Context, oc *exutil.CLI) []string { csvList, err := dynamicClient.Resource(csvGVR).Namespace("").List(ctx, metav1.ListOptions{}) if err != nil { + // Report the error so it doesn't silently appear as "no operators found" + found = append(found, fmt.Sprintf("ERROR: Failed to list ClusterServiceVersions: %v", err)) return found } @@ -687,28 +806,31 @@ func checkSecurityOperators(ctx context.Context, oc *exutil.CLI) []string { } } + g.By(fmt.Sprintf("checkSecurityOperators: found %d operators", len(found))) return found } -func getAuditLogProfile(ctx context.Context, oc *exutil.CLI) string { +func getAuditLogProfile(ctx context.Context, oc *exutil.CLI) (string, error) { configClient := oc.AdminConfigClient() apiserver, err := configClient.ConfigV1().APIServers().Get(ctx, "cluster", metav1.GetOptions{}) if err != nil { - return "Unknown" + return "", err } if apiserver.Spec.Audit.Profile != "" { - return string(apiserver.Spec.Audit.Profile) + g.By(fmt.Sprintf("getAuditLogProfile: %s", apiserver.Spec.Audit.Profile)) + return string(apiserver.Spec.Audit.Profile), nil } - return "Default" + g.By("getAuditLogProfile: Default") + return "Default", nil } func getNonRunningMonitoringPods(ctx context.Context, oc *exutil.CLI) []string { var notRunning []string pods, err := oc.AdminKubeClient().CoreV1().Pods("openshift-monitoring").List(ctx, metav1.ListOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to list monitoring pods") for _, pod := range pods.Items { if pod.Status.Phase != corev1.PodRunning && pod.Status.Phase != corev1.PodSucceeded { @@ -716,10 +838,11 @@ func getNonRunningMonitoringPods(ctx context.Context, oc *exutil.CLI) []string { } } + g.By(fmt.Sprintf("getNonRunningMonitoringPods: found %d pods", len(notRunning))) return notRunning } -func getPrometheusRulesCount(ctx context.Context, oc *exutil.CLI) int { +func getPrometheusRulesCount(ctx context.Context, oc *exutil.CLI) (int, error) { dynamicClient := oc.AdminDynamicClient() rulesGVR := schema.GroupVersionResource{ Group: "monitoring.coreos.com", @@ -729,13 +852,15 @@ func getPrometheusRulesCount(ctx context.Context, oc *exutil.CLI) int { rulesList, err := dynamicClient.Resource(rulesGVR).Namespace("").List(ctx, metav1.ListOptions{}) if err != nil { - return 0 + return 0, err } - return len(rulesList.Items) + count := len(rulesList.Items) + g.By(fmt.Sprintf("getPrometheusRulesCount: found %d rules", count)) + return count, nil } -func verifyEtcdUsesTLS(ctx context.Context, oc *exutil.CLI) bool { +func verifyEtcdUsesTLS(ctx context.Context, oc *exutil.CLI) (bool, error) { dynamicClient := oc.AdminDynamicClient() etcdGVR := schema.GroupVersionResource{ Group: "operator.openshift.io", @@ -745,7 +870,7 @@ func verifyEtcdUsesTLS(ctx context.Context, oc *exutil.CLI) bool { etcd, err := dynamicClient.Resource(etcdGVR).Get(ctx, "cluster", metav1.GetOptions{}) if err != nil { - return false + return false, err } // Check spec.observedConfig.servingInfo for TLS configuration @@ -753,11 +878,11 @@ func verifyEtcdUsesTLS(ctx context.Context, oc *exutil.CLI) bool { if err == nil && found { // Check for minTLSVersion field if minTLSVersion, exists, _ := unstructured.NestedString(servingInfo, "minTLSVersion"); exists && minTLSVersion != "" { - return true + return true, nil } // Check for cipherSuites field if cipherSuites, exists, _ := unstructured.NestedStringSlice(servingInfo, "cipherSuites"); exists && len(cipherSuites) > 0 { - return true + return true, nil } } @@ -767,12 +892,13 @@ func verifyEtcdUsesTLS(ctx context.Context, oc *exutil.CLI) bool { tlsFields := []string{"certFile", "keyFile", "caFile", "clientTLS", "peerTLS", "serverTLS"} for _, field := range tlsFields { if _, exists := spec[field]; exists { - return true + return true, nil } } } - return false + g.By("verifyEtcdUsesTLS: no TLS configuration found") + return false, nil } // countDatabasePods returns the count of database pods @@ -781,7 +907,7 @@ func countDatabasePods(ctx context.Context, oc *exutil.CLI) int { count := 0 pods, err := oc.AdminKubeClient().CoreV1().Pods("").List(ctx, metav1.ListOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to list pods") dbImages := []string{"mysql", "postgres", "mongo", "mariadb"} @@ -802,16 +928,107 @@ func countDatabasePods(ctx context.Context, oc *exutil.CLI) int { } } + g.By(fmt.Sprintf("countDatabasePods: found %d pods", count)) return count } -// countClusterAdminServiceAccountBindings returns the count of ServiceAccounts with cluster-admin role +// countClusterAdminServiceAccountBindings returns the count of unexpected ServiceAccounts with cluster-admin role +// Known system ServiceAccounts are excluded from the count // Details are not returned to avoid information disclosure in test logs func countClusterAdminServiceAccountBindings(ctx context.Context, oc *exutil.CLI) int { - count := 0 + // Known-good ServiceAccounts that legitimately need cluster-admin for platform operations + knownGoodServiceAccounts := map[string]map[string]bool{ + "kube-system": { + "attachdetach-controller": true, + "certificate-controller": true, + "clusterrole-aggregation-controller": true, + "cronjob-controller": true, + "daemon-set-controller": true, + "deployment-controller": true, + "disruption-controller": true, + "endpoint-controller": true, + "endpointslice-controller": true, + "endpointslicemirroring-controller": true, + "ephemeral-volume-controller": true, + "expand-controller": true, + "generic-garbage-collector": true, + "horizontal-pod-autoscaler": true, + "job-controller": true, + "namespace-controller": true, + "node-controller": true, + "persistent-volume-binder": true, + "pod-garbage-collector": true, + "pv-protection-controller": true, + "pvc-protection-controller": true, + "replicaset-controller": true, + "replication-controller": true, + "resourcequota-controller": true, + "service-account-controller": true, + "service-controller": true, + "statefulset-controller": true, + "ttl-after-finished-controller": true, + "ttl-controller": true, + }, + "openshift-kube-controller-manager": { + "kube-controller-manager": true, + }, + "openshift-cluster-version": { + "default": true, + }, + "openshift-config-operator": { + "openshift-config-operator": true, + }, + "openshift-controller-manager": { + "openshift-controller-manager": true, + }, + "openshift-kube-apiserver": { + "kube-apiserver": true, + }, + "openshift-kube-scheduler": { + "openshift-kube-scheduler": true, + }, + "openshift-apiserver": { + "openshift-apiserver-sa": true, + }, + "openshift-apiserver-operator": { + "openshift-apiserver-operator": true, + }, + "openshift-authentication-operator": { + "authentication-operator": true, + }, + "openshift-cluster-storage-operator": { + "cluster-storage-operator": true, + }, + "openshift-cluster-samples-operator": { + "cluster-samples-operator": true, + }, + "openshift-etcd-operator": { + "etcd-operator": true, + }, + "openshift-kube-controller-manager-operator": { + "kube-controller-manager-operator": true, + }, + "openshift-kube-apiserver-operator": { + "kube-apiserver-operator": true, + }, + "openshift-kube-scheduler-operator": { + "openshift-kube-scheduler-operator": true, + }, + "openshift-machine-config-operator": { + "machine-config-controller": true, + "machine-config-operator": true, + }, + "openshift-network-operator": { + "default": true, + }, + "openshift-operator-lifecycle-manager": { + "olm-operator-serviceaccount": true, + }, + } + count := 0 bindings, err := oc.AdminKubeClient().RbacV1().ClusterRoleBindings().List(ctx, metav1.ListOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to list cluster role bindings") for _, binding := range bindings.Items { if binding.RoleRef.Name != "cluster-admin" { @@ -820,11 +1037,24 @@ func countClusterAdminServiceAccountBindings(ctx context.Context, oc *exutil.CLI for _, subject := range binding.Subjects { if subject.Kind == "ServiceAccount" { + // Check if this is a known-good system ServiceAccount + namespace := subject.Namespace + name := subject.Name + + if nsMap, exists := knownGoodServiceAccounts[namespace]; exists { + if nsMap[name] { + // This is a known-good ServiceAccount, skip it + continue + } + } + + // This is an unexpected ServiceAccount with cluster-admin count++ } } } + g.By(fmt.Sprintf("countClusterAdminServiceAccountBindings: found %d unexpected bindings", count)) return count } @@ -834,7 +1064,7 @@ func countNFSPersistentVolumes(ctx context.Context, oc *exutil.CLI) int { count := 0 pvs, err := oc.AdminKubeClient().CoreV1().PersistentVolumes().List(ctx, metav1.ListOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to list persistent volumes") for _, pv := range pvs.Items { if pv.Spec.NFS != nil { @@ -842,46 +1072,84 @@ func countNFSPersistentVolumes(ctx context.Context, oc *exutil.CLI) int { } } + g.By(fmt.Sprintf("countNFSPersistentVolumes: found %d volumes", count)) return count } // countInsecureRegistries returns the count of insecure registries // Details are not returned to avoid information disclosure in test logs -func countInsecureRegistries(ctx context.Context, oc *exutil.CLI) int { +func countInsecureRegistries(ctx context.Context, oc *exutil.CLI) (int, error) { configClient := oc.AdminConfigClient() imageConfig, err := configClient.ConfigV1().Images().Get(ctx, "cluster", metav1.GetOptions{}) if err != nil { - return 0 + return 0, err } if imageConfig.Spec.RegistrySources.InsecureRegistries != nil { - return len(imageConfig.Spec.RegistrySources.InsecureRegistries) + count := len(imageConfig.Spec.RegistrySources.InsecureRegistries) + g.By(fmt.Sprintf("countInsecureRegistries: found %d registries", count)) + return count, nil } - return 0 + g.By("countInsecureRegistries: found 0 registries") + return 0, nil } // hasRegistryExternalRoute returns whether an external registry route exists // Details are not returned to avoid information disclosure in test logs -func hasRegistryExternalRoute(ctx context.Context, oc *exutil.CLI) bool { +func hasRegistryExternalRoute(ctx context.Context, oc *exutil.CLI) (bool, error) { routeClient := oc.AdminRouteClient().RouteV1() routes, err := routeClient.Routes("openshift-image-registry").List(ctx, metav1.ListOptions{}) if err != nil { - return false + return false, err } - return len(routes.Items) > 0 + hasRoute := len(routes.Items) > 0 + g.By(fmt.Sprintf("hasRegistryExternalRoute: %v", hasRoute)) + return hasRoute, nil } // skipIfNotBaremetal skips the test if not running on baremetal platform func skipIfNotBaremetal(oc *exutil.CLI) { + ctx := context.Background() g.By("checking platform type") infra, err := oc.AdminConfigClient().ConfigV1().Infrastructures().Get( - context.Background(), "cluster", metav1.GetOptions{}) + ctx, "cluster", metav1.GetOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) - if infra.Status.PlatformStatus.Type != configv1.BareMetalPlatformType { - e2eskipper.Skipf("Security penetration tests only run on baremetal platform") + platformType := infra.Status.PlatformStatus.Type + g.By(fmt.Sprintf("Detected platform type: %s", platformType)) + + // If platform is BareMetal, allow the test + if platformType == configv1.BareMetalPlatformType { + return } + + // If platform is None, check if it's SNO on baremetal + if platformType == configv1.NonePlatformType { + // Check if this is a Single Node OpenShift (SNO) + nodes, err := oc.AdminKubeClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to list nodes") + + if len(nodes.Items) == 1 { + g.By("Detected Single Node OpenShift (SNO)") + // For SNO, check if the single node is baremetal by looking at labels or annotations + node := nodes.Items[0] + + // Check for baremetal-related labels or annotations + if _, hasBMCLabel := node.Labels["metal3.io/bmc-address"]; hasBMCLabel { + g.By("SNO node has baremetal BMC label - allowing test") + return + } + + // Check infrastructure platformSpec for baremetal hints + if infra.Status.InfrastructureName != "" { + g.By(fmt.Sprintf("SNO infrastructure name: %s - allowing test", infra.Status.InfrastructureName)) + return + } + } + } + + e2eskipper.Skipf("Security penetration tests only run on baremetal platform (detected: %s)", platformType) }