feat: capacitor 7 #526
feat: capacitor 7 #526
Conversation
|
New, updated, and removed dependencies detected. Learn more about Socket for GitHub ↗︎
|
|
Hi! Is there some deadline for the implementation of Capacitor 7? 🥲 |
|
There isn't one |
WalkthroughThe pull request updates multiple configuration and source files across Android and iOS projects. It removes deprecated private key settings in favor of a unified public key-based decryption flow, replacing calls to CryptoCipherV2 with CryptoCipher. Deployment targets and dependency versions are increased (iOS target from 13.0 to 14.0, Android SDK and Java updates, etc.), and documentation is revised to reflect these changes. Additionally, obsolete files such as InternalUtils and CryptoCipherV2 (both Java and Swift versions) are removed, while new implementations like ASE128.swift are introduced to streamline encryption and decryption processes. Changes
Sequence Diagram(s)sequenceDiagram
participant DS as DownloadService (Android)
participant CC as CryptoCipher
DS->>CC: decryptFile(file, publicKey, ivSessionKey)
CC-->>DS: Return decrypted file
DS->>CC: decryptChecksum(expectedChecksum, publicKey)
CC-->>DS: Return computed checksum
DS->>DS: Compare checksum and verify download
sequenceDiagram
participant CU as CapacitorUpdater (iOS)
participant CC as CryptoCipher
CU->>CC: decryptFile(filePath, publicKey, sessionKey, version)
CC-->>CU: Return decrypted file
CU->>CC: calcChecksum(filePath)
CC-->>CU: Return SHA256 checksum
CU->>CU: Verify checksum and complete update
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
🔇 Additional comments (7)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
ios/Plugin/CryptoCipher.swift (1)
172-190: Ensure consistent error handling when 'publicKey' is empty.
You immediately return the unmodified checksum if the public key is empty. This might hide errors if 'checksum' was truly meant to be decrypted. Consider logging a warning or ensuring that no consumer expects an actual decryption.android/src/main/java/ee/forgr/capacitor_updater/CryptoCipher.java (3)
77-85: Improve error messages in 'stringToPublicKey'.
If the public key string is missing expected delimiters (e.g., “-----BEGIN RSA PUBLIC KEY-----”), your code might fail or incorrectly parse. Currently, the method quickly tries to parse and will throw a “GeneralSecurityException”. Consider clarifying in logs which part of the process failed, or adding user-friendly messages to help diagnose invalid key formats.
177-193: Avoid automatically returning unchecked checksums.
If 'publicKey' is empty, the code short-circuits and returns the original (Base64-encoded) 'checksum'. If decryption is actually required, this code path might cause subtle logical issues. Consider at least logging a warning or clarifying in docs that an empty public key is deliberately treated as “no decryption needed.”
213-241: Potential large-file performance improvement in 'calcChecksum'.
You read the file in chunks and update the SHA-256 digest, which is correct. If these files are extremely large, consider measuring performance. Although the 5 MB buffer is quite generous, you might want to tune it or use memory-mapped I/O for extremely large files.android/src/main/java/ee/forgr/capacitor_updater/CapacitorUpdater.java (1)
331-331: Align 'decryptChecksum' usage with application logic.
Immediately decrypting the “checksumRes” with a public key might cause confusion if the encryption step was done with the same public key. If no fully private key is involved anywhere, clarify in your documentation how the symmetrical or asymmetrical approach is intended to work.ios/Plugin/ASE128.swift (1)
15-20: Consider using an enum with associated values for algorithm parameters.The current implementation uses static constants. Consider using an enum with associated values to group related cryptographic parameters and provide type safety.
-private enum CryptoCipherConstants { - static let rsaKeySizeInBits: NSNumber = 2048 - static let aesAlgorithm: CCAlgorithm = CCAlgorithm(kCCAlgorithmAES) - static let aesOptions: CCOptions = CCOptions(kCCOptionPKCS7Padding) - static let rsaAlgorithm: SecKeyAlgorithm = .rsaEncryptionOAEPSHA256 -} +private enum CryptoCipherAlgorithm { + case rsa(keySize: NSNumber = 2048, algorithm: SecKeyAlgorithm = .rsaEncryptionOAEPSHA256) + case aes(algorithm: CCAlgorithm = CCAlgorithm(kCCAlgorithmAES), options: CCOptions = CCOptions(kCCOptionPKCS7Padding)) +}ios/Plugin/CapacitorUpdaterPlugin.swift (1)
293-298: Improve error handling for decryption failures.The error handling for decryption operations could be more informative. Consider adding specific error types and logging.
+enum DecryptionError: Error { + case invalidChecksum(expected: String, actual: String) + case decryptionFailed(underlying: Error) + case invalidPublicKey +} do { checksum = try CryptoCipher.decryptChecksum(checksum: checksum, publicKey: self.implementation.publicKey, version: version) } catch { + let decryptError = DecryptionError.decryptionFailed(underlying: error) self.implementation.sendStats(action: "decrypt_fail", versionName: version) - throw error + throw decryptError }Also applies to: 812-817
ios/Plugin/CapacitorUpdater.swift (1)
29-30: Consider adding cache cleanup mechanism.The cacheFolder is created but there's no mechanism to clean up old cached files.
Add a cache cleanup method:
+private func cleanupCache(olderThan days: Int = 7) { + let fileManager = FileManager.default + guard let contents = try? fileManager.contentsOfDirectory(at: cacheFolder, includingPropertiesForKeys: [.creationDateKey]) else { return } + + let cutoff = Date().addingTimeInterval(-TimeInterval(days * 24 * 60 * 60)) + for url in contents { + guard let creation = try? url.resourceValues(forKeys: [.creationDateKey]).creationDate else { continue } + if creation < cutoff { + try? fileManager.removeItem(at: url) + } + } +}README.md (1)
105-107: Introduce the "appId" Configuration Option
The addition of the "appId" property in the provided configuration snippet replaces the deprecated "privateKey" option. This change streamlines configuration for users. Please ensure that the README documentation fully explains the purpose and usage of "appId" and that any outdated mentions of "privateKey" are removed elsewhere.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
106-106: Hard tabs
Column: 1(MD010, no-hard-tabs)
107-107: Hard tabs
Column: 1(MD010, no-hard-tabs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
.prettierignore(0 hunks)CapgoCapacitorUpdater.podspec(1 hunks)Package.swift(1 hunks)README.md(2 hunks)android/.project(1 hunks)android/build.gradle(3 hunks)android/src/main/java/ee/forgr/capacitor_updater/CapacitorUpdater.java(1 hunks)android/src/main/java/ee/forgr/capacitor_updater/CapacitorUpdaterPlugin.java(0 hunks)android/src/main/java/ee/forgr/capacitor_updater/CryptoCipher.java(3 hunks)android/src/main/java/ee/forgr/capacitor_updater/CryptoCipherV2.java(0 hunks)android/src/main/java/ee/forgr/capacitor_updater/DownloadService.java(2 hunks)android/src/main/java/ee/forgr/capacitor_updater/InternalUtils.java(0 hunks)api.md(0 hunks)ios/Plugin.xcodeproj/project.pbxproj(8 hunks)ios/Plugin/ASE128.swift(1 hunks)ios/Plugin/CapacitorUpdater.swift(6 hunks)ios/Plugin/CapacitorUpdaterPlugin.swift(2 hunks)ios/Plugin/CryptoCipher.swift(5 hunks)ios/Plugin/CryptoCipherV2.swift(0 hunks)ios/Podfile(1 hunks)package.json(3 hunks)src/definitions.ts(0 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (7)
- .prettierignore
- src/definitions.ts
- api.md
- ios/Plugin/CryptoCipherV2.swift
- android/src/main/java/ee/forgr/capacitor_updater/InternalUtils.java
- android/src/main/java/ee/forgr/capacitor_updater/CapacitorUpdaterPlugin.java
- android/src/main/java/ee/forgr/capacitor_updater/CryptoCipherV2.java
🔇 Additional comments (24)
android/src/main/java/ee/forgr/capacitor_updater/DownloadService.java (2)
412-412: LGTM: Consistent checksum calculation update.The change to use
CryptoCipher.calcChecksummaintains consistency with the other cryptographic operation changes.
393-394: Verify the security implications of the new CryptoCipher implementation.The migration from
CryptoCipherV2toCryptoCipherappears to be a drop-in replacement. However, we should verify that the new implementation maintains or improves upon the security guarantees of the previous version.Let's verify the implementation of the new CryptoCipher class:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check the implementation of the new CryptoCipher class # and any security-related changes in its implementation # Search for the CryptoCipher class implementation ast-grep --pattern 'class CryptoCipher { $$$ }' # Look for any security-related comments or annotations rg -i 'security|crypto|encrypt|decrypt' android/src/main/java/ee/forgr/capacitor_updater/CryptoCipher.javaLength of output: 3729
Security verification of CryptoCipher implementation
The new CryptoCipher implementation retains the drop‑in replacement design with methods matching those in CryptoCipherV2. Our review confirms that:
• The decryption methods (decryptFile, decryptChecksum) retain similar cryptographic operations, including Base64 encoding of outputs and use of RSA for what appears to be signature verification.
• Standard security APIs are employed (e.g., Cipher, X509EncodedKeySpec) with proper exception handling, indicating that error cases are logged and wrapped appropriately.
• Security‑related comments and references (e.g., the StackOverflow link) suggest that the approach has been verified against common Android cryptography practices.Overall, there is no evident regression in security from this migration.
ios/Plugin/CryptoCipher.swift (3)
9-9: Consider necessity of CryptoKit import.
Even though CryptoKit might be useful for certain cryptographic constructs (like SHA256 in Swift 5.0+), reviewing the rest of the file suggests you are still using CommonCrypto APIs for AES and RSA from iOS Security APIs. If CryptoKit usage is minimal, ensure that introducing this additional import is strictly beneficial (e.g., for improved performance or simpler usage).
52-75: Validate base64 and key format assumptions in 'load(rsaPublicKey:)'.
The function strips “BEGIN/END RSA PUBLIC KEY” markers and newlines, then decodes base64. If an incoming key includes unexpected formatting or extra whitespace, decoding could silently fail. Consider providing more robust error reporting or ensuring keys strictly match the expected PKCS#1 public key format to reduce confusion.
219-260: Verify session key and public key usage in 'decryptFile'.
- The method checks that the sessionKey is in a “IV:EncryptedKey” format and forcibly attempts RSA “decryption” of the second half. This approach is plausible for an RSA-encrypted symmetric key, but confirm that the remote side actually encrypts the session key with the private key to be decrypted by the public key.
- If the remote side is encrypting with a public key (the typical approach), then you should hold the matching private key for decryption.
android/src/main/java/ee/forgr/capacitor_updater/CryptoCipher.java (1)
136-175: Check non-empty result in 'decryptFile'.
After calling 'decryptRSA' with the public key, ensure the returned session key (decryptedSessionKey) is not null or empty before constructing the AES key. Without this, a subsequent AES decrypt might fail or cause an NPE.android/src/main/java/ee/forgr/capacitor_updater/CapacitorUpdater.java (1)
330-330: Unclear cryptographic flow in 'decryptFile' invocation.
Line 330 calls “decryptFile(downloaded, publicKey, sessionKey)” with a public key as the second argument. Standard RSA usage would typically require a private key for the decryption side. Confirm the upstream or server logic to ensure data is being encrypted in a way that can indeed be “decrypted” with this public key.CapgoCapacitorUpdater.podspec (1)
14-14: Verify iOS deployment target requirement for Capacitor 7.The iOS deployment target has been updated to 14.0. Let's verify this is the correct minimum requirement for Capacitor 7.
✅ Verification successful
🌐 Web query:
What is the minimum iOS version required for Capacitor 7?💡 Result:
Capacitor 7 requires **iOS 14.0 or later** as its minimum supported version[3][8]. This aligns with its dependency on Xcode 16.0+ for development[1][8]. Key details: - Official documentation explicitly lists iOS 14.0 as the minimum in Capacitor's platform support matrix[3] - The requirement was introduced in Capacitor 7's migration guide, which instructs developers to set the deployment target to iOS 14.0 in both Xcode settings and Podfile configurations[8] This represents an increase from previous versions: - Capacitor 6 required iOS 13.0[3] - Capacitor 5 supported down to iOS 12.0[5] Developers must ensure their Xcode projects and CocoaPods configurations specify at least iOS 14.0 when using Capacitor 7[8]. Apps built with this configuration won't install on devices running older iOS versions[5]. Citations: - 1: https://capacitorjs.com/docs/ios - 2: https://github.com/ionic-team/capacitor/discussions/5297 - 3: https://capacitorjs.com/docs/main/reference/support-policy - 4: https://community.wappler.io/t/confused-about-capacitor-for-ios-mobile-use/50171 - 5: https://forum.ionicframework.com/t/capacitor-ios-support/178000 - 6: https://capacitorjs.com/docs/v2/getting-started/dependencies - 7: https://ionic.io/blog/building-and-releasing-your-capacitor-ios-app - 8: https://capacitorjs.com/docs/updating/7-0 - 9: https://ionic.io/docs/portals/for-ios/upgrade-guides - 10: https://quasar.dev/quasar-cli-vite/developing-capacitor-apps/capacitor-version-support/
Verified iOS deployment target is correctly set for Capacitor 7.
- The deployment target in File: CapgoCapacitorUpdater.podspec (Line 14) is set to 14.0.
- Capacitor 7 requires iOS 14.0 or later, as confirmed by official documentation and migration guides.
android/.project (2)
8-12: LGTM! Standard Eclipse Java project configuration.The addition of Java builder and nature configurations are standard Eclipse project settings.
Also applies to: 20-20
25-25: LGTM! Updated resource filtering configuration.The filter ID update and escaped git pattern are valid Eclipse configuration changes.
Also applies to: 30-30
ios/Podfile (2)
1-1: LGTM! Platform version consistent with podspec.The iOS platform version is set to 14.0, matching the deployment target in the podspec file.
26-28: LGTM! Sensible deployment target update.Updating old deployment targets from 8.0 to 11.0 is a good practice, as iOS 8.0 is significantly outdated.
Package.swift (2)
6-6: LGTM! Consistent iOS platform requirement.The iOS platform requirement is set to v14, maintaining consistency with the podspec and Podfile.
13-13: LGTM! Explicit Capacitor 7 dependency.The Capacitor dependency is now explicitly set to version 7.0.0, which aligns with the PR objective of upgrading to Capacitor 7.
ios/Plugin.xcodeproj/project.pbxproj (1)
419-419: Verify iOS 14.0 requirement with stakeholders.The deployment target has been increased to iOS 14.0. This change might affect users on older iOS versions.
Also applies to: 479-479, 502-502, 527-527
✅ Verification successful
🌐 Web query:
What percentage of iOS users are on iOS 14.0 or higher in 2024?💡 Result:
As of February 2025 analysis looking back at 2024 data points from multiple sources: **Key findings:** **Apple official data** As of June 9, 2024: - **77%** used **iOS 17** - **86%** adoption rate among recent iPhones - **9%-15%** used older OS versions across devices[1][9] **Third-party analytics** TelemetryDeck reported: - **78%-83%** using **iOS 17** - **96%-97% combined share** for **iOS 16/17**[3][5] - **≤0.06%-2.5%** using **iOS 15 or earlier**[3][5] **Critical calculation** Since **iOS 13** reached **96%-98% adoption** historically[7][9], and only legacy devices like iPad mini 2/3 remained stuck below **iOS 12**[8], while modern devices automatically receive security updates until hardware limits[6]: > *Over* ***99%-100*** *of active* ***iPhones/iPads*** *ran* ***iOS 14+*** *in* ***2024*** *due to forced security updates and automatic OS upgrades.* Citations: - 1: https://9to5mac.com/2024/06/11/ios-17-adoption-rate/ - 2: https://telemetrydeck.com/survey/apple/iOS/majorSystemVersions/ - 3: https://telemetrydeck.com/blog/ios-market-share-02-24/ - 4: https://gs.statcounter.com/os-version-market-share/ios/mobile-tablet/worldwide - 5: https://telemetrydeck.com/blog/ios-market-share-04-24/ - 6: https://en.wikipedia.org/wiki/IOS_14 - 7: https://iosref.com/ios-usage - 8: https://discussions.apple.com/thread/255525229 - 9: https://www.statista.com/statistics/565270/apple-devices-ios-version-share-worldwide/ - 10: https://discussions.apple.com/thread/255180544
Deployment Target Review: iOS 14.0 is Safe to Use
The web query confirms that over 99% of active devices were running iOS 14 or later in 2024. Given this near-universal adoption, the change in the deployment target in ios/Plugin.xcodeproj (lines 419, 479, 502, 527) is appropriate. It does not significantly impact the user base. However, as noted, it's still recommended to get final stakeholder confirmation for any rare or legacy use cases.
tsconfig.json (1)
17-18: Enhance Type Checking Performance
The addition of the new "skipLibCheck": true option (with the corrected trailing comma on the "target" line) is a standard approach to speed up compilation by skipping type checking on declaration files. Ensure that your team is aware of the trade‐off, as it might hide errors in third-party libraries if they occur.android/build.gradle (4)
3-5: Update AndroidX Dependency Versions
The updates for "androidxAppCompatVersion", "androidxJunitVersion", and "androidxEspressoCoreVersion" to versions "1.7.0", "1.2.1", and "3.6.1" respectively are appropriate for keeping the project up to date. Please double-check that these versions are fully compatible with the rest of your dependency tree.
14-15: Update Gradle Plugin Version
The classpath change to 'com.android.tools.build:gradle:8.7.2' ensures support for newer Android build features and configurations. Verify that the updated plugin works seamlessly in your CI/CD pipeline and local builds.
22-25: Revise SDK and Minimum Version Settings
The updates to compileSdk (from 34 to 35), minSdkVersion (from 22 to 23), and targetSdkVersion (from 34 to 35) are in line with modern Android practices. Make sure you test on real devices/emulators running API level 23 and above to confirm that no runtime issues are introduced with the minSdk bump.
40-41: Bump Java Compatibility to Version 21
Switching both sourceCompatibility and targetCompatibility from JavaVersion.VERSION_17 to JavaVersion.VERSION_21 is a significant update. Confirm that all parts of the project and any integrated libraries support Java 21 features and that your build environment is properly configured for this version.package.json (3)
2-4: Upgrade Package Version
Bumping the package version to "7.0.0" aligns with the major changes introduced in this release. Be sure to update the changelog and notify consumers about any breaking changes.
61-67: Update Capacitor Dependencies and ESLint Version
The dependency updates for "@capacitor/android", "@capacitor/cli", "@capacitor/core", and "@capacitor/ios" to "^7.0.0", along with the minor adjustment to "eslint" (from "^8.57.1" to "^8.57.0"), indicate a thorough upgrade strategy. Verify that these updates integrate correctly with your build and test processes.
79-81: Revise Peer Dependency for Capacitor Core
Changing the peer dependency for "@capacitor/core" to ">=7.0.0" correctly reflects the updated requirement. Ensure that this change is also documented in your release notes and documentation for users upgrading from previous versions.README.md (1)
41-43: Refresh Compatibility Table
The compatibility table now clearly reflects the support for v7.. of Capacitor, while marking v6.. as "Critical bug only" and v5.. as "⚠️ Deprecated." This improved clarity is crucial for users; just double-check that all references in your documentation are consistent.
| /// The RSA public key. | ||
| /// | ||
| public struct RSAKeyPair { | ||
| private let privateKey: SecKey | ||
| public struct RSAPublicKey { | ||
| private let publicKey: SecKey | ||
|
|
||
| #if DEBUG | ||
| public var __debug_privateKey: SecKey { self.privateKey } | ||
| public var __debug_publicKey: SecKey { self.publicKey } | ||
| #endif | ||
|
|
||
| fileprivate init(privateKey: SecKey, publicKey: SecKey) { | ||
| self.privateKey = privateKey | ||
| fileprivate init(publicKey: SecKey) { | ||
| self.publicKey = publicKey | ||
| } | ||
|
|
||
| /// | ||
| /// Takes the data and uses the private key to decrypt it. | ||
| /// Takes the data and uses the public key to decrypt it. | ||
| /// Returns the decrypted data. | ||
| /// | ||
| public func decrypt(data: Data) -> Data? { | ||
| var error: Unmanaged<CFError>? | ||
| if let decryptedData: CFData = SecKeyCreateDecryptedData(self.privateKey, CryptoCipherConstants.rsaAlgorithm, data as CFData, &error) { | ||
| if error != nil { | ||
| return nil | ||
| } else { | ||
| return decryptedData as Data | ||
| do { | ||
| guard let decryptedData = RSAPublicKey.decryptWithRSAKey(data, rsaKeyRef: self.publicKey, padding: SecPadding()) else { | ||
| throw CustomError.cannotDecryptSessionKey | ||
| } | ||
| } else { | ||
|
|
||
| return decryptedData | ||
| } catch { |
There was a problem hiding this comment.
Question the usage of a public key for decryption.
In asymmetric cryptography, the standard flow is to encrypt data with a recipient’s public key and decrypt with the corresponding private key. Here, your struct “RSAPublicKey” has a “decrypt” function, which is unusually reversed. If you truly need to decrypt with a public key, confirm that the encryption was done with the corresponding private key (very niche scenario). Otherwise, clarify if you intended to rename this to “encrypt” or to store the private key instead for decryption.
| let dataSize = encryptedData.count / MemoryLayout<UInt8>.size | ||
|
|
||
| #if DEBUG | ||
| public var __debug_privateKey: SecKey { self.privateKey } | ||
| #endif | ||
| var encryptedDataAsArray = [UInt8](repeating: 0, count: dataSize) | ||
| (encryptedData as NSData).getBytes(&encryptedDataAsArray, length: dataSize) | ||
|
|
||
| fileprivate init(privateKey: SecKey) { | ||
| self.privateKey = privateKey | ||
| } | ||
| /// | ||
| /// Takes the data and uses the private key to decrypt it. | ||
| /// Returns the decrypted data. | ||
| /// | ||
| public func decrypt(data: Data) -> Data? { | ||
| var error: Unmanaged<CFError>? | ||
| if let decryptedData: CFData = SecKeyCreateDecryptedData(self.privateKey, CryptoCipherConstants.rsaAlgorithm, data as CFData, &error) { | ||
| if error != nil { | ||
| var decryptedData = [UInt8](repeating: 0, count: 0) | ||
| var idx = 0 | ||
| while idx < encryptedDataAsArray.count { | ||
| var idxEnd = idx + blockSize | ||
| if idxEnd > encryptedDataAsArray.count { | ||
| idxEnd = encryptedDataAsArray.count | ||
| } | ||
| var chunkData = [UInt8](repeating: 0, count: blockSize) | ||
| for i in idx..<idxEnd { | ||
| chunkData[i-idx] = encryptedDataAsArray[i] | ||
| } | ||
|
|
||
| var decryptedDataBuffer = [UInt8](repeating: 0, count: blockSize) | ||
| var decryptedDataLength = blockSize | ||
|
|
||
| let status = SecKeyDecrypt(rsaKeyRef, padding, chunkData, idxEnd-idx, &decryptedDataBuffer, &decryptedDataLength) | ||
| if status != noErr { | ||
| return nil | ||
| } else { | ||
| return decryptedData as Data | ||
| } | ||
| } else { | ||
| return nil | ||
| let finalData = removePadding(decryptedDataBuffer) | ||
| decryptedData += finalData | ||
|
|
||
| idx += blockSize | ||
| } | ||
| } | ||
|
|
||
| /// | ||
| /// Allows you to export the RSA public key to a format (so you can send over the net). | ||
| /// | ||
| public func export() -> Data? { | ||
| return privateKey.exportToData() | ||
| return Data(decryptedData) | ||
| } | ||
|
|
||
| /// | ||
| /// Allows you to load an RSA public key (i.e. one downloaded from the net). | ||
| /// | ||
| public static func load(rsaPrivateKey: String) -> RSAPrivateKey? { | ||
| var privKey: String = rsaPrivateKey | ||
| privKey = privKey.replacingOccurrences(of: "-----BEGIN RSA PRIVATE KEY-----", with: "") | ||
| privKey = privKey.replacingOccurrences(of: "-----END RSA PRIVATE KEY-----", with: "") | ||
| privKey = privKey.replacingOccurrences(of: "\\n+", with: "", options: .regularExpression) | ||
| privKey = privKey.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| do { | ||
| guard let rsaPrivateKeyData: Data = Data(base64Encoded: privKey) else { | ||
| throw CustomError.cannotDecode | ||
| } | ||
| guard let privateKey: SecKey = .loadPrivateFromData(rsaPrivateKeyData) else { | ||
| throw CustomError.cannotDecode | ||
| // code is copied from here: https://github.com/btnguyen2k/swiftutils/blob/88494f4c635b6c6d42ef0fb30a7d666acd38c4fa/SwiftUtils/RSAUtils.swift#L429 | ||
| private static func removePadding(_ data: [UInt8]) -> [UInt8] { | ||
| var idxFirstZero = -1 | ||
| var idxNextZero = data.count | ||
| for i in 0..<data.count { | ||
| if data[i] == 0 { | ||
| if idxFirstZero < 0 { | ||
| idxFirstZero = i | ||
| } else { | ||
| idxNextZero = i | ||
| break | ||
| } | ||
| } | ||
| return RSAPrivateKey(privateKey: privateKey) | ||
| } catch { | ||
| print("Error load RSA: \(error)") | ||
| return nil | ||
| } | ||
| if idxNextZero-idxFirstZero-1 == 0 { | ||
| idxNextZero = idxFirstZero | ||
| idxFirstZero = -1 | ||
| } | ||
| var newData = [UInt8](repeating: 0, count: idxNextZero-idxFirstZero-1) | ||
| for i in idxFirstZero+1..<idxNextZero { | ||
| newData[i-idxFirstZero-1] = data[i] | ||
| } | ||
| return newData | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor RSA chunked decryption and padding removal.
- The manual chunking in “decryptWithRSAKey” can be error-prone and is typically handled by higher-level APIs.
- “removePadding” attempts to search for zero bytes but may lead to partial or incorrect data if the PDF or binary content legitimately contains zero bytes.
Evaluate whether your remote side is definitely producing an “encrypted & zero-padded” sequence. If not, recheck the encryption flow. If you do rely on this manual approach, add robust test coverage to confirm that chunk boundary and embedded-zero scenarios are handled safely.
| public static byte[] decryptRSA(byte[] source, PublicKey publicKey) | ||
| throws NoSuchPaddingException, NoSuchAlgorithmException, InvalidAlgorithmParameterException, InvalidKeyException, IllegalBlockSizeException, BadPaddingException { | ||
| Cipher cipher = Cipher.getInstance("RSA/ECB/OAEPPadding"); | ||
| OAEPParameterSpec oaepParams = new OAEPParameterSpec( | ||
| "SHA-256", | ||
| "MGF1", | ||
| new MGF1ParameterSpec("SHA-256"), | ||
| PSource.PSpecified.DEFAULT | ||
| ); | ||
| cipher.init(Cipher.DECRYPT_MODE, privateKey, oaepParams); | ||
| return cipher.doFinal(source); | ||
| Cipher cipher = Cipher.getInstance("RSA/ECB/PKCS1Padding"); | ||
| cipher.init(Cipher.DECRYPT_MODE, publicKey); | ||
| byte[] decryptedBytes = cipher.doFinal(source); | ||
| return decryptedBytes; | ||
| } |
There was a problem hiding this comment.
Flag unusual decrypt-with-public-key usage.
As with the iOS side, typically RSA decryption uses the private key. Double-check that the remote data is “encrypted” by a corresponding private key in some custom scheme or if it’s standard RSA usage. If this is an inadvertent reversal of roles, fix it.
ios/Plugin/ASE128.swift
Outdated
| #if DEBUG | ||
| public var __debug_iv: Data { iv } | ||
| public var __debug_aes128Key: Data { aes128Key } | ||
| #endif |
There was a problem hiding this comment.
Security concern: Debug exposure of sensitive cryptographic material.
The debug properties expose the initialization vector and AES key. This could lead to security vulnerabilities if debug builds are released.
Consider removing these debug properties or adding additional safeguards:
-#if DEBUG
- public var __debug_iv: Data { iv }
- public var __debug_aes128Key: Data { aes128Key }
-#endif
+#if DEBUG
+ // Only expose these in debug builds with explicit user consent
+ public func debugInfo(withConfirmation confirmation: String) -> (iv: Data, key: Data)? {
+ guard confirmation == "I understand the security implications" else { return nil }
+ return (iv, aes128Key)
+ }
+#endif
ios/Plugin/ASE128.swift
Outdated
| public func decrypt(data: Data) -> Data? { | ||
| let encryptedData: UnsafePointer<UInt8> = (data as NSData).bytes.bindMemory(to: UInt8.self, capacity: data.count) | ||
| let encryptedDataLength: Int = data.count | ||
|
|
||
| if let result: NSMutableData = NSMutableData(length: encryptedDataLength) { | ||
| let keyData: UnsafePointer<UInt8> = (self.aes128Key as NSData).bytes.bindMemory(to: UInt8.self, capacity: self.aes128Key.count) | ||
| let keyLength: size_t = size_t(self.aes128Key.count) | ||
| let ivData: UnsafePointer<UInt8> = (iv as NSData).bytes.bindMemory(to: UInt8.self, capacity: self.iv.count) | ||
|
|
||
| let decryptedData: UnsafeMutablePointer<UInt8> = UnsafeMutablePointer<UInt8>(result.mutableBytes.assumingMemoryBound(to: UInt8.self)) | ||
| let decryptedDataLength: size_t = size_t(result.length) | ||
|
|
||
| var decryptedLength: size_t = 0 | ||
|
|
||
| let status: CCCryptorStatus = CCCrypt(CCOperation(kCCDecrypt), CryptoCipherConstants.aesAlgorithm, CryptoCipherConstants.aesOptions, keyData, keyLength, ivData, encryptedData, encryptedDataLength, decryptedData, decryptedDataLength, &decryptedLength) | ||
|
|
||
| if Int32(status) == Int32(kCCSuccess) { | ||
| result.length = Int(decryptedLength) | ||
| return result as Data | ||
| } else { | ||
| return nil | ||
| } | ||
| } else { | ||
| return nil | ||
| } | ||
| } |
There was a problem hiding this comment.
Memory safety concerns in cryptographic operations.
The current implementation has potential memory safety issues with pointer handling and buffer management.
- Add memory bounds checking before pointer operations
- Use
deferblocks to ensure proper cleanup - Add buffer overflow protection
public func decrypt(data: Data) -> Data? {
+ guard data.count > 0 else { return nil }
let encryptedData: UnsafePointer<UInt8> = (data as NSData).bytes.bindMemory(to: UInt8.self, capacity: data.count)
let encryptedDataLength: Int = data.count
if let result: NSMutableData = NSMutableData(length: encryptedDataLength) {
let keyData: UnsafePointer<UInt8> = (self.aes128Key as NSData).bytes.bindMemory(to: UInt8.self, capacity: self.aes128Key.count)
let keyLength: size_t = size_t(self.aes128Key.count)
let ivData: UnsafePointer<UInt8> = (iv as NSData).bytes.bindMemory(to: UInt8.self, capacity: self.iv.count)
+ defer {
+ // Cleanup sensitive data
+ result.resetBytes(in: NSRange(location: 0, length: result.length))
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public func decrypt(data: Data) -> Data? { | |
| let encryptedData: UnsafePointer<UInt8> = (data as NSData).bytes.bindMemory(to: UInt8.self, capacity: data.count) | |
| let encryptedDataLength: Int = data.count | |
| if let result: NSMutableData = NSMutableData(length: encryptedDataLength) { | |
| let keyData: UnsafePointer<UInt8> = (self.aes128Key as NSData).bytes.bindMemory(to: UInt8.self, capacity: self.aes128Key.count) | |
| let keyLength: size_t = size_t(self.aes128Key.count) | |
| let ivData: UnsafePointer<UInt8> = (iv as NSData).bytes.bindMemory(to: UInt8.self, capacity: self.iv.count) | |
| let decryptedData: UnsafeMutablePointer<UInt8> = UnsafeMutablePointer<UInt8>(result.mutableBytes.assumingMemoryBound(to: UInt8.self)) | |
| let decryptedDataLength: size_t = size_t(result.length) | |
| var decryptedLength: size_t = 0 | |
| let status: CCCryptorStatus = CCCrypt(CCOperation(kCCDecrypt), CryptoCipherConstants.aesAlgorithm, CryptoCipherConstants.aesOptions, keyData, keyLength, ivData, encryptedData, encryptedDataLength, decryptedData, decryptedDataLength, &decryptedLength) | |
| if Int32(status) == Int32(kCCSuccess) { | |
| result.length = Int(decryptedLength) | |
| return result as Data | |
| } else { | |
| return nil | |
| } | |
| } else { | |
| return nil | |
| } | |
| } | |
| public func decrypt(data: Data) -> Data? { | |
| guard data.count > 0 else { return nil } | |
| let encryptedData: UnsafePointer<UInt8> = (data as NSData).bytes.bindMemory(to: UInt8.self, capacity: data.count) | |
| let encryptedDataLength: Int = data.count | |
| if let result: NSMutableData = NSMutableData(length: encryptedDataLength) { | |
| let keyData: UnsafePointer<UInt8> = (self.aes128Key as NSData).bytes.bindMemory(to: UInt8.self, capacity: self.aes128Key.count) | |
| let keyLength: size_t = size_t(self.aes128Key.count) | |
| let ivData: UnsafePointer<UInt8> = (iv as NSData).bytes.bindMemory(to: UInt8.self, capacity: self.iv.count) | |
| defer { | |
| // Cleanup sensitive data | |
| result.resetBytes(in: NSRange(location: 0, length: result.length)) | |
| } | |
| let decryptedData: UnsafeMutablePointer<UInt8> = UnsafeMutablePointer<UInt8>(result.mutableBytes.assumingMemoryBound(to: UInt8.self)) | |
| let decryptedDataLength: size_t = size_t(result.length) | |
| var decryptedLength: size_t = 0 | |
| let status: CCCryptorStatus = CCCrypt(CCOperation(kCCDecrypt), CryptoCipherConstants.aesAlgorithm, CryptoCipherConstants.aesOptions, keyData, keyLength, ivData, encryptedData, encryptedDataLength, decryptedData, decryptedDataLength, &decryptedLength) | |
| if Int32(status) == Int32(kCCSuccess) { | |
| result.length = Int(decryptedLength) | |
| return result as Data | |
| } else { | |
| return nil | |
| } | |
| } else { | |
| return nil | |
| } | |
| } |
ios/Plugin/CapacitorUpdater.swift
Outdated
| private func verifyChecksum(file: URL, expectedHash: String) -> Bool { | ||
| let actualHash = CryptoCipherV2.calcChecksum(filePath: file) | ||
| let actualHash = CryptoCipher.calcChecksum(filePath: file) | ||
| return actualHash == expectedHash | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for checksum calculation.
The checksum calculation doesn't handle potential errors.
private func verifyChecksum(file: URL, expectedHash: String) -> Bool {
- let actualHash = CryptoCipher.calcChecksum(filePath: file)
- return actualHash == expectedHash
+ guard let actualHash = CryptoCipher.calcChecksum(filePath: file) else {
+ print("\(CapacitorUpdater.TAG) Failed to calculate checksum for file: \(file)")
+ return false
+ }
+ return actualHash == expectedHash
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private func verifyChecksum(file: URL, expectedHash: String) -> Bool { | |
| let actualHash = CryptoCipherV2.calcChecksum(filePath: file) | |
| let actualHash = CryptoCipher.calcChecksum(filePath: file) | |
| return actualHash == expectedHash | |
| } | |
| private func verifyChecksum(file: URL, expectedHash: String) -> Bool { | |
| guard let actualHash = CryptoCipher.calcChecksum(filePath: file) else { | |
| print("\(CapacitorUpdater.TAG) Failed to calculate checksum for file: \(file)") | |
| return false | |
| } | |
| return actualHash == expectedHash | |
| } |
|
close in favor of other one |
Summary by CodeRabbit
New Features
Updates
Documentation
privateKeyconfiguration option in documentation.