RFC: Create SecretBuffer and use it to help keep LibGit2's secrets #27565

mbauman · 2018-06-13T21:57:40Z

This PR is building upon @omus' #24738, transitioned to using an IO-like API instead of an AbstractString one. It definitely makes some things more awkward, but on the plus side, it also has made me vet and correct many places where we were still transitioning the secrets through Strings in LibGit2.

~~It has the major downside that the returned object from Base.prompt changes drastically depending upon the password keyword.~~ Edit: Base.prompt and Base.getpass are now detangled.

I initially tried following the approach in #24738, wherein all the credential information (including usernames and key paths) is considered secret. This proved to be horribly cumbersome and a huge dungeon of kludges and work-arounds. This PR now only considers the passwords themselves to be secret — and that works quite well.

omus

Fantastic work! You found some good solutions to some issues I was struggling with. The IO based SecureBuffer design worked out really well. I'll do some further testing yet.

omus · 2018-06-14T00:55:26Z

stdlib/LibGit2/src/types.jl

@@ -1180,7 +1180,7 @@ function objtype(obj_type::Consts.OBJECT)
    end
 end

-import Base.securezero!
+import Base.Base.shred!


Find replace issue?

omus · 2018-06-14T01:00:22Z

stdlib/LibGit2/src/gitcredential.jl

+            while !eof(io) && (c = read(io, UInt8)) != UInt8('\n')
+                write(value, c)
+            end
+            seek(value, 0)


Personal preference but I prefer seekstart

omus · 2018-06-14T01:01:44Z

stdlib/LibGit2/test/libgit2.jl

-        result = let
-            $code
+        result = open($input_code) do fp
+            eval(deserialize(fp))


Why is this change now necessary?

I'm not sure if it's still necessary, but it definitely was at one point and it seems like a good improvement. Before we were trusting that the string representation of a expression was round-trippable. This isn't just Exprs, it also includes embedded credential objects spliced directly into the AST. If their stringification isn't round-trippable, this fails.

omus · 2018-06-14T01:03:59Z

stdlib/LibGit2/test/libgit2.jl

