Removed encryption padding for PoP flow #2203
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
iamgusain
reviewed
Oct 12, 2023
Comment on lines
436
to
438
| if (enableImport && Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { | ||
| purposes |= KeyProperties.PURPOSE_WRAP_KEY; | ||
| } |
There was a problem hiding this comment.
We don't need the PURPOSE_WRAP_KEY as well in this case.
iamgusain
approved these changes
Oct 12, 2023
iamgusain
reviewed
Oct 12, 2023
| @@ -450,10 +431,8 @@ private void initialize28(@androidx.annotation.NonNull final KeyPairGenerator ke | |||
| final boolean useStrongbox, | |||
| final boolean enableImport, | |||
| final boolean trySetAttestationChallenge) throws InvalidAlgorithmParameterException { | |||
| int purposes = KeyProperties.PURPOSE_SIGN | |||
There was a problem hiding this comment.
we also need this change in initialize23 method
fadidurah
approved these changes
Oct 17, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue description : When a device is newly Intune enrolled in A14, the user is able to login to Defender, but unable to retrieve the pop access token. As a fallback, Defender allows onboarding without pop token (which is why you were able to onboard fully to Defender). However, in absence of the correct token Defender backend does not acknowledge heartbeat calls we send every few hours to maintain device compliance. Heartbeat failure subsequently fails the signals being sent to Intune backend marking the device non-compliant (Device_Threat_Health_Not_Reported reported by Intune).
This is an issue especially when policies are setup to make the device noncompliant if Defender is not setup (this is what Microsoft has setup internally).
Bug link : https://identitydivision.visualstudio.com/Engineering/_workitems/edit/2574078
Previous temporary fix : #2053
The fix in the PR above was only to catch the exception and swallow it and skip trying to use strong box for key pair generation.
Current fix : After I raised this issue with google, they suggested that we should reduce the keyParameters and try. By removing the encryption paddings, the issue seems to be resolved. Also checked with Amit and it seems we are not doing any encryption or decryption using the key pair generated. We are only signing. So, I also removed some of the purposes like VERIFY, ENCRYPT and DECRYPT. While I am still following up with them on why it is specifically failing on Android 14, I have applied the fix they suggested.
Testing : Tested with MsalTestApp with AuthScheme as POP and verified that acquireToken interactive and silent flows are working as expected. Also, I ran the UI tests for POP.
Note : This fix is yet to be verified by Defender team.