From afc81a232e42a05aad922bb5e6e174b589519756 Mon Sep 17 00:00:00 2001 From: Atul-source Date: Tue, 3 Sep 2024 23:07:06 +0530 Subject: [PATCH] fixing security issue Signed-off-by: Atul-source --- .github/workflows/codeql.yaml | 41 +++++-- apis/handlers/restart_linux.go | 66 +++++++++-- artifact/artifact.go | 204 +++++++++++++++++++++++++++++++++ bpfprogs/bpf.go | 185 ++---------------------------- bpfprogs/bpf_test.go | 3 +- bpfprogs/bpfdebug.go | 2 +- bpfprogs/nfconfig.go | 6 +- main.go | 6 +- restart/restart.go | 58 +++++----- stats/metrics.go | 2 +- 10 files changed, 341 insertions(+), 232 deletions(-) create mode 100644 artifact/artifact.go diff --git a/.github/workflows/codeql.yaml b/.github/workflows/codeql.yaml index 5ab013bb..b355b5ad 100644 --- a/.github/workflows/codeql.yaml +++ b/.github/workflows/codeql.yaml @@ -26,25 +26,48 @@ jobs: actions: read contents: read security-events: write - + strategy: fail-fast: false matrix: language: [ 'go' ] steps: - - name: Harden Runner - uses: step-security/harden-runner@5c7944e73c4c2a096b17a9cb74d65b6c2bbafbde - with: - egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs - - name: Checkout repository - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 + uses: actions/checkout@2d7d9f7ff5b310f983d059b68785b3c74d8b8edd - name: Initialize CodeQL - uses: github/codeql-action/init@4dd16135b69a43b6c8efb853346f8437d92d3c93 + uses: github/codeql-action/init@e1f83c153a6cb7134f035e16e2626b216e7168c9 with: languages: ${{ matrix.language }} + - name: Autobuild + uses: github/codeql-action/autobuild@9e39a05578dd315aad814d3c71bd03472cc5b815 + - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@4dd16135b69a43b6c8efb853346f8437d92d3c93 + uses: github/codeql-action/analyze@4067cdab784c667cf1b7fa95169f3a0e0a381d63 + with: + category: "/language:${{matrix.language}}" + output: sarif-results + upload: failure-only + + - name: filter-sarif + uses: advanced-security/filter-sarif@59d0a64b3c0a34d787819f6659708915b6210582 + with: + patterns: | + +**/*.go + -artifact/artifact.go + input: sarif-results/go.sarif + output: sarif-results/go.sarif + + - name: Upload SARIF + uses: github/codeql-action/upload-sarif@2bbafcdd7fbf96243689e764c2f15d9735164f33 + with: + sarif_file: sarif-results/go.sarif + + - name: Upload loc as a Build Artifact + uses: actions/upload-artifact@b18b1d32f3f31abcdc29dee3f2484801fe7822f4 + with: + name: sarif-results + path: sarif-results + retention-days: 1 diff --git a/apis/handlers/restart_linux.go b/apis/handlers/restart_linux.go index 27d04a1b..c8d48866 100644 --- a/apis/handlers/restart_linux.go +++ b/apis/handlers/restart_linux.go @@ -11,12 +11,14 @@ import ( "net" "os" "os/exec" + "regexp" "strconv" "strings" "syscall" "time" "net/http" + "net/url" "github.com/rs/zerolog/log" @@ -71,6 +73,46 @@ func HandleRestart(bpfcfg *bpfprogs.NFConfigs) http.HandlerFunc { statusCode = http.StatusInternalServerError return } + + match, _ := regexp.MatchString(`^v\d+\.\d+\.\d+$`, t.Version) + if !match { + mesg = fmt.Sprintf("version naming convention is wrong it will like vx.y.z") + log.Error().Msg(mesg) + statusCode = http.StatusInternalServerError + return + } + machineHostname, err := os.Hostname() + if err != nil { + mesg = fmt.Sprintf("failed to get os hostname") + log.Error().Msg(mesg) + statusCode = http.StatusInternalServerError + return + } + if machineHostname != t.HostName { + mesg = fmt.Sprintf("this api request is not for provided host") + log.Error().Msg(mesg) + statusCode = http.StatusInternalServerError + return + } + URL, err := url.Parse(t.ArtifactURL) + if err != nil { + mesg = fmt.Sprintf("url format is wrong") + log.Error().Msg(mesg) + statusCode = http.StatusInternalServerError + return + } + if URL.Scheme != models.HttpScheme && URL.Scheme != models.FileScheme && URL.Scheme != models.HttpsScheme { + mesg = fmt.Sprintf("currently only http,https,file is supported") + log.Error().Msg(mesg) + statusCode = http.StatusInternalServerError + return + } + if strings.Contains(t.ArtifactURL, "..") { + mesg = fmt.Sprintf("bad string") + log.Error().Msg(mesg) + statusCode = http.StatusInternalServerError + return + } defer func() { models.IsReadOnly = false }() @@ -88,14 +130,14 @@ func HandleRestart(bpfcfg *bpfprogs.NFConfigs) http.HandlerFunc { err, oldCfgPath := restart.ReadSymlink(bpfcfg.HostConfig.BasePath + "/latest/l3afd.cfg") if err != nil { - mesg = fmt.Sprintf("failed read simlink: %v", err) + mesg = fmt.Sprintf("failed read symlink: %v", err) log.Error().Msg(mesg) statusCode = http.StatusInternalServerError return } err, oldBinPath := restart.ReadSymlink(bpfcfg.HostConfig.BasePath + "/latest/l3afd") if err != nil { - mesg = fmt.Sprintf("failed to read simlink: %v", err) + mesg = fmt.Sprintf("failed to read symlink: %v", err) log.Error().Msg(mesg) statusCode = http.StatusInternalServerError return @@ -179,22 +221,22 @@ func HandleRestart(bpfcfg *bpfprogs.NFConfigs) http.HandlerFunc { err = cmd.Start() if err != nil { log.Error().Msgf("%v", err) - mesg = mesg + fmt.Sprintf("not able to start new instance %v", err) + mesg = mesg + fmt.Sprintf("unable to start new instance %v", err) // write a function a to do cleanup of other process if necessary err = cmd.Process.Kill() if err != nil { log.Error().Msgf("%v", err) - mesg = mesg + fmt.Sprintf("not able to kill the new instance %v", err) + mesg = mesg + fmt.Sprintf("unable to kill the new instance %v", err) } err = bpfcfg.StartAllUserProgramsAndProbes() if err != nil { log.Error().Msgf("%v", err) - mesg = mesg + fmt.Sprintf("not able to start all userprograms and probes: %v", err) + mesg = mesg + fmt.Sprintf("unable to start all userprograms and probes: %v", err) } err = pidfile.CreatePID(bpfcfg.HostConfig.PIDFilename) if err != nil { log.Error().Msgf("%v", err) - mesg = mesg + fmt.Sprintf("not able to create pid file: %v", err) + mesg = mesg + fmt.Sprintf("unable to create pid file: %v", err) } err = restart.RollBackSymlink(oldCfgPath, oldBinPath, oldVersion, t.Version, bpfcfg.HostConfig) if err != nil { @@ -242,17 +284,17 @@ func HandleRestart(bpfcfg *bpfprogs.NFConfigs) http.HandlerFunc { err = cmd.Process.Kill() if err != nil { log.Error().Msgf("%v", err) - mesg = mesg + fmt.Sprintf("not able to kill the new instance %v", err) + mesg = mesg + fmt.Sprintf("unable to kill the new instance %v", err) } err = bpfcfg.StartAllUserProgramsAndProbes() if err != nil { log.Error().Msgf("%v", err) - mesg = mesg + fmt.Sprintf("not able to start all userprograms and probes: %v", err) + mesg = mesg + fmt.Sprintf("unable to start all userprograms and probes: %v", err) } err = pidfile.CreatePID(bpfcfg.HostConfig.PIDFilename) if err != nil { log.Error().Msgf("%v", err) - mesg = mesg + fmt.Sprintf("not able to create pid file: %v", err) + mesg = mesg + fmt.Sprintf("unable to create pid file: %v", err) } err = restart.RollBackSymlink(oldCfgPath, oldBinPath, oldVersion, t.Version, bpfcfg.HostConfig) if err != nil { @@ -273,17 +315,17 @@ func HandleRestart(bpfcfg *bpfprogs.NFConfigs) http.HandlerFunc { err = cmd.Process.Kill() if err != nil { log.Error().Msgf("%v", err) - mesg = mesg + fmt.Sprintf("not able to kill the new instance %v", err) + mesg = mesg + fmt.Sprintf("unable to kill the new instance %v", err) } err = bpfcfg.StartAllUserProgramsAndProbes() if err != nil { log.Error().Msgf("%v", err) - mesg = mesg + fmt.Sprintf("not able to start all userprograms and probes: %v", err) + mesg = mesg + fmt.Sprintf("unable to start all userprograms and probes: %v", err) } err = pidfile.CreatePID(bpfcfg.HostConfig.PIDFilename) if err != nil { log.Error().Msgf("%v", err) - mesg = mesg + fmt.Sprintf("not able to create pid file: %v", err) + mesg = mesg + fmt.Sprintf("unable to create pid file: %v", err) } err = restart.RollBackSymlink(oldCfgPath, oldBinPath, oldVersion, t.Version, bpfcfg.HostConfig) if err != nil { diff --git a/artifact/artifact.go b/artifact/artifact.go new file mode 100644 index 00000000..430a9888 --- /dev/null +++ b/artifact/artifact.go @@ -0,0 +1,204 @@ +package artifact + +import ( + "archive/tar" + "archive/zip" + "bytes" + "compress/gzip" + "fmt" + "io" + "net/http" + "net/url" + "os" + "path/filepath" + "strings" + "sync" + "time" + + "github.com/l3af-project/l3afd/v2/models" +) + +var ( + copyBufPool sync.Pool = sync.Pool{New: func() interface{} { return new(bytes.Buffer) }} +) + +func DownloadArtifact(urlpath string, timeout time.Duration, buf *bytes.Buffer) error { + URL, err := ValidateURL(urlpath) + if err != nil { + return err + } + switch URL.Scheme { + case models.HttpScheme, models.HttpsScheme: + { + timeOut := time.Duration(timeout) * time.Second + var netTransport = &http.Transport{ + ResponseHeaderTimeout: timeOut, + } + client := http.Client{Transport: netTransport, Timeout: timeOut} + // Get the data + resp, err := client.Get(URL.String()) + if err != nil { + return fmt.Errorf("download failed: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("get request returned unexpected status code: %d (%s), %d was expected\n\tResponse Body: %s", resp.StatusCode, http.StatusText(resp.StatusCode), http.StatusOK, buf.Bytes()) + } + buf.ReadFrom(resp.Body) + return nil + } + case models.FileScheme: + { + if FileExists(URL.Path) { + f, err := os.Open(URL.Path) + if err != nil { + return fmt.Errorf("opening err : %w", err) + } + buf.ReadFrom(f) + f.Close() + } else { + return fmt.Errorf("artifact is not found") + } + return nil + } + default: + return fmt.Errorf("unknown url scheme") + } +} +func ExtractArtifact(artifactName string, buf *bytes.Buffer, tempDir string) error { + switch artifact := artifactName; { + case strings.HasSuffix(artifact, ".zip"): + { + c := bytes.NewReader(buf.Bytes()) + zipReader, err := zip.NewReader(c, int64(c.Len())) + if err != nil { + return fmt.Errorf("failed to create zip reader: %w", err) + } + for _, file := range zipReader.File { + + zippedFile, err := file.Open() + if err != nil { + return fmt.Errorf("unzip failed: %w", err) + } + defer zippedFile.Close() + + extractedFilePath, err := ValidatePath(file.Name, tempDir) + if err != nil { + return err + } + + if file.FileInfo().IsDir() { + os.MkdirAll(extractedFilePath, file.Mode()) + } else { + outputFile, err := os.OpenFile( + extractedFilePath, + os.O_WRONLY|os.O_CREATE|os.O_TRUNC, + file.Mode(), + ) + if err != nil { + return fmt.Errorf("unzip failed to create file: %w", err) + } + defer outputFile.Close() + + buf := copyBufPool.Get().(*bytes.Buffer) + _, err = io.CopyBuffer(outputFile, zippedFile, buf.Bytes()) + if err != nil { + return fmt.Errorf("GetArtifacts failed to copy files: %w", err) + } + copyBufPool.Put(buf) + } + } + return nil + } + case strings.HasSuffix(artifact, ".tar.gz"): + { + archive, err := gzip.NewReader(buf) + if err != nil { + return fmt.Errorf("failed to create Gzip reader: %w", err) + } + defer archive.Close() + tarReader := tar.NewReader(archive) + + for { + header, err := tarReader.Next() + + if err == io.EOF { + break + } else if err != nil { + return fmt.Errorf("untar failed: %w", err) + } + + fPath, err := ValidatePath(header.Name, tempDir) + if err != nil { + return err + } + + info := header.FileInfo() + if info.IsDir() { + if err = os.MkdirAll(fPath, info.Mode()); err != nil { + return fmt.Errorf("untar failed to create directories: %w", err) + } + continue + } + + file, err := os.OpenFile(fPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, info.Mode()) + if err != nil { + return fmt.Errorf("untar failed to create file: %w", err) + } + defer file.Close() + + buf := copyBufPool.Get().(*bytes.Buffer) + _, err = io.CopyBuffer(file, tarReader, buf.Bytes()) + if err != nil { + return fmt.Errorf("GetArtifacts failed to copy files: %w", err) + } + copyBufPool.Put(buf) + } + return nil + } + default: + return fmt.Errorf("unknown artifact format") + } +} + +func ValidateURL(urlpath string) (*url.URL, error) { + URL, err := url.Parse(urlpath) + if err != nil { + return nil, fmt.Errorf("unknown url format : %w", err) + } + if URL.Scheme == "" { + return nil, fmt.Errorf("URL scheme is missing") + } + if URL.Host == "" { + return nil, fmt.Errorf("URL host is missing") + } + // Forbid fragment in the URL to prevent potential attacks + if URL.Fragment != "" { + return nil, fmt.Errorf("URL must not contain a fragment") + } + if strings.Contains(URL.Path, "..") { + return nil, fmt.Errorf("URL path must not contain '..'") + } + return URL, nil +} + +func ValidatePath(filePath string, destination string) (string, error) { + destpath := filepath.Join(destination, filePath) + if strings.Contains(filePath, "..") { + return "", fmt.Errorf(" file contains filepath (%s) that includes (..)", filePath) + } + if !strings.HasPrefix(destpath, filepath.Clean(destination)+string(os.PathSeparator)) { + return "", fmt.Errorf("%s: illegal file path", filePath) + } + return destpath, nil +} + +// fileExists checks if a file exists or not +func FileExists(filename string) bool { + info, err := os.Stat(filename) + if os.IsNotExist(err) { + return false + } + return !info.IsDir() +} diff --git a/bpfprogs/bpf.go b/bpfprogs/bpf.go index 4f0ab31d..beb6799d 100644 --- a/bpfprogs/bpf.go +++ b/bpfprogs/bpf.go @@ -5,17 +5,12 @@ package bpfprogs import ( - "archive/tar" - "archive/zip" "bytes" - "compress/gzip" "container/ring" "context" "errors" "fmt" - "io" "net" - "net/http" "net/url" "os" "os/exec" @@ -23,10 +18,10 @@ import ( "path/filepath" "strconv" "strings" - "sync" "time" "unsafe" + "github.com/l3af-project/l3afd/v2/artifact" "github.com/l3af-project/l3afd/v2/config" "github.com/l3af-project/l3afd/v2/models" "github.com/l3af-project/l3afd/v2/stats" @@ -39,8 +34,7 @@ import ( ) var ( - execCommand = exec.Command - copyBufPool sync.Pool = sync.Pool{New: func() interface{} { return new(bytes.Buffer) }} + execCommand = exec.Command ) //lint:ignore U1000 avoid false linter error on windows, since this variable is only used in linux code @@ -171,7 +165,7 @@ func LoadRootProgram(ifaceName string, direction string, progType string, conf * // On l3afd crashing scenario verify root program are unloaded properly by checking existence of persisted maps // if map file exists then root program didn't clean up pinned map files - if fileExists(rootProgBPF.MapNamePath) { + if artifact.FileExists(rootProgBPF.MapNamePath) { log.Warn().Msgf("previous instance of root program %s persisted map %s file exists", rootProgBPF.Program.Name, rootProgBPF.MapNamePath) if err := rootProgBPF.RemoveRootProgMapFile(ifaceName); err != nil { log.Warn().Err(err).Msgf("previous instance of root program %s map file not removed successfully - %s ", rootProgBPF.Program.Name, rootProgBPF.MapNamePath) @@ -615,7 +609,7 @@ func (b *BPF) GetArtifacts(conf *config.Config) error { isDefaultURLUsed = true } - URL, err := url.Parse(RepoURL) + _, err = url.Parse(RepoURL) if err != nil { if isDefaultURLUsed { return fmt.Errorf("unknown ebpf-repo format : %w", err) @@ -624,156 +618,19 @@ func (b *BPF) GetArtifacts(conf *config.Config) error { } } - URL.Path = path.Join(URL.Path, b.Program.Name, b.Program.Version, platform, b.Program.Artifact) - log.Info().Msgf("Retrieving artifact - %s", URL) - err = DownloadArtifact(URL, conf.HttpClientTimeout, buf) + urlpath := path.Join(RepoURL, b.Program.Name, b.Program.Version, platform, b.Program.Artifact) + log.Info().Msgf("Retrieving artifact - %s", urlpath) + err = artifact.DownloadArtifact(urlpath, conf.HttpClientTimeout, buf) tempDir := filepath.Join(conf.BPFDir, b.Program.Name, b.Program.Version) - err = ExtractArtifact(b.Program.Artifact, buf, tempDir) + err = artifact.ExtractArtifact(b.Program.Artifact, buf, tempDir) if err != nil { - return fmt.Errorf("not able to extract artifact %w", err) + return fmt.Errorf("unable to extract artifact %w", err) } newDir := strings.Split(b.Program.Artifact, ".") b.FilePath = filepath.Join(tempDir, newDir[0]) return nil } -func DownloadArtifact(URL *url.URL, timeout time.Duration, buf *bytes.Buffer) error { - switch URL.Scheme { - case models.HttpScheme, models.HttpsScheme: - { - timeOut := time.Duration(timeout) * time.Second - var netTransport = &http.Transport{ - ResponseHeaderTimeout: timeOut, - } - client := http.Client{Transport: netTransport, Timeout: timeOut} - - // Get the data - resp, err := client.Get(URL.String()) - if err != nil { - return fmt.Errorf("download failed: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return fmt.Errorf("get request returned unexpected status code: %d (%s), %d was expected\n\tResponse Body: %s", resp.StatusCode, http.StatusText(resp.StatusCode), http.StatusOK, buf.Bytes()) - } - buf.ReadFrom(resp.Body) - return nil - } - case models.FileScheme: - { - if fileExists(URL.Path) { - f, err := os.Open(URL.Path) - if err != nil { - return fmt.Errorf("opening err : %w", err) - } - buf.ReadFrom(f) - f.Close() - } else { - return fmt.Errorf("artifact is not found") - } - return nil - } - default: - return fmt.Errorf("unknown url scheme") - } -} -func ExtractArtifact(artifactName string, buf *bytes.Buffer, tempDir string) error { - switch artifact := artifactName; { - case strings.HasSuffix(artifact, ".zip"): - { - c := bytes.NewReader(buf.Bytes()) - zipReader, err := zip.NewReader(c, int64(c.Len())) - if err != nil { - return fmt.Errorf("failed to create zip reader: %w", err) - } - for _, file := range zipReader.File { - - zippedFile, err := file.Open() - if err != nil { - return fmt.Errorf("unzip failed: %w", err) - } - defer zippedFile.Close() - - extractedFilePath, err := ValidatePath(file.Name, tempDir) - if err != nil { - return err - } - - if file.FileInfo().IsDir() { - os.MkdirAll(extractedFilePath, file.Mode()) - } else { - outputFile, err := os.OpenFile( - extractedFilePath, - os.O_WRONLY|os.O_CREATE|os.O_TRUNC, - file.Mode(), - ) - if err != nil { - return fmt.Errorf("unzip failed to create file: %w", err) - } - defer outputFile.Close() - - buf := copyBufPool.Get().(*bytes.Buffer) - _, err = io.CopyBuffer(outputFile, zippedFile, buf.Bytes()) - if err != nil { - return fmt.Errorf("GetArtifacts failed to copy files: %w", err) - } - copyBufPool.Put(buf) - } - } - return nil - } - case strings.HasSuffix(artifact, ".tar.gz"): - { - archive, err := gzip.NewReader(buf) - if err != nil { - return fmt.Errorf("failed to create Gzip reader: %w", err) - } - defer archive.Close() - tarReader := tar.NewReader(archive) - - for { - header, err := tarReader.Next() - - if err == io.EOF { - break - } else if err != nil { - return fmt.Errorf("untar failed: %w", err) - } - - fPath, err := ValidatePath(header.Name, tempDir) - if err != nil { - return err - } - - info := header.FileInfo() - if info.IsDir() { - if err = os.MkdirAll(fPath, info.Mode()); err != nil { - return fmt.Errorf("untar failed to create directories: %w", err) - } - continue - } - - file, err := os.OpenFile(fPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, info.Mode()) - if err != nil { - return fmt.Errorf("untar failed to create file: %w", err) - } - defer file.Close() - - buf := copyBufPool.Get().(*bytes.Buffer) - _, err = io.CopyBuffer(file, tarReader, buf.Bytes()) - if err != nil { - return fmt.Errorf("GetArtifacts failed to copy files: %w", err) - } - copyBufPool.Put(buf) - } - return nil - } - default: - return fmt.Errorf("unknown artifact format") - } -} - // create rules file func (b *BPF) createUpdateRulesFile(direction string) (string, error) { @@ -791,15 +648,6 @@ func (b *BPF) createUpdateRulesFile(direction string) (string, error) { } -// fileExists checks if a file exists or not -func fileExists(filename string) bool { - info, err := os.Stat(filename) - if os.IsNotExist(err) { - return false - } - return !info.IsDir() -} - // Add eBPF map into BPFMaps list func (b *BPF) AddBPFMap(mapName string) error { bpfMap, err := b.GetBPFMap(mapName) @@ -871,7 +719,7 @@ func (b *BPF) MonitorMaps(ifaceName string, intervals int) error { _, ok := b.MetricsBpfMaps[mapKey] if !ok { if err := b.AddMetricsBPFMap(element.Name, element.Aggregator, element.Key, intervals); err != nil { - return fmt.Errorf("not able to fetch map %s key %d aggregator %s : %w", element.Name, element.Key, element.Aggregator, err) + return fmt.Errorf("unable to fetch map %s key %d aggregator %s : %w", element.Name, element.Key, element.Aggregator, err) } } bpfMap := b.MetricsBpfMaps[mapKey] @@ -1203,17 +1051,6 @@ func (b *BPF) VerifyCleanupMaps(chain bool) error { return nil } -func ValidatePath(filePath string, destination string) (string, error) { - destpath := filepath.Join(destination, filePath) - if strings.Contains(filePath, "..") { - return "", fmt.Errorf(" file contains filepath (%s) that includes (..)", filePath) - } - if !strings.HasPrefix(destpath, filepath.Clean(destination)+string(os.PathSeparator)) { - return "", fmt.Errorf("%s: illegal file path", filePath) - } - return destpath, nil -} - // LoadBPFProgram - This method loads the eBPF program natively. func (b *BPF) LoadBPFProgram(ifaceName string) error { ObjectFile := filepath.Join(b.FilePath, b.Program.ObjectFile) @@ -1540,7 +1377,7 @@ func (b *BPF) PinBpfMaps(ifaceName string) error { mapFilename = filepath.Join(b.HostConfig.BpfMapDefaultPath, ifaceName, k) } // In case one of the program pins the map then other program will skip - if !fileExists(mapFilename) { + if !artifact.FileExists(mapFilename) { if err := v.Pin(mapFilename); err != nil { return fmt.Errorf("eBPF program %s map %s:failed to pin the map err - %w", b.Program.Name, mapFilename, err) } diff --git a/bpfprogs/bpf_test.go b/bpfprogs/bpf_test.go index ab237644..6c7455b9 100644 --- a/bpfprogs/bpf_test.go +++ b/bpfprogs/bpf_test.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/golang/mock/gomock" + "github.com/l3af-project/l3afd/v2/artifact" "github.com/l3af-project/l3afd/v2/config" "github.com/l3af-project/l3afd/v2/mocks" "github.com/l3af-project/l3afd/v2/models" @@ -645,7 +646,7 @@ func Test_fileExists(t *testing.T) { }, } for _, tt := range tests { - if fileExists(tt.fileName) != tt.exist { + if artifact.FileExists(tt.fileName) != tt.exist { t.Errorf("Invalid filename") } } diff --git a/bpfprogs/bpfdebug.go b/bpfprogs/bpfdebug.go index eeb5d3d9..1384ccfb 100644 --- a/bpfprogs/bpfdebug.go +++ b/bpfprogs/bpfdebug.go @@ -28,7 +28,7 @@ func SetupBPFDebug(ebpfChainDebugAddr string, BPFConfigs *NFConfigs) { } listener, err := net.ListenTCP("tcp", tcpAddr) if err != nil { - log.Fatal().Err(err).Msgf("Not able to create net Listen") + log.Fatal().Err(err).Msgf("unable to create net Listen") } models.AllNetListeners.Store("debug_http", listener) } diff --git a/bpfprogs/nfconfig.go b/bpfprogs/nfconfig.go index aaba97c1..199e79db 100644 --- a/bpfprogs/nfconfig.go +++ b/bpfprogs/nfconfig.go @@ -1623,7 +1623,7 @@ func (c *NFConfigs) StartAllUserProgramsAndProbes() error { prg := b.ProgMapCollection b.ProgMapCollection = nil if err := b.LoadBPFProgram(iface); err != nil { - return fmt.Errorf("not able to load probes %w", err) + return fmt.Errorf("unable to load probes %w", err) } b.Program.EntryFunctionName = ef if b.ProgMapCollection != nil { @@ -1667,7 +1667,7 @@ func (c *NFConfigs) StartAllUserProgramsAndProbes() error { prg := b.ProgMapCollection b.ProgMapCollection = nil if err := b.LoadBPFProgram(iface); err != nil { - return fmt.Errorf("not able to load probes %w", err) + return fmt.Errorf("unable to load probes %w", err) } b.Program.EntryFunctionName = ef if b.ProgMapCollection != nil { @@ -1711,7 +1711,7 @@ func (c *NFConfigs) StartAllUserProgramsAndProbes() error { prg := b.ProgMapCollection b.ProgMapCollection = nil if err := b.LoadBPFProgram(iface); err != nil { - return fmt.Errorf("not able to load probes %w", err) + return fmt.Errorf("unable to load probes %w", err) } b.Program.EntryFunctionName = ef if b.ProgMapCollection != nil { diff --git a/main.go b/main.go index 23b37f96..cf63e56c 100644 --- a/main.go +++ b/main.go @@ -263,14 +263,14 @@ func setupForRestart(ctx context.Context, conf *config.Config) error { models.IsReadOnly = true // Now you need to write client side code conn, err := net.Dial("unix", models.HostSock) - HandleErr(err, "not able to dial unix domain socket") + HandleErr(err, "unable to dial unix domain socket") defer conn.Close() decoder := gob.NewDecoder(conn) var t models.L3AFALLHOSTDATA err = decoder.Decode(&t) - HandleErr(err, "not able to decode") + HandleErr(err, "unable to decode") machineHostname, err := os.Hostname() - HandleErr(err, "not able to fetch the hostname") + HandleErr(err, "unable to fetch the hostname") l, err := restart.Getnetlistener(3, "stat_server") HandleErr(err, "getting stat_server listener failed") diff --git a/restart/restart.go b/restart/restart.go index 9d86f1b2..73eb12a0 100644 --- a/restart/restart.go +++ b/restart/restart.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "net" - "net/url" "os" "path/filepath" "strings" @@ -15,6 +14,7 @@ import ( "github.com/cilium/ebpf" "github.com/cilium/ebpf/link" + "github.com/l3af-project/l3afd/v2/artifact" "github.com/l3af-project/l3afd/v2/bpfprogs" "github.com/l3af-project/l3afd/v2/config" "github.com/l3af-project/l3afd/v2/models" @@ -209,7 +209,7 @@ func Convert(ctx context.Context, t models.L3AFALLHOSTDATA, hostconfig *config.C for _, r := range v { f, err := DeserilazeProgram(ctx, r, hostconfig, k) if err != nil { - log.Err(err).Msg("Deserilization failed for xdpingress") + log.Err(err).Msg("Deserialization failed for xdpingress") return nil, err } l.PushBack(f) @@ -223,7 +223,7 @@ func Convert(ctx context.Context, t models.L3AFALLHOSTDATA, hostconfig *config.C for _, r := range v { f, err := DeserilazeProgram(ctx, r, hostconfig, k) if err != nil { - log.Err(err).Msg("Deserilization failed for tcingress") + log.Err(err).Msg("Deserialization failed for tcingress") return nil, err } l.PushBack(f) @@ -237,7 +237,7 @@ func Convert(ctx context.Context, t models.L3AFALLHOSTDATA, hostconfig *config.C for _, r := range v { f, err := DeserilazeProgram(ctx, r, hostconfig, k) if err != nil { - log.Err(err).Msg("Deserilization failed for tcegress") + log.Err(err).Msg("Deserialization failed for tcegress") return nil, err } l.PushBack(f) @@ -277,7 +277,7 @@ func Getnetlistener(fd int, fname string) (*net.TCPListener, error) { } lf, e := l.(*net.TCPListener) if !e { - return nil, fmt.Errorf("not able to covert to tcp listner") + return nil, fmt.Errorf("unable to covert to tcp listner") } file.Close() return lf, nil @@ -306,51 +306,50 @@ func GetNewVersion(urlpath string, oldVersion, newVersion string, conf *config.C return nil } newVersionPath := conf.BasePath + "/" + newVersion + if !strings.HasPrefix(conf.BasePath, filepath.Clean(newVersionPath)+string(os.PathSeparator)) { + return fmt.Errorf("malicious input given to the restart api") + } err := os.RemoveAll(newVersionPath) if err != nil { - return fmt.Errorf("Error while deleting directory: %w", err) + return fmt.Errorf("error while deleting directory: %w", err) } err = os.MkdirAll(newVersionPath, 0750) if err != nil { - return fmt.Errorf("Error while creating directory: %w", err) + return fmt.Errorf("error while creating directory: %w", err) } // now I need to download artifacts - URL, err := url.Parse(urlpath) - if err != nil { - return fmt.Errorf("unknown url format : %w", err) - } buf := &bytes.Buffer{} - err = bpfprogs.DownloadArtifact(URL, conf.HttpClientTimeout, buf) + err = artifact.DownloadArtifact(urlpath, conf.HttpClientTimeout, buf) if err != nil { - return fmt.Errorf("not able to download artifacts %w", err) + return fmt.Errorf("unable to download artifacts %w", err) } sp := strings.Split(urlpath, "/") artifactName := sp[len(sp)-1] - err = bpfprogs.ExtractArtifact(artifactName, buf, newVersionPath) + err = artifact.ExtractArtifact(artifactName, buf, newVersionPath) dir := strings.Split(artifactName, ".")[0] if err != nil { - return fmt.Errorf("not able to extract artifacts %w", err) + return fmt.Errorf("unable to extract artifacts %w", err) } // you need to store the old path for rollback purposes - // we will remove simlink + // we will remove symlink err = RemoveSymlink(conf.BasePath + "/latest/l3afd") if err != nil { - return fmt.Errorf("not able to remove simlink %w", err) + return fmt.Errorf("unable to remove symlink %w", err) } err = RemoveSymlink(conf.BasePath + "/latest/l3afd.cfg") if err != nil { - return fmt.Errorf("not able to remove simlink %w", err) + return fmt.Errorf("unable to remove symlink %w", err) } - // add new simlink + // add new symlink err = AddSymlink(newVersionPath+"/"+dir+"/l3afd", conf.BasePath+"/latest/l3afd") if err != nil { - return fmt.Errorf("not able to add simlink %w", err) + return fmt.Errorf("unable to add symlink %w", err) } err = AddSymlink(newVersionPath+"/"+dir+"/l3afd.cfg", conf.BasePath+"/latest/l3afd.cfg") if err != nil { - return fmt.Errorf("not able to add simlink %w", err) + return fmt.Errorf("unable to add symlink %w", err) } // now we are good return nil @@ -361,31 +360,34 @@ func RollBackSymlink(oldCfgPath, oldBinPath string, oldVersion, newVersion strin return nil } // you need to store the old path for rollback purposes - // we will remove simlink + // we will remove symlink err := RemoveSymlink(conf.BasePath + "/latest/l3afd") if err != nil { - return fmt.Errorf("not able to remove simlink %w", err) + return fmt.Errorf("unable to remove symlink %w", err) } err = RemoveSymlink(conf.BasePath + "/latest/l3afd.cfg") if err != nil { - return fmt.Errorf("not able to remove simlink %w", err) + return fmt.Errorf("unable to remove symlink %w", err) } - // add new simlink + // add new symlink err = AddSymlink(oldBinPath, conf.BasePath+"/latest/l3afd") if err != nil { - return fmt.Errorf("not able to add simlink %w", err) + return fmt.Errorf("unable to add symlink %w", err) } err = AddSymlink(oldCfgPath, conf.BasePath+"/latest/l3afd.cfg") if err != nil { - return fmt.Errorf("not able to add simlink %w", err) + return fmt.Errorf("unable to add symlink %w", err) } newVersionPath := conf.BasePath + "/" + newVersion + if strings.Contains(newVersionPath, "..") { + return fmt.Errorf("malicious path") + } err = os.RemoveAll(newVersionPath) if err != nil { - return fmt.Errorf("Error while deleting directory: %w", err) + return fmt.Errorf("error while deleting directory: %w", err) } return nil } diff --git a/stats/metrics.go b/stats/metrics.go index 99ac560b..d49ecec4 100644 --- a/stats/metrics.go +++ b/stats/metrics.go @@ -130,7 +130,7 @@ func SetupMetrics(hostname, daemonName, metricsAddr string) { } listener, err := net.ListenTCP("tcp", tcpAddr) if err != nil { - log.Fatal().Err(err).Msgf("Not able to create net Listen") + log.Fatal().Err(err).Msgf("unable to create net Listen") } models.AllNetListeners.Store("stat_http", listener) }