JLLeitschuh / bulk-security-pr-generator

Generate thousands of pull requests to fix widespread security vulnerabilities across GitHub.
MIT License
34 stars 14 forks source link

check whether the server supports HTTPS before making a PR #3

Open monperrus opened 4 years ago

monperrus commented 4 years ago

some servers don't support https.

if you check for this before opening a PR, you will reduce the number of annoying PRs such as:

JLLeitschuh commented 4 years ago

If your server you're depending upon doesn't support HTTPS I recommend reaching out to your server maintainer to get this resolved. You still have a security vulnerability and unfortunately in some cases, I only provide half the fix, you may need to work with your upstream supply chain to resolve the other half.

The other alternative is to vendor the libraries you depend upon directly inside of your repository.

nielsbasjes commented 4 years ago

First of all I think this is a great effort to make things better!

In some cases however it is not that easy.

Two examples I found:

So using https fails the build, using http makes the build insecure.

Which I reported here https://issues.apache.org/jira/browse/DRILL-7529

JLLeitschuh commented 4 years ago

I remember running into the issue with GoDaddy's certs at my last job when running with OpenJDK 10. Early versions of OpenJDK 10 didn't ship with the GoDaddy certs.


To further complicate the issue, there are some users that do this:

<url>http://my-custom-repo.example/repo/aaa/{custom.maven.variable}/m2</url>

For cases like that, this wouldn't have been able to check either without actually doing the maven variable resolution (not something I really wanted to attempt).

PyvesB commented 4 years ago

To further complicate the issue, there are some users that do this:

<url>http://my-custom-repo.example/repo/aaa/{custom.maven.variable}/m2</url>

For cases like that, this wouldn't have been able to check either without actually doing the maven variable resolution (not something I really wanted to attempt).

This seems like a rather obscure edge case, I've never seen it done in practice. How about simply dropping the extra check for those? I reckon it would still cover a vast majority of cases (probably >99%?).

I only provide half the fix, you may need to work with your upstream supply chain to resolve the other half.

Not quite. In a lot of cases the right answer is switch to a different Maven repository. If the check was performed beforehand, you could raise an issue instead of submitting an invalid pull request.

Regardless, thanks for raising awareness around some of these issues! 👍

PyvesB commented 4 years ago

As a side note, it may be useful contributing to Maven, for example to print out a warning when a non secure repository is being used. 😉

JLLeitschuh commented 4 years ago

I've already got an open issue with them. https://issues.apache.org/jira/browse/MNG-6673

I work for Gradle, we started doing this in Gradle 6.0 and will be dropping support for HTTP without explicit opt-in in Gradle 7.0.