From cc4eaf5d156870a1c58b56b846cf732a63567d01 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Sat, 31 Oct 2026 18:21:57 +0100 Subject: [PATCH] feat(files): presigned part uploads to S3 object storage A client (for example an importer) can now upload a file's content straight into the S3 bucket behind Nextcloud's object storage, keeping PHP out of the data path: the server starts a multipart upload, hands out one presigned request per part, and lists and completes the upload as before. Every part is signed with its SHA-256, so S3 refuses bytes that don't match it, and the bucket's credentials never leave the server. Configurations that would need a secret on the client (SSE-C keys) or can't sign a checksum (V2 signatures) say so. IObjectStoreStorage names the object store and the object of a file for callers outside the object store code, through storage wrappers. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/composer/composer/autoload_classmap.php | 2 + lib/composer/composer/autoload_static.php | 2 + .../Files/ObjectStore/ObjectStoreStorage.php | 5 +- lib/private/Files/ObjectStore/S3.php | 92 +++++++++++++++++- .../IObjectStorePresignedUpload.php | 72 ++++++++++++++ .../Files/ObjectStore/IObjectStoreStorage.php | 35 +++++++ .../ObjectStore/S3PresignedUploadTest.php | 96 +++++++++++++++++++ tests/lib/Files/ObjectStore/S3Test.php | 36 +++++++ 8 files changed, 338 insertions(+), 2 deletions(-) create mode 100644 lib/public/Files/ObjectStore/IObjectStorePresignedUpload.php create mode 100644 lib/public/Files/ObjectStore/IObjectStoreStorage.php create mode 100644 tests/lib/Files/ObjectStore/S3PresignedUploadTest.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 358118e8d059e..876372265370d 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -596,6 +596,8 @@ 'OCP\\Files\\ObjectStore\\IObjectStore' => $baseDir . '/lib/public/Files/ObjectStore/IObjectStore.php', 'OCP\\Files\\ObjectStore\\IObjectStoreMetaData' => $baseDir . '/lib/public/Files/ObjectStore/IObjectStoreMetaData.php', 'OCP\\Files\\ObjectStore\\IObjectStoreMultiPartUpload' => $baseDir . '/lib/public/Files/ObjectStore/IObjectStoreMultiPartUpload.php', + 'OCP\\Files\\ObjectStore\\IObjectStorePresignedUpload' => $baseDir . '/lib/public/Files/ObjectStore/IObjectStorePresignedUpload.php', + 'OCP\\Files\\ObjectStore\\IObjectStoreStorage' => $baseDir . '/lib/public/Files/ObjectStore/IObjectStoreStorage.php', 'OCP\\Files\\ReservedWordException' => $baseDir . '/lib/public/Files/ReservedWordException.php', 'OCP\\Files\\Search\\ISearchBinaryOperator' => $baseDir . '/lib/public/Files/Search/ISearchBinaryOperator.php', 'OCP\\Files\\Search\\ISearchComparison' => $baseDir . '/lib/public/Files/Search/ISearchComparison.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 5caaf52478f18..0d3b6eeb95f75 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -637,6 +637,8 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\Files\\ObjectStore\\IObjectStore' => __DIR__ . '/../../..' . '/lib/public/Files/ObjectStore/IObjectStore.php', 'OCP\\Files\\ObjectStore\\IObjectStoreMetaData' => __DIR__ . '/../../..' . '/lib/public/Files/ObjectStore/IObjectStoreMetaData.php', 'OCP\\Files\\ObjectStore\\IObjectStoreMultiPartUpload' => __DIR__ . '/../../..' . '/lib/public/Files/ObjectStore/IObjectStoreMultiPartUpload.php', + 'OCP\\Files\\ObjectStore\\IObjectStorePresignedUpload' => __DIR__ . '/../../..' . '/lib/public/Files/ObjectStore/IObjectStorePresignedUpload.php', + 'OCP\\Files\\ObjectStore\\IObjectStoreStorage' => __DIR__ . '/../../..' . '/lib/public/Files/ObjectStore/IObjectStoreStorage.php', 'OCP\\Files\\ReservedWordException' => __DIR__ . '/../../..' . '/lib/public/Files/ReservedWordException.php', 'OCP\\Files\\Search\\ISearchBinaryOperator' => __DIR__ . '/../../..' . '/lib/public/Files/Search/ISearchBinaryOperator.php', 'OCP\\Files\\Search\\ISearchComparison' => __DIR__ . '/../../..' . '/lib/public/Files/Search/ISearchComparison.php', diff --git a/lib/private/Files/ObjectStore/ObjectStoreStorage.php b/lib/private/Files/ObjectStore/ObjectStoreStorage.php index 536adf76f87c3..20f1f750d54ff 100644 --- a/lib/private/Files/ObjectStore/ObjectStoreStorage.php +++ b/lib/private/Files/ObjectStore/ObjectStoreStorage.php @@ -30,6 +30,7 @@ use OCP\Files\ObjectStore\IObjectStore; use OCP\Files\ObjectStore\IObjectStoreMetaData; use OCP\Files\ObjectStore\IObjectStoreMultiPartUpload; +use OCP\Files\ObjectStore\IObjectStoreStorage; use OCP\Files\Storage\IChunkedFileWrite; use OCP\Files\Storage\IStorage; use OCP\IDBConnection; @@ -38,7 +39,7 @@ use Override; use Psr\Log\LoggerInterface; -class ObjectStoreStorage extends Common implements IChunkedFileWrite { +class ObjectStoreStorage extends Common implements IChunkedFileWrite, IObjectStoreStorage { use CopyDirectory; protected IObjectStore $objectStore; @@ -270,6 +271,7 @@ public function getPermissions(string $path): int { * * @return string the unified resource name used to identify the object */ + #[\Override] public function getURN(int $fileId): string { return $this->objectPrefix . $fileId; } @@ -604,6 +606,7 @@ public function writeStream(string $path, $stream, ?int $size = null): int { return $totalWritten; } + #[\Override] public function getObjectStore(): IObjectStore { return $this->objectStore; } diff --git a/lib/private/Files/ObjectStore/S3.php b/lib/private/Files/ObjectStore/S3.php index eeacfa3fca620..50b5a34c1a014 100644 --- a/lib/private/Files/ObjectStore/S3.php +++ b/lib/private/Files/ObjectStore/S3.php @@ -12,8 +12,13 @@ use OCP\Files\ObjectStore\IObjectStore; use OCP\Files\ObjectStore\IObjectStoreMetaData; use OCP\Files\ObjectStore\IObjectStoreMultiPartUpload; +use OCP\Files\ObjectStore\IObjectStorePresignedUpload; + +class S3 implements IObjectStore, IObjectStoreMultiPartUpload, IObjectStorePresignedUpload, IObjectStoreMetaData { + /** S3's limits for one part of a multipart upload. */ + private const MAX_PART_NUMBER = 10000; + private const MAX_PART_SIZE = 5 * 1024 * 1024 * 1024; -class S3 implements IObjectStore, IObjectStoreMultiPartUpload, IObjectStoreMetaData { use S3ConnectionTrait; use S3ObjectTrait; @@ -102,6 +107,91 @@ public function abortMultipartUpload($urn, $uploadId): void { ]); } + #[\Override] + public function canPreSignUploads(): bool { + // A customer-provided key would have to be sent with every part, and + // legacy (V2) signatures can't sign a checksum. + return $this->getSSECKey() === null && empty($this->params['legacy_auth']); + } + + #[\Override] + public function initiatePreSignedMultipartUpload(string $urn, ?string $mimetype = null): string { + if (!$this->canPreSignUploads()) { + throw new Exception('This object store configuration can\'t take presigned uploads'); + } + $args = [ + 'Bucket' => $this->bucket, + 'Key' => $urn, + 'ChecksumAlgorithm' => 'SHA256', + 'StorageClass' => $this->storageClass, + ] + $this->getServerSideEncryptionParameters(); + if ($mimetype !== null && $mimetype !== '') { + $args['ContentType'] = $mimetype; + } + $uploadId = $this->getConnection()->createMultipartUpload($args)->get('UploadId'); + if (!is_string($uploadId) || $uploadId === '') { + throw new Exception('No upload id returned'); + } + return $uploadId; + } + + #[\Override] + public function preSignUploadPart(string $urn, string $uploadId, int $partNumber, int $size, string $sha256, \DateTimeInterface $expiration): array { + if ($partNumber < 1 || $partNumber > self::MAX_PART_NUMBER) { + throw new \InvalidArgumentException('The part number must be between 1 and ' . self::MAX_PART_NUMBER); + } + if ($size < 0 || $size > self::MAX_PART_SIZE) { + throw new \InvalidArgumentException('A part has at most ' . self::MAX_PART_SIZE . ' bytes'); + } + if (preg_match('/^[0-9a-f]{64}$/', $sha256) !== 1) { + throw new \InvalidArgumentException('The SHA-256 must be lowercase hex'); + } + if (!$this->canPreSignUploads()) { + throw new Exception('This object store configuration can\'t take presigned uploads'); + } + $command = $this->getConnection()->getCommand('UploadPart', [ + 'Bucket' => $this->bucket, + 'Key' => $urn, + 'UploadId' => $uploadId, + 'PartNumber' => $partNumber, + 'ContentLength' => $size, + 'ChecksumSHA256' => base64_encode((string)hex2bin($sha256)), + ]); + $request = $this->getConnection()->createPresignedRequest($command, $expiration); + + // The client has to send exactly the headers the signature covers; + // its HTTP library sets the host from the URL. + parse_str($request->getUri()->getQuery(), $query); + $signed = is_string($query['X-Amz-SignedHeaders'] ?? null) ? explode(';', $query['X-Amz-SignedHeaders']) : []; + $headers = []; + foreach ($signed as $name) { + if ($name !== 'host' && $request->hasHeader($name)) { + $headers[$name] = $request->getHeaderLine($name); + } + } + return ['method' => 'PUT', 'url' => (string)$request->getUri(), 'headers' => $headers]; + } + + #[\Override] + public function getPreSignedUploadParts(string $urn, string $uploadId): array { + $parts = []; + foreach ($this->getMultipartUploads($urn, $uploadId) as $part) { + $checksum = $part['ChecksumSHA256'] ?? null; + $sha256 = null; + if (is_string($checksum) && $checksum !== '') { + $raw = base64_decode($checksum, true); + $sha256 = ($raw !== false && strlen($raw) === 32) ? bin2hex($raw) : null; + } + $parts[] = [ + 'number' => (int)$part['PartNumber'], + 'size' => (int)$part['Size'], + 'sha256' => $sha256, + ]; + } + usort($parts, static fn (array $a, array $b): int => $a['number'] <=> $b['number']); + return $parts; + } + private function parseS3Metadata(array $metadata): array { $result = []; foreach ($metadata as $key => $value) { diff --git a/lib/public/Files/ObjectStore/IObjectStorePresignedUpload.php b/lib/public/Files/ObjectStore/IObjectStorePresignedUpload.php new file mode 100644 index 0000000000000..1fb64fe81ae6d --- /dev/null +++ b/lib/public/Files/ObjectStore/IObjectStorePresignedUpload.php @@ -0,0 +1,72 @@ +} + * @throws \InvalidArgumentException for a part number, size or hash out of range + * @throws \Exception when the request can't be signed + * @since 36.0.0 + */ + public function preSignUploadPart(string $urn, string $uploadId, int $partNumber, int $size, string $sha256, \DateTimeInterface $expiration): array; + + /** + * The parts the store holds for the upload, ordered by number, with the + * SHA-256 it checked (null when the store kept none). + * + * @return list + * @throws \Exception when the upload doesn't exist + * @since 36.0.0 + */ + public function getPreSignedUploadParts(string $urn, string $uploadId): array; +} diff --git a/lib/public/Files/ObjectStore/IObjectStoreStorage.php b/lib/public/Files/ObjectStore/IObjectStoreStorage.php new file mode 100644 index 0000000000000..e645e4946bccb --- /dev/null +++ b/lib/public/Files/ObjectStore/IObjectStoreStorage.php @@ -0,0 +1,35 @@ + 'nextcloud', + 'hostname' => 's3.example.com', + 'port' => 443, + 'region' => 'eu-central-1', + 'key' => 'AKIDEXAMPLE', + 'secret' => 'wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY', + 'use_path_style' => true, + 'verify_bucket_exists' => false, + ]); + } + + public function testPresignsOnePartWithItsChecksum(): void { + $expires = new \DateTimeImmutable('+15 minutes'); + $request = $this->store()->preSignUploadPart('urn:oid:42', 'upload-1', 3, 5242880, self::SHA256, $expires); + + $this->assertSame('PUT', $request['method']); + $this->assertSame(['x-amz-checksum-sha256' => base64_encode(hex2bin(self::SHA256))], $request['headers']); + $url = parse_url($request['url']); + $this->assertSame('https', $url['scheme']); + $this->assertSame('s3.example.com', $url['host']); + $this->assertSame('/nextcloud/urn%3Aoid%3A42', $url['path']); + parse_str($url['query'], $query); + $this->assertSame('upload-1', $query['uploadId']); + $this->assertSame('3', $query['partNumber']); + $this->assertSame('900', $query['X-Amz-Expires']); + $this->assertContains('x-amz-checksum-sha256', explode(';', $query['X-Amz-SignedHeaders'])); + $this->assertMatchesRegularExpression('/^[0-9a-f]{64}$/', $query['X-Amz-Signature']); + $this->assertStringNotContainsString('wJalrXUtnFEMI', $request['url']); + } + + public static function invalidParts(): array { + return [ + 'part 0' => [0, 1, self::SHA256], + 'part 10001' => [10001, 1, self::SHA256], + 'negative size' => [1, -1, self::SHA256], + 'larger than 5 GiB' => [1, 5 * 1024 * 1024 * 1024 + 1, self::SHA256], + 'uppercase hash' => [1, 1, strtoupper(self::SHA256)], + 'short hash' => [1, 1, 'abc'], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('invalidParts')] + public function testRefusesPartsOutOfRange(int $number, int $size, string $sha256): void { + $this->expectException(\InvalidArgumentException::class); + $this->store()->preSignUploadPart('urn:oid:42', 'upload-1', $number, $size, $sha256, new \DateTimeImmutable('+15 minutes')); + } + + public function testCanPreSignOnlyWithoutCustomerKeysAndWithV4Signatures(): void { + $this->assertTrue($this->store()->canPreSignUploads()); + $this->assertTrue($this->store(['sse_kms_enabled' => true])->canPreSignUploads()); + $this->assertFalse($this->store(['sse_c_key' => base64_encode(random_bytes(32))])->canPreSignUploads()); + $this->assertFalse($this->store(['legacy_auth' => true])->canPreSignUploads()); + } + + public function testRefusesToPresignWithACustomerKey(): void { + $this->expectException(\Exception::class); + $this->store(['sse_c_key' => base64_encode(random_bytes(32))]) + ->preSignUploadPart('urn:oid:42', 'upload-1', 1, 1, self::SHA256, new \DateTimeImmutable('+15 minutes')); + } + + public function testObjectStoreStorageNamesItsObjects(): void { + $objectStore = new StorageObjectStore(new Temporary([])); + $storage = new ObjectStoreStorage(['objectstore' => $objectStore, 'objectPrefix' => 'urn:test:']); + + $this->assertInstanceOf(IObjectStoreStorage::class, $storage); + $this->assertTrue($storage->instanceOfStorage(IObjectStoreStorage::class)); + $this->assertSame($objectStore, $storage->getObjectStore()); + $this->assertSame('urn:test:7', $storage->getURN(7)); + } +} diff --git a/tests/lib/Files/ObjectStore/S3Test.php b/tests/lib/Files/ObjectStore/S3Test.php index 94719484f24a8..a702e0a8e8985 100644 --- a/tests/lib/Files/ObjectStore/S3Test.php +++ b/tests/lib/Files/ObjectStore/S3Test.php @@ -190,4 +190,40 @@ public function testFileSizes($size): void { $this->assertNoUpload('testfilesizes'); } + + public function testPreSignedMultipartUpload(): void { + $this->cleanupAfter('presigned'); + $s3 = $this->getInstance(); + if (!$s3->canPreSignUploads()) { + $this->markTestSkipped('The object store configuration can\'t take presigned uploads'); + } + $first = str_repeat('A', 5 * 1024 * 1024); + $last = 'the end'; + $expires = new \DateTimeImmutable('+15 minutes'); + $client = new \GuzzleHttp\Client(); + $put = static function (array $request, string $body) use ($client): int { + return $client->request($request['method'], $request['url'], [ + 'headers' => $request['headers'], + 'body' => $body, + 'http_errors' => false, + ])->getStatusCode(); + }; + + $uploadId = $s3->initiatePreSignedMultipartUpload('presigned', 'text/plain'); + $one = $s3->preSignUploadPart('presigned', $uploadId, 1, strlen($first), hash('sha256', $first), $expires); + $two = $s3->preSignUploadPart('presigned', $uploadId, 2, strlen($last), hash('sha256', $last), $expires); + + $this->assertGreaterThanOrEqual(400, $put($two, 'other bytes'), 'A part that doesn\'t match its checksum is refused'); + $this->assertSame(200, $put($one, $first)); + $this->assertSame(200, $put($two, $last)); + $this->assertSame([ + ['number' => 1, 'size' => strlen($first), 'sha256' => hash('sha256', $first)], + ['number' => 2, 'size' => strlen($last), 'sha256' => hash('sha256', $last)], + ], $s3->getPreSignedUploadParts('presigned', $uploadId)); + + $size = $s3->completeMultipartUpload('presigned', $uploadId, $s3->getMultipartUploads('presigned', $uploadId)); + $this->assertSame(strlen($first) + strlen($last), $size); + $this->assertSame($first . $last, stream_get_contents($s3->readObject('presigned'))); + $this->assertNoUpload('presigned'); + } }