From 3c3c1001bb09c38a16724dc80bac759789012525 Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Mon, 25 Nov 2024 14:15:08 +0100 Subject: [PATCH 1/9] Fix security issues --- pkg/imagepuller/daemonset.go | 7 +++++-- pkg/resourcemanager/delegate.go | 15 +++++++++------ pkg/types/storage.go | 2 +- pkg/utils/minio.go | 2 +- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/pkg/imagepuller/daemonset.go b/pkg/imagepuller/daemonset.go index 7d5a6f9f..6795e065 100644 --- a/pkg/imagepuller/daemonset.go +++ b/pkg/imagepuller/daemonset.go @@ -21,9 +21,10 @@ package imagepuller import ( //"k8s.io/apimachinery/pkg/watch" "context" + "crypto/rand" "fmt" "log" - "math/rand" + "math/big" "os" "sync" "time" @@ -191,7 +192,9 @@ func setWorkingNodes(kubeClientset kubernetes.Interface) error { func generatePodGroupName() string { b := make([]byte, lengthStr) for i := range b { - b[i] = letterBytes[rand.Intn(len(letterBytes))] + max := big.NewInt(int64(len(letterBytes))) + randomNumber, _ := rand.Int(rand.Reader, max) + b[i] = letterBytes[randomNumber.Int64()] } return "pod-group-" + string(b) } diff --git a/pkg/resourcemanager/delegate.go b/pkg/resourcemanager/delegate.go index dfa5a225..014df265 100644 --- a/pkg/resourcemanager/delegate.go +++ b/pkg/resourcemanager/delegate.go @@ -18,11 +18,12 @@ package resourcemanager import ( "bytes" + "crypto/rand" "crypto/tls" "encoding/json" "fmt" "log" - "math/rand" + "math/big" "net/http" "net/url" "path" @@ -131,7 +132,7 @@ func DelegateJob(service *types.Service, event string, logger *log.Logger) error // Make HTTP client var transport http.RoundTripper = &http.Transport{ // Enable/disable SSL verification - TLSClientConfig: &tls.Config{InsecureSkipVerify: !cluster.SSLVerify}, + TLSClientConfig: &tls.Config{InsecureSkipVerify: !cluster.SSLVerify}, // #nosec } client := &http.Client{ Transport: transport, @@ -193,7 +194,7 @@ func DelegateJob(service *types.Service, event string, logger *log.Logger) error // Make HTTP client var transport http.RoundTripper = &http.Transport{ // Enable/disable SSL verification - TLSClientConfig: &tls.Config{InsecureSkipVerify: !replica.SSLVerify}, + TLSClientConfig: &tls.Config{InsecureSkipVerify: !replica.SSLVerify}, // #nosec } client := &http.Client{ Transport: transport, @@ -269,7 +270,7 @@ func updateServiceToken(replica types.Replica, cluster types.Cluster) (string, e // Make HTTP client var transport http.RoundTripper = &http.Transport{ // Enable/disable SSL verification - TLSClientConfig: &tls.Config{InsecureSkipVerify: !cluster.SSLVerify}, + TLSClientConfig: &tls.Config{InsecureSkipVerify: !cluster.SSLVerify}, // #nosec } client := &http.Client{ Transport: transport, @@ -344,7 +345,7 @@ func getClusterStatus(service *types.Service) { // Make HTTP client var transport http.RoundTripper = &http.Transport{ // Enable/disable SSL verification - TLSClientConfig: &tls.Config{InsecureSkipVerify: !cluster.SSLVerify}, + TLSClientConfig: &tls.Config{InsecureSkipVerify: !cluster.SSLVerify}, // #nosec } client := &http.Client{ Transport: transport, @@ -395,7 +396,9 @@ func getClusterStatus(service *types.Service) { if dist >= 0 { fmt.Println("Resources available in ClusterID", replica.ClusterID) if service.Delegation == "random" { - randPriority := rand.Intn(noDelegateCode) + max := big.NewInt(int64(noDelegateCode)) + randomNumber, _ := rand.Int(rand.Reader, max) + randPriority := randomNumber.Int64() replica.Priority = uint(randPriority) fmt.Println("Priority ", replica.Priority, " with ", service.Delegation, " delegation") } else if service.Delegation == "load-based" { diff --git a/pkg/types/storage.go b/pkg/types/storage.go index f2671da9..05fc22bc 100644 --- a/pkg/types/storage.go +++ b/pkg/types/storage.go @@ -122,7 +122,7 @@ func (minIOProvider MinIOProvider) GetS3Client() *s3.S3 { // Disable tls verification in client transport if Verify == false if !minIOProvider.Verify { tr := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // #nosec } s3MinIOConfig.HTTPClient = &http.Client{Transport: tr} } diff --git a/pkg/utils/minio.go b/pkg/utils/minio.go index 8a71b6e7..22d2a6c2 100644 --- a/pkg/utils/minio.go +++ b/pkg/utils/minio.go @@ -80,7 +80,7 @@ func MakeMinIOAdminClient(cfg *types.Config) (*MinIOAdminClient, error) { // Disable tls verification in client transport if verify == false if !cfg.MinIOProvider.Verify { tr := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // #nosec } adminClient.SetCustomTransport(tr) } From 2dab7cc2a1a2627da14207fe37ae32bbee17b513 Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Mon, 25 Nov 2024 14:33:11 +0100 Subject: [PATCH 2/9] Fix staticcheck issues --- pkg/backends/knative.go | 5 ++--- pkg/handlers/job.go | 2 +- pkg/utils/auth/auth.go | 8 ++++---- pkg/utils/minio.go | 5 ++--- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/pkg/backends/knative.go b/pkg/backends/knative.go index 566352c4..2893eeb3 100644 --- a/pkg/backends/knative.go +++ b/pkg/backends/knative.go @@ -21,7 +21,6 @@ import ( "fmt" "log" "net/http" - "os" "strconv" "github.com/grycap/oscar/v3/pkg/imagepuller" @@ -33,8 +32,8 @@ import ( knclientset "knative.dev/serving/pkg/client/clientset/versioned" ) -// Custom logger -var knativeLogger = log.New(os.Stdout, "[KNATIVE] ", log.Flags()) +// Custom logger - uncomment if needed +// var knativeLogger = log.New(os.Stdout, "[KNATIVE] ", log.Flags()) // KnativeBackend struct to represent a Knative client type KnativeBackend struct { diff --git a/pkg/handlers/job.go b/pkg/handlers/job.go index e0319d63..758feae5 100644 --- a/pkg/handlers/job.go +++ b/pkg/handlers/job.go @@ -157,7 +157,7 @@ func MakeJobHandler(cfg *types.Config, kubeClientset kubernetes.Interface, back c.Next() // Initialize event envVar and args var - event := v1.EnvVar{} + var event v1.EnvVar var args []string if cfg.InterLinkAvailable && service.InterLinkNodeName != "" { diff --git a/pkg/utils/auth/auth.go b/pkg/utils/auth/auth.go index 57795a48..0df154e3 100644 --- a/pkg/utils/auth/auth.go +++ b/pkg/utils/auth/auth.go @@ -110,11 +110,11 @@ func GetLoggerMiddleware() gin.HandlerFunc { func GetUIDFromContext(c *gin.Context) (string, error) { uidOrigin, uidExists := c.Get("uidOrigin") if !uidExists { - return "", fmt.Errorf("Missing EGI user uid") + return "", fmt.Errorf("missing EGI user uid") } uid, uidParsed := uidOrigin.(string) if !uidParsed { - return "", fmt.Errorf("Error parsing uid origin: %v", uidParsed) + return "", fmt.Errorf("error parsing uid origin: %v", uidParsed) } return uid, nil } @@ -122,11 +122,11 @@ func GetUIDFromContext(c *gin.Context) (string, error) { func GetMultitenancyConfigFromContext(c *gin.Context) (*MultitenancyConfig, error) { mcUntyped, mcExists := c.Get("multitenancyConfig") if !mcExists { - return nil, fmt.Errorf("Missing multitenancy config") + return nil, fmt.Errorf("missing multitenancy config") } mc, mcParsed := mcUntyped.(*MultitenancyConfig) if !mcParsed { - return nil, fmt.Errorf("Error parsing multitenancy config") + return nil, fmt.Errorf("error parsing multitenancy config") } return mc, nil } diff --git a/pkg/utils/minio.go b/pkg/utils/minio.go index 22d2a6c2..2ad3061e 100644 --- a/pkg/utils/minio.go +++ b/pkg/utils/minio.go @@ -21,10 +21,8 @@ import ( "crypto/tls" "encoding/json" "fmt" - "log" "net/http" "net/url" - "os" "time" "github.com/grycap/oscar/v3/pkg/types" @@ -33,7 +31,8 @@ import ( const ALL_USERS_GROUP = "all_users_group" -var minioLogger = log.New(os.Stdout, "[MINIO] ", log.Flags()) +// Custom logger - uncomment if needed +// var minioLogger = log.New(os.Stdout, "[MINIO] ", log.Flags()) // MinIOAdminClient struct to represent a MinIO Admin client to configure webhook notifications type MinIOAdminClient struct { From df688f992ab0d703b5e4dd2e5887dd0aca6f73b5 Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Mon, 25 Nov 2024 16:10:50 +0100 Subject: [PATCH 3/9] Avoid flake error with examples --- .flake8 | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .flake8 diff --git a/.flake8 b/.flake8 new file mode 100644 index 00000000..bdc1f5e3 --- /dev/null +++ b/.flake8 @@ -0,0 +1,2 @@ +[flake8] +exclude = examples From 03596d1b0ce2f1620cc5a756ecec25ca802b8e6f Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Wed, 27 Nov 2024 09:24:28 +0100 Subject: [PATCH 4/9] Fix sec issues --- pkg/backends/k8s.go | 2 +- pkg/backends/knative.go | 2 +- pkg/backends/openfaas.go | 2 +- pkg/handlers/logs.go | 6 +++--- pkg/resourcemanager/delegate.go | 4 ++-- pkg/types/expose.go | 4 ++-- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/backends/k8s.go b/pkg/backends/k8s.go index 0879991d..98a9cde6 100644 --- a/pkg/backends/k8s.go +++ b/pkg/backends/k8s.go @@ -66,7 +66,7 @@ func (k *KubeBackend) ListServices() ([]*types.Service, error) { services := []*types.Service{} for _, cm := range configmaps.Items { - service, err := getServiceFromConfigMap(&cm) + service, err := getServiceFromConfigMap(&cm) // #nosec G601 if err != nil { return nil, err } diff --git a/pkg/backends/knative.go b/pkg/backends/knative.go index 2893eeb3..6167ab05 100644 --- a/pkg/backends/knative.go +++ b/pkg/backends/knative.go @@ -83,7 +83,7 @@ func (kn *KnativeBackend) ListServices() ([]*types.Service, error) { services := []*types.Service{} for _, cm := range configmaps.Items { - service, err := getServiceFromConfigMap(&cm) + service, err := getServiceFromConfigMap(&cm) // #nosec G601 if err != nil { return nil, err } diff --git a/pkg/backends/openfaas.go b/pkg/backends/openfaas.go index 5743e21a..54ede05d 100644 --- a/pkg/backends/openfaas.go +++ b/pkg/backends/openfaas.go @@ -92,7 +92,7 @@ func (of *OpenfaasBackend) ListServices() ([]*types.Service, error) { services := []*types.Service{} for _, cm := range configmaps.Items { - service, err := getServiceFromConfigMap(&cm) + service, err := getServiceFromConfigMap(&cm) // #nosec G601 if err != nil { return nil, err } diff --git a/pkg/handlers/logs.go b/pkg/handlers/logs.go index cc809827..90d073e8 100644 --- a/pkg/handlers/logs.go +++ b/pkg/handlers/logs.go @@ -87,10 +87,10 @@ func MakeJobsInfoHandler(back types.ServerlessBackend, kubeClientset kubernetes. for _, contStatus := range pod.Status.ContainerStatuses { if contStatus.Name == types.ContainerName { if contStatus.State.Running != nil { - jobsInfo[jobName].StartTime = &contStatus.State.Running.StartedAt + jobsInfo[jobName].StartTime = &(contStatus.State.Running.StartedAt) } else if contStatus.State.Terminated != nil { - jobsInfo[jobName].StartTime = &contStatus.State.Terminated.StartedAt - jobsInfo[jobName].FinishTime = &contStatus.State.Terminated.FinishedAt + jobsInfo[jobName].StartTime = &(contStatus.State.Terminated.StartedAt) + jobsInfo[jobName].FinishTime = &(contStatus.State.Terminated.FinishedAt) } } } diff --git a/pkg/resourcemanager/delegate.go b/pkg/resourcemanager/delegate.go index 014df265..87292360 100644 --- a/pkg/resourcemanager/delegate.go +++ b/pkg/resourcemanager/delegate.go @@ -398,14 +398,14 @@ func getClusterStatus(service *types.Service) { if service.Delegation == "random" { max := big.NewInt(int64(noDelegateCode)) randomNumber, _ := rand.Int(rand.Reader, max) - randPriority := randomNumber.Int64() + randPriority := randomNumber.Uint64() replica.Priority = uint(randPriority) fmt.Println("Priority ", replica.Priority, " with ", service.Delegation, " delegation") } else if service.Delegation == "load-based" { //Map the totalClusterCPU range to a smaller range (input range 0 to 32 cpu to output range 100 to 0 priority) totalClusterCPU := clusterStatus.CPUFreeTotal mappedCPUPriority := mapToRange(totalClusterCPU, 0, 32000, 100, 0) - replica.Priority = uint(mappedCPUPriority) + replica.Priority = uint(mappedCPUPriority) // #nosec G115 fmt.Println("Priority ", replica.Priority, " with ", service.Delegation, " delegation") } else if service.Delegation != "static" { replica.Priority = noDelegateCode diff --git a/pkg/types/expose.go b/pkg/types/expose.go index 09b758b4..16f72cce 100644 --- a/pkg/types/expose.go +++ b/pkg/types/expose.go @@ -250,7 +250,7 @@ func getPodTemplateSpec(service Service, cfg *Config) v1.PodTemplateSpec { podSpec.Containers[i].Ports = []v1.ContainerPort{ { Name: podPortName, - ContainerPort: int32(service.Expose.APIPort), + ContainerPort: int32(service.Expose.APIPort), // #nosec G115 }, } podSpec.Containers[i].VolumeMounts[0].ReadOnly = false @@ -352,7 +352,7 @@ func getServiceSpec(service Service, cfg *Config) *v1.Service { Port: servicePortNumber, TargetPort: intstr.IntOrString{ Type: 0, - IntVal: int32(service.Expose.APIPort), + IntVal: int32(service.Expose.APIPort), // #nosec G115 }, } service_type := v1.ServiceType(typeClusterIP) From 370c2756eed927133c21b0d926161a391c51dff6 Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Wed, 27 Nov 2024 10:10:45 +0100 Subject: [PATCH 5/9] Fix sec issues --- pkg/backends/knative.go | 5 +++- pkg/handlers/create.go | 45 ++++++++++++++++++++++++++++-------- pkg/handlers/delete.go | 10 ++++++-- pkg/handlers/update.go | 10 ++++++-- pkg/imagepuller/daemonset.go | 16 +++++++++---- pkg/types/expose.go | 30 +++++++++++++++++++----- pkg/types/expose_test.go | 6 +++++ pkg/utils/auth/auth.go | 4 ++-- pkg/utils/auth/oidc.go | 5 +++- pkg/utils/minio.go | 10 ++++++-- pkg/utils/token.go | 2 +- 11 files changed, 113 insertions(+), 30 deletions(-) diff --git a/pkg/backends/knative.go b/pkg/backends/knative.go index 6167ab05..3d08ba78 100644 --- a/pkg/backends/knative.go +++ b/pkg/backends/knative.go @@ -128,7 +128,10 @@ func (kn *KnativeBackend) CreateService(service types.Service) error { //Create an expose service if service.Expose.APIPort != 0 { - types.CreateExpose(service, kn.kubeClientset, kn.config) + err = types.CreateExpose(service, kn.kubeClientset, kn.config) + if err != nil { + return err + } } //Create deaemonset to cache the service image on all the nodes if service.ImagePrefetch { diff --git a/pkg/handlers/create.go b/pkg/handlers/create.go index 30e16bdc..3c17193d 100644 --- a/pkg/handlers/create.go +++ b/pkg/handlers/create.go @@ -128,8 +128,14 @@ func MakeCreateHandler(cfg *types.Config, back types.ServerlessBackend) gin.Hand if len(uids) > 0 { for _, uid := range uids { sk, _ := auth.GenerateRandomKey(8) - minIOAdminClient.CreateMinIOUser(uid, sk) - mc.CreateSecretForOIDC(uid, sk) + cmuErr := minIOAdminClient.CreateMinIOUser(uid, sk) + if cmuErr != nil { + log.Printf("Error creating MinIO user for user %s: %v", uid, cmuErr) + } + csErr := mc.CreateSecretForOIDC(uid, sk) + if csErr != nil { + log.Printf("Error creating secret for user %s: %v", uid, csErr) + } } } } @@ -148,7 +154,10 @@ func MakeCreateHandler(cfg *types.Config, back types.ServerlessBackend) gin.Hand // Register minio webhook and restart the server if err := registerMinIOWebhook(service.Name, service.Token, service.StorageProviders.MinIO[types.DefaultProvider], cfg); err != nil { - back.DeleteService(service) + derr := back.DeleteService(service) + if derr != nil { + log.Printf("Error deleting service: %v\n", derr) + } c.String(http.StatusInternalServerError, err.Error()) return } @@ -160,7 +169,10 @@ func MakeCreateHandler(cfg *types.Config, back types.ServerlessBackend) gin.Hand } else { c.String(http.StatusInternalServerError, err.Error()) } - back.DeleteService(service) + derr := back.DeleteService(service) + if derr != nil { + log.Printf("Error deleting service: %v\n", derr) + } return } @@ -332,7 +344,10 @@ func createBuckets(service *types.Service, cfg *types.Config, minIOAdminClient * provID, provName = getProviderInfo(out.Provider) // Check if the provider identifier is defined in StorageProviders if !isStorageProviderDefined(provName, provID, service.StorageProviders) { - disableInputNotifications(service.GetMinIOWebhookARN(), service.Input, cfg.MinIOProvider) + dinErr := disableInputNotifications(service.GetMinIOWebhookARN(), service.Input, cfg.MinIOProvider) + if dinErr != nil { + log.Printf("Error disabling input notifications: %v\n", dinErr) + } return fmt.Errorf("the StorageProvider \"%s.%s\" is not defined", provName, provID) } @@ -358,11 +373,17 @@ func createBuckets(service *types.Service, cfg *types.Config, minIOAdminClient * if aerr.Code() == s3.ErrCodeBucketAlreadyExists || aerr.Code() == s3.ErrCodeBucketAlreadyOwnedByYou { log.Printf("The bucket \"%s\" already exists\n", splitPath[0]) } else { - disableInputNotifications(service.GetMinIOWebhookARN(), service.Input, cfg.MinIOProvider) + dinErr := disableInputNotifications(service.GetMinIOWebhookARN(), service.Input, cfg.MinIOProvider) + if dinErr != nil { + log.Printf("Error disabling input notifications: %v\n", dinErr) + } return fmt.Errorf("error creating bucket %s: %v", splitPath[0], err) } } else { - disableInputNotifications(service.GetMinIOWebhookARN(), service.Input, cfg.MinIOProvider) + dinErr := disableInputNotifications(service.GetMinIOWebhookARN(), service.Input, cfg.MinIOProvider) + if dinErr != nil { + log.Printf("Error disabling input notifications: %v\n", dinErr) + } return fmt.Errorf("error creating bucket %s: %v", splitPath[0], err) } } @@ -375,7 +396,10 @@ func createBuckets(service *types.Service, cfg *types.Config, minIOAdminClient * Key: aws.String(folderKey), }) if err != nil { - disableInputNotifications(service.GetMinIOWebhookARN(), service.Input, cfg.MinIOProvider) + dinErr := disableInputNotifications(service.GetMinIOWebhookARN(), service.Input, cfg.MinIOProvider) + if dinErr != nil { + log.Printf("Error disabling input notifications: %v\n", dinErr) + } return fmt.Errorf("error creating folder \"%s\" in bucket \"%s\": %v", folderKey, splitPath[0], err) } } @@ -386,7 +410,10 @@ func createBuckets(service *types.Service, cfg *types.Config, minIOAdminClient * if err == cdmi.ErrBadRequest { log.Printf("Error creating \"%s\" folder in Onedata. Error: %v\n", path, err) } else { - disableInputNotifications(service.GetMinIOWebhookARN(), service.Input, cfg.MinIOProvider) + dinErr := disableInputNotifications(service.GetMinIOWebhookARN(), service.Input, cfg.MinIOProvider) + if dinErr != nil { + log.Printf("Error disabling input notifications: %v\n", dinErr) + } return fmt.Errorf("error connecting to Onedata's Oneprovider \"%s\". Error: %v", service.StorageProviders.Onedata[provID].OneproviderHost, err) } } diff --git a/pkg/handlers/delete.go b/pkg/handlers/delete.go index 19d0e126..7e85ef98 100644 --- a/pkg/handlers/delete.go +++ b/pkg/handlers/delete.go @@ -71,7 +71,10 @@ func MakeDeleteHandler(cfg *types.Config, back types.ServerlessBackend) gin.Hand // Split buckets and folders from path bucket := strings.SplitN(path, "/", 2) var users []string - minIOAdminClient.UpdateUsersInGroup(users, bucket[0], true) + err = minIOAdminClient.UpdateUsersInGroup(users, bucket[0], true) + if err != nil { + log.Printf("error updating MinIO users in group: %v", err) + } } if service.Mount.Path != "" { @@ -79,7 +82,10 @@ func MakeDeleteHandler(cfg *types.Config, back types.ServerlessBackend) gin.Hand // Split buckets and folders from path bucket := strings.SplitN(path, "/", 2) var users []string - minIOAdminClient.UpdateUsersInGroup(users, bucket[0], true) + err = minIOAdminClient.UpdateUsersInGroup(users, bucket[0], true) + if err != nil { + log.Printf("error updating MinIO users in group: %v", err) + } } // Disable input notifications diff --git a/pkg/handlers/update.go b/pkg/handlers/update.go index e7dea13e..fc1a83c1 100644 --- a/pkg/handlers/update.go +++ b/pkg/handlers/update.go @@ -150,7 +150,10 @@ func MakeUpdateHandler(cfg *types.Config, back types.ServerlessBackend) gin.Hand // Register minio webhook and restart the server if err := registerMinIOWebhook(newService.Name, newService.Token, newService.StorageProviders.MinIO[types.DefaultProvider], cfg); err != nil { - back.UpdateService(*oldService) + uerr := back.UpdateService(*oldService) + if uerr != nil { + log.Println(uerr.Error()) + } c.String(http.StatusInternalServerError, err.Error()) return } @@ -163,7 +166,10 @@ func MakeUpdateHandler(cfg *types.Config, back types.ServerlessBackend) gin.Hand c.String(http.StatusInternalServerError, err.Error()) } // If updateBuckets fails restore the oldService - back.UpdateService(*oldService) + uerr := back.UpdateService(*oldService) + if uerr != nil { + log.Println(uerr.Error()) + } return } } diff --git a/pkg/imagepuller/daemonset.go b/pkg/imagepuller/daemonset.go index 6795e065..22cc2167 100644 --- a/pkg/imagepuller/daemonset.go +++ b/pkg/imagepuller/daemonset.go @@ -62,7 +62,11 @@ var stopper chan struct{} func CreateDaemonset(cfg *types.Config, service types.Service, kubeClientset kubernetes.Interface) error { DaemonSetLoggerInfo.Println("Creating daemonset for service:", service.Name) //Set needed variables - setWorkingNodes(kubeClientset) + err := setWorkingNodes(kubeClientset) + if err != nil { + DaemonSetLoggerInfo.Println(err) + return fmt.Errorf("failed to set working nodes: %s", err.Error()) + } podGroup = generatePodGroupName() daemonsetName = "image-puller-" + service.Name @@ -70,7 +74,7 @@ func CreateDaemonset(cfg *types.Config, service types.Service, kubeClientset kub daemon := getDaemonset(cfg, service) //Create daemonset - _, err := kubeClientset.AppsV1().DaemonSets(cfg.ServicesNamespace).Create(context.TODO(), daemon, metav1.CreateOptions{}) + _, err = kubeClientset.AppsV1().DaemonSets(cfg.ServicesNamespace).Create(context.TODO(), daemon, metav1.CreateOptions{}) if err != nil { DaemonSetLoggerInfo.Println(err) return fmt.Errorf("failed to create daemonset: %s", err.Error()) @@ -147,15 +151,19 @@ func watchPods(kubeClientset kubernetes.Interface, cfg *types.Config) { } //Add event handler that gets all the pods status - podInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ + _, err := podInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ UpdateFunc: handleUpdatePodEvent, }) + if err != nil { + DaemonSetLoggerInfo.Println(err) + log.Fatalf("Failed to add event handler: %s", err.Error()) + } <-stopper //Delete daemonset when all pods are in state "Running" DaemonSetLoggerInfo.Println("Deleting daemonset...") - err := kubeClientset.AppsV1().DaemonSets(cfg.ServicesNamespace).Delete(context.TODO(), daemonsetName, metav1.DeleteOptions{}) + err = kubeClientset.AppsV1().DaemonSets(cfg.ServicesNamespace).Delete(context.TODO(), daemonsetName, metav1.DeleteOptions{}) if err != nil { DaemonSetLoggerInfo.Println(err) log.Fatalf("Failed to delete daemonset: %s", err.Error()) diff --git a/pkg/types/expose.go b/pkg/types/expose.go index 16f72cce..4b7403b7 100644 --- a/pkg/types/expose.go +++ b/pkg/types/expose.go @@ -323,7 +323,10 @@ func updateDeployment(service Service, kubeClientset kubernetes.Interface, cfg * return err } - kubeClientset.AutoscalingV1().HorizontalPodAutoscalers(cfg.ServicesNamespace).Get(context.TODO(), getHPAName(service.Name), metav1.GetOptions{}) + _, err = kubeClientset.AutoscalingV1().HorizontalPodAutoscalers(cfg.ServicesNamespace).Get(context.TODO(), getHPAName(service.Name), metav1.GetOptions{}) + if err != nil { + return err + } hpa := getHortizontalAutoScaleSpec(service, cfg) _, err = kubeClientset.AutoscalingV1().HorizontalPodAutoscalers(cfg.ServicesNamespace).Update(context.TODO(), hpa, metav1.UpdateOptions{}) if err != nil { @@ -420,7 +423,10 @@ func createIngress(service Service, kubeClientset kubernetes.Interface, cfg *Con return err } if service.Expose.SetAuth { - createSecret(service, kubeClientset, cfg) + cerr := createSecret(service, kubeClientset, cfg) + if cerr != nil { + return cerr + } } return nil } @@ -441,13 +447,22 @@ func updateIngress(service Service, kubeClientset kubernetes.Interface, cfg *Con secret := existsSecret(serviceName, kubeClientset, cfg) if secret { if service.Expose.SetAuth { - updateSecret(service, kubeClientset, cfg) + uerr := updateSecret(service, kubeClientset, cfg) + if uerr != nil { + return uerr + } } else { - deleteSecret(service.Name, kubeClientset, cfg) + derr := deleteSecret(service.Name, kubeClientset, cfg) + if derr != nil { + return derr + } } } else { if service.Expose.SetAuth { - createSecret(service, kubeClientset, cfg) + cerr := createSecret(service, kubeClientset, cfg) + if cerr != nil { + return cerr + } } } @@ -547,7 +562,10 @@ func deleteIngress(name string, kubeClientset kubernetes.Interface, cfg *Config) if err != nil { return err } - deleteSecret(name, kubeClientset, cfg) + errd := deleteSecret(name, kubeClientset, cfg) + if errd != nil { + return errd + } return nil } diff --git a/pkg/types/expose_test.go b/pkg/types/expose_test.go index 69bab5b3..09e2450a 100644 --- a/pkg/types/expose_test.go +++ b/pkg/types/expose_test.go @@ -286,6 +286,12 @@ func TestDeleteIngress(t *testing.T) { Namespace: "namespace", }, }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-ing-auth-expose", + Namespace: "namespace", + }, + }, } kubeClientset := testclient.NewSimpleClientset(K8sObjects...) diff --git a/pkg/utils/auth/auth.go b/pkg/utils/auth/auth.go index 0df154e3..cea4ad6b 100644 --- a/pkg/utils/auth/auth.go +++ b/pkg/utils/auth/auth.go @@ -50,8 +50,8 @@ func CustomAuth(cfg *types.Config, kubeClientset kubernetes.Interface) gin.Handl // Slice to add default user to all users group on MinIO var oscarUser = []string{"console"} - minIOAdminClient.CreateAllUsersGroup() - minIOAdminClient.UpdateUsersInGroup(oscarUser, "all_users_group", false) + minIOAdminClient.CreateAllUsersGroup() // #nosec G104 + minIOAdminClient.UpdateUsersInGroup(oscarUser, "all_users_group", false) // #nosec G104 oidcHandler := getOIDCMiddleware(kubeClientset, minIOAdminClient, cfg.OIDCIssuer, cfg.OIDCSubject, cfg.OIDCGroups, nil) return func(c *gin.Context) { diff --git a/pkg/utils/auth/oidc.go b/pkg/utils/auth/oidc.go index 40dbd54d..b7ad6c30 100644 --- a/pkg/utils/auth/oidc.go +++ b/pkg/utils/auth/oidc.go @@ -159,7 +159,10 @@ func (om *oidcManager) GetUserInfo(rawToken string) (*userInfo, error) { var claims struct { EdupersonEntitlement []string `json:"eduperson_entitlement"` } - ui.Claims(&claims) + cerr := ui.Claims(&claims) + if cerr != nil { + return nil, err + } // Create "userInfo" struct and add the groups return &userInfo{ diff --git a/pkg/utils/minio.go b/pkg/utils/minio.go index 2ad3061e..87d0352b 100644 --- a/pkg/utils/minio.go +++ b/pkg/utils/minio.go @@ -140,7 +140,10 @@ func (minIOAdminClient *MinIOAdminClient) PublicToPrivateBucket(bucketName strin } actualPolicy := &Policy{} - json.Unmarshal(policyInfo.Policy, actualPolicy) + errUm := json.Unmarshal(policyInfo.Policy, actualPolicy) + if errUm != nil { + return errUm + } index := 0 // Search for the resource index resources := actualPolicy.Statement[0].Resource @@ -303,7 +306,10 @@ func createPolicy(adminClient *madmin.AdminClient, bucketName string, allUsers b } actualPolicy := &Policy{} - json.Unmarshal(policyInfo.Policy, actualPolicy) + jsonErr = json.Unmarshal(policyInfo.Policy, actualPolicy) + if jsonErr != nil { + return jsonErr + } // Add new resource and create policy actualPolicy.Statement[0].Resource = append(actualPolicy.Statement[0].Resource, rs) diff --git a/pkg/utils/token.go b/pkg/utils/token.go index a58d914a..57f480be 100644 --- a/pkg/utils/token.go +++ b/pkg/utils/token.go @@ -24,7 +24,7 @@ import ( // GenerateToken generates a random hexadecimal token func GenerateToken() string { b := make([]byte, 32) - rand.Read(b) + rand.Read(b) // #nosec G104 return hex.EncodeToString(b) } From 0c75526ff157d846fbc43cbc7257f980da0edb02 Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Wed, 27 Nov 2024 10:13:15 +0100 Subject: [PATCH 6/9] Add gosec test --- .github/workflows/tests.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 3017a1ad..7ac7a8d5 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -18,6 +18,11 @@ jobs: - name: Run tests run: go test ./pkg/... -cover -coverprofile=profile.cov + - name: Run Gosec Security Scanner + uses: securego/gosec@master + with: + args: ./... + - name: Report coverage uses: codacy/codacy-coverage-reporter-action@v1 with: From 330ea760f62a0c080ab82a8b0fdab29fa900f54d Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Wed, 27 Nov 2024 11:45:27 +0100 Subject: [PATCH 7/9] Fix sec issues --- pkg/resourcemanager/delegate.go | 12 ++++++++---- pkg/types/storage.go | 3 ++- pkg/utils/minio.go | 3 ++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/pkg/resourcemanager/delegate.go b/pkg/resourcemanager/delegate.go index 87292360..f1aa7a41 100644 --- a/pkg/resourcemanager/delegate.go +++ b/pkg/resourcemanager/delegate.go @@ -130,9 +130,10 @@ func DelegateJob(service *types.Service, event string, logger *log.Logger) error req.Header.Add("Authorization", "Bearer "+strings.TrimSpace(token)) // Make HTTP client + // #nosec var transport http.RoundTripper = &http.Transport{ // Enable/disable SSL verification - TLSClientConfig: &tls.Config{InsecureSkipVerify: !cluster.SSLVerify}, // #nosec + TLSClientConfig: &tls.Config{InsecureSkipVerify: !cluster.SSLVerify}, } client := &http.Client{ Transport: transport, @@ -192,9 +193,10 @@ func DelegateJob(service *types.Service, event string, logger *log.Logger) error } // Make HTTP client + // #nosec var transport http.RoundTripper = &http.Transport{ // Enable/disable SSL verification - TLSClientConfig: &tls.Config{InsecureSkipVerify: !replica.SSLVerify}, // #nosec + TLSClientConfig: &tls.Config{InsecureSkipVerify: !replica.SSLVerify}, } client := &http.Client{ Transport: transport, @@ -268,9 +270,10 @@ func updateServiceToken(replica types.Replica, cluster types.Cluster) (string, e req.SetBasicAuth(cluster.AuthUser, cluster.AuthPassword) // Make HTTP client + // #nosec var transport http.RoundTripper = &http.Transport{ // Enable/disable SSL verification - TLSClientConfig: &tls.Config{InsecureSkipVerify: !cluster.SSLVerify}, // #nosec + TLSClientConfig: &tls.Config{InsecureSkipVerify: !cluster.SSLVerify}, } client := &http.Client{ Transport: transport, @@ -343,9 +346,10 @@ func getClusterStatus(service *types.Service) { req.SetBasicAuth(cluster.AuthUser, cluster.AuthPassword) // Make HTTP client + // #nosec var transport http.RoundTripper = &http.Transport{ // Enable/disable SSL verification - TLSClientConfig: &tls.Config{InsecureSkipVerify: !cluster.SSLVerify}, // #nosec + TLSClientConfig: &tls.Config{InsecureSkipVerify: !cluster.SSLVerify}, } client := &http.Client{ Transport: transport, diff --git a/pkg/types/storage.go b/pkg/types/storage.go index 05fc22bc..87de81b1 100644 --- a/pkg/types/storage.go +++ b/pkg/types/storage.go @@ -121,8 +121,9 @@ func (minIOProvider MinIOProvider) GetS3Client() *s3.S3 { // Disable tls verification in client transport if Verify == false if !minIOProvider.Verify { + // #nosec tr := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // #nosec + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, } s3MinIOConfig.HTTPClient = &http.Client{Transport: tr} } diff --git a/pkg/utils/minio.go b/pkg/utils/minio.go index 87d0352b..4c8fdad1 100644 --- a/pkg/utils/minio.go +++ b/pkg/utils/minio.go @@ -77,9 +77,10 @@ func MakeMinIOAdminClient(cfg *types.Config) (*MinIOAdminClient, error) { } // Disable tls verification in client transport if verify == false + // #nosec if !cfg.MinIOProvider.Verify { tr := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // #nosec + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, } adminClient.SetCustomTransport(tr) } From 530c9fe45e42f6243bc4f46a1902931e92c1b521 Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Fri, 29 Nov 2024 09:06:19 +0100 Subject: [PATCH 8/9] minor change --- pkg/utils/minio.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/utils/minio.go b/pkg/utils/minio.go index 4c8fdad1..36a5218b 100644 --- a/pkg/utils/minio.go +++ b/pkg/utils/minio.go @@ -77,8 +77,8 @@ func MakeMinIOAdminClient(cfg *types.Config) (*MinIOAdminClient, error) { } // Disable tls verification in client transport if verify == false - // #nosec if !cfg.MinIOProvider.Verify { + // #nosec tr := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, } From f9ae520b45164fa04be313dc75a3230f36b05297 Mon Sep 17 00:00:00 2001 From: SergioLangaritaBenitez Date: Fri, 10 Jan 2025 13:06:09 +0100 Subject: [PATCH 9/9] resolve test --- pkg/handlers/create.go | 5 ++++- pkg/handlers/delete.go | 10 ++++++++-- pkg/handlers/update.go | 6 ++++-- pkg/utils/minio.go | 13 +++++++++---- 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/pkg/handlers/create.go b/pkg/handlers/create.go index 3ed9b274..7458a7d2 100644 --- a/pkg/handlers/create.go +++ b/pkg/handlers/create.go @@ -373,7 +373,10 @@ func createBuckets(service *types.Service, cfg *types.Config, minIOAdminClient * } if !isAdminUser { - minIOAdminClient.CreateAddPolicy(b, service.AllowedUsers[i], false) + err = minIOAdminClient.CreateAddPolicy(b, service.AllowedUsers[i], false) + if err != nil { + return err + } } } } diff --git a/pkg/handlers/delete.go b/pkg/handlers/delete.go index 102bab39..f9272ba6 100644 --- a/pkg/handlers/delete.go +++ b/pkg/handlers/delete.go @@ -178,7 +178,10 @@ func deleteBuckets(service *types.Service, cfg *types.Config, minIOAdminClient * // Delete user's buckets if isolated spaces had been created if strings.ToUpper(service.IsolationLevel) == "USER" && len(service.BucketList) > 0 { // Delete all private buckets - deletePrivateBuckets(service, minIOAdminClient, s3Client) + err = deletePrivateBuckets(service, minIOAdminClient, s3Client) + if err != nil { + return fmt.Errorf("error while disable the input notification") + } } } @@ -195,7 +198,10 @@ func deleteBuckets(service *types.Service, cfg *types.Config, minIOAdminClient * // Check if the provider identifier is defined in StorageProviders if !isStorageProviderDefined(provName, provID, service.StorageProviders) { // TODO fix - disableInputNotifications(s3Client, service.GetMinIOWebhookARN(), "") + err := disableInputNotifications(s3Client, service.GetMinIOWebhookARN(), "") + if err != nil { + return fmt.Errorf("error while disable the input notification") + } return fmt.Errorf("the StorageProvider \"%s.%s\" is not defined", provName, provID) } diff --git a/pkg/handlers/update.go b/pkg/handlers/update.go index ffdb1eff..3a0edef7 100644 --- a/pkg/handlers/update.go +++ b/pkg/handlers/update.go @@ -129,8 +129,10 @@ func MakeUpdateHandler(cfg *types.Config, back types.ServerlessBackend) gin.Hand return } - disableInputNotifications(s3Client, oldService.GetMinIOWebhookARN(), splitPath[0]) - + err = disableInputNotifications(s3Client, oldService.GetMinIOWebhookARN(), splitPath[0]) + if err != nil { + return + } // Register minio webhook and restart the server if err := registerMinIOWebhook(newService.Name, newService.Token, newService.StorageProviders.MinIO[types.DefaultProvider], cfg); err != nil { uerr := back.UpdateService(*oldService) diff --git a/pkg/utils/minio.go b/pkg/utils/minio.go index 33eff21c..fa5a261e 100644 --- a/pkg/utils/minio.go +++ b/pkg/utils/minio.go @@ -307,8 +307,10 @@ func (minIOAdminClient *MinIOAdminClient) CreateAddPolicy(bucketName string, pol policy = []byte(p) } else { actualPolicy := &Policy{} - json.Unmarshal(policyInfo.Policy, actualPolicy) - + err := json.Unmarshal(policyInfo.Policy, actualPolicy) + if err != nil { + return fmt.Errorf("error unmarshal, the policy is not in correct format") + } // Add new resource and create policy actualPolicy.Statement = []Statement{ { @@ -402,7 +404,10 @@ func (minIOAdminClient *MinIOAdminClient) RemoveFromPolicy(bucketName string, po return fmt.Errorf("policy '%s' does not exist: %v", policyName, errInfo) } actualPolicy := &Policy{} - json.Unmarshal(policyInfo.Policy, actualPolicy) + err := json.Unmarshal(policyInfo.Policy, actualPolicy) + if err != nil { + return fmt.Errorf("error unmarshal, the policy is not in correct format") + } if len(actualPolicy.Statement[0].Resource) == 1 { if err := minIOAdminClient.adminClient.RemoveCannedPolicy(context.TODO(), policyName); err != nil { return fmt.Errorf("error removing canned policy: %v", err) @@ -422,7 +427,7 @@ func (minIOAdminClient *MinIOAdminClient) RemoveFromPolicy(bucketName string, po return jsonErr } - err := minIOAdminClient.adminClient.AddCannedPolicy(context.TODO(), policyName, []byte(policy)) + err = minIOAdminClient.adminClient.AddCannedPolicy(context.TODO(), policyName, []byte(policy)) if err != nil { return fmt.Errorf("error creating MinIO policy for user %s: %v", policyName, err) }