Bug 53082 - Behavior differs between Android and iOS when removing an entry from Key Chain
Summary: Behavior differs between Android and iOS when removing an entry from Key Chain
Status: IN_PROGRESS
Alias: None
Product: Components
Classification: Xamarin
Component: Xamarin Components ()
Version: N/A
Hardware: PC Windows
: --- normal
Target Milestone: ---
Assignee: Miljenko Cvjetko Mel
URL:
Depends on:
Blocks:
 
Reported: 2017-03-06 18:08 UTC by John Hardman
Modified: 2017-03-08 11:52 UTC (History)
7 users (show)

Tags: Mono SecKeyChain iOS Android Xamarin.Auth Remove
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report for Bug 53082 on Developer Community or GitHub if you have new information to add and do not yet see a matching new report.

If the latest results still closely match this report, you can use the original description:

  • Export the original title and description: Developer Community HTML or GitHub Markdown
  • Copy the title and description into the new report. Adjust them to be up-to-date if needed.
  • Add your new information.

In special cases on GitHub you might also want the comments: GitHub Markdown with public comments

Related Links:
Status:
IN_PROGRESS

Description John Hardman 2017-03-06 18:08:47 UTC
An issue that I am seeing in Xamarin.Auth makes me think there is an inconsistency in Mono between Android and iOS.
Using sample code for Xamarin.Auth, I delete an entry from Xamarin.Auth.KeyChainAccountStore when the Xamarin.Auth Completed event is fired. The sample code does not first check whether the entry is in the store before trying to remove it. On Android, this did not cause a problem, but on iOS a generic System.Exception is thrown, containing a Message about ItemNotFound.

Looking at the source of Xamarin.Auth, it appears that the different behavior will be as a consequence of different behavior in Mono, in the following method:

public static SecStatusCode Remove(SecRecord record);

I haven't seen the source, but I would assume that on Android it returns SecStatusCode.Success even when an item is not found, whereas on iOS it returns a value other than Success (probably SecStatusCode.ItemNotFound). The behavior should be consistent between the two platforms.

Ideally, two changes should be made:
(1) the inconsistency in Mono would be fixed, 
and 
(2) Xamarin.Auth would be updated to throw an exception with a useful type, rather than System.Exception.
Comment 1 John Hardman 2017-03-06 18:10:09 UTC
I should have mentioned that the Remove method is in SecKeyChain
Comment 2 Sebastien Pouliot 2017-03-07 15:17:03 UTC
Xamarin.Auth does not ship with XI.

SecRecord is an iOS/Mac API so I'm not sure what Android uses to emulate it. However XI/XM bindings are returning the error codes from the operating system (without any translation). Changing this would be a breaking change (and an hard one to track) for existing applications.
Comment 3 Miljenko Cvjetko Mel 2017-03-08 11:52:20 UTC
Jon

As Sebastien mentioned Xamarin.Auth is not shipped with Xamarin.iOS. It is completely separate utility/library. Xamarin.Auth is a abstraction over OAuth protocols and secure storage, similar to Xamarin.Forms over UI frameworks, but without all the goodies (Attributes for CustomRenderers, DependencyService etc).

So, here is iOS implementation:

https://github.com/xamarin/Xamarin.Auth/blob/portable-bait-and-switch/source/Xamarin.Auth.XamarinIOS/PlatformSpecific/KeyChainAccountStore.Aync.cs

As you can see there are several workarounds for problems, for example reading/searching SecRecord:

https://github.com/xamarin/Xamarin.Auth/blob/portable-bait-and-switch/source/Xamarin.Auth.XamarinIOS/PlatformSpecific/KeyChainAccountStore.Aync.cs#L43-L65

and using those:

https://github.com/xamarin/Xamarin.Auth/blob/portable-bait-and-switch/source/Xamarin.Auth.XamarinIOS/PlatformSpecific/KeyChainAccountStore.Aync.cs#L76-L78

So, I would suggest you not to directly use SecKeyChain.Remove(), but to try with Xamarin.Auth API Delete and DeleteAsync:

https://github.com/xamarin/Xamarin.Auth/blob/portable-bait-and-switch/source/Xamarin.Auth.XamarinIOS/PlatformSpecific/KeyChainAccountStore.Aync.cs#L324

Regarding Android: Android uses KeyStore not SecKeyChain (iOS)

https://github.com/xamarin/Xamarin.Auth/blob/portable-bait-and-switch/source/Xamarin.Auth.XamarinAndroid/PlatformSpecific/AndroidAccountStore.cs#L36

And API is here (especially Delete*):

https://github.com/xamarin/Xamarin.Auth/blob/portable-bait-and-switch/source/Xamarin.Auth.XamarinAndroid/PlatformSpecific/AndroidAccountStore.Async.cs#L76

I have added Xamarin.Auth.AccountStoreException in those API methods and it will be
published most likely today, so No.2 (Xamarin.Auth would be updated to throw an exception with a useful type, rather than System.Exception) is fixed.

Minimal sample would help me to investigate faster and possibly add fix for SeckKeyChain.Remove().

Please, when nuget 1.3.2.6 is pusblished let me know about the outcome here or on github-issues (reference this bug here).