Skip to content
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

implement android keystore to prevent unwarranted access to private key #35

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

harlo
Copy link
Member

@harlo harlo commented Oct 15, 2015

(harlo#3)

While access to the internal data is protected on non-rooted devices, a rooted device could potentially access the contents of the iocipher storage and abuse the private key.

Proposed fixes:

  1. use android keystore to further protect access to private key's credentials, that way it can only be accessed in-app

We should further discuss properly using Android Keystore, especially in relation to rooted devices.

@n8fr8
Copy link
Member

n8fr8 commented Oct 15, 2015

How does this interact with Cacheword? That is controlling the key used to unlock IOCipher. Shouldn't we implement this capability in Cacheword itself, and not just in CameraV?

@eighthave
Copy link
Member

yes it would be great if cacheword used the Android Keystore, as long as it provides a real security improvement and doesn't harm usability. I haven't looked too deeply into the Android KeyStore stuff, so I can't say too much about it.

Here's what Abel has to say on the topic:
https://dev.guardianproject.info/issues/3119

@harlo
Copy link
Member Author

harlo commented Oct 23, 2015

Agree, but I didn't want to touch Cacheword because I am not as familiar with the codebase. Also worth noting, it appears that my upgrade to Marshmallow borked this feature as I implemented it (although working fine on pre-6 devices). Will follow up in coming days...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants