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

Only update resources when necessary to keep signature valid #3323

Merged
merged 2 commits into from Feb 27, 2018

Conversation

Projects
None yet
2 participants
@tlecomte
Contributor

tlecomte commented Feb 21, 2018

This fixes #2526. Pyinstaller is updating the resources for python35.dll and makes its signature invalid. This then prevents that dll from being signed with the 'signtool' utility (which then can have more consequences, like the inability to publish the app to a repository where signing is mandatory).

In this commit, the resource update is skipped if it is not necessary, i.e. it is only performed if there have been som redirections or if the dll has been made private.

Test scenario: run pyinstaller on a Python app (https://github.com/tlecomte/friture) without these changes and 'signtool' fails to sign python35.dll with the error code 0x800700C1, ERROR_BAD_EXE_FORMAT. With these changes, the packaged python35.dll still shows its "Python Software Foundation" signature, and 'signtool' can successfully re-sign it.

Only update resources when necessary to keep signature valid
This fixes #2526. Pyinstaller is updating the resources for python35.dll and makes its signature invalid. This then prevents that dll from being signed with the 'signtool' utility (which then can have more consequences, like the inability to publish the app to a repository where signing is mandatory).

In this commit, the resource update is skipped if it is not necessary, i.e. it is only performed if there have been som redirections or if the dll has been made private.
@htgoebel

This comment has been minimized.

Member

htgoebel commented Feb 25, 2018

Thanks for the pull-request. I'm curious if breaking the signature is expected if updating the resource is necessary?!

(Of course the signature should change, if the content changes, but will developer be able to deal with this?)

@htgoebel htgoebel added the Windows label Feb 25, 2018

@tlecomte

This comment has been minimized.

Contributor

tlecomte commented Feb 25, 2018

That sounds like a good question, and one I am unable to answer.

@tlecomte

This comment has been minimized.

Contributor

tlecomte commented Feb 25, 2018

Maybe pyinstaller could also remove the signature when it has to update the resources, so that the dll can be re-signed ? (since signtool seemed to be unhappy with a dll that has an invalid signature).
Anyway those questions look out-of-scope for this PR, in my humble opinion of a developer who just tries to get signing to work on a typical project...

@htgoebel

This comment has been minimized.

Member

htgoebel commented Feb 25, 2018

I agree, this pull-request improves the situation, and I'll merge it if tests pass.

@htgoebel htgoebel merged commit 2611919 into pyinstaller:develop Feb 27, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@tlecomte

This comment has been minimized.

Contributor

tlecomte commented Mar 6, 2018

Thanks!

@htgoebel htgoebel added this to the PyInstaller 3.4 milestone Mar 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment