👍

It should be verified that keepassxc follows security best practices on windows, macOS and linux.
I just noticed this on windows but I don't have enough time to look into all platforms.

Address randomization on Linux seems to be implemented with PIC and therefore incurs a slight performance penalty (at least on x86, x86_64 doesn't have these problems as much). At least that's what I found in this (rather dated) article: https://insights.sei.cmu.edu/cert/2014/02/differences-between-aslr-on-windows-and-linux.html
However, at least some randomization of shared libs etc. is enabled by default on almost all Linux distributions.

Alright, did some more digging. For Linux, we need the options -pie -fPIE for full ASLR. Otherwise the ELF memory section will not be randomized. At least on my system, though, -pie is on by default, so Clang complains that the option is unused. If we add this option, we should maybe also add -Qunused-arguments to suppress that warning.

For stack protection, -fstack-protector can be used, which is already enabled in our CMakeLists.txt. At the cost of performance, this could be replaced with -fstack-protector-all or at least -fstack-protector-strong.

The other thing we could enable is relocation read-only (relro). Partial relro is already enabled with -z,relro. By adding -z,now we could change this to full relro, which also makes the full GOT read-only.

I changed CMakeLists.txt with 2.1.2 before and with 2.1.3 (shows 2.1.2 in about) today with the modifications phoerious mentioned and i get with hardening-check under Debian Jessie:

/usr/local/bin/keepassxc:
Position Independent Executable: yes
Stack protected: yes
Fortify Source functions: no, only unprotected functions found!
Read-only relocations: yes
Immediate binding: yes

with an unmodified CMakeLists.txt i get:

src/keepassxc:
Position Independent Executable: no, normal executable!
Stack protected: yes
Fortify Source functions: no, only unprotected functions found!
Read-only relocations: yes
Immediate binding: no, not found!

Fortify Source functions didn't work for me with keepassx and keepassxc, but apart from that keepassxc works fine.

Fortify source is enabled for release builds in CMakeLists.txt. But it might be possible, that the setting is not really applied because the regular expression in the surrounding if condition is not wrapped in double quotes. I had something like that earlier. But as I understand it, it only checks for overflows in C string functions anyway. So it would be useful at most for code on external libraries.

Any news about this? Can we merge?

We should add the additional flags I suggested and maybe check if there are similar flags for the Mac.
And this branch also needs to be rebased first.

According to https://en.wikipedia.org/wiki/Data_Execution_Prevention#macOS all 64-bit executables have NX stack and heap (DEP) since 10.5 and https://en.wikipedia.org/wiki/Address_space_layout_randomization#OS_X states that all executables since 10.7 support ASLR.

$ otool -hv KeePassX 
Mach header
      magic cputype cpusubtype  caps    filetype ncmds sizeofcmds      flags
MH_MAGIC_64  **X86_64**        ALL LIB64     EXECUTE    21       2624   NOUNDEFS DYLDLINK TWOLEVEL BINDS_TO_WEAK **PIE**

I guess that means we only need to enable the flags for Linux and are set and ready?

I think so. It seems that its enabled by default on macOS and you need to "opt-out" to disable it.

DLLs on windows are relocatable by default but nevertheless DEP+ASLR should be enabled.
Verified with dumpbin /headers libkeepassx-autotype-windows.dll.

Could you enable the option to allow me to push to your branch? I would like to add my Linux changes, but can't do so without copying the branch which would need a new PR.

Allow edits from maintainers. is already enabled.

Then I don't know why I couldn't push. Maybe the plus in the branch name or something. I cloned your repository instead of adding it as a remote, now it works.

@droidmonkey @TheZ3ro Could you have a look at the changes? I think we can merge this if you don't see any further issues. We need to rebase first, though.

Travis is failing

Somehow it can't find the right Zlib version. The two clang builds passed, though. Probably some Travis/Redis fuckup again. I restarted the build, let's see.

Looks like the error message is just completely misleading. -Qunused-arguments appears to be a clang-specific option, so I fixed it by only applying it when clang is used as a compiler.

Why did you use add_gcc_compiler_cflags instead of add_gcc_compiler_flags?

Because I am stupid (or my autocompletion is). I changed it and squashed the last two commits into the "Harden Linux binary" commit.