@@ -318,27 +323,26 @@ end
        url = LibGit2.git_url(
            scheme="https",
            username="user",
-            password="pass",
+            # password="pass",


We should just remove these lines

omus · 2018-06-14T01:09:02Z

test/strings/secure.jl

@@ -0,0 +1,45 @@
+## Secure strings ##


This test file is out of date now. I appear to have forgotten to add this into the test suite...

Oh man, I totally missed this — thanks!

Here's a modernized version of this test suite:

using Base: SecretBuffer, SecretBuffer!, shred!, isshredded using Base.Test @testset "SecretBuffer" begin @testset "original unmodified" begin str = "foobar" secret = SecretBuffer(str) @test read(secret, String) == str seekstart(secret) @test shred!(secret) === secret @test read(secret, String) != "" @test str != "foobar" end @testset "finalizer" begin v = UInt8[1, 2] secret_a = SecretBuffer!(v) secret_b = secret_a secret_a = nothing GC.gc() @test all(iszero, v) @test !isshredded(secret_b) end end

omus · 2018-06-14T01:09:21Z

stdlib/LibGit2/test/libgit2.jl

@@ -2424,7 +2525,10 @@ mktempdir() do dir
            @test err == exhausted_error
            @test auth_attempts == 3
            @test p.explicit == invalid_cred
-            @test p.credential != invalid_cred
+            # @test p.credential != invalid_cred # TODO!!!


I'll try to look into this

These tests can be re-enabled if you set the non-secret fields of UserPasswordCredential and SSHCredential to "" in their respective shred! methods.

omus · 2018-06-14T01:11:45Z

stdlib/LibGit2/src/gitcredential.jl

+GitCredential(cred::UserPasswordCredential, url::AbstractString) = parse(GitCredential, url)
+
+Base.:(==)(c1::GitCredential, c2::GitCredential) = (c1.protocol, c1.host, c1.path, c1.username, c1.password, c1.use_http_path) ==
+                                                   (c2.protocol, c2.host, c2.path, c2.username, c2.password, c2.use_http_path)


My personal preference is to use == and && over multiple lines

omus · 2018-06-14T01:12:50Z

base/secretbuffer.jl

+
+Initialize a new `SecretBuffer` with `data` and securely zero the original source argument.
+"""
+SecretBuffer!(s::Cstring) = SecretBuffer!(convert(Ptr{UInt8}, s))


Great use of !. Highly approve!

omus · 2018-06-14T01:19:23Z

I initially tried following the approach in #24738, wherein all the credential information (including usernames and key paths) is considered secret. This proved to be horribly cumbersome and a huge dungeon of kludges and work-arounds. This PR now only considers the passwords themselves to be secret — and that works quite well.

I totally agree. I have seen these dungeons you speak of and dare not speak of them myself.

omus · 2018-06-14T01:26:58Z

CI failing due to:

Some tests did not pass: 74 passed, 0 failed, 2 errored, 0 broken.OldPkg/pkg: Error During Test at /tmp/julia/share/julia/stdlib/v0.7/OldPkg/test/pkg.jl:303
  Test threw exception MethodError(Base.securezero!, (LibGit2.CachedCredentials(Dict{String,LibGit2.AbstractCredential}()),), 0x00006b7f)
  Expression: OldPkg.update()
  MethodError: no method matching securezero!(::LibGit2.CachedCredentials)
  Closest candidates are:
    securezero!(!Matched::Cstring) at util.jl:444
    securezero!(!Matched::AbstractArray{#s620,N} where N where #s620<:Number) at util.jl:443

vtjnash · 2018-06-14T01:32:07Z

base/secretbuffer.jl

+
+An IOBuffer-like object where the contents will be securely wiped when garbage collected. However, it is
+considered best practice to wipe the buffer using `Base.shred!(::SecretBuffer)` as soon as the
+secure data are no longer required. Avoid initializing with converting to strings as they are


"Avoid initializing with converting..." reads poorly. Perhaps first mention that they can be initialized, then add the note to avoid String second?

Missed an "and" there — I was intending: "Avoid initializing with and converting to". Good point about swapping ordering here.

vtjnash · 2018-06-14T01:33:09Z

base/secretbuffer.jl

+convert(::Type{SecretBuffer}, s::AbstractString) = SecretBuffer(String(s))
+SecretBuffer(str::AbstractString) = SecretBuffer(String(str))
+function SecretBuffer(str::String)
+    buf = unsafe_wrap(Vector{UInt8}, str)


Use codepoints

vtjnash · 2018-06-14T01:33:58Z

base/secretbuffer.jl

+Initialize a new `SecretBuffer` with `data` and securely zero the original source argument.
+"""
+SecretBuffer!(s::Cstring) = SecretBuffer!(convert(Ptr{UInt8}, s))
+function SecretBuffer!(p::Ptr{UInt8})


should be called unsafe_SecretBuffer!, calling our normal naming convention, and should take a length argument (with a default value of calling :strlen)

Ugh, that's ugly. But I see your point.

Definitely ugly

vtjnash · 2018-06-14T01:35:00Z

base/secretbuffer.jl

+function SecretBuffer!(d::Vector{UInt8})
+    s = SecretBuffer(sizehint=length(d))
+    for i in 1:len
+        write(s, unsafe_load(p, i))


p isn't defined. missing test coverage?

vtjnash · 2018-06-14T01:37:36Z

base/secretbuffer.jl

+end
+
+function unsafe_convert(::Type{Ptr{UInt8}}, s::SecretBuffer)
+    # Add a hidden nul byte just past the end of the valid region


That's usually only applicable for conversion to ::Type{Cstring} (and also should check that there are no embedded \nul values)

Ah, yes, that'd be much better — I'll change this to a conversion to Cstring and fixup the accompanying ccalls.

vtjnash · 2018-06-14T01:54:26Z

stdlib/LibGit2/src/gitcredential.jl

-    cred.username !== nothing && securezero!(cred.username)
-    cred.password !== nothing && securezero!(cred.password)
+function Base.shred!(cred::GitCredential)
+    cred.protocol !== nothing && (cred.protocol = nothing)


the check for nothing is unnecessary

vtjnash · 2018-06-14T01:55:19Z

stdlib/LibGit2/src/gitcredential.jl

+    a.host = b.host
+    a.path = b.path
+    a.username = b.username
+    a.password = b.password


Seems like this still should be copy(b.password)

vtjnash · 2018-06-14T01:57:10Z

stdlib/LibGit2/src/gitcredential.jl

        if key == "url"
            # Any components which are missing from the URL will be set to empty
            # https://git-scm.com/docs/git-credential#git-credential-codeurlcode
+            Base.shred!(cred)


Seems like this can be part of the copy! definition

In this case yes but the copy! method could be used for duplication where you do not want to destroy the source.

Isn't cred the destination?

Yes it is. I had some wires crossed. Your suggestion is good 👍

I had to go check myself too. That's perhaps a good point too though, that we should destroy the source argument? Maybe take!(dest, src) for that, or just do it manually.

vtjnash · 2018-06-14T01:59:17Z

stdlib/LibGit2/src/types.jl

-        c = new(user, pass)
-        finalizer(securezero!, c)
-        return c
+    user::AbstractString


keep String

vtjnash · 2018-06-14T01:59:27Z

stdlib/LibGit2/src/types.jl

-        c = new(user, pass, prvkey, pubkey)
-        finalizer(securezero!, c)
-        return c
+    user::AbstractString


keep as String

vtjnash · 2018-06-14T02:04:44Z

base/secretbuffer.jl

+end
+
+function final_shred!(s::SecretBuffer)
+    if !isshredded(s)


it would likely be cheaper and faster to call shred! unconditionally

Agreed. Originally this code issued a warning to alert developers that they weren't explicitly calling shred!

omus · 2018-06-14T01:42:58Z

base/secretbuffer.jl

+
+An IOBuffer-like object where the contents will be securely wiped when garbage collected. However, it is
+considered best practice to wipe the buffer using `Base.shred!(::SecretBuffer)` as soon as the
+secure data are no longer required. Avoid initializing with converting to strings as they are


"secure data is no longer required"

omus · 2018-06-14T02:01:58Z

base/secretbuffer.jl

+end
+function SecretBuffer!(d::Vector{UInt8})
+    s = SecretBuffer(sizehint=length(d))
+    for i in 1:len


len not defined

omus · 2018-06-14T16:36:56Z

stdlib/LibGit2/src/gitcredential.jl

-        key, value = split(readline(io), '=')
-
+        key = readuntil(io, '=')
+        if key == "password"


We may also want to do this if key == "url" since that URL could contain a password

We still parse that URL with a regex which stashes things into Strings. I decided to just consider URLs insecure, but perhaps we should re-write that, too.

That approach makes sense. I think we should add a comment which calls the potential security issue and leave it at that. The "url" key doesn't seem commonly used so it isn't a big concern for me.

Any ideas as to where this would most make sense to document? Another alternative would just be to completely not support passwords in URLs. We'd just throw an error if m[:password] exists. I kinda like that.

I think it makes sense to open an issue about this and reference the issue below if key == "url". I believe we can address the issue later by rewriting the URL parsing and reading in the password with a SecretBuffer. We can try to tackle this now but I don't want to add extra work for you.

Raising an error will still result in the password information being stored in a string which is no better than what we currently have.

mbauman · 2018-06-14T17:04:23Z

Thanks for the very thorough reviews Jameson and Curtis!

mbauman · 2018-06-15T22:30:38Z

I somewhat cannot believe that this passes tests on Windows as I've done all the win-specific changes blindly. Is there reasonable test coverage that I can trust that? Or should I hunt down a windows VM to do some extra interactive testing myself?

Keno · 2018-06-16T01:32:31Z

I usually call things good enough if tests pass, but ping me on slack for access to a Windows test VM if you want.

staticfloat

You're a beast. Looks great to me.

staticfloat · 2018-06-16T07:07:27Z

base/secretbuffer.jl

+
+convert(::Type{SecretBuffer}, s::AbstractString) = SecretBuffer(String(s))
+SecretBuffer(str::AbstractString) = SecretBuffer(String(str))
+function SecretBuffer(str::String)


I think it bears creating a docstring for the ::String constructor that mentions that the string will not be zeroed out.

staticfloat · 2018-06-16T07:08:40Z

base/secretbuffer.jl

+"""
+    SecretBuffer!(data::Vector{UInt8})
+
+Initialize a new `SecretBuffer` with `data` and securely zero the original source argument.


The way this is phrased makes it sound like data and "the original source argument" are two different things? Perhaps rephrase to something like "Initialize a new SecretBuffer from data, securely zeroing it afterwards."

staticfloat · 2018-06-16T07:14:01Z

stdlib/LibGit2/src/gitcredential.jl

+    a.host = b.host
+    a.path = b.path
+    a.username = b.username
+    a.password = b.password == nothing ? nothing : copy(b.password)


It seems like copy(nothing) should just return nothing. :P

chethega · 2018-06-18T17:23:06Z

base/secretbuffer.jl

+show(io::IO, s::SecretBuffer) = print(io, "SecretBuffer(\"*******\")")
+
+hash(s::SecretBuffer, h::UInt) = hash(SecretBuffer, hash((s.data, s.size, s.ptr), h))
+==(s1::SecretBuffer, s2::SecretBuffer) = (view(s1.data, 1:s1.size), s1.ptr) == (view(s2.data, 1:s2.size), s2.ptr)


I know that secretbuffer is meant for uninitialized memory disclosure scenarios and the following does not matter for use in libgit2. Nevertheless, I think it would be good practice to make comparison constant time for something called secretbuffer. Somebody will eventually misuse it to compare a secret to an attacker-supplied string and leak the secret via timing side-channel.

A simple sample implementation could be (no early return, no branching, no conditional moves, @noinline to prevent the compiler from being clever):

@noinline function _bufcmp(data1::Vector{UInt8}, data2::Vector{UInt8}, sz::Int) res = UInt8(0) for i = 1:sz res |= xor(data1[i], data2[i]) end return res end ==(s1::SecretBuffer, s2::SecretBuffer) = (s1.ptr == s2.ptr) && (s1.size == s2.size) && (UInt8(0) == _bufcmp(s1.data, s2.data, s1.size))

That's a good point. These methods are also just strange in the first place. They break with the convention for other IO subtypes — those don't do equality by value. Plus I just now noticed there's a bug in the agreement between hash and ==.

I'd love to figure out how to remove them, but they are used both in LibGit2's source and its tests and I don't have a good replacement.

Keno · 2018-06-19T22:29:35Z

What's missing to merge this?

stevengj · 2018-06-20T15:30:39Z

It has the major downside that the returned object from Base.prompt changes drastically depending upon the password keyword.

Maybe promptpassword (getpass?) should just be a separate function?

stevengj · 2018-06-20T15:32:14Z

base/secretbuffer.jl

+    return byte
+end
+
+final_shred!(s::SecretBuffer) = shred!(s)


Shouldn't it log a warning or something if !isshredded(s)? Waiting until gc runs seems like a bad idea.

iblislin · 2018-06-21T06:27:19Z

FreeBSD CI got a segfault. seems related.
(starts from line 97: https://freebsdci.julialang.org/#/builders/1/builds/10478/steps/8/logs/stdio

martinholters · 2018-06-21T07:22:13Z

Ref. #27702 regarding likely cause of the segfault.

Originally used `isequal` to deal with `Nullable`

Move lower in the bootstrap order to allow `@warn`.

mbauman · 2018-06-21T16:58:38Z

Ok, once CI gives this the green-light I think it's good to go. I believe I've addressed all review comments now.

stevengj · 2018-06-21T17:01:52Z

base/util.jl

+object containing the secret.
+
+Note that on Windows, the secret might be displayed as it is typed; see
+`Base.winprompt` for securely retrieving username/password pairs from a secure


winprompt seems misnamed now… should it be wingetpass?

(also, the word "secure" appears in this sentence twice, which seems a bit redundant)

It returns a username/password pair. It's more like wingetcredentials or some such. But these are all un-exported functions and can be changed down the line. I'm inclined to leave it be.

stevengj · 2018-06-21T17:02:45Z

base/util.jl

@@ -467,36 +479,30 @@ function getpass(prompt::AbstractString)
 end
 else
 function getpass(prompt::AbstractString)


Should prompt have a default value, e.g. "Password"?

It can always be added later if someone else feels strongly about it.

stevengj · 2018-06-21T17:03:14Z

base/util.jl

 if Sys.iswindows()
 function getpass(prompt::AbstractString)
-    print(prompt)
+    print(prompt, ':')


The extra space there doesn't matter all that much because (ideally) the typed input doesn't get echo'ed back. In any case, this was required to match Base.prompt's behavior, which doesn't use a space either.

The system might still show a blinking cursor of some kind. On MacOS it displays a key cursor

I think that Base.prompt should also show a space, as it looks even uglier to omit the space when the input is not hidden.

stevengj · 2018-06-21T17:09:32Z

It would be nice to have something analogous to the Python Keyring library, but I guess that could be an external package.

omus · 2018-06-21T17:43:40Z

These CachedCredentials uses in Pkg3 should be shredded:

julia/stdlib/Pkg/src/Operations.jl

Line 433 in c1eb3e8

creds = LibGit2.CachedCredentials()

julia/stdlib/Pkg/src/Types.jl

Line 420 in c1eb3e8

creds = LibGit2.CachedCredentials()

julia/stdlib/Pkg/src/Types.jl

Line 488 in c1eb3e8

creds = LibGit2.CachedCredentials()

julia/stdlib/Pkg/src/Types.jl

Line 793 in c1eb3e8

creds = LibGit2.CachedCredentials()

mbauman · 2018-06-21T18:15:29Z

Oh interesting; perhaps I should add a warning if those credential wrapper objects are shredded by the finalizer, too? That's why SecretString isn't warning here — the secrets they hold get explicitly shredded by a different finalizer, so when their finalizer runs all looks well.

Edit: no, that's wrong — none of the LibGit2 credential objects have finalizers. Perhaps our test suite just doesn't exercise those methods with repos that require passwords?

stevengj · 2018-06-21T18:57:52Z

base/util.jl

@@ -459,7 +459,7 @@ function getpass end

 if Sys.iswindows()
 function getpass(prompt::AbstractString)
-    print(prompt, ':')
+    print(prompt, ": ")


Honestly, it seems a bit weird to me that we add a colon at all. Can't we let the caller decide this? For example, what if they want to display a prompt "True (y/n)? " or "Enter password here => "?

It all stems from the behavior of Base.prompt, where you can also specify a default, which will appear as message [default]:.

omus · 2018-06-22T05:22:56Z

I would guess that the Pkg tests do not target repos requiring passwords. In that case the CachedCredentials instance will contain no secrets and no finalizer warning would appear. If a credential was used it would be cached and then secret's finalizer would warn us about the unshredded secret.

mbauman · 2018-06-22T19:25:53Z

There we go, finally some green checks are starting to roll in from CI. I propose we merge this as it stands so Keno can finally tag a beta. Base.prompt, Base.getpass, and Base.winprompt are all documented-but-unexported APIs, and we can bikeshed them to our hearts delight in a different PR. The main point here is to get secure handling correct and avoid the segfaults that have plagued the buildbots.

ararslan · 2018-06-25T23:57:36Z

This appears to have caused an issue with using credential helpers. I think @omus is investigating.

ararslan · 2018-06-26T00:01:55Z

stdlib/LibGit2/src/utils.jl

@@ -107,6 +111,11 @@ provided the URL produced will use the alternative [scp-like syntax](https://git
    provided. Cannot be specified when using the scp-like syntax.
  * `path::AbstractString=""`: the path to use in the output if provided.

+!!!warning


This needs a space between ! and warning for it to be recognized correctly. Currently it shows up literally as !!!warning in the docstring rather than being rendered nicely.

ararslan requested a review from omus June 13, 2018 23:37

ararslan added libgit2 The libgit2 library or the LibGit2 stdlib module security System security concerns and vulnerabilities labels Jun 13, 2018

Keno added this to the 0.7 milestone Jun 13, 2018

omus reviewed Jun 14, 2018

View reviewed changes

vtjnash reviewed Jun 14, 2018

View reviewed changes

omus reviewed Jun 14, 2018

View reviewed changes

Keno mentioned this pull request Jun 16, 2018

WIP: Hilbert/Banach space structures #27401

Merged

staticfloat approved these changes Jun 16, 2018

View reviewed changes

chethega reviewed Jun 18, 2018

View reviewed changes

omus mentioned this pull request Jun 20, 2018

WIP: RFC: Create type SecureString #24738

Closed

stevengj reviewed Jun 20, 2018

View reviewed changes

Keno modified the milestones: 0.7, 0.7-beta Jun 21, 2018

omus and others added 5 commits June 21, 2018 11:05

Create type SecureString

41d87d5

Switch GitCredential equality to use ==

8abc616

Originally used `isequal` to deal with `Nullable`

Rename to SecretBuffer

16e6523

Rename file to reflect new name

e35a635

Updates from review

cafe850

mbauman added 3 commits June 21, 2018 11:05

Document SecretBuffer(::AbstractString)

dff0faa

Add warning about storing passwords in URLs to git_url

34c6271

Warn if not shred!ed before hitting finalizer

3e8960f

Move lower in the bootstrap order to allow `@warn`.

mbauman force-pushed the mb+cv/secretbuffer branch from bba1513 to 3e8960f Compare June 21, 2018 15:39

mbauman added 2 commits June 21, 2018 12:48

Detangle Base.prompt and Base.getpass

a5b1baa

Fixup SecretBuffer! docstrong from review

9ffda78

stevengj reviewed Jun 21, 2018

View reviewed changes

Add a space after : in prompts

5836427

stevengj reviewed Jun 21, 2018

View reviewed changes

mbauman force-pushed the mb+cv/secretbuffer branch from 0645771 to c9b9cb8 Compare June 21, 2018 21:48

mbauman force-pushed the mb+cv/secretbuffer branch 2 times, most recently from 2358901 to 9ab9f63 Compare June 22, 2018 07:37

mbauman added 3 commits June 22, 2018 11:32

Have shred!(f, x) return what f(x) returns

3c13230

Shred! creds used in Pkg

a927bbb

remove redundant secure

5a50d9b

mbauman force-pushed the mb+cv/secretbuffer branch from 9ab9f63 to 5a50d9b Compare June 22, 2018 15:32

Keno merged commit 160fc7d into master Jun 23, 2018

StefanKarpinski deleted the mb+cv/secretbuffer branch June 23, 2018 01:21

ararslan reviewed Jun 26, 2018

View reviewed changes

This was referenced Jun 26, 2018

Address some missed shred calls #27801

Merged

Copy SecretBuffer to avoid inadvertent shred #27802

Merged

RFC: Create SecretBuffer and use it to help keep LibGit2's secrets #27565

RFC: Create SecretBuffer and use it to help keep LibGit2's secrets #27565

Conversation

mbauman commented Jun 13, 2018 • edited Loading

omus left a comment • edited Loading

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

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

omus commented Jun 14, 2018 • edited Loading

omus commented Jun 14, 2018

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

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

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

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

mbauman commented Jun 14, 2018

mbauman commented Jun 15, 2018

Keno commented Jun 16, 2018

staticfloat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

staticfloat Jun 16, 2018 • edited by ararslan Loading

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

Keno commented Jun 19, 2018

stevengj commented Jun 20, 2018 • edited Loading

stevengj Jun 20, 2018 • edited Loading

Choose a reason for hiding this comment

iblislin commented Jun 21, 2018

martinholters commented Jun 21, 2018

mbauman commented Jun 21, 2018

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

mbauman Jun 21, 2018 • edited Loading

Choose a reason for hiding this comment

stevengj Jun 21, 2018 • edited Loading

Choose a reason for hiding this comment

stevengj commented Jun 21, 2018 • edited Loading

omus commented Jun 21, 2018 • edited Loading

mbauman commented Jun 21, 2018 • edited Loading

mbauman commented Jun 13, 2018 •

edited

Loading

omus left a comment •

edited

Loading

omus commented Jun 14, 2018 •

edited

Loading

staticfloat Jun 16, 2018 •

edited by ararslan

Loading

stevengj commented Jun 20, 2018 •

edited

Loading

stevengj Jun 20, 2018 •

edited

Loading

mbauman Jun 21, 2018 •

edited

Loading

stevengj Jun 21, 2018 •

edited

Loading

stevengj commented Jun 21, 2018 •

edited

Loading

omus commented Jun 21, 2018 •

edited

Loading

mbauman commented Jun 21, 2018 •

edited

Loading

stevengj Jun 21, 2018 •

edited

Loading

omus commented Jun 22, 2018 •

edited

Loading