Skip to content

Commit

Permalink
Fix sec issues
Browse files Browse the repository at this point in the history
  • Loading branch information
micafer committed Nov 27, 2024
1 parent 03596d1 commit 370c275
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 30 deletions.
5 changes: 4 additions & 1 deletion pkg/backends/knative.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
45 changes: 36 additions & 9 deletions pkg/handlers/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
}
Expand All @@ -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
}
Expand All @@ -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
}

Expand Down Expand Up @@ -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)
}

Expand All @@ -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)
}
}
Expand All @@ -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)
}
}
Expand All @@ -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)
}
}
Expand Down
10 changes: 8 additions & 2 deletions pkg/handlers/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,21 @@ 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 != "" {
path := strings.Trim(service.Mount.Path, " /")
// 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
Expand Down
10 changes: 8 additions & 2 deletions pkg/handlers/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
}
Expand Down
16 changes: 12 additions & 4 deletions pkg/imagepuller/daemonset.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,19 @@ 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

//Get daemonset definition
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())
Expand Down Expand Up @@ -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())
Expand Down
30 changes: 24 additions & 6 deletions pkg/types/expose.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
}
}

Expand Down Expand Up @@ -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
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/types/expose_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
Expand Down
4 changes: 2 additions & 2 deletions pkg/utils/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion pkg/utils/auth/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
10 changes: 8 additions & 2 deletions pkg/utils/minio.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/utils/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

0 comments on commit 370c275

Please sign in to comment.