From 722ca625d090c9e6830a82a0a050efd201b73f5e Mon Sep 17 00:00:00 2001 From: jennyf19 Date: Thu, 19 Sep 2019 11:50:48 -0700 Subject: [PATCH] Jennyf/ios13 (#1655) * Part 1: ios 13 - Generate a nonce to send in the broker request, only if msauthv3 (iOS broker 6.3.19+) is installed on the device - Check that the nonce in the broker response matches the nonce sent in the request - Update info.plist in dev app for msauthv3 * more updates * pr feedback --- devApps/XFormsApp.iOS/Info.plist | 1 + .../Exceptions/AdalError.cs | 5 ++ .../Exceptions/AdalErrorMessage.cs | 2 + .../Internal/Flows/BrokerParameter.cs | 8 +-- .../iOS/AuthenticationContinuationHelper.cs | 20 +++++- .../Platforms/iOS/iOSBroker.cs | 62 +++++++++++++++++-- 6 files changed, 88 insertions(+), 10 deletions(-) diff --git a/devApps/XFormsApp.iOS/Info.plist b/devApps/XFormsApp.iOS/Info.plist index fba08066b..fd2e7286e 100644 --- a/devApps/XFormsApp.iOS/Info.plist +++ b/devApps/XFormsApp.iOS/Info.plist @@ -24,6 +24,7 @@ LSApplicationQueriesSchemes msauth + msauthv3 CFBundleURLTypes diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Exceptions/AdalError.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Exceptions/AdalError.cs index dedd378b5..6692247cd 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Exceptions/AdalError.cs +++ b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Exceptions/AdalError.cs @@ -272,6 +272,11 @@ public static class AdalError /// public const string BrokerReponseHashMismatch = "broker_response_hash_mismatch"; + /// + /// Broker response nonce does not match the request nonce sent by MSAL.NET for iOS broker >= v6.3.19 + /// + public const string BrokerNonceMismatch = "broker_nonce_mismatch"; + /// /// Device certificate not found. /// diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Exceptions/AdalErrorMessage.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Exceptions/AdalErrorMessage.cs index 126a48362..9e96362d0 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Exceptions/AdalErrorMessage.cs +++ b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Exceptions/AdalErrorMessage.cs @@ -89,6 +89,8 @@ internal static class AdalErrorMessage public const string RedirectUriContainsFragment = "'redirectUri' must NOT include a fragment component"; public const string ServiceReturnedError = "Service returned error. Check InnerException for more details"; public const string BrokerReponseHashMismatch = "Unencrypted broker response hash did not match the expected hash"; + public const string BrokerNonceMismatch = "Broker response nonce does not match the request nonce sent by MSAL.NET." + + "Please see https://aka.ms/adal-net-ios-13-broker for more details. "; public const string StsMetadataRequestFailed = "Metadata request to Access Control service failed. Check InnerException for more details"; diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Flows/BrokerParameter.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Flows/BrokerParameter.cs index 34ce9c5dc..c0b00db97 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Flows/BrokerParameter.cs +++ b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Flows/BrokerParameter.cs @@ -1,8 +1,5 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. namespace Microsoft.IdentityModel.Clients.ActiveDirectory.Internal.Flows { @@ -21,5 +18,6 @@ internal static class BrokerParameter public const string Claims = "claims"; public const string SilentBrokerFlow = "silent_broker_flow"; public const string BrokerInstallUrl = "broker_install_url"; + public const string BrokerNonce = "broker_nonce"; } } diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/AuthenticationContinuationHelper.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/AuthenticationContinuationHelper.cs index 45aa0cd08..8995d47ea 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/AuthenticationContinuationHelper.cs +++ b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/AuthenticationContinuationHelper.cs @@ -26,6 +26,7 @@ //------------------------------------------------------------------------------ using System; +using System.Diagnostics; using Foundation; using Microsoft.IdentityModel.Clients.ActiveDirectory.Internal.Platform; @@ -44,7 +45,24 @@ public static class AuthenticationContinuationHelper /// True if the response is from broker, False otherwise. public static bool IsBrokerResponse(string sourceApplication) { - return sourceApplication != null && sourceApplication.Equals("com.microsoft.azureauthenticator", StringComparison.OrdinalIgnoreCase); + Debug.WriteLine("IsBrokerResponse called with sourceApplication {0}", sourceApplication); + + if (sourceApplication != null && sourceApplication.Equals("com.microsoft.azureauthenticator", StringComparison.OrdinalIgnoreCase)) + { + return true; + } + if (string.IsNullOrEmpty(sourceApplication)) + { + // For iOS 13+, SourceApplication will not be returned + // Customers will need to install iOS broker >= 6.3.19 + // ADAL.NET will generate a nonce (guid), which broker will + // return in the response. ADAL.NET will validate a match in iOSBroker.cs + // So if SourceApplication is null, just return, ADAL.NET will throw a + // specific error message if the nonce does not match. + return true; + } + + return false; } /// diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSBroker.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSBroker.cs index 3003308b4..57155679a 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSBroker.cs +++ b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSBroker.cs @@ -49,6 +49,9 @@ internal class iOSBroker : IBroker private readonly ICoreLogger _logger; + private string _brokerRequestNonce; + private bool _brokerV3Installed = false; + public iOSBroker(ICoreLogger logger) { _logger = logger; @@ -66,21 +69,42 @@ public bool CanInvokeBroker return false; } - var res = false; + bool canStartBroker = false; if (pp.UseBroker) { pp.CallerViewController.InvokeOnMainThread(() => { - res = UIApplication.SharedApplication.CanOpenUrl(new NSUrl("msauth://")); - _logger.Info("iOS Broker can be invoked. "); + if (IsBrokerInstalled("msauthv3://")) + { + _logger.Info("iOS Broker (msauthv3://) can be invoked. "); + _brokerV3Installed = true; + canStartBroker = true; + } }); + + if (!canStartBroker) + { + pp.CallerViewController.InvokeOnMainThread(() => + { + if (IsBrokerInstalled("msauth://")) + { + _logger.Info("iOS Broker (msauth://) can be invoked. "); + canStartBroker = true; + } + }); + } } - return res; + return canStartBroker; } } + private bool IsBrokerInstalled(string brokerUriScheme) + { + return UIApplication.SharedApplication.CanOpenUrl(new NSUrl(brokerUriScheme)); + } + public async Task AcquireTokenUsingBrokerAsync(IDictionary brokerPayload) { if (brokerPayload.ContainsKey(BrokerParameter.SilentBrokerFlow)) @@ -105,6 +129,13 @@ public async Task AcquireTokenUsingBrokerAsync(IDictionary r string responseActualHash = PlatformProxyFactory.GetPlatformProxy().CryptographyManager.CreateSha256Hash(decryptedResponse); byte[] rawHash = Convert.FromBase64String(responseActualHash); string hash = BitConverter.ToString(rawHash); + if (expectedHash.Equals(hash.Replace("-", ""), StringComparison.OrdinalIgnoreCase)) { responseDictionary = EncodingHelper.ParseKeyValueList(decryptedResponse, '&', false, null); @@ -187,6 +219,15 @@ private AdalResultWrapper ResultFromBrokerResponse(IDictionary r }; _logger.InfoPii("Broker response hash mismatch: " + response.Error, "Broker response hash mismatch. "); } + + if (!ValidateBrokerResponseNonceWithRequestNonce(responseDictionary)) + { + response = new TokenResponse + { + Error = AdalError.BrokerReponseHashMismatch, + ErrorDescription = AdalErrorMessage.BrokerReponseHashMismatch + }; + } } var dateTimeOffset = new DateTimeOffset(new DateTime(1970, 1, 1, 0, 0, 0, 0, DateTimeKind.Utc)); @@ -194,6 +235,19 @@ private AdalResultWrapper ResultFromBrokerResponse(IDictionary r return response.GetResult(dateTimeOffset, dateTimeOffset); } + private bool ValidateBrokerResponseNonceWithRequestNonce(IDictionary brokerResponseDictionary) + { + if (!string.IsNullOrEmpty(_brokerRequestNonce)) + { + string brokerResponseNonce = brokerResponseDictionary.ContainsKey(BrokerParameter.BrokerNonce) + ? brokerResponseDictionary[BrokerParameter.BrokerNonce] + : null; + + return string.Equals(brokerResponseNonce, _brokerRequestNonce); + } + return false; + } + public static void SetBrokerResponse(NSUrl responseUrl) { brokerResponse = responseUrl;