Enable ASLR and DEP in Windows builds

[solardiz]

I'm not a Windows person at all, but apparently on Windows ASLR and DEP are opt-in by programs, and so we need to enable them explicitly, maybe like I did for our 1.8.0-jumbo-1 build here:

http://www.openwall.com/lists/john-users/2016/08/07/1

See also my 2012 john-dev posting referenced from there.

I think we should make this standard in our configure-generated Makefile, which I'd like someone else to do (magnum? Jim? Dhiru?)

Also, maybe I should make it standard in Cygwin targets in the (legacy) Makefile in core.

Related reading, including about a problem and workaround for mingw-w64:

https://insights.sei.cmu.edu/cert/2018/08/when-aslr-is-not-really-aslr---the-case-of-incorrect-assumptions-and-bad-defaults.html

[solardiz]

"Good first issue" for someone who's on Windows and into development for Windows?

[solardiz]

@claudioandre-br Maybe you can look into this issue, or at least let us know what its status is for your Windows builds? Thanks!

[claudioandre-br]

I have ASLR and DEP enabled for all builds, inside the "after_build" step inside CI. Any testing happens in these "enabled" Windows builds.

[solardiz]

@claudioandre-br Great. Thanks. Can you also get that into our normal build process - that is, into our configure-generated Makefile?

[claudioandre-br]

What I can do and test (only) inside CI is this [edited]:

From 682d9fefbe6161177c7673154b120568a46c5a7e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Claudio=20Andr=C3=A9?= <claudioandre.br@gmail.com>
Date: Sat, 30 Mar 2019 12:10:34 -0300
Subject: [PATCH 1/3] windows: create new Makefile targets

Enable ASLR and DEP in Windows builds. Plus, do some renames to .txt and
use DOS linefeeds. JtR have been Windowsed.
---
 src/Makefile.in | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/Makefile.in b/src/Makefile.in
index d5aa90a27..4109fc68c 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -30,10 +30,12 @@ RM = rm -f
 MKDIR = @MKDIR_P@
 TR = tr
 SED = @SED@
+FIND = @FIND@
 NULL = /dev/null
 SORT = @SORT@
 STRIP = @STRIP@
 CPPCHECK=cppcheck
+PEFLAGS=peflags --dynamicbase=true --nxcompat=true

 SHELL = /bin/sh
 VPATH = @srcdir@
@@ -972,6 +974,19 @@ strip: default
    @echo Stripping executables.
    $(STRIP) $(PROJ)

+.PHONY: peflags
+peflags: default
+   @echo Securing executables.
+   $(PEFLAGS) $(PROJ)
+
+.PHONY: windows
+windows: peflags
+   @echo Deploying for Windows.
+   $(SED) -i -e 's/\r*$$/\r/' ../doc/*
+   $(SED) -i -e 's/\r*$$/\r/' ../run/*.conf
+   $(SED) -i -e 's/\r*$$/\r/' ../run/password.lst
+   $(FIND) ../doc -type f -not -name "*.txt" -exec $(MV) "{}" "{}".txt \;
+
 ### cppcheck static coverage analysis
 #
 .PHONY: cppcheck
-- 
2.19.1

[solardiz]

@claudioandre-br Thanks!

Ideally, we'd run peflags on specific .exe files we've built, rather than on *.exe. In fact, we're passing $(PROJ) to there like we do to strip, so perhaps it contains exactly the list of files we need, and *.exe is redundant?

What or who would invoke the windows target?

Is this how your Windows "packages" are built now? Including those sed's and mv's?

I thought we'd obsolete the rename of john.conf to john.ini for Windows now, leaving it as john.conf. Does this even still work with some .conf files including other .conf files?

I think we don't support 8.3 filesystems in jumbo anyway - we have filenames longer than 8, so no need to limit the "extension" to 3 chars anymore.

Is the rename to .txt still desirable on Windows? Are DOS linefeeds still desirable on Windows? Perhaps we should continue making these changes. I'm just not sure.

[claudioandre-br]

*.exe is redundant?

Ctrl+C and Ctrl+V

What or who would invoke the windows target?

I don't know. But, if you are creating something for windows, you should do it to the end. In this case the peflag is the "fake" target.

Is this how your Windows "packages" are built now? Including those sed's and mv's?

No. It is 100% magnumripper. And it works in a regular CMD.

I thought we'd obsolete the rename of john.conf to john.ini for Windows

I added what came to my mind, I will remove the rename.

Is the rename to .txt still desirable on Windows? Are DOS linefeeds still desirable on Windows? Perhaps we should continue making these changes. I'm just not sure.

• notepad can't handle UNIX linefeeds
• txt is (still) a special file type.
• etc.

[solardiz]

@claudioandre-br Somehow I often find your comments moderately cryptic. %-) It's one of those cases now.

What do you mean by "Ctrl+C and Ctrl+V"? Copy-paste of what from where? Regardless, what matters is the resulting code (Makefile, etc.), not how we arrived at it.

When I asked "What or who would invoke the windows target?" I indirectly suggested that we include it or these actions in target(s) used for build of JtR on Windows. Perhaps add these actions to the cygwin* targets in Makefile.legacy, and have configure add them to the default target when producing a Makefile for Windows. Maybe do it by running $(MAKE) windows-deploy as the last step. OTOH, I see an issue with that: if we include renaming of files that came from our original tree, then that can only be done once, whereas a build is supposed to be something that can be redone on an existing tree. So maybe the sed's and mv's should be a separate sub-target, not invoked in default Windows builds (but only invoked by someone intending to package a build), whereas the peflags invocation(s) should be in default Windows targets. Do you agree?

OK, let's continue renaming to .txt for our packages.

[claudioandre-br]

*.exe is redundant?

Yes, it is. I copied the peflags from the command line I used inside CI, where there is no $PROJ.

"What or who would invoke the windows target?"

I will, in after_build: step that runs before testing inside CI. This way:

diff --git a/appveyor.yml b/appveyor.yml
index c39261df9..d54b813ba 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -166,7 +166,8 @@ after_build:
       Write-Host "==  Enable ASLR and DEP for the Windows binary of JtR  ==" -ForegroundColor White
       Write-Host "---------------------------------------------------------"

-  - peflags --dynamicbase=true --nxcompat=true *.exe
+  - ps: |
+      shell "make windows"

   # Compute checksums
   - ps: |

OTOH, I see an issue with that: if we include renaming of files that came from our original tree, then that can only be done once, whereas a build is supposed to be something that can be redone on an existing tree.

I removed the renames and tested it on CI. It works after the fixes and using .conf

[solardiz]

Thank you, @claudioandre-br. I appreciate your taking care of this for our CI-based builds.

@claudioandre-br @magnumripper Should we perhaps add at least the peflags invocation to our standard Windows builds, like I suggested in my previous comment here? Do it before our upcoming release? I am uncomfortable having our users' own builds lack ASLR & DEP. It would be unexpected for them to have their builds weaker than ours.

[magnumripper]

I don't even get what you are talking about. What is peflags? I don't touch Windows anymore, almost ever. I used to do it, for a living, and I simply got fed up after many years. If you think we ought to do it, I think so too 😄

[solardiz]

If you think we ought to do it, I think so too

Well, someone got to do it. @magnumripper I thought it'd be you, since you autoconf'ed john and maintain that. You can find the needed peflags command in Claudio's posting above and by following links I included in my issue description.

[solardiz]

I also almost don't touch Windows, but it's a platform we support and we got to be responsible in supporting it. Building binaries that don't enable ASLR & DEP is rightly considered irresponsible these days.

[magnumripper]

I have no means to test it except the build bots, and I have no idea what it does

[solardiz]

I have no means to test it except the build bots

That's OK. We can ask others to help test it.

I have no idea what it does

This is explained via links from my issue description here.

[solardiz]

I'll close this as the "pending verify" status doesn't appear to encourage testing by anyone anyway.