Attachments require digest or show explanatory error.

// FREEBIE

Signal/src/views/AttachmentPointerView.swift

@@ -79,11 +79,11 @@ class AttachmentPointerView: UIView {
         self.addSubview(progressView)
         progressView.autoPinWidthToSuperview()
         progressView.autoPinEdge(.top, to: .bottom, of: nameLabel, withOffset: 6)
-        progressView.autoSetDimension(.height, toSize: 8)
 
         self.addSubview(statusLabel)
         statusLabel.textAlignment = .center
         statusLabel.adjustsFontSizeToFitWidth = true
+        statusLabel.numberOfLines = 2
 
         statusLabel.textColor = self.textColor
         statusLabel.font = UIFont.ows_footnote()
@@ -119,14 +119,16 @@ class AttachmentPointerView: UIView {
             case .downloading:
                 return NSLocalizedString("ATTACHMENT_DOWNLOADING_STATUS_IN_PROGRESS", comment: "Status label when an attachment is currently downloading")
             case .failed:
-                return NSLocalizedString("ATTACHMENT_DOWNLOADING_STATUS_FAILED", comment: "Status label when an attachment download has failed.")
+                return self.attachmentPointer.mostRecentFailureLocalizedText ?? NSLocalizedString("ATTACHMENT_DOWNLOADING_STATUS_FAILED", comment: "Status label when an attachment download has failed.")
             }
         }()
 
         if attachmentPointer.state == .downloading {
             progressView.isHidden = false
+            progressView.autoSetDimension(.height, toSize: 8)
         } else {
             progressView.isHidden = true
+            progressView.autoSetDimension(.height, toSize: 0)
         }
     }
 

Signal/translations/en.lproj/Localizable.strings

@@ -547,6 +547,9 @@
 /* Error message when attempting to send message */
 "ERROR_DESCRIPTION_UNREGISTERED_RECIPIENT" = "Contact is not a Signal user.";
 
+/* Error message when unable to receive an attachment because the sending client is too old. */
+"ERROR_MESSAGE_ATTACHMENT_FROM_OLD_CLIENT" = "Failure: Ask sender to update Signal and resend.";
+
 /* No comment provided by engineer. */
 "ERROR_MESSAGE_DUPLICATE_MESSAGE" = "Received a duplicated message.";
 

SignalServiceKit/src/Messages/Attachments/OWSAttachmentsProcessor.m

@@ -160,7 +160,7 @@ static const CGFloat kAttachmentDownloadProgressTheta = 0.001f;
     void (^markAndHandleFailure)(NSError *) = ^(NSError *error) {
         // Ensure enclosing transaction is complete.
         dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
-            [self setAttachment:attachment didFailInMessage:message];
+            [self setAttachment:attachment didFailInMessage:message error:error];
             failureHandler(error);
         });
     };
@@ -246,21 +246,32 @@ static const CGFloat kAttachmentDownloadProgressTheta = 0.001f;
                       success:(void (^)(TSAttachmentStream *attachmentStream))successHandler
                       failure:(void (^)(NSError *error))failureHandler
 {
-    NSData *plaintext =
+    NSError *decryptError;
-        [Cryptography decryptAttachment:cipherText withKey:attachment.encryptionKey digest:attachment.digest];
+    NSData *plaintext = [Cryptography decryptAttachment:cipherText
+                                                withKey:attachment.encryptionKey
+                                                 digest:attachment.digest
+                                                  error:&decryptError];
+
+    if (decryptError) {
+        DDLogError(@"%@ failed to decrypt with error: %@", self.tag, decryptError);
+        failureHandler(decryptError);
+        return;
+    }
 
     if (!plaintext) {
         NSError *error = OWSErrorWithCodeDescription(OWSErrorCodeFailedToDecryptMessage, NSLocalizedString(@"ERROR_MESSAGE_INVALID_MESSAGE", @""));
-        return failureHandler(error);
+        failureHandler(error);
+        return;
     }
 
     TSAttachmentStream *stream = [[TSAttachmentStream alloc] initWithPointer:attachment];
 
-    NSError *error;
+    NSError *writeError;
-    [stream writeData:plaintext error:&error];
+    [stream writeData:plaintext error:&writeError];
-    if (error) {
+    if (writeError) {
-        DDLogError(@"%@ Failed writing attachment stream with error: %@", self.tag, error);
+        DDLogError(@"%@ Failed writing attachment stream with error: %@", self.tag, writeError);
-        return failureHandler(error);
+        failureHandler(writeError);
+        return;
     }
 
     [stream save];
@@ -405,8 +416,11 @@ static const CGFloat kAttachmentDownloadProgressTheta = 0.001f;
     }
 }
 
