[Xamarin.Android.Build.Tasks] use CRC64 over SHA1 and MD5 #3649
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dellis1972
reviewed
Sep 19, 2019
src/Xamarin.Android.Build.Tasks/Tasks/GetAdditionalResourcesFromAssemblies.cs
Outdated
Show resolved
Hide resolved
|
We are doing to have to invalidate builds for this upgrade, we somehow need to delete any old files still using the MD5 hash (especially generated Java stuff). |
jonathanpeppers
force-pushed
the
java.interop-crc64
branch
from
180a75d
to
6962483
Compare
jonpryor
reviewed
Sep 20, 2019
jonathanpeppers
force-pushed
the
java.interop-crc64
branch
2 times, most recently
from
495584f
to
577e19a
Compare
|
Still working on how to fix the designer: This would be a crc64 hash now. |
jonathanpeppers
force-pushed
the
java.interop-crc64
branch
from
577e19a
to
c728db5
Compare
jonathanpeppers
commented
Sep 20, 2019
jonathanpeppers
force-pushed
the
java.interop-crc64
branch
3 times, most recently
from
f42733c
to
de6ded6
Compare
jonathanpeppers
force-pushed
the
java.interop-crc64
branch
from
de6ded6
to
47bf1ad
Compare
jonathanpeppers
added a commit
to jonathanpeppers/xamarin-android
that referenced
this pull request
Sep 24, 2019
Context: https://dev.azure.com/DevDiv/DevDiv/_build/results?buildId=3075928&view=ms.vss-test-web.test-result-details I was originally using this in xamarin#3649, but it seemed better to do this in a separate PR. If a `DeviceTest` fails, it can take a screenshot, add an attachment to the failed test, and archive it as an artifact. This will help troubleshoot if things like a button press fails, and if we add more tests like this down the road.
Context: xamarin#1580 Bump to xamarin/java.interop@cd91e48 Changes: xamarin/java.interop@be6048e...cd91e48 Use of MD5 results in an inability to use Xamarin.Android on FIPS-enabled machines, because when the **System cryptography: [Use FIPS compliant algorithms][0] for encryption, hashing, and signing** setting is enabled on Windows: !['Use Fips compliant algorithms' setting][1] then the `System.Security.Cryptography.MD5.Create()` method will throw `InvalidOperationException`. In order to support FIPS-enabled machines, use a 64-bit [Cyclic redundancy check][2] algorithm (CRC64) to instead generate the Java package names, e.g. `crc64aae7d955a89b38db.Name` and `crc64217c8705f00a5054.Name`. A simple implementation using a lookup table will suffice for our purposes, ported to C# from: https://github.com/gityf/crc/blob/8045f50ba6e4193d4ee5d2539025fef26e613c9f/crc/crc64.c The use of CRC64 avoids the use of MD5, thus permitting execution on FIPS-enabled Windows machines, and also results in *shorter* directory names -- as the Java package name is a directory name -- which helps reduce Windows `MAX_PATH` issues. ~~ Changes so far ~~ All usage of `MD5.Create()` or `new SHA1Managed()` have been either: * Switched to use `new Crc64()` * Used `Files.HashString` if more appropriate * `<GetAdditionalResourcesFromAssemblies/>` now emits `XA0120` if `new SHA1Managed()` fails. We will deprecate this code path. Calls to `jarsigner` now use: -sigalg SHA256withRSA -digestalg SHA-256 The previous default can be restored via `$(AndroidApkSigningAlgorithm)` and `$(AndroidApkDigestAlgorithm)` MSBuild properties. `$(AndroidPackageNamingPolicy)` now defaults to `LowercaseCrc64`. The previous default can be restored by setting `AndroidPackageNamingPolicy=LowercaseMD5`. `JNIEnv.Initialize()` sets `JavaNativeTypeManager.PackageNamingPolicy` via the `__XA_PACKAGE_NAMING_POLICY__` environment variable. `@(AndroidEnvironment)` values generated by Xamarin.Android at build time are now placed in `$(IntermediateOutputPath)__environment__.txt`. A new `_CleanIntermediateIfPackageNamingPolicyChanges` target will run `_CleanMonoAndroidIntermediateDir` if `$(AndroidPackageNamingPolicy)` changes. This is necessary because many Java source files will be invalid. [0]: https://docs.microsoft.com/en-us/windows/security/threat-protection/security-policy-settings/system-cryptography-use-fips-compliant-algorithms-for-encryption-hashing-and-signing [1]: https://user-images.githubusercontent.com/24926263/38981221-7e0bf7c4-43dc-11e8-8f16-9b6d284a55fb.PNG [2]: https://en.wikipedia.org/wiki/Cyclic_redundancy_check
jonathanpeppers
force-pushed
the
java.interop-crc64
branch
from
47bf1ad
to
e53865a
Compare
jonathanpeppers
added a commit
to jonathanpeppers/xamarin-android
that referenced
this pull request
Sep 24, 2019
Context: https://dev.azure.com/DevDiv/DevDiv/_build/results?buildId=3075928&view=ms.vss-test-web.test-result-details I was originally using this in xamarin#3649, but it seemed better to do this in a separate PR. If a `DeviceTest` fails, it can take a screenshot, add an attachment to the failed test, and archive it as an artifact. This will help troubleshoot if things like a button press fails, and if we add more tests like this down the road. I used `/data/local/tmp/screenshot.png` as this path has been used by `Fast Deployment` for a long time and Android hasn't changed it.
|
This is ready to review (I got all the tests to pass), but we still will need to wait on the designer changes before we can merge. There is only one BCL test failure with the GC that seems unrelated. |
grendello
approved these changes
Sep 25, 2019
|
@garuma: Please let us know when we can merge this PR. I assume that would be once https://github.com/xamarin/designer/pull/2394 is merged. |
jonpryor
pushed a commit
that referenced
this pull request
Sep 30, 2019
Context: #1580 Bump to xamarin/java.interop@b98f4232 Changes: xamarin/java.interop@4fd3539...b98f423 Use of MD5 results in an inability to use Xamarin.Android on FIPS- enabled machines, because when the **System cryptography: [Use FIPS compliant algorithms][0] for encryption, hashing, and signing** setting is enabled on Windows: !['Use Fips compliant algorithms' setting][1] then the `System.Security.Cryptography.MD5.Create()` method will throw `InvalidOperationException`. In order to support FIPS-enabled machines, use a 64-bit [Cyclic redundancy check][2] algorithm (CRC64) to instead generate the Java package names, e.g. `crc64aae7d955a89b38db.Name` and `crc64217c8705f00a5054.Name`. A simple implementation using a lookup table will suffice for our purposes, ported to C# from: https://github.com/gityf/crc/blob/8045f50ba6e4193d4ee5d2539025fef26e613c9f/crc/crc64.c The use of CRC64 avoids the use of MD5, thus permitting execution on FIPS-enabled Windows machines, and also results in *shorter* directory names -- as the Java package name is a directory name -- which helps reduce Windows `MAX_PATH` issues. All usage of `MD5.Create()` or `new SHA1Managed()` have been either: * Switched to use `new Crc64()` * Used `Files.HashString()` if more appropriate * `<GetAdditionalResourcesFromAssemblies/>` now emits warning `XA0120` if `new SHA1Managed()` fails. We will deprecate this code path. Calls to `jarsigner` now use: -sigalg SHA256withRSA -digestalg SHA-256 The previous default can be restored via `$(AndroidApkSigningAlgorithm)` and `$(AndroidApkDigestAlgorithm)` MSBuild properties. `$(AndroidPackageNamingPolicy)` now defaults to `LowercaseCrc64`. The previous default can be restored by setting `$(AndroidPackageNamingPolicy)=LowercaseMD5`. `JNIEnv.Initialize()` sets `JavaNativeTypeManager.PackageNamingPolicy` via the `__XA_PACKAGE_NAMING_POLICY__` environment variable. `@(AndroidEnvironment)` values generated by Xamarin.Android at build time are now placed in `$(IntermediateOutputPath)__environment__.txt`. A new `_CleanIntermediateIfPackageNamingPolicyChanges` target will run `_CleanMonoAndroidIntermediateDir` if `$(AndroidPackageNamingPolicy)` changes. This is necessary because many Java source files will be invalid. [0]: https://docs.microsoft.com/en-us/windows/security/threat-protection/security-policy-settings/system-cryptography-use-fips-compliant-algorithms-for-encryption-hashing-and-signing [1]: https://user-images.githubusercontent.com/24926263/38981221-7e0bf7c4-43dc-11e8-8f16-9b6d284a55fb.PNG [2]: https://en.wikipedia.org/wiki/Cyclic_redundancy_check
jonpryor
pushed a commit
that referenced
this pull request
Oct 1, 2019
Context: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3096255&view=ms.vss-test-web.build-test-results-tab&runId=8756400&resultId=100484&paneView=debug Commit 88a1d6c introduced multiple unit test regressions in e.g. `ManagedResourceParserTests.GenerateDesignerFileFromRtxt()`: …\GenerateDesignerFileFromRtxtTrue Some Space\Resource.designer.cs and …\bin\TestRelease\Expected\GenerateDesignerFileWithLibraryReferenceExpected.cs do not match. Expected: True But was: False The cause of the breakage is that commit 88a1d6c removed the use of `BaseTest.ReadAllBytesIgnoringLineEndings()` for file comparisons. On Windows, the generated `Resource.designer.cs` file contains Windows line endings (CRLF), while the in-repro copy has Unix (LF) line endings. This causes the file comparisons to fail. The test failures weren't seen on PR #3649 because the test failures occur on the **MSBuild - Windows** test job added in f91cd2d, which hadn't yet been merged when PR #3649 was being written. In exploring these test failures, a usability problem arose: we knew that the file contents differed, because of the assert message, but we didn't know *how* the file contents differed, nor *could* we obtain the files until 936459c, because they weren't uploaded. Add a `BaseTest.AssertFileContentsMatch()` method, which: 1. Uses `ReadAllBytesIgnoringLineEndings()`, which fixes the regressions introduced in 88a1d6c. 2. Adds the files as Azure test attachments, so that we can easily see their contents from the unit test failure screen.
jonpryor
pushed a commit
that referenced
this pull request
Oct 1, 2019
Context: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3096255&view=ms.vss-test-web.build-test-results-tab&runId=8756400&resultId=100484&paneView=debug Commit 88a1d6c introduced multiple unit test regressions in e.g. `ManagedResourceParserTests.GenerateDesignerFileFromRtxt()`: …\GenerateDesignerFileFromRtxtTrue Some Space\Resource.designer.cs and …\bin\TestRelease\Expected\GenerateDesignerFileWithLibraryReferenceExpected.cs do not match. Expected: True But was: False The cause of the breakage is that commit 88a1d6c removed the use of `BaseTest.ReadAllBytesIgnoringLineEndings()` for file comparisons. On Windows, the generated `Resource.designer.cs` file contains Windows line endings (CRLF), while the in-repro copy has Unix (LF) line endings. This causes the file comparisons to fail. The test failures weren't seen on PR #3649 because the test failures occur on the **MSBuild - Windows** test job added in f91cd2d, which hadn't yet been merged when PR #3649 was being written. In exploring these test failures, a usability problem arose: we knew that the file contents differed, because of the assert message, but we didn't know *how* the file contents differed, nor *could* we obtain the files until 936459c, because they weren't uploaded. Add a `BaseTest.AssertFileContentsMatch()` method, which: 1. Uses `ReadAllBytesIgnoringLineEndings()`, which fixes the regressions introduced in 88a1d6c. 2. Adds the files as Azure test attachments, so that we can easily see their contents from the unit test failure screen.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context: #1580
Bump to xamarin/java.interop@397013ed
Changes: xamarin/java.interop@be6048e...397013e
Use of MD5 results in an inability to use Xamarin.Android on
FIPS-enabled machines, because when the System cryptography:
Use FIPS compliant algorithms for encryption,
hashing, and signing setting is enabled on Windows:
then the
System.Security.Cryptography.MD5.Create()method willthrow
InvalidOperationException.In order to support FIPS-enabled machines, use a 64-bit
Cyclic redundancy check algorithm (CRC64) to instead generate
the Java package names, e.g.
crc64aae7d955a89b38db.Nameandcrc64217c8705f00a5054.Name. A simple implementation using alookup table will suffice for our purposes, ported to C# from:
The use of CRC64 avoids the use of MD5, thus permitting execution on
FIPS-enabled Windows machines, and also results in shorter
directory names -- as the Java package name is a directory name --
which helps reduce Windows
MAX_PATHissues.Changes so far
All usage of
MD5.Create()ornew SHA1Managed()have been either:new Crc64()Files.HashStringif more appropriateCalls to
jarsignernow use:The previous default can be restored via
$(AndroidApkSigningAlgorithm)and$(AndroidApkDigestAlgorithm)MSBuild properties.