Harden Maven registry trust for pom.xml sourced entries#1970
Open
TristanInSec wants to merge 1 commit intogoogle:mainfrom
Open
Harden Maven registry trust for pom.xml sourced entries#1970TristanInSec wants to merge 1 commit intogoogle:mainfrom
TristanInSec wants to merge 1 commit intogoogle:mainfrom
Conversation
Registries discovered while scanning third party pom.xml files are passed into MavenRegistryAPIClient.AddRegistry as untrusted input, since any repository URL declared inside a scanned project is under the control of whoever authored that project. Two tightenings: - AddRegistry now validates the URL: it must use http or https and resolve exclusively to public unicast addresses. Loopback, private, link local, unspecified and multicast destinations are rejected so a scanned pom.xml cannot steer requests at the local host, cloud metadata services or internal infrastructure. The default registry configured through NewMavenRegistryAPIClient is exempt, so users can still point the client at an internal Artifactory or Nexus. - MavenRegistry now carries a TrustedForAuth flag. It is only set by the default registry path, and AddRegistry forces it off for any registry it accepts. Requests against untrusted registries go out with a nil HTTPAuthentication, so a scanned pom.xml can no longer reuse a known settings.xml server ID to exfiltrate the associated credentials to an attacker chosen URL. The three fetch helpers (getProject, getVersionMetadata, getArtifactMetadata) now route through a small authFor helper that centralises this policy. Unit tests cover scheme rejection, public address enforcement, DNS rebind style cases, default registry overwrite attempts, and the credential gating behaviour. Issue: google#1877
Collaborator
|
It would be good if you could contrast the differences in your PR with #1931, as I believe that is attempting to address the same issue in a similar but not identical way |
Author
|
Quick contrast: #1931 narrowly targets the credential exfiltration angle. It rejects non- #1970 treats every registry coming out of a scanned pom.xml as untrusted input regardless of whether credentials happen to be configured for its ID, and splits the hardening into two layers:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follows up on #1877 and ports the hardening that was originally staged against
osv-scanner(PR google/osv-scanner#2713, closed in favour of landing the fix here). Registries discovered while scanning a user supplied pom.xml are treated as untrusted input throughoutclients/datasource.MavenRegistryAPIClient.AddRegistrynow validates the URL before recording it. The scheme must behttporhttps, and the host must resolve exclusively to public unicast addresses. Loopback, private, link local, unspecified and multicast destinations are rejected so a scanned pom.xml cannot steer requests at the local host, cloud metadata endpoints, or internal infrastructure. The default registry passed toNewMavenRegistryAPIClientis exempt, so users can still point the client at an internal Artifactory or Nexus mirror.MavenRegistrynow carries aTrustedForAuthflag. It is only set on the default registry constructed throughNewMavenRegistryAPIClient;AddRegistryalways clears it, even for callers that try to set it explicitly. The three fetch helpers (getProject,getVersionMetadata,getArtifactMetadata) now go through a smallauthForhelper that refuses to attachsettings.xmlcredentials to untrusted registries. That prevents a scanned pom.xml from reusing a knownsettings.xml<server>ID to exfiltrate the associated credentials to an attacker-chosen URL.Test plan
go test ./clients/datasource/...go test ./clients/... ./internal/mavenutil/... ./enricher/transitivedependency/pomxml/... ./guidedremediation/internal/manifest/maven/...go vet ./clients/datasource/...TrustedForAuthsmuggling via a caller supplied struct. ExistingTestMultipleRegistry,TestUpdateDefaultRegistryandTestWithoutRegistriesMaintainsAuthDatastublookupHostso their mock HTTP servers remain reachable under the new validator.