ChristopherA
commented
2 years ago We have an initial quote for a doing a review. We don't need to do these all at once, we can stage them.
***Security*** Estimated Total: 5.5 hours Trevor says: There’s comparatively little surface area that needs review in the context of specific security trade-offs (as opposed to reviewing API usage correctness), so these are the only items that stand out independently of what Greg already identified. Under Greg’s section 7 “iCloud sync/keychain interface” (see below): • confirm user expectations around partial-failure reporting and review saving/syncing operations as appropriate (2 hours). • confirm product commitments and user expectations around securely/robustly syncing with iCloud, review Apple documentation for compatibility and in-app user messaging for compliance (2 hours, may require additional time if questions need to be submitted in an Apple DTS). Under Greg’s section 8 “Password/FaceID/Fingerprint”: • confirm user expectations around statements like “protects them with 2FA and biometrics” with client and review implementation for compliance (1.5 hours) ***General*** Greg says: The SeedTool code base itself is quite large (in terms of the number of source files) but each source file is small and encapsulates a single concept. There are 162 Swift source files in the main app. The workspace also uses 28 Swift packages, 17 are referenced directly. Many of these packages are published by BlockchainCommons, some are from other repositories and a number are published by the app developer (Wolf McNally). I have not included reviewing these packages as part of this scoping effort. Overall, this does seem on the surface like a well implemented application that follows common Swift coding practices. The use of iOS APIs is relatively straight forward and the app conforms to good practices (there are no build or runtime warnings). Given that the app is written in SwiftUI there is probably a reasonable path to making a native Mac app rather than using Mac Catalyst (I suspect that the reason they choose Catalyst is to minimize the changes required to use the native Mac versions of some APIs). The only major deficit that I see is the lack of API documentation (i.e. DocC style method documentation) which might be helpful for a developer using this as a reference implementation. I am not sure that we can provide much value by doing an in-depth code review of SeedTool from a development structure, iOS API use, and best practices point of view. It is possible that a more detailed investigation would expose some optimization potential but I didn’t see anything obvious in my initial pass through the code. If we do want to proceed with a code review, I would propose reviewing the following areas: 1) Overall app design/implementation (4 - 8 hours) - review overall app design - build and run looking for system errors - file by file survey of code for incomplete implementations or obvious inefficiencies - use performance/debug tools to look for memory leaks, energy usage 2) Printing (2 - 4 hours) - review the use of Apple’s printing APIs 3) Camera (1 - 2 hours) - review the use of Apple’s camera APIs - review of image processing code associated with QR code scanning 4) Clipboard (1 - 2 hours) - Review clipboard usage 5) Share Sheets (1 - 2 hours) - Save Image - Copy to Clipboard - Save to Files (including MicroSD) - Print 6) NFC (1 - 2 hours) - review use of Apple’s NFC API 7) iCloud sync/keychain interface (8 - 16 hours) - review use of Apple APIs - verify CloudKit storage using CloudKit Console - Trevor will review the security aspects as a separate task 8) Password/FaceID/Fingerprint (1 - 2 hours) - review use of Apple APIs - Trevor will review security aspects of using the results of the authentication as a separate task. 9) Image handling (2 - 4 hours) - review image code for the “Save Image” operation - review image code for generating QR codes and icons The total review time for the General items would be 20 - 40 hours although the upper end of that is likely fairly generous. The milestones could be as fine grained as the numbered items although some items could be grouped together.
wolfmcnally
shannona
Our goal in this project is to get an independent, third-party review of the Gordian Seed Tool code base, which we can make public, not only to assess and reassure users of our reference code and application, but to also establish our own expectations of the best practices for other cryptographic tools.
Initially, our goal is not to audit all the code, but to instead focus on being sure that we are meeting or exceeding the best practices of leveraging iOS APIs for secure use of iCloud & Keychain.