-- (void)setAttachment:(TSAttachmentPointer *)pointer didFailInMessage:(nullable TSMessage *)message
+- (void)setAttachment:(TSAttachmentPointer *)pointer
+     didFailInMessage:(nullable TSMessage *)message
+                error:(NSError *)error
 {
+    pointer.mostRecentFailureLocalizedText = error.localizedDescription;
     pointer.state = TSAttachmentPointerStateFailed;
     [pointer save];
     if (message) {

SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.h

@@ -30,6 +30,7 @@ typedef NS_ENUM(NSUInteger, TSAttachmentPointerState) {
 
 @property (nonatomic, readonly) NSString *relay;
 @property (atomic) TSAttachmentPointerState state;
+@property (nullable, atomic) NSString *mostRecentFailureLocalizedText;
 
 // Though now required, `digest` may be null for pre-existing records or from
 // messages received from other clients

SignalServiceKit/src/Util/Cryptography.h

@@ -54,7 +54,12 @@ typedef NS_ENUM(NSInteger, TSMACType) {
 + (NSData *)decryptAppleMessagePayload:(NSData *)payload withSignalingKey:(NSString *)signalingKeyString;
 
 #pragma mark encrypt and decrypt attachment data
-+ (NSData *)decryptAttachment:(NSData *)dataToDecrypt withKey:(NSData *)key digest:(nullable NSData *)digest;
+
+// Though digest can and will be nil for legacy clients, we now reject attachments lacking a digest.
++ (NSData *)decryptAttachment:(NSData *)dataToDecrypt
+                      withKey:(NSData *)key
+                       digest:(nullable NSData *)digest
+                        error:(NSError **)error;
 
 + (NSData *)encryptAttachmentData:(NSData *)attachmentData
                            outKey:(NSData *_Nonnull *_Nullable)outKey

SignalServiceKit/src/Util/Cryptography.m

@@ -5,6 +5,7 @@
 #import "Cryptography.h"
 #import "NSData+Base64.h"
 #import "NSData+OWSConstantTimeCompare.h"
+#import "OWSError.h"
 #import <CommonCrypto/CommonCryptor.h>
 #import <CommonCrypto/CommonHMAC.h>
 #import <openssl/evp.h>
@@ -293,11 +294,25 @@ const NSUInteger kAES256_KeyByteLength = 32;
                               digest:nil];
 }
 
-+ (NSData *)decryptAttachment:(NSData *)dataToDecrypt withKey:(NSData *)key digest:(nullable NSData *)digest
++ (NSData *)decryptAttachment:(NSData *)dataToDecrypt
+                      withKey:(NSData *)key
+                       digest:(nullable NSData *)digest
+                        error:(NSError **)error;
 {
+    if (digest.length <= 0) {
+        // This *could* happen with sufficiently outdated clients.
+        DDLogError(@"%@ Refusing to decrypt attachment without a digest.", self.tag);
+        *error = OWSErrorWithCodeDescription(OWSErrorCodeFailedToDecryptMessage,
+            NSLocalizedString(@"ERROR_MESSAGE_ATTACHMENT_FROM_OLD_CLIENT",
+                @"Error message when unable to receive an attachment because the sending client is too old."));
+        return nil;
+    }
+
     if (([dataToDecrypt length] < AES_CBC_IV_LENGTH + HMAC256_OUTPUT_LENGTH) ||
         ([key length] < AES_KEY_SIZE + HMAC256_KEY_LENGTH)) {
         DDLogError(@"%@ Message shorter than crypto overhead!", self.tag);
+        *error = OWSErrorWithCodeDescription(
+            OWSErrorCodeFailedToDecryptMessage, NSLocalizedString(@"ERROR_MESSAGE_INVALID_MESSAGE", @""));
         return nil;
     }