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.

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

Maven Wrapper's sha256 property breaks on some systems #1578

Open
mhalbritter opened this issue Oct 22, 2024 · 8 comments
Open

Maven Wrapper's sha256 property breaks on some systems #1578

mhalbritter opened this issue Oct 22, 2024 · 8 comments

Comments

@mhalbritter
Copy link
Contributor

Actually, I think there may be a problem with this. It looks like the current Maven Wrapper script that is generated for Maven 3.9.9, includes some logic starting at line 175 that will switch the downloaded file type from .zip to .tar.gz if the unzip command isn't found in the environment you are running mvnw on. (https://github.com/apache/maven-wrapper/blob/maven-wrapper-3.3.2/maven-wrapper-distribution/src/resources/only-mvnw#L175-L179)

This means that the included SHA256 might be wrong when the download URL for the binaries for Maven gets switched to .tar.gz.

It ultimately seems like this might be a deficiency in the Maven wrapper, since it doesn't seem to me that you can specify SHA256 sums for both of the possible downloads that could occur (one for .tar.gz and one for .zip).

Originally posted by @cdelashmutt-pivotal in #1577 (comment)

@mhalbritter mhalbritter self-assigned this Oct 22, 2024
@mhalbritter
Copy link
Contributor Author

mhalbritter commented Oct 22, 2024

Here's the issue on the Maven bugtracker.

There are other issues reported for the sha256 feature (like this one or this one) which makes me think that this feature isn't ready for production yet.

@mhalbritter
Copy link
Contributor Author

mhalbritter commented Oct 22, 2024

We're currently using only-script mode for the wrapper. That means it doesn't download the maven-wrapper.jar, but Maven directly. If we'd use bin or source or script, it would use the maven-wrapper.jar. Source code for it is here. This also supports SHA256 checksum verification and always uses the Zip file (and not falling back to the .tar.gz).

With mode bin, the maven-wrapper.jar is added directly in the .mvn/wrapper folder.

With mode source, we get a MavenWrapperDownloader.java. This is used if wget or curl is not available. It gets compiled and it's job is to download the maven-wrapper.jar.

With mode script, there's no JAR or java file, and it uses wget or curl to download the maven-wrapper.jar.

With mode script-only, there's no maven-wrapper.jar involved at all and the download of Maven is done directly by the script with wget or curl. And depending on the presence of unzip, it either downloads the zip for maven or the tar.gz.

@mhalbritter
Copy link
Contributor Author

mhalbritter commented Oct 22, 2024

I think we have those options:

  • drop the whole checksum dance all together and use type=script-only. Con of this approach: If someone manages to MITM the download (there's HTTPS involved, so that may not be that easy), a malicious Maven distribution is now on the machine of the user. However, Maven wrapper defaults to type=script-only, so we'd only do what mvn wrapper:wrapper would do by default.

  • use type=bin and add distributionSha256Sum to the properties. This way, maven-wrapper.jar is directly in the repo (and therefore trusted) and the downloaded Maven is checksummed. Con of this approach is that the users have to trust us that the maven-wrapper.jar file is really the one created by type=bin and we didn't put malicious code in it.

  • use type=script and add wrapperSha256Sum and distributionSha256Sum. The downloaded wrapper is checksummed by wrapperSha256Sum and the downloaded Maven is checksummed by distributionSha256Sum. Con of this approach is that it only works if the user has curl or wget on their machine

  • use type=source and add wrapperSha256Sum and distributionSha256Sum. The downloaded wrapper is checksummed by wrapperSha256Sum and the downloaded Maven is checksummed by distributionSha256Sum. Con of this approach is the additional MavenWrapperDownloader.java.

@mhalbritter
Copy link
Contributor Author

mhalbritter commented Oct 22, 2024

This bug prevents proper usage of the wrapperSha256Sum in the properties file. If the properties file has CRLF line endings, this error appears:

Error: Failed to validate Maven wrapper SHA-256, your Maven wrapper might be compromised.
Investigate or delete /home/mhalbritter/tmp/maven-wrapper-test/.mvn/wrapper/maven-wrapper.jar to attempt a clean download.
If you updated your Maven version, you need to update the specified wrapperSha256Sum property.

When using LF line endings, everything works.

@mhalbritter
Copy link
Contributor Author

Until this is fixed, we can't add checksums to the properties file.

@mhalbritter
Copy link
Contributor Author

In the meantime, I reverted the SHA256 feature in 9e26824.

@mhalbritter
Copy link
Contributor Author

I've opened apache/maven-wrapper#158 which fixes the CLRF bug on Maven Wrapper side. With this change, we can use -Dtype=source with wrapperSha256Sum and distributionSha256Sum.

@mhalbritter
Copy link
Contributor Author

apache/maven-wrapper#158 has been merged, now we need to wait for a new maven wrapper release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant