[UPGRADE] Netty 4.1.110 => 4.1.118

[chibenwa]

Addresses CVE-2025-24970: Input validation for native SSL

[chibenwa]

Can somebody in the team have a look at why tests fils?

[Arsnael]

Something wrong with the s3 sdk update it seems?

https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-2638/2/#showFailuresLink

Lots of S3 tests failing on this: java.lang.RuntimeException: software.amazon.awssdk.services.s3.model.S3Exception: trailing checksum is not supported (Service: S3, Status Code: 400, Request ID: fda7aa2f0b4895168189, Extended Request ID: fda7aa2f0b4895168189)

[Arsnael]

It looks tricky... it seems trailing checksums are only supported over TLS now and not http. Likely why the tests are failing.

Trying to find a way to disable this thing for tests...

[chibenwa]

Do we have an upstream report on this?
Maybe worth opening an issue on the S3 driver?

[Arsnael]

I don't know why that's the kind of answers I got first when looking for it, seems a wrong path...

Seems for the trailing checksums aws added a header x-amz-trailer.
When looking at scality code => https://github.com/scality/cloudserver/blob/development/8.8/lib/api/apiUtils/object/validateChecksumHeaders.js#L8

They just dont want to treat it and reject directly an error if present. dont seem to want to support it either => scality/cloudserver#5553

With minio tests, it's working, I don't have the trailing checksum issue. However there is an other problem. It seems when we try to delete buckets, we have errors like this:

software.amazon.awssdk.services.s3.model.S3Exception: Missing required header for this request: Content-Md5

When I disable checksums in the aws sdk client configuration as well for scality tests I get similar answers with delete buckets. Not sure if it's an issue with the s3 sdk client... I will dig more

[chibenwa]

There's no point upgrading to an outdated and also vulnerable version of netty...

[chibenwa]

(we need this alignment between netty and s3)

[Arsnael]

Sorry it took me a while to write the reason of why. First of all, as explained below, those two versions seem to have the same version of netty, as link pasted to the code source, aka 4.1.115. So I don't see how your change is addressing the CVE on this side.

Second, upgrading to 2.30.17 (that is aligned with the correct version of netty) implies serious breaking changes, as explained below, that requires being careful and proper testing before jumping in.

[Arsnael]

Let me explain why the downgrade to 2.29.52 for the aws s3 sdk version in the fixup.

Some serious breaking changes have been introduced since 2.30.0, with trailing checksums and overall some S3 default integrity protection changes explained here: aws/aws-sdk-java-v2#5802

This implies changes server side as well for object storages to keep being S3-compatible with the s3 sdk starting from that version.

As said earlier, Minio I had an issue when deleting buckets, with missing md5-content header for the request. Similar to this issue opened on aws s3 sdk backlog: aws/aws-sdk-java-v2#5848

By upgrading locally to latest minio version, the tests were green, means it got fixed.

I can't say the same for scality though that we use mainly for tests. As commented above, they refuse the new header introduced in those new versions, and even if you disable the checksums in the client configuration and upgrade to latest image, I still have the issue with md5-content header missing for delete bucket requests.

I guess initially @chibenwa you wanted to align netty versions with s3 sdk. But as I see, 2.30.16 is using netty 4.1.115 (https://github.com/aws/aws-sdk-java-v2/blob/2.30.16/pom.xml#L116) and 2.29.52 is as well (https://github.com/aws/aws-sdk-java-v2/blob/2.29.52/pom.xml#L116).

It's from 2.30.17 that netty 4.1.118 seems being used (which is available on maven central now btw). But that introduces some massive breaking changes IMO that we should take the time to carefully consider and test. At least a refactoring to use minio on all our test instead of scality that seems determined to not make an effort for supporting this move. Also might need to upgrade production environments and test as well OVH s3 api if supported yet or not...

Thus why I would suggest this in two steps.

[vttranlina]

they refuse the new header introduced in those new versions, and even if you disable the checksums in the client configuration

just curious,
client still sending with header x-amz-trailer to s3 server even after "disable the checksums in the client" ?
looks like a sdk bug

[Arsnael]

client still sending with header x-amz-trailer to s3 server even after "disable the checksums in the client" ?

Does not, as explained (but maybe I was unclear). But I still have the following issue with delete bucket (missing Content-md5 header) on the scality side, even latest image, while on minio it's been fixed.

looks like a sdk bug

It's not. It's just aws enforcing that for their own object storage and forcing a breaking change on other object storage that are s3-compatible

[vttranlina]

Does not, as explained (but maybe I was unclear). But I still have the following issue with delete bucket (missing Content-md5 header) on the scality side, even latest image, while on minio it's been fixed.

I got it,
I tried, it throws exception when we delete list object (S3BlobStoreDAO#deleteObjects)
I used env AWS_REQUEST_CHECKSUM_CALCULATION=WHEN_REQUIRED for disable x-amz-trailer feature.
Maybe this env also make library remove Content-MD5 headers when request (not only x-amz-trailer)
And cloudserver alway strict require Content-MD5 for delete objects.

I will try to manually add Content-MD5 for delete objects request, and see how it work

[Arsnael]

I will try to manually add Content-MD5 for delete objects request, and see how it work

I thought about this, I don't think this is the right solution, just to satisfy tests with an object storage that does not seem to wish to be s3 compatible with latest aws s3 sdk versions anymore, I'm not sure it should be done for prod case IMO

EDIT: You can see that other object storages (like if you try to run minio tests with the latest minio image) did upgrade to keep being s3 compatible with latest sdk versions.

[vttranlina]

I think it is not only for satisfy tests,
We can give a option configuration for S3BlobStoreDAO (that can changed by admin)
if has any S3 service providers does not compatible with this feature in production, it can break james

[chibenwa]

They just dont want to treat it and reject directly an error if present. dont seem to want to support it either => scality/cloudserver#5553

I guess that's the poison of using S3 apis (proprietary) as a defacto standard for S3...

Maybe let's plan first changing the base S3 image to something more compliant than Scality. We then would ensure OVH compatibility then upgrade the S3 driver.

[Arsnael]

Maybe let's plan first changing the base S3 image to something more compliant than Scality. We then would ensure OVH compatibility then upgrade the S3 driver.

I'm ok with this. Shall we merge this first? Does not fully solve the issue but limits it. Then we create a ticket to tackle more carefully those S3 changes?

[quantranhong1999]

+1 for resolving the S3 SDK breaking change issue in another (maybe a dedicated Jira) ticket.

I think it is not only for satisfy tests,
We can give a option configuration for S3BlobStoreDAO (that can changed by admin)
if has any S3 service providers does not compatible with this feature in production, it can break james

I share the same concern with @vttranlina.

[chibenwa]

Shall we merge this first?

IMO let's validate OVH and Minio (updated) works then let's upgrade.

As uggly as it is, S3 is not an openstandard and what we witness is anti-concurrential practices.

The only alternative to an upgrade is to adopt another driver more third party friendly.

BTW we are not the only one to witness this, far from this.