Open mhalbritter opened 1 month ago
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.
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.
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
.
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.
Until this is fixed, we can't add checksums to the properties file.
In the meantime, I reverted the SHA256 feature in 9e2682402a36f1c60196de20ffcb3b57daaf4e54.
I've opened https://github.com/apache/maven-wrapper/pull/158 which fixes the CLRF bug on Maven Wrapper side. With this change, we can use -Dtype=source
with wrapperSha256Sum
and distributionSha256Sum
.
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 runningmvnw
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 https://github.com/spring-io/initializr/issues/1577#issuecomment-2427879125