Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes so AltCover will run when FIPS compliance is required #214

Closed
hfickes opened this issue Mar 22, 2024 · 13 comments
Closed

Changes so AltCover will run when FIPS compliance is required #214

hfickes opened this issue Mar 22, 2024 · 13 comments
Labels
Third party Problem lies in a consumed library which may or may not have a work-round

Comments

@hfickes
Copy link

hfickes commented Mar 22, 2024

Short Version: Can Altcover use a FIPS compliant algorithm such as SHA384 instead of SHA-1?

Long Version
Altcover throws an exception if a machine is configured to be FIPS compliant. The US government requires compliance with FIPS-140-3 for many systems. A description of this is at:

https://csrc.nist.gov/CSRC/media/Projects/cryptographic-module-validation-program/documents/fips%20140-3/FIPS%20140-3%20IG.pdf

and there are plenty of discussions regarding the usefulness of FIPS, though many refer to older versions of the specification.

Anyhow, we use AltCover on development VMs where we have FIPS compliance turned off but we need to run these tests on government machines where FIPS compliance is enabled.

You can enable/disable FIPS compliance on Windows via Group Policy or by setting the DWORD registry entry at:

HKLM\System\CurrentControlSet\Control\Lsa\FIPSAlgorithmPolicy\Enabled to 1 for enabled, and 0 for disabled

When FIPS compliance is on, running Altcover throws an exception while instrumenting the assemblies at:

