CLDSRV-636: SSE with both internal/external KMS (Cherry-pick) #5800

BourgoisMickael · 2025-05-14T10:02:31Z

Cherry-pick from implementation for hotfix/9.2.0 updated to match latest cloudserver

CLDSRV-636: SSE with both internal & external KMS (HF 9.2.0.12) #5788

See above PR for description

(cherry picked from commit db43aab)

(cherry picked from commit 6bbe2a8)

To avoid breaking changes for clients (cherry picked from commit f762831)

(cherry picked from commit e4adb8f)

(cherry picked from commit 0429cd7)

(cherry picked from commit 1be8cd8)

Issue: S3C-9996 MPU was not accepting configuredMasterKeyId (from object or bucket) for Create and Complete MPU. But used the configuredMasterKeyId (from object or bucket) for UploadPart making it possible to have parts encrypted with a different key than the key in object metadata after completion. Resulting in potential error on GetObject later: - If bucket had configuredMasterKeyId but no masterKeyId (no AES before or aws:kms without configuredKey) it would crash on decryption. - If bucket had configuredMasterKeyId and a masterKeyId it would be used to decrypt parts potentially encrypted with a configuredMasterKeyId, decrypting gibberish - On UploadPart object level encryption could be provided (outside sdk/cli) which is bad. ___ The expected behavior fixed here is to use the provided SSE on CreateMPU and then for UploadPart and CompleteMPU use the SSE provided at CreateMPU. The MPU calls also return the SSE headers now. The CopyObject returned SSE headers are fixed as well, the configuredMasterKeyId was ignored This fix allow seamless continuation of ongoing MPU during SSE migration. ___ There is no migration to fix already existing completed MPU. (cherry picked from commit 8129376)

(cherry picked from commit 415781a)

(cherry picked from commit eb282f4)

(cherry picked from commit ea6fa9f)

(cherry picked from commit 1324204)

bert-e · 2025-05-14T10:02:35Z

Hello bourgoismickael,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request `TBA`
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2025-05-14T10:04:14Z

Request integration branches

Waiting for integration branch creation to be requested by the user.

To request integration branches, please comment on this pull request with the following command:

/create_integration_branches

Alternatively, the /approve and /create_pull_requests commands will automatically
create the integration branches.

Copilot

Pull Request Overview

This PR cherry-picks changes from the hotfix branch to update server‐side encryption (SSE) with support for both internal and external KMS. Key changes include updating the encryption header handling (using the new getKeyIdFromArn logic and kmsHideScalityArn flag), refactoring the KMS wrapper to use a factory for backend instantiation, and updating the bucket/object encryption APIs accordingly.

