Bug 37370 - Changes to RijndaelManaged/PasswordDeriveBytes encryption classes between Mono 4.0.0 & Mono 4.2.0
Summary: Changes to RijndaelManaged/PasswordDeriveBytes encryption classes between Mon...
Status: RESOLVED ANSWERED
Alias: None
Product: Class Libraries
Classification: Mono
Component: System.Security ()
Version: 4.2.0 (C6)
Hardware: Macintosh Mac OS
: Highest normal
Target Milestone: Untriaged
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2016-01-04 06:36 UTC by Shane Raiteri
Modified: 2016-01-08 16:26 UTC (History)
3 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
SampleAppSolution (289.41 KB, application/zip)
2016-01-04 06:36 UTC, Shane Raiteri
Details
Minimal console app reproduction (2.38 KB, application/zip)
2016-01-06 17:15 UTC, Aleksey Kliger
Details
Alg.Key (Mono 4.0.5) (78.77 KB, image/png)
2016-01-07 15:21 UTC, Shane Raiteri
Details
Alg.IV (Mono 4.0.5) (44.09 KB, image/png)
2016-01-07 15:22 UTC, Shane Raiteri
Details
Alg.IV (Mono 4.2.1) (44.77 KB, image/png)
2016-01-07 15:22 UTC, Shane Raiteri
Details
Alg.Key (Mono 4.2.1) (78.62 KB, image/png)
2016-01-07 15:23 UTC, Shane Raiteri
Details
Fix for the bug (workaround) (19.36 KB, application/zip)
2016-01-08 00:11 UTC, Shane Raiteri
Details


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 on GitHub or Developer Community with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED ANSWERED

Description Shane Raiteri 2016-01-04 06:36:19 UTC
Created attachment 14428 [details]
SampleAppSolution

Hi Mono/Xamarin team,

We've recently released a version of our app targeting Mono 4.2.1, and are having issues with the method of encryption (which previously worked for Mono 4.0.0 and older).

In the release notes, changes to System.Security.Cryptography are mentioned, but not to the extent a non Xamarin engineer would be able to diagnose:

--------------------------------------------------------------

mscorlib assembly
Around 500 .NET types were replaced with the Microsoft implementation, with small changes made to make Microsoft’s code portable.

Some themes in this release include:

Replacing some of the key types in the System namespace
Continued our work to replace large chunks of the globalization stack in Mono with Microsoft’s implementation
Portable parts of System.IO
Large parts of System.Reflection, those that do not depend on Mono runtime internals.
Large parts of System.Resources
The portable parts of System.Runtime.InteropServices
The portable parts of System.Security.Cryptography
Most of System.Text
Most of System.Runtime.Serialization
The threadpool
Most of SafeHandles

--------------------------------------------------------------

I've attached a sample application that shows both an iOS & Android app failing to decrypt a previously encrypted value (from NSUserDefaults & SharedPreferences respectively) after upgrading the component versions of Xamarin.iOS and Xamarin.Android. 

It's not until I upgrade the frameworks to at least the following versions, that the decryption algorithm stops working:

Xamarin.iOS -> 9.2 (and above)
Xamarin.Android -> 6.0 (and above)

Both of these say they are now built against Mono 4.2 in the release notes - if we build an older version of these components with Mono 4.2 installed and run the app, there are no decryption issues. Only when we upgrade to the component versions that say they utilise Mono 4.2 correctly do we see problems.

STEPS TO REPRODUCE:

1. Build and run the attached app with a pre Mono 4.2 build configuration (as mentioned before, having Mono 4.2 instead of Mono 4.0 is ok - just as long as the Xamarin.iOS & Xamarin.Android versions are no higher than the ones mentioned):

--------------------------------------------------------------

=== Xamarin Studio ===

Version 5.9.8 (build 0)
Installation UUID: 4ace342a-f743-41a6-861e-fed9e2ffc216
Runtime:
	Mono 4.2.1 (explicit/6dd2d0d)
	GTK+ 2.24.23 (Raleigh theme)

	Package version: 402010102

=== Apple Developer Tools ===

Xcode 7.2 (9548)
Build 7C68

=== Xamarin.Android ===

