cleanup per codereview

- reference OWS OpenSSL - clarify comments - fix typo // FREEBIE
8 years ago · 0ab958f03a
parent 0f9f26a577
commit 0ab958f03a
6 changed files with 31 additions and 26 deletions
--- a/2
+++ b/2
@ -8,7 +8,7 @@ target 'Signal' do
    pod 'JSQMessagesViewController',  git: 'https://github.com/WhisperSystems/JSQMessagesViewController.git', branch: 'signal-master'
    #pod 'JSQMessagesViewController',   path: '../JSQMessagesViewController'
    pod 'PureLayout'
-    pod 'OpenSSL', git: 'https://github.com/charlesmchen/OpenSSL-Pod', branch: '1.0.2l'
+    pod 'OpenSSL',                    git: 'https://github.com/WhisperSystems/OpenSSL-Pod'
    pod 'Reachability'
    pod 'SignalServiceKit',           path: '.'
    pod 'SocketRocket',               :git => 'https://github.com/facebook/SocketRocket.git'
--- a/Podfile.lock
+++ b/Podfile.lock
@ -115,7 +115,7 @@ DEPENDENCIES:
  - ATAppUpdater
  - AxolotlKit (from `https://github.com/WhisperSystems/SignalProtocolKit.git`)
  - JSQMessagesViewController (from `https://github.com/WhisperSystems/JSQMessagesViewController.git`, branch `signal-master`)
-  - OpenSSL (from `https://github.com/charlesmchen/OpenSSL-Pod`, branch `1.0.2l`)
+  - OpenSSL (from `https://github.com/WhisperSystems/OpenSSL-Pod`)
  - PureLayout
  - Reachability
  - SignalServiceKit (from `.`)
@ -128,8 +128,7 @@ EXTERNAL SOURCES:
    :branch: signal-master
    :git: https://github.com/WhisperSystems/JSQMessagesViewController.git
  OpenSSL:
-    :branch: 1.0.2l
-    :git: https://github.com/charlesmchen/OpenSSL-Pod
+    :git: https://github.com/WhisperSystems/OpenSSL-Pod
  SignalServiceKit:
    :path: .
  SocketRocket:
@ -143,8 +142,8 @@ CHECKOUT OPTIONS:
    :commit: 8c10f920bb566fe1051f92abd360a2e3d5530e00
    :git: https://github.com/WhisperSystems/JSQMessagesViewController.git
  OpenSSL:
-    :commit: 07d058fd05b794bae017e1d196b0338003b8c29e
-    :git: https://github.com/charlesmchen/OpenSSL-Pod
+    :commit: b2d3c149102032a09aefbfb470885db4aa83efad
+    :git: https://github.com/WhisperSystems/OpenSSL-Pod
  SocketRocket:
    :commit: 877ac7438be3ad0b45ef5ca3969574e4b97112bf
    :git: https://github.com/facebook/SocketRocket.git
@ -172,6 +171,6 @@ SPEC CHECKSUMS:
  UnionFind: c33be5adb12983981d6e827ea94fc7f9e370f52d
  YapDatabase: cd911121580ff16675f65ad742a9eb0ab4d9e266

-PODFILE CHECKSUM: 6ed599004f6559be1963002dd4dd6a1360e642d4
+PODFILE CHECKSUM: 2f847bb25e70d1d376f38cf21ae08624fa6ed67d

 COCOAPODS: 1.3.1
--- a/SignalServiceKit/Example/TSKitiOSTestApp/Podfile
+++ b/SignalServiceKit/Example/TSKitiOSTestApp/Podfile
@ -5,7 +5,7 @@ target 'TSKitiOSTestApp' do
  pod 'SocketRocket', git: 'https://github.com/facebook/SocketRocket.git'
  pod 'AxolotlKit',  git: 'https://github.com/WhisperSystems/SignalProtocolKit.git'
  pod 'SignalServiceKit', :path => '../../../SignalServiceKit.podspec'
-  pod 'OpenSSL', git: 'https://github.com/charlesmchen/OpenSSL-Pod', branch: '1.0.2l'
+  pod 'OpenSSL', git: 'https://github.com/WhisperSystems/OpenSSL-Pod'

  target 'TSKitiOSTestAppTests' do
     inherit! :search_paths
--- a/SignalServiceKit/Example/TSKitiOSTestApp/Podfile.lock
+++ b/SignalServiceKit/Example/TSKitiOSTestApp/Podfile.lock
@ -38,7 +38,7 @@ PODS:
  - Reachability (3.2)
  - SAMKeychain (1.5.2)
  - SignalServiceKit (0.9.0):
