Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: exclude known instrumentation jars from being erroneously identified #2796

Merged
merged 2 commits into from
Apr 22, 2024
+39 −2
Merged

fix: exclude known instrumentation jars from being erroneously identified #2796

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3b20eb
fix: exclude known instrumentation jars from being erroneously identi…
kzantow Apr 19, 2024
f7bd031
chore: update comment to hopefully be more clear
kzantow Apr 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions syft/pkg/cataloger/java/archive_parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,13 @@ func (j *archiveParser) discoverMainPackage(ctx context.Context) (*pkg.Package,
return nil, nil
}

// check for existence of Weave-Classes manifest key in order to exclude jars getting misrepresented as
// their targeted counterparts, e.g. newrelic spring and tomcat instrumentation
if _, ok := manifest.Main.Get("Weave-Classes"); ok {
log.Debugf("excluding archive due to Weave-Classes manifest entry: %s", j.location)
return nil, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to skip matching on this jar entirely, or is there a similar heuristic where we could correctly find the NewRelic jar?

Copy link
Contributor Author

@kzantow kzantow Apr 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are finding the newrelic jar, these are subcomponents of it. E.g. this PR reports:

$go run ./cmd/syft ~/Downloads/newrelic-java-8.9.1.zip 
NAME                       VERSION  TYPE                         
agent-bridge               8.9.1    java-archive                  
agent-bridge-datastore     8.9.1    java-archive                  
newrelic                   8.9.1    java-archive                  
newrelic-api               8.9.1    java-archive  (+1 duplicate)  
newrelic-api-javadoc                java-archive                  
newrelic-api-sources                java-archive                  
newrelic-security-agent    1.1.0    java-archive                  
newrelic-security-api      1.1.0    java-archive                  
newrelic-weaver-api        8.9.1    java-archive                  
newrelic-weaver-scala-api  8.9.1    java-archive

I don't think it's especially valuable to try to identify these separately.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see.

I misunderstood the comment at https://github.com/anchore/syft/pull/2796/files#diff-0965089b69371402427667e60ad9d7ad5698ae0b330b5c7430e2b0c671655fe5R1356 // we expect no packages to be discovered from this - I thought that meant we wouldn't find the New Relic jar at all.

What is meant by that comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is for a manifest with the Weave-Classes attribute being excluded. I updated the comment to hopefully be more clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should note I can't seem to find any vulnerabilities reported against the Java agent or related components anywhere, though. But it seems that the vulnerabilities reported are not against individual components but rather the platform: https://nvd.nist.gov/products/cpe/search/results?namingFormat=2.3&keyword=newrelic

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there are currently no known public vulnerabilities filed against the Newrelic java agent components

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and there are currently no separate maven components published for the instrumentation jars, so when there are vulns in future they would likely be placed against one or multiple of the published maven jars which we are still capturing (though probably the groupie and artifact I'd may be incorrect, but that is a separate issue)

}

// grab and assign digest for the entire archive
digests, err := getDigestsFromArchive(j.archivePath)
if err != nil {
Expand Down
6 changes: 5 additions & 1 deletion syft/pkg/cataloger/java/archive_parser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1168,7 +1168,6 @@ func Test_parseJavaArchive_regressions(t *testing.T) {
expectedPkgs []pkg.Package
expectedRelationships []artifact.Relationship
assignParent bool
want bool
}{
{
name: "duplicate jar regression - go case (issue #2130)",
Expand Down Expand Up @@ -1351,6 +1350,11 @@ func Test_parseJavaArchive_regressions(t *testing.T) {
},
},
},
{
name: "exclude instrumentation jars with Weave-Classes in manifest",
fixtureName: "spring-instrumentation-4.3.0-1.0",
expectedPkgs: nil, // we expect no packages to be discovered from this
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
6 changes: 5 additions & 1 deletion syft/pkg/cataloger/java/test-fixtures/jar-metadata/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ JACKSON_CORE = jackson-core-2.15.2
SBT_JACKSON_CORE = com.fasterxml.jackson.core.jackson-core-2.15.2
OPENSAML_CORE = opensaml-core-3.4.6
API_ALL_SOURCES = api-all-2.0.0-sources
SPRING_INSTRUMENTATION = spring-instrumentation-4.3.0-1.0

$(CACHE_DIR):
mkdir -p $(CACHE_DIR)
Expand All @@ -19,4 +20,7 @@ $(CACHE_DIR)/$(OPENSAML_CORE).jar: $(CACHE_DIR)
cd $(OPENSAML_CORE) && zip -r $(CACHE_PATH)/$(OPENSAML_CORE).jar .

$(CACHE_DIR)/$(API_ALL_SOURCES).jar: $(CACHE_DIR)
cd $(API_ALL_SOURCES) && zip -r $(CACHE_PATH)/$(API_ALL_SOURCES).jar .
cd $(API_ALL_SOURCES) && zip -r $(CACHE_PATH)/$(API_ALL_SOURCES).jar .

$(CACHE_DIR)/$(SPRING_INSTRUMENTATION).jar: $(CACHE_DIR)
cd $(SPRING_INSTRUMENTATION) && zip -r $(CACHE_PATH)/$(SPRING_INSTRUMENTATION).jar .
22 changes: 22 additions & 0 deletions ...ger/java/test-fixtures/jar-metadata/spring-instrumentation-4.3.0-1.0/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
Manifest-Version: 1.0
Implementation-Title: com.newrelic.instrumentation.spring-4.3.0
Implementation-Version: 1.0
Illegal-Classes:
Weave-Violation-Filter: METHOD_MISSING_REQUIRED_ANNOTATIONS,CLASS_MISS
ING_REQUIRED_ANNOTATIONS
Reference-Classes: org/springframework/core/annotation/AnnotationUtils
,org/springframework/web/bind/annotation/DeleteMapping,org/springfram
ework/web/bind/annotation/PatchMapping,org/springframework/web/bind/a
nnotation/PostMapping,org/springframework/web/bind/annotation/PutMapp
ing,org/springframework/web/bind/annotation/RequestMapping,org/spring
framework/web/method/HandlerMethod,org/springframework/web/servlet/Mo
delAndView
Class-Required-Annotations:
Method-Required-Annotations:
Implementation-Title-Alias: spring_annotations
Weave-Classes: org/springframework/web/bind/annotation/GetMapping,org/
springframework/web/servlet/mvc/method/AbstractHandlerMethodAdapter
Weave-Methods: "handleInternal(Ljavax/servlet/http/HttpServletRequest;
Ljavax/servlet/http/HttpServletResponse;Lorg/springframework/web/meth
od/HandlerMethod;)Lorg/springframework/web/servlet/ModelAndView;"
Implementation-Vendor: New Relic
Loading