Skip to content

Commit 9e5a6ff

Browse files
cuppettclaude
andcommitted
test(objectstore): verify S3 encryption headers on the wire and probe provider support
Add an offline test that drives S3 against an AWS SDK MockHandler and asserts the server-side encryption headers each request actually sends, for every sse mode. The SDK only serializes members an operation defines, so this checks serialized requests rather than parameter arrays. Add a capability probe for the live tests. It checks whether the provider accepts and echoes each ServerSideEncryption value, so providers without SSE-S3, SSE-KMS or DSSE-KMS (e.g. MinIO, Ceph) skip with the provider's error instead of failing. Tighten existing assertions to exact warning counts and exception messages, add coverage for the new sensitive config keys and for edge cases in sse parsing, remove duplicate tests, and make S3SSEKMSTest resolve legacy key precedence the same way production does. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Stephen Cuppett <steve@cuppett.com>
1 parent dae5715 commit 9e5a6ff

7 files changed

Lines changed: 561 additions & 83 deletions

File tree

‎apps/settings/tests/SetupChecks/S3SseCEncryptionTest.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ public static function dataRun(): array {
4343
'non-S3 objectstore class' => [['class' => 'OC\\Files\\ObjectStore\\Swift', 'arguments' => ['sse_c_key' => 'x']], null, SetupResult::SUCCESS],
4444
'S3 with no sse* arguments' => [$s3([]), null, SetupResult::SUCCESS],
4545
'S3 with sse-kms' => [$s3(['sse' => 'sse-kms']), null, SetupResult::SUCCESS],
46-
'S3 with sse-kms-dsse' => [$s3(['sse' => 'sse-kms-dsse']), null, SetupResult::SUCCESS],
4746
'S3 with legacy sse_c_key only' => [$s3(['sse_c_key' => 'x']), null, SetupResult::WARNING],
4847
'S3 with explicit sse => sse-c' => [$s3(['sse' => 'sse-c', 'sse_c_key' => 'x']), null, SetupResult::WARNING],
4948
'S3 with both legacy keys set (sse_c_key wins by precedence)' => [$s3(['sse_c_key' => 'x', 'sse_kms_enabled' => true]), null, SetupResult::WARNING],
@@ -61,7 +60,9 @@ public static function dataRun(): array {
6160

6261
#[\PHPUnit\Framework\Attributes\DataProvider('dataRun')]
6362
public function testRun(?array $objectStore, ?array $objectStoreMultibucket, string $expected): void {
64-
$this->urlGenerator->method('linkToDocs')->willReturn('admin-s3-sse-c');
63+
$this->urlGenerator->method('linkToDocs')
64+
->with('admin-s3-sse-c')
65+
->willReturn('https://docs.example.com/admin-s3-sse-c');
6566

6667
$this->config->method('getSystemValue')
6768
->willReturnCallback(function (string $key, $default = null) use ($objectStore, $objectStoreMultibucket) {
@@ -76,5 +77,11 @@ public function testRun(?array $objectStore, ?array $objectStoreMultibucket, str
7677

7778
$result = $check->run();
7879
$this->assertEquals($expected, $result->getSeverity());
80+
if ($expected === SetupResult::WARNING) {
81+
$this->assertSame('https://docs.example.com/admin-s3-sse-c', $result->getLinkToDoc());
82+
$this->assertStringContainsString('Migrate to SSE-KMS', $result->getDescription());
83+
} else {
84+
$this->assertNull($result->getLinkToDoc());
85+
}
7986
}
8087
}
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace Test\Files\ObjectStore;
11+
12+
use Aws\S3\Exception\S3Exception;
13+
use OC\Files\ObjectStore\S3;
14+
15+
/**
16+
* Detects which `ServerSideEncryption` values the configured S3 provider actually supports,
17+
* so live tests skip on providers without them (e.g. MinIO without KES, or anything other
18+
* than AWS for DSSE-KMS) instead of failing.
19+
*
20+
* Support is probed with a real PutObject + HeadObject rather than inferred from the
21+
* endpoint: the value counts as supported only if the provider accepts it and echoes it
22+
* back. Results are cached per test class.
23+
*/
24+
trait S3EncryptionCapabilityTrait {
25+
/** @var array<string, ?string> ServerSideEncryption value => null if supported, otherwise the reason it is not */
26+
private static array $encryptionSupport = [];
27+
28+
/**
29+
* @param S3 $s3 instance for the bucket under test; its own `sse*` configuration is not used
30+
* @param string $serverSideEncryption 'AES256', 'aws:kms' or 'aws:kms:dsse'
31+
* @param ?string $kmsKeyId KMS key for the 'aws:kms*' values, or null for the bucket default
32+
* @return ?string null if supported, otherwise why not
33+
*/
34+
protected static function getEncryptionUnsupportedReason(S3 $s3, string $serverSideEncryption, ?string $kmsKeyId = null): ?string {
35+
if (array_key_exists($serverSideEncryption, self::$encryptionSupport)) {
36+
return self::$encryptionSupport[$serverSideEncryption];
37+
}
38+
39+
$connection = $s3->getConnection();
40+
$urn = 'sse-capability-probe-' . bin2hex(random_bytes(8));
41+
$params = ['Bucket' => $s3->getBucket(), 'Key' => $urn, 'Body' => 'probe', 'ServerSideEncryption' => $serverSideEncryption];
42+
if ($kmsKeyId !== null && str_starts_with($serverSideEncryption, 'aws:kms')) {
43+
$params['SSEKMSKeyId'] = $kmsKeyId;
44+
}
45+
46+
try {
47+
$connection->putObject($params);
48+
$echoed = $connection->headObject(['Bucket' => $s3->getBucket(), 'Key' => $urn])->get('ServerSideEncryption');
49+
$reason = $echoed === $serverSideEncryption
50+
? null
51+
: "the provider accepted ServerSideEncryption '$serverSideEncryption' but reports '" . ($echoed ?? 'none') . "'";
52+
} catch (S3Exception $e) {
53+
// 501 is how MinIO reports SSE without a configured KMS; any other 5xx is an outage
54+
$status = (int)$e->getStatusCode();
55+
if ($status >= 500 && $status !== 501) {
56+
throw $e;
57+
}
58+
$reason = "the provider rejected ServerSideEncryption '$serverSideEncryption' (HTTP $status " . $e->getAwsErrorCode() . ': ' . $e->getAwsErrorMessage() . ')';
59+
} finally {
60+
try {
61+
$connection->deleteObject(['Bucket' => $s3->getBucket(), 'Key' => $urn]);
62+
} catch (S3Exception) {
63+
// never written
64+
}
65+
}
66+
67+
return self::$encryptionSupport[$serverSideEncryption] = $reason;
68+
}
69+
70+
/**
71+
* Skip the current test unless the provider supports $serverSideEncryption.
72+
*/
73+
protected static function requireEncryptionSupport(S3 $s3, string $serverSideEncryption, ?string $kmsKeyId = null): void {
74+
$reason = self::getEncryptionUnsupportedReason($s3, $serverSideEncryption, $kmsKeyId);
75+
if ($reason !== null) {
76+
self::markTestSkipped("S3 provider does not support '$serverSideEncryption': $reason");
77+
}
78+
}
79+
}

0 commit comments

Comments
 (0)