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

Allow providing absolute path to the certificate file when executing scons #10943

Merged
merged 1 commit into from
Apr 8, 2020

Conversation

lukaszgo1
Copy link
Contributor

Link to issue number:

None, discovered when working on #10493

Summary of the issue:

When wanting to self-sign NVDA launcher it is not possible to provide absolute path to the certificate file. This is not documented, error given by certutil when this is done is quite cryptic but more importantly storing certificate inside the repo increases chances of committing it mistakenly to the public.

Description of how this pull request fixes the issue:

Sconscript used to sign appx version of NVDAno longer assumes that the given path starts at the top dir of the repo. If the path is absolute it is used as is, if not it is being joined with the top directory of the repo.

Testing performed:

With the following layout:

  • file called selfsigned.pfx stored at d:\
  • NVDA repository stored at d:\my_repos_nvda
  • The file from point 1 stored at d:\my_repos\nvda\appveyor\selfsigned.pfx

tried the following scons invocations from the top dir of the repo:

  • scons launcher appx certFile=d:\selfsigned.pfx
  • scons launcher appx certFile=appveyor\selfsigned.pfx
  • scons launcher appx certFile=..\..\selfsigned.pfx

In all cases signing process succeeded

Known issues with pull request:

  • It would be good to create a try branch from this one before merge just to make sure that signed .appx packages can still be created on AppVeyor. I don't have such permission @michaelDCurran @feerrenrut @josephsl could one of you help here?
  • The process of retrieving publisher from the certificate parses output of certutil dump to find the needed info. This unfortunately fails on non-english versions of Windows (the certutil output is localized). I don't think this should be fixed as part of this pr.

Change log entry:

Either none or:
Changes for developers:

  • It is now possible to provide absolute path to the certificate file when signing an NVDA executable.

@michaelDCurran
Copy link
Member

I confirmed this built fine with a try build.

@michaelDCurran michaelDCurran merged commit 2d73010 into nvaccess:master Apr 8, 2020
@nvaccessAuto nvaccessAuto added this to the 2020.1 milestone Apr 8, 2020
@lukaszgo1 lukaszgo1 deleted the betterPathToCertFile branch April 8, 2020 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants