Add missing timestamper cert to the newer catalog signing APIs - #4061
Merged
Mike Richmond (mirichmo) merged 2 commits intoJun 27, 2017
Conversation
…alog signing APIs
| } | ||
| } | ||
|
|
||
| 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
Travis Plunk (TravisEz13)
approved these changes
Jun 22, 2017
| } | ||
|
|
||
| [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.
Contributor
Author
There was a problem hiding this comment.
Good idea. Will change.
Dongbo Wang (daxian-dbw)
approved these changes
Jun 22, 2017
Member
|
I created issue #4087 to document the lack of tests |
Contributor
Author
|
Mike Richmond (@mirichmo) Is there anything preventing this from being merged? I also need to port this to internal branches. Thanks! |
Thatgfsj (Thatgfsj)
pushed a commit
to Thatgfsj/PowerShell
that referenced
this pull request
Aug 6, 2026
…t-AuthenticodeSignature (PowerShell#4061) * Fixed missing functionality to add timestamper cert for the newer catalog signing APIs for Get-AuthenticodeSignature
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
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.