Unhandled Exception: System.InvalidOperationException: This implementation is not part of the Windows Platform FIPS validated cryptographic algorithms.
at System.Security.Cryptography.SHA1Managed..ctor()
at Mono.Cecil.CryptoService.HashStream(Stream stream, ImageWriter writer, Int32& strong_name_pointer)
at Mono.Cecil.CryptoService.StrongName(Stream stream, ImageWriter writer, WriterParameters parameters)
at Mono.Cecil.ModuleWriter.Write(ModuleDefinition module, Disposable1 stream, WriterParameters parameters) at Mono.Cecil.ModuleWriter.WriteModule(ModuleDefinition module, Disposable1 stream, WriterParameters parameters)
at Mono.Cecil.ModuleDefinition.Write(Stream stream, WriterParameters parameters)
at AltCover.Instrument.I.writeAssembly(AssemblyDefinition assembly, String path) in ///AltCover.Engine/Instrument.fs:line 512
at AltCover.Instrument.I.writeAssemblies(AssemblyDefinition definition, String file, IEnumerable1 targets, FSharpFunc2 sink) in /
//AltCover.Engine/Instrument.fs:line 870
at AltCover.Instrument.I.visitAfterAssembly(InstrumentContext state, AssemblyEntry assembly) in ///AltCover.Engine/Instrument.fs:line 1132
at AltCover.Instrument.I.instrumentationVisitorWrapper(FSharpFunc2 core, InstrumentContext state, Node node) in /_//AltCover.Engine/Instrument.fs:line 1468 at AltCover.Main.I.visitors@755.Invoke(InstrumentContext state, Node node) at AltCover.Visitor.stateful@1652-1.Invoke(T node) in /_//AltCover.Engine/Visitor.fs:line 1654 at Microsoft.FSharp.Primitives.Basics.List.mapToFreshConsTail[a,b](FSharpList1 cons, FSharpFunc2 f, FSharpList1 x) in D:\a_work\1\s\src\FSharp.Core\local.fs:line 236
at Microsoft.FSharp.Primitives.Basics.List.map[T,TResult](FSharpFunc2 mapping, FSharpList1 x) in D:\a_work\1\s\src\FSharp.Core\local.fs:line 247
at Microsoft.FSharp.Collections.SeqModule.Fold[T,TState](FSharpFunc2 folder, TState state, IEnumerable1 source) in D:\a_work\1\s\src\FSharp.Core\seq.fs:line 913
at AltCover.Visitor.visit(IEnumerable1 visitors, IEnumerable1 assemblies) in /
//AltCover.Engine/Visitor.fs:line 1639
at AltCover.Main.I.result@735.Invoke(Unit unitVar0) in ///AltCover.Engine/Main.fs:line 758
at AltCover.PathOperation.DoPathOperation[TAny](FSharpFunc2 operation, FSharpFunc2 handle) in /
//AltCover.Engine/Output.fs:line 23
at AltCover.CommandLine.doPathOperation@433-1.Invoke(FSharpFunc`2 f, a defaultValue, Boolean store)
at AltCover.Main.I.doInstrumentation(String[] arguments) in /_//AltCover.Engine/Main.fs:line 730

I've looked at the code (first time I've read F# code outside of articles) and I can see where SHA-1 is being used. Could this be replaced with something that is compliant for signing such as SHA-384.

SHA-1 is compliant for some uses, but not for signing. From the document linked above:

"Whether a security function is approved may be context sensitive to the service in which it is included. For
example, at Overall Security Rating 1, the SHA-1 function is an approved algorithm if it is used for an
integrity check service, but it is not approved if it is used as part of a digital signature generation service.
Therefore, it is not required to provide the indicator at the API level of cryptographic functions, as long as the
service implementing the API provides the corresponding indicator that unambiguously indicates the approved
security services"

Thank,
Herb

@SteveGilham SteveGilham added the Third party Problem lies in a consumed library which may or may not have a work-round label Mar 23, 2024
@SteveGilham
Copy link
Owner

SteveGilham commented Mar 23, 2024

That's a simple one in principle - use of SHA1Managed rather than the compliant SHA1CryptoServiceProvider implementation. Unfortunately the exception is being thrown from the third party Mono.Cecil library while writing the instrumented assemblies, and not my code. I've opened issue jbevain/cecil#938 as a forwarding of this one, and submitted PR jbevain/cecil#939 with the fix.

@SteveGilham
Copy link
Owner

As to why SHA1 - the use that's causing the issue is that the strong name PublicKeyToken value is formed from the first 8 bytes of the SHA1 hash of the strong name public key. The hash is also used in the OpenCover format where a whole-file hash is taken of each assembly to distinguish different builds of the same name.

@SteveGilham
Copy link
Owner

Release 8.8.10 uses a patched version of Cecil that invokes the compliant SHA1 implementation, so hopefully will address the issue.

@SteveGilham SteveGilham added the ready to close Believed addressed, waiting to hear to the contrary label Apr 6, 2024
@hfickes
Copy link
Author

hfickes commented Apr 9, 2024

Unfortunately, that didn't help. What I see in the stack trace is the following:

Unhandled Exception: System.InvalidOperationException: This implementation is not part of the Windows Platform FIPS validated cryptographic algorithms.

at System.Security.Cryptography.SHA1Managed..ctor()
at Mono.Cecil.CryptoService.HashStream(Stream stream, ImageWriter writer, Int32& strong_name_pointer)
at Mono.Cecil.CryptoService.StrongName(Stream stream, ImageWriter writer, WriterParameters parameters)
at Mono.Cecil.ModuleWriter.Write(ModuleDefinition module, Disposable1 stream, WriterParameters parameters) at Mono.Cecil.ModuleWriter.WriteModule(ModuleDefinition module, Disposable1 stream, WriterParameters parameters)
at Mono.Cecil.ModuleDefinition.Write(Stream stream, WriterParameters parameters)
at AltCover.Instrument.I.writeAssembly(AssemblyDefinition assembly, String path) in ///AltCover.Engine/Instrument.fs:line 512
at AltCover.Instrument.I.writeAssemblies(AssemblyDefinition definition, String file, IEnumerable1 targets, FSharpFunc2 sink) in /
//AltCover.Engine/Instrument.fs:line 870
at AltCover.Instrument.I.visitAfterAssembly(InstrumentContext state, AssemblyEntry assembly) in ///AltCover.Engine/Instrument.fs:line 1132
at AltCover.Instrument.I.instrumentationVisitorWrapper(FSharpFunc2 core, InstrumentContext state, Node node) in /_//AltCover.Engine/Instrument.fs:line 1468 at AltCover.Main.I.visitors@755.Invoke(InstrumentContext state, Node node) at AltCover.Visitor.stateful@1652-1.Invoke(T node) in /_//AltCover.Engine/Visitor.fs:line 1654 at Microsoft.FSharp.Primitives.Basics.List.mapToFreshConsTail[a,b](FSharpList1 cons, FSharpFunc2 f, FSharpList1 x) in D:\a_work\1\s\src\FSharp.Core\local.fs:line 236
at Microsoft.FSharp.Primitives.Basics.List.map[T,TResult](FSharpFunc2 mapping, FSharpList1 x) in D:\a_work\1\s\src\FSharp.Core\local.fs:line 247
at Microsoft.FSharp.Collections.SeqModule.Fold[T,TState](FSharpFunc2 folder, TState state, IEnumerable1 source) in D:\a_work\1\s\src\FSharp.Core\seq.fs:line 913
at AltCover.Visitor.visit(IEnumerable1 visitors, IEnumerable1 assemblies) in /
//AltCover.Engine/Visitor.fs:line 1639
at AltCover.Main.I.result@735.Invoke(Unit unitVar0) in ///AltCover.Engine/Main.fs:line 758
at AltCover.PathOperation.DoPathOperation[TAny](FSharpFunc2 operation, FSharpFunc2 handle) in /
//AltCover.Engine/Output.fs:line 23
at AltCover.CommandLine.doPathOperation@433-1.Invoke(FSharpFunc`2 f, a defaultValue, Boolean store)
at AltCover.Main.I.doInstrumentation(String[] arguments) in /_//AltCover.Engine/Main.fs:line 730

