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

Bootstrapper.DigitalSignature.Apply hide console output #373

Closed
Lechl opened this issue May 24, 2018 · 11 comments
Closed

Bootstrapper.DigitalSignature.Apply hide console output #373

Lechl opened this issue May 24, 2018 · 11 comments

Comments

@Lechl
Copy link

Lechl commented May 24, 2018

Hello Oleg,

i'm using AppVeyor for automated builds. I have a bootstrapper which i sign with a pfx certificate. Works perfect!

But in the AppVeyor build i see the console output of the signtool. This is bad for me because I have no option to suppress the log showing the password of the certificate.

Few enhancement options are: Replace the password always with something like '*****', or add an parameter hiddenLog with default value to the Sign function. Or maybe add a ApplySilent function.

What do you think?

@oleg-shilo
Copy link
Owner

The easiest way for you is to override DidgitalSignature.Apply and do your silent signing.

As a starting point you can use the current implementation:

public virtual int Apply(string fileToSign)
{
    var retValue = CommonTasks.Tasks.DigitalySign(fileToSign, 
                                                  PfxFilePath, 
                                                  TimeUrl?.AbsoluteUri, 
                                                  Password,
                                                  PrepareOptionalArguments(), 
                                                  WellKnownLocations, 
                                                 UseCertificateStore);

    Console.WriteLine(retValue != 0
        ? $"Could not sign the {fileToSign} file."
        : $"The file {fileToSign} was signed successfully.");

    return retValue;
}

Where,

static public int DigitalySign(string fileToSign, string pfxFile, string timeURL, string password,

However in the next release you will have the possibility to modify (or even clear) the output content before it is displayed in the console:

ExternalTool.ConsoleOut = (line) => Console.WriteLine(line.Replace("secret", "******"))

oleg-shilo added a commit that referenced this issue May 26, 2018
Accumulative fixes and minor improvements.

-----
v1.6.5.0
- Added session extension `e.Session.GetMainWindow()` for properly displaying message box from CAs
- Updated all sample projects to target v4.6.1 runtime
- Issue #373: Bootstrapper.DigitalSignature.Apply hide console output
- Issue #372: MsiFile location

v1.6.4.3
- Issue #354: Adding an InternetShortcuts throw NullReferenceException
- Assorted internal fixes (XML docs etc.)
- All compatible `IGenericEntity.Process(ProcessingContext)` cases moved to `WixEntity.CreateAndInsertParentComponent`

v1.6.4.2
* Issue #344: Duplicate symbol 'Directory:ProgramFilesFolder' found

v1.6.4.1
* Updated all IGenericEntity implementations to support 'no-directory' deployments
* Removed all references to the obsolete `IncludeWixExtension`
* Implemented clean algorithm for handling no-dir scenarios without resorting to the `%ProgramFiles%\WixSharp\DummyDir`.
  Algorithm is controlled by `AutoElements.LagacyDummyDirAlgorithm`
* Issue #341: UAC Text is not localized
* Implemented adding custom error description of he ManagedUI.ExitDialog depending on the Install error or cancellation.
@Lechl
Copy link
Author

Lechl commented May 28, 2018

Thank you Oleg!

@oleg-shilo
Copy link
Owner

👍

@dguder
Copy link

dguder commented May 30, 2018

I also want to have the password hidden. But I don't understand your proposal.
From my point of view the line L1612 in CommonTasks.ExternalTool() is the point where the password will be written to log/output. And this is not handled by the new log handler. BTW: The signtool.exe log doesn't display any passwords.
So for the moment a "simple" override doesn't help. IMHO ExternalTool() and DigitalySign(...) must be changed.

@oleg-shilo
Copy link
Owner

Don't I look silly now? I have just released half-done re-implementation of already existing feature.
There was no need for that release. The feature was already available all along.

@Lechl this message is also important for you.

You are absolutely right. This is the line you are referring to:

Compiler.OutputWriteLine("Execute:\n\"" + this.ExePath + "\" " + this.Arguments);

And the default value of Compiler.OutputWriteLine is

class Compiler
{
    public static Action<string> OutputWriteLine = Console.WriteLine;
    . . . 

Thus if you use it as is it will just print the output (e.g. signtool.exe) without any changes. However you can assign it to a new handler that can process the output or even completely "swallow" it.

Compiler.OutputWriteLine = (line)=>{};

I will try to re-release and revoke the change later this week.

Thank you.

@Lechl
Copy link
Author

Lechl commented May 30, 2018

Oh god, we all overlooked this. Thank's @oleg-shilo! Can be closed.

@oleg-shilo
Copy link
Owner

After further considerations I decided to keep that additional output filtering as it allows output massaging per ExternalTool instance. This in turn let me mask the password specifically in the signing routine.

var tool = new ExternalTool
{
    WellKnownLocations = wellKnownLocations ?? @"C:\Program Files\Microsoft SDKs\Windows\v6.0A\bin;" +
    ...
    ExePath = "signtool.exe",
    Arguments = sha1,
    ConsoleOut = (line) => Compiler.OutputWriteLine(line.Replace(password, "***"))
}

Meaning that from the next release you may remove Compiler.OutputWriteLine = (line)=>{}; as the password will be masked automatically.

@dguder
Copy link

dguder commented Jun 1, 2018

Thx @oleg-shilo. I'm sorry, I did not look into Compiler.OutputWriteLine() so I overlooked this too.
And 👍 for the default masking of the password.

With following defined before signature.Apply(exe) the password will be hidden:

Compiler.OutputWriteLine = (line) => Console.WriteLine(line.Replace(secret, "******"));

oleg-shilo added a commit that referenced this issue Jun 3, 2018
* Issue #377: How to register a new MIME type with custom document icon
* Issue #373: Bootstrapper.DigitalSignature.Apply hide console output
* Added missing IconFile support
* SetupEvent sample extended to show how restart itself elevated if required.
@oleg-shilo
Copy link
Owner

oleg-shilo commented Jun 4, 2018

To: @Lechl

Hi Dominic, can you please give me some details on your use of AppVeyor. I am evaluating it for the commercial product CI and want to avoid problems that you have described.

My question are:

  • Do you use AppVeyor free or commercial account?
  • If it is a commercial account, does it mean that the log/output details are still exposed publicly?

Thank you
Oleg

@Lechl
Copy link
Author

Lechl commented Jun 4, 2018

@oleg-shilo

  • I'm using commercial account.
  • No, them are not desposed publicly. But in our case not everyone who has access to the account should have access to the private key.

The default masking is very good, so no one has the chance to get the password via changing the code (eg. remove the Compiler.OutputWriteLine = (line)=>{};)

@oleg-shilo
Copy link
Owner

Great, thank you.

oleg-shilo added a commit that referenced this issue Jun 16, 2018
* Implemented NSIS-based bootstrapper.
* Issue #387: Certificate password can be null
* Issue #377: How to register a new MIME type with custom document icon
* Issue #373: Bootstrapper.DigitalSignature.Apply hide console output
* Added missing IconFile support
* SetupEvent sample extended to show how restart itself elevated if required.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants