Skip to content
This repository has been archived by the owner on Apr 1, 2024. It is now read-only.

Import of RSA key crashes on iOS9 #29

Closed
dsizomin opened this issue Aug 5, 2015 · 36 comments
Closed

Import of RSA key crashes on iOS9 #29

dsizomin opened this issue Aug 5, 2015 · 36 comments
Assignees
Milestone

Comments

@dsizomin
Copy link

dsizomin commented Aug 5, 2015

Hi,

I'm using PCLCrypto to verify RSA signature on Android/iOS.
After tests in iOS9 Beta it appeared that importing public key is not working on iOS9. The exact result is InvalidOperationException "SecItemAdd returns 0".

The strange thing is that return result (OSStatus) is 0, which means "no error". However, the reference handle which has to be set by SecItemAdd is 0. Please refer to the code below:

        IAsymmetricKeyAlgorithmProvider algorithmProvider = WinRTCrypto.AsymmetricKeyAlgorithmProvider.OpenAlgorithm(AsymmetricAlgorithm.RsaSignPkcs1Sha256);

        ICryptographicKey generatedKeyPair = algorithmProvider.CreateKeyPair(2048);

        byte[] generatedPublicKey = generatedKeyPair.ExportPublicKey(CryptographicPublicKeyBlobType.Pkcs1RsaPublicKey);

        ICryptographicKey importedKey = algorithmProvider.ImportPublicKey(generatedPublicKey, CryptographicPublicKeyBlobType.Pkcs1RsaPublicKey); //InvalidOperationException

Any thoughts or ideas how to overcome this issue?

Thanks in advance!
Denys

@AArnott
Copy link
Owner

AArnott commented Aug 5, 2015

Interesting. Thanks for the report. I wonder if SecItemAdd fails because that particular is already in the key store (since you just generated it). That would explain why the error is 0 (since it didn't really fail, it just didn't need to happen). If that's the case, I expect we can get PclCrypto to work with that. Can you confirm, perhaps by saving a public key and then restoring it from a file and importing it later (another session of your app, perhaps)?

I don't have an iOS 9 beta device or else I might try this myself.

@AArnott AArnott added the bug label Aug 5, 2015
@dsizomin
Copy link
Author

dsizomin commented Aug 5, 2015

Generating key was added in my example just for the sake of demonstration. In real scenario, we import key in base64 format, and it breaks as well. Here is example:

        IAsymmetricKeyAlgorithmProvider algorithmProvider = WinRTCrypto.AsymmetricKeyAlgorithmProvider.OpenAlgorithm(AsymmetricAlgorithm.RsaSignPkcs1Sha256);

        byte[] publicKey = Convert.FromBase64String(@"MIIBCgKCAQEAwFEZOEUWzpPxy0WEafdL2yDDcTympXmUTnbYGfUDH3LMtMr3qNF5B6VWWDuOEd29ApkB6EsBbiUn0GRAeQU250JV3wwz9JPTTw0tBcs5fohKlAFGNQKH5UtSzUw549Fv5wZfBwTGlXZheadSmZ6W7uOD4LtYdtsvZyDgGae51g0BKgbzi08g+gbKjBo+9WpS80S6pMc/YQvVAMVB7g9+MlK3PKIXjyQdfgmMe+5Jn1cGX3DiH6T5P2+iQQY4xGad69c/BaMRVIsP2TESOeINGdPxBiMxmmnMgHz5OjQ5sg+Il+zoJgjV2QECzbtIIZnsoTa5XE1jyP5xqSqq7TrnsQIDAQAB");

        ICryptographicKey importedKey = algorithmProvider.ImportPublicKey(publicKey, CryptographicPublicKeyBlobType.Pkcs1RsaPublicKey);

Besides that, AFAIK, adding the item to keychain which already exists should result in
status errSecDuplicateItem -25299 (according to Apple documentation).

Today I reproduced the issue on iOS9 emulator. I think I'll be able to test on real device tomorrow and confirm if it the case on it.

@AArnott
Copy link
Owner

AArnott commented Aug 5, 2015

Hmmm... Well Apple's official position is that importing keys at all is unsupported. The way PCLCrypto does it anyway is a well-known backdoor approach, but it may be that Apple is finally choosing to close it. Or it's just an iOS 9 bug. I suggest you also file an issue with them so that at least they have a chance to fix it on their end as well.

@dsizomin
Copy link
Author

dsizomin commented Aug 6, 2015

I tried to do pretty same stuff but with objective C, and it seems like it's really a change in iOS. The following code works as expected on iOS 8.4, but on iOS 9 CFTypeRef result remains 0x0:

NSString *base64data = @"MIIBCQKCAQDAURk4RRbOk/HLRYRp90vbIMNxPKaleZROdtgZ9QMfcsy0yveo0XkHpVZYO44R3b0CmQHoSwFuJSfQZEB5BTbnQlXfDDP0k9NPDS0Fyzl+iEqUAUY1AoflS1LNTDnj0W/nBl8HBMaVdmF5p1KZnpbu44Pgu1h22y9nIOAZp7nWDQEqBvOLTyD6BsqMGj71alLzRLqkxz9hC9UAxUHuD34yUrc8ohePJB1+CYx77kmfVwZfcOIfpPk/b6JBBjjEZp3r1z8FoxFUiw/ZMRI54g0Z0/EGIzGaacyAfPk6NDmyD4iX7OgmCNXZAQLNu0ghmeyhNrlcTWPI/nGpKqrtOuexAgMBAAE=";    
NSData *decodedData = [[NSData alloc] initWithBase64EncodedString:base64data options:0];    

CFUUIDRef udid = CFUUIDCreate(NULL);
NSString *udidString = (__bridge_transfer NSString *) CFUUIDCreateString(NULL, udid);
NSData* atag = [udidString dataUsingEncoding:NSUTF8StringEncoding];    

NSDictionary *query =
@{
  (__bridge id)kSecClass : (__bridge id)kSecClassKey,
  (__bridge id)kSecAttrApplicationTag : atag,
  (__bridge id)kSecAttrKeyType : (__bridge id)kSecAttrKeyTypeRSA,
  (__bridge id)kSecAttrAccessible : (__bridge id)kSecAttrAccessibleWhenUnlocked,
  (__bridge id)kSecValueData : decodedData,
  (__bridge id)kSecAttrKeyClass : (__bridge id)kSecAttrKeyClassPublic,
  (__bridge id)kSecReturnRef : [NSNumber numberWithBool:YES]
};

CFTypeRef result = nil;
OSStatus status = SecItemAdd((__bridge CFDictionaryRef)query, &result);

I think I will file that to Apple guys to see if it's a bug or expected behavior on iOS 9.

@AArnott
Copy link
Owner

AArnott commented Aug 6, 2015

Thanks. Let us know how it goes. And if iOS 9 intends to keep their current behavior, I hope they can suggest a workaround for us.

@FunThom
Copy link

FunThom commented Sep 18, 2015

Hi,

after the official iOS 9 release, I still got the same error as dsizomin (InvalidOperationException: "SecItemAdd returns 0") when importing a public key. Is there any progress here?

@dsizomin:
What is Apple's statement in this case?

Best regards,
Thomas

@dsizomin
Copy link
Author

Hi,

Sorry for making you guys wait.

