hardening: fix #18995 #28029

cstrahan · 2017-08-08T03:45:49Z

Motivation for this change

This should (hopefully) fix #18995.

This moves hardeningLDFlags to extraBefore, so e.g. user supplied -O flags should take precedence over the hardening flags. (EDIT: I'm actually not sure how I feel about this; see commentary below)

Also, this changes the wrappers such that they now expect a whitelist of hardening flags, instead of (predominately) expecting a blacklist. Instead of taking $hardeningEnable and $hardeningDisable from the environment, they now use $NIX_HARDENING_ENABLE instead. If the user has set hardeningEnable and or hardeningDisable in a call to mkDerivation, we pass along the appropriate NIX_HARDENING_ENABLE. stdenv/setup.sh now sets $NIX_HARDENING_ENABLE to the default enabled hardening flags if the variable is unset.

This latter change is desirable for a couple reasons:

Most wrapper configuration in Nix works via environment variables starting with NIX_; hardening{Enable,Disable} are the exceptions.
When gcc et al are installed system-wide, most users don't realize that they have to explicitely opt out of hardening, which results in plenty of confusion and time lost trying to figure out why/when things are mysteriously broken.

This should provide the same behavior as before, aside from fixing the unintuitive bits mentioned above.

Note that I left a bit of add-hardening.sh oddly indented; I wanted to keep the diff readable during review. I'll fix the indentation before we merge.

If it makes sense, I can move the derivation of $NIX_HARDENING_ENABLE from Nix to stdenv/setup.sh, so that setting hardening{Enable,Disable} from Nix in a regular old derivation will work just as well as from mkDerivation (right now, that'll only happen for the latter). Please let me know how feel about this, either way.

Feel free to ask me any questions, or point out any concerns.

~~I'm still testing this at the moment.~~ This now works as described. See comments below for any open questions.

Things done

Please check what applies. Note that these are not hard requirements but merely serve as information for reviewers.

Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

Ericson2314 · 2017-08-08T06:01:48Z

How is all handled now? (Granted looking on phone.)

CC @orivej

Ericson2314 · 2017-08-08T06:04:20Z

Nevermind did not scroll down enough. I like this: Nix > bash.

orivej · 2017-08-08T07:12:36Z

pkgs/stdenv/generic/setup.sh

@@ -3,6 +3,9 @@ set -o pipefail

 : ${outputs:=out}

+# If unset, assume the default hardening flags.
+: ${NIX_HARDENING_ENABLE="fortify stackprotector pic strictoverflow format relro bindnow"}


It seems that = instead of := works, but is it documented?

Yeah. Per the docs:

When not performing substring expansion, using the form described below (e.g., ‘:-’), Bash tests for a parameter that is unset or null. Omitting the colon results in a test only for a parameter that is unset. Put another way, if the colon is included, the operator tests for both parameter’s existence and that its value is not null; if the colon is omitted, the operator tests only for existence.

https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html#Shell-Parameter-Expansion

Interesting, this means we can write e.g. ${NIX_STORE-} instead of ${NIX_STORE:-}.

Perhaps this should go in the setup-hook for cc-wrapper? setup.sh isn't exactly a paragon of clean layering, but I wouldn't want to make it worse.

I could do that; do I then need to do something similar for bintools, though?

Yeah if we go with two separate env vars, then it is not a problem.

orivej · 2017-08-08T07:15:11Z

pkgs/stdenv/generic/make-derivation.nix

+
+      hardeningEnable =
+        let val = attrs.hardeningEnable or [ ];
+        in if builtins.isList val then val else [ val ];


This is implemented in lib.toList.

Nice find, thanks! I'll make that change shortly-ish.

orivej · 2017-08-08T07:20:21Z

Why do you move hardening args before command line args, now that hardening is not enabled outside nix-build environment? AFAIK programs often overspecify -O flags, and -O1 or -O0 will mess with hardening.

globin · 2017-08-08T08:18:11Z

Please don't merge yet, I want to go through this thoroughly and test if this still applies the hardening correctly (maybe add a checksecPhase using checksec)

cstrahan · 2017-08-08T10:54:16Z

Why do you move hardening args before command line args, now that hardening is not enabled outside nix-build environment? AFAIK programs often overspecify -O flags, and -O1 or -O0 will mess with hardening.

Assuming I interpreted the statements in #18995 correctly, some people were complaining about -O flags not working within builds, so I figured I'd try to address that.

As you point out, though, that does mean that the hardening would silently not apply if e.g. a Makefile sets -O. So, I'm thinking I should back out of that change, and within package definitions, people should just learn to unset the appropriate hardening flags if the target software requires, say, -O instead of -O2 to work (and, of course, we should document that as a potential gotcha in the user guide).

I think I'll make that change soonish (hopefully several people can approve of and validate that idea).

And thanks for reviewing this :)

cstrahan · 2017-08-08T11:05:44Z

I tried pretty hard to keep the behavior of hardening{Enable,Disable} the same as before. For example,

  hardeningEnable = [ "pie" ];
  hardeningDisable = [ "fortify" ];

should behave as before: add "pie" (which isn't/wasn't set by default), and remove "fortify" (which is/was otherwise set by default), while including all of the other default flags as before.

If there are any deviations that I'm missing, please do point them out.

cstrahan · 2017-08-08T11:43:57Z

/cc @Ralith

dezgeg · 2017-08-08T19:18:16Z

pkgs/stdenv/generic/make-derivation.nix

@@ -107,6 +123,8 @@ rec {
            ++ optional (elem "host"   configurePlatforms) "--host=${stdenv.hostPlatform.config}"
            ++ optional (elem "target" configurePlatforms) "--target=${stdenv.targetPlatform.config}";

+        } // lib.optionalAttrs (hardeningDisable != [] || hardeningEnable != []) {


IIRC doing this sort of stuff on the Nix level is somewhat discouraged since it slows down evaluation of Nixpkgs.

Well, more bash causes other problems too. I don't think its so clear cut.

Ralith · 2017-08-09T01:45:03Z

now that hardening is not enabled outside nix-build environment?

Does this change actually do that? I thought everyone was pretty much in agreement that nix-shell should always provide exactly the build environment, lest testing become a nightmare.

As you point out, though, that does mean that the hardening would silently not apply if e.g. a Makefile sets -O. So, I'm thinking I should back out of that change, and within package definitions, people should just learn to unset the appropriate hardening flags if the target software requires, say, -O instead of -O2 to work (and, of course, we should document that as a potential gotcha in the user guide).

Almost every single person who wants to do C/C++ development with Nix will (continue to) be badly bitten if you leave things the way they currently are in master and have been historically, user guide or no. Completely and silently breaking debug info unless you set an esoteric configuration flag that nobody outside of Nix has any context for is an incredibly bad user experience. If packages are broken, fix the packages. I'm not even convinced there's a significant proportion of packages that have -O0 or -O1 hardcoded in their release builds in the first place without a good reason (or at all, for that matter), and when there is a good reason, silently clobbering it represents a frustrating landmine for packagers.

To be very clear, this goes for both nix-shell environments and nix-build itself. I often need to build dependencies with debug info, and Nix should not sabotage that.

orivej · 2017-08-09T02:31:02Z

Does this change actually do that? I thought everyone was pretty much in agreement that nix-shell should always provide exactly the build environment, lest testing become a nightmare.

This change lets standalone gcc not add hardening flags, in which case it no longer matters to move them before command line arguments. nix-build and nix-shell continue to make gcc add them by default.

Could you clarify how you build packages and their dependencies with debug info? Do you add separateDebugInfo = true; or NIX_CFLAGS_COMPILE to their definitions, or pass extra flags to configure/make/cmake, or modify their sources to build with debug info by default?

Ralith · 2017-08-09T02:49:14Z

This change lets standalone gcc not add hardening flags, in which case it no longer matters to move them before command line arguments

I'm not sure who's making non-trivial use of gcc outside of a nix-shell environment (or an actual package build), considering that having dependencies is useful, and indeed that letting you manage multiple distinct build environments with nix-shell is one of Nix's largest selling points to the developer audience.

Could you clarify how you build packages and their dependencies with debug info? Do you add separateDebugInfo = true; or NIX_CFLAGS_COMPILE to their definitions, or pass extra flags to configure/make/cmake, or modify their sources to build with debug info by default?

When I'm working in nix-shell, i.e. compiling my project by invoking my build system by hand, I expect gcc -O0 -g to do what it does in every other environment in the world. When I want a debug build of a dependency, that usually means changing the dependency's build configuration (i.e. passing flags to configure or cmake) because there's often more to getting a "debug build" of a project than simply passing -g, such as (but not limited to) different sets of preprocessor definitions.

cstrahan · 2017-08-09T07:31:16Z

@Ralith This continues to provide hardening support for both nix-build nix-shell. Outside of either of those two, NIX_HARDENING_ENABLE env var won't be set, and thus system-wide gcc will no longer default to enabling hardening.

Regarding optimization and debug builds: I only see three options:

Allow hardening to silently fail because the user and/or build system passed -O
Silently override the user/build-system supplied -O with -O2 when hardening is enabled
When the user supplied flag conflicts with the hardening flag, have the wrapper print an informative error and exit non-zero

At the present moment, this implements number 1, though as noted earlier I'm interested in moving to either of the latter two options.

Ralith · 2017-08-09T17:35:47Z

Outside of either of those two, NIX_HARDENING_ENABLE env var won't be set, and thus system-wide gcc will no longer default to enabling hardening.

Right, but a system-wide gcc isn't very useful to anyone regardless.

At the present moment, this implements number 1, though as noted earlier I'm interested in moving to either of the latter two options.

Option 2 is what Nix has without this PR, and is why there is a long string of confused and unhappy developers in #18995, plus however many more who never managed to find the issue. It makes Nix hostile to developers who aren't already aware of the problem, and cost me personally a considerable amount of time, very nearly driving me to abandon Nix before I determined the cause.

3 would be tolerable; it's weird and nonstandard but you can't miss it, so it won't waste peoples' time the way 2 does, assuming the message is clear enough. It'd also make packages with broken builds obvious, which would clarify how common a problem that is.

bjornfor · 2017-08-09T20:19:50Z

Right, but a system-wide gcc isn't very useful to anyone regardless.

I think the point is that plain GCC provided by Nix should behave like any other GCC on any other distro. Injecting hardening flags is something that should be provided by the build environment, as environment variables or explicit command line options to the compiler.

Ralith · 2017-08-10T19:26:07Z

I don't think using gcc outside of a Nix build environment is a very important case, frankly. I'm happy to see it no longer be affected by the issue, but that doesn't make the issue any less serious for the case I actually use.
A large part of the appeal of Nix for development is its ability to construct pure, immutable, reproducible, independent per-project build environments for you to work in; relying on external, globally installed tools is contrary to that paradigm. If installing things globally and switching them out every time I needed something different worked for me I'd still be on Arch.

cstrahan · 2017-08-15T16:37:53Z

@fpletz @globin Ping.

As far as I'm concerned, the only thing this is waiting on is a decision on how to handle user supplied -O et al; see my comment here: #28029 (comment)

cstrahan · 2017-08-15T16:54:15Z

I don't know yet how I feel about this, but this popped into my head I figured I'd share it before I forget: I wonder if it might make sense to print the enabled hardening flags when first starting a nix-shell, and give a hint that they might want to tweak them with hardening{Disable,Enable}. The downside is that I personally don't like the thought of nix-shell spewing a bunch of text on each use, though that could be possibly addressed by having tools like nix-shell be verbose by default with some config mechanism to opt out. That's all very rough reasoning, and not directly related to this PR, but I figured I'd get that thought out in writing before I forget, FWIW.

Ericson2314 · 2017-08-15T18:44:05Z

pkgs/build-support/cc-wrapper/add-hardening.sh


-# Intentionally word-split in case 'hardeningDisable' is defined in Nix. The
+# Create table of unsupported flags for this toolchain.
+for flag in @hardening_unsupported_flags@; do


Do we still need the disabled map if mkDerivation converts everything to a whitelist? I rather have this filter the whitelist.

Hmm, upon further reading, maybe something like:

if [[ -n "$NIX_DEBUG" ]]; then declare -A hardeningDisableMap=() fi for flag in @hardening_unsupported_flags@; do [[ -n ${hardeningEnableMap[$flag]} ]] || continue if [[ -n "$NIX_DEBUG" ]]; then hardeningEnableMap[$flag]=1 fi unset hardeningEnableMap[$flag] done if [[ -n "$NIX_DEBUG" ]]; then // print disabled flags as before fi

Then it is clear that all the logic follows from the whitelist, and the disable map is only used for debugging.

Sounds good, will do 👍

Ericson2314 · 2017-08-15T18:52:32Z

pkgs/build-support/cc-wrapper/add-hardening.sh

-for flag in ${hardeningDisable[@]} @hardening_unsupported_flags@
-do
-  hardeningDisableMap[$flag]=1
+for flag in ${NIX_HARDENING_ENABLE-}; do


Could we do NIX_CC_WRAPPER_@infixSalt@_HARDENING_ENABLE? For the sake of clean laying I've wanted to use the NIX_CC_WRAPPER prefix instead, so might as well start with a new environment variable. add-flags.sh can take care of dealing with the @infixSalt@ part; it should be fairly easy to cargo cult.

What is infixSalt? I've never seen it before, but all of a sudden Nixpkgs has dozens of references to it...

@edolstra it's so we can have multiple cc-wrappers that don't conflict for cross compilation. I left a bunch of comments in https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/cc-wrapper/setup-hook.sh and https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/cc-wrapper/add-flags.sh

@Ericson2314 This still needs to be changed to NIX_@infixSalt@_HARDENING_ENABLE (under the assumption that we'll stick to the old name for now, rather than NIX_CC_WRAPPER_HARDENING_ENABLE), right?

I must confess that I haven't found the time to learn how your cross compiling changes work, though I hope to soon-ish. Of course "soon" for me probably translates to "in a couple eons" for most people, given my schedule as of late...

ElvishJerricco · 2018-01-19T19:18:56Z

As far as #28029 (comment) goes, I'm also in favor of either solution 1 or 3. I'd say that if the breakage in nixpkgs with 3 is minimal, that's better. But it's not worth the effort of going through and fixing hundreds of derivations, so if it comes to that, 1 is probably better.

cstrahan · 2018-01-26T01:05:07Z

pkgs/stdenv/generic/make-derivation.nix

@@ -107,6 +119,8 @@ rec {
            ++ optional (elem "host"   configurePlatforms) "--host=${stdenv.hostPlatform.config}"
            ++ optional (elem "target" configurePlatforms) "--target=${stdenv.targetPlatform.config}";

+        } // lib.optionalAttrs (hardeningDisable != [] || hardeningEnable != []) {
+          NIX_HARDENING_ENABLE = enabledHardeningOptions;


@Ericson2314 Regarding your comment regarding NIX_CC_WRAPPER_@infixSalt@_HARDENING_ENABLE, what's the appropriate thing to do here? Should I mostly leave this alone (the only change, perhaps, being a rename to NIX_CC_WRAPPER_HARDENING_ENABLE), and then have (per another one of your recommendations) the setupHook for the CC-wrapper use the $NIX_CC_WRAPPER_HARDENING_ENABLE when $NIX_CC_WRAPPER_@infixSalt@_HARDENING_ENABLE isn't defined? Curious what the exact logic should be here.

https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/cc-wrapper/add-flags.sh#L7-L13 in there goes NIX_CC_WRAPPER+HARDENING_ENABLE.

this line becomes NIX_CC_WRAPPER_HARDENING_ENABLE ..

Now that there's a bintools wrapper too, we would have to do an analogous thing there with this plan. So OTOH if we want just one variable with both, then maybe we shouldn't add the _CC_WRAPPER to the name. But, no matter what we still want to do the infix salt.

oxij · 2018-04-28T11:53:59Z

Okay, after some more investigation this is not as scary as I though. I mixed up i686 and x86_64 evaluations a bit, so

pkgs/development/libraries/a52dec/default.nix

only needs ``` hardeningDisable = stdenv.lib.optional stdenv.isi686 "pic"; # with the above removed # doCheck fails 1 out of 1 tests with "BAD GLOBAL SYMBOLS" ``` to pass (and that's likely a bug in the tests themselves) and ilmbase seems to be broken on i686 (which is kinda scary since wine depends on it), but it's not hardenings' fault. It is still the case that many packages broke with `-Wformat-security`, but I got sweating here thinking that format hardening could make a difference in tests. But it does not. Good. Then, I think, this PR needs no follow-ups in that regard.

FRidh · 2018-05-23T16:23:23Z

ghc on staging is broken and I think it is due to this change.
https://hydra.nixos.org/build/72807800

Please investigate this further as staging has been blocked now for over a month.

Ralith · 2018-05-23T20:42:58Z

cc1: error: -Wformat-security ignored without -Wformat [-Werror=format-security]
cc1: some warnings being treated as errors

Bad combination of flags, somehow?

oxij · 2018-05-24T01:19:34Z

GHC is not the only package that has this issue. I have a whole bunch of fixes for exactly that error message. Will PR in the next couple of days.

Ericson2314 · 2018-05-24T06:50:39Z

pkgs/build-support/bintools-wrapper/ld-wrapper.sh

@@ -57,8 +57,8 @@ fi

 source @out@/nix-support/add-hardening.sh

-extraAfter=("${hardeningLDFlags[@]}")
-extraBefore=()
+extraAfter=()


@oxij @FRidh if we do all of this PR but this part, we should have the same behavior as before. That seems like a good start to me

Sounds good.

I really need to go to sleep so feel free to try it.

Can the hardening flags be injected at the start instead of the end? I'd hate to see us reintroduce the problems that prompted this PR in the first place.

Hardening currently triggers warnings, and warnings cause the build to fail. So we hardcode `-w` in the CPP options.

ElvishJerricco · 2018-05-29T02:53:45Z

Can we please revert this on master until the many broken packages are fixed on staging?

oxij · 2018-05-29T03:43:18Z

I fixed everything I know of on master #41192

https://github.com/Clozure/ccl/blob/15d7e26377efa27eeb7086960f090f8c8ab5330e/lisp-kernel/linuxx8664/Makefile#L29

It moved our -O2 before their -O3, but -O3 inlines "checkValueVecEqual(&,&)" into "checkValueVecEqual(*,*)".

* master: (30 commits) bitlbee: fix build git: 2.17.0 -> 2.17.1 sc-controller: 0.4.2 -> 0.4.3 zeroc-ice: fix parallel building pythonPackages.bsddb3: fix build after 0fd461d haskellPackages.hlibgit2: fix build after #28029 gdk-pixbuf: patch library rpath references on darwin virtualbox: fix build after #28029 GHCJS darwin fixes (#41120) neovim-remote: 1.8.6 -> 2.0.5 messenger-for-desktop: remove (#41224) typora: gnome2 cleanup (#41167) discord: gnome2 cleanup (#41174) skypeforlinux: gnome2 cleanup (#41176) wire-desktop: gnome2 cleanup (#41155) hyper: gnome2 cleanup (#41170) drone: 0.5 -> 0.8.5 (#41200) racket: use proper uname, allow for unix sockets kytea: fix build after #28029 yoda: fix build after #28029 ...

cstrahan · 2018-05-31T18:34:26Z

The breakage is mostly, if not entirely, due to the change in the order in which the flags are inserted. If the breakage is too great to keep things as they are now, I would recommend committing the couple line diff needed to revert just that part of this change set. That would retain 99% of the benefits while fixing any currently broken packages.

When -O2 from hardening does not redefine -O3 from CMake, the build fails with: src/qpid/broker/SelectorExpression.cpp: In member function ‘qpid::broker::Expression* qpid::broker::Parse::orExpression(qpid::broker::Tokeniser&)’: src/qpid/broker/SelectorExpression.cpp:1041:13: error: ‘*((void*)& s +17)’ may be used uninitialized in this function [-Werror=maybe-uninitialized] if (s[1]=='b' || s[1]=='B') {

After #28029 it is necessary to add "strictoverflow" to the disabled hardening flags. That probably has something to do with the `-O3` option in palps makefile. This commit also adds a test to check for this regression (as it only occured at runtime).

cstrahan requested a review from Ericson2314 as a code owner August 8, 2017 03:45

FRidh requested a review from globin August 8, 2017 07:03

orivej reviewed Aug 8, 2017

View reviewed changes

globin requested a review from fpletz August 8, 2017 08:18

cstrahan added the 1.severity: mass-rebuild label Aug 8, 2017

dezgeg reviewed Aug 8, 2017

View reviewed changes

fpletz mentioned this pull request Aug 15, 2017

WIP: rewrite hardening in plain nix #19512

Closed

10 tasks

Ericson2314 reviewed Aug 15, 2017

View reviewed changes

bjornfor mentioned this pull request Oct 18, 2017

GNAT Invalid command line option, can't compile Ada code #30533

Closed

cstrahan mentioned this pull request Jan 18, 2018

GCC has unwanted flags #18995

Open

cstrahan commented Jan 26, 2018

View reviewed changes

Ericson2314 reviewed May 24, 2018

View reviewed changes

mboes added a commit to mboes/rules_go that referenced this pull request May 26, 2018

Workaround for NixOS/nixpkgs#28029

6a2b1f7

Hardening currently triggers warnings, and warnings cause the build to fail. So we hardcode `-w` in the CPP options.

oxij mentioned this pull request May 29, 2018

Build of ghc-8.0.2 is broken #41166

Closed

oxij mentioned this pull request May 29, 2018

remove some format hardenings #41192

Merged

1 task

aespinosa pushed a commit to aespinosa/nixpkgs that referenced this pull request May 29, 2018

ccl: fix build after NixOS#28029

06f1ae5

https://github.com/Clozure/ccl/blob/15d7e26377efa27eeb7086960f090f8c8ab5330e/lisp-kernel/linuxx8664/Makefile#L29

orivej added a commit that referenced this pull request May 29, 2018

cmake_2_8: fix build after #28029

83c98f3

orivej added a commit that referenced this pull request May 29, 2018

dadadodo: fix build after #28029

2c92213

orivej added a commit that referenced this pull request May 29, 2018

luaPackages.mpack: fix build after #28029

1e97008

orivej added a commit that referenced this pull request May 29, 2018

p0f: fix build after #28029

e27df9e

orivej added a commit that referenced this pull request May 29, 2018

pacparser: fix build after #28029

198c91d

orivej added a commit to orivej/nixpkgs that referenced this pull request May 29, 2018

yoda: fix build after NixOS#28029

842f1af

orivej added a commit to orivej/nixpkgs that referenced this pull request May 29, 2018

kytea: fix build after NixOS#28029

51af2c3

It moved our -O2 before their -O3, but -O3 inlines "checkValueVecEqual(&,&)" into "checkValueVecEqual(*,*)".

orivej added a commit that referenced this pull request May 29, 2018

virtualbox: fix build after #28029

7cadf50

orivej added a commit that referenced this pull request May 30, 2018

haskellPackages.hlibgit2: fix build after #28029

e4df80a

orivej added a commit that referenced this pull request May 30, 2018

ns-3: fix build after #28029

0159beb

orivej added a commit that referenced this pull request May 30, 2018

uade123: fix build after #28029

a94021e

timokau mentioned this pull request Jun 26, 2018

palp: fix runtime error introduced by #28029 #42633

Merged

9 tasks

orivej mentioned this pull request Jul 3, 2018

stdenv: enable colors #42932

Closed

9 tasks

nh2 mentioned this pull request May 4, 2019

fortify hardening still overrides debug builds in many cases #60919

Open

hardening: fix #18995 #28029

hardening: fix #18995 #28029

Conversation

cstrahan commented Aug 8, 2017 • edited

Motivation for this change

Things done

Ericson2314 commented Aug 8, 2017

Ericson2314 commented Aug 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orivej commented Aug 8, 2017 • edited

globin commented Aug 8, 2017

cstrahan commented Aug 8, 2017

cstrahan commented Aug 8, 2017

cstrahan commented Aug 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ralith commented Aug 9, 2017

orivej commented Aug 9, 2017

Ralith commented Aug 9, 2017

cstrahan commented Aug 9, 2017

Ralith commented Aug 9, 2017

bjornfor commented Aug 9, 2017

Ralith commented Aug 10, 2017

cstrahan commented Aug 15, 2017

cstrahan commented Aug 15, 2017

Choose a reason for hiding this comment

Ericson2314 Aug 15, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ElvishJerricco commented Jan 19, 2018

cstrahan Jan 26, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oxij commented Apr 28, 2018 via email

FRidh commented May 23, 2018

Ralith commented May 23, 2018

oxij commented May 24, 2018 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ericson2314 May 24, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ElvishJerricco commented May 29, 2018 • edited

oxij commented May 29, 2018

cstrahan commented May 31, 2018

cstrahan commented Aug 8, 2017 •

edited

orivej commented Aug 8, 2017 •

edited

Ericson2314 Aug 15, 2017 •

edited

cstrahan Jan 26, 2018 •

edited

Ericson2314 May 24, 2018 •

edited

ElvishJerricco commented May 29, 2018 •

edited