Skip to content

Fixes #3855-Update MASTG-KNOW-0048.md#3856

Open
simge-yigit wants to merge 5 commits into
OWASP:masterfrom
simge-yigit:patch-1
Open

Fixes #3855-Update MASTG-KNOW-0048.md#3856
simge-yigit wants to merge 5 commits into
OWASP:masterfrom
simge-yigit:patch-1

Conversation

@simge-yigit

Copy link
Copy Markdown
Collaborator

Clarified KeyChain usage and access permissions, emphasizing secure handling of private keys and certificates. Fixes #3855

This PR closes #3855

Description

This fixes the three issues flagged in #3855:

-Dropped the store() section — it doesn't apply to AndroidKeyStore, so it was just creating confusion.

-The old claim that "every application can access KeyChain" was misleading. Clarified that apps actually need user consent through choosePrivateKeyAlias(), and that private keys are never handed over as raw material.

-Narrowed the whole document to KeyChain specifically — swapped out AndroidKeyStore checks for KeyChain-native verification steps, and added getSecurityLevel() (API 31+) next to the now-deprecated isInsideSecureHardware().


AI Tool Disclosure

Check exactly one option.

  • [ x] This contribution does not include AI-generated content.
  • This contribution includes AI-generated content.

If AI tools were used to generate or substantially modify code or text, complete the following.

  • AI tools used: e.g. ChatGPT, GitHub Copilot, Claude.
  • Models and versions: e.g. GPT-, Claude .
  • Prompt summary: brief description of the key prompts or instructions.
  • Your mobile security expertise level: low, medium, high.

For first-time contributors using AI tools.

  • Provide an export of the AI chat or session, for example a shared link or PDF attachment.
  • Ensure the commit history shows an initial commit with AI-generated content followed by commits that demonstrate review, correction, and improvement.

Undisclosed use of AI tools will result in the PR being closed. Large rewrites or bulk changes generated by AI require explicit prior approval from the maintainers. Learn more in "Use of AI tools in contributions".


Contributor Checklist

  • [x ] I have read and understood the contributing guidelines.
  • [x ] I followed the project style guide.
  • [x ] I validated the technical correctness of my changes and understand the topic.
  • [x ] This PR adds clear value and is not spam or low-effort content.

Relevant documentation.

Contributors are expected to understand basic git and GitHub workflows, including forks, branches, commits, and pull requests. The project does not provide training. Pull requests that do not meet these minimum requirements may be closed without review.

Clarified KeyChain usage and access permissions, emphasizing secure handling of private keys and certificates.
Fixes OWASP#3855
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

Thank you for your contribution! However, you are not assigned to any of the linked issues:

To contribute to this project, please:

  1. Request to be assigned to the issue you want to work on
  2. Wait for a maintainer to assign you
  3. Reopen this PR once you are assigned

This helps us coordinate contributions and avoid duplicate work.

@github-actions github-actions Bot closed this Jun 5, 2026
@cpholguera cpholguera reopened this Jun 6, 2026
@cpholguera

Copy link
Copy Markdown
Collaborator

Thanks for raising this. I agree with the core issue. The original text mixed AndroidKeyStore, KeyChain, and file based Java KeyStore concepts.

AndroidKeyStore should not be described as something the app saves to disk, and KeyChain access is mediated by user consent or device policy rather than being globally readable by every app.

That said, I do not think the current PR fully resolves the conceptual split yet. The revised text still risks presenting KeyChain as the general solution for protecting app sensitive data. I think we should separate the guidance more clearly. Use AndroidKeyStore for app owned cryptographic keys used to protect local data, and use KeyChain only for user selected, system wide private key and certificate credentials. This can be clarified in a note here and adding a link with @MASTG-KNOW to the corresponding article.

One more thing. Since the revised text makes several specific Android API and security claims, it would be good to add links to the relevant official Android documentation while we are touching this section. Even if the previous version was under referenced, this is a good chance to cite the relevant official docs. Thank you!

Add explanation of KeyChain vs AndroidKeyStore differences and references
Add explanation of KeyChain vs AndroidKeyStore differences and reference
@cpholguera cpholguera marked this pull request as draft June 7, 2026 05:45
@cpholguera

Copy link
Copy Markdown
Collaborator

Converting to draft until it follows the content guidelines.

@simge-yigit

Copy link
Copy Markdown
Collaborator Author

@cpholguera,
Thank you for the review.Could you please clarify which content guidelines are not being followed?

@cpholguera

Copy link
Copy Markdown
Collaborator

Hi @simge-yigit , please check them out here:

https://mas.owasp.org/contributing/writing-content/mastg-knowledge.instructions/

You should be able to quickly identify the issue. Otherwise let me know. Thank you!

@cpholguera

Copy link
Copy Markdown
Collaborator

We'd appreciate if you indicate what's not according to the guidelines and fix one thing at a time (one commit per fix). This will help us better review your PR and for you to familiarize yourself with the project. Thank you!

@simge-yigit

Copy link
Copy Markdown
Collaborator Author

Thank you for your recommendation.

When I reviewed the documentation again, I noticed that I had included some content with a test-guide nature on the knowledge page. Therefore, I first removed those sections and then carried out a general revision of the document in order to explain the KeyChain concept in a more comprehensive way.

The reason I did not make such an extensive revision at the beginning was that the first version of the text had already been accepted as a PR. At that stage, I treated it as a template and continued following the same structure. However, according to the knowledge documentation standards, I later reviewed the content again and adjusted it to make it more appropriate.

I did not go into detail about the difference between Android Keystore and KeyChain because I believe the current text explains KeyChain sufficiently. I would appreciate your feedback on this point.

@simge-yigit simge-yigit marked this pull request as ready for review June 7, 2026 16:58
@cpholguera cpholguera requested a review from TheDauntless June 8, 2026 08:13
@cpholguera

Copy link
Copy Markdown
Collaborator

Hi @simge-yigit , I've assigned @TheDauntless to review this and the other PR you opened. He'll soon give you feedback, thank you!

@simge-yigit

simge-yigit commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator Author

Hi @TheDauntless, I hope you're doing well. I just wanted to kindly follow up regarding the review of this PR whenever you have time. Looking forward to your feedback. Thank you

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.

MASTG-KNOW-0048: contradictory store() guidance for AndroidKeyStore + misleading KeyChain access claim

2 participants