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

Python3: Cannot sign side-loadable Windows Store appx package #9762

Closed
michaelDCurran opened this issue Jun 17, 2019 · 11 comments
Closed

Python3: Cannot sign side-loadable Windows Store appx package #9762

michaelDCurran opened this issue Jun 17, 2019 · 11 comments
Milestone

Comments

@michaelDCurran
Copy link
Member

Steps to reproduce:

Build the appx package using the threshold_py3_staging branch, providing an authenticode certificate:

scons appx certFile=<path> certPassword=<password>

Actual behavior:

The package builds, but then signtool shows the following error:

SignTool Error: SignedCode::Sign returned error: 0x800700C1
        For more information, please see https://aka.ms/badexeformat

Expected behavior:

Signtool should succeedd.

more details

It seems this error exists due to the existance of python37.dll in the appx package. I.e. removing it allows the package to be signed.
Trying to sign python37.dll individually also causes this same signtool error.
For testing purposes, including python27.dll in the package instead, or in fact signing it individually with signtool, does not cause the error.
Extracting the file sections with 7zip, both files report some crc errors, but:
python37.dll seems to contain a certificate section (with a data error), where python27.dll does not contain a certificate section at all.
So, it looks like python37.dll is supposed to be signed, but might have a corrupt signature. Yet, get-authenticodesignature in Powershell does not report an invalid signature, rather it reports no signature at all.
I tried using ImageRemoveCertificate from imagehlp.dll to remove the certificate, but that seems to fail as well.

@michaelDCurran
Copy link
Member Author

Oh dear: looking at this further I see that python37.dll in Python's installation directory is correctly signed with a valid certificate, but, once it is copied into dist with py2exe, it is missing its certificate, or at least ends up with a corrupt one. Something very odd is going on here at the copying stage.

@michaelDCurran
Copy link
Member Author

Python37.dll is missing 7kb when copied. The real one is 3521kb. the copied one is 3514kb.
I have tracked it down to just after py2exe copies the dll with shutil.copy2, it then does the following:

with UpdateResources(dst, delete_existing=False) as resource:
                resource.add_string(1000, "py2exe")

It is this action that seems to truncate the file, corrupting the certificate.
@LeonarddeR @albertosottile

@michaelDCurran
Copy link
Member Author

Changing the resource on a signed file logically makes the file invalid, as the signature will not match.
@albertosottile do you know the reason why py2exe needs to add this string resource to python dlls?
I think that in Python2, python27.dll was never signed in the first place. Python3 has started signing its dll. Which of course is a good thing for them to do.

@LeonarddeR
Copy link
Collaborator

Work around in #9796. This means that there's still an issue in py2exe regarding this.

@albertosottile
Copy link

albertosottile commented Jun 24, 2019

@albertosottile do you know the reason why py2exe needs to add this string resource to python dlls?
I think that in Python2, python27.dll was never signed in the first place. Python3 has started signing its dll. Which of course is a good thing for them to do.

Thanks for finding this. To be honest, I was not aware of this string addition. I can definitely see why this is extremely dangerous in the context of signed binaries, so my first instinct would be to just remove the use of this method entirely. Indeed, some preliminary tests seem to suggest that this add_write method is no longer needed.

Even though I ported all the commit messages from the original SVN repository, the commits for the python3 branch were not there from the beginning. According to the original commit message of this branch, "development happened on https://ctypes-stuff.googlecode.com/svn/trunk/mf", but this URL is now dead, and so it looks that the history of these files is gone.

Hence, I can only speculate about the reason why that string was added in the binaries. I think it was needed to distinguish between regular python interpreters and the interpreter embedded by py2exe. This hypothesis is supported by this comment in distutils_buildexe (a module that is no longer used, to further complicate things).

In any case, this detection feature should no longer be relevant since now this purpose is served by the bootloader. So, I believe we could get rid of that extra writer, and tests seem to confirm that.

In the next steps, I'll push this change to my repository and, if everything goes right in CI, include it in the next release of py2exe.

EDIT: the fix passed the CI stage and some real-world packaging tests that I made on my PC. Also, your workaround seems to prove that python DLL tampering is no longer needed in py2exe. This fix will be included in the next release of py2exe.

@albertosottile
Copy link

@LeonarddeR How do you feel about the status of the NVDA py2exe release? Do you think that further problems are yet to be discovered? I ask because I would like to release py2exe 0.9.3.1 more or less when you deem that everything has been settled down from your side, so you can just use the new version and remove all the custom workarounds.

@LeonarddeR
Copy link
Collaborator

LeonarddeR commented Jun 25, 2019 via email

@albertosottile
Copy link

Version 0.9.3.1 of py2exe has just been released (GitHub). This update includes all the fixes you need to pack NVDA, up to now. I encourage you to update your URLs and patches accordingly. As usual, thanks for all your efforts.

@josephsl
Copy link
Collaborator

josephsl commented Jun 25, 2019 via email

@albertosottile
Copy link

Hi, may I propose announcing this on various Python mailing lists and forums so many people can share this news? Also, if you can, please feel free to talk to original Py2exe creators so they can work with you as well. Thanks.

Hi, in principle I like your proposal, but I am not subscribed to any Python mailing list. Hence, if you could announce it wherever you prefer, that would be great.

About the original py2exe creators, I am not sure about what would the best approach be. The original py2exe project is not officially abandoned, yet the last release was published almost five years ago. So, I do not know if the authors would still be interested in contributing to this project. I was planning to contact the creators in any case if/when the fork will reach a decent maturity stage, to publish the wheels on pypi using the original name. Asking now to them to contribute to this fork seems a little weird to me, after all these years. But I might be utterly wrong so, if you know them or you think otherwise, please feel free to contact them and introduce them to this project.

@josephsl
Copy link
Collaborator

josephsl commented Jun 25, 2019 via email

lukaszgo1 added a commit to lukaszgo1/nvda that referenced this issue Dec 23, 2020
Latest version of Py2exe no longer invalidates signature of Python's dll so this workaround is no longer needed
lukaszgo1 added a commit to lukaszgo1/nvda that referenced this issue Mar 1, 2021
Latest version of Py2exe no longer invalidates signature of Python's dll so this workaround is no longer needed
lukaszgo1 added a commit to lukaszgo1/nvda that referenced this issue Mar 9, 2021
Latest version of Py2exe no longer invalidates signature of Python's dll so this workaround is no longer needed
michaelDCurran pushed a commit that referenced this issue Mar 11, 2021
Latest version of Py2exe no longer invalidates signature of Python's dll so this workaround is no longer needed
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

No branches or pull requests

5 participants