diff --git a/pkg/k8sutil/common.go b/pkg/k8sutil/common.go index 635c255..85dda24 100644 --- a/pkg/k8sutil/common.go +++ b/pkg/k8sutil/common.go @@ -1,8 +1,8 @@ package k8sutil import ( - "math/rand" - "time" + "crypto/rand" + "math/big" marklogicv1 "github.com/marklogic/marklogic-operator-kubernetes/api/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -108,10 +108,15 @@ func generateRandomAlphaNumeric(length int) string { const charset = "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" - seededRand := rand.New(rand.NewSource(time.Now().UnixNano())) result := make([]byte, length) for i := range result { - result[i] = charset[seededRand.Intn(len(charset))] + num, err := rand.Int(rand.Reader, big.NewInt(int64(len(charset)))) + if err != nil { + // Fallback to a deterministic character if crypto/rand fails + result[i] = charset[i%len(charset)] + } else { + result[i] = charset[num.Int64()] + } } return string(result) } diff --git a/pkg/k8sutil/handler.go b/pkg/k8sutil/handler.go index 2bc8fbb..0faeff5 100644 --- a/pkg/k8sutil/handler.go +++ b/pkg/k8sutil/handler.go @@ -10,7 +10,10 @@ func (oc *OperatorContext) ReconsileMarklogicGroupHandler() (reconcile.Result, e if result := oc.ReconcileServices(); result.Completed() { return result.Output() } - setOperatorInternalStatus(oc, "Created") + err := setOperatorInternalStatus(oc, "Created") + if err != nil { + oc.ReqLogger.Error(err, "Failed to set operator internal status") + } if result := oc.ReconcileConfigMap(); result.Completed() { return result.Output() diff --git a/pkg/k8sutil/statefulset.go b/pkg/k8sutil/statefulset.go index 509bcef..9ec6256 100644 --- a/pkg/k8sutil/statefulset.go +++ b/pkg/k8sutil/statefulset.go @@ -80,11 +80,18 @@ func (oc *OperatorContext) ReconcileStatefulset() (reconcile.Result, error) { statefulSetDef := generateStatefulSetsDef(objectMeta, statefulSetParams, marklogicServerAsOwner(cr), containerParams) if err != nil { if apierrors.IsNotFound(err) { - oc.createStatefulSet(statefulSetDef, cr) + err := oc.createStatefulSet(statefulSetDef, cr) + if err != nil { + logger.Error(err, "Failed to create statefulSet") + return result.Error(err).Output() + } oc.Recorder.Event(oc.MarklogicGroup, "Normal", "StatefulSetCreated", "MarkLogic statefulSet created successfully") return result.Done().Output() } - result.Error(err).Output() + _, outputErr := result.Error(err).Output() + if outputErr != nil { + logger.Error(outputErr, "Failed to process result error") + } } if err != nil { logger.Error(err, "Cannot create standalone statefulSet for MarkLogic") @@ -488,7 +495,7 @@ func generateVolumes(stsName string, containerParams containerParameters) []core }, }) } - if containerParams.Tls.CertSecretNames != nil && len(containerParams.Tls.CertSecretNames) > 0 { + if len(containerParams.Tls.CertSecretNames) > 0 { projectionSources := []corev1.VolumeProjection{} for i, secretName := range containerParams.Tls.CertSecretNames { projectionSource := corev1.VolumeProjection{ diff --git a/test/utils/certs.go b/test/utils/certs.go index cae11ca..81d7df5 100644 --- a/test/utils/certs.go +++ b/test/utils/certs.go @@ -3,19 +3,54 @@ package utils import ( "fmt" "os/exec" + "path/filepath" "strings" ) +// validatePath performs basic validation on file paths to prevent command injection +func validatePath(path string) error { + // Use filepath.Clean to sanitize the path + cleanPath := filepath.Clean(path) + + // Ensure the path doesn't contain shell metacharacters + if strings.ContainsAny(cleanPath, ";|&$`(){}[]<>") { + return fmt.Errorf("path contains invalid characters: %s", path) + } + + // Ensure it's not an absolute path to prevent access to system directories + if filepath.IsAbs(cleanPath) && !strings.HasPrefix(cleanPath, "/tmp/") { + return fmt.Errorf("path must be relative or in /tmp: %s", path) + } + + return nil +} + func GenerateCertificates(path string, caPath string) error { var err error fmt.Println("====Generating TLS Certificates") - geeTlsKeyCmd := strings.Replace("openssl genpkey -algorithm RSA -out path/tls.key", "path", path, -1) - genCsrCmd := strings.Replace("openssl req -new -key path/tls.key -config path/server.cnf -out path/tls.csr", "path", path, -1) - genCrtCmd := strings.Replace(strings.Replace("openssl x509 -req -CA caPath/cacert.pem -CAkey caPath/ca-private-key.pem -CAcreateserial -CAserial path/cacert.srl -in path/tls.csr -out path/tls.crt -days 365", "path", path, -1), "caPath", caPath, -1) - rvariable := []string{geeTlsKeyCmd, genCsrCmd, genCrtCmd} - for _, j := range rvariable { - cmd := exec.Command("bash", "-c", j) + + // Validate paths to prevent command injection + if err := validatePath(path); err != nil { + return fmt.Errorf("invalid path: %w", err) + } + if err := validatePath(caPath); err != nil { + return fmt.Errorf("invalid caPath: %w", err) + } + + // Use safer approach with individual commands instead of shell execution + commands := [][]string{ + {"openssl", "genpkey", "-algorithm", "RSA", "-out", filepath.Join(path, "tls.key")}, + {"openssl", "req", "-new", "-key", filepath.Join(path, "tls.key"), "-config", filepath.Join(path, "server.cnf"), "-out", filepath.Join(path, "tls.csr")}, + {"openssl", "x509", "-req", "-CA", filepath.Join(caPath, "cacert.pem"), "-CAkey", filepath.Join(caPath, "ca-private-key.pem"), "-CAcreateserial", "-CAserial", filepath.Join(path, "cacert.srl"), "-in", filepath.Join(path, "tls.csr"), "-out", filepath.Join(path, "tls.crt"), "-days", "365"}, + } + + for _, cmdArgs := range commands { + // #nosec G204 - Command arguments are constructed from validated paths + cmd := exec.Command(cmdArgs[0], cmdArgs[1:]...) err = cmd.Run() + if err != nil { + return fmt.Errorf("failed to execute command %v: %w", cmdArgs, err) + } } return err } @@ -23,17 +58,28 @@ func GenerateCertificates(path string, caPath string) error { func GenerateCACertificate(caPath string) error { var err error fmt.Println("====Generating CA Certificates") - genKeyCmd := strings.Replace("openssl genrsa -out caPath/ca-private-key.pem 2048", "caPath", caPath, -1) - genCACertCmd := strings.Replace("openssl req -new -x509 -days 3650 -key caPath/ca-private-key.pem -out caPath/cacert.pem -subj '/CN=TlsTest/C=US/ST=California/L=RedwoodCity/O=Progress/OU=MarkLogic'", "caPath", caPath, -1) - pwdCMD := "pwd" - rvariable := []string{pwdCMD, genKeyCmd, genCACertCmd} - for _, j := range rvariable { - fmt.Println(j) - cmd := exec.Command("bash", "-c", j) + + // Validate path to prevent command injection + if err := validatePath(caPath); err != nil { + return fmt.Errorf("invalid caPath: %w", err) + } + + // Use safer approach with individual commands instead of shell execution + commands := [][]string{ + {"pwd"}, + {"openssl", "genrsa", "-out", filepath.Join(caPath, "ca-private-key.pem"), "2048"}, + {"openssl", "req", "-new", "-x509", "-days", "3650", "-key", filepath.Join(caPath, "ca-private-key.pem"), "-out", filepath.Join(caPath, "cacert.pem"), "-subj", "/CN=TlsTest/C=US/ST=California/L=RedwoodCity/O=Progress/OU=MarkLogic"}, + } + + for _, cmdArgs := range commands { + fmt.Println(strings.Join(cmdArgs, " ")) + // #nosec G204 - Command arguments are constructed from validated paths + cmd := exec.Command(cmdArgs[0], cmdArgs[1:]...) err = cmd.Run() if err != nil { fmt.Println(err) + return fmt.Errorf("failed to execute command %v: %w", cmdArgs, err) } } - return err + return nil } diff --git a/test/utils/utils.go b/test/utils/utils.go index 787c624..04b63ca 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -23,6 +23,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "strings" "testing" "time" @@ -48,9 +49,43 @@ func warnError(err error) { fmt.Fprintf(GinkgoWriter, "warning: %v\n", err) } +// validateURL performs basic validation on URLs to prevent command injection +func validateURL(url string) error { + if !strings.HasPrefix(url, "https://") { + return fmt.Errorf("URL must use HTTPS: %s", url) + } + // Basic validation to ensure URL doesn't contain shell metacharacters + if strings.ContainsAny(url, ";|&$`(){}[]<>") { + return fmt.Errorf("URL contains invalid characters: %s", url) + } + return nil +} + +// validateKindClusterName validates cluster names for kind +func validateKindClusterName(name string) error { + // Kind cluster names should only contain alphanumeric characters and hyphens + if !regexp.MustCompile(`^[a-zA-Z0-9-]+$`).MatchString(name) { + return fmt.Errorf("invalid cluster name: %s", name) + } + return nil +} + +// validateImageName validates Docker image names +func validateImageName(name string) error { + // Basic validation for Docker image names + if strings.ContainsAny(name, ";|&$`(){}[]<>") { + return fmt.Errorf("image name contains invalid characters: %s", name) + } + return nil +} + // InstallPrometheusOperator installs the prometheus Operator to be used to export the enabled metrics. func InstallPrometheusOperator() error { url := fmt.Sprintf(prometheusOperatorURL, prometheusOperatorVersion) + if err := validateURL(url); err != nil { + return fmt.Errorf("invalid prometheus operator URL: %w", err) + } + // #nosec G204 - URL is validated and constructed from trusted constants cmd := exec.Command("kubectl", "create", "-f", url) _, err := Run(cmd) return err @@ -79,6 +114,11 @@ func Run(cmd *exec.Cmd) ([]byte, error) { // UninstallPrometheusOperator uninstalls the prometheus func UninstallPrometheusOperator() { url := fmt.Sprintf(prometheusOperatorURL, prometheusOperatorVersion) + if err := validateURL(url); err != nil { + warnError(fmt.Errorf("invalid prometheus operator URL: %w", err)) + return + } + // #nosec G204 - URL is validated and constructed from trusted constants cmd := exec.Command("kubectl", "delete", "-f", url) if _, err := Run(cmd); err != nil { warnError(err) @@ -88,6 +128,11 @@ func UninstallPrometheusOperator() { // UninstallCertManager uninstalls the cert manager func UninstallCertManager() { url := fmt.Sprintf(certmanagerURLTmpl, certmanagerVersion) + if err := validateURL(url); err != nil { + warnError(fmt.Errorf("invalid cert manager URL: %w", err)) + return + } + // #nosec G204 - URL is validated and constructed from trusted constants cmd := exec.Command("kubectl", "delete", "-f", url) if _, err := Run(cmd); err != nil { warnError(err) @@ -97,6 +142,10 @@ func UninstallCertManager() { // InstallCertManager installs the cert manager bundle. func InstallCertManager() error { url := fmt.Sprintf(certmanagerURLTmpl, certmanagerVersion) + if err := validateURL(url); err != nil { + return fmt.Errorf("invalid cert manager URL: %w", err) + } + // #nosec G204 - URL is validated and constructed from trusted constants cmd := exec.Command("kubectl", "apply", "-f", url) if _, err := Run(cmd); err != nil { return err @@ -119,6 +168,16 @@ func LoadImageToKindClusterWithName(name string) error { if v, ok := os.LookupEnv("KIND_CLUSTER"); ok { cluster = v } + + // Validate inputs to prevent command injection + if err := validateImageName(name); err != nil { + return fmt.Errorf("invalid image name: %w", err) + } + if err := validateKindClusterName(cluster); err != nil { + return fmt.Errorf("invalid cluster name: %w", err) + } + + // #nosec G204 - Inputs are validated for safety kindOptions := []string{"load", "docker-image", name, "--name", cluster} cmd := exec.Command("kind", kindOptions...) _, err := Run(cmd)