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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix using SignTool options #156

Merged
merged 13 commits into from Nov 1, 2022
Merged

Fix using SignTool options #156

merged 13 commits into from Nov 1, 2022

Conversation

YehudaKremer
Copy link
Owner

@YehudaKremer YehudaKremer commented Oct 29, 2022

Hello @zobo

In this PR I'm trying to fix problems with SignTool.

this PR includes the changes:

  1. user can use SignTool freely as the office documentation instructing
  2. when trying to sign with an installed certificate by using one of the options '/n', '/i' and '/sha1', this package will try to extract the certificate Subject with PowerShell script you provided

About .crt, I don't think SignTool supports .crt files, if I am wrong can you please provide me an example, thanks.

I would appreciate it if you could go through the changes in your spare time 馃憤

@YehudaKremer YehudaKremer added bug Something isn't working enhancement New feature or request labels Oct 29, 2022
@YehudaKremer YehudaKremer changed the title fix using SignTool options Fix using SignTool options Oct 29, 2022
@zobo
Copy link

zobo commented Oct 30, 2022

The PR looks good, indeed the simple split by space caused issues when I was experimenting. I like the use of extensions.

Regarding CRT. The docs say:

/f聽SignCertFile
Specifies the signing certificate in a file. If the file is in Personal Information Exchange (PFX) format and protected by a password, use the /p option to specify the password. If the file does not contain private keys, use the /csp and /kc options to specify the CSP and private key container name.

My experience is that if I provide a .crt file, the SignTool will figure out what private key to use. I did not need to add /csp and /kc options.

I'd suggest to also look for the case where a certificate file is provided, but no password and then extract the subject via:
new-object System.Security.Cryptography.X509Certificates.X509Certificate2("..\file.crt") | select -expandproperty Subject -First 1

I'll try to provide the code, but I'll be leaving for a week of travel soon.

'the Publisher is the certificate "Subject" in this exact format: "CN=Contoso Software, O=Contoso Corporation, C=US"');
_logger.stdout('see where you can found your certificate Subject:');
_logger.stdout(
'https://drive.google.com/file/d/1oAsnrp2Kf-jZ_kaRjyF5llQ0YZy1IwNe/view?usp=sharing'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest you store the image into this GitHub repository and link to it via a commit url - to make it permanent.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done here 1149448

} else if (_config.certificatePath != null &&
extension(_config.certificatePath!).toLowerCase() == '.pfx') {
subject = await _getPfxCertificateSubject();
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick dry code - have not tested it....

else if (_config.certificatePath != null)  {
  subject = await _getCrtCertificateSubject();
}
  Future<String> _getCrtCertificateSubject() async {
    _logger.trace('getting crt certificate Subject');

    var certificateDetailsProcess = await _executePowershellCommand(
        "new-object System.Security.Cryptography.X509Certificates.X509Certificate2(\"${_config.certificatePath}\") | select -expandproperty Subject -First 1
");

    certificateDetailsProcess.exitOnError();

    var subject = (certificateDetailsProcess.stdout as String).trim();

    return subject;
  }

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done here: b252a98

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also added the documentation bc0d17e

@YehudaKremer
Copy link
Owner Author

Good notes and examples,
I will add them to this branch soon as possible.

Thank You! 馃憤

@YehudaKremer YehudaKremer merged commit 5587d23 into main Nov 1, 2022
@YehudaKremer
Copy link
Owner Author

Hello @zobo

I publish a new version (3.7.0) with these changes:

Thank you!

@YehudaKremer YehudaKremer deleted the SignToolFix branch November 1, 2022 14:03
@zobo
Copy link

zobo commented Nov 10, 2022

Looks like everything is working great. Thank you again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants