Add missing timestamper cert to the newer catalog signing APIs #4061
Conversation
…alog signing APIs
PaulHigin
added
WG-Cmdlets
| } | ||
| } | ||
|
|
||
| Diagnostics.Assert(((error == 0) && (signature != null)) || (error != 0), "GetSignatureFromWintrustData: general crypto failure"); |
There was a problem hiding this comment.
Question about this case, will sync offline
PaulHigin
changed the title
| } | ||
|
|
||
| [ArchitectureSensitive] | ||
| private static bool TryGetProviderSigner(IntPtr wvtStateData, ref IntPtr pProvSigner, ref X509Certificate2 timestamperCert) |
There was a problem hiding this comment.
It seems to me both pProvSigner and timestamperCert are better to be an out parameter instead of a ref parameter.
| else | ||
| { | ||
| signature = new Signature(filename, error, cert); | ||
| } |
There was a problem hiding this comment.
Once the parameters pProvSigner and timestamperCert are changed to out parameters, you can use the new language feature in C# for out parameters:
if (int.TryParse(input, out int result))
WriteLine(result);
else
WriteLine("Could not parse input");So this code block can be rewritten to:
if (TryGetProviderSigner(phStateData, out IntPtr pProvSigner, out X509Certificate2 timestamperCert) && timestamperCert != null)
{
signature = new Signature(filename, error, cert, timestamperCert);
}
else
{
signature = new Signature(filename, error, cert);
}
| X509Certificate2 signerCert = null; | ||
| IntPtr pProvSigner = IntPtr.Zero; | ||
| X509Certificate2 timestamperCert = null; | ||
| if (TryGetProviderSigner(wtd.hWVTStateData, ref pProvSigner, ref timestamperCert)) |
There was a problem hiding this comment.
Same here. Once change to out parameters, you can use the new feature in C# to make the code cleaner.
There was a problem hiding this comment.
Good idea. Will change.
|
I created issue #4087 to document the lack of tests |
|
@mirichmo Is there anything preventing this from being merged? I also need to port this to internal branches. Thanks! |
6 participants
This fixes Issue #4060
Code to obtain the file signature time stamp cert was missing from the newer catalog signing APIs, with the result that Get-AuthenticodeSignature would not include the time stamp cert in the Signature object.
Fix is to refactor and use the older API code for obtaining this.
I didn't add a test for this because no catalog signing tests are in GitHub yet and porting them will be a separate work item.