From 6cfb655717fabb993c90763f7434fb9e197be392 Mon Sep 17 00:00:00 2001 From: Stephen Cuppett Date: Sat, 25 Jul 2026 09:24:00 -0500 Subject: [PATCH 1/4] feat(objectstore): add DSSE-KMS support and unify S3 sse configuration Follow-up to #57623 (SSE-KMS). Adds a single `sse` objectstore argument (none/sse-s3/sse-c/sse-kms/sse-kms-dsse) so DSSE-KMS, encryption context, and S3 Bucket Keys can be configured without ambiguous or silently conflicting combinations of booleans. `sse_c_key` and `sse_kms_enabled` remain as deprecated aliases so existing configs keep working unchanged. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Stephen Cuppett --- config/config.sample.php | 25 +++ .../Files/ObjectStore/S3ConfigTrait.php | 9 + .../Files/ObjectStore/S3ConnectionTrait.php | 149 +++++++++----- .../Files/ObjectStore/S3EncryptionMode.php | 30 +++ lib/private/SystemConfig.php | 6 + tests/lib/Files/ObjectStore/S3SSEKMSTest.php | 37 ++-- .../S3ServerSideEncryptionTest.php | 187 ++++++++++++++++++ 7 files changed, 384 insertions(+), 59 deletions(-) create mode 100644 lib/private/Files/ObjectStore/S3EncryptionMode.php create mode 100644 tests/lib/Files/ObjectStore/S3ServerSideEncryptionTest.php diff --git a/config/config.sample.php b/config/config.sample.php index 061d081d65581..a7cfbc3d6a35a 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -2116,6 +2116,31 @@ // using Amazon S3 (or any other implementation that supports it) we recommend enabling it by using "when_supported". 'request_checksum_calculation' => 'when_required', 'response_checksum_validation' => 'when_required', + // optional: Server-side encryption of objects at rest. + // Valid values are: + // - '' (default): no server-side encryption requested by Nextcloud + // - 'sse-s3': S3-managed AES256 key + // - 'sse-c': customer-provided key, see 'sse_c_key' below + // - 'sse-kms': single layer of AWS KMS-managed encryption + // - 'sse-kms-dsse': dual layer of AWS KMS-managed encryption (DSSE-KMS), higher cost + // and latency than 'sse-kms' but required by some compliance standards + // https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingKMSEncryption.html + // https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingDSSEncryption.html + 'sse' => '', + // optional: customer-provided key for 'sse' => 'sse-c', base64 encoded + 'sse_c_key' => '', + // optional: ARN of the AWS KMS key to use for 'sse-kms' / 'sse-kms-dsse'. + // If omitted, the bucket's default KMS key (or the AWS-managed 'aws/s3' key) is used. + // The key must be a symmetric key in the same AWS region as the bucket. + 'sse_kms_key_id' => '', + // optional: 'sse-kms' / 'sse-kms-dsse' only. Additional AWS KMS encryption context, + // e.g. to scope key usage to a tenant via a matching kms:EncryptionContext condition + // on the key policy. This is stored alongside the object and visible in AWS CloudTrail, + // so it must not contain secrets. + 'sse_kms_encryption_context' => [], + // optional: 'sse-kms' only, not supported for 'sse-kms-dsse'. Enables S3 Bucket Keys, + // which can reduce AWS KMS request costs by up to 99% for SSE-KMS. + 'sse_kms_bucket_key' => false, ], ], diff --git a/lib/private/Files/ObjectStore/S3ConfigTrait.php b/lib/private/Files/ObjectStore/S3ConfigTrait.php index 1c6eaf437ae2f..c1a5e9c2bb690 100644 --- a/lib/private/Files/ObjectStore/S3ConfigTrait.php +++ b/lib/private/Files/ObjectStore/S3ConfigTrait.php @@ -41,4 +41,13 @@ trait S3ConfigTrait { private bool $useMultipartCopy = true; protected int $retriesMaxAttempts; + + protected S3EncryptionMode $sseMode = S3EncryptionMode::None; + + protected ?string $sseKmsKeyId = null; + + /** Already base64-encoded JSON, ready to hand to the AWS SDK */ + protected ?string $sseKmsEncryptionContext = null; + + protected bool $sseKmsBucketKey = false; } diff --git a/lib/private/Files/ObjectStore/S3ConnectionTrait.php b/lib/private/Files/ObjectStore/S3ConnectionTrait.php index 3837f0b869f8b..569f6d796e8a8 100644 --- a/lib/private/Files/ObjectStore/S3ConnectionTrait.php +++ b/lib/private/Files/ObjectStore/S3ConnectionTrait.php @@ -62,6 +62,7 @@ protected function parseParams($params) { $this->copySizeLimit = $params['copySizeLimit'] ?? 5242880000; $this->useMultipartCopy = (bool)($params['useMultipartCopy'] ?? true); $this->retriesMaxAttempts = $params['retriesMaxAttempts'] ?? 5; + $this->parseEncryptionParams($params); $params['region'] = empty($params['region']) ? 'eu-west-1' : $params['region']; $params['hostname'] = empty($params['hostname']) ? 's3.' . $params['region'] . '.amazonaws.com' : $params['hostname']; $params['s3-accelerate'] = $params['hostname'] === 's3-accelerate.amazonaws.com' || $params['hostname'] === 's3-accelerate.dualstack.amazonaws.com'; @@ -337,77 +338,129 @@ protected function getSSECParameters(bool $copy = false): array { } /** - * Get SSE-KMS key ID from configuration - * @return string|null KMS key ARN/ID or null for bucket default key + * Resolve and validate the `sse*` objectstore arguments into `$this->sseMode` and friends. + * + * `sse_c_key` and `sse_kms_enabled` are kept as deprecated aliases for `sse` set to + * `sse-c` / `sse-kms` respectively, so existing configurations keep working unchanged. + * Conflicting combinations are rejected rather than silently resolved, since a config + * that requests one encryption mode but silently gets another (or none) is worse than + * a startup error. + * + * @throws \Exception on invalid or conflicting `sse*` configuration */ - protected function getSSEKMSKeyId(): ?string { - if (isset($this->params['sse_kms_key_id']) && !empty($this->params['sse_kms_key_id'])) { - return $this->params['sse_kms_key_id']; + private function parseEncryptionParams(array $params): void { + $explicitMode = null; + if (isset($params['sse']) && $params['sse'] !== '') { + $explicitMode = S3EncryptionMode::tryFrom($params['sse']); + if ($explicitMode === null) { + $valid = implode(', ', array_map( + static fn (S3EncryptionMode $mode): string => "'{$mode->value}'", + array_filter(S3EncryptionMode::cases(), static fn (S3EncryptionMode $mode): bool => $mode !== S3EncryptionMode::None) + )); + throw new \Exception("Invalid 'sse' objectstore argument '{$params['sse']}', expected one of: {$valid}."); + } } - return null; - } - /** - * Check if SSE-KMS is enabled - * @return bool - */ - protected function isSSEKMSEnabled(): bool { - return !empty($this->params['sse_kms_enabled']) && $this->params['sse_kms_enabled'] === true; + $hasLegacySseC = !empty($params['sse_c_key']); + $hasLegacySseKms = !empty($params['sse_kms_enabled']); + + if ($hasLegacySseC && $hasLegacySseKms) { + throw new \Exception("The 'sse_c_key' and 'sse_kms_enabled' objectstore arguments are mutually exclusive, use the 'sse' argument to select a single encryption mode."); + } + + if ($explicitMode !== null) { + if ($hasLegacySseC && $explicitMode !== S3EncryptionMode::SseC) { + throw new \Exception("The 'sse_c_key' objectstore argument requires 'sse' to be 'sse-c' (or unset)."); + } + if ($hasLegacySseKms && !$explicitMode->isKms()) { + throw new \Exception("The 'sse_kms_enabled' objectstore argument requires 'sse' to be 'sse-kms' or 'sse-kms-dsse' (or unset)."); + } + $mode = $explicitMode; + } elseif ($hasLegacySseC) { + $mode = S3EncryptionMode::SseC; + } elseif ($hasLegacySseKms) { + $mode = S3EncryptionMode::SseKms; + } else { + $mode = S3EncryptionMode::None; + } + + $this->sseMode = $mode; + + $keyId = (isset($params['sse_kms_key_id']) && $params['sse_kms_key_id'] !== '') ? (string)$params['sse_kms_key_id'] : null; + if ($keyId !== null && !$mode->isKms()) { + throw new \Exception("The 'sse_kms_key_id' objectstore argument requires 'sse' to be 'sse-kms' or 'sse-kms-dsse'."); + } + $this->sseKmsKeyId = $keyId; + + $context = $params['sse_kms_encryption_context'] ?? null; + if ($context !== null) { + if (!$mode->isKms()) { + throw new \Exception("The 'sse_kms_encryption_context' objectstore argument requires 'sse' to be 'sse-kms' or 'sse-kms-dsse'."); + } + if (!is_array($context)) { + throw new \Exception("The 'sse_kms_encryption_context' objectstore argument must be an array of string key-value pairs."); + } + foreach ($context as $contextKey => $contextValue) { + if (!is_string($contextKey) || !is_string($contextValue)) { + throw new \Exception("The 'sse_kms_encryption_context' objectstore argument must be an array of string key-value pairs."); + } + } + $this->sseKmsEncryptionContext = base64_encode(json_encode($context, JSON_THROW_ON_ERROR)); + } else { + $this->sseKmsEncryptionContext = null; + } + + $bucketKey = (bool)($params['sse_kms_bucket_key'] ?? false); + if ($bucketKey && $mode !== S3EncryptionMode::SseKms) { + $reason = $mode === S3EncryptionMode::SseKmsDsse + ? "S3 Bucket Keys are not supported for DSSE-KMS ('sse' => 'sse-kms-dsse')" + : "'sse_kms_bucket_key' requires 'sse' to be 'sse-kms'"; + throw new \Exception("The 'sse_kms_bucket_key' objectstore argument cannot be enabled: {$reason}."); + } + $this->sseKmsBucketKey = $bucketKey; } /** - * Get SSE-KMS parameters for S3 operations + * Get SSE-KMS / DSSE-KMS parameters for S3 operations. * - * When SSE-KMS is enabled, AWS S3 encrypts objects server-side using - * AWS Key Management Service (KMS) keys. This provides: - * - Centralized key management via AWS KMS - * - Audit trail of key usage - * - No client-side encryption overhead - * - Automatic key rotation support - * - * @param bool $copy Whether this is for a copy operation (unused for KMS) * @return array Parameters to merge into S3 API calls */ - protected function getSSEKMSParameters(bool $copy = false): array { - if (!$this->isSSEKMSEnabled()) { - return []; - } - + private function getSSEKMSParameters(): array { $params = [ - 'ServerSideEncryption' => 'aws:kms', + 'ServerSideEncryption' => $this->sseMode === S3EncryptionMode::SseKmsDsse ? 'aws:kms:dsse' : 'aws:kms', ]; - // Add specific KMS key if configured, otherwise use bucket default key - $keyId = $this->getSSEKMSKeyId(); - if ($keyId !== null) { - $params['SSEKMSKeyId'] = $keyId; + // Add specific KMS key if configured, otherwise use the bucket default key + if ($this->sseKmsKeyId !== null) { + $params['SSEKMSKeyId'] = $this->sseKmsKeyId; } - // Note: For copy operations, S3 re-encrypts with the destination key - // No special source parameters needed (unlike SSE-C) + if ($this->sseKmsEncryptionContext !== null) { + $params['SSEKMSEncryptionContext'] = $this->sseKmsEncryptionContext; + } + + if ($this->sseKmsBucketKey) { + $params['BucketKeyEnabled'] = true; + } return $params; } /** - * Get unified server-side encryption parameters + * Get server-side encryption parameters for S3 operations, for whichever `sse` mode is configured. * - * Supports both SSE-C (customer-provided keys) and SSE-KMS (AWS-managed keys). - * SSE-C takes precedence if both are configured (for backward compatibility - * during migration from SSE-C to SSE-KMS). - * - * @param bool $copy Whether this is for a copy operation + * @param bool $copy Whether this is for a copy operation (only relevant for SSE-C) * @return array Encryption parameters to merge into S3 API calls */ protected function getServerSideEncryptionParameters(bool $copy = false): array { - // SSE-C takes precedence for backward compatibility during migration - $sseC = $this->getSSECParameters($copy); - if (!empty($sseC)) { - return $sseC; - } - - // Fall back to SSE-KMS if enabled - return $this->getSSEKMSParameters($copy); + return match ($this->sseMode) { + S3EncryptionMode::None => [], + S3EncryptionMode::SseS3 => ['ServerSideEncryption' => 'AES256'], + S3EncryptionMode::SseC => $this->getSSECParameters($copy), + // Unlike SSE-C, KMS/DSSE-KMS have no CopySource* variant: on CopyObject these + // headers describe the destination object only, so $copy is not needed here. + S3EncryptionMode::SseKms, S3EncryptionMode::SseKmsDsse => $this->getSSEKMSParameters(), + }; } public function isUsePresignedUrl(): bool { diff --git a/lib/private/Files/ObjectStore/S3EncryptionMode.php b/lib/private/Files/ObjectStore/S3EncryptionMode.php new file mode 100644 index 0000000000000..59a0821e0c2e1 --- /dev/null +++ b/lib/private/Files/ObjectStore/S3EncryptionMode.php @@ -0,0 +1,30 @@ + true, 'secret' => true, 'sse_c_key' => true, + // SSE-KMS encryption context may carry tenant identifiers and is marked + // sensitive by the AWS SDK itself (visible in CloudTrail, but not a secret + // we want dumped in support requests) + 'sse_kms_encryption_context' => true, // Swift v2 'username' => true, 'password' => true, @@ -98,6 +102,8 @@ class SystemConfig { // S3 'key' => true, 'secret' => true, + 'sse_c_key' => true, + 'sse_kms_encryption_context' => true, // Swift v2 'username' => true, 'password' => true, diff --git a/tests/lib/Files/ObjectStore/S3SSEKMSTest.php b/tests/lib/Files/ObjectStore/S3SSEKMSTest.php index 618af2af7f206..b931aad1e06da 100644 --- a/tests/lib/Files/ObjectStore/S3SSEKMSTest.php +++ b/tests/lib/Files/ObjectStore/S3SSEKMSTest.php @@ -10,22 +10,24 @@ namespace Test\Files\ObjectStore; use OC\Files\ObjectStore\S3; +use OC\Files\ObjectStore\S3EncryptionMode; use OCP\IConfig; use OCP\Server; /** - * Test suite for AWS SSE-KMS (Server-Side Encryption with Key Management Service). + * Test suite for AWS SSE-KMS and DSSE-KMS (Server-Side Encryption with Key Management Service). * - * SSE-KMS provides: - * - AWS-managed server-side encryption + * SSE-KMS / DSSE-KMS provide: + * - AWS-managed server-side encryption (DSSE-KMS applies two independent layers) * - Centralized key management via AWS KMS * - Audit trail of key usage via CloudTrail * - No client-side encryption overhead * - Automatic key rotation support * * Configuration options: - * - sse_kms_enabled: true - Enable SSE-KMS + * - sse: 'sse-kms' or 'sse-kms-dsse' - Enable single- or dual-layer KMS encryption * - sse_kms_key_id: (optional) Specific KMS key ARN, or use bucket default + * - sse_kms_enabled: true - deprecated alias for sse => 'sse-kms' */ #[\PHPUnit\Framework\Attributes\Group('PRIMARY-s3')] #[\PHPUnit\Framework\Attributes\Group('SSE-KMS')] @@ -43,8 +45,10 @@ public static function setUpBeforeClass(): void { } $arguments = $config['arguments'] ?? []; - if (empty($arguments['sse_kms_enabled'])) { - self::markTestSkipped('SSE-KMS not enabled. Set sse_kms_enabled=true in objectstore config'); + $sse = S3EncryptionMode::tryFrom($arguments['sse'] ?? ''); + $isKmsEnabled = ($sse !== null && $sse->isKms()) || !empty($arguments['sse_kms_enabled']); + if (!$isKmsEnabled) { + self::markTestSkipped("SSE-KMS not enabled. Set sse => 'sse-kms' or 'sse-kms-dsse' in the objectstore config"); } } @@ -57,6 +61,17 @@ protected function getInstance() { return $this->instance; } + /** + * The `ServerSideEncryption` value expected on objects, given the configured `sse` mode + * (or its deprecated `sse_kms_enabled` alias, which always maps to single-layer SSE-KMS). + */ + private function getExpectedServerSideEncryption(): string { + $config = Server::get(IConfig::class)->getSystemValue('objectstore'); + $arguments = $config['arguments'] ?? []; + $sse = S3EncryptionMode::tryFrom($arguments['sse'] ?? ''); + return $sse === S3EncryptionMode::SseKmsDsse ? 'aws:kms:dsse' : 'aws:kms'; + } + /** * Test basic write and read with SSE-KMS */ @@ -212,7 +227,7 @@ public function testKMSMetadataPresent(): void { ]); // Verify SSE is KMS - $this->assertEquals('aws:kms', $result->get('ServerSideEncryption'), + $this->assertEquals($this->getExpectedServerSideEncryption(), $result->get('ServerSideEncryption'), 'Object should have SSE-KMS encryption'); // If specific key configured, verify it's used @@ -252,7 +267,7 @@ public function testZeroByteFileWithKMS(): void { $this->assertEquals(0, $metadata->get('ContentLength'), 'Zero-byte file should have ContentLength of 0'); - $this->assertEquals('aws:kms', $metadata->get('ServerSideEncryption'), + $this->assertEquals($this->getExpectedServerSideEncryption(), $metadata->get('ServerSideEncryption'), 'Zero-byte file should still have SSE-KMS encryption'); } @@ -285,7 +300,7 @@ public function testLargeFileSizesWithKMS(int $size): void { 'Key' => $urn, ]); - $this->assertEquals('aws:kms', $metadata->get('ServerSideEncryption'), + $this->assertEquals($this->getExpectedServerSideEncryption(), $metadata->get('ServerSideEncryption'), "Object should have SSE-KMS encryption for size $size"); $this->assertEquals($size, $metadata->get('ContentLength'), "Size should match for $size byte file"); @@ -324,7 +339,7 @@ public function testMultipartCopyWithKMS(): void { 'Key' => 'kms-test-multipart-copy-target', ]); - $this->assertEquals('aws:kms', $metadata->get('ServerSideEncryption'), + $this->assertEquals($this->getExpectedServerSideEncryption(), $metadata->get('ServerSideEncryption'), 'Copied object should have SSE-KMS encryption'); } @@ -393,7 +408,7 @@ public function testOverwriteWithKMS(): void { 'Key' => 'kms-test-overwrite', ]); - $this->assertEquals('aws:kms', $metadata->get('ServerSideEncryption'), + $this->assertEquals($this->getExpectedServerSideEncryption(), $metadata->get('ServerSideEncryption'), 'Overwritten object should still have SSE-KMS encryption'); } } diff --git a/tests/lib/Files/ObjectStore/S3ServerSideEncryptionTest.php b/tests/lib/Files/ObjectStore/S3ServerSideEncryptionTest.php new file mode 100644 index 0000000000000..6d3a6c3353b10 --- /dev/null +++ b/tests/lib/Files/ObjectStore/S3ServerSideEncryptionTest.php @@ -0,0 +1,187 @@ + 'test-bucket'] + $arguments); + } + + /** + * @return array Parameters S3 would merge into an S3 API call + */ + private function getServerSideEncryptionParameters(S3 $s3, bool $copy = false): array { + $method = new \ReflectionMethod($s3, 'getServerSideEncryptionParameters'); + return $method->invoke($s3, $copy); + } + + public function testNoEncryptionByDefault(): void { + $s3 = $this->makeS3([]); + $this->assertSame([], $this->getServerSideEncryptionParameters($s3)); + } + + public function testSseS3(): void { + $s3 = $this->makeS3(['sse' => 'sse-s3']); + $this->assertSame(['ServerSideEncryption' => 'AES256'], $this->getServerSideEncryptionParameters($s3)); + } + + public function testSseC(): void { + $rawKey = random_bytes(32); + $s3 = $this->makeS3(['sse' => 'sse-c', 'sse_c_key' => base64_encode($rawKey)]); + + $this->assertSame([ + 'SSECustomerAlgorithm' => 'AES256', + 'SSECustomerKey' => $rawKey, + 'SSECustomerKeyMD5' => md5($rawKey, true), + ], $this->getServerSideEncryptionParameters($s3)); + + $this->assertSame([ + 'CopySourceSSECustomerAlgorithm' => 'AES256', + 'CopySourceSSECustomerKey' => $rawKey, + 'CopySourceSSECustomerKeyMD5' => md5($rawKey, true), + ], $this->getServerSideEncryptionParameters($s3, true)); + } + + public function testSseCLegacyAliasWithoutExplicitSse(): void { + // Configs from before the 'sse' argument existed must keep working unchanged. + $rawKey = random_bytes(32); + $s3 = $this->makeS3(['sse_c_key' => base64_encode($rawKey)]); + + $this->assertSame([ + 'SSECustomerAlgorithm' => 'AES256', + 'SSECustomerKey' => $rawKey, + 'SSECustomerKeyMD5' => md5($rawKey, true), + ], $this->getServerSideEncryptionParameters($s3)); + } + + public function testSseKms(): void { + $s3 = $this->makeS3(['sse' => 'sse-kms']); + $this->assertSame(['ServerSideEncryption' => 'aws:kms'], $this->getServerSideEncryptionParameters($s3)); + } + + public function testSseKmsWithKeyId(): void { + $s3 = $this->makeS3([ + 'sse' => 'sse-kms', + 'sse_kms_key_id' => 'arn:aws:kms:us-east-1:123456789012:key/test-key', + ]); + $this->assertSame([ + 'ServerSideEncryption' => 'aws:kms', + 'SSEKMSKeyId' => 'arn:aws:kms:us-east-1:123456789012:key/test-key', + ], $this->getServerSideEncryptionParameters($s3)); + } + + public function testSseKmsWithEncryptionContext(): void { + $s3 = $this->makeS3([ + 'sse' => 'sse-kms', + 'sse_kms_encryption_context' => ['tenant' => 'acme'], + ]); + $this->assertSame([ + 'ServerSideEncryption' => 'aws:kms', + // SSEKMSEncryptionContext is a plain string shape in the AWS SDK, so Nextcloud + // must base64-encode the JSON itself; the SDK does not do this automatically. + 'SSEKMSEncryptionContext' => base64_encode(json_encode(['tenant' => 'acme'])), + ], $this->getServerSideEncryptionParameters($s3)); + } + + public function testSseKmsWithBucketKey(): void { + $s3 = $this->makeS3(['sse' => 'sse-kms', 'sse_kms_bucket_key' => true]); + $this->assertSame([ + 'ServerSideEncryption' => 'aws:kms', + 'BucketKeyEnabled' => true, + ], $this->getServerSideEncryptionParameters($s3)); + } + + public function testSseKmsDsse(): void { + $s3 = $this->makeS3(['sse' => 'sse-kms-dsse', 'sse_kms_key_id' => 'arn:aws:kms:us-east-1:123456789012:key/test-key']); + $this->assertSame([ + 'ServerSideEncryption' => 'aws:kms:dsse', + 'SSEKMSKeyId' => 'arn:aws:kms:us-east-1:123456789012:key/test-key', + ], $this->getServerSideEncryptionParameters($s3)); + } + + public function testSseKmsDsseRejectsBucketKey(): void { + // S3 Bucket Keys are not supported for DSSE-KMS. + $this->expectException(\Exception::class); + $this->expectExceptionMessage('not supported for DSSE-KMS'); + $this->makeS3(['sse' => 'sse-kms-dsse', 'sse_kms_bucket_key' => true]); + } + + /** + * @dataProvider dataTruthyLegacyKmsEnabled + */ + #[\PHPUnit\Framework\Attributes\DataProvider('dataTruthyLegacyKmsEnabled')] + public function testSseKmsLegacyAliasAcceptsAnyTruthyValue(mixed $value): void { + // Prior to this change, 'sse_kms_enabled' required a strict `=== true` check, so + // e.g. 1 or 'true' from an environment-driven config would silently disable encryption. + $s3 = $this->makeS3(['sse_kms_enabled' => $value]); + $this->assertSame(['ServerSideEncryption' => 'aws:kms'], $this->getServerSideEncryptionParameters($s3)); + } + + public static function dataTruthyLegacyKmsEnabled(): array { + return [ + 'bool true' => [true], + 'int 1' => [1], + 'string "true"' => ['true'], + 'string "1"' => ['1'], + ]; + } + + public function testConflictingLegacyKeysAreRejected(): void { + $this->expectException(\Exception::class); + $this->expectExceptionMessage('mutually exclusive'); + $this->makeS3(['sse_c_key' => base64_encode(random_bytes(32)), 'sse_kms_enabled' => true]); + } + + public function testExplicitSseConflictingWithLegacySseCKeyIsRejected(): void { + $this->expectException(\Exception::class); + $this->makeS3(['sse' => 'sse-kms', 'sse_c_key' => base64_encode(random_bytes(32))]); + } + + public function testExplicitSseConflictingWithLegacyKmsEnabledIsRejected(): void { + $this->expectException(\Exception::class); + $this->makeS3(['sse' => 'sse-c', 'sse_kms_enabled' => true]); + } + + public function testInvalidSseValueIsRejected(): void { + $this->expectException(\Exception::class); + $this->expectExceptionMessage("Invalid 'sse'"); + $this->makeS3(['sse' => 'sse-does-not-exist']); + } + + public function testKmsKeyIdWithoutKmsModeIsRejected(): void { + $this->expectException(\Exception::class); + $this->makeS3(['sse' => 'sse-s3', 'sse_kms_key_id' => 'arn:aws:kms:us-east-1:123456789012:key/test-key']); + } + + public function testEncryptionContextWithoutKmsModeIsRejected(): void { + $this->expectException(\Exception::class); + $this->makeS3(['sse' => 'sse-c', 'sse_c_key' => base64_encode(random_bytes(32)), 'sse_kms_encryption_context' => ['tenant' => 'acme']]); + } + + public function testEncryptionContextRejectsNonStringValues(): void { + $this->expectException(\Exception::class); + $this->makeS3(['sse' => 'sse-kms', 'sse_kms_encryption_context' => ['tenant' => 123]]); + } + + public function testBucketKeyWithoutSseKmsModeIsRejected(): void { + $this->expectException(\Exception::class); + $this->makeS3(['sse' => 'sse-s3', 'sse_kms_bucket_key' => true]); + } +} From 8364c303e3018f2cae6398e8876acf2460e2a89c Mon Sep 17 00:00:00 2001 From: Stephen Cuppett Date: Sat, 25 Jul 2026 10:32:48 -0500 Subject: [PATCH 2/4] fix(objectstore): resolve conflicting S3 sse config by precedence, not exception Conflicting sse* arguments now resolve deterministically instead of throwing: sse_c_key + sse_kms_enabled both set falls back to SSE-C, matching the behaviour already released in Nextcloud 34, and an explicit sse argument always overrides the deprecated keys. Every resolution is recorded and logged once via getConnection(), including a warning on any SSE-C usage, so the fallback is no longer silent. sse_kms_bucket_key with sse-kms-dsse also moves from a hard exception to a warning: verified against a live bucket that S3 accepts the request and simply does not apply the bucket key, rather than rejecting it as AWS's own documentation suggests. Adds a live test suite (S3EncryptionModesLiveTest) exercising the full sse mode matrix and cross-mode reads against a real AWS bucket and KMS key, and extends the offline S3ServerSideEncryptionTest with precedence, warning, and logger-emission coverage. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Stephen Cuppett --- .../Files/ObjectStore/S3ConfigTrait.php | 3 + .../Files/ObjectStore/S3ConnectionTrait.php | 106 ++++-- .../ObjectStore/S3EncryptionModesLiveTest.php | 343 ++++++++++++++++++ .../S3ServerSideEncryptionTest.php | 243 +++++++++++-- 4 files changed, 644 insertions(+), 51 deletions(-) create mode 100644 tests/lib/Files/ObjectStore/S3EncryptionModesLiveTest.php diff --git a/lib/private/Files/ObjectStore/S3ConfigTrait.php b/lib/private/Files/ObjectStore/S3ConfigTrait.php index c1a5e9c2bb690..bbe52ea0d97be 100644 --- a/lib/private/Files/ObjectStore/S3ConfigTrait.php +++ b/lib/private/Files/ObjectStore/S3ConfigTrait.php @@ -50,4 +50,7 @@ trait S3ConfigTrait { protected ?string $sseKmsEncryptionContext = null; protected bool $sseKmsBucketKey = false; + + /** @var string[] Non-fatal `sse*` configuration issues found by parseEncryptionParams(), logged by logEncryptionWarnings() */ + protected array $encryptionWarnings = []; } diff --git a/lib/private/Files/ObjectStore/S3ConnectionTrait.php b/lib/private/Files/ObjectStore/S3ConnectionTrait.php index 569f6d796e8a8..e358973db2da8 100644 --- a/lib/private/Files/ObjectStore/S3ConnectionTrait.php +++ b/lib/private/Files/ObjectStore/S3ConnectionTrait.php @@ -38,6 +38,9 @@ trait S3ConnectionTrait { private ?ICache $existingBucketsCache = null; private bool $usePresignedUrl = false; + /** @var array Deduplicates encryption warnings across instances/calls, keyed by message */ + private static array $loggedEncryptionWarnings = []; + protected function parseParams($params) { if (empty($params['bucket'])) { throw new \Exception('Bucket has to be configured.'); @@ -97,6 +100,8 @@ public function getConnection() { return $this->connection; } + $this->logEncryptionWarnings(); + if ($this->existingBucketsCache === null) { $this->existingBucketsCache = Server::get(ICacheFactory::class) ->createLocal('s3-bucket-exists-cache'); @@ -341,14 +346,19 @@ protected function getSSECParameters(bool $copy = false): array { * Resolve and validate the `sse*` objectstore arguments into `$this->sseMode` and friends. * * `sse_c_key` and `sse_kms_enabled` are kept as deprecated aliases for `sse` set to - * `sse-c` / `sse-kms` respectively, so existing configurations keep working unchanged. - * Conflicting combinations are rejected rather than silently resolved, since a config - * that requests one encryption mode but silently gets another (or none) is worse than - * a startup error. + * `sse-c` / `sse-kms` respectively, so existing configurations keep working unchanged. An + * explicit `sse` always wins over both. If neither is set and both deprecated keys are, + * `sse_c_key` (SSE-C) takes precedence, matching the behaviour Nextcloud 34 shipped with. * - * @throws \Exception on invalid or conflicting `sse*` configuration + * Configuration that can be deterministically resolved never blocks boot: it is instead + * recorded in `$this->encryptionWarnings` for `logEncryptionWarnings()` to log once a + * connection is actually made. Only configuration with no reasonable resolution throws. + * + * @throws \Exception on `sse*` configuration with no reasonable resolution */ private function parseEncryptionParams(array $params): void { + $this->encryptionWarnings = []; + $explicitMode = null; if (isset($params['sse']) && $params['sse'] !== '') { $explicitMode = S3EncryptionMode::tryFrom($params['sse']); @@ -364,39 +374,51 @@ private function parseEncryptionParams(array $params): void { $hasLegacySseC = !empty($params['sse_c_key']); $hasLegacySseKms = !empty($params['sse_kms_enabled']); - if ($hasLegacySseC && $hasLegacySseKms) { - throw new \Exception("The 'sse_c_key' and 'sse_kms_enabled' objectstore arguments are mutually exclusive, use the 'sse' argument to select a single encryption mode."); - } - if ($explicitMode !== null) { - if ($hasLegacySseC && $explicitMode !== S3EncryptionMode::SseC) { - throw new \Exception("The 'sse_c_key' objectstore argument requires 'sse' to be 'sse-c' (or unset)."); + $mode = $explicitMode; + if ($hasLegacySseC && $mode !== S3EncryptionMode::SseC) { + $this->encryptionWarnings[] = "Ignoring the 'sse_c_key' objectstore argument: 'sse' is explicitly set to '{$mode->value}'."; } - if ($hasLegacySseKms && !$explicitMode->isKms()) { - throw new \Exception("The 'sse_kms_enabled' objectstore argument requires 'sse' to be 'sse-kms' or 'sse-kms-dsse' (or unset)."); + if ($hasLegacySseKms && !$mode->isKms()) { + $this->encryptionWarnings[] = "Ignoring the 'sse_kms_enabled' objectstore argument: 'sse' is explicitly set to '{$mode->value}'."; } - $mode = $explicitMode; - } elseif ($hasLegacySseC) { - $mode = S3EncryptionMode::SseC; - } elseif ($hasLegacySseKms) { - $mode = S3EncryptionMode::SseKms; + } elseif ($hasLegacySseC || $hasLegacySseKms) { + if ($hasLegacySseC && $hasLegacySseKms) { + // This is the Nextcloud 34 behaviour: SSE-C silently took precedence over + // SSE-KMS whenever both were configured. Kept for backward compatibility, + // but no longer silent. + $this->encryptionWarnings[] = "Both 'sse_c_key' and 'sse_kms_enabled' are set; 'sse_c_key' (SSE-C) takes precedence, as it did in Nextcloud 34. Set the 'sse' objectstore argument to select a single encryption mode explicitly."; + } + $mode = $hasLegacySseC ? S3EncryptionMode::SseC : S3EncryptionMode::SseKms; + $deprecatedKey = $hasLegacySseC ? 'sse_c_key' : 'sse_kms_enabled'; + $this->encryptionWarnings[] = "The '{$deprecatedKey}' objectstore argument is deprecated, use 'sse' => '{$mode->value}' instead."; } else { $mode = S3EncryptionMode::None; } + if ($mode === S3EncryptionMode::SseC) { + // SSE-C is a supported mode, not merely a deprecated way to reach one, so this + // warning is intentionally emitted regardless of how 'sse-c' was selected. + $this->encryptionWarnings[] = "SSE-C ('sse' => 'sse-c') requires you to manage and distribute the encryption key yourself. Consider 'sse' => 'sse-kms' or 'sse-kms-dsse' instead, which let AWS KMS manage the key."; + if (empty($params['sse_c_key'])) { + // Unlike a merely redundant option, this cannot be resolved: silently falling + // back to no encryption would defeat an explicit request for SSE-C. + throw new \Exception("The 'sse' objectstore argument is set to 'sse-c' but no 'sse_c_key' is configured."); + } + } + $this->sseMode = $mode; $keyId = (isset($params['sse_kms_key_id']) && $params['sse_kms_key_id'] !== '') ? (string)$params['sse_kms_key_id'] : null; if ($keyId !== null && !$mode->isKms()) { - throw new \Exception("The 'sse_kms_key_id' objectstore argument requires 'sse' to be 'sse-kms' or 'sse-kms-dsse'."); + $this->encryptionWarnings[] = "Ignoring the 'sse_kms_key_id' objectstore argument: it only applies when 'sse' is 'sse-kms' or 'sse-kms-dsse'."; + $keyId = null; } $this->sseKmsKeyId = $keyId; $context = $params['sse_kms_encryption_context'] ?? null; if ($context !== null) { - if (!$mode->isKms()) { - throw new \Exception("The 'sse_kms_encryption_context' objectstore argument requires 'sse' to be 'sse-kms' or 'sse-kms-dsse'."); - } + // A structurally invalid context can never be resolved, regardless of mode. if (!is_array($context)) { throw new \Exception("The 'sse_kms_encryption_context' objectstore argument must be an array of string key-value pairs."); } @@ -405,21 +427,49 @@ private function parseEncryptionParams(array $params): void { throw new \Exception("The 'sse_kms_encryption_context' objectstore argument must be an array of string key-value pairs."); } } - $this->sseKmsEncryptionContext = base64_encode(json_encode($context, JSON_THROW_ON_ERROR)); - } else { - $this->sseKmsEncryptionContext = null; + if (!$mode->isKms()) { + $this->encryptionWarnings[] = "Ignoring the 'sse_kms_encryption_context' objectstore argument: it only applies when 'sse' is 'sse-kms' or 'sse-kms-dsse'."; + $context = null; + } } + $this->sseKmsEncryptionContext = $context !== null ? base64_encode(json_encode($context, JSON_THROW_ON_ERROR)) : null; $bucketKey = (bool)($params['sse_kms_bucket_key'] ?? false); if ($bucketKey && $mode !== S3EncryptionMode::SseKms) { + // Merely redundant, not impossible: verified against a real bucket that S3 + // accepts BucketKeyEnabled alongside 'aws:kms:dsse' and simply does not apply + // it (HeadObject afterwards shows no BucketKeyEnabled), rather than rejecting + // the request outright, despite AWS's own documentation describing S3 Bucket + // Keys as unsupported for DSSE-KMS. $reason = $mode === S3EncryptionMode::SseKmsDsse - ? "S3 Bucket Keys are not supported for DSSE-KMS ('sse' => 'sse-kms-dsse')" - : "'sse_kms_bucket_key' requires 'sse' to be 'sse-kms'"; - throw new \Exception("The 'sse_kms_bucket_key' objectstore argument cannot be enabled: {$reason}."); + ? "S3 Bucket Keys are not applied for DSSE-KMS ('sse' => 'sse-kms-dsse')" + : "it only applies when 'sse' is 'sse-kms'"; + $this->encryptionWarnings[] = "Ignoring the 'sse_kms_bucket_key' objectstore argument: {$reason}."; + $bucketKey = false; } $this->sseKmsBucketKey = $bucketKey; } + /** + * Log any warnings recorded by `parseEncryptionParams()`, once a connection is actually + * made, deduplicated by message so a long-lived request/worker doesn't repeat the same + * warning on every call. + */ + private function logEncryptionWarnings(): void { + if ($this->encryptionWarnings === []) { + return; + } + + $logger = Server::get(LoggerInterface::class); + foreach ($this->encryptionWarnings as $warning) { + if (isset(self::$loggedEncryptionWarnings[$warning])) { + continue; + } + self::$loggedEncryptionWarnings[$warning] = true; + $logger->warning($warning, ['app' => 'objectstore']); + } + } + /** * Get SSE-KMS / DSSE-KMS parameters for S3 operations. * diff --git a/tests/lib/Files/ObjectStore/S3EncryptionModesLiveTest.php b/tests/lib/Files/ObjectStore/S3EncryptionModesLiveTest.php new file mode 100644 index 0000000000000..c9c223982841b --- /dev/null +++ b/tests/lib/Files/ObjectStore/S3EncryptionModesLiveTest.php @@ -0,0 +1,343 @@ +getSystemValue('objectstore'); + if (!is_array($config) || $config['class'] !== S3::class) { + self::markTestSkipped('S3 primary storage not configured'); + } + + $keyId = getenv('NC_TEST_S3_KMS_KEY_ID'); + if (!$keyId) { + self::markTestSkipped('Set NC_TEST_S3_KMS_KEY_ID to a real AWS KMS key ARN to run the live sse* permutation matrix'); + } + + self::$baseArguments = $config['arguments'] ?? []; + self::$kmsKeyId = $keyId; + } + + #[\Override] + protected function tearDown(): void { + // Any instance from makeS3() can delete any urn: they all share one bucket. + if ($this->cleanupUrns !== []) { + $s3 = $this->makeS3([]); + foreach ($this->cleanupUrns as $urn) { + try { + $s3->deleteObject($urn); + } catch (\Exception) { + // already gone, or never actually got written + } + } + } + + parent::tearDown(); + } + + private function cleanupAfter(string $urn): string { + $this->cleanupUrns[] = $urn; + return $urn; + } + + /** + * @param array $overrides 'sse*' (and other) arguments overriding the configured primary + * objectstore for this one instance; every other test in this + * file starts from a clean slate, with no 'sse*' carried over. + */ + private function makeS3(array $overrides): S3 { + $arguments = array_filter( + self::$baseArguments, + static fn (string $key): bool => !str_starts_with($key, 'sse'), + ARRAY_FILTER_USE_KEY, + ); + return new S3($overrides + $arguments); + } + + private function stringToStream(string $data) { + $stream = fopen('php://temp', 'w+'); + fwrite($stream, $data); + rewind($stream); + return $stream; + } + + private function headObject(S3 $s3, string $urn): array { + return $s3->getConnection()->headObject([ + 'Bucket' => $s3->getBucket(), + 'Key' => $urn, + ])->toArray(); + } + + /** + * Some AWS accounts/buckets are configured to reject any upload that requests SSE-C + * outright ("this bucket has blocked upload requests that specify Server Side Encryption + * with Customer provided keys"), independent of anything Nextcloud does. That is a + * deliberate security control, not a bug to route around, so SSE-C tests skip on this + * specific error rather than failing -- and still run for real on a bucket without it. + */ + private function writeObjectOrSkipIfSseCBlocked(S3 $s3, string $urn, string $data): void { + try { + $s3->writeObject($urn, $this->stringToStream($data)); + } catch (S3Exception $e) { + if ($e->getAwsErrorCode() === 'AccessDenied' && str_contains($e->getAwsErrorMessage() ?? '', 'Customer provided keys')) { + self::markTestSkipped('This bucket/account blocks SSE-C uploads by policy: ' . $e->getAwsErrorMessage()); + } + throw $e; + } + } + + /** + * Asserts that reading $urn throws, the way S3ObjectTrait::readObject() does for a GetObject + * error response (e.g. missing/wrong SSE-C key). Mirrors ObjectStoreTestCase::testReadNonExisting's + * pattern of tolerating the incidental fopen() PHP warning SeekableHttpStream raises alongside it. + */ + private function assertReadFails(S3 $s3, string $urn): void { + $warnings = []; + try { + set_error_handler(function (int $errno, string $errstr) use (&$warnings): bool { + if ($errno === E_DEPRECATED || $errno === E_USER_DEPRECATED) { + return true; + } + $warnings[] = $errstr; + return true; + }); + stream_get_contents($s3->readObject($urn)); + $this->fail("Expected reading '$urn' to fail"); + } catch (\Exception) { + foreach ($warnings as $warning) { + $this->assertStringStartsWith('fopen(', $warning); + } + } finally { + restore_error_handler(); + } + } + + // --- Per-mode matrix ----------------------------------------------------------------- + + public function testNoEncryption(): void { + $urn = $this->cleanupAfter('live-sse-none'); + $s3 = $this->makeS3(['sse' => '']); + $s3->writeObject($urn, $this->stringToStream('none')); + + // AWS applies its own bucket-default encryption (AES256) even when Nextcloud sends + // no encryption headers at all; what matters here is that Nextcloud didn't ask for + // KMS, not that the object ends up with literally no encryption. + $head = $this->headObject($s3, $urn); + $this->assertNotEquals('aws:kms', $head['ServerSideEncryption'] ?? null); + $this->assertNotEquals('aws:kms:dsse', $head['ServerSideEncryption'] ?? null); + $this->assertSame('none', stream_get_contents($s3->readObject($urn))); + } + + public function testSseS3(): void { + $urn = $this->cleanupAfter('live-sse-s3'); + $s3 = $this->makeS3(['sse' => 'sse-s3']); + $s3->writeObject($urn, $this->stringToStream('sse-s3')); + + $head = $this->headObject($s3, $urn); + $this->assertSame('AES256', $head['ServerSideEncryption']); + $this->assertSame('sse-s3', stream_get_contents($s3->readObject($urn))); + } + + public function testSseC(): void { + $urn = $this->cleanupAfter('live-sse-c'); + $key = base64_encode(random_bytes(32)); + $s3 = $this->makeS3(['sse' => 'sse-c', 'sse_c_key' => $key]); + $this->writeObjectOrSkipIfSseCBlocked($s3, $urn, 'sse-c'); + + $this->assertSame('sse-c', stream_get_contents($s3->readObject($urn))); + + // Genuinely differs from SSE-KMS/SSE-S3/none: reading without the customer key fails. + $this->assertReadFails($this->makeS3(['sse' => '']), $urn); + } + + public function testSseKmsBucketDefaultKey(): void { + $urn = $this->cleanupAfter('live-sse-kms-default'); + $s3 = $this->makeS3(['sse' => 'sse-kms']); + $s3->writeObject($urn, $this->stringToStream('sse-kms')); + + $head = $this->headObject($s3, $urn); + $this->assertSame('aws:kms', $head['ServerSideEncryption']); + $this->assertSame('sse-kms', stream_get_contents($s3->readObject($urn))); + } + + public function testSseKmsWithKeyId(): void { + $urn = $this->cleanupAfter('live-sse-kms-keyid'); + $s3 = $this->makeS3(['sse' => 'sse-kms', 'sse_kms_key_id' => self::$kmsKeyId]); + $s3->writeObject($urn, $this->stringToStream('sse-kms-keyid')); + + $head = $this->headObject($s3, $urn); + $this->assertSame('aws:kms', $head['ServerSideEncryption']); + $this->assertSame(self::$kmsKeyId, $head['SSEKMSKeyId']); + } + + public function testSseKmsWithEncryptionContext(): void { + // Verified live: unlike ServerSideEncryption/SSEKMSKeyId, S3 does not echo the + // encryption context back on HeadObject/GetObject -- it's write-only, used for KMS + // authorization at PutObject/CopyObject/CreateMultipartUpload time. Confirming it was + // actually applied would require reading the kms:EncryptionContext condition off a + // CloudTrail Decrypt event, which is out of scope here; this just proves AWS accepts + // the header (rather than erroring) and the object still round-trips normally. + $urn = $this->cleanupAfter('live-sse-kms-context'); + $s3 = $this->makeS3([ + 'sse' => 'sse-kms', + 'sse_kms_key_id' => self::$kmsKeyId, + 'sse_kms_encryption_context' => ['tenant' => 'acme'], + ]); + $s3->writeObject($urn, $this->stringToStream('sse-kms-context')); + + $head = $this->headObject($s3, $urn); + $this->assertSame('aws:kms', $head['ServerSideEncryption']); + $this->assertSame('sse-kms-context', stream_get_contents($s3->readObject($urn))); + } + + public function testSseKmsWithBucketKey(): void { + $urn = $this->cleanupAfter('live-sse-kms-bucketkey'); + $s3 = $this->makeS3([ + 'sse' => 'sse-kms', + 'sse_kms_key_id' => self::$kmsKeyId, + 'sse_kms_bucket_key' => true, + ]); + $s3->writeObject($urn, $this->stringToStream('sse-kms-bucketkey')); + + $head = $this->headObject($s3, $urn); + $this->assertSame('aws:kms', $head['ServerSideEncryption']); + $this->assertTrue($head['BucketKeyEnabled'] ?? false); + } + + public function testSseKmsDsseWithKeyId(): void { + $urn = $this->cleanupAfter('live-sse-dsse-keyid'); + $s3 = $this->makeS3(['sse' => 'sse-kms-dsse', 'sse_kms_key_id' => self::$kmsKeyId]); + $s3->writeObject($urn, $this->stringToStream('sse-dsse-keyid')); + + $head = $this->headObject($s3, $urn); + $this->assertSame('aws:kms:dsse', $head['ServerSideEncryption']); + $this->assertSame(self::$kmsKeyId, $head['SSEKMSKeyId']); + } + + public function testSseKmsDsseWithEncryptionContext(): void { + // See testSseKmsWithEncryptionContext(): the context is not echoed back by S3. + $urn = $this->cleanupAfter('live-sse-dsse-context'); + $s3 = $this->makeS3([ + 'sse' => 'sse-kms-dsse', + 'sse_kms_key_id' => self::$kmsKeyId, + 'sse_kms_encryption_context' => ['tenant' => 'acme'], + ]); + $s3->writeObject($urn, $this->stringToStream('sse-dsse-context')); + + $head = $this->headObject($s3, $urn); + $this->assertSame('aws:kms:dsse', $head['ServerSideEncryption']); + $this->assertSame('sse-dsse-context', stream_get_contents($s3->readObject($urn))); + } + + public function testSseKmsDsseBucketKeyIsIgnoredByAws(): void { + // Matches S3ServerSideEncryptionTest::testSseKmsDsseBucketKeyIsWarnedAndIgnored: + // verified live that AWS accepts the request and simply doesn't apply the bucket + // key, rather than rejecting it, despite documenting it as unsupported. + $urn = $this->cleanupAfter('live-sse-dsse-bucketkey'); + $s3 = $this->makeS3([ + 'sse' => 'sse-kms-dsse', + 'sse_kms_key_id' => self::$kmsKeyId, + 'sse_kms_bucket_key' => true, + ]); + $s3->writeObject($urn, $this->stringToStream('sse-dsse-bucketkey')); + + $head = $this->headObject($s3, $urn); + $this->assertSame('aws:kms:dsse', $head['ServerSideEncryption']); + $this->assertFalse($head['BucketKeyEnabled'] ?? false); + } + + // --- Cross-mode reads, the ones that catch real bugs ---------------------------------- + + public function testDsseObjectReadableWithNoEncryptionHeaders(): void { + // Proves the read path needs no encryption headers for KMS/DSSE-KMS: AWS rejects + // GET/HEAD requests that carry SSE-KMS headers with an HTTP 400, so if Nextcloud's + // read path ever started sending them, this would fail instead of silently working. + $urn = $this->cleanupAfter('live-cross-dsse-write-plain-read'); + $writer = $this->makeS3(['sse' => 'sse-kms-dsse', 'sse_kms_key_id' => self::$kmsKeyId]); + $writer->writeObject($urn, $this->stringToStream('cross-mode')); + + $reader = $this->makeS3(['sse' => '']); + $this->assertTrue($reader->objectExists($urn)); + $this->assertSame('cross-mode', stream_get_contents($reader->readObject($urn))); + } + + public function testSseKmsObjectReadableByDsseInstanceAndViceVersa(): void { + $kmsUrn = $this->cleanupAfter('live-cross-kms-write'); + $dsseUrn = $this->cleanupAfter('live-cross-dsse-write'); + + $kms = $this->makeS3(['sse' => 'sse-kms', 'sse_kms_key_id' => self::$kmsKeyId]); + $dsse = $this->makeS3(['sse' => 'sse-kms-dsse', 'sse_kms_key_id' => self::$kmsKeyId]); + + $kms->writeObject($kmsUrn, $this->stringToStream('written-as-kms')); + $dsse->writeObject($dsseUrn, $this->stringToStream('written-as-dsse')); + + $this->assertSame('written-as-kms', stream_get_contents($dsse->readObject($kmsUrn))); + $this->assertSame('written-as-dsse', stream_get_contents($kms->readObject($dsseUrn))); + } + + public function testSseCObjectNotReadableWithoutCustomerKey(): void { + // The negative-path counterpart to testSseC(), kept as its own cross-mode case: + // a plain reader must not be able to read an SSE-C object. + $urn = $this->cleanupAfter('live-cross-ssec-negative'); + $key = base64_encode(random_bytes(32)); + $writer = $this->makeS3(['sse' => 'sse-c', 'sse_c_key' => $key]); + $this->writeObjectOrSkipIfSseCBlocked($writer, $urn, 'ssec-negative'); + + $this->assertReadFails($this->makeS3(['sse' => '']), $urn); + } + + public function testMultipartUploadUnderDsse(): void { + // Parts inherit their encryption from CreateMultipartUpload, not from UploadPart; + // this is the case that would catch a regression there. putSizeLimit is lowered so + // a modest 6 MB body actually exercises the multipart path (default is 100 MB). + $urn = $this->cleanupAfter('live-dsse-multipart'); + $s3 = $this->makeS3([ + 'sse' => 'sse-kms-dsse', + 'sse_kms_key_id' => self::$kmsKeyId, + 'putSizeLimit' => 5 * 1024 * 1024, + ]); + + $data = str_repeat('M', 6 * 1024 * 1024); + $s3->writeObject($urn, $this->stringToStream($data)); + + $head = $this->headObject($s3, $urn); + $this->assertSame('aws:kms:dsse', $head['ServerSideEncryption']); + $this->assertSame($data, stream_get_contents($s3->readObject($urn))); + } +} diff --git a/tests/lib/Files/ObjectStore/S3ServerSideEncryptionTest.php b/tests/lib/Files/ObjectStore/S3ServerSideEncryptionTest.php index 6d3a6c3353b10..549c52a0215cb 100644 --- a/tests/lib/Files/ObjectStore/S3ServerSideEncryptionTest.php +++ b/tests/lib/Files/ObjectStore/S3ServerSideEncryptionTest.php @@ -10,6 +10,7 @@ namespace Test\Files\ObjectStore; use OC\Files\ObjectStore\S3; +use Psr\Log\LoggerInterface; use Test\TestCase; /** @@ -20,6 +21,18 @@ * it never opens a connection. */ class S3ServerSideEncryptionTest extends TestCase { + #[\Override] + protected function setUp(): void { + parent::setUp(); + + // S3ConnectionTrait::$loggedEncryptionWarnings is a static, class-wide dedup cache + // (intentionally: it survives across the many S3 instances a long-lived + // request/worker constructs). Reset it so tests asserting a warning IS logged don't + // depend on which other test using an identical message ran first in this process. + $property = new \ReflectionProperty(S3::class, 'loggedEncryptionWarnings'); + $property->setValue(null, []); + } + private function makeS3(array $arguments): S3 { return new S3(['bucket' => 'test-bucket'] + $arguments); } @@ -32,14 +45,24 @@ private function getServerSideEncryptionParameters(S3 $s3, bool $copy = false): return $method->invoke($s3, $copy); } + /** + * @return string[] Warnings recorded by parseEncryptionParams(), not yet flushed to the logger + */ + private function getEncryptionWarnings(S3 $s3): array { + $property = new \ReflectionProperty($s3, 'encryptionWarnings'); + return $property->getValue($s3); + } + public function testNoEncryptionByDefault(): void { $s3 = $this->makeS3([]); $this->assertSame([], $this->getServerSideEncryptionParameters($s3)); + $this->assertSame([], $this->getEncryptionWarnings($s3)); } public function testSseS3(): void { $s3 = $this->makeS3(['sse' => 'sse-s3']); $this->assertSame(['ServerSideEncryption' => 'AES256'], $this->getServerSideEncryptionParameters($s3)); + $this->assertSame([], $this->getEncryptionWarnings($s3)); } public function testSseC(): void { @@ -57,6 +80,20 @@ public function testSseC(): void { 'CopySourceSSECustomerKey' => $rawKey, 'CopySourceSSECustomerKeyMD5' => md5($rawKey, true), ], $this->getServerSideEncryptionParameters($s3, true)); + + // SSE-C is warned about in every form, including this explicit, non-deprecated one: + // it is a supported mode, not merely a deprecated way to reach one. + $warnings = $this->getEncryptionWarnings($s3); + $this->assertCount(1, $warnings); + $this->assertStringContainsString('SSE-C', $warnings[0]); + } + + public function testSseCWithoutKeyIsRejected(): void { + // Unlike a merely redundant option, this cannot be resolved: silently falling back + // to no encryption would defeat an explicit request for SSE-C. + $this->expectException(\Exception::class); + $this->expectExceptionMessage("no 'sse_c_key' is configured"); + $this->makeS3(['sse' => 'sse-c']); } public function testSseCLegacyAliasWithoutExplicitSse(): void { @@ -69,11 +106,19 @@ public function testSseCLegacyAliasWithoutExplicitSse(): void { 'SSECustomerKey' => $rawKey, 'SSECustomerKeyMD5' => md5($rawKey, true), ], $this->getServerSideEncryptionParameters($s3)); + + // Both the deprecated-key warning and the generic SSE-C warning apply. + $warnings = $this->getEncryptionWarnings($s3); + $this->assertCount(2, $warnings); + $this->assertStringContainsString('sse_c_key', $warnings[0]); + $this->assertStringContainsString('deprecated', $warnings[0]); + $this->assertStringContainsString('SSE-C', $warnings[1]); } public function testSseKms(): void { $s3 = $this->makeS3(['sse' => 'sse-kms']); $this->assertSame(['ServerSideEncryption' => 'aws:kms'], $this->getServerSideEncryptionParameters($s3)); + $this->assertSame([], $this->getEncryptionWarnings($s3)); } public function testSseKmsWithKeyId(): void { @@ -85,6 +130,7 @@ public function testSseKmsWithKeyId(): void { 'ServerSideEncryption' => 'aws:kms', 'SSEKMSKeyId' => 'arn:aws:kms:us-east-1:123456789012:key/test-key', ], $this->getServerSideEncryptionParameters($s3)); + $this->assertSame([], $this->getEncryptionWarnings($s3)); } public function testSseKmsWithEncryptionContext(): void { @@ -98,6 +144,7 @@ public function testSseKmsWithEncryptionContext(): void { // must base64-encode the JSON itself; the SDK does not do this automatically. 'SSEKMSEncryptionContext' => base64_encode(json_encode(['tenant' => 'acme'])), ], $this->getServerSideEncryptionParameters($s3)); + $this->assertSame([], $this->getEncryptionWarnings($s3)); } public function testSseKmsWithBucketKey(): void { @@ -106,6 +153,7 @@ public function testSseKmsWithBucketKey(): void { 'ServerSideEncryption' => 'aws:kms', 'BucketKeyEnabled' => true, ], $this->getServerSideEncryptionParameters($s3)); + $this->assertSame([], $this->getEncryptionWarnings($s3)); } public function testSseKmsDsse(): void { @@ -114,13 +162,20 @@ public function testSseKmsDsse(): void { 'ServerSideEncryption' => 'aws:kms:dsse', 'SSEKMSKeyId' => 'arn:aws:kms:us-east-1:123456789012:key/test-key', ], $this->getServerSideEncryptionParameters($s3)); + $this->assertSame([], $this->getEncryptionWarnings($s3)); } - public function testSseKmsDsseRejectsBucketKey(): void { - // S3 Bucket Keys are not supported for DSSE-KMS. - $this->expectException(\Exception::class); - $this->expectExceptionMessage('not supported for DSSE-KMS'); - $this->makeS3(['sse' => 'sse-kms-dsse', 'sse_kms_bucket_key' => true]); + public function testSseKmsDsseBucketKeyIsWarnedAndIgnored(): void { + // AWS documents S3 Bucket Keys as unsupported for DSSE-KMS, but verified against a + // real bucket: S3 accepts BucketKeyEnabled alongside 'aws:kms:dsse' and simply does + // not apply it, rather than rejecting the request. Matches every other redundant + // 'sse_kms_*' option: warn and drop, don't block boot over something S3 itself tolerates. + $s3 = $this->makeS3(['sse' => 'sse-kms-dsse', 'sse_kms_bucket_key' => true]); + $this->assertSame(['ServerSideEncryption' => 'aws:kms:dsse'], $this->getServerSideEncryptionParameters($s3)); + + $warnings = $this->getEncryptionWarnings($s3); + $this->assertCount(1, $warnings); + $this->assertStringContainsString('sse_kms_bucket_key', $warnings[0]); } /** @@ -143,20 +198,69 @@ public static function dataTruthyLegacyKmsEnabled(): array { ]; } - public function testConflictingLegacyKeysAreRejected(): void { - $this->expectException(\Exception::class); - $this->expectExceptionMessage('mutually exclusive'); - $this->makeS3(['sse_c_key' => base64_encode(random_bytes(32)), 'sse_kms_enabled' => true]); + public function testLegacySseKmsEnabledAloneWarnsOnlyAboutDeprecation(): void { + $s3 = $this->makeS3(['sse_kms_enabled' => true]); + $this->assertSame(['ServerSideEncryption' => 'aws:kms'], $this->getServerSideEncryptionParameters($s3)); + + $warnings = $this->getEncryptionWarnings($s3); + $this->assertCount(1, $warnings); + $this->assertStringContainsString('sse_kms_enabled', $warnings[0]); + $this->assertStringContainsString('deprecated', $warnings[0]); } - public function testExplicitSseConflictingWithLegacySseCKeyIsRejected(): void { - $this->expectException(\Exception::class); - $this->makeS3(['sse' => 'sse-kms', 'sse_c_key' => base64_encode(random_bytes(32))]); + public function testBothLegacyKeysResolveToSseCByPrecedence(): void { + // Matches the Nextcloud 34 behaviour: SSE-C silently took precedence over SSE-KMS + // whenever both were configured. That precedence is kept, but is no longer silent. + $rawKey = random_bytes(32); + $s3 = $this->makeS3(['sse_c_key' => base64_encode($rawKey), 'sse_kms_enabled' => true]); + + $this->assertSame([ + 'SSECustomerAlgorithm' => 'AES256', + 'SSECustomerKey' => $rawKey, + 'SSECustomerKeyMD5' => md5($rawKey, true), + ], $this->getServerSideEncryptionParameters($s3)); + + $warnings = $this->getEncryptionWarnings($s3); + $this->assertGreaterThanOrEqual(1, count($warnings)); + $this->assertTrue( + (bool)array_filter($warnings, static fn (string $w): bool => str_contains($w, 'precedence')), + 'Expected a precedence warning, got: ' . implode(' | ', $warnings) + ); } - public function testExplicitSseConflictingWithLegacyKmsEnabledIsRejected(): void { - $this->expectException(\Exception::class); - $this->makeS3(['sse' => 'sse-c', 'sse_kms_enabled' => true]); + public function testExplicitSseKmsOverridesLegacySseCKey(): void { + // An explicit 'sse' always wins, even over the deprecated key that would otherwise + // take precedence under the Nextcloud 34 rule. + $s3 = $this->makeS3([ + 'sse' => 'sse-kms', + 'sse_c_key' => base64_encode(random_bytes(32)), + ]); + + $this->assertSame(['ServerSideEncryption' => 'aws:kms'], $this->getServerSideEncryptionParameters($s3)); + + $warnings = $this->getEncryptionWarnings($s3); + $this->assertCount(1, $warnings); + $this->assertStringContainsString('sse_c_key', $warnings[0]); + $this->assertStringContainsString('Ignoring', $warnings[0]); + } + + public function testExplicitSseCOverridesLegacyKmsEnabled(): void { + $rawKey = random_bytes(32); + $s3 = $this->makeS3([ + 'sse' => 'sse-c', + 'sse_c_key' => base64_encode($rawKey), + 'sse_kms_enabled' => true, + ]); + + $this->assertSame([ + 'SSECustomerAlgorithm' => 'AES256', + 'SSECustomerKey' => $rawKey, + 'SSECustomerKeyMD5' => md5($rawKey, true), + ], $this->getServerSideEncryptionParameters($s3)); + + $warnings = $this->getEncryptionWarnings($s3); + $ignoreWarnings = array_filter($warnings, static fn (string $w): bool => str_contains($w, 'sse_kms_enabled') && str_contains($w, 'Ignoring')); + $this->assertNotEmpty($ignoreWarnings, 'Expected an "ignoring sse_kms_enabled" warning, got: ' . implode(' | ', $warnings)); } public function testInvalidSseValueIsRejected(): void { @@ -165,14 +269,34 @@ public function testInvalidSseValueIsRejected(): void { $this->makeS3(['sse' => 'sse-does-not-exist']); } - public function testKmsKeyIdWithoutKmsModeIsRejected(): void { - $this->expectException(\Exception::class); - $this->makeS3(['sse' => 'sse-s3', 'sse_kms_key_id' => 'arn:aws:kms:us-east-1:123456789012:key/test-key']); + public function testKmsKeyIdWithoutKmsModeIsWarnedAndIgnored(): void { + $s3 = $this->makeS3(['sse' => 'sse-s3', 'sse_kms_key_id' => 'arn:aws:kms:us-east-1:123456789012:key/test-key']); + $this->assertSame(['ServerSideEncryption' => 'AES256'], $this->getServerSideEncryptionParameters($s3)); + + $warnings = $this->getEncryptionWarnings($s3); + $this->assertCount(1, $warnings); + $this->assertStringContainsString('sse_kms_key_id', $warnings[0]); } - public function testEncryptionContextWithoutKmsModeIsRejected(): void { - $this->expectException(\Exception::class); - $this->makeS3(['sse' => 'sse-c', 'sse_c_key' => base64_encode(random_bytes(32)), 'sse_kms_encryption_context' => ['tenant' => 'acme']]); + public function testEncryptionContextWithoutKmsModeIsWarnedAndIgnored(): void { + $rawKey = random_bytes(32); + $s3 = $this->makeS3([ + 'sse' => 'sse-c', + 'sse_c_key' => base64_encode($rawKey), + 'sse_kms_encryption_context' => ['tenant' => 'acme'], + ]); + + $this->assertSame([ + 'SSECustomerAlgorithm' => 'AES256', + 'SSECustomerKey' => $rawKey, + 'SSECustomerKeyMD5' => md5($rawKey, true), + ], $this->getServerSideEncryptionParameters($s3)); + + $warnings = $this->getEncryptionWarnings($s3); + $this->assertTrue( + (bool)array_filter($warnings, static fn (string $w): bool => str_contains($w, 'sse_kms_encryption_context')), + 'Expected a warning about sse_kms_encryption_context, got: ' . implode(' | ', $warnings) + ); } public function testEncryptionContextRejectsNonStringValues(): void { @@ -180,8 +304,81 @@ public function testEncryptionContextRejectsNonStringValues(): void { $this->makeS3(['sse' => 'sse-kms', 'sse_kms_encryption_context' => ['tenant' => 123]]); } - public function testBucketKeyWithoutSseKmsModeIsRejected(): void { + public function testMalformedEncryptionContextIsRejectedRegardlessOfMode(): void { + // A structurally invalid context can never be resolved, even for a mode where the + // context itself would otherwise just be redundant and warned-about. $this->expectException(\Exception::class); - $this->makeS3(['sse' => 'sse-s3', 'sse_kms_bucket_key' => true]); + $this->makeS3([ + 'sse' => 'sse-c', + 'sse_c_key' => base64_encode(random_bytes(32)), + 'sse_kms_encryption_context' => 'not-an-array', + ]); + } + + public function testBucketKeyWithoutSseKmsModeIsWarnedAndIgnored(): void { + $s3 = $this->makeS3(['sse' => 'sse-s3', 'sse_kms_bucket_key' => true]); + $this->assertSame(['ServerSideEncryption' => 'AES256'], $this->getServerSideEncryptionParameters($s3)); + + $warnings = $this->getEncryptionWarnings($s3); + $this->assertCount(1, $warnings); + $this->assertStringContainsString('sse_kms_bucket_key', $warnings[0]); + } + + /** + * Unlike the other tests here, this exercises getConnection() (with verify_bucket_exists + * disabled, so it makes no network call) to prove parseEncryptionParams()'s warnings + * actually reach the logger. + */ + public function testWarningsAreLoggedViaGetConnection(): void { + $rawKey = random_bytes(32); + $s3 = $this->makeS3([ + 'sse_c_key' => base64_encode($rawKey), + 'verify_bucket_exists' => false, + ]); + + $expectedWarnings = $this->getEncryptionWarnings($s3); + $this->assertNotEmpty($expectedWarnings); + + $loggedMessages = []; + $logger = $this->createMock(LoggerInterface::class); + $logger->method('warning') + ->willReturnCallback(function (string $message, array $context = []) use (&$loggedMessages): void { + $loggedMessages[] = $message; + }); + $this->overwriteService(LoggerInterface::class, $logger); + + $s3->getConnection(); + + $this->assertSame($expectedWarnings, $loggedMessages); + } + + /** + * The dedup is keyed on message text and static across instances (a long-lived + * request/worker constructs the primary object storage repeatedly), so a second, + * distinct S3 instance with the same 'sse*' configuration must not log again. + */ + public function testIdenticalWarningsAreNotLoggedTwiceAcrossInstances(): void { + $rawKey = random_bytes(32); + $makeIdenticallyConfiguredS3 = fn (): S3 => $this->makeS3([ + 'sse_c_key' => base64_encode($rawKey), + 'verify_bucket_exists' => false, + ]); + + $loggedMessages = []; + $logger = $this->createMock(LoggerInterface::class); + $logger->method('warning') + ->willReturnCallback(function (string $message, array $context = []) use (&$loggedMessages): void { + $loggedMessages[] = $message; + }); + $this->overwriteService(LoggerInterface::class, $logger); + + $first = $makeIdenticallyConfiguredS3(); + $expectedWarnings = $this->getEncryptionWarnings($first); + $first->getConnection(); + $this->assertSame($expectedWarnings, $loggedMessages, 'First instance should log its warnings'); + + $second = $makeIdenticallyConfiguredS3(); + $second->getConnection(); + $this->assertSame($expectedWarnings, $loggedMessages, 'Second instance with the same warnings should not log them again'); } } From 24a1cb78757638834c2f31f2af3d1a41a81dc288 Mon Sep 17 00:00:00 2001 From: Stephen Cuppett Date: Sat, 25 Jul 2026 11:04:42 -0500 Subject: [PATCH 3/4] fix(settings): correct and surface the S3 SSE-C deprecation properly The SSE-C deprecation warning added in the previous commit hedged that upstream hadn't announced a deprecation. That was wrong: SSE-C is deprecated since Nextcloud 34 because Amazon S3 disabled SSE-C by default for new buckets in April 2026 (independently reproduced against a live bucket). Corrects the warning text and a comment that had denied the deprecation, and adds the admin-facing signal that was missing: a SetupCheck surfaced in Settings > Overview, following the same version+reason phrasing convention used elsewhere for deprecated config values (e.g. FilenameValidator, PhpOutdated). Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Stephen Cuppett --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/settings/lib/AppInfo/Application.php | 2 + .../lib/SetupChecks/S3SseCEncryption.php | 76 +++++++++++++++++++ .../SetupChecks/S3SseCEncryptionTest.php | 74 ++++++++++++++++++ config/config.sample.php | 6 +- .../Files/ObjectStore/S3ConnectionTrait.php | 9 ++- 7 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 apps/settings/lib/SetupChecks/S3SseCEncryption.php create mode 100644 apps/settings/tests/SetupChecks/S3SseCEncryptionTest.php diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index 618e4cff3c619..4fc223b850a7c 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -132,6 +132,7 @@ 'OCA\\Settings\\SetupChecks\\RandomnessSecure' => $baseDir . '/../lib/SetupChecks/RandomnessSecure.php', 'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => $baseDir . '/../lib/SetupChecks/ReadOnlyConfig.php', 'OCA\\Settings\\SetupChecks\\RepairSanitizeSystemTagsAvailable' => $baseDir . '/../lib/SetupChecks/RepairSanitizeSystemTagsAvailable.php', + 'OCA\\Settings\\SetupChecks\\S3SseCEncryption' => $baseDir . '/../lib/SetupChecks/S3SseCEncryption.php', 'OCA\\Settings\\SetupChecks\\SchedulingTableSize' => $baseDir . '/../lib/SetupChecks/SchedulingTableSize.php', 'OCA\\Settings\\SetupChecks\\SecurityHeaders' => $baseDir . '/../lib/SetupChecks/SecurityHeaders.php', 'OCA\\Settings\\SetupChecks\\ServerIdConfig' => $baseDir . '/../lib/SetupChecks/ServerIdConfig.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index 9fdba0484599c..2341bea6d30ef 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -147,6 +147,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\RandomnessSecure' => __DIR__ . '/..' . '/../lib/SetupChecks/RandomnessSecure.php', 'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => __DIR__ . '/..' . '/../lib/SetupChecks/ReadOnlyConfig.php', 'OCA\\Settings\\SetupChecks\\RepairSanitizeSystemTagsAvailable' => __DIR__ . '/..' . '/../lib/SetupChecks/RepairSanitizeSystemTagsAvailable.php', + 'OCA\\Settings\\SetupChecks\\S3SseCEncryption' => __DIR__ . '/..' . '/../lib/SetupChecks/S3SseCEncryption.php', 'OCA\\Settings\\SetupChecks\\SchedulingTableSize' => __DIR__ . '/..' . '/../lib/SetupChecks/SchedulingTableSize.php', 'OCA\\Settings\\SetupChecks\\SecurityHeaders' => __DIR__ . '/..' . '/../lib/SetupChecks/SecurityHeaders.php', 'OCA\\Settings\\SetupChecks\\ServerIdConfig' => __DIR__ . '/..' . '/../lib/SetupChecks/ServerIdConfig.php', diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index 5faf7b5110b0c..7ae86ade5d8dd 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -69,6 +69,7 @@ use OCA\Settings\SetupChecks\RandomnessSecure; use OCA\Settings\SetupChecks\ReadOnlyConfig; use OCA\Settings\SetupChecks\RepairSanitizeSystemTagsAvailable; +use OCA\Settings\SetupChecks\S3SseCEncryption; use OCA\Settings\SetupChecks\SchedulingTableSize; use OCA\Settings\SetupChecks\SecurityHeaders; use OCA\Settings\SetupChecks\ServerIdConfig; @@ -212,6 +213,7 @@ public function register(IRegistrationContext $context): void { $context->registerSetupCheck(RandomnessSecure::class); $context->registerSetupCheck(ReadOnlyConfig::class); $context->registerSetupCheck(RepairSanitizeSystemTagsAvailable::class); + $context->registerSetupCheck(S3SseCEncryption::class); $context->registerSetupCheck(SecurityHeaders::class); $context->registerSetupCheck(ServerIdConfig::class); $context->registerSetupCheck(SchedulingTableSize::class); diff --git a/apps/settings/lib/SetupChecks/S3SseCEncryption.php b/apps/settings/lib/SetupChecks/S3SseCEncryption.php new file mode 100644 index 0000000000000..f4541d4f8dcdd --- /dev/null +++ b/apps/settings/lib/SetupChecks/S3SseCEncryption.php @@ -0,0 +1,76 @@ +l10n->t('S3 SSE-C encryption'); + } + + #[\Override] + public function getCategory(): string { + return 'security'; + } + + /** + * @param array $arguments the 'arguments' of an 'objectstore'/'objectstore_multibucket' config + */ + private function isSseCInEffect(array $arguments): bool { + $sse = $arguments['sse'] ?? ''; + if ($sse !== '') { + return $sse === 'sse-c'; + } + + // No explicit 'sse' argument: matches the precedence in + // OC\Files\ObjectStore\S3ConnectionTrait::parseEncryptionParams() -- the legacy + // 'sse_c_key' argument wins over 'sse_kms_enabled' whenever both are set. + return !empty($arguments['sse_c_key']); + } + + #[\Override] + public function run(): SetupResult { + foreach (['objectstore', 'objectstore_multibucket'] as $key) { + $objectStore = $this->config->getSystemValue($key, null); + if (!is_array($objectStore) || ($objectStore['class'] ?? null) !== 'OC\\Files\\ObjectStore\\S3') { + continue; + } + + if ($this->isSseCInEffect($objectStore['arguments'] ?? [])) { + return SetupResult::warning( + $this->l10n->t('This instance uses S3 SSE-C (customer-provided key) encryption, deprecated since Nextcloud 34: Amazon S3 disabled SSE-C by default for new buckets in April 2026. Migrate to SSE-KMS instead.'), + $this->urlGenerator->linkToDocs('admin-s3-sse-c'), + ); + } + } + + return SetupResult::success($this->l10n->t('No deprecated S3 SSE-C encryption configuration found.')); + } +} diff --git a/apps/settings/tests/SetupChecks/S3SseCEncryptionTest.php b/apps/settings/tests/SetupChecks/S3SseCEncryptionTest.php new file mode 100644 index 0000000000000..aa35fee03386e --- /dev/null +++ b/apps/settings/tests/SetupChecks/S3SseCEncryptionTest.php @@ -0,0 +1,74 @@ +l10n = $this->createMock(IL10N::class); + $this->l10n->method('t') + ->willReturnCallback(function ($message, array $replace = []) { + return vsprintf($message, $replace); + }); + $this->config = $this->createMock(IConfig::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + } + + public static function dataRun(): array { + $s3 = static fn (array $arguments): array => ['class' => 'OC\\Files\\ObjectStore\\S3', 'arguments' => $arguments]; + + return [ + 'no objectstore configured' => [null, null, SetupResult::SUCCESS], + 'non-S3 objectstore class' => [['class' => 'OC\\Files\\ObjectStore\\Swift', 'arguments' => ['sse_c_key' => 'x']], null, SetupResult::SUCCESS], + 'S3 with no sse* arguments' => [$s3([]), null, SetupResult::SUCCESS], + 'S3 with sse-kms' => [$s3(['sse' => 'sse-kms']), null, SetupResult::SUCCESS], + 'S3 with sse-kms-dsse' => [$s3(['sse' => 'sse-kms-dsse']), null, SetupResult::SUCCESS], + 'S3 with legacy sse_c_key only' => [$s3(['sse_c_key' => 'x']), null, SetupResult::WARNING], + 'S3 with explicit sse => sse-c' => [$s3(['sse' => 'sse-c', 'sse_c_key' => 'x']), null, SetupResult::WARNING], + 'S3 with both legacy keys set (sse_c_key wins by precedence)' => [$s3(['sse_c_key' => 'x', 'sse_kms_enabled' => true]), null, SetupResult::WARNING], + 'S3 with explicit sse overriding legacy sse_c_key' => [$s3(['sse' => 'sse-kms', 'sse_c_key' => 'x']), null, SetupResult::SUCCESS], + 'multibucket S3 with legacy sse_c_key only' => [null, $s3(['sse_c_key' => 'x']), SetupResult::WARNING], + 'multibucket S3 with sse-kms' => [null, $s3(['sse' => 'sse-kms']), SetupResult::SUCCESS], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('dataRun')] + public function testRun(?array $objectStore, ?array $objectStoreMultibucket, string $expected): void { + $this->urlGenerator->method('linkToDocs')->willReturn('admin-s3-sse-c'); + + $this->config->method('getSystemValue') + ->willReturnCallback(function (string $key, $default = null) use ($objectStore, $objectStoreMultibucket) { + return match ($key) { + 'objectstore' => $objectStore ?? $default, + 'objectstore_multibucket' => $objectStoreMultibucket ?? $default, + default => $default, + }; + }); + + $check = new S3SseCEncryption($this->l10n, $this->config, $this->urlGenerator); + + $result = $check->run(); + $this->assertEquals($expected, $result->getSeverity()); + } +} diff --git a/config/config.sample.php b/config/config.sample.php index a7cfbc3d6a35a..3df4eeacd29ae 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -2120,14 +2120,16 @@ // Valid values are: // - '' (default): no server-side encryption requested by Nextcloud // - 'sse-s3': S3-managed AES256 key - // - 'sse-c': customer-provided key, see 'sse_c_key' below + // - 'sse-c': customer-provided key, see 'sse_c_key' below (deprecated, see below) // - 'sse-kms': single layer of AWS KMS-managed encryption // - 'sse-kms-dsse': dual layer of AWS KMS-managed encryption (DSSE-KMS), higher cost // and latency than 'sse-kms' but required by some compliance standards // https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingKMSEncryption.html // https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingDSSEncryption.html 'sse' => '', - // optional: customer-provided key for 'sse' => 'sse-c', base64 encoded + // optional: customer-provided key for 'sse' => 'sse-c', base64 encoded. + // Deprecated since Nextcloud 34: Amazon S3 disabled SSE-C by default for new + // buckets in April 2026. Use 'sse' => 'sse-kms' or 'sse-kms-dsse' instead. 'sse_c_key' => '', // optional: ARN of the AWS KMS key to use for 'sse-kms' / 'sse-kms-dsse'. // If omitted, the bucket's default KMS key (or the AWS-managed 'aws/s3' key) is used. diff --git a/lib/private/Files/ObjectStore/S3ConnectionTrait.php b/lib/private/Files/ObjectStore/S3ConnectionTrait.php index e358973db2da8..bda7e1de4b5f7 100644 --- a/lib/private/Files/ObjectStore/S3ConnectionTrait.php +++ b/lib/private/Files/ObjectStore/S3ConnectionTrait.php @@ -397,9 +397,12 @@ private function parseEncryptionParams(array $params): void { } if ($mode === S3EncryptionMode::SseC) { - // SSE-C is a supported mode, not merely a deprecated way to reach one, so this - // warning is intentionally emitted regardless of how 'sse-c' was selected. - $this->encryptionWarnings[] = "SSE-C ('sse' => 'sse-c') requires you to manage and distribute the encryption key yourself. Consider 'sse' => 'sse-kms' or 'sse-kms-dsse' instead, which let AWS KMS manage the key."; + // SSE-C itself is deprecated since Nextcloud 34, not just the legacy 'sse_c_key' + // argument shape, so this warning is emitted regardless of how 'sse-c' was + // selected. Amazon S3 disabled SSE-C by default for new buckets in April 2026 + // (https://aws.amazon.com/blogs/storage/advanced-notice-amazon-s3-to-disable-the-use-of-sse-c-encryption-by-default-for-all-new-buckets-and-select-existing-buckets-in-april-2026/), + // which is why this also requires managing and distributing the key yourself. + $this->encryptionWarnings[] = "SSE-C ('sse' => 'sse-c') is deprecated since Nextcloud 34: Amazon S3 disabled SSE-C by default for new buckets in April 2026, and it requires you to manage the encryption key yourself. Use 'sse' => 'sse-kms' or 'sse-kms-dsse' instead, which let AWS KMS manage the key."; if (empty($params['sse_c_key'])) { // Unlike a merely redundant option, this cannot be resolved: silently falling // back to no encryption would defeat an explicit request for SSE-C. From 7d0a9008e5696c4861be5303ea062d1f789a7043 Mon Sep 17 00:00:00 2001 From: Stephen Cuppett Date: Sat, 25 Jul 2026 11:44:28 -0500 Subject: [PATCH 4/4] fix(autoload): Autoloader update Signed-off-by: Stephen Cuppett --- apps/dav/composer/composer/autoload_static.php | 4 ++-- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index fad494f1c6fe6..5aa979e007acc 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -7,14 +7,14 @@ class ComposerStaticInitDAV { public static $prefixLengthsPsr4 = array ( - 'O' => + 'O' => array ( 'OCA\\DAV\\' => 8, ), ); public static $prefixDirsPsr4 = array ( - 'OCA\\DAV\\' => + 'OCA\\DAV\\' => array ( 0 => __DIR__ . '/..' . '/../lib', ), diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index beafb4700c69d..a6753dd275f42 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1884,6 +1884,7 @@ 'OC\\Files\\ObjectStore\\S3' => $baseDir . '/lib/private/Files/ObjectStore/S3.php', 'OC\\Files\\ObjectStore\\S3ConfigTrait' => $baseDir . '/lib/private/Files/ObjectStore/S3ConfigTrait.php', 'OC\\Files\\ObjectStore\\S3ConnectionTrait' => $baseDir . '/lib/private/Files/ObjectStore/S3ConnectionTrait.php', + 'OC\\Files\\ObjectStore\\S3EncryptionMode' => $baseDir . '/lib/private/Files/ObjectStore/S3EncryptionMode.php', 'OC\\Files\\ObjectStore\\S3ObjectTrait' => $baseDir . '/lib/private/Files/ObjectStore/S3ObjectTrait.php', 'OC\\Files\\ObjectStore\\S3Signature' => $baseDir . '/lib/private/Files/ObjectStore/S3Signature.php', 'OC\\Files\\ObjectStore\\StorageObjectStore' => $baseDir . '/lib/private/Files/ObjectStore/StorageObjectStore.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index e024c1fb44b9f..5ec94b1ab7ca5 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1925,6 +1925,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Files\\ObjectStore\\S3' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/S3.php', 'OC\\Files\\ObjectStore\\S3ConfigTrait' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/S3ConfigTrait.php', 'OC\\Files\\ObjectStore\\S3ConnectionTrait' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/S3ConnectionTrait.php', + 'OC\\Files\\ObjectStore\\S3EncryptionMode' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/S3EncryptionMode.php', 'OC\\Files\\ObjectStore\\S3ObjectTrait' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/S3ObjectTrait.php', 'OC\\Files\\ObjectStore\\S3Signature' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/S3Signature.php', 'OC\\Files\\ObjectStore\\StorageObjectStore' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/StorageObjectStore.php',