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

Jump to bottom

bump(ftp): commons-net 3.11.1 (was 3.8.0) #2945

Merged
merged 6 commits into from
Oct 2, 2024
Merged

bump(ftp): commons-net 3.11.1 (was 3.8.0) #2945

merged 6 commits into from
Oct 2, 2024

Conversation

ennru
Copy link
Member

@ennru ennru commented Dec 13, 2022 •
edited
Loading

CVE-2021-37533: Apache Commons Net's FTP client trusts the host from PASV response by default

Tests in our FtpsWithProxyStageTest started failing which makes apache/commons-net#90 look suspicious. We had to disable the test.

Refs

@ennru ennru added p:ftp dependency-change For PRs changing the version of a dependency. labels Dec 13, 2022
efgpinto
efgpinto approved these changes Dec 13, 2022
View reviewed changes
Copy link
Contributor

@efgpinto efgpinto left a comment

Choose a reason for hiding this comment

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

LGTM

@johanandren
Copy link
Member

One of the failing tests was ftps, let's run again and see if it was a random failure

@ennru
Copy link
Member Author

ennru commented Dec 13, 2022

Doesn't look too good javax.net.ssl.SSLException: Unsupported or unrecognized SSL message.

@ennru
Copy link
Member Author

ennru commented Dec 13, 2022

This error is not directly connected to the change of default in 3.9.0, it fails locally with

Start of log messages of test that failed with assertion failed: fishForMessage(OnNext(_) or OnComplete) found unexpected message OnError(javax.net.ssl.SSLException: Unsupported or unrecognized SSL message
	at java.base/sun.security.ssl.SSLSocketInputRecord.handleUnknownRecord(SSLSocketInputRecord.java:451)
	at java.base/sun.security.ssl.SSLSocketInputRecord.decode(SSLSocketInputRecord.java:175)
	at java.base/sun.security.ssl.SSLTransport.decode(SSLTransport.java:111)
	at java.base/sun.security.ssl.SSLSocketImpl.decode(SSLSocketImpl.java:1505)
	at java.base/sun.security.ssl.SSLSocketImpl.readHandshakeRecord(SSLSocketImpl.java:1420)
	at java.base/sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:455)
	at java.base/sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:426)
	at org.apache.commons.net.ftp.FTPSClient._openDataConnection_(FTPSClient.java:278)
	at org.apache.commons.net.ftp.FTPClient._retrieveFileStream(FTPClient.java:915)
	at org.apache.commons.net.ftp.FTPClient.retrieveFileStream(FTPClient.java:2841)
	at akka.stream.alpakka.ftp.impl.CommonFtpOperations.$anonfun$retrieveFileInputStream$1(CommonFtpOperations.scala:71)
	at scala.util.Try$.apply(Try.scala:210)```

@johanandren
Copy link
Member

As in, it was already failing?

@ennru
Copy link
Member Author

ennru commented Dec 13, 2022

As in, it was already failing?

No, as in fails only after the upgrade, but switching back the default the CVE reported doesn't help.

@ennru
Copy link
Member Author

ennru commented Dec 14, 2022

There were not many changes in commons-net between 3.8.0 and 3.9.0:
https://issues.apache.org/jira/browse/NET-642?jql=project%20%3D%20NET%20AND%20fixVersion%20%3D%203.9.0

Tests in our FtpsWithProxyStageTest fail which makes apache/commons-net#90 look suspicious.

@sebastian-alfers
Copy link
Contributor

When trying to reproduce locally, I do get another error:

--> [akka.stream.alpakka.ftp.FtpsWithProxyStageTest: listFiles] Start of log messages of test that failed with assertion failed: expected class akka.stream.testkit.TestSubscriber$OnNext, found class akka.stream.testkit.TestSubscriber$OnError (OnError(java.net.ConnectException: Connection refused (Connection refused)
	at java.base/java.net.PlainSocketImpl.socketConnect(Native Method)

It is the same on master so I assume the setup is broken on my end. @johanandren are you able to reproduce the error from this build? Locally, it is just:

./scripts/ftp-servers.sh

and then

sbt "ftp/testOnly *.FtpsWithProxyStageTest"

@johanandren
Copy link
Member

Waiting for upstream issue https://issues.apache.org/jira/browse/NET-718

@ennru ennru force-pushed the ennru-ftp-bumps branch from f91d73a to a51b0dd Compare March 13, 2024 08:20
@ennru ennru changed the title bump(ftp): commons-net 3.9.0 (was 3.8.0), sshj 0.34.0 (was 0.33.0) bump(ftp): commons-net 3.10.0 (was 3.8.0), sshj 0.38.0 (was 0.33.0) Mar 13, 2024
@ennru
Copy link
Member Author

ennru commented Mar 13, 2024

Bumped commons-net to 3.10.0 but according to their issue tracker the problem is not fixed.

@ennru ennru mentioned this pull request Mar 28, 2024
Closed
@ennru ennru marked this pull request as draft April 5, 2024 18:03
@ennru
Copy link
Member Author

ennru commented Apr 5, 2024

When getting back to this, notice even

@ennru ennru mentioned this pull request Sep 27, 2024
Closed
@ennru
Copy link
Member Author

ennru commented Sep 30, 2024

sshj already bumped with

@sebastian-alfers sebastian-alfers changed the title bump(ftp): commons-net 3.10.0 (was 3.8.0), sshj 0.38.0 (was 0.33.0) bump(ftp): commons-net 3.10.0 (was 3.8.0) Oct 1, 2024
ennru
ennru commented Oct 1, 2024
View reviewed changes
project/Dependencies.scala Outdated Show resolved Hide resolved
@ennru ennru changed the title bump(ftp): commons-net 3.10.0 (was 3.8.0) bump(ftp): commons-net 3.11.1 (was 3.8.0) Oct 1, 2024
@ennru ennru marked this pull request as ready for review October 1, 2024 15:15
johanandren
johanandren approved these changes Oct 2, 2024
View reviewed changes
Copy link
Member

@johanandren johanandren left a comment

Choose a reason for hiding this comment

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

LGTM

ftp/src/test/java/akka/stream/alpakka/ftp/FtpsWithProxyStageTest.java
@@ -21,7 +21,7 @@
import java.util.concurrent.CompletionStage;
import java.util.function.Function;

public class FtpsWithProxyStageTest extends BaseFtpSupport implements CommonFtpStageTest {
@Ignore public class FtpsWithProxyStageTest extends BaseFtpSupport implements CommonFtpStageTest {
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to add a comment explaining why it is ignored

@ennru ennru merged commit 7697465 into akka:main Oct 2, 2024
47 of 48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johanandren johanandren johanandren approved these changes

@efgpinto efgpinto efgpinto approved these changes

Assignees
No one assigned
Labels
dependency-change For PRs changing the version of a dependency. p:ftp
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ennru @johanandren @sebastian-alfers @efgpinto