A month ago I submitted a bug report to Apple (#22228229 iOS9 : SecItemAdd does not initialize 'CFTypeRef result' parameter).
Recently Apple closed this issue as duplicate of bug #21810530.

However, because of security reasons and blah-blah Apple does not allow you to explore bugs opened by other people, so I have no chance to find out the state of original bug.

I'm still trying to get the answer from Apple, but as iOS9 is released, and the issue is still there, most likely, we got to find a workaround for that.

Best regards,
Denys

@AArnott
Copy link
Owner

AArnott commented Sep 18, 2015

Dang.
I don't have time (nor an iOS 9 device) to research this. But I would appreciate a PR that fixes it.

@FunThom
Copy link

FunThom commented Oct 2, 2015

Hi again,

after setting up the necessary development environment, I tried to start a little research about the error.
My first approach was to debug into the PCLCrypto library to understand what happened and when.

Then I asked google about the occurring error:
https://forums.developer.apple.com/thread/15129
https://forums.developer.apple.com/message/38361#38361
StCredZero/SCZ-BasicEncodingRules-iOS#6 (comment)

Using this information I changed the class PCLCrypto.Formatters.KeyFormatter (in PCLCrypto.Shared.Formatters directory) to this:

    protected internal static RSAParameters NegotiateSizes(RSAParameters parameters)
    {
        if (HasPrivateKey(parameters))
        {
            if (CapiKeyFormatter.IsCapiCompatible(parameters))
            {
                // Don't change a thing. Everything is perfect.
                return parameters;
            }

            // First change:
            // iOS 9 needs an unchanged Modulus
            // parameters.Modulus = TrimLeadingZero(parameters.Modulus);

            parameters.D = TrimLeadingZero(parameters.D);
            int keyLength = Math.Max(parameters.Modulus.Length, parameters.D.Length);
            parameters.Modulus = TrimOrPadZeroToLength(parameters.Modulus, keyLength);
            parameters.D = TrimOrPadZeroToLength(parameters.D, keyLength);

            int halfKeyLength = (keyLength + 1) / 2;
            parameters.P = TrimOrPadZeroToLength(parameters.P, halfKeyLength);
            parameters.Q = TrimOrPadZeroToLength(parameters.Q, halfKeyLength);
            parameters.DP = TrimOrPadZeroToLength(parameters.DP, halfKeyLength);
            parameters.DQ = TrimOrPadZeroToLength(parameters.DQ, halfKeyLength);
            parameters.InverseQ = TrimOrPadZeroToLength(parameters.InverseQ, halfKeyLength);
        }

        // Second change:
        // iOS 9 needs an unchanged Modulus            
        // else
        // {   
        //   parameters.Modulus = TrimLeadingZero(parameters.Modulus);                
        // }

        parameters.Exponent = TrimLeadingZero(parameters.Exponent);
        return parameters;
    }

Then I compiled PCLCrypto portable and iOS and started testing. Because of Apple's "great"
backward compatibility, I cannot test this fix on iOS 8.3 (Downgrade not possible anymore), but on

  • iOS 8.4.1
  • iOS 9.0
  • iOS 9.0.1
  • iOS 9.0.2

and it works!

@AArnott:
What is your opinion about this?

Unfortunately, I don't have time to test this fix on other platforms, like Android etc. :-(

@AArnott
Copy link
Owner

AArnott commented Oct 3, 2015

I'll review and get back to you, @FunThom. Thanks for sharing. I have an older iPhone that I may be able to test this on.

@fmcosta66
Copy link

Hi there @AArnott ! Are you trying to solve this issue? We've got an application to distribute and this problem ruined it. Have you tested the solution of @FunThom ? I've tested with no positive result.

@AArnott
Copy link
Owner

AArnott commented Oct 27, 2015

I haven't been able to get Xamarin to work for me lately, at all. :( So no progress to report from my side.

@asheffield
Copy link

Hi AArnott, our xamarin.forms (iOS/android) application is also getting this error. Any work around yet?

@fmcosta66
Copy link

Hi there @AArnott ! I think I've discovered what was going on and was able to solve this issue. I've tried the @FunThom solution but it did not work for me. This was the solution that worked for me:

First i was having problems with the compilation of the PCLCrypto. So i've updated xamarin to the latest Beta version (now a Stable one) and then I was able to solve that "NSDictionary...set_item" compilation error (now Xamarin.iOS is 9.1). Then i've searched your code and found that in the Pkcs1KeyFormatter class there is a property called "prependLeadingZeroOnCertainElements", that was false by default. I've changed it to true and now i can encrypt messages with iOS 9/9.1.

Now my main problem is that I cannot test this in iOS < 9.1 and was unable to detect what version of the OS is being executed to define that variable as true/false depending on it being 9 or 8 and below. Can you help me with this issue?

Thanks!

@AArnott
Copy link
Owner

AArnott commented Oct 29, 2015

Thanks for sharing, @fmcosta66. I haven't had luck with Xamarin compiling Android or iOS lately. But I'll be working at it this weekend and will try out what you're suggesting.

@fmcosta66
Copy link

One more thing...i was able to test in a iOS 8.4 emulator and it worked, even without any specific code for each of the iOS versions. So maybe the answer is really this one. Hope you can confirm that...

@asheffield
Copy link

Can you use #if IOS and #endif, then check the version using dependencyservice with - UIDevice.CurrentDevice.CheckSystemVersion(9, 0)
or UIDevice.CurrentDevice.SystemVersion.

@xillkey
Copy link

xillkey commented Oct 31, 2015

I think you can use javascript solve this problem , see http://blog.csdn.net/suusatoshigi/article/details/49535391

@AArnott
Copy link
Owner

AArnott commented Nov 7, 2015

@fmcosta66 We can't change the default parameter value because that would cause KeyFormatter's two fields Pkcs1PrependZeros and Pkcs1 to mean the same thing. It would make more sense to redirect users of Pkcs1 to Pkcs1PrependZeros where it makes sense. Do you happen to know which callers need the behavior?
We also have to consider that this code is shared across most of the platforms, so unless the change works on all of them, we'll have to put #if around the changes.

@AArnott AArnott closed this as completed in 98459d2 Nov 8, 2015
@AArnott
Copy link
Owner

AArnott commented Nov 8, 2015

I am actively working on this. It's not all fixed yet.

@AArnott AArnott reopened this Nov 8, 2015
@AArnott
Copy link
Owner

AArnott commented Nov 8, 2015

Fixed in v1.0 branch with 0ccfa0d

@AArnott AArnott closed this as completed Nov 8, 2015
@AArnott AArnott added this to the 1.0.86 milestone Nov 8, 2015
@AArnott AArnott self-assigned this Nov 8, 2015
@AArnott
Copy link
Owner

AArnott commented Nov 8, 2015

Alright, v1.0.86 is on nuget.org with this fixed as far as I've been able to test.
Please confirm and comment whether you have good or bad news.

@asheffield
Copy link

I installed v1.0.86 and it is working for me on iOS devices with iOS9.1 and iOS7.1.2, and on android devices with kitkat 4.4.2 & 4.4.4. Thank you.

@AArnott
Copy link
Owner

AArnott commented Nov 11, 2015

Yay! Thanks for confirming, @asheffield !

@mrhyh
Copy link

mrhyh commented Nov 11, 2015

I found a solution, in the hope that useful to you。http://blog.csdn.net/ylgwhyh/article/details/49756209

@AArnott
Copy link
Owner

AArnott commented Nov 11, 2015

@mrhyh The solution you refer to indicates the need to add a leading 0 to the modulus. That's already part of the fix I've published. Is it not working for you?

@mrhyh
Copy link

mrhyh commented Nov 12, 2015

Yes! It is working for me.

@AArnott
Copy link
Owner

AArnott commented Nov 12, 2015

great!

@ammarMheir
Copy link

ammarMheir commented Mar 20, 2017

I am recently running into this exact same issue, I downloaded the latest source code and noticed that the changes implemented from this fix have been removed? Is this due to a limitation on iOS itself or should there be another way to properly import RSA Keys?

@AArnott
Copy link
Owner

AArnott commented Mar 21, 2017

This was a while back, but IIRC the early fix was removed in favor of a generally more reliably handling of leading zeros. What version of pclcrypto are you using when you see the problem? (hint: it might not be the version of source code at the tip of master)

@ammarMheir
Copy link

Thanks for your quick response, I'm using the latest stable release from NuGet (2.0.147). We are creating the Keypair within the app and then the app crashes at ImportPublicKey with the following exception.

Unhandled Exception: System.InvalidOperationException: SecItemAdd return 0 at PCLCrypto.RsaAsymmetricKeyAlgorithmProvider.ImportPublicKey (System.Byte[] keyBlob, PCLCrypto.CryptographicPublicKeyBlobType blobType)

The keypair is created by calling:
asym = AsymmetricKeyAlgorithmProvider.OpenAlgorithm(AsymmetricAlgorithm.RsaPkcs1);
key = asym.CreateKeyPair(512);
var publicKey = key.ExportPublicKey();
var publicKeyString = Convert.ToBase64String(publicKey);

and the crash occurs here:
var pubKey = asym.ImportPublicKey(Convert.FromBase64String(publicKeyString),CryptographicPublicKeyBlobType.Pkcs1RsaPublicKey);

I was just reading through the changelog for version 2.0 since this was fixed in version 1.0 and I noticed a few changes to the leading zeros issue, removing Mono.Security and changes for non-CAPI compliant RSA keys. I just wasn't sure if one of those changes is what caused this fix to no longer work. (Unless there is something else we need to be doing)

@AArnott
Copy link
Owner

AArnott commented Mar 21, 2017

since this was fixed in version 1.0

Do you mean that PCLCrypto 1.0 currently works on iOS 9 but 2.0.147 does not?

We are creating the Keypair within the app and then the app crashes at ImportPublicKey

This is interesting. The issues I was recalling was with key pair interop across platforms. So creating and using the key pair within the iOS app should work -- the fact that it's not suggests iOS (once again) changed and broke something, which will have to be re-investigated.

@ammarMheir
Copy link

Sorry no, I tried installing PCLCrypto 1.0 and that doesn't work on iOS 9, was only mentioning that I understand that the fix for this particular issue was implemented in version 1.0. So I thought that something might have changed in version 2.0.147.

Version 2.0.147 installs and works fine, just crashes when attempting to import a public key.

@AArnott
Copy link
Owner

AArnott commented Mar 21, 2017

I can't say how many weeks may go by before I get a chance to spin up my mac mini and investigate this on the latest iOS 9 emulator. But as has happened a couple times before, if someone in the community can send a PR that fixes this (and fixes or adds a unit test to cover it), I'd be happy to review and accept the PR.

@afilipea
Copy link

afilipea commented May 7, 2019

My app crash only in iOS 9 + version when i'm trying Sign bytes with my Private key, i´m using PCL Crypto Version 2.0.147.. Any Solutions??
WinRTCrypto.CryptographicEngine.Sign(cryptoKey, bytes);

@AArnott
Copy link
Owner

AArnott commented May 9, 2019

@afilipea Given your crash is different (signing rather than importing) (and this issue is closed), please file a new issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants