Skip to content

Commit

Permalink
Support pipe command (#3692)
Browse files Browse the repository at this point in the history
* Support safe pipeline shell, To #51793350

Signed-off-by: cheyang <[email protected]>

* Support safe pipeline shell, To #51793350

Signed-off-by: cheyang <[email protected]>

* Support safe pipeline shell, To #51793350

Signed-off-by: cheyang <[email protected]>

* Fix shell options, To #53185204

Signed-off-by: cheyang <[email protected]>

* Fix ut for shell pipelines, To #53185204

Signed-off-by: cheyang <[email protected]>

* Fix ut for shell pipelines, To #53185204

Signed-off-by: cheyang <[email protected]>

* Add ut, To #51568429

Signed-off-by: cheyang <[email protected]>

* Add ut, To #51568429

Signed-off-by: cheyang <[email protected]>

* Add ut, To #51568429

Signed-off-by: cheyang <[email protected]>

* Add ut, To #51568429

Signed-off-by: cheyang <[email protected]>

* Add ut, To #51568429

Signed-off-by: cheyang <[email protected]>

* Add ut, To #51568429

Signed-off-by: cheyang <[email protected]>

* Add ut, To #51568429

Signed-off-by: cheyang <[email protected]>

* Add ut, To #51568429

Signed-off-by: cheyang <[email protected]>

---------

Signed-off-by: cheyang <[email protected]>
  • Loading branch information
cheyang committed Jan 19, 2024
1 parent d285cf5 commit 5a1bb89
Show file tree
Hide file tree
Showing 12 changed files with 340 additions and 7 deletions.
5 changes: 5 additions & 0 deletions pkg/ddc/alluxio/operations/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,11 @@ func (a AlluxioFileUtils) exec(command []string, verbose bool) (stdout string, s

// execWithoutTimeout
func (a AlluxioFileUtils) execWithoutTimeout(command []string, verbose bool) (stdout string, stderr string, err error) {
err = utils.ValidateCommandSlice(command)
if err != nil {
return
}

stdout, stderr, err = kubeclient.ExecCommandInContainer(a.podName, a.container, a.namespace, command)
if err != nil {
a.log.Info("Stdout", "Command", command, "Stdout", stdout)
Expand Down
5 changes: 4 additions & 1 deletion pkg/ddc/goosefs/hcfs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,10 @@ func TestGetHCFSStatus(t *testing.T) {
t.Fatal(err.Error())
}
engine := newGooseFSEngineHCFS(fakeClient, "hbase", "fluid")
out, _ := engine.GetHCFSStatus()
out, err := engine.GetHCFSStatus()
if err != nil {
t.Fatal(err.Error())
}
wrappedUnhook()
status := &v1alpha1.HCFSStatus{
Endpoint: "goosefs://hbase-master-0.fluid:2333",
Expand Down
6 changes: 6 additions & 0 deletions pkg/ddc/goosefs/operations/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"strings"
"time"

"github.com/fluid-cloudnative/fluid/pkg/utils"
"github.com/fluid-cloudnative/fluid/pkg/utils/kubeclient"
"github.com/go-logr/logr"
)
Expand Down Expand Up @@ -501,6 +502,11 @@ func (a GooseFSFileUtils) exec(command []string, verbose bool) (stdout string, s

// execWithoutTimeout
func (a GooseFSFileUtils) execWithoutTimeout(command []string, verbose bool) (stdout string, stderr string, err error) {
err = utils.ValidateCommandSlice(command)
if err != nil {
return
}

stdout, stderr, err = kubeclient.ExecCommandInContainer(a.podName, a.container, a.namespace, command)
if err != nil {
a.log.Info("Stdout", "Command", command, "Stdout", stdout)
Expand Down
6 changes: 6 additions & 0 deletions pkg/ddc/jindo/operations/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"strings"
"time"

"github.com/fluid-cloudnative/fluid/pkg/utils"
"github.com/fluid-cloudnative/fluid/pkg/utils/kubeclient"
"github.com/go-logr/logr"
)
Expand Down Expand Up @@ -66,6 +67,11 @@ func (a JindoFileUtils) exec(command []string, verbose bool) (stdout string, std

// execWithoutTimeout
func (a JindoFileUtils) execWithoutTimeout(command []string, verbose bool) (stdout string, stderr string, err error) {
err = utils.ValidateCommandSlice(command)
if err != nil {
return
}

stdout, stderr, err = kubeclient.ExecCommandInContainer(a.podName, a.container, a.namespace, command)
if err != nil {
a.log.Info("Stdout", "Command", command, "Stdout", stdout)
Expand Down
6 changes: 6 additions & 0 deletions pkg/ddc/jindofsx/operations/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"strings"
"time"

"github.com/fluid-cloudnative/fluid/pkg/utils"
"github.com/fluid-cloudnative/fluid/pkg/utils/kubeclient"
"github.com/go-logr/logr"
)
Expand Down Expand Up @@ -87,6 +88,11 @@ func (a JindoFileUtils) execWithTimeOut(command []string, verbose bool, second i

// execWithoutTimeout
func (a JindoFileUtils) execWithoutTimeout(command []string, verbose bool) (stdout string, stderr string, err error) {
err = utils.ValidateCommandSlice(command)
if err != nil {
return
}

stdout, stderr, err = kubeclient.ExecCommandInContainer(a.podName, a.container, a.namespace, command)
if err != nil {
a.log.Info("Stdout", "Command", command, "Stdout", stdout)
Expand Down
5 changes: 3 additions & 2 deletions pkg/ddc/juicefs/data_load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ package juicefs

import (
"errors"
"github.com/fluid-cloudnative/fluid/pkg/common"
"os"
"path/filepath"
"reflect"
"strings"
"testing"

"github.com/fluid-cloudnative/fluid/pkg/common"

"github.com/brahma-adshonor/gohook"
appsv1 "k8s.io/api/apps/v1"
v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -621,7 +622,7 @@ func TestJuiceFSEngine_CheckExistenceOfPath(t *testing.T) {
}
notExist, err = engine.CheckExistenceOfPath(targetDataload)
if !(err == nil && notExist == false) {
t.Errorf("fail to exec the function")
t.Errorf("fail to exec the function due to %v", err)
}
wrappedUnhook()
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/ddc/juicefs/operations/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"github.com/go-logr/logr"

"github.com/fluid-cloudnative/fluid/pkg/utils"
"github.com/fluid-cloudnative/fluid/pkg/utils/kubeclient"
)

Expand Down Expand Up @@ -325,6 +326,12 @@ func (j JuiceFileUtils) exec(command []string) (stdout string, stderr string, er

// execWithoutTimeout
func (j JuiceFileUtils) execWithoutTimeout(command []string) (stdout string, stderr string, err error) {
// validate the pipe command with white list
err = utils.ValidateCommandSlice(command)
if err != nil {
return
}

stdout, stderr, err = kubeclient.ExecCommandInContainer(j.podName, j.container, j.namespace, command)
if err != nil {
j.log.Info("Stdout", "Command", command, "Stdout", stdout)
Expand Down
7 changes: 7 additions & 0 deletions pkg/ddc/thin/operations/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"strings"
"time"

"github.com/fluid-cloudnative/fluid/pkg/utils"
"github.com/fluid-cloudnative/fluid/pkg/utils/kubeclient"
"github.com/go-logr/logr"
)
Expand Down Expand Up @@ -155,6 +156,12 @@ func (t ThinFileUtils) exec(command []string, verbose bool) (stdout string, stde

// execWithoutTimeout
func (t ThinFileUtils) execWithoutTimeout(command []string, verbose bool) (stdout string, stderr string, err error) {
// validate the pipe command with white list
err = utils.ValidateCommandSlice(command)
if err != nil {
return
}

stdout, stderr, err = kubeclient.ExecCommandInContainer(t.podName, t.container, t.namespace, command)
if err != nil {
t.log.Info("Stdout", "Command", command, "Stdout", stdout)
Expand Down
6 changes: 5 additions & 1 deletion pkg/utils/helm/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,11 @@ func GetChartVersion(chart string) (version string, err error) {
"|", "grep", "version:"}
log.V(1).Info("Exec bash -c", "args", args)

cmd := exec.Command("bash", "-c", strings.Join(args, " "))
// cmd := exec.Command("bash", "-c", strings.Join(args, " "))
cmd, err := utils.PipeCommand("bash", "-c", strings.Join(args, " "))
if err != nil {
return "", err
}
out, err := cmd.Output()
if err != nil {
return "", err
Expand Down
6 changes: 3 additions & 3 deletions pkg/utils/helm/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func TestGenerateHelmTemplate(t *testing.T) {

func TestGetChartVersion(t *testing.T) {
LookPathCommon := func(file string) (string, error) {
return "test-path", nil
return "helm", nil
}
LookPathErr := func(file string) (string, error) {
return "", errors.New("fail to run the command")
Expand Down Expand Up @@ -190,10 +190,10 @@ func TestGetChartVersion(t *testing.T) {
}
version, err := GetChartVersion("fluid:v0.6.0")
if err != nil {
t.Errorf("fail to exec the function")
t.Errorf("fail to exec the function due to %v", err)
}
if version != "v0.6.0" {
t.Errorf("fail to get the version of the helm")
t.Errorf("fail to get the version of the helm due to %v", err)
}
wrappedUnhookOutput()
wrappedUnhookStat()
Expand Down
156 changes: 156 additions & 0 deletions pkg/utils/shell_pipes.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
/*
Copyright 2024 The Fluid Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package utils

import (
"fmt"
"os/exec"
"strings"
)

func PipeCommand(name string, arg ...string) (cmd *exec.Cmd, err error) {
// prepare the slice for ValidatePipeCommandSlice
var commands []string
commands = append(commands, name)
commands = append(commands, arg...)

// validate commands
err = ValidatePipeCommandSlice(commands)
if err != nil {
return nil, err
}

return exec.Command(name, arg...), nil
}

// ValidateCommandSlice takes in a slice of shell commands and returns an error if any are invalid.
// The function looks specifically for pipe commands (i.e., commands that contain a '|').
// If a pipe command is found in the slice, ValidatePipeCommandSlice is called for further validation.
func ValidateCommandSlice(shellCommandSlice []string) (err error) {
isPossiblePipeCommand := false

for _, command := range shellCommandSlice {
if strings.Contains(command, "|") {
isPossiblePipeCommand = true
}
}

if isPossiblePipeCommand {
err = ValidatePipeCommandSlice(shellCommandSlice)
} // else {
// // Todo: need handle no PossiblePipeCommand
// }
return
}

func ValidatePipeCommandSlice(shellCommandSlice []string) (err error) {
// Make sure the shell command is allowed
var AllowedShellCommands = map[string]bool{
"bash -c": true,
"sh -c": true,
}

// check if shellCommandSlice has enough arguments
if len(shellCommandSlice) < 3 {
return fmt.Errorf("insufficient arguments. Expected at least 3, received %d", len(shellCommandSlice))
}
// We assume -c always directly follows the shell command
shellCommand := strings.Join(strings.Fields(shellCommandSlice[0]+" "+shellCommandSlice[1]), " ")
if _, ok := AllowedShellCommands[shellCommand]; !ok {
return fmt.Errorf("unknown shell command: %s", shellCommand)
}

for _, command := range shellCommandSlice[2:] {
if err := ValidateShellPipeString(command); err != nil {
return err
}
}
return
}

// ValidateShellPipeString function checks whether the input command string is safe to execute.
// It checks whether all parts of a pipeline command start with any command prefixes defined in AllowedCommands
// It also checks for any illegal sequences that may lead to command injection attack.
func ValidateShellPipeString(command string) error {
// Define illegal sequences that may lead to command injection attack
illegalSequences := []string{"&", ";", "$", "'", "`", "(", ")", "||", ">>"}
// Separate parts of pipeline command
pipelineCommands := strings.Split(command, "|")

// AllowedCommands is a global map that contains all allowed command prefixes.
var AllowedCommands = map[string]bool{
"ls": false,
"df": false,
"mount": false,
"alluxio": false,
"goosefs": false,
"kubectl": false,
"helm": false,
}

// AllowedPipeCommands is a map that contains all allowed pipe command prefixes.
var allowedPipeCommands = map[string]bool{
"grep": false, // false means partial match
"wc -l": true, // true means full match (wc -l is exactly the allowed command)
// Add more commands as you see fit
}

// Check each part of pipeline command
for i, cmd := range pipelineCommands {
cmd = strings.Join(
strings.Fields(
strings.TrimSpace(cmd)), " ")

if i > 0 {
// Check whether command starts with any allowed command prefix
validCmd := isValidCommand(cmd, allowedPipeCommands)

// If none of the allowed command prefix is found, throw error
if !validCmd {
return fmt.Errorf("full pipeline command not supported: part %d contains unsupported command '%s', the whole command %s", i+1, cmd, command)
}
} else {
validCmd := isValidCommand(cmd, AllowedCommands)
// If none of the allowed command prefix is found, throw error
if !validCmd {
return fmt.Errorf("full pipeline command not supported: part %d contains unsupported command '%s', the whole command %s", i+1, cmd, command)
}
}

// Check for illegal sequences in command
for _, illegalSeq := range illegalSequences {
if strings.Contains(cmd, illegalSeq) {
return fmt.Errorf("unsafe pipeline command %s, illegal sequence detected: %s in part %d: '%s'", command, illegalSeq, i+1, cmd)
}
}
}

// If no error found, return nil
return nil
}

// Defining a function to check if the command is valid
func isValidCommand(cmd string, allowedCommands map[string]bool) bool {
for cmdPrefix, exactMatch := range allowedCommands {
if exactMatch && cmd == cmdPrefix {
return true
} else if !exactMatch && strings.HasPrefix(cmd, cmdPrefix) {
return true
}
}
return false
}
Loading

0 comments on commit 5a1bb89

Please sign in to comment.