From b4258102c211b408eee19517264c424a70b89588 Mon Sep 17 00:00:00 2001 From: Reny Paul Date: Wed, 21 Feb 2024 11:46:20 +0530 Subject: [PATCH 1/6] Issue: #286 Description This change allows packages to be fetched from repository other than https://repo.maven.apache.org/maven2/ Get username and password from settings file for the corresponding server ID mentioned in the pom file Added support for authentication if username and password mentioned while fetching the package --- pkg/dependency/parser/java/pom/parse.go | 42 +++++- pkg/dependency/parser/java/pom/parse_test.go | 2 +- pkg/dependency/parser/java/pom/settings.go | 49 ++++++- .../parser/java/pom/settings_test.go | 136 ++++++++++++++++++ .../java/pom/testdata/conf/settings.xml | 66 --------- .../settings/global/conf/settings.xml | 21 +++ .../testdata/settings/user/.m2/settings.xml | 21 +++ 7 files changed, 262 insertions(+), 75 deletions(-) create mode 100644 pkg/dependency/parser/java/pom/settings_test.go delete mode 100644 pkg/dependency/parser/java/pom/testdata/conf/settings.xml create mode 100644 pkg/dependency/parser/java/pom/testdata/settings/global/conf/settings.xml create mode 100755 pkg/dependency/parser/java/pom/testdata/settings/user/.m2/settings.xml diff --git a/pkg/dependency/parser/java/pom/parse.go b/pkg/dependency/parser/java/pom/parse.go index 408a0bfd0bc6..0b14255f3ed9 100644 --- a/pkg/dependency/parser/java/pom/parse.go +++ b/pkg/dependency/parser/java/pom/parse.go @@ -53,6 +53,7 @@ type parser struct { localRepository string remoteRepositories []string offline bool + servers []Server } func NewParser(filePath string, opts ...option) types.Parser { @@ -78,6 +79,7 @@ func NewParser(filePath string, opts ...option) types.Parser { localRepository: localRepository, remoteRepositories: o.remoteRepos, offline: o.offline, + servers: s.Servers, } } @@ -87,6 +89,32 @@ func (p *parser) Parse(r dio.ReadSeekerAt) ([]types.Library, []types.Dependency, return nil, nil, xerrors.Errorf("failed to parse POM: %w", err) } + var remoteRepositories []string + for _, rep := range content.Repositories.Repository { + if rep.Releases.Enabled == "false" && rep.Snapshots.Enabled == "false" { + continue + } + + repoURL, err := url.Parse(rep.URL) + if err != nil { + log.Logger.Debugf("Unable to parse remote repository url: %s", err) + continue + } + + for _, server := range p.servers { + if rep.ID == server.ID && server.Username != "" && server.Password != "" { + repoURL.User = url.UserPassword(server.Username, server.Password) + break + } + } + + log.Logger.Debugf("Adding repository %s: %s", rep.ID, rep.URL) + remoteRepositories = append(remoteRepositories, repoURL.String()) + } + + // Add central maven repository or repositories obtained using `WithRemoteRepos` function. + p.remoteRepositories = append(remoteRepositories, p.remoteRepositories...) + root := &pom{ filePath: p.rootPath, content: content, @@ -644,8 +672,20 @@ func fetchPOMFromRemoteRepository(repo string, paths []string) (*pom, error) { paths = append([]string{repoURL.Path}, paths...) repoURL.Path = path.Join(paths...) - resp, err := http.Get(repoURL.String()) + client := &http.Client{} + req, err := http.NewRequest("GET", repoURL.String(), nil) + if err != nil { + log.Logger.Debugf("Request failed for %s/%s", repoURL.Host, repoURL.Path) + return nil, nil + } + if repoURL.User != nil { + password, _ := repoURL.User.Password() + req.SetBasicAuth(repoURL.User.Username(), password) + } + + resp, err := client.Do(req) if err != nil || resp.StatusCode != http.StatusOK { + log.Logger.Debugf("Failed to fetch from %s/%s", repoURL.Host, repoURL.Path) return nil, nil } defer resp.Body.Close() diff --git a/pkg/dependency/parser/java/pom/parse_test.go b/pkg/dependency/parser/java/pom/parse_test.go index 837306e9482d..7c7410bd9834 100644 --- a/pkg/dependency/parser/java/pom/parse_test.go +++ b/pkg/dependency/parser/java/pom/parse_test.go @@ -1226,7 +1226,7 @@ func TestPom_Parse(t *testing.T) { var remoteRepos []string if tt.local { // for local repository - t.Setenv("MAVEN_HOME", "testdata") + t.Setenv("MAVEN_HOME", "testdata/settings/global") } else { // for remote repository h := http.FileServer(http.Dir(filepath.Join("testdata", "repository"))) diff --git a/pkg/dependency/parser/java/pom/settings.go b/pkg/dependency/parser/java/pom/settings.go index 7237875db98f..42153155706a 100644 --- a/pkg/dependency/parser/java/pom/settings.go +++ b/pkg/dependency/parser/java/pom/settings.go @@ -8,24 +8,59 @@ import ( "golang.org/x/net/html/charset" ) +type Server struct { + ID string `xml:"id"` + Username string `xml:"username"` + Password string `xml:"password"` +} + type settings struct { - LocalRepository string `xml:"localRepository"` + LocalRepository string `xml:"localRepository"` + Servers []Server `xml:"servers>server"` +} + +// serverFound checks that servers already contain server. +// Maven compares servers by ID only. +func serverFound(servers []Server, id string) bool { + for _, server := range servers { + if server.ID == id { + return true + } + } + return false } func readSettings() settings { + s := settings{} + userSettingsPath := filepath.Join(os.Getenv("HOME"), ".m2", "settings.xml") userSettings, err := openSettings(userSettingsPath) - if err == nil && userSettings.LocalRepository != "" { - return userSettings + if err == nil { + s = userSettings } - globalSettingsPath := filepath.Join(os.Getenv("MAVEN_HOME"), "conf", "settings.xml") + // Some package managers use this path by default + mavenHome := "/usr/share/maven" + if mHome := os.Getenv("MAVEN_HOME"); mHome != "" { + mavenHome = mHome + } + globalSettingsPath := filepath.Join(mavenHome, "conf", "settings.xml") globalSettings, err := openSettings(globalSettingsPath) - if err == nil && globalSettings.LocalRepository != "" { - return globalSettings + if err == nil { + // We need to merge global and user settings. User settings being dominant. + // https://maven.apache.org/settings.html#quick-overview + if s.LocalRepository == "" { + s.LocalRepository = globalSettings.LocalRepository + } + // Maven checks user servers first, then global servers + for _, server := range globalSettings.Servers { + if !serverFound(s.Servers, server.ID) { + s.Servers = append(s.Servers, server) + } + } } - return settings{} + return s } func openSettings(filePath string) (settings, error) { diff --git a/pkg/dependency/parser/java/pom/settings_test.go b/pkg/dependency/parser/java/pom/settings_test.go new file mode 100644 index 000000000000..0d4941a18f97 --- /dev/null +++ b/pkg/dependency/parser/java/pom/settings_test.go @@ -0,0 +1,136 @@ +package pom + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_ReadSettings(t *testing.T) { + tests := []struct { + name string + envs map[string]string + wantSettings settings + }{ + { + name: "happy path with only global settings", + envs: map[string]string{ + "HOME": "", + "MAVEN_HOME": filepath.Join("testdata", "settings", "global"), + }, + wantSettings: settings{ + LocalRepository: "testdata/repository", + Servers: []Server{ + { + ID: "global-server", + }, + { + ID: "server-with-credentials", + Username: "test-user", + Password: "test-password-from-global", + }, + { + ID: "server-with-name-only", + Username: "test-user-only", + }, + }, + }, + }, + { + name: "happy path with only user settings", + envs: map[string]string{ + "HOME": filepath.Join("testdata", "settings", "user"), + "MAVEN_HOME": "NOT_EXISTING_PATH", + }, + wantSettings: settings{ + LocalRepository: "testdata/user/repository", + Servers: []Server{ + { + ID: "user-server", + }, + { + ID: "server-with-credentials", + Username: "test-user", + Password: "test-password", + }, + { + ID: "server-with-name-only", + Username: "test-user-only", + }, + }, + }, + }, + { + // $ mvn help:effective-settings + //[INFO] ------------------< org.apache.maven:standalone-pom >------------------- + //[INFO] --- maven-help-plugin:3.4.0:effective-settings (default-cli) @ standalone-pom --- + //Effective user-specific configuration settings: + // + // + // + // /root/testdata/user/repository + // + // + // user-server + // + // + // test-user + // *** + // server-with-credentials + // + // + // test-user-only + // server-with-name-only + // + // + // global-server + // + // + // + name: "happy path with global and user settings", + envs: map[string]string{ + "HOME": filepath.Join("testdata", "settings", "user"), + "MAVEN_HOME": filepath.Join("testdata", "settings", "global"), + }, + wantSettings: settings{ + LocalRepository: "testdata/user/repository", + Servers: []Server{ + { + ID: "user-server", + }, + { + ID: "server-with-credentials", + Username: "test-user", + Password: "test-password", + }, + { + ID: "server-with-name-only", + Username: "test-user-only", + }, + { + ID: "global-server", + }, + }, + }, + }, + { + name: "without settings", + envs: map[string]string{ + "HOME": "", + "MAVEN_HOME": "NOT_EXISTING_PATH", + }, + wantSettings: settings{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for env, settingsDir := range tt.envs { + t.Setenv(env, settingsDir) + } + + gotSettings := readSettings() + require.Equal(t, tt.wantSettings, gotSettings) + }) + } +} diff --git a/pkg/dependency/parser/java/pom/testdata/conf/settings.xml b/pkg/dependency/parser/java/pom/testdata/conf/settings.xml deleted file mode 100644 index 133a101b1bbd..000000000000 --- a/pkg/dependency/parser/java/pom/testdata/conf/settings.xml +++ /dev/null @@ -1,66 +0,0 @@ - - - - - - - testdata/repository - - - - - - - - - - - - - - - - - diff --git a/pkg/dependency/parser/java/pom/testdata/settings/global/conf/settings.xml b/pkg/dependency/parser/java/pom/testdata/settings/global/conf/settings.xml new file mode 100644 index 000000000000..791406988b03 --- /dev/null +++ b/pkg/dependency/parser/java/pom/testdata/settings/global/conf/settings.xml @@ -0,0 +1,21 @@ + + + testdata/repository + + + + global-server + + + server-with-credentials + test-user + test-password-from-global + + + server-with-name-only + test-user-only + + + diff --git a/pkg/dependency/parser/java/pom/testdata/settings/user/.m2/settings.xml b/pkg/dependency/parser/java/pom/testdata/settings/user/.m2/settings.xml new file mode 100755 index 000000000000..aa720b72c82d --- /dev/null +++ b/pkg/dependency/parser/java/pom/testdata/settings/user/.m2/settings.xml @@ -0,0 +1,21 @@ + + + testdata/user/repository + + + + user-server + + + server-with-credentials + test-user + test-password + + + server-with-name-only + test-user-only + + + From a3d326980feaed878607c1f4620ca80064bb161e Mon Sep 17 00:00:00 2001 From: Reny Paul Date: Wed, 21 Feb 2024 12:28:59 +0530 Subject: [PATCH 2/6] Fix linting error --- pkg/dependency/parser/java/pom/parse.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/dependency/parser/java/pom/parse.go b/pkg/dependency/parser/java/pom/parse.go index 0b14255f3ed9..690cdb6ca000 100644 --- a/pkg/dependency/parser/java/pom/parse.go +++ b/pkg/dependency/parser/java/pom/parse.go @@ -90,8 +90,9 @@ func (p *parser) Parse(r dio.ReadSeekerAt) ([]types.Library, []types.Dependency, } var remoteRepositories []string + const disabled = "false" for _, rep := range content.Repositories.Repository { - if rep.Releases.Enabled == "false" && rep.Snapshots.Enabled == "false" { + if rep.Releases.Enabled == disabled && rep.Snapshots.Enabled == disabled { continue } @@ -673,7 +674,7 @@ func fetchPOMFromRemoteRepository(repo string, paths []string) (*pom, error) { repoURL.Path = path.Join(paths...) client := &http.Client{} - req, err := http.NewRequest("GET", repoURL.String(), nil) + req, err := http.NewRequest("GET", repoURL.String(), http.NoBody) if err != nil { log.Logger.Debugf("Request failed for %s/%s", repoURL.Host, repoURL.Path) return nil, nil From 651fcffd562eea5179d37ce8ad27c0df7f73aee2 Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Wed, 21 Feb 2024 15:27:11 +0600 Subject: [PATCH 3/6] refactor(pom): check servers for repositories when parsing pom --- pkg/dependency/parser/java/pom/parse.go | 33 ++----------- pkg/dependency/parser/java/pom/pom.go | 63 +++++++++++++++++-------- 2 files changed, 48 insertions(+), 48 deletions(-) diff --git a/pkg/dependency/parser/java/pom/parse.go b/pkg/dependency/parser/java/pom/parse.go index 690cdb6ca000..ab62d1404869 100644 --- a/pkg/dependency/parser/java/pom/parse.go +++ b/pkg/dependency/parser/java/pom/parse.go @@ -89,38 +89,15 @@ func (p *parser) Parse(r dio.ReadSeekerAt) ([]types.Library, []types.Dependency, return nil, nil, xerrors.Errorf("failed to parse POM: %w", err) } - var remoteRepositories []string - const disabled = "false" - for _, rep := range content.Repositories.Repository { - if rep.Releases.Enabled == disabled && rep.Snapshots.Enabled == disabled { - continue - } - - repoURL, err := url.Parse(rep.URL) - if err != nil { - log.Logger.Debugf("Unable to parse remote repository url: %s", err) - continue - } - - for _, server := range p.servers { - if rep.ID == server.ID && server.Username != "" && server.Password != "" { - repoURL.User = url.UserPassword(server.Username, server.Password) - break - } - } - - log.Logger.Debugf("Adding repository %s: %s", rep.ID, rep.URL) - remoteRepositories = append(remoteRepositories, repoURL.String()) - } - - // Add central maven repository or repositories obtained using `WithRemoteRepos` function. - p.remoteRepositories = append(remoteRepositories, p.remoteRepositories...) - root := &pom{ filePath: p.rootPath, content: content, } + remoteRepositories := root.repositories(p.servers) + // Add central maven repository or repositories obtained using `WithRemoteRepos` function. + p.remoteRepositories = append(remoteRepositories, p.remoteRepositories...) + // Analyze root POM result, err := p.analyze(root, analysisOptions{lineNumber: true}) if err != nil { @@ -341,7 +318,7 @@ func (p *parser) analyze(pom *pom, opts analysisOptions) (analysisResult, error) } // Update remoteRepositories - p.remoteRepositories = utils.UniqueStrings(append(p.remoteRepositories, pom.repositories()...)) + p.remoteRepositories = utils.UniqueStrings(append(p.remoteRepositories, pom.repositories(p.servers)...)) // Parent parent, err := p.parseParent(pom.filePath, pom.content.Parent) diff --git a/pkg/dependency/parser/java/pom/pom.go b/pkg/dependency/parser/java/pom/pom.go index 7fd7204cbac3..52934ee67bee 100644 --- a/pkg/dependency/parser/java/pom/pom.go +++ b/pkg/dependency/parser/java/pom/pom.go @@ -5,12 +5,14 @@ import ( "fmt" "io" "maps" + "net/url" "reflect" "strings" "github.com/samber/lo" "golang.org/x/xerrors" + "github.com/aquasecurity/trivy/pkg/dependency/parser/log" "github.com/aquasecurity/trivy/pkg/dependency/parser/types" "github.com/aquasecurity/trivy/pkg/dependency/parser/utils" ) @@ -113,12 +115,29 @@ func (p pom) licenses() []string { }) } -func (p pom) repositories() []string { +func (p pom) repositories(servers []Server) []string { var urls []string for _, rep := range p.content.Repositories.Repository { - if rep.Releases.Enabled != "false" { - urls = append(urls, rep.URL) + // Add only enabled repositories + if rep.Releases.Enabled == "false" && rep.Snapshots.Enabled == "false" { + continue + } + + repoURL, err := url.Parse(rep.URL) + if err != nil { + log.Logger.Debugf("Unable to parse remote repository url: %s", err) + continue + } + + for _, server := range servers { + if rep.ID == server.ID && server.Username != "" && server.Password != "" { + repoURL.User = url.UserPassword(server.Username, server.Password) + break + } } + + log.Logger.Debugf("Adding repository %s: %s", rep.ID, rep.URL) + urls = append(urls, repoURL.String()) } return urls } @@ -139,23 +158,7 @@ type pomXML struct { Dependencies pomDependencies `xml:"dependencies"` } `xml:"dependencyManagement"` Dependencies pomDependencies `xml:"dependencies"` - Repositories struct { - Text string `xml:",chardata"` - Repository []struct { - Text string `xml:",chardata"` - ID string `xml:"id"` - Name string `xml:"name"` - URL string `xml:"url"` - Releases struct { - Text string `xml:",chardata"` - Enabled string `xml:"enabled"` - } `xml:"releases"` - Snapshots struct { - Text string `xml:",chardata"` - Enabled string `xml:"enabled"` - } `xml:"snapshots"` - } `xml:"repository"` - } `xml:"repositories"` + Repositories pomRepositories `xml:"repositories"` } type pomParent struct { @@ -350,3 +353,23 @@ func findDep(name string, depManagement []pomDependency) (pomDependency, bool) { return item.Name() == name }) } + +type pomRepositories struct { + Text string `xml:",chardata"` + Repository []pomRepository `xml:"repository"` +} + +type pomRepository struct { + Text string `xml:",chardata"` + ID string `xml:"id"` + Name string `xml:"name"` + URL string `xml:"url"` + Releases struct { + Text string `xml:",chardata"` + Enabled string `xml:"enabled"` + } `xml:"releases"` + Snapshots struct { + Text string `xml:",chardata"` + Enabled string `xml:"enabled"` + } `xml:"snapshots"` +} From e081fcbf6b1a801f5d1e75f6bcf339dccc16d6c7 Mon Sep 17 00:00:00 2001 From: Reny Paul Date: Wed, 21 Feb 2024 15:23:00 +0530 Subject: [PATCH 4/6] Removed duplicate code --- pkg/dependency/parser/java/pom/parse.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/dependency/parser/java/pom/parse.go b/pkg/dependency/parser/java/pom/parse.go index ab62d1404869..acc2e261ca06 100644 --- a/pkg/dependency/parser/java/pom/parse.go +++ b/pkg/dependency/parser/java/pom/parse.go @@ -94,10 +94,6 @@ func (p *parser) Parse(r dio.ReadSeekerAt) ([]types.Library, []types.Dependency, content: content, } - remoteRepositories := root.repositories(p.servers) - // Add central maven repository or repositories obtained using `WithRemoteRepos` function. - p.remoteRepositories = append(remoteRepositories, p.remoteRepositories...) - // Analyze root POM result, err := p.analyze(root, analysisOptions{lineNumber: true}) if err != nil { From f9c31dc3f3de97d788a9bbf5ef542397f9df17d4 Mon Sep 17 00:00:00 2001 From: Reny Paul Date: Wed, 21 Feb 2024 19:18:09 +0530 Subject: [PATCH 5/6] Refinments - Added comments where there credentials are fetched - Updated the order of the repo to reduce the number of tries to fetch - Avoid fetching pom file without version, since it will anyways fail in the tries --- pkg/dependency/parser/java/pom/parse.go | 10 +++++++--- pkg/dependency/parser/java/pom/pom.go | 2 ++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/dependency/parser/java/pom/parse.go b/pkg/dependency/parser/java/pom/parse.go index acc2e261ca06..62a3fbf32221 100644 --- a/pkg/dependency/parser/java/pom/parse.go +++ b/pkg/dependency/parser/java/pom/parse.go @@ -314,7 +314,7 @@ func (p *parser) analyze(pom *pom, opts analysisOptions) (analysisResult, error) } // Update remoteRepositories - p.remoteRepositories = utils.UniqueStrings(append(p.remoteRepositories, pom.repositories(p.servers)...)) + p.remoteRepositories = utils.UniqueStrings(append(pom.repositories(p.servers), p.remoteRepositories...)) // Parent parent, err := p.parseParent(pom.filePath, pom.content.Parent) @@ -588,6 +588,10 @@ func (p *parser) openPom(filePath string) (*pom, error) { }, nil } func (p *parser) tryRepository(groupID, artifactID, version string) (*pom, error) { + if len(version) == 0 { + return nil, xerrors.Errorf("Version missing for %s:%s", groupID, artifactID) + } + // Generate a proper path to the pom.xml // e.g. com.fasterxml.jackson.core, jackson-annotations, 2.10.0 // => com/fasterxml/jackson/core/jackson-annotations/2.10.0/jackson-annotations-2.10.0.pom @@ -649,7 +653,7 @@ func fetchPOMFromRemoteRepository(repo string, paths []string) (*pom, error) { client := &http.Client{} req, err := http.NewRequest("GET", repoURL.String(), http.NoBody) if err != nil { - log.Logger.Debugf("Request failed for %s/%s", repoURL.Host, repoURL.Path) + log.Logger.Debugf("Request failed for %s%s", repoURL.Host, repoURL.Path) return nil, nil } if repoURL.User != nil { @@ -659,7 +663,7 @@ func fetchPOMFromRemoteRepository(repo string, paths []string) (*pom, error) { resp, err := client.Do(req) if err != nil || resp.StatusCode != http.StatusOK { - log.Logger.Debugf("Failed to fetch from %s/%s", repoURL.Host, repoURL.Path) + log.Logger.Debugf("Failed to fetch from %s%s", repoURL.Host, repoURL.Path) return nil, nil } defer resp.Body.Close() diff --git a/pkg/dependency/parser/java/pom/pom.go b/pkg/dependency/parser/java/pom/pom.go index 52934ee67bee..e041cb10fddd 100644 --- a/pkg/dependency/parser/java/pom/pom.go +++ b/pkg/dependency/parser/java/pom/pom.go @@ -129,6 +129,8 @@ func (p pom) repositories(servers []Server) []string { continue } + // Get the credentials from settings.xml based on matching server id + // with the repository id from pom.xml and use it for accessing the repository url for _, server := range servers { if rep.ID == server.ID && server.Username != "" && server.Password != "" { repoURL.User = url.UserPassword(server.Username, server.Password) From 77df461197bb026a735e81ba525306b89af623d1 Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Thu, 22 Feb 2024 12:29:23 +0600 Subject: [PATCH 6/6] fix linter error --- pkg/dependency/parser/java/pom/parse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/dependency/parser/java/pom/parse.go b/pkg/dependency/parser/java/pom/parse.go index 62a3fbf32221..ec935b37988c 100644 --- a/pkg/dependency/parser/java/pom/parse.go +++ b/pkg/dependency/parser/java/pom/parse.go @@ -588,7 +588,7 @@ func (p *parser) openPom(filePath string) (*pom, error) { }, nil } func (p *parser) tryRepository(groupID, artifactID, version string) (*pom, error) { - if len(version) == 0 { + if version == "" { return nil, xerrors.Errorf("Version missing for %s:%s", groupID, artifactID) }