Version: 5.1.9.0 (Enterprise Edition)
Android SDK: /Users/internetbanking/Library/Developer/Xamarin/android-sdk-macosx
	Supported Android versions:
		2.3    (API level 10)
		4.0.3  (API level 15)
		4.1    (API level 16)
		4.2    (API level 17)
		4.3    (API level 18)
		4.4    (API level 19)
		4.4.87 (API level 20)
		5.0    (API level 21)
		5.1    (API level 22)
		6.0    (API level 23)
Java SDK: /usr
java version "1.8.0_45"
Java(TM) SE Runtime Environment (build 1.8.0_45-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.45-b02, mixed mode)

=== Xamarin Android Player ===

Version: Unknown version
Location: /Applications/Xamarin Android Player.app

=== Xamarin.iOS ===

Version: 9.1.0.31 (Enterprise Edition)
Hash: bae2cdb
Branch: master
Build date: 2015-11-09 17:44:57-0500

=== Xamarin.Mac ===

Version: 2.4.0.109 (Starter Edition)

=== Build Information ===

Release ID: 509080000
Git revision: cc5f6e5658589ca7f46210c57fad947e75f30abd
Build date: 2015-10-21 19:27:41-04
Xamarin addins: d77f191bd7d3451adf837b85b38f2b7c60004400

=== Operating System ===

Mac OS X 10.11.2
Darwin Digital-Bank-Macbook-Pro-Hotel.local 15.2.0 Darwin Kernel Version 15.2.0
    Fri Nov 13 19:56:56 PST 2015
    root:xnu-3248.20.55~2/RELEASE_X86_64 x86_64

--------------------------------------------------------------

2. Build & run the apps for both platforms, and click the 'Encrypt and store value' button on the main screen (that will use an arbitrary value to be encrypted).
3. For good measure, launch the app again & see that the value can still be decrypted.

4. Upgrade the IDE to a post Mono 4.2 build configuration:

--------------------------------------------------------------

=== Xamarin Studio ===

Version 5.10.1 (build 6)
Installation UUID: 4ace342a-f743-41a6-861e-fed9e2ffc216
Runtime:
	Mono 4.2.1 (explicit/6dd2d0d)
	GTK+ 2.24.23 (Raleigh theme)

	Package version: 402010102

=== Xamarin.Profiler ===

Not Installed

=== Apple Developer Tools ===

Xcode 7.2 (9548)
Build 7C68

=== Xamarin.Mac ===

Version: 2.4.0.109 (Starter Edition)

=== Xamarin.iOS ===

Version: 9.4.0.0 (Enterprise Edition)
Hash: 7322991
Branch: master
Build date: 2015-12-08 16:20:29-0500

=== Xamarin.Android ===

Version: 6.0.0.34 (Enterprise Edition)
Android SDK: /Users/internetbanking/Library/Developer/Xamarin/android-sdk-macosx
	Supported Android versions:
		2.3    (API level 10)
		4.0.3  (API level 15)
		4.1    (API level 16)
		4.2    (API level 17)
		4.3    (API level 18)
		4.4    (API level 19)
		4.4.87 (API level 20)
		5.0    (API level 21)
		5.1    (API level 22)
		6.0    (API level 23)

SDK Tools Version: 25 rc1
SDK Platform Tools Version: 23.1
SDK Build Tools Version: 23.0.2

Java SDK: /usr
java version "1.8.0_45"
Java(TM) SE Runtime Environment (build 1.8.0_45-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.45-b02, mixed mode)

=== Xamarin Android Player ===

Version: 0.6.2
Location: /Applications/Xamarin Android Player.app

=== Build Information ===

Release ID: 510010006
Git revision: 0b60eecdb531933734519c13257d16a780274aab
Build date: 2015-12-04 20:28:20-05
Xamarin addins: 9876fd7c9837977178411ec7375b4352c0a0d6af
Build lane: monodevelop-lion-cycle6-baseline

=== Operating System ===

Mac OS X 10.11.2
Darwin Digital-Bank-Macbook-Pro-Hotel.local 15.2.0 Darwin Kernel Version 15.2.0
    Fri Nov 13 19:56:56 PST 2015
    root:xnu-3248.20.55~2/RELEASE_X86_64 x86_64

--------------------------------------------------------------

5. Clean the entire solution.
6. Build & run the app again - value will still exist in storage, but we can no longer decrypt it using the information we used before (invalid PKCS padding). That will be visible in the 3rd field shown on the screen.
6a. Issue comes from within the System.Security.Cryptography classes utilised in CryptoExtensions.cs.

NEXT STEPS:

As we have already made changes to our iOS application, we have broken decryption of some important values we store for the latest version released to the store. As we are looking to minimise the impact of this bug in the unreleased Android app (and going forward), we'd like to ask to following questions:

1. Is there a legitimate fix for this (or is it just a change in implementation due upgrading the Mono Framework)?
2. If there is a fix, what would be the best strategy going forward for forwards/backwards support?

You'll also see the encrypt/decrypt process still works in the UnitTest project - it's only an issue when building against the versions of Xamarin.iOS & Xamarin.Android mentioned above.

Thanks for your assistance in advance,
Shane Raiteri.
Comment 1 Shane Raiteri 2016-01-04 23:16:01 UTC
Issue is reproducible in both real devices, as well as simulators/emulators.
Comment 2 Shane Raiteri 2016-01-06 02:45:19 UTC
Any further updates on where this is at?
Comment 3 Aleksey Kliger 2016-01-06 17:15:29 UTC
Created attachment 14469 [details]
Minimal console app reproduction
Comment 4 Aleksey Kliger 2016-01-06 17:15:55 UTC
Attaching smaller console-only reproduction.

Output with mono 4.2.1:

$ mono --version

Mono JIT compiler version 4.2.1 (explicit/54ce6bd Tue Oct 13 17:50:58 EDT 2015)
Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com
	TLS:           normal
	SIGSEGV:       altstack
	Notification:  kqueue
	Architecture:  x86
	Disabled:      none
	Misc:          softdebug 
	LLVM:          yes(3.6.0svn-mono-(detached/a173357)
	GC:            sgen

$ mono Main.exe

original: [Hello World!]
encrypted: [kGwgdqesVXRINJseSlyB6A==]
decrypted: [Hello World!]

Output with mono 4.0.5:

$ mono --version

Mono JIT compiler version 4.0.5 (mono-4.0.0-branch-c5sr5/1d8d582 Wed Jan  6 11:58:18 EST 2016)
Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com
	TLS:           normal
	SIGSEGV:       altstack
	Notification:  kqueue
	Architecture:  x86
	Disabled:      none
	Misc:          softdebug 
	LLVM:          yes(3.6.0svn-mono-master/9f79399)
	GC:            sgen

$ mono Main.exe

original: [Hello World!]
encrypted: [8xJHP5TqnJiqC3ucfubMDA==]
decrypted: [Hello World!]
Comment 5 Aleksey Kliger 2016-01-06 20:46:47 UTC
The actual Rijndael implementation seems to be innocent.
Looks like the behavior of PasswordDeriveBytes changed between Mono 4.2 and 4.0.5: the Key (but not the IV) is different between the two versions.
Comment 6 Shane Raiteri 2016-01-07 05:34:26 UTC
Thanks Aleksey,

While we do realize the PasswordDeriveBytes has been marked as deprecated (for some time now), is it possible for us to use the Mono 4.0.5 PasswordDeriveBytes implementation so we can include it in the app to:

1. Decrypt the old keys
2. Re-encrypt the old keys and store again (upgrading to using Rfc2898DeriveBytes instead of PasswordDeriveBytes)

This would mean we don't impact any of our customers going forward, and all previously stored data can be decrypted & re-saved.

Thanks,
Shane.
Comment 7 Shane Raiteri 2016-01-07 15:21:16 UTC
Created attachment 14476 [details]
Alg.Key (Mono 4.0.5)
Comment 8 Shane Raiteri 2016-01-07 15:22:12 UTC
Created attachment 14477 [details]
Alg.IV (Mono 4.0.5)
Comment 9 Shane Raiteri 2016-01-07 15:22:41 UTC
Created attachment 14478 [details]
Alg.IV (Mono 4.2.1)
Comment 10 Shane Raiteri 2016-01-07 15:23:06 UTC
Created attachment 14479 [details]
Alg.Key (Mono 4.2.1)
Comment 11 Shane Raiteri 2016-01-07 16:55:07 UTC
I definitely agree that the bug is a change in implementation for the GetBytes() method of PasswordDeriveBytes. Looking back at older unresolved bugs, is it possible that it's related directly to https://bugzilla.novell.com/show_bug.cgi?id=372893?

I've attached an additional 4 screenshots (Key/IV values), and you'll see in the Mono 4.2.1 screenshots that bytes 4, 5, 6 & 7 in the IV are leaked as bytes 0, 1, 2 & 3 in the Key.

If bytes 0, 1, 2 & 3 in the Key had been generated as per the Mono 4.0.5 key, we'd have the Key being a 100% match between both versions.



I've also tried taking an older implementation from https://github.com/mosa/Mono-Class-Libraries (Mono 2.6) for PasswordDeriveBytes, and the implementation works between 2.6 & 4.0.5, but breaks again on 4.2.1. I've started replacing the inner references to other Mono classes as I encounter them in the GetBytes() method (and subcalls) just so I can see where the breaking functionality is:

SHA1CryptoServiceProvider.cs
Buffer.cs

And I get down to the Internal Method BlockCopyInternal() (no longer used - rewritten as InternalBlockCopy()), but have not continued any further.

Without copying some old class files into our project (assuming that is even possible), I'm not sure what the right course of action going forward should be.

Thanks,
Shane.
Comment 12 Aleksey Kliger 2016-01-07 18:37:15 UTC
Shane,

I agree the right long term plan is to include the legacy PasswordDeriveBytes in your app and migrate to Rfc2898DeriveBytes.

I think you're almost there with extracting the legacy PDB: If you take the entire subclass chain from SHA1CryptoServiceProvider.cs up (ie SHA1.cs, HashAlgorithm.cs) and PasswordDeriveBytes.cs from 4.0.5 and clean them up to build outside corlib (move to some other namespace; have PasswordDeriveBytes construct a SHA1CSP directly instead of through HashAlgorithm.Create(); get rid of Local.GetText; etc) you should get something that will build on 4.2.1 that has the same behavior as 4.0.5 did.

The issue is that HashAlgorithm on 4.2.1 is from MS reference sources, which calls the underlying hash implementation in a different way, so it's not enough to just grab SHA1CSP from 4.0.5, you have to take everything.

I don't think there's any problem with Buffer.cs (let me know if think otherwise after pulling HashAlgorithm out).

Cheers,
Aleksey
Comment 13 Shane Raiteri 2016-01-08 00:11:01 UTC
Thanks for the direction Aleksey,

I've probably pulled out a bit more than I actually need - however the new console app now works regardless of the Mono.framework version installed. It does look that the HashAlgorithm call changes were the culprit & I didn't have to do anything with the Buffer.cs class.

Here's the output I now see when going between different versions of Xamarin.Android & Xamarin.iOS that were previously broken:

Using Mono version: 4.0.5
original: [Hello World!]
encrypted: [8xJHP5TqnJiqC3ucfubMDA==]
decrypted: [Hello World!]

Using Mono version: 4.2.1
original: [Hello World!]
encrypted: [8xJHP5TqnJiqC3ucfubMDA==]
decrypted: [Hello World!]

Looks like encryption & decryption are consistently matching. I just wanted to double check with you that I've not taken anything incorrectly in my new MonoOld.System.Security.Cryptography namespace that would result in a similar issue in the future.

If that's ok, then I'm happy to have this bug marked as closed. Are there any parts of this that should be made public to assist someone else who could run into this issue?

Thanks very much for your assistance on this one,
Shane.
Comment 14 Shane Raiteri 2016-01-08 00:11:36 UTC
Created attachment 14492 [details]
Fix for the bug (workaround)
Comment 15 Aleksey Kliger 2016-01-08 16:26:58 UTC
Shane,

Thanks for providing the isolated classes.  I'm going to make this bug public in case others run into a similar issue.

Cheers,
Aleksey