ROBERT SDK for iOS issueshttps://gitlab.inria.fr/stopcovid19/stopcovid-robertsdk-ios/-/issues2020-05-14T09:59:07+02:00https://gitlab.inria.fr/stopcovid19/stopcovid-robertsdk-ios/-/issues/10Google reCAPTCHA2020-05-14T09:59:07+02:00Florent MorinGoogle reCAPTCHAWhen using `Google reCAPTCHA` _(as presented in server part...)_, app requires a `WKWebView` component.
And this part of the app can't use SSL Pinning due to external ownership.
So it's an exposure to many potential security breaches:
...When using `Google reCAPTCHA` _(as presented in server part...)_, app requires a `WKWebView` component.
And this part of the app can't use SSL Pinning due to external ownership.
So it's an exposure to many potential security breaches:
* JS injection via MitM
* exploit by using a WebKit security breach, especially if iOS is not updated with last update
* replace captcha code (by MitM attack) to solve external captcha.
And, if reCAPTCHA is used for authentication _(and not human detection, its primary goal)_, it can be solved by any "Mechanical Turk" API. (I use this to automate my unit tests...)
Benefits / risks balance is not clear.https://gitlab.inria.fr/stopcovid19/stopcovid-robertsdk-ios/-/issues/9Some code style remarks2020-05-13T23:15:04+02:00Adam ViaudSome code style remarksCode looks clean overall but I want to point out a few things:
* why not use an uninhabited type like an empty enum (enum with no cases) in **RBConstants.swift** to prevent instantiations of the `RBConstants` type ?
* in **RBManager.swi...Code looks clean overall but I want to point out a few things:
* why not use an uninhabited type like an empty enum (enum with no cases) in **RBConstants.swift** to prevent instantiations of the `RBConstants` type ?
* in **RBManager.swift** which seems to describe a singleton, why not clearly add a private initializer to make sure that only one instance will be created ?
* you seem to retrieve timestamps in a few places (in **RBManager.swift** or **Message/MessageManager.swift**) using a helper `timeIntervalSince1900` of `Date` which returns an `Int`, so why those are wrapped again in Int values like this: `Int(Date().timeIntervalSince1900)` ?https://gitlab.inria.fr/stopcovid19/stopcovid-robertsdk-ios/-/issues/7OWASP MSTG2020-06-05T15:24:13+02:00Florent MorinOWASP MSTGIt can be a good thing to follow [OWASP MSTG](https://github.com/OWASP/owasp-mstg) for mobile security.
Especially checklists for auto-diagnostic.It can be a good thing to follow [OWASP MSTG](https://github.com/OWASP/owasp-mstg) for mobile security.
Especially checklists for auto-diagnostic.https://gitlab.inria.fr/stopcovid19/stopcovid-robertsdk-ios/-/issues/6`CryptoKit` integration2020-06-05T15:23:54+02:00Florent Morin`CryptoKit` integration`CryptoKit` is more performant, easier and secured than `CommonCrypto` since iOS 13.
And iOS 13 is used by large majority of users.
It's probably possible to use `CryptoKit` "if available".`CryptoKit` is more performant, easier and secured than `CommonCrypto` since iOS 13.
And iOS 13 is used by large majority of users.
It's probably possible to use `CryptoKit` "if available".https://gitlab.inria.fr/stopcovid19/stopcovid-robertsdk-ios/-/issues/5Good job 😁2020-05-12T20:47:00+02:00Florent MorinGood job 😁Code is well written.
Syntax is clear.Code is well written.
Syntax is clear.https://gitlab.inria.fr/stopcovid19/stopcovid-robertsdk-ios/-/issues/4Bluetooth messages security for privacy2020-06-05T15:23:28+02:00Florent MorinBluetooth messages security for privacyIt doesn't appear in source code.
What about bluetooth tracking limitation for privacy?
Did you plan to ignore some messages to avoid sniffing?
Did you plan to apply a delta in epoch generation to avoid tracking?
Thanks.It doesn't appear in source code.
What about bluetooth tracking limitation for privacy?
Did you plan to ignore some messages to avoid sniffing?
Did you plan to apply a delta in epoch generation to avoid tracking?
Thanks.https://gitlab.inria.fr/stopcovid19/stopcovid-robertsdk-ios/-/issues/3Lots of `var` declarations for non-modified values2020-05-12T20:46:08+02:00Florent MorinLots of `var` declarations for non-modified valuesAll is described in the title: `var` can probably be replaced by `let`.
Especially in models, for security reasons.
Perhaps it's due to "develop" phase.All is described in the title: `var` can probably be replaced by `let`.
Especially in models, for security reasons.
Perhaps it's due to "develop" phase.https://gitlab.inria.fr/stopcovid19/stopcovid-robertsdk-ios/-/issues/2`print()` everywhere2020-05-12T20:45:35+02:00Florent Morin`print()` everywhereFor security reasons, `print()` should disappear. (breakpoints are also good for debugging 😉)For security reasons, `print()` should disappear. (breakpoints are also good for debugging 😉)https://gitlab.inria.fr/stopcovid19/stopcovid-robertsdk-ios/-/issues/1Integrate as a framework2020-05-12T20:44:52+02:00Florent MorinIntegrate as a frameworkCode looks like an extraction of the app project and not a code for a framework.
So, excuse me if it's already integrated as a framework.
For runtime security, it seems to be a minimum requirement to integrate the sensitive part of the ...Code looks like an extraction of the app project and not a code for a framework.
So, excuse me if it's already integrated as a framework.
For runtime security, it seems to be a minimum requirement to integrate the sensitive part of the code as an internal framework.
Like this, ACL can apply securely, by limiting the risk of security error.
Only `public` or `open` code will be visible inside the sandboxed environment at runtime.
And, for example, if a malware execute runtime in the app, this protection will ensure another level of security.