AesGcm, AesCcm #31389

krwq · 2018-07-26T17:15:17Z

Fixes: https://github.com/dotnet/corefx/issues/23629
Low level functions for AES-GCM, AES-CCM

API shape (equivalent for AesCcm)

    public sealed class AesGcm : IDisposable
    {
        public AesGcm(ReadOnlySpan<byte> key) { }
        public AesGcm(byte[] key) { }
        public static KeySizes TagByteSizes { get => throw null; }
        public static KeySizes NonceByteSizes { get => throw null; }
        public void Encrypt(ReadOnlySpan<byte> nonce, ReadOnlySpan<byte> plaintext, Span<byte> ciphertext, Span<byte> tag, ReadOnlySpan<byte> associatedData = default) => throw null;
        public void Encrypt(byte[] nonce, byte[] plaintext, byte[] ciphertext, byte[] tag, byte[] associatedData = null) => throw null;
        public void Decrypt(ReadOnlySpan<byte> nonce, ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> tag, Span<byte> plaintext, ReadOnlySpan<byte> associatedData = default) => throw null;
        public void Decrypt(byte[] nonce, byte[] ciphertext, byte[] tag, byte[] plaintext, byte[] associatedData = default) => throw null;
        public void Dispose() { }
    }

    public sealed class AesCcm : IDisposable { /* same as gcm */ }

Design notes:

There is no good API design which would generalize Authenticated Encryption algorithms – each algorithm has different properties and pitfalls – for now “bare bones” implementation
Even though AES-GCM does allow streaming most of the people use and will use it incorrectly – we won’t support that – users should encrypt multiple times instead
Nonce is required by AES-GCM/AES-CCM - there is a known (unfortunately not by everyone) pitfall of using same nonce with the same key more than once – we considered abstraction which would make it less likely to happen but does not completely prevent it in some scenarios (abstracting it did not have good response in the issue 😄)

vcsjones · 2018-07-26T19:24:49Z

~~~It would be nice if this had platform specific types and AesG/Ccm delegated to them depending on the platform. For example, a AesGcmCng and an AesGcmOpenSsl (similar to the design of ECDsa).~~

It would be nice if the macOS platform used the same implementation from Unix and optionally "light up" macOS of they happen to have a suitable version of OpenSSL installed.

The motivation for this is so that there is something that is workable in macOS. I realize OpenSSL is not exactly in-the-box on macOS (aside from an ancient version and an fork of it as libressl). However, for aiding in local development, it would be useful to be able to use it if I happen to have it installed with homebrew or however I get it installed (similar to how things worked in the 1.x days).

Tornhoof · 2018-07-26T19:29:36Z

src/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/OpenSslCipher.cs

-                CheckBoolReturn(Interop.Crypto.EvpCipherFinalEx(_ctx, outputCurrent, out bytesWritten));
-                outputBytes += bytesWritten;
-            }
+            Span<byte> outputSpan = new Span<byte>(output).Slice(outputBytes);


output.AsSpan(outputBytes)

Tornhoof · 2018-07-26T19:30:35Z

src/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/OpenSslCipher.cs

-                    inputCurrent,
-                    count);
-            }
+            ReadOnlySpan<byte> inputSpan = new ReadOnlySpan<byte>(input).Slice(inputOffset);


input.AsSpan(inputOffset)

Tornhoof · 2018-07-26T19:30:53Z

src/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/OpenSslCipher.cs

-                    count);
-            }
+            ReadOnlySpan<byte> inputSpan = new ReadOnlySpan<byte>(input).Slice(inputOffset);
+            Span<byte> outputSpan = new Span<byte>(output).Slice(outputOffset);


output.AsSpan(outputOffset)

Tornhoof · 2018-07-26T19:44:10Z

src/Common/src/Interop/Windows/BCrypt/Cng.cs

                }
-                Buffer.BlockCopy(key, 0, blob, sizeof(BCRYPT_KEY_DATA_BLOB_HEADER), keySize);
+
+                key.CopyTo(blob.AsSpan().Slice(sizeof(BCRYPT_KEY_DATA_BLOB_HEADER)));