Reviewed Changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/functional/aws-node-sdk/test/object/encryptionHeaders.js	Update encryption header expectations using getKeyIdFromArn and config flag
package.json	Changed Arsenal dependency to track KMS-related improvements
lib/utilities/collectResponseHeaders.js	Updates to conditionally transform KMS key IDs in response headers
lib/kms/wrapper.js	Major refactoring using a factory pattern and returning masterKeyArn alongside
lib/kms/in_memory/backend.js & lib/kms/file/backend.js	Updated backend functions to consistently use key ARN formatting
lib/api/*.js	Updates in objectPut, objectGet, objectCopy, MPU and related APIs to support new SSE handling
lib/api/apiUtils/object/sseHeaders.js	New module to centralize setting of SSE headers
lib/api/apiUtils/bucket/updateEncryption.js	Updated encryption migration logic
lib/api/apiUtils/bucket/bucketEncryption.js	Adjusted to always include ARN prefix when necessary
lib/Config.js	Changes to support new KMS configuration and SSE migration options
config.json	Updated configuration flags and kmsAWS details

Comments suppressed due to low confidence (1)

lib/kms/wrapper.js:216

The callback in createBucketKey now returns both masterKeyId and masterKeyArn. Please ensure that all downstream consumers are updated to use masterKeyArn where applicable for consistency.

return cb(null, { masterKeyId, masterKeyArn });

lib/api/objectPutPart.js

lib/Config.js

anurag4DSB

approving without much review as cherry picked so code must be already approved

anurag4DSB

approving without much review as cherry picked so code must be already approved

BourgoisMickael · 2025-05-15T14:08:26Z

/create_integration_branches

bert-e · 2025-05-15T14:08:41Z

Conflict

A conflict has been raised during the creation of
integration branch w/8.8/improvement/CLDSRV-636-kms-cherry-pick with contents from improvement/CLDSRV-636-kms-cherry-pick
and development/8.8.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/8.8/improvement/CLDSRV-636-kms-cherry-pick origin/development/8.8
 $ git merge origin/improvement/CLDSRV-636-kms-cherry-pick
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/8.8/improvement/CLDSRV-636-kms-cherry-pick

The following options are set: create_integration_branches

bert-e · 2025-05-15T15:07:25Z

Conflict

A conflict has been raised during the creation of
integration branch w/9.0/improvement/CLDSRV-636-kms-cherry-pick with contents from w/8.8/improvement/CLDSRV-636-kms-cherry-pick
and development/9.0.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/9.0/improvement/CLDSRV-636-kms-cherry-pick origin/development/9.0
 $ git merge origin/w/8.8/improvement/CLDSRV-636-kms-cherry-pick
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/9.0/improvement/CLDSRV-636-kms-cherry-pick

The following options are set: create_integration_branches

bert-e · 2025-05-15T15:30:00Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge improvement/CLDSRV-636-kms-cherry-pick into
development/7.70
w/8.8/improvement/CLDSRV-636-kms-cherry-pick will be merged into development/8.8
w/9.0/improvement/CLDSRV-636-kms-cherry-pick will be merged into development/9.0
w/9.1/improvement/CLDSRV-636-kms-cherry-pick will be merged into development/9.1

The following branches will NOT be impacted:

development/7.10
development/7.4

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

The following options are set: create_integration_branches

bert-e · 2025-05-15T15:30:01Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

The following options are set: create_integration_branches

BourgoisMickael · 2025-05-15T16:35:42Z

/approve

bert-e · 2025-05-15T16:36:07Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/7.70
✔️ development/8.8
✔️ development/9.0
✔️ development/9.1

The following branches will NOT be impacted:

development/7.10
development/7.4

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve, create_integration_branches

bert-e · 2025-05-15T16:55:21Z

Queue build failed

The corresponding build for the queue failed:

Checkout the status page.
Identify the failing build and review the logs.
If no issue is found, re-run the build.
If an issue is identified, checkout the steps below to remove
the pull request from the queue for further analysis and maybe rebase/merge.

Remove the pull request from the queue

Add a /wait comment on this pull request.
Click on login on the status page.
Go into the manage page.
Find the option called Rebuild the queue and click on it.
Bert-E will loop again on all pull requests to put the valid ones
in the queue again, while skipping the one with the /wait comment.
Wait for the new queue to merge, then merge/rebase your pull request
with the latest changes to then work on a proper fix.
Once the issue is fixed, delete the /wait comment and
follow the usual process to merge the pull request.

bert-e · 2025-05-15T18:04:06Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/7.70
✔️ development/8.8
✔️ development/9.0
✔️ development/9.1

The following branches have NOT changed:

development/7.10
development/7.4

Please check the status of the associated issue CLDSRV-636.

Goodbye bourgoismickael.

BourgoisMickael added 11 commits May 14, 2025 11:26

CLDSRV-636: Speed a test up with fake timer (10s)

b24e3ac

(cherry picked from commit db43aab)

CLDSRV-636: Prefix KMS Key with an arn

f6bbd3b

(cherry picked from commit 6bbe2a8)

CLDSRV-636: Option for KMS to hide scality arn

7ed7149

To avoid breaking changes for clients (cherry picked from commit f762831)

CLDSRV-636: SSE Migration function

80cc7e8

(cherry picked from commit e4adb8f)

CLDSRV-636: Migrate SSE on object(Get|Head|Put)

90a9875

(cherry picked from commit 0429cd7)

CLDSRV-636: Handle multi KMS clients for migration

a8a48de

(cherry picked from commit 1be8cd8)

CLDSRV-636: Allow previous KMS for MPU parts

70fead7

(cherry picked from commit 415781a)

CLDSRV-636: Migrate SSE on CopyObject and init MPU

51e068b

(cherry picked from commit eb282f4)

CLDSRV-636: Fix unit tests with KMS arn prefix

ae7398b

(cherry picked from commit ea6fa9f)

CLDSRV-636: Fix functional tests encryption

570103a

(cherry picked from commit 1324204)

scality deleted a comment from bert-e May 14, 2025

BourgoisMickael requested a review from Copilot May 14, 2025 10:04

Copilot AI reviewed May 14, 2025

View reviewed changes

lib/api/objectPutPart.js Show resolved Hide resolved

lib/Config.js Show resolved Hide resolved

anurag4DSB reviewed May 14, 2025

View reviewed changes

anurag4DSB approved these changes May 14, 2025

View reviewed changes

BourgoisMickael requested review from jonathan-gramain, tcarmet, tmacro, dvasilas and a team May 14, 2025 17:00

BourgoisMickael added 2 commits May 15, 2025 16:07

CLDSRV-636: Bump arsenal

320f182

CLDSRV-636: Bump version

6f8b630

BourgoisMickael force-pushed the improvement/CLDSRV-636-kms-cherry-pick branch from a9dce59 to 6f8b630 Compare May 15, 2025 14:07

BourgoisMickael requested review from williamlardier and fredmnl May 15, 2025 16:20

tcarmet approved these changes May 15, 2025

View reviewed changes

bert-e merged commit f9f27ce into development/7.70 May 15, 2025
11 checks passed

bert-e deleted the improvement/CLDSRV-636-kms-cherry-pick branch May 15, 2025 18:04

CLDSRV-636: SSE with both internal/external KMS (Cherry-pick) #5800

CLDSRV-636: SSE with both internal/external KMS (Cherry-pick) #5800

Uh oh!

Conversation

BourgoisMickael commented May 14, 2025

Uh oh!

bert-e commented May 14, 2025

Hello bourgoismickael,

Uh oh!

bert-e commented May 14, 2025

Request integration branches

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

anurag4DSB left a comment

Choose a reason for hiding this comment

Uh oh!

anurag4DSB left a comment

Choose a reason for hiding this comment

Uh oh!

BourgoisMickael commented May 15, 2025

Uh oh!

bert-e commented May 15, 2025

Conflict

Uh oh!

bert-e commented May 15, 2025

Conflict

Uh oh!

bert-e commented May 15, 2025

Integration data created

Uh oh!

bert-e commented May 15, 2025

Waiting for approval

Uh oh!

BourgoisMickael commented May 15, 2025

Uh oh!

bert-e commented May 15, 2025

In the queue

Uh oh!

bert-e commented May 15, 2025

Queue build failed

Uh oh!

bert-e commented May 15, 2025

Uh oh!

Uh oh!

Uh oh!