@SteveGilham
Copy link
Owner

Are you certain that you are using the 8.8.x release?

I had wondered if maybe I had ended up getting the unpatched Cecil in the package by mistake, but having just downloaded 8.8.10 from nuget.org, decompiling the Mono.Cecil assembly from that reassures me that it is not invoking the non-compliant SHA1Managed implementation that is being reported.

Screenshot 2024-04-09 171751

@hfickes
Copy link
Author

hfickes commented Apr 9, 2024

You are correct. I forgot to make a change in the path I was using so I still was running the 8.7.3 version of Altcover.

But when I changed to the 8.8.10 version, I got a completely new error. The following is from the log file.

Mono.Cecil.AssemblyResolutionException: Failed to resolve assembly: 'System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089'
at Mono.Cecil.BaseAssemblyResolver.Resolve(AssemblyNameReference name, ReaderParameters parameters)
at Mono.Cecil.DefaultAssemblyResolver.Resolve(AssemblyNameReference name)
at Mono.Cecil.MetadataResolver.Resolve(TypeReference type)
at Mono.Cecil.TypeReference.Resolve()
at Mono.Cecil.Mixin.CheckedResolve(TypeReference self)
at Mono.Cecil.MetadataBuilder.GetConstantType(TypeReference constant_type, Object constant)
at Mono.Cecil.MetadataBuilder.AddConstant(IConstantProvider owner, TypeReference type)
at Mono.Cecil.MetadataBuilder.AddField(FieldDefinition field)
at Mono.Cecil.MetadataBuilder.AddFields(TypeDefinition type)
at Mono.Cecil.MetadataBuilder.AddType(TypeDefinition type)
at Mono.Cecil.MetadataBuilder.AddTypes()
at Mono.Cecil.MetadataBuilder.BuildTypes()
at Mono.Cecil.MetadataBuilder.BuildModule()
at Mono.Cecil.MetadataBuilder.BuildMetadata()
at Mono.Cecil.ModuleWriter.<>c.b__2_0(MetadataBuilder builder, MetadataReader )
at Mono.Cecil.ModuleDefinition.Read[TItem,TRet](TItem item, Func3 read) at Mono.Cecil.ModuleWriter.Write(ModuleDefinition module, Disposable1 stream, WriterParameters parameters)
at Mono.Cecil.ModuleWriter.WriteModule(ModuleDefinition module, Disposable1 stream, WriterParameters parameters) at Mono.Cecil.ModuleDefinition.Write(Stream stream, WriterParameters parameters) at AltCover.Instrument.I.writeAssembly(AssemblyDefinition assembly, String path) in /_//AltCover.Engine/Instrument.fs:line 512 at AltCover.Instrument.I.writeAssemblies(AssemblyDefinition definition, String file, IEnumerable1 targets, FSharpFunc2 sink) in /_//AltCover.Engine/Instrument.fs:line 870 at AltCover.Instrument.I.visitAfterAssembly(InstrumentContext state, AssemblyEntry assembly) in /_//AltCover.Engine/Instrument.fs:line 1132 at AltCover.Instrument.I.instrumentationVisitorWrapper(FSharpFunc2 core, InstrumentContext state, Node node) in /
//AltCover.Engine/Instrument.fs:line 1468
at AltCover.Main.I.visitors@776.Invoke(InstrumentContext state, Node node)
at AltCover.Visitor.stateful@1659-1.Invoke(T node) in ///AltCover.Engine/Visitor.fs:line 1661
at Microsoft.FSharp.Primitives.Basics.List.mapToFreshConsTail[a,b](FSharpList1 cons, FSharpFunc2 f, FSharpList1 x) in D:\a\_work\1\s\src\FSharp.Core\local.fs:line 236 at Microsoft.FSharp.Primitives.Basics.List.map[T,TResult](FSharpFunc2 mapping, FSharpList1 x) in D:\a\_work\1\s\src\FSharp.Core\local.fs:line 247 at Microsoft.FSharp.Collections.SeqModule.Fold[T,TState](FSharpFunc2 folder, TState state, IEnumerable1 source) in D:\a\_work\1\s\src\FSharp.Core\seq.fs:line 913 at AltCover.Visitor.visit(IEnumerable1 visitors, IEnumerable1 assemblies) in /_//AltCover.Engine/Visitor.fs:line 1646 at AltCover.Main.I.result@756.Invoke(Unit unitVar0) in /_//AltCover.Engine/Main.fs:line 779 at AltCover.PathOperation.DoPathOperation[TAny](FSharpFunc2 operation, FSharpFunc`2 handle) in /
//AltCover.Engine/Output.fs:line 21
AssemblyReference =
System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089
FileName =