AsSpan(sizeof(BCRYPT_KEY_DATA_BLOB_HEADER))

Please revisit other uses of new Span/Slice/AsSpan as necessary.

krwq · 2018-07-26T19:47:56Z

src/System.Security.Cryptography.Algorithms/ref/System.Security.Cryptography.Algorithms.cs

        public static new System.Security.Cryptography.Aes Create() { throw null; }
        public static new System.Security.Cryptography.Aes Create(string algorithmName) { throw null; }
    }
+    public class AesGcm : IDisposable


i think these should be sealed

krwq · 2018-07-26T20:12:41Z

Eh, seems like CCM might not work on Ubuntu 14.04 - per openssl release notes it seems to have been added between OpenSSL 1.0.2h and OpenSSL 1.1.0 and 14.04 seems to be on 1.0.1. ~~@bartonjs how do we handle such cases? OpenSSL version check and PNSE if lower than supported? (do we already have version checks anywhere?)~~

Seems like we already do that in some places:

corefx/src/Common/tests/System/Net/SslProtocolSupport.cs

Line 24 in 3cc6fda

    
           (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) && PlatformDetection.OpenSslVersion < new Version(1, 0, 2) && !PlatformDetection.IsDebian))

krwq · 2018-07-26T20:18:41Z

@vcsjones - I personally think this would be unnecessarily complicated but do not have strong opinion about it. As for openssl on OSX - considering I'll likely need to add version checks I think it might be possible to make it check for presence of OpenSSL and make it work on OSX if OpenSSL with appropriate version is installed. Not sure if this will happen in this PR or later though.

vcsjones · 2018-07-26T20:28:37Z

@krwq

I personally think this would be unnecessarily complicated but do not have strong opinion about it.

That's fine, you could also just remove the macos exclusion here. https://github.com/dotnet/corefx/pull/31389/files#diff-f5613604087291d4cee8422733c29366R383

Since ubuntu Ubuntu 14.04 might be forcing you to implement a openssl version check, you may get that for macOS for "free" as well too.

do we already have version checks anywhere?

See and it's usages

corefx/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSslVersion.cs

Line 17 in 711c428

internal static Version OpenSslVersion

bartonjs · 2018-07-26T17:20:25Z

src/Common/src/Interop/Windows/BCrypt/Cng.cs

                }
-                Buffer.BlockCopy(key, 0, blob, sizeof(BCRYPT_KEY_DATA_BLOB_HEADER), keySize);
+
+                key.CopyTo(blob.AsSpan().Slice(sizeof(BCRYPT_KEY_DATA_BLOB_HEADER)));


