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

Removing thumbprint check from PKeyAuth challenge #1828

Merged
merged 7 commits into from
May 25, 2023

Conversation

melissaahn
Copy link
Contributor

@melissaahn melissaahn commented May 19, 2023

Summary

Please see the common PR for description: AzureAD/microsoft-authentication-library-common-for-android#2045

@github-actions github-actions bot added the msal label May 19, 2023
@melissaahn melissaahn added No-Changelog This change does not update the changelog. and removed msal labels May 19, 2023
@melissaahn melissaahn changed the base branch from dev to melissaahn/VerifyBrokerWithSha2 May 19, 2023 22:47
@melissaahn melissaahn marked this pull request as ready for review May 19, 2023 23:28
@melissaahn melissaahn requested a review from a team as a code owner May 19, 2023 23:28
@rpdome
Copy link
Member

rpdome commented May 22, 2023

Please double check the failing tests.

Copy link
Member

@rpdome rpdome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like it's probably caused by the order you pushed the branch. once you get the common in and have this pointed to dev... it's probably solved.

@melissaahn melissaahn changed the base branch from melissaahn/VerifyBrokerWithSha2 to dev May 25, 2023 17:02
melissaahn added a commit to AzureAD/microsoft-authentication-library-common-for-android that referenced this pull request May 25, 2023
### Summary
In the PKeyAuth protocol, the non-interactive flow can send a
[thumbprint-based certificate
challenge](https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-pkap/31066cbf-2462-4cb6-b0b9-68af2f21a5d0).
This thumbprint is meant to serve as a hint to the client side to
cross-check the thumbprint of the current device certificate.
While we have logic for this check, it currently isn't being used due to
[this line always returning
true](https://github.com/AzureAD/microsoft-authentication-library-common-for-android/blob/478f706bf412b3bcc754ec90152568cb23826670/common4j/src/main/com/microsoft/identity/common/java/challengehandlers/PKeyAuthChallenge.java#L148)
(go into the isValidIssuer method to see why).
Additionally, the thumbprint is (supposedly, based on me asking some
folks) hashed with SHA-1, and we're currently working to remove most of
our use of SHA-1. [The iOS team has already removed their thumbprint
verification
logic](AzureAD/microsoft-authentication-library-for-objc#871),
so we're going to do the same.
This PR removes the logic related to the PKeyAuth thumbprint. No
additions are being made.

### Related PRs
- Broker: AzureAD/ad-accounts-for-android#2290
- MSAL:
AzureAD/microsoft-authentication-library-for-android#1828
- ADAL:
AzureAD/azure-activedirectory-library-for-android#1733
@melissaahn melissaahn merged commit 0e65b1f into dev May 25, 2023
5 checks passed
@melissaahn melissaahn deleted the melissaahn/RemoveThumbprintPKeyAuth branch May 25, 2023 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No-Changelog This change does not update the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants