Xplat Signing/Verification : Fix broken tests for setting privatekey of a X509Certificate2

[heng-liu]

Bug

Fixes: NuGet/Home#8626
Regression: No

• Last working version:
• How are we preventing it in future:

Fix

Details:
For exception as following when setting the private key of an X509Certificate2 certificate:
System.PlatformNotSupportedException : Operation is not supported on this platform. ,
use the workaround mentioned in this dotnet/corefx issue :
dotnet/corefx#14284 (comment)
(above issue was moved to dotnet/runtime#19581 (comment))

For exceptions as following when trying to call Org.BouncyCastle.Security.DotNetUtilities.ToRSA for netcore:
System.PlatformNotSupportedException : 'CspParameters' requires Windows Cryptographic API (CAPI), which is not available on this platform. (bcgit/bc-csharp#160)
use the workaround mentioned in this dotnet/corefx issue:
dotnet/core#3020 (comment)

Affected tests are:

AuthorPrimarySignatures.GetTrustResultAsync_WithRevokedPrimaryCertificate_ReturnsSuspectAsync
RepositoryPrimarySignatures.GetTrustResultAsync_WithRevokedPrimaryCertificate_ReturnsSuspectAsync
VerifyAsync_WithRevokedCountersignatureCertificate_ReturnsSuspectAsync
VerifySignaturesAsync_ExpiredCertificateAndTimestamp_SuccessAsync
VerifySignaturesAsync_ExpiredCertificateAndTimestampWithTooLargeRange_FailsAsync

Testing/Validation

Tests Added: Yes/No
Reason for not adding tests:
Validation:

[dominoFire]

Keep the good work. It would be good to see the number of tests that are enable by those changes.

[dominoFire]

Consider checking that the type casting is successful. Also, consider minimizing the repeated code in #if, #else directives. For example:

Suggested change

	#if IS_DESKTOP
	var privateKey = DotNetUtilities.ToRSA(issueCertificateOptions.KeyPair.Private as RsaPrivateCrtKeyParameters);
	#else
	var rsaParameters = DotNetUtilities.ToRSAParameters(issueCertificateOptions.KeyPair.Private as RsaPrivateCrtKeyParameters);
	RSACryptoServiceProvider rsaCsp = new RSACryptoServiceProvider();
	rsaCsp.ImportParameters(rsaParameters);
	var privateKey = rsaCsp;
	#endif
	#if IS_DESKTOP
	var privateKeyPair = issueCertificateOptions.KeyPair.Private as RsaPrivateCrtKeyParameters;
	Assert.NotNull(privateKeyPair);
	#if IS_DESKTOP
	var privateKey = DotNetUtilities.ToRSA(privateKeyPair);
	#else
	var rsaParameters = DotNetUtilities.ToRSAParameters(privateKeyPair);
	RSACryptoServiceProvider rsaCsp = new RSACryptoServiceProvider();
	rsaCsp.ImportParameters(rsaParameters);
	var privateKey = rsaCsp;
	#endif

[heng-liu]

Thanks! Fixed the type casting by:

Assert.IsType<RsaPrivateCrtKeyParameters>(keyPair.Private);

var privateKeyParameters = (RsaPrivateCrtKeyParameters)keyPair.Private;

[dominoFire]

Consider using a name alias to avoid prefixing the type with the full name (namespace + type). In this case:

Suggested change

	using (var signatureRequest = new AuthorSignPackageRequest(testCertificate, NuGet.Common.HashAlgorithmName.SHA256))
	using (var signatureRequest = new AuthorSignPackageRequest(testCertificate, HashAlgorithmName.SHA256))

Also, at the beginning of the file, you could add:

using HashAlgorithmName = NuGet.Common.HashAlgorithmName;

[heng-liu]

Thanks! Fixed.

[dtivel]

Please review https://github.com/NuGet/NuGet.Client/blob/dev/docs/coding-guidelines.md

Make sure documents are formatted/indented properly.

[dtivel]

From a quick glance, it doesn't look like X509Certificate2.Dispose() disposes of X509Certificate2.PrivateKey. Here RSACryptoServiceProvider implements IDisposable but this instance doesn't appear to be disposed of anywhere.

We should consider disposing of PrivateKey. Open an issue if necessary.

[heng-liu]

Thanks! Issue NuGet/Home#9114 raised ( X509Certificate2.PrivateKey is not disposed in X509Certificate2.Dispose() for netcore)

[heng-liu]

Keep the good work. It would be good to see the number of tests that are enable by those changes.

Thanks for your advise! Added the affected tests list in PR