-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proofreading fixes 0x04e #2278
Proofreading fixes 0x04e #2278
Conversation
Document/0x04e-Testing-Authentication-and-Session-Management.md
Outdated
Show resolved
Hide resolved
Document/0x04e-Testing-Authentication-and-Session-Management.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Laancelot, could you please take a look at my suggestions? We can fix this section by removing a lot of content. Thank you!
Document/0x04e-Testing-Authentication-and-Session-Management.md
Outdated
Show resolved
Hide resolved
Document/0x04e-Testing-Authentication-and-Session-Management.md
Outdated
Show resolved
Hide resolved
Document/0x04e-Testing-Authentication-and-Session-Management.md
Outdated
Show resolved
Hide resolved
Document/0x04e-Testing-Authentication-and-Session-Management.md
Outdated
Show resolved
Hide resolved
Document/0x04e-Testing-Authentication-and-Session-Management.md
Outdated
Show resolved
Hide resolved
Commit suggestion Co-authored-by: Carlos Holguera <perezholguera@gmail.com>
Commit suggestion Co-authored-by: Carlos Holguera <perezholguera@gmail.com>
Commit suggestion Co-authored-by: Carlos Holguera <perezholguera@gmail.com>
Commit suggestion Co-authored-by: Carlos Holguera <perezholguera@gmail.com>
|
||
In Android, the developer can use `Settings.Secure.ANDROID_ID` till Android 8.0 (API level 26) to identify an application instance. Note that starting at Android 8.0 (API level 26), `ANDROID_ID` is no longer a device unique ID. Instead, it becomes scoped by the combination of app signing key, user and device. So validating `ANDROID_ID` for device blocking could be tricky for these Android versions. Because if an app changes its signing key, the `ANDROID_ID` will change and it won't be able to recognize old users devices. Therefore, it's better to store the `ANDROID_ID` encrypted and privately in a private a shared preferences file using a randomly generated key from the `AndroidKeyStore` and preferably AES_GCM encryption. The moment the app signature changes, the application can check for a delta and register the new `ANDROID_ID`. The moment this new ID changes without a new application signing key, it should indicate that something else is wrong. | ||
Next, the device binding can be extended by signing requests with a key stored in the `Keychain` for iOS and in the `KeyStore` in Android can reassure strong device binding. | ||
You should also test if using different IPs, different locations and/or different time-slots will trigger the right type of information in all scenarios. | ||
|
||
Lastly, the blocking of the devices should be tested, by blocking a registered instance of the app and see if it is then no longer allowed to authenticate. | ||
Note: in case of an application which requires L2 protection, it can be a good idea to warn a user even before the first authentication on a new device. Instead: warn the user already when a second instance of the app is registered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still find that sentence unclear? Sorry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one "Note: in case of an application which requires L2 protection, it can be a good idea to warn a user even before the first authentication on a new device. Instead: warn the user already when a second instance of the app is registered."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me too, let's just remove them as I suggested in https://github.com/OWASP/owasp-mastg/pull/2278/files#r996281939. Line 561 is now taking care of this.
Removing one extra line
Document/0x04e-Testing-Authentication-and-Session-Management.md
Outdated
Show resolved
Hide resolved
Document/0x04e-Testing-Authentication-and-Session-Management.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great improvements @Laancelot! Thanks!
With this we can merge, please take a look at the last 2 changes I've made to remove those lines. |
typos, links
Thank you for submitting a Pull Request to the OWASP MASTG. Please make sure that:
If your PR is related to an issue. Please end your PR test with the following line:
This PR closes #< insert number here >.