Skip to content

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

Merged
ennru merged 6 commits into
akka:mainfrom
ennru:ennru-ftp-bumps
Oct 2, 2024
Merged

bump(ftp): commons-net 3.11.1 (was 3.8.0)#2945
ennru merged 6 commits into
akka:mainfrom
ennru:ennru-ftp-bumps

Conversation

@ennru

@ennru ennru commented Dec 13, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@efgpinto efgpinto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@johanandren

johanandren commented Dec 13, 2022

Copy link
Copy Markdown
Contributor

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

@ennru

ennru commented Dec 13, 2022

Copy link
Copy Markdown
Contributor Author

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

@ennru

ennru commented Dec 13, 2022

Copy link
Copy Markdown
Contributor Author

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

johanandren commented Dec 13, 2022

Copy link
Copy Markdown
Contributor

As in, it was already failing?

@ennru

ennru commented Dec 13, 2022

Copy link
Copy Markdown
Contributor Author

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

ennru commented Dec 14, 2022

Copy link
Copy Markdown
Contributor Author

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

sebastian-alfers commented Mar 1, 2023

Copy link
Copy Markdown
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

johanandren commented May 8, 2023

Copy link
Copy Markdown
Contributor

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

ennru commented Mar 13, 2024

Copy link
Copy Markdown
Contributor Author

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

ennru commented Apr 5, 2024

Copy link
Copy Markdown
Contributor Author

When getting back to this, notice even

@ennru ennru mentioned this pull request Sep 27, 2024
Closed
@ennru

ennru commented Sep 30, 2024

Copy link
Copy Markdown
Contributor Author

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
Comment thread project/Dependencies.scala Outdated
@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

@johanandren johanandren left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread ftp/src/test/java/akka/stream/alpakka/ftp/FtpsWithProxyStageTest.java
import java.util.function.Function;

public class FtpsWithProxyStageTest extends BaseFtpSupport implements CommonFtpStageTest {
@Ignore public class FtpsWithProxyStageTest extends BaseFtpSupport implements CommonFtpStageTest {

@johanandren johanandren Oct 2, 2024

Copy link
Copy Markdown
Contributor

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
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

+1 more reviewer

@efgpinto efgpinto efgpinto approved these changes

Reviewers whose approvals may not affect merge requirements

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