From 748930239d6543875ab9a6efab15409be74b1512 Mon Sep 17 00:00:00 2001 From: Tomofumi Hayashi Date: Wed, 14 Feb 2024 16:13:55 +0900 Subject: [PATCH] Add filepath sanity check --- cmd/kubeconfig_generator/main.go | 20 +++++++++++++------- cmd/multus-daemon/main.go | 18 ++++++++++++++---- pkg/server/api/api.go | 2 +- pkg/server/config/manager.go | 11 ++++++++--- pkg/server/server.go | 2 +- 5 files changed, 37 insertions(+), 16 deletions(-) diff --git a/cmd/kubeconfig_generator/main.go b/cmd/kubeconfig_generator/main.go index e2a653f23..943c42f46 100644 --- a/cmd/kubeconfig_generator/main.go +++ b/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) } } diff --git a/cmd/multus-daemon/main.go b/cmd/multus-daemon/main.go index 39d83671f..11d38f05c 100644 --- a/cmd/multus-daemon/main.go +++ b/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,7 +177,12 @@ 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 } @@ -185,9 +190,14 @@ func cniServerConfig(configFilePath string) (*srv.ControllerNetConf, error) { } 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) diff --git a/pkg/server/api/api.go b/pkg/server/api/api.go index 10e6239f6..b9228f0dc 100644 --- a/pkg/server/api/api.go +++ b/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) }, }, diff --git a/pkg/server/config/manager.go b/pkg/server/config/manager.go index 80a5d6c6b..6b450c13a 100644 --- a/pkg/server/config/manager.go +++ b/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) } diff --git a/pkg/server/server.go b/pkg/server/server.go index 3f3583601..bc5c4a403 100644 --- a/pkg/server/server.go +++ b/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))