Merge pull request #1229 from s1061123/fix/filepath

Add filepath sanity check

cmd/kubeconfig_generator/main.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"os/signal"
+	"path/filepath"
 	"syscall"
 	"text/template"
 	"time"
@@ -58,10 +59,15 @@ users:
 func main() {
 	certDir := pflag.StringP("certdir", "", "/tmp", "specify cert directory")
 	bootstrapConfig := pflag.StringP("bootstrap-config", "", "/tmp/kubeconfig", "specify bootstrap kubernetes config")
-	kubeconfigPath := pflag.StringP("kubeconfig", "", "/run/multus/kubeconfig", "specify output kubeconfig path")
+	kubeconfigPathRaw := pflag.StringP("kubeconfig", "", "/run/multus/kubeconfig", "specify output kubeconfig path")
 	certDurationString := pflag.StringP("cert-duration", "", "10m", "specify certificate duration")
 	helpFlag := pflag.BoolP("help", "h", false, "show help message and quit")
 
+	kubeconfigPath, err := filepath.Abs(*kubeconfigPathRaw)
+	if err != nil {
+		klog.Fatalf("illegal path %s in kubeconfigPath %s: %v", kubeconfigPath, *kubeconfigPathRaw, err)
+	}
+
 	pflag.Parse()
 	if *helpFlag {
 		pflag.PrintDefaults()
@@ -102,9 +108,9 @@ func main() {
 		klog.Fatalf("failed to start cert manager: %v", err)
 	}
 
-	fp, err := os.OpenFile(*kubeconfigPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600)
+	fp, err := os.OpenFile(kubeconfigPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600)
 	if err != nil {
-		klog.Fatalf("cannot create kubeconfig file %q: %v", *kubeconfigPath, err)
+		klog.Fatalf("cannot create kubeconfig file %q: %v", kubeconfigPath, err)
 	}
 
 	// render kubeconfig
@@ -125,15 +131,15 @@ func main() {
 		klog.Fatalf("cannot save kubeconfig: %v", err)
 	}
 
-	klog.Infof("kubeconfig %q is saved", *kubeconfigPath)
+	klog.Infof("kubeconfig %q is saved", kubeconfigPath)
 
 	// wait for signal
 	sigterm := make(chan os.Signal, 1)
 	signal.Notify(sigterm, syscall.SIGINT, syscall.SIGTERM, syscall.SIGKILL)
 	<-sigterm
-	klog.Infof("signal received. remove kubeconfig %q and quit.", *kubeconfigPath)
-	err = os.Remove(*kubeconfigPath)
+	klog.Infof("signal received. remove kubeconfig %q and quit.", kubeconfigPath)
+	err = os.Remove(kubeconfigPath)
 	if err != nil {
-		klog.Errorf("failed to remove kubeconfig %q: %v", *kubeconfigPath, err)
+		klog.Errorf("failed to remove kubeconfig %q: %v", kubeconfigPath, err)
 	}
 }

cmd/multus-daemon/main.go

@@ -154,7 +154,7 @@ func startMultusDaemon(ctx context.Context, daemonConfig *srv.ControllerNetConf,
 	}
 
 	if daemonConfig.MetricsPort != nil {
-		go utilwait.UntilWithContext(ctx, func(ctx context.Context) {
+		go utilwait.UntilWithContext(ctx, func(_ context.Context) {
 			http.Handle("/metrics", promhttp.Handler())
 			logging.Debugf("metrics port: %d", *daemonConfig.MetricsPort)
 			logging.Debugf("metrics: %s", http.ListenAndServe(fmt.Sprintf(":%d", *daemonConfig.MetricsPort), nil))
@@ -177,17 +177,27 @@ func startMultusDaemon(ctx context.Context, daemonConfig *srv.ControllerNetConf,
 }
 
 func cniServerConfig(configFilePath string) (*srv.ControllerNetConf, error) {
-	configFileContents, err := os.ReadFile(configFilePath)
+	path, err := filepath.Abs(configFilePath)
+	if err != nil {
+		return nil, fmt.Errorf("illegal path %s in server config path %s: %w", path, configFilePath, err)
+	}
+
+	configFileContents, err := os.ReadFile(path)
 	if err != nil {
 		return nil, err
 	}
 	return srv.LoadDaemonNetConf(configFileContents)
 }
 
 func copyUserProvidedConfig(multusConfigPath string, cniConfigDir string) error {
-	srcFile, err := os.Open(multusConfigPath)
+	path, err := filepath.Abs(multusConfigPath)
+	if err != nil {
+		return fmt.Errorf("illegal path %s in multusConfigPath %s: %w", path, multusConfigPath, err)
+	}
+
+	srcFile, err := os.Open(path)
 	if err != nil {
-		return fmt.Errorf("failed to open (READ only) file %s: %w", multusConfigPath, err)
+		return fmt.Errorf("failed to open (READ only) file %s: %w", path, err)
 	}
 
 	dstFileName := cniConfigDir + "/" + filepath.Base(multusConfigPath)

pkg/server/api/api.go

@@ -53,7 +53,7 @@ func DoCNI(url string, req interface{}, socketPath string) ([]byte, error) {
 
 	client := &http.Client{
 		Transport: &http.Transport{
-			Dial: func(proto, addr string) (net.Conn, error) {
+			Dial: func(_, _ string) (net.Conn, error) {
 				return net.Dial("unix", socketPath)
 			},
 		},

pkg/server/config/manager.go

@@ -66,9 +66,14 @@ func NewManager(config MultusConf) (*Manager, error) {
 
 // overrideCNIVersion overrides cniVersion in cniConfigFile, it should be used only in kind case
 func overrideCNIVersion(cniConfigFile string, multusCNIVersion string) error {
-	masterCNIConfigData, err := os.ReadFile(cniConfigFile)
+	path, err := filepath.Abs(cniConfigFile)
 	if err != nil {
-		return fmt.Errorf("failed to read cni config %s: %v", cniConfigFile, err)
+		return fmt.Errorf("illegal path %s in cni config path %s: %w", path, cniConfigFile, err)
+	}
+
+	masterCNIConfigData, err := os.ReadFile(path)
+	if err != nil {
+		return fmt.Errorf("failed to read cni config %s: %v", path, err)
 	}
 
 	var primaryCNIConfigData map[string]interface{}
@@ -82,7 +87,7 @@ func overrideCNIVersion(cniConfigFile string, multusCNIVersion string) error {
 		return fmt.Errorf("couldn't update cluster network config: %v", err)
 	}
 
-	err = os.WriteFile(cniConfigFile, configBytes, 0644)
+	err = os.WriteFile(path, configBytes, 0644)
 	if err != nil {
 		return fmt.Errorf("couldn't update cluster network config: %v", err)
 	}

pkg/server/server.go

@@ -353,7 +353,7 @@ func (s *Server) Start(ctx context.Context, l net.Listener) {
 	waitCancel()
 
 	go func() {
-		utilwait.UntilWithContext(ctx, func(ctx context.Context) {
+		utilwait.UntilWithContext(ctx, func(_ context.Context) {
 			logging.Debugf("open for business")
 			if err := s.Serve(l); err != nil {
 				utilruntime.HandleError(fmt.Errorf("CNI server Serve() failed: %v", err))