Instead of AsSpan().Slice( you should be able to just do AsSpan(

bartonjs · 2018-07-26T17:27:31Z

src/Common/src/Microsoft/Win32/SafeHandles/SafeKeyHandle.cs

+    /// <summary>
+    ///     Copies bytes and holds them as native blob
+    /// </summary>
+    internal unsafe sealed class SafeKeyHandle : SafeHandle


I don't know that this really belongs in Common. And nothing about it really makes it "a key", other than usage. It's really SafeExternalizedDataHandle.

Assuming (from hallway discussions) that this is just for Linux CCM, just store it as byte[], don't bother with moving it to native memory (unless there's a persistence requirement for this pointer in OpenSSL, but I'm pretty sure they copy the data, not hold the pointer)

The only concern with byte[] is that it can be moved around in memory

Yep. But there's an outstanding feature request to get GC compaction to clean up after itself. And we already do it with the Aes class.

bartonjs · 2018-07-26T17:33:26Z

...ommon/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCipherTests.Data.cs

+
+            public override string ToString()
+            {
+                return $"{Source}; Id={CaseId}; KeyLen={BitLength(Key)}; NonceLen={BitLength(Nonce)}; PTLen={BitLength(Plaintext)}; AADLen={BitLength(AssociatedData)}; TagLen={BitLength(Tag)}";


This should just be Source and ID. The purpose of this class is to make the test log easier to read. The places that are varying on nonce length should rename their source to take that into account.

The motivation here was that some NIST test vectors have duplicate Count = X in the same file - not sure how to rename the source to account for that (except for doing it as above except with hard-coded string)

ok nvm, explained later

bartonjs · 2018-07-26T17:39:33Z

...ommon/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCipherTests.Data.cs

+            // cases 17, 18 have not supported nonce size
+        };
+
+        // [ CaseId; Key.Length; Nonce.Length; Plaintext.Length; AssociatedData.Length; Tag.Length ] is unique


I'd like to see the Source take into account the file and a short form of the section, just to keep the test log easier to read.

Something like Source = $"{NistGcmTestVectors} - gcmEncryptExtV128 - 128/96/0/0/128",, then the Count= item would be the CaseId here.

bartonjs · 2018-07-26T17:42:58Z

...ommon/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCipherTests.Data.cs

+        }
+
+        // CaseId is unique per test case
+        private static AEADTest[] s_nistGcmSpecTestCases = new AEADTest[]


The field can be declared readonly. This applies to the other case-sets, too.

bartonjs · 2018-07-26T20:57:22Z

src/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/AesGcm.Windows.cs

+    {
+        private static readonly SafeAlgorithmHandle s_aesGcm = AesBCryptModes.OpenAesAlgorithm(Cng.BCRYPT_CHAIN_MODE_GCM);
+        private SafeKeyHandle _keyHandle;
+        public static KeySizes TagByteSizes { get; } = AesAEAD.GetTagLengths(s_aesGcm);


Why bother asking the OS for the answer we already know? I'd just make this be defined as a literal in the AnyOS version.

I think this is implementation-specific

IIRC, Windows only allows a single size here, and we locked the OpenSSL/Unix one to the single Windows value.

@bartonjs I was wondering if this should be hard coded or "always ask OS" - will change to hard-coded as it probably won't change in the future

bartonjs · 2018-07-26T20:58:07Z

src/System.Security.Cryptography.Algorithms/tests/AesCcmTests.cs

+            const int datalength = 32;
+            byte[] originalData = Enumerable.Range(1, datalength).Select((x) => (byte)x).ToArray();
+            byte[] nonce = Convert.FromBase64String("tBMp3WSvLDA2ZhtG");
+            byte[] key = Convert.FromBase64String("1aGU7ZDP4Iq+zUaRmXzrLA==");


Most of the rest of crypto uses hex.

bartonjs · 2018-07-26T21:01:07Z

src/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/AesCcm.cs

+
+            if (!AesAEAD.MatchesKeySizes(tagSizeInBytes, TagByteSizes))
+            {
+                throw new CryptographicException(SR.Cryptography_InvalidTagLength);


These feel like ArgumentException

bartonjs · 2018-07-26T21:01:33Z

src/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/AesGcm.Unix.cs

+            AesAEAD.CheckKeySize(keyLength);
+
+            _ctxHandle = Interop.Crypto.EvpCipherCreatePartial(
+                        GetCipher(keyLength),


This seems overly indented

bartonjs · 2018-07-26T21:02:59Z

src/System.Security.Cryptography.Algorithms/tests/AesCcmTests.cs

+        public static void NullKey()
+        {
+            Assert.Throws<ArgumentNullException>(() => new AesCcm((byte[])null));
+            Assert.Throws<ArgumentNullException>(() => new AesCcm((ReadOnlySpan<byte>)null));


The span ctor should not throw an ArgumentNullException.

krwq · 2018-07-26T21:18:57Z

@vcsjones I think we can make OSX work but I currently don't have clean OSX to test scenarios where openssl >= 1.0.0 is not present (CIs have it preinstalled). I think I'll add version detection to non-OSX for now and will leave the issue open to fix it later (or maybe someone could help testing this). We need to make sure we get PNSE and not any other exception in such cases

vcsjones · 2018-07-26T21:25:44Z

or maybe someone could help testing this

Happy to help if you want to go that direction. I have a 2012 MBP that has a bare 10.12 installation. I will make sure not to install openssl on it :-).

krwq · 2018-07-26T21:30:29Z

@vcsjones thank you for offering your help!

vcsjones · 2018-07-26T21:31:44Z

In the interest of moving the PR along, I'm fine with going in with macOS throwing PNSE in this PR. I can open a separate issue once this is merged to have macOS use OpenSSL if present with the version check and do that myself since I'm the one that wants it, assuming there are no other objections to it. 😊

bartonjs · 2018-07-26T21:34:01Z

On Linux we have a requirement that OpenSSL be found, so getting a TypeInitializationException due to the... TargetInvocationException(?) is acceptable.

On macOS we should really do a PNSE, since it's an optional component.

If we let Ubuntu 14.04 and macOS both do PNSE:

internal static class AeadAvailability
{
    internal static readonly bool IsSupported = CheckSupport();

    private static void CheckSupport()
    {
        try
        {
            if (Interop.Crypto.HoweverWeCheckOpenSslVersions > Whatever)
            {
                return true;
            }
        }
        catch (TypeInitializationException)
        {
        }

        return false;
    }
}

I think.

The intermediate type is needed to keep the TypeInitializationException from Interop.Crypto from cascading.

bartonjs · 2018-07-26T21:44:16Z

Though instead of checking versions it might make more sense to just add a shim method to answer the question.

Assuming that Aes128Gcm cipher (and friends) weren't defined in the version that Ubuntu 14.04 has, they'd need to be moved to non-required portable bind, and you can make a detector method return API_EXISTS(EVP_aes_128_gcm)

See the handling for EC_GF2m_simple_method for how this will need to work.

morganbr · 2018-07-26T21:45:35Z

src/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/AesCcm.Unix.cs

+                }
+                else
+                {
+                    byte nonEmptyArray = 123;


I'm not sure I understand what this block is for... is it to handle a 0 length ciphertext that still has associated data? If so, I don't understand why we're passing in this value

yes, will add comment

morganbr · 2018-07-26T21:47:54Z

src/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/AesCcm.Unix.cs

+                {
+                    if (!Interop.Crypto.EvpCipherUpdate(ctx, ref MemoryMarshal.GetReference(plaintext), out plaintextBytesWritten, ref MemoryMarshal.GetReference(ciphertext), ciphertext.Length))
+                    {
+                        throw new CryptographicException(SR.Cryptography_AuthTagMismatch);


EvpCipherUpdate doesn't check the tag (it can't since you might add more data). This should have a different message.

Actually for CCM data is fixed and known upfront and EvpCipherUpdate does check the tag (what you said is true for GCM though - CCM always fails when calling *FinalEx)

Do any of the tests feed ciphertext that is too long or too short into decrypt? If not, please add those test cases.

bartonjs · 2018-07-26T21:48:23Z

src/System.Security.Cryptography.Algorithms/tests/AesCcmTests.cs

+                byte[] nonce = new byte[AesCcm.NonceByteSizes.MinSize];
+                byte[] tag = new byte[AesCcm.TagByteSizes.MinSize];
+                rng.GetBytes(key);
+                rng.GetBytes(nonce);


RandomNumberGenerator.Fill(key) lets you get rid of the using.

bartonjs · 2018-07-26T21:49:15Z

src/System.Security.Cryptography.Algorithms/tests/AesCcmTests.cs

+        [InlineData(41)]
+        [InlineData(48)]
+        [InlineData(50)]
+        public static void EncryptDecryptRoundtripNoAAD(int dataLength)


What is this adding over the NIST cases? (If it's adding something I'm happy to have it, but I'm not seeing what it is)

for gcm probably not much - will remove - for ccm - nist test cases are not great (most of them have nonce size we don't support) and haven't seen any cases without AAD (will remove those which don't add value)

morganbr · 2018-07-26T21:51:20Z

src/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/AesGcm.Unix.cs

+            }
+            else
+            {
+                byte nonEmptyArray = 123;


Do we even need to support 0-length plaintext? What's the case for just producing authenticated data with AES?

I think for something like TLS that's "I'm sending an empty keepalive frame"

@morganbr mixed feeling about supporting vs non-supporting this scenario - on one hand there is probably no good scenarios where you should do that over i.e. calculating hash on the other hand it might make sense if for whatever reason you got such data over the wire (or the other end expected it) and would like to validate it. Since this is a low-level API I do not think we should not support it

bartonjs · 2018-07-26T21:51:52Z

src/System.Security.Cryptography.Algorithms/tests/AesCcmTests.cs

+                byte[] key = new byte[keyLength];
+                rng.GetBytes(key);
+
+                Assert.Throws<CryptographicException>(() => new AesCcm(key));


Since this is "your input is bad" it should be something based on ArgumentException

morganbr · 2018-07-26T21:52:31Z

src/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/AesGcm.Unix.cs

+
+                if (!Interop.Crypto.EvpCipherFinalEx(_ctxHandle, ref MemoryMarshal.GetReference(plaintext.Slice(plaintextBytesWritten)), out int bytesWritten))
+                {
+                    throw Interop.Crypto.CreateOpenSslCryptographicException();


Shouldn't this have the invalid tag string?

yes, thanks for spotting

morganbr · 2018-07-26T22:00:59Z

src/System.Security.Cryptography.Algorithms/tests/AesGcmTests.cs

+                {
+                    aesGcm.Encrypt(nonce, plaintext, ciphertext, tag);
+
+                    int tamperedIdx = r.Next() % tag.Length;


Since this is random, it should log the tag on failure

bartonjs · 2018-07-26T22:49:06Z

src/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/AesCcm.Unix.cs

+            ref byte nullRef = ref MemoryMarshal.GetReference(Span<byte>.Empty);
+
+            int keyLength = _keyHandle.Key.Length * 8;
+            using (SafeEvpCipherCtxHandle ctx = Interop.Crypto.EvpCipherCreatePartial(


A comment is probably warranted for why CCM opens a new key handle every time but GCM doesn't

Update: I guess you had one up at the field. Nevermind.

krwq · 2018-08-06T17:59:17Z

had to rebase after merge conflict

karelz · 2018-08-15T16:18:00Z

@krwq we do not use api-ready-for-review on PRs. Please create a new issue (or reuse existing one) with the API proposal.

krwq · 2018-08-21T21:55:27Z

API got approved today, squashing and merging...

* AesGcm, AesCcm * add osx stubs * disable tests on osx and desktop (APIs not there/not supported) * TagByteSizes on OSX * fix TagByteSizes compilation error * apply review feedback * fix typo when setting tag/nonce length in ccm * add missing SetCcmTagLength * attempt to detect if ccm is available on Ubuntu 14.04 * disable tests on ubuntu 14.04, attempt for osx support * attempt to fix osx * fix osx * review feedback * disable new ccm testcases on ubuntu 14.04 * attempt to make Ubuntu 14.04 work * add Interop.Initialization to OSX * fix KeySizeHelpers after merge conflict Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>

pgolebiowski · 2018-08-21T22:44:18Z

Tears in my eyes. Thank you, @krwq! ❤️

* AesGcm, AesCcm * add osx stubs * disable tests on osx and desktop (APIs not there/not supported) * TagByteSizes on OSX * fix TagByteSizes compilation error * apply review feedback * fix typo when setting tag/nonce length in ccm * add missing SetCcmTagLength * attempt to detect if ccm is available on Ubuntu 14.04 * disable tests on ubuntu 14.04, attempt for osx support * attempt to fix osx * fix osx * review feedback * disable new ccm testcases on ubuntu 14.04 * attempt to make Ubuntu 14.04 work * add Interop.Initialization to OSX * fix KeySizeHelpers after merge conflict Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>

pgolebiowski · 2018-10-20T23:30:35Z

Why 3.0 and not 2.2 if it's already merged?

Reference: https://github.com/dotnet/core/blob/master/roadmap.md.

@karelz

karelz · 2018-10-22T16:28:48Z

@pgolebiowski master flows automatically only into 3.0. .NET Core 2.2 is more servicing release for lower layers (CoreCLR/CoreFx). There is more churn (incl. automatic flow from master branch) in higher levels (e.g. ASP.NET Core).

pgolebiowski · 2018-10-22T16:35:42Z

Thank you for the clarification, sir.

oliverjanik · 2019-07-30T07:44:58Z

Anyway this can be used from the old .NET Framework?

karelz · 2019-07-30T09:00:06Z

No, changes in Core don't flow into .NET Framework. Too much risk of breaking something.

* AesGcm, AesCcm * add osx stubs * disable tests on osx and desktop (APIs not there/not supported) * TagByteSizes on OSX * fix TagByteSizes compilation error * apply review feedback * fix typo when setting tag/nonce length in ccm * add missing SetCcmTagLength * attempt to detect if ccm is available on Ubuntu 14.04 * disable tests on ubuntu 14.04, attempt for osx support * attempt to fix osx * fix osx * review feedback * disable new ccm testcases on ubuntu 14.04 * attempt to make Ubuntu 14.04 work * add Interop.Initialization to OSX * fix KeySizeHelpers after merge conflict Commit migrated from dotnet/corefx@831dc11

krwq requested a review from bartonjs July 26, 2018 17:15

krwq added * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Jul 26, 2018

krwq requested review from GrabYourPitchforks and morganbr July 26, 2018 18:41

Tornhoof reviewed Jul 26, 2018

View reviewed changes

krwq commented Jul 26, 2018

View reviewed changes

bartonjs reviewed Jul 26, 2018

View reviewed changes

morganbr reviewed Jul 26, 2018

View reviewed changes

bartonjs reviewed Jul 26, 2018

View reviewed changes

morganbr reviewed Jul 26, 2018

View reviewed changes

bartonjs reviewed Jul 26, 2018

View reviewed changes

morganbr reviewed Jul 26, 2018

View reviewed changes

bartonjs reviewed Jul 26, 2018

View reviewed changes

krwq force-pushed the aes-gcm,ccm-final branch from e61107d to 298aa72 Compare August 6, 2018 17:58

karelz added area-System.Security blocked Issue/PR is blocked on something - see comments and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Aug 15, 2018

karelz assigned krwq Aug 15, 2018

krwq removed * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) blocked Issue/PR is blocked on something - see comments labels Aug 21, 2018

krwq merged commit 831dc11 into dotnet:master Aug 21, 2018

karelz added this to the 3.0 milestone Sep 6, 2018

dvsekhvalnov mentioned this pull request Dec 24, 2018

Linux support dvsekhvalnov/jose-jwt#45

Closed

lewurm mentioned this pull request Feb 1, 2019

[2018-08] Bump corert mono/mono#12721

Merged

bartonjs mentioned this pull request Jul 3, 2019

Consider adding AesGcm to .NET Standard 2.1 dotnet/standard#1295

Closed

mcblair mentioned this pull request Aug 5, 2019

AmazonS3EncryptionClient cannot decrypt 'AES/GCM/NoPadding' SES messages aws/aws-sdk-net#748

Closed

This was referenced Jan 31, 2020

Why not stream based AES-GCM? dotnet/runtime#27348

Closed

Add MemoryLockedBytes for critical pieces of data dotnet/runtime#27405

Closed

AesGcm, AesCcm #31389

AesGcm, AesCcm #31389

Uh oh!

Conversation

krwq commented Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vcsjones commented Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Tornhoof Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tornhoof Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tornhoof Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krwq Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krwq commented Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krwq commented Jul 26, 2018

Uh oh!

vcsjones commented Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krwq commented Jul 26, 2018

Uh oh!

vcsjones commented Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krwq commented Jul 26, 2018

Uh oh!

vcsjones commented Jul 26, 2018

Uh oh!

bartonjs commented Jul 26, 2018

krwq commented Jul 26, 2018 •

edited

Loading

vcsjones commented Jul 26, 2018 •

edited

Loading

Tornhoof Jul 26, 2018 •

edited

Loading

Tornhoof Jul 26, 2018 •

edited

Loading

Tornhoof Jul 26, 2018 •

edited

Loading

krwq Jul 26, 2018 •

edited

Loading

krwq commented Jul 26, 2018 •

edited

Loading

vcsjones commented Jul 26, 2018 •

edited

Loading

vcsjones commented Jul 26, 2018 •

edited

Loading

krwq Jul 26, 2018 •

edited

Loading

krwq Jul 26, 2018 •

edited

Loading