-    - '25519'
+    - 25519
    - AFNetworking
    - AxolotlKit
    - CocoaLumberjack
@ -108,7 +108,7 @@ PODS:

 DEPENDENCIES:
  - AxolotlKit (from `https://github.com/WhisperSystems/SignalProtocolKit.git`)
-  - OpenSSL (from `https://github.com/charlesmchen/OpenSSL-Pod`, branch `1.0.2l`)
+  - OpenSSL (from `https://github.com/WhisperSystems/OpenSSL-Pod`)
  - SignalServiceKit (from `../../../SignalServiceKit.podspec`)
  - SocketRocket (from `https://github.com/facebook/SocketRocket.git`)

@ -116,8 +116,7 @@ EXTERNAL SOURCES:
  AxolotlKit:
    :git: https://github.com/WhisperSystems/SignalProtocolKit.git
  OpenSSL:
-    :branch: 1.0.2l
-    :git: https://github.com/charlesmchen/OpenSSL-Pod
+    :git: https://github.com/WhisperSystems/OpenSSL-Pod
  SignalServiceKit:
    :path: ../../../SignalServiceKit.podspec
  SocketRocket:
@ -128,14 +127,14 @@ CHECKOUT OPTIONS:
    :commit: 28afe5c1dbcfdea73d147e464c53d191d1e3ea50
    :git: https://github.com/WhisperSystems/SignalProtocolKit.git
  OpenSSL:
-    :commit: 07d058fd05b794bae017e1d196b0338003b8c29e
-    :git: https://github.com/charlesmchen/OpenSSL-Pod
+    :commit: b2d3c149102032a09aefbfb470885db4aa83efad
+    :git: https://github.com/WhisperSystems/OpenSSL-Pod
  SocketRocket:
    :commit: 877ac7438be3ad0b45ef5ca3969574e4b97112bf
    :git: https://github.com/facebook/SocketRocket.git

 SPEC CHECKSUMS:
-  '25519': dc4bad7e2dbcbf1efa121068a705a44cd98c80fc
+  25519: dc4bad7e2dbcbf1efa121068a705a44cd98c80fc
  AFNetworking: 5e0e199f73d8626b11e79750991f5d173d1f8b67
  AxolotlKit: a9530d6835baae0f204b1f6b9dd79b7901176f0d
  CocoaLumberjack: aa9dcab71bdf9eaf2a63bbd9ddc87863efe45457
@ -153,6 +152,6 @@ SPEC CHECKSUMS:
  UnionFind: c33be5adb12983981d6e827ea94fc7f9e370f52d
  YapDatabase: cd911121580ff16675f65ad742a9eb0ab4d9e266

-PODFILE CHECKSUM: 1d91553ff6ddb91426b1bcb43d0433c2c3cbe272
+PODFILE CHECKSUM: 8eff8ab93f8a0a1024e17b16fee43f3a93656c2c

-COCOAPODS: 1.2.1
+COCOAPODS: 1.3.1
--- a/SignalServiceKit/Example/TSKitiOSTestApp/TSKitiOSTestApp.xcodeproj/project.pbxproj
+++ b/SignalServiceKit/Example/TSKitiOSTestApp/TSKitiOSTestApp.xcodeproj/project.pbxproj
@ -468,7 +468,7 @@
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 			shellPath = /bin/sh;
-			shellScript = "diff \"${PODS_PODFILE_DIR_PATH}/Podfile.lock\" \"${PODS_ROOT}/Manifest.lock\" > /dev/null\nif [ $? != 0 ] ; then\n    # print error to STDERR\n    echo \"error: The sandbox is not in sync with the Podfile.lock. Run 'pod install' or update your CocoaPods installation.\" >&2\n    exit 1\nfi\n";
+			shellScript = "diff \"${PODS_PODFILE_DIR_PATH}/Podfile.lock\" \"${PODS_ROOT}/Manifest.lock\" > /dev/null\nif [ $? != 0 ] ; then\n    # print error to STDERR\n    echo \"error: The sandbox is not in sync with the Podfile.lock. Run 'pod install' or update your CocoaPods installation.\" >&2\n    exit 1\nfi\n# This output is used by Xcode 'outputs' to avoid re-running this script phase.\necho \"SUCCESS\" > \"${SCRIPT_OUTPUT_FILE_0}\"\n";
 			showEnvVarsInLog = 0;
 		};
 		276B029791E679B0E87877B7 /* [CP] Copy Pods Resources */ = {
@ -551,7 +551,7 @@
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 			shellPath = /bin/sh;
-			shellScript = "diff \"${PODS_PODFILE_DIR_PATH}/Podfile.lock\" \"${PODS_ROOT}/Manifest.lock\" > /dev/null\nif [ $? != 0 ] ; then\n    # print error to STDERR\n    echo \"error: The sandbox is not in sync with the Podfile.lock. Run 'pod install' or update your CocoaPods installation.\" >&2\n    exit 1\nfi\n";
+			shellScript = "diff \"${PODS_PODFILE_DIR_PATH}/Podfile.lock\" \"${PODS_ROOT}/Manifest.lock\" > /dev/null\nif [ $? != 0 ] ; then\n    # print error to STDERR\n    echo \"error: The sandbox is not in sync with the Podfile.lock. Run 'pod install' or update your CocoaPods installation.\" >&2\n    exit 1\nfi\n# This output is used by Xcode 'outputs' to avoid re-running this script phase.\necho \"SUCCESS\" > \"${SCRIPT_OUTPUT_FILE_0}\"\n";
 			showEnvVarsInLog = 0;
 		};
 /* End PBXShellScriptBuildPhase section */
--- a/SignalServiceKit/src/Util/Cryptography.m
+++ b/SignalServiceKit/src/Util/Cryptography.m
@ -416,8 +416,12 @@ const NSUInteger kAES256_KeyByteLength = 32;
    }

    int bytesEncrypted = 0;
+
    // Provide the message to be encrypted, and obtain the encrypted output.
-    // EVP_EncryptUpdate can be called multiple times if necessary
+    //
+    // If we wanted to save memory, we could encrypt piece-wise from a plaintext iostream -
+    // feeding each chunk to EVP_EncryptUpdate, which can be called multiple times.
+    // For simplicity, we currently encrypt the entire plaintext in one shot.
    if (EVP_EncryptUpdate(ctx, ciphertext.mutableBytes, &bytesEncrypted, plaintext.bytes, (int)plaintext.length)
        != kOpenSSLSuccess) {
        OWSFail(@"%@ encryptUpdate failed", self.tag);
@ -440,13 +444,13 @@ const NSUInteger kAES256_KeyByteLength = 32;
        return nil;
    }

-    /* Get the tag */
+    // Get the tag
    if (EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, kAESGCM256_TagLength, authTag.mutableBytes) != kOpenSSLSuccess) {
        OWSFail(@"%@ failed to write tag", self.tag);
        return nil;
    }

-    /* Clean up */
+    // Clean up
    EVP_CIPHER_CTX_free(ctx);

    // build up return value: initializationVector || ciphertext || authTag
@ -506,7 +510,10 @@ const NSUInteger kAES256_KeyByteLength = 32;
    }

    // Provide the message to be decrypted, and obtain the plaintext output.
-    // EVP_DecryptUpdate can be called multiple times if necessary
+    //
+    // If we wanted to save memory, we could decrypt piece-wise from an iostream -
+    // feeding each chunk to EVP_DecryptUpdate, which can be called multiple times.
+    // For simplicity, we currently decrypt the entire ciphertext in one shot.
    int decryptedBytes = 0;
    if (EVP_DecryptUpdate(ctx, plaintext.mutableBytes, &decryptedBytes, ciphertext.bytes, (int)ciphertext.length)
        != kOpenSSLSuccess) {
@ -520,7 +527,7 @@ const NSUInteger kAES256_KeyByteLength = 32;
    }

    // Set expected tag value. Works in OpenSSL 1.0.1d and later
-    if (EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, (int)authTagFromEncrypt.length, authTagFromEncrypt.bytes)
+    if (EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, (int)authTagFromEncrypt.length, (void *)authTagFromEncrypt.bytes)
        != kOpenSSLSuccess) {
        OWSFail(@"%@ Failed to set auth tag in decrypt.", self.tag);
        return nil;
@ -529,7 +536,7 @@ const NSUInteger kAES256_KeyByteLength = 32;
    // Finalise the decryption. A positive return value indicates success,
    // anything else is a failure - the plaintext is not trustworthy.
    int finalBytes = 0;
-    int decryptStatus = EVP_DecryptFinal_ex(ctx, plaintext.bytes + decryptedBytes, &finalBytes);
+    int decryptStatus = EVP_DecryptFinal_ex(ctx, (unsigned char *)(plaintext.bytes + decryptedBytes), &finalBytes);

    // AESGCM doesn't write any final bytes
    OWSAssert(finalBytes == 0);
@ -542,7 +549,7 @@ const NSUInteger kAES256_KeyByteLength = 32;
    } else {
        // This should only happen if the user has changed their profile key, which should only
        // happen currently if they re-register.
-        DDLogError(@"%@ Decrypt verificaiton failed", self.tag);
+        DDLogError(@"%@ Decrypt verification failed", self.tag);
        return nil;
    }
 }