NuGet.exe uses a non-FIPS-compliant implementation of SHA256

[hobelinm]

Using NuGet.exe and the Credential Provider from:
https://{account}.pkgs.visualstudio.com/_apis/public/nuget/client/CredentialProviderBundle.zip
We get the following error when trying to use NuGet.exe on machines where FIPS is enabled:

'This implementation is not part of the Windows Platform FIPS validated cryptographic algorithms.'

We need to be able to use NuGet.exe and the credential provider from FIPS enabled systems

[vtbassmatt]

From StackOverflow it looks like NuGet doesn't work under FIPS. Once the core client is working, we can explore whether the credential provider also needs changes.

[vtbassmatt]

Update: NuGet 2.8.6 works under FIPS configuration. 3.2.1, 3.3.0, and 3.4.0-rc do not. Also, the failure seems to happen on install; pack and push are fine.

The VSTS Credential Provider and Authentication Helper both authenticate correctly.

[blowdart]

I was looking at this a bit. I can see at least one problem in CryptoHashProvider.cs.

In lines 107 and 110 you're newing up AlgorithmCryptoServiceProvider. That doesn't do the right thing,

Using new SHA256CryptoServiceProvider() can return a non-FIPS-compliant object. Calling SHA256.Create() or SHA512.Create() should take FIPS into account (at least in desktop & core, I have no idea about Mono).

There's no real reason I can think of to use the managed implementations, and they've gone in core.

[yishaigalatzer]

Thanks for all the feedback.

@TimBarham this needs to go into dev, and also into 3.4 RTM (please wait for my ok to merge to 3.4 RTM) - Timeline - by EOW

[TimBarham]

@hobelinm - I've not been able to repro this issue (I've verified on my machine it uses the correct class when FIPS compliance is turned on, and it is succeeding). I was wondering if you could provide me a stack trace? You should be able to get that using the -verbosity detailed switch when running nuget.exe. I'm interested to see specifically what class is throwing the exception you are seeing.

[TimBarham]

@blowdart - you mention that the XxxCryptoServiceProvider classes don't necessarily use a FIPS compliant algorithm. However, all the documentation I can find indicate that they do, and my own testing confirms this. The exception @hobelinm is seeing is actually thrown in the creator of any class that doesn't use a FIPS compliant algorithm (if FIPS compliance is turned on in Windows). The SHAXxxManaged classes will throw this, but the XxxCryptoServiceProvider classes don't. Am I missing something here? Of course, regardless of that, I do think your suggestion is still correct - we don't need to do the check we are doing. We can just call SHAXxx.create() and it will use the appropriate concrete type (at least in .Net 4.5 scenarios) depending on whether FIPS compliance is turned on or not (I have verified that).

[blowdart]

My comment came from an internal thread on the new core crypto pieces :)

Added bonus CryptoServiceProvider uses CAPI. Create uses CNG, which is, newer and obviously better, because it's newer :)

[TimBarham]

Yeah, I noticed SHAXxx.Create() was giving us CNG classes when FIPS compliance was turned on... And I agree it is the right thing to do to switch to using that... but even with our current implementation I'm confused as to why @hobelinm is hitting this issue, and I hate being confused :).

[blowdart]

I blame @yishaigalatzer. It's a good default.

[TimBarham]

Another data point: @vtbassmatt was able to provide me with a machine on which this repro's. The exception he is seeing is actually in PackageExtractor, here:

using (var sha512 = SHA512.Create())
{
    packageHash = Convert.ToBase64String(sha512.ComputeHash(nupkgStream));
}

It is SHA512.Create() that fails. On my machine, this call works as expected, returning an SHA512Managed instance when FIPS compliance is turned off, and an SHA512Cng instance when it is turned on. So it always works. On Matt's repro machine, it returns an SHA512Managed instance regardless, and so fails when FIPS is on. Interestingly, the code in CryptoHashProviderm where we do the check, works fine on the repro machine.

[TimBarham]

Sorry, accidentally hit the close button.

[rrelyea]

please check into Dev. Then port to 3.4.1* branches. (sync with Deepak on timing for 3.4.1 branch...as he is driving a build from there now)