[S3] Implement S3 native Server-Side encryption: SSE-KMS

[tsdicloud]

My name is Bernd Rederlechner from T-Systems, and I am just launching a larger project with NextCloud. This is my first pull-request, so please be patient with me. Nonetheless, this patch is on our critical path and already disussed with NextCloud GmbH as critical patch to integrate with priority into product. In case of doubts, just contact @schiessle.

Functionality:

Amazon S3 supports 2 different kinds of in-platform, server-side encryption (SSE) for securely-managed keys

SSE-KMS per file:

SSE-KMS is enabled by configuring an KMS-keyid and sending it in additional headers to S3.
The new configuration looks like this:

'objectstore' => [
        'class' => '\\OC\\Files\\ObjectStore\\S3',
        'arguments' => [
                'bucket' => 'nextcloud',
                'autocreate' => true,
                'key'    => 'EJ39ITYZEUH5BGWDRUFY',
                'secret' => 'M5MrXTRjkyMaxXPe2FRXMTfTfbKEnZCu+7uRTVSj',
                'hostname' => 'example.com',
                'port' => 1234,
                'use_ssl' => true,
                'region' => 'optional',
                // required for some non Amazon S3 implementations
                'use_path_style'=>true
                'ssekmskeyid' => 'arn:aws:kms:eu-de:1ba8....a:key/037b8d...e',
        ],
],

Tests:

(1) running tests/lib/Files/ObjectStore/S3Test.phpwithout ssekmskeyid set: passed
(2) running tests/lib/Files/ObjectStore/S3Test.phpwith ssekmskeyid set: passed

SSE-KMS with bucket key:

The disadvantage of SSE-KMS per file is the number of requests to KMS key management system (which you pay per request).
For this reason, Amazon S3 offer the possibility to securely add a bucket key to reduce KMS traffic, see:
https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucket-key.html

Pure SSE-KMS per file could be a little bit more flexible in some scenarios or prefered by some security specialists. So, both options are available with this patch.
The new configuration looks like this:

'objectstore' => [
        'class' => '\\OC\\Files\\ObjectStore\\S3',
        'arguments' => [
                'bucket' => 'nextcloud',
                'autocreate' => true,
                'key'    => 'EJ39ITYZEUH5BGWDRUFY',
                'secret' => 'M5MrXTRjkyMaxXPe2FRXMTfTfbKEnZCu+7uRTVSj',
                'hostname' => 'example.com',
                'port' => 1234,
                'use_ssl' => true,
                'region' => 'optional',
                // required for some non Amazon S3 implementations
                'use_path_style'=>true
                'ssekmsbucketkeyid' => 'arn:aws:kms:eu-de:1ba8....a:key/037b8d...e',
        ],
],

If bucket key is set, the setting ssekmskeyid is ignored.

Tests:

(1) running tests/lib/Files/ObjectStore/S3Test.phpwithout ssekmsbucketkeyid set: passed
(2) running tests/lib/Files/ObjectStore/S3Test.phpwith ssekmsbucketkeyid set: passed

Help needed

• I am not sure whether the approach above properly integrates into unittest suite. Advice to improve here is appreciated.
• Where are the new configuration options documented to shipt with future releases?

Possible Solution for:
#10767
#11826
(partly solves #22077 for Aws compatible S3 stores)

The current Implementation also has an incomfortable bug that e.g. avoids the Creation/Invitation of new Users
(at least on our Installation). Thus, the patch also fixes:
#23370 (with the proposed solution from there; see also see https://stackoverflow.com/questions/11247507/fclose-18-is-not-a-valid-stream-resource/11247555)
#26093 which was found during debugging

[kesselb]

Hi @tsdicloud 👋

Where s are the new configuration options documented to shipt with future releases?

https://github.com/nextcloud/documentation/blob/master/admin_manual/configuration_files/primary_storage.rst

Static code analysis / static-code-analysis (pull_request)

This failure should be fixed by a rebase. It's fixed on master already.

[mashedkeyboard]

I've only had a cursory look over this PR, but I don't think it would fix #22077, would it? At best, it would be a workaround for the issue, but it would only even be a workaround if you were using Amazon S3 storage rather than another S3-compatible provider.

[tsdicloud]

I've only had a cursory look over this PR, but I don't think it would fix #22077, would it? At best, it would be a workaround for the issue, but it would only even be a workaround if you were using Amazon S3 storage rather than another S3-compatible provider.

You are right, I changed the pull request acordingly.

[LukasReschke]

Seems something in the rebase went wrong? :)

[tsdicloud]

You are right, and another rebase did the job - nearly completely. Don´t know what happened to the one file that now breaks my psalm. Any idea?

[juliushaertl]

Not sure if that is a faulty conflict resolution but this drops the existing verify_bucket_exists parameter so existing installations with that would again check for bucket existence on every connection attempt. I guess the autocreate parameter can be dropped and instead the existing verify_bucket_exists could just be used for this check.

[tsdicloud]

This is another inconsistency (and contradiction to documentation) that has been corrected along the way. See issue
#26093

[tsdicloud]

even if this is changed, it never worked properly. So why bother?

[juliushaertl]

@tsdicloud Could you do another rebase to latest master to resolve the conflict?