FusionLog =

Data =
seq []
InnerException =

TargetSite =
Mono.Cecil.AssemblyDefinition Resolve(Mono.Cecil.AssemblyNameReference, Mono.Cecil.ReaderParameters)
HelpLink =

Source =
"Mono.Cecil"
HResult =
-2147024894

@SteveGilham
Copy link
Owner

When writing an assembly, Cecil needs to perform assembly linkage against the dependencies. One or more of your assemblies is using types from System.Windows.Forms, so it's asking you to tell it where that assembly is, so it can perform the linkage, since it's not in the same folder as is being instrumented, nor is it in the nuget cache.

This is done with the --dependency=[full path of assembly] option (or equivalents).

This wouldn't happen to be a .Net Framework application expecting to find files in the GAC, would it now?

@hfickes
Copy link
Author

hfickes commented Apr 10, 2024

Yes, it is a .Net Framework 4.8 application using the GAC. However, this worked before with 8.7.3. The only change was to use the newer altcover version 8.8.10. Switching back, it worked again.

I suspect this has to do with changes other than the FIPS one you made, so I can open a different issue if you'd prefer.

@SteveGilham
Copy link
Owner

That's odd, as the changes since 8.7.3 have just a) added an entry to each of 2 enums and b) allowed the report file name to be a relative path and c) patched the SHA1 implementation used by Cecil.

However, it's simple enough to add the GAC to the list of places to look for dependencies, if it's not being picked up automagically.

@SteveGilham
Copy link
Owner

It occurs to me to ask if this error failing to find the System.Windows.Forms assembly is being observed on one of the machines where the FIPS compliance failure was happening - in particular, if the difference in environment is between a development machine and a user machine without developer tools, more than simply a switch of a registry setting.

@hfickes
Copy link
Author

hfickes commented Apr 10, 2024

It happens both on the machine with FIPS compliance enabled, and on our test machine where we keep it turned off so that we can run Altcover 8.7 and earlier.

This happens on the same machine we've been using all along. All that I change to make it happen is to switch from 8.7.3 to 8.8.10.

@SteveGilham
Copy link
Owner

Release 8.8.21 explicitly adds the GAC to the locations searched for dependencies; which I trust will resolve the newer problem.

@hfickes
Copy link
Author

hfickes commented Apr 15, 2024

That works great for us! Thank you very much.

@hfickes hfickes closed this as completed Apr 15, 2024
@SteveGilham SteveGilham removed the ready to close Believed addressed, waiting to hear to the contrary label Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Third party Problem lies in a consumed library which may or may not have a work-round
Projects
None yet
Development

No branches or pull requests

2 participants