From 5483fb0e92185d33e1d7aeba28c6c85604cf924c Mon Sep 17 00:00:00 2001 From: Max Radermacher Date: Mon, 4 May 2026 12:24:02 -0500 Subject: [PATCH] Split DownloadMetadata into distinct pieces --- .../Contacts/OWSSyncManager.swift | 3 +- .../Devices/LinkAndSyncManager.swift | 9 +- .../Jobs/IncomingContactSyncJobQueue.swift | 10 +- .../IncomingContactSyncJobRecord.swift | 51 ++--- .../AttachmentContentValidator.swift | 44 ++-- .../AttachmentContentValidatorImpl.swift | 12 +- .../AttachmentContentValidatorMock.swift | 4 +- .../Downloads/AttachmentDownloadManager.swift | 73 ++----- .../AttachmentDownloadManagerImpl.swift | 192 ++++++++++-------- .../AttachmentDownloadManagerMock.swift | 7 +- .../Storage/JobRecords/JobRecordTest.swift | 31 ++- 11 files changed, 218 insertions(+), 218 deletions(-) diff --git a/SignalServiceKit/Contacts/OWSSyncManager.swift b/SignalServiceKit/Contacts/OWSSyncManager.swift index 322831cb32..14d12f886f 100644 --- a/SignalServiceKit/Contacts/OWSSyncManager.swift +++ b/SignalServiceKit/Contacts/OWSSyncManager.swift @@ -634,6 +634,7 @@ extension OWSSyncManager: SyncManagerProtocol, SyncManagerProtocolSwift { syncMessage.blob.hasCdnNumber, let cdnKey = syncMessage.blob.cdnKey?.nilIfEmpty, let encryptionKey = syncMessage.blob.key?.nilIfEmpty, + let attachmentKey = try? AttachmentKey(combinedKey: encryptionKey), let digest = syncMessage.blob.digest?.nilIfEmpty, syncMessage.blob.hasSize else { @@ -643,7 +644,7 @@ extension OWSSyncManager: SyncManagerProtocol, SyncManagerProtocolSwift { SSKEnvironment.shared.smJobQueuesRef.incomingContactSyncJobQueue.add( cdnNumber: syncMessage.blob.cdnNumber, cdnKey: cdnKey, - encryptionKey: encryptionKey, + attachmentKey: attachmentKey, digest: digest, plaintextLength: syncMessage.blob.size, isComplete: syncMessage.isComplete, diff --git a/SignalServiceKit/Devices/LinkAndSyncManager.swift b/SignalServiceKit/Devices/LinkAndSyncManager.swift index 81fba41745..112c346d05 100644 --- a/SignalServiceKit/Devices/LinkAndSyncManager.swift +++ b/SignalServiceKit/Devices/LinkAndSyncManager.swift @@ -341,7 +341,6 @@ public class LinkAndSyncManagerImpl: LinkAndSyncManager { let downloadedFileUrl = try await downloadEphemeralBackup( cdnNumber: cdnNumber, cdnKey: cdnKey, - ephemeralBackupKey: ephemeralBackupKey, progress: progress.child(for: .downloadingBackup), ) @@ -633,16 +632,14 @@ public class LinkAndSyncManagerImpl: LinkAndSyncManager { private func downloadEphemeralBackup( cdnNumber: UInt32, cdnKey: String, - ephemeralBackupKey: MessageRootBackupKey, progress: OWSProgressSink, ) async throws -> URL { return try await attachmentDownloadManager.downloadEncryptedTransientAttachment( - metadata: AttachmentDownloads.DownloadMetadata( - mimeType: MimeType.applicationOctetStream.rawValue, + downloadMetadata: AttachmentDownloads.DownloadMetadata( cdnNumber: cdnNumber, - encryptionKey: ephemeralBackupKey.serialize(), - source: .linkNSyncBackup(cdnKey: cdnKey), + source: .transitTier(cdnKey: cdnKey), ), + expectedDownloadSize: nil, progress: progress, ) } diff --git a/SignalServiceKit/Jobs/IncomingContactSyncJobQueue.swift b/SignalServiceKit/Jobs/IncomingContactSyncJobQueue.swift index a5559b47ae..d9bdf5c691 100644 --- a/SignalServiceKit/Jobs/IncomingContactSyncJobQueue.swift +++ b/SignalServiceKit/Jobs/IncomingContactSyncJobQueue.swift @@ -35,7 +35,7 @@ public class IncomingContactSyncJobQueue { public func add( cdnNumber: UInt32, cdnKey: String, - encryptionKey: Data, + attachmentKey: AttachmentKey, digest: Data, plaintextLength: UInt32?, isComplete: Bool, @@ -44,7 +44,7 @@ public class IncomingContactSyncJobQueue { let jobRecord = IncomingContactSyncJobRecord( cdnNumber: cdnNumber, cdnKey: cdnKey, - encryptionKey: encryptionKey, + attachmentKey: attachmentKey, digest: digest, plaintextLength: plaintextLength, isCompleteContactSync: isComplete, @@ -100,9 +100,11 @@ private class IncomingContactSyncJobRunner: JobRunner { jobRecord.anyRemove(transaction: tx) } return - case .transient(let downloadMetadata): + case .transient(let downloadMetadata, let decryptionMetadata): fileUrl = try await DependenciesBridge.shared.attachmentDownloadManager.downloadTransientAttachment( - metadata: downloadMetadata, + downloadMetadata: downloadMetadata, + decryptionMetadata: decryptionMetadata, + expectedDownloadSize: decryptionMetadata.plaintextLength, progress: nil, ) } diff --git a/SignalServiceKit/Jobs/JobRecords/IncomingContactSyncJobRecord.swift b/SignalServiceKit/Jobs/JobRecords/IncomingContactSyncJobRecord.swift index bc5a8d8e20..98ef4fd828 100644 --- a/SignalServiceKit/Jobs/JobRecords/IncomingContactSyncJobRecord.swift +++ b/SignalServiceKit/Jobs/JobRecords/IncomingContactSyncJobRecord.swift @@ -17,37 +17,42 @@ public final class IncomingContactSyncJobRecord: JobRecord { public enum DownloadInfo { case invalid - case transient(AttachmentDownloads.DownloadMetadata) + case transient(downloadMetadata: AttachmentDownloads.DownloadMetadata, decryptionMetadata: DecryptionMetadata) } public var downloadInfo: DownloadInfo { - if + guard let cdnKey, let cdnNumber, let encryptionKey, let digest, let plaintextLength - { - return .transient(.init( - mimeType: MimeType.applicationOctetStream.rawValue, - cdnNumber: cdnNumber, - encryptionKey: encryptionKey, - source: .transitTier( - cdnKey: cdnKey, - integrityCheck: .ciphertextDigest(digest), - plaintextLength: plaintextLength, - ), - )) + else { + owsAssertDebug( + cdnKey == nil + && cdnNumber == nil + && encryptionKey == nil + && digest == nil + && plaintextLength == nil, + "Either all fields should be set or none!", + ) + return .invalid } - owsAssertDebug( - cdnKey == nil - && cdnNumber == nil - && encryptionKey == nil - && digest == nil - && plaintextLength == nil, - "Either all fields should be set or none!", + guard let attachmentKey = try? AttachmentKey(combinedKey: encryptionKey) else { + owsFailDebug("couldn't parse contact sync attachment key") + return .invalid + } + return .transient( + downloadMetadata: AttachmentDownloads.DownloadMetadata( + cdnNumber: cdnNumber, + source: .transitTier(cdnKey: cdnKey), + ), + decryptionMetadata: DecryptionMetadata( + key: attachmentKey, + integrityCheck: .ciphertextDigest(digest), + plaintextLength: UInt64(safeCast: plaintextLength), + ), ) - return .invalid } public let isCompleteContactSync: Bool @@ -55,7 +60,7 @@ public final class IncomingContactSyncJobRecord: JobRecord { public init( cdnNumber: UInt32, cdnKey: String, - encryptionKey: Data, + attachmentKey: AttachmentKey, digest: Data, plaintextLength: UInt32?, isCompleteContactSync: Bool, @@ -66,7 +71,7 @@ public final class IncomingContactSyncJobRecord: JobRecord { self.cdnNumber = cdnNumber self.cdnKey = cdnKey - self.encryptionKey = encryptionKey + self.encryptionKey = attachmentKey.combinedKey self.digest = digest self.plaintextLength = plaintextLength diff --git a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift index a0ac6b8c72..8e933e5027 100644 --- a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift +++ b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidator.swift @@ -138,29 +138,39 @@ public protocol AttachmentContentValidator { mimeType: String, ) async throws -> RevalidatedAttachment - /// Validate and prepare a backup media file's contents, based on the provided mimetype. - /// Returns a PendingAttachment with validated contents, ready to be inserted. - /// Note the content type may be `invalid`; we can still create an Attachment from these. - /// Errors are thrown if data reading/parsing/decryption fails. + /// Validate and prepare a backup media file's contents, based on the + /// provided mimetype. Returns a PendingAttachment with validated contents, + /// ready to be inserted. Note the content type may be `invalid`; we can + /// still create an Attachment from these. Errors are thrown if data + /// reading/parsing/decryption fails. /// - /// Unlike attachments from the live service, integrityCheck is not required; we can guarantee - /// correctness for backup media files since they come from the local user. + /// Unlike attachments from the live service, integrityCheck is not + /// required; we can guarantee correctness for backup media files since they + /// come from the local user. /// - /// Unlike transit tier attachments, backup attachments are encrypted twice: once when uploaded - /// to the transit tier, and again when copied to the media tier. This means validating media tier - /// attachments required decrypting the file twice to allow validating the actual contents of the attachment. + /// Unlike transit tier attachments, backup attachments are encrypted twice: + /// once when uploaded to the transit tier, and again when copied to the + /// media tier. This means validating media tier attachments requires + /// "decrypting" the file twice to allow validating the actual contents of + /// the attachment. /// - /// Strictly speaking we don't usually need content type validation either, but the set of valid - /// contents can change over time so it is best to re-validate. + /// Strictly speaking we don't usually need content type validation either, + /// but the set of valid contents can change over time so it is best to + /// re-validate. /// - /// - Parameter outerDecryptionData: The media tier decryption metadata use as the outer layer of encryption. - /// - Parameter innerDecryptionData: The transit tier decryption metadata. - /// - Parameter finalEncryptionKey: The encryption key used to encrypt the file in it's final destination. If the finalEncryptionKey - /// matches the encryption key in `innerEncryptionData`, this re-encryption will be skipped. + /// - Parameter outerAttachmentKey: The media tier key use as the outer + /// layer of encryption. + /// + /// - Parameter innerDecryptionMetadata: The transit tier decryption + /// metadata (or another media tier key if this is a thumbnail). + /// + /// - Parameter finalEncryptionKey: The encryption key used to encrypt the + /// file in its final destination. If this key matches the encryption key in + /// `innerDecryptionMetadata`, this re-encryption will be skipped. func validateBackupMediaFileContents( fileUrl: URL, - outerDecryptionData: DecryptionMetadata, - innerDecryptionData: DecryptionMetadata, + outerAttachmentKey: AttachmentKey, + innerDecryptionMetadata: DecryptionMetadata, finalAttachmentKey: AttachmentKey, mimeType: String, renderingFlag: AttachmentReference.RenderingFlag, diff --git a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorImpl.swift b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorImpl.swift index 861f3e29e3..c433891308 100644 --- a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorImpl.swift +++ b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorImpl.swift @@ -143,8 +143,8 @@ public class AttachmentContentValidatorImpl: AttachmentContentValidator { public func validateBackupMediaFileContents( fileUrl: URL, - outerDecryptionData: DecryptionMetadata, - innerDecryptionData: DecryptionMetadata, + outerAttachmentKey: AttachmentKey, + innerDecryptionMetadata: DecryptionMetadata, finalAttachmentKey: AttachmentKey, mimeType: String, renderingFlag: AttachmentReference.RenderingFlag, @@ -160,16 +160,16 @@ public class AttachmentContentValidatorImpl: AttachmentContentValidator { ) try Cryptography.decryptFile( at: fileUrl, - metadata: outerDecryptionData, + metadata: DecryptionMetadata(key: outerAttachmentKey), output: tmpFileUrl, ) func makeInputType(plaintextLength: UInt64) -> InputType { return InputType.encryptedFile( tmpFileUrl, - inputAttachmentKey: innerDecryptionData.key, + inputAttachmentKey: innerDecryptionMetadata.key, plaintextLength: UInt32(plaintextLength), - integrityCheck: innerDecryptionData.integrityCheck, + integrityCheck: innerDecryptionMetadata.integrityCheck, ) } @@ -177,7 +177,7 @@ public class AttachmentContentValidatorImpl: AttachmentContentValidator { var sha256 = SHA256() try Cryptography.decryptFile( at: tmpFileUrl, - metadata: innerDecryptionData, + metadata: innerDecryptionMetadata, output: { data in decryptedLength += UInt64(data.count) sha256.update(data: data) diff --git a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorMock.swift b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorMock.swift index 31ef080a26..08e411f451 100644 --- a/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorMock.swift +++ b/SignalServiceKit/Messages/Attachments/V2/ContentValidation/AttachmentContentValidatorMock.swift @@ -52,8 +52,8 @@ open class AttachmentContentValidatorMock: AttachmentContentValidator { open func validateBackupMediaFileContents( fileUrl: URL, - outerDecryptionData: DecryptionMetadata, - innerDecryptionData: DecryptionMetadata, + outerAttachmentKey: AttachmentKey, + innerDecryptionMetadata: DecryptionMetadata, finalAttachmentKey: AttachmentKey, mimeType: String, renderingFlag: AttachmentReference.RenderingFlag, diff --git a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManager.swift b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManager.swift index f1f6e2a05a..d3217e492d 100644 --- a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManager.swift +++ b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManager.swift @@ -25,83 +25,35 @@ public enum AttachmentDownloads { public static var attachmentDownloadAttachmentIDKey: String { "attachmentDownloadAttachmentIDKey" } public struct DownloadMetadata { - public let mimeType: String public let cdnNumber: UInt32 - public let encryptionKey: Data public let source: Source public enum Source { - case transitTier(cdnKey: String, integrityCheck: AttachmentIntegrityCheck, plaintextLength: UInt32?) - case mediaTierFullsize( - cdnReadCredential: MediaTierReadCredential, - outerEncryptionMetadata: MediaTierEncryptionMetadata, - integrityCheck: AttachmentIntegrityCheck, - plaintextLength: UInt32?, - ) - case mediaTierThumbnail( - cdnReadCredential: MediaTierReadCredential, - outerEncyptionMetadata: MediaTierEncryptionMetadata, - innerEncryptionMetadata: MediaTierEncryptionMetadata, - ) - case linkNSyncBackup(cdnKey: String) + case transitTier(cdnKey: String) + case mediaTier(type: MediaTierType, cdnReadCredential: MediaTierReadCredential, mediaId: Data) + + public enum MediaTierType { + case fullsize + case thumbnail + } var asQueuedDownloadSource: QueuedAttachmentDownloadRecord.SourceType { switch self { case .transitTier: return .transitTier - case .mediaTierFullsize: + case .mediaTier(type: .fullsize, _, _): return .mediaTierFullsize - case .mediaTierThumbnail: + case .mediaTier(type: .thumbnail, _, _): return .mediaTierThumbnail - case .linkNSyncBackup: - return .transitTier } } } - public var integrityCheck: AttachmentIntegrityCheck? { - switch source { - case .transitTier(_, let integrityCheck, _): - return integrityCheck - case .mediaTierFullsize(_, _, let integrityCheck, _): - return integrityCheck - case .mediaTierThumbnail: - // No integrityCheck for media tier thumbnails; they come from the local user. - return nil - case .linkNSyncBackup: - // No integrityCheck for link'n'sync backups; they come from the local user. - return nil - } - } - - public var plaintextLength: UInt32? { - switch source { - case .transitTier(_, _, let plaintextLength): - return plaintextLength - case .mediaTierFullsize(_, _, _, let plaintextLength): - return plaintextLength - case .mediaTierThumbnail: - // Thumbnails don't include a length out of band. - // They may be padded with 0s to hit bucket sizes, but - // we take advantage of the fact that jpegs support - // no-op trailing 0s (and all thumbnails are jpegs). - return nil - case .linkNSyncBackup: - // Link'n'sync backups don't include a length out - // of band because gzip ignores padding. - return nil - } - } - public init( - mimeType: String, cdnNumber: UInt32, - encryptionKey: Data, source: Source, ) { - self.mimeType = mimeType self.cdnNumber = cdnNumber - self.encryptionKey = encryptionKey self.source = source } } @@ -155,12 +107,15 @@ public protocol AttachmentDownloadManager { ) async throws -> URL func downloadEncryptedTransientAttachment( - metadata: AttachmentDownloads.DownloadMetadata, + downloadMetadata: AttachmentDownloads.DownloadMetadata, + expectedDownloadSize: UInt64?, progress: OWSProgressSink?, ) async throws -> URL func downloadTransientAttachment( - metadata: AttachmentDownloads.DownloadMetadata, + downloadMetadata: AttachmentDownloads.DownloadMetadata, + decryptionMetadata: DecryptionMetadata, + expectedDownloadSize: UInt64?, progress: OWSProgressSink?, ) async throws -> URL diff --git a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift index 7ed5a21927..b173bbd792 100644 --- a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift +++ b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift @@ -191,26 +191,36 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { } public func downloadEncryptedTransientAttachment( - metadata: AttachmentDownloads.DownloadMetadata, + downloadMetadata: DownloadMetadata, + expectedDownloadSize: UInt64?, progress: OWSProgressSink?, ) async throws -> URL { // We want to avoid large downloads from a compromised or buggy service. let maxDownloadSize = self.remoteConfigProvider.currentConfig().attachmentMaxEncryptedReceiveBytes - let downloadState = DownloadState(type: .transientAttachment(metadata, uuid: UUID())) + let downloadState = DownloadState(type: .transientAttachment(downloadMetadata, uuid: UUID())) return try await self.downloadQueue.enqueueDownload( downloadState: downloadState, maxDownloadSizeBytes: maxDownloadSize, - expectedDownloadSize: metadata.plaintextLength.map({ .estimatedSizeBytes(UInt64(safeCast: $0)) }) ?? .useHeadRequest, + expectedDownloadSize: expectedDownloadSize.map({ .estimatedSizeBytes($0) }) ?? .useHeadRequest, progress: progress, ) } public func downloadTransientAttachment( - metadata: AttachmentDownloads.DownloadMetadata, + downloadMetadata: DownloadMetadata, + decryptionMetadata: DecryptionMetadata, + expectedDownloadSize: UInt64?, progress: OWSProgressSink?, ) async throws -> URL { - let encryptedFileUrl = try await downloadEncryptedTransientAttachment(metadata: metadata, progress: progress) - return try await self.decrypter.decryptTransientAttachment(encryptedFileUrl: encryptedFileUrl, metadata: metadata) + let encryptedFileUrl = try await downloadEncryptedTransientAttachment( + downloadMetadata: downloadMetadata, + expectedDownloadSize: expectedDownloadSize, + progress: progress, + ) + return try await self.decrypter.decryptTransientAttachment( + encryptedFileUrl: encryptedFileUrl, + metadata: decryptionMetadata, + ) } public func enqueueDownloadOfAttachmentsForMessage( @@ -783,21 +793,25 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { let downloadMetadata: DownloadMetadata let downloadSizeSource: DownloadQueue.DownloadSizeSource let maxDownloadSizeBytes: UInt64 + let validationMetadata: Decrypter.ValidationMetadata switch record.sourceType { case .transitTier: // We only download from the latest transit tier info. guard let transitTierInfo = attachment.latestTransitTierInfo else { return .unretryableError(OWSAssertionError("Attempting to download an attachment without cdn info")) } - downloadMetadata = .init( - mimeType: attachment.mimeType, + guard let attachmentKey = try? AttachmentKey(combinedKey: transitTierInfo.encryptionKey) else { + return .unretryableError(OWSAssertionError("can't download file with malformed attachment key")) + } + downloadMetadata = DownloadMetadata( cdnNumber: transitTierInfo.cdnNumber, - encryptionKey: transitTierInfo.encryptionKey, - source: .transitTier( - cdnKey: transitTierInfo.cdnKey, - integrityCheck: transitTierInfo.integrityCheck, - plaintextLength: transitTierInfo.unencryptedByteCount, - ), + source: .transitTier(cdnKey: transitTierInfo.cdnKey), + ) + validationMetadata = .transitTier( + mimeType: attachment.mimeType, + attachmentKey: attachmentKey, + plaintextLength: transitTierInfo.unencryptedByteCount, + integrityCheck: transitTierInfo.integrityCheck, ) downloadSizeSource = transitTierInfo.unencryptedByteCount.map({ .estimatedSizeBytes(Cryptography.estimatedTransitTierCDNSize( @@ -817,7 +831,7 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { let mediaTierInfo = attachment.mediaTierInfo, let mediaName = attachment.mediaName, let backupKey = db.read(block: { accountKeyStore.getMediaRootBackupKey(tx: $0) }), - let encryptionMetadata = buildCdnEncryptionMetadata(mediaName: mediaName, backupKey: backupKey, type: .outerLayerFullsizeOrThumbnail), + let outerEncryptionMetadata = buildCdnEncryptionMetadata(mediaName: mediaName, backupKey: backupKey, type: .outerLayerFullsizeOrThumbnail), let cdnCredential = await fetchBackupCdnReadCredential( for: cdnNumber, backupKey: backupKey, @@ -826,18 +840,30 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { else { return .unretryableError(OWSAssertionError("Attempting to download an attachment without cdn info")) } - let integrityCheck = AttachmentIntegrityCheck.plaintextHash(mediaTierInfo.plaintextHash) + guard let outerAttachmentKey = try? outerEncryptionMetadata.attachmentKey() else { + return .unretryableError(OWSAssertionError("can't download media file with malformed media key")) + } + guard let attachmentKey = try? AttachmentKey(combinedKey: attachment.encryptionKey) else { + return .unretryableError(OWSAssertionError("can't download media file with malformed attachment key")) + } downloadMetadata = .init( - mimeType: attachment.mimeType, cdnNumber: cdnNumber, - encryptionKey: attachment.encryptionKey, - source: .mediaTierFullsize( + source: .mediaTier( + type: .fullsize, cdnReadCredential: cdnCredential, - outerEncryptionMetadata: encryptionMetadata, - integrityCheck: integrityCheck, - plaintextLength: mediaTierInfo.unencryptedByteCount, + mediaId: outerEncryptionMetadata.mediaId, ), ) + validationMetadata = .mediaTier( + mimeType: attachment.mimeType, + outerAttachmentKey: outerAttachmentKey, + innerDecryptionMetadata: DecryptionMetadata( + key: attachmentKey, + integrityCheck: .plaintextHash(mediaTierInfo.plaintextHash), + plaintextLength: UInt64(safeCast: mediaTierInfo.unencryptedByteCount), + ), + localAttachmentKey: attachmentKey, + ) downloadSizeSource = .estimatedSizeBytes(Cryptography.estimatedMediaTierCDNSize( unencryptedSize: UInt64(safeCast: mediaTierInfo.unencryptedByteCount), ) ?? { owsFail("can always produce estimate for 32-bit byte count") }()) @@ -868,21 +894,30 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { else { return .unretryableError(OWSAssertionError("Attempting to download an attachment without cdn info")) } + guard let outerAttachmentKey = try? outerEncryptionMetadata.attachmentKey() else { + return .unretryableError(OWSAssertionError("can't download thumbnail with malformed outer media key")) + } + guard let innerAttachmentKey = try? innerEncryptionMetadata.attachmentKey() else { + return .unretryableError(OWSAssertionError("can't download thumbnail with malformed inner media key")) + } + guard let attachmentKey = try? AttachmentKey(combinedKey: attachment.encryptionKey) else { + return .unretryableError(OWSAssertionError("can't download thumbnail with malformed attachment key")) + } - let thumbnailMimeType = MimeTypeUtil.thumbnailMimetype( - fullsizeMimeType: attachment.mimeType, - quality: .backupThumbnail, - ) downloadMetadata = .init( - mimeType: thumbnailMimeType, cdnNumber: cdnNumber, - encryptionKey: attachment.encryptionKey, - source: .mediaTierThumbnail( + source: .mediaTier( + type: .thumbnail, cdnReadCredential: cdnReadCredential, - outerEncyptionMetadata: outerEncryptionMetadata, - innerEncryptionMetadata: innerEncryptionMetadata, + mediaId: outerEncryptionMetadata.mediaId, ), ) + validationMetadata = .mediaTier( + mimeType: MimeTypeUtil.thumbnailMimetype(fullsizeMimeType: attachment.mimeType, quality: .backupThumbnail), + outerAttachmentKey: outerAttachmentKey, + innerDecryptionMetadata: DecryptionMetadata(key: innerAttachmentKey), + localAttachmentKey: attachmentKey, + ) // We don't know thumbnail sizes and don't want to issue a // request for each one to check. Just estimate as the max size. downloadSizeSource = .estimatedSizeBytes(Cryptography.estimatedMediaTierCDNSize( @@ -912,7 +947,7 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { do { pendingAttachment = try await decrypter.validateAndPrepare( encryptedFileUrl: downloadedFileUrl, - metadata: downloadMetadata, + validationMetadata: validationMetadata, ) } catch let error { return .unretryableError(OWSAssertionError("Failed to validate: \(error)")) @@ -1386,7 +1421,7 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { // MARK: - Downloads - typealias DownloadMetadata = AttachmentDownloads.DownloadMetadata + public typealias DownloadMetadata = AttachmentDownloads.DownloadMetadata private enum DownloadError: Error { case oversize @@ -1405,16 +1440,14 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { return info.backupLocationUrl() case .attachment(let metadata, _), .transientAttachment(let metadata, _): switch metadata.source { - case .transitTier(let cdnKey, _, _), .linkNSyncBackup(let cdnKey): + case .transitTier(let cdnKey): guard let encodedKey = cdnKey.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) else { throw OWSAssertionError("Invalid cdnKey.") } return "attachments/\(encodedKey)" - case - .mediaTierFullsize(let cdnCredential, let outerEncryptionMetadata, _, _), - .mediaTierThumbnail(let cdnCredential, let outerEncryptionMetadata, _): + case .mediaTier(_, let cdnCredential, let mediaId): let prefix = cdnCredential.mediaTierUrlPrefix() - return "\(prefix)/\(outerEncryptionMetadata.mediaId.asBase64Url)" + return "\(prefix)/\(mediaId.asBase64Url)" } } } @@ -1434,9 +1467,9 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { return metadata.cdnAuthHeaders case .attachment(let metadata, _), .transientAttachment(let metadata, _): switch metadata.source { - case .transitTier, .linkNSyncBackup: + case .transitTier: return [:] - case .mediaTierFullsize(let cdnCredential, _, _, _), .mediaTierThumbnail(let cdnCredential, _, _): + case .mediaTier(_, let cdnCredential, _): return cdnCredential.cdnAuthHeaders } } @@ -1448,9 +1481,9 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { return metadata.isExpired case .attachment(let metadata, _), .transientAttachment(let metadata, _): switch metadata.source { - case .transitTier, .linkNSyncBackup: + case .transitTier: return false - case .mediaTierFullsize(let cdnCredential, _, _, _), .mediaTierThumbnail(let cdnCredential, _, _): + case .mediaTier(_, let cdnCredential, _): return cdnCredential.isExpired } } @@ -1902,7 +1935,7 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { func decryptTransientAttachment( encryptedFileUrl: URL, - metadata: DownloadMetadata, + metadata: DecryptionMetadata, ) async throws -> URL { return try await decryptionQueue.run { do { @@ -1912,15 +1945,7 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { isAvailableWhileDeviceLocked: false, ) - try Cryptography.decryptAttachment( - at: encryptedFileUrl, - metadata: DecryptionMetadata( - key: AttachmentKey(combinedKey: metadata.encryptionKey), - integrityCheck: metadata.integrityCheck, - plaintextLength: metadata.plaintextLength.map(UInt64.init(safeCast:)), - ), - output: outputUrl, - ) + try Cryptography.decryptAttachment(at: encryptedFileUrl, metadata: metadata, output: outputUrl) return outputUrl } catch let error { @@ -1966,53 +1991,56 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager { } } + enum ValidationMetadata { + case transitTier( + mimeType: String, + attachmentKey: AttachmentKey, + plaintextLength: UInt32?, + integrityCheck: AttachmentIntegrityCheck, + ) + + case mediaTier( + mimeType: String, + /// "Outer" encryption; always derived via MediaRootBackupKey. + outerAttachmentKey: AttachmentKey, + /// "Inner" encryption: transit tier encryption for full-size files and + /// always derived via MediaRootBackupKey for thumbnails. + innerDecryptionMetadata: DecryptionMetadata, + /// Encryption key used to store the file "at rest"/locally. Matches the + /// "inner" encryption for full-size files (which is also the transit tier + /// encryption we use for non-backed-up attachments). Matches the full-size + /// attachment encryption key for thumbnails. + localAttachmentKey: AttachmentKey, + ) + } + func validateAndPrepare( encryptedFileUrl: URL, - metadata: DownloadMetadata, + validationMetadata: ValidationMetadata, ) async throws -> PendingAttachment { let attachmentValidator = self.attachmentValidator return try await decryptionQueue.run { - switch metadata.source { - case .transitTier(_, let integrityCheck, let plaintextLength): + switch validationMetadata { + case .transitTier(let mimeType, let attachmentKey, let plaintextLength, let integrityCheck): return try await attachmentValidator.validateDownloadedContents( ofEncryptedFileAt: encryptedFileUrl, - attachmentKey: AttachmentKey(combinedKey: metadata.encryptionKey), + attachmentKey: attachmentKey, plaintextLength: plaintextLength, integrityCheck: integrityCheck, - mimeType: metadata.mimeType, + mimeType: mimeType, renderingFlag: .default, sourceFilename: nil, ) - case .mediaTierFullsize(_, let outerEncryptionMetadata, let integrityCheck, let plaintextLength): - let innerPlaintextLength: UInt64? = { - guard let plaintextLength else { return nil } - return UInt64(safeCast: plaintextLength) - }() + case .mediaTier(let mimeType, let outerAttachmentKey, let innerDecryptionMetadata, let localEncryptionKey): return try await attachmentValidator.validateBackupMediaFileContents( fileUrl: encryptedFileUrl, - outerDecryptionData: DecryptionMetadata(key: outerEncryptionMetadata.attachmentKey()), - innerDecryptionData: DecryptionMetadata( - key: AttachmentKey(combinedKey: metadata.encryptionKey), - integrityCheck: integrityCheck, - plaintextLength: innerPlaintextLength, - ), - finalAttachmentKey: AttachmentKey(combinedKey: metadata.encryptionKey), - mimeType: metadata.mimeType, + outerAttachmentKey: outerAttachmentKey, + innerDecryptionMetadata: innerDecryptionMetadata, + finalAttachmentKey: localEncryptionKey, + mimeType: mimeType, renderingFlag: .default, sourceFilename: nil, ) - case .mediaTierThumbnail(_, let outerEncryptionMetadata, let innerEncryptionData): - return try await attachmentValidator.validateBackupMediaFileContents( - fileUrl: encryptedFileUrl, - outerDecryptionData: DecryptionMetadata(key: outerEncryptionMetadata.attachmentKey()), - innerDecryptionData: DecryptionMetadata(key: innerEncryptionData.attachmentKey()), - finalAttachmentKey: AttachmentKey(combinedKey: metadata.encryptionKey), - mimeType: metadata.mimeType, - renderingFlag: .default, - sourceFilename: nil, - ) - case .linkNSyncBackup: - throw OWSAssertionError("Should not be validating link'n'sync backups") } } } diff --git a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerMock.swift b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerMock.swift index f821341ef3..be89663f35 100644 --- a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerMock.swift +++ b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerMock.swift @@ -27,7 +27,8 @@ open class AttachmentDownloadManagerMock: AttachmentDownloadManager { } public func downloadEncryptedTransientAttachment( - metadata: AttachmentDownloads.DownloadMetadata, + downloadMetadata: AttachmentDownloads.DownloadMetadata, + expectedDownloadSize: UInt64?, progress: (any OWSProgressSink)?, ) async throws -> URL { try! await Task.sleep(nanoseconds: TimeInterval.infinity.clampedNanoseconds) @@ -35,7 +36,9 @@ open class AttachmentDownloadManagerMock: AttachmentDownloadManager { } public func downloadTransientAttachment( - metadata: AttachmentDownloads.DownloadMetadata, + downloadMetadata: AttachmentDownloads.DownloadMetadata, + decryptionMetadata: DecryptionMetadata, + expectedDownloadSize: UInt64?, progress: OWSProgressSink?, ) async throws -> URL { try! await Task.sleep(nanoseconds: TimeInterval.infinity.clampedNanoseconds) diff --git a/SignalServiceKit/tests/Storage/JobRecords/JobRecordTest.swift b/SignalServiceKit/tests/Storage/JobRecords/JobRecordTest.swift index d670581907..1c9df7c6e9 100644 --- a/SignalServiceKit/tests/Storage/JobRecords/JobRecordTest.swift +++ b/SignalServiceKit/tests/Storage/JobRecords/JobRecordTest.swift @@ -155,12 +155,12 @@ extension IncomingContactSyncJobRecord: ValidatableModel { IncomingContactSyncJobRecord( cdnNumber: 3, cdnKey: "hello", - encryptionKey: Data(base64Encoded: "mMiOmZhbHNlLCJzdXBlciI6eyJ1b")!, - digest: Data(base64Encoded: "291bnQiOjYsInJlY29yZFR5cGUiO")!, + encryptionKey: Data(repeating: 2, count: 64), + digest: Data(repeating: 3, count: 32), plaintextLength: 55, isCompleteContactSync: true, ), - Data(#"{"ICSJR_digest":"291bnQiOjYsInJlY29yZFR5cGUiO","ICSJR_plaintextLength":55,"ICSJR_cdnKey":"hello","status":1,"failureCount":0,"label":"IncomingContactSync","uniqueId":"894EAC5E-918B-434C-A7CE-C24BB8F47932","recordType":61,"ICSJR_cdnNumber":3,"ICSJR_encryptionKey":"mMiOmZhbHNlLCJzdXBlciI6eyJ1b","isCompleteContactSync":true}"#.utf8), + Data(#"{"ICSJR_digest":"AwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwM=","ICSJR_plaintextLength":55,"ICSJR_cdnKey":"hello","status":1,"failureCount":0,"label":"IncomingContactSync","uniqueId":"894EAC5E-918B-434C-A7CE-C24BB8F47932","recordType":61,"ICSJR_cdnNumber":3,"ICSJR_encryptionKey":"AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg==","isCompleteContactSync":true}"#.utf8), ), ] @@ -173,26 +173,25 @@ extension IncomingContactSyncJobRecord: ValidatableModel { switch (downloadInfo, against.downloadInfo) { case (.invalid, .invalid): break - case let (.transient(lhsInfo), .transient(rhsInfo)): - guard - lhsInfo.mimeType == rhsInfo.mimeType, - lhsInfo.cdnNumber == rhsInfo.cdnNumber, - lhsInfo.encryptionKey == rhsInfo.encryptionKey - else { + case let (.transient(lhsDownloadMetadata, lhsDecryptionMetadata), .transient(rhsDownloadMetadata, rhsDecryptionMetadata)): + guard lhsDownloadMetadata.cdnNumber == rhsDownloadMetadata.cdnNumber else { throw ValidatableModelError.failedToValidate } - switch (lhsInfo.source, rhsInfo.source) { - case let (.transitTier(lhsCdnKey, lhsDigest, lhsPlaintextLength), .transitTier(rhsCdnKey, rhsDigest, rhsPlaintextLength)): - guard - lhsCdnKey == rhsCdnKey, - lhsDigest == rhsDigest, - lhsPlaintextLength == rhsPlaintextLength - else { + switch (lhsDownloadMetadata.source, rhsDownloadMetadata.source) { + case let (.transitTier(lhsCdnKey), .transitTier(rhsCdnKey)): + guard lhsCdnKey == rhsCdnKey else { throw ValidatableModelError.failedToValidate } default: throw ValidatableModelError.failedToValidate } + guard + lhsDecryptionMetadata.key.combinedKey == rhsDecryptionMetadata.key.combinedKey, + lhsDecryptionMetadata.integrityCheck == rhsDecryptionMetadata.integrityCheck, + lhsDecryptionMetadata.plaintextLength == rhsDecryptionMetadata.plaintextLength + else { + throw ValidatableModelError.failedToValidate + } case (.invalid, _), (.transient, _): throw ValidatableModelError.failedToValidate }