Menu
▾
▴
1 Attachments
#280 Add !unfinalize to makensis
I wrote this patch based on the idea in this thread: http://forums.winamp.com/showthread.php?t=399106.
Basically it allows the uninstaller to be finalized outside of makensis before being added to the installer.
I wrote this on ubuntu 12.04. I refactored most of !tempfile so that I can use it for the uninstaller as well. I also refactored the postbuild commands so that it can be called for both the installer and the uninstaller.
I've only tested a few output installers to check the operation, I haven't compiled the source code on windows yet so that side will have to be checked.
Whoops, I noticed I missed a block of code when I was moving stuff around. Here's an updated patch.
I've improved the code again, this time refactoring the file handling parts out to their own functions so that I can use them in more than one location.
The linux side appears to be working fine, running !finalize 'nautilus "%1"' opens a window with the uninstaller, !unfinalize does the same if I disable file deletion within nsis. I did run two tests on windows with !unfinalize 'explorer.exe "%1"' but it always exits with an error code, but !finalize does work (it launches the installer).
I don't know how much further I can improve the code, so this is probably the last version from me.
Last edit: Jason 2016-12-24
Well I was wrong, I can improve the code more. Now makensis can change the type of uninstaller it produces based on the existance of any !unfinalize commands. This means it acts the same as standard NSIS by default (to save installer space), and any !unfinalize command forces the whole uninstaller out to a file so that it can be used by other programs, for example signing the uninstaller.
Once again, this patch was written on ubuntu 12.04, and only resulting installers have been tested on windows.
Hi Jason
I patched NSIS 3.01 with this patch and tried signing with osslsigncode on Linux using a self-signed certificate for testing. The installer (created via !finalize) appears signed properly and I get a "Digital Signatures" tab in the properties dialog on Windows for the installer .exe file. The uninstall.exe (that was passed through !unfinalize) that the installer installs works fine as an uninstaller, but it lacks a "Digital Signatures" tab in the properties dialog. So I copied this uninstall.exe back to Linux and tried to verify its signature with "osslsigncode verify". It returns:
It appears the checksum mismatches.
The installer .exe file passes verification by "osslsigncode verify":
So the embedded uninstaller doesn't appear to be correct.
Mukund
Ok, in build.cpp on about line 3356 there is this command: _tremove(fpath);
Comment out that line and that will leave the uninstaller file in /tmp/. Then check the hash of this file with the one extracted from the installer on windows. This will give a clue of where the problem is.
I am wondering whether this patch actually works with NSIS 3.03. If so, is there any hope it will be integrated into the main sources anytime soon?
It should work with 3.03, the core uninstaller code hasn't changed much at all.
read_data_from_file() could be optimized a bit, other than that I'm still happy with this patch.
Thanks Jason. We have tried hard, but we failed: the patch nsis_uninst4.diff cannot be applied to nsis 3.03. There are many rejects: in some cases the changes in 3.03 seem to go in the direction of the patch, in other case the changes in 3.03 are in contrast with the ones of the patch. After a day of work we had to give up. Porting the patch required knowledge of nsis internals that we do not have.
We have not discussed this internally IIRC. Kichik is away for a couple of weeks and I'm a bit busy but I can refactor the post build commands to make this patch easier. We already have getfilesize and readfile helpers in util.cpp now.
I had a look and there has been some refactoring of the code, so I spent a few hours updating the patch. Try this out, I haven't tested it at all though.
I saw that there are some new helpers, so I'm using those helpers instead of my versions now. And I already refactored the post build commands, but it could be improved further in the preprocessing side to remove duplicate code.
Thanks! The new patch applies smoothly to nsis 3.03. Apparently, the patch does not break !finalize or other functionalities, but !unfinalize crashes, apparently because of a buffer overflow. I attach the valgrind output.
Yeah, that's exactly where I thought it would be, it's in alloc_and_get_temp_file_path(). I'll have to find a different way to get the length. Notice how the temp filename is truncated? sizeof() isn't the right way to get the length. I'll see what I can do.
Yes, in the output I see "Adding Authenticode signature to /tmp/ma", whereas an empty file called "makensis0A4FoY" was created under /tmp.
Yep, try this one. I hardcoded the length for it because the string is hardcoded too.
The crash has gone, and in the valgrind output we now have only the two warnings on "Conditional jump or move depends on uninitialised value(s)" in CEXEBuild::GetMacro(wchar_t const, wchar_t*) (scriptpp.cpp:238), which perhaps are unrelated to the patch.
The code-signing script is invoked once on the uninstaller and once on the installer, and it terminates successfully in both cases. However, while the installer is correctly signed (according to Windows 10), the uninstaller is not (according to the same). We will now try to follow the advice given to Mukund Sivaraman (commenting out the removal of the uninstaller) and see if we can get more information on the issue.
I think I found the problem, try this one.
alloc_and_get_temp_file_path returns the wrong size on Windows. Well, it returns the allocated size but I think if we care at all, we would care about the length of the path, not the actual memory. my_strncpy returns the string length (excluding \0).
Here is what we now know:
1) the /tmp/makensisDqv5p6 file has actually been signed;
2) the uninstall.exe that ends up being installed by the installer is not signed;
3) /tmp/makensisDqv5p6 size is around 1 MB, half the size of the installed uninstall.exe, which is around 2 MB;
4) despite the small size, /tmp/makensisDqv5p6, once renamed and substituted to uninstall.exe, seems to work as expected (it deletes all files, I am not sure it does the other things, such as restoring the registry);
5) if we take the installed uninstall.exe and we sign it, its size increases by around 7 kB, i.e., it remains around 2 MB.
(The previous post referred to nsis_uninst6.diff, now trying the new patch nsis_uninst7.diff.)
The same observations obtained with nsis_uninst6.diff have been obtained with nsis_uninst7.diff. How can such a big difference in size be explained?
So, that took longer than I thought. I ended up separating the uninstallers out to their own datablock and now it works (most of the code was correct, it was just the final bit with the reading into the datablock I had wrong). I used a blank command (!unfinalize "") and a linux build of upx (!unfinalize "upx '%1'") to test the branches. The upx one fails because it compresses the file, it only works if you are appending stuff on the end of it.
Try it out.
[edit] There is a little code clean up to do, I'll do that another day since it's not urgent.
Last edit: Jason 2018-10-24
What does "it only works if you are appending stuff on the end of it" mean, does it crash or create a corrupted uninstaller? If that is the case then you must abort with a error message or pad the uninstaller with \0.
Why is write_data_to_file() part of CEXEBuild? It looks like a generic helper function.
I created helper functions like run_postbuild_cmds and alloc_and_get_temp_file_path in trunk so we can concentrate on the uninstaller bits here but it probably invalidated your previous patch. diff8 must be applied to SVN r7024 if that is the case.
when the uninstaller is run, it fails with a crc failure. I guess technically upx should be used on the exehead only, like an uninstaller version of !packhdr. I'll have to check what upx does to the uninstaller before I read the uninstaller back in to makensis. !unpackhdr would be relatively straight forward to add, because the exehead with patched icons is in a buffer, so we can just refactor pack_exe_header() so that it calls a helper that writes it out and runs the packer.
Yeah it is, I should move it to util.cpp.
It works: thanks! I hope this